Yes, makes sense, so I would refine my proposal to: - Create a ConfigurationContextBuilder. - We could hereby access a new ConfigurationContextBuilder from the ConfigurationProvider. - Extend the ConfigurationProvider with the switch(ConfigurationContext) method.
2015-02-24 14:11 GMT+01:00 Romain Manni-Bucau <[email protected]>: > Hmm > > Personally I'd prefer a builder (+ a clear lifecycle of the app build > phase) and a switch(ConfigContext) method to mutate it at runtime > build behing handled by the builder. switch would just be responsible > to 1) check the new config if needed, 2) fire the event(s) of the > update 3) do the real mutation > > said otherwise builder extracts the config of the config in its own > part and decorelate it from runtime > > wdyt? > > > > > > Romain Manni-Bucau > @rmannibucau > http://www.tomitribe.com > http://rmannibucau.wordpress.com > https://github.com/rmannibucau > > > 2015-02-24 14:01 GMT+01:00 Anatole Tresch <[email protected]>: > > See inline... > > > > 2015-02-24 13:39 GMT+01:00 Romain Manni-Bucau <[email protected]>: > > > >> Ok so ConfigurationContextUpdates is "part of the SPI" thanks to > >> ConfigurationContext itself, am I right? > >> > > Yep. Also adding property sources already was there... > > > > > > > > > >> That said seeing it I really think we should just use a builder where > >> custom extensions (ServiceLoader in SE, events in CDI, getBeanOfType > >> in Spring) can add/remove components during "build" phase. > >> > > > > The problem is that configuration changes during runtime exactly change > > these things. E.g. > > > > - 1) > > a file could be removed -> the according propertysource > > ( > > s > > ) > > must be removed. > > - 2) a file could be added -> additional property source(s) must be > > registered. > > > > - 3) a file was updated -> coud simply be an update of the > > propertysource, but it could also mean that the ordinal has been > changed, > > so the whole chain must be reordered. In case a file produces entries > with > > different ordinals, multiple propertysources may be affected, mappping > > logically to 1 and 2 again. > > > > So if we agree (and many people want that) that configuration can change > > during runtime, we must allow this. > > If we want to prevent that we can simply never return a > > ConfigurationContextUpdate instance, everything will stay read-only. > > Breaking things basically is still a question of adding good default > > entries. Of course, if someone also removes the defaults, things may get > > out of order, but I assume this is not a common case... > > > > Doing everything with a bulder is possible. We would then add according > > methods > > > > ConfigurationContextBuilder getBuilder(); > > void apply(ConfigurationContext context); > > > > to the ConfigurationContext. WDYT? > > > > > > > >> Why I worry about it is this kind of thing: > >> > >> 1) start a context > >> 2) start to use the config > >> 3) wow this is awesome > >> 4) a part of the app modify it > >> 5) oops 2 and 3 are broken > >> > >> builder creating immutable objects sounds far better to me (by itself, > >> then you can still mutate sources/converters if that's what you want > >> but that's not what we provide) > >> > >> > >> > >> > >> > >> Romain Manni-Bucau > >> @rmannibucau > >> http://www.tomitribe.com > >> http://rmannibucau.wordpress.com > >> https://github.com/rmannibucau > >> > >> > >> 2015-02-24 13:24 GMT+01:00 Anatole Tresch <[email protected]>: > >> > Hi Roamin > >> > > >> > not sure, if I get your concerns: > >> > > >> > - the ConfigurationContext already is part of the current SPI. > >> > > >> > - the only change is that I extracted the methods for changing the > >> context > >> > into a separate process step/artifact with a fluent API style to have > >> more > >> > control on changes applied. And I added additional functions, so I > also > >> can > >> > remove PropertySources, and have similar mechanisms for filters and > the > >> > other aspects. From what I wanted to achieve, currently only > >> PropertySource > >> > management per se, would be enough. But for me it would look somehow > >> > weired, if I can change PropertySources, but not the rest... > >> > > >> > Anatole > >> > > >> > > >> > > >> > > >> > 2015-02-24 11:57 GMT+01:00 Romain Manni-Bucau <[email protected] > >: > >> > > >> >> Hmm > >> >> > >> >> I have to admit I'm a bit lost, is the SPI useful then? Do we need > >> >> another spi? Maybe to start we shouldnt use any spi then move to spi > >> >> once API is finished > >> >> > >> >> That said this update new class looks like a builder to get context > >> >> immutable which sounds more common and easier to understand to me > >> >> > >> >> > >> >> > >> >> Romain Manni-Bucau > >> >> @rmannibucau > >> >> http://www.tomitribe.com > >> >> http://rmannibucau.wordpress.com > >> >> https://github.com/rmannibucau > >> >> > >> >> > >> >> 2015-02-24 10:30 GMT+01:00 Anatole Tresch <[email protected]>: > >> >> > Hi all > >> >> > > >> >> > I would like to propose a small but effective change in the > >> >> > ConfigurationContext SPI (the explanation why comes later in this > >> email). > >> >> > Currently > >> >> > it is defined as: > >> >> > > >> >> > *public interface *ConfigurationContext { > >> >> > > >> >> > *void *addPropertySources(PropertySource... propertySourcesToAdd); > >> >> > List<PropertySource> getPropertySources(); > >> >> > <T> *void *addPropertyConverter(TypeLiteral<T> typeToConvert, > >> >> > PropertyConverter<T> propertyConverter); > >> >> > <T> List<PropertyConverter<T>> getPropertyConverters(TypeLiteral<T> > >> >> type); > >> >> > List<PropertyFilter> getPropertyFilters(); > >> >> > PropertyValueCombinationPolicy getPropertyValueCombinationPolicy(); > >> >> > > >> >> > } > >> >> > > >> >> > My proposal is add an additional ConfigurationContextUpdates > >> interface, > >> >> > that allows to apply multiple changes to a ConfigurationContext and > >> >> finally > >> >> > apply the changes, once they are done. So the interface would be > >> changed > >> >> as > >> >> > follows: > >> >> > > >> >> > *public interface *ConfigurationContext { > >> >> > > >> >> > List<PropertySource> getPropertySources(); > >> >> > <T> List<PropertyConverter<T>> getPropertyConverters(TypeLiteral<T> > >> >> type); > >> >> > List<PropertyFilter> getPropertyFilters(); > >> >> > PropertyValueCombinationPolicy getPropertyValueCombinationPolicy(); > >> >> > > >> >> > // moved methods: > >> >> > *// void *addPropertySources(PropertySource... > propertySourcesToAdd); > >> >> > // <T> *void *addPropertyConverter(TypeLiteral<T> typeToConvert, > >> >> > // PropertyConverter<T> > >> propertyConverter); > >> >> > *ConfigurationContextUpdates startUpdate(); // new* > >> >> > > >> >> > } > >> >> > > >> >> > ConfigurationContextUpdates would be defined as follows: > >> >> > > >> >> > *public interface *ConfigurationContextUpdates { > >> >> > > >> >> > ConfigurationContext getContext(); > >> >> > > >> >> > *default *ConfigurationContextUpdates > >> >> > addPropertySources(PropertySource... propertySourcesToAdd); > >> >> > ConfigurationContextUpdates > >> >> > addPropertySources(Collection<PropertySource> > propertySourcesToAdd); > >> >> > *default *ConfigurationContextUpdates > >> >> > removePropertySources(PropertySource... propertySourcesToRemove); > >> >> > ConfigurationContextUpdates > >> >> > removePropertySources(Collection<PropertySource> > >> >> propertySourcesToRemove); > >> >> > * default *ConfigurationContextUpdates > >> >> > removePropertySources(Predicate<PropertySource> selector); > >> >> > > >> >> > *default *ConfigurationContextUpdates > >> >> > addPropertyFilters(PropertyFilter... filters); > >> >> > ConfigurationContextUpdates > >> >> > addPropertyFilters(Collection<PropertyFilter> filters); > >> >> > *default *ConfigurationContextUpdates > >> >> > removePropertyFilters(PropertyFilter... filters); > >> >> > *default *ConfigurationContextUpdates > >> >> > removePropertyFilters(Predicate<PropertyFilter> selector); > >> >> > ConfigurationContextUpdates > >> >> > removePropertyFilters(Collection<PropertyFilter> filters); > >> >> > > >> >> > > >> >> > <T> ConfigurationContextUpdates > >> addPropertyConverter(TypeLiteral<T> > >> >> > typeToConvert, > >> >> > > >> >> > PropertyConverter<T> propertyConverter); > >> >> > *default *ConfigurationContextUpdates > >> >> > removePropertyConverters(PropertyConverter<?>... converters); > >> >> > ConfigurationContextUpdates > >> >> > removePropertyConverters(Collection<PropertyConverter<?>> > converters); > >> >> > > >> >> > ConfigurationContextUpdates > >> >> > setPropertyValueCombinationPolicy(PropertyValueCombinationPolicy > >> policy); > >> >> > > >> >> > /** > >> >> > * Apply all the changes to the underlying context. > >> >> > * @throws java.lang.IllegalStateException if the operation is > >> called > >> >> > multiple times, or another update was already > >> >> > * applied before. > >> >> > */ > >> >> > *void *apply(); > >> >> > } > >> >> > > >> >> > Now here are the reasons, why I think this makes sense: > >> >> > > >> >> > - it is much more easy to lock/synchronize the current state of > a > >> >> > ConfigurationContext, since only for the time where the apply() > is > >> >> > running special synchronization logi > >> >> > - a configuration context can not support being mutable at all > by > >> >> simply > >> >> > throwing a UnsupportedMethodException, when startUpdate() is > >> called. > >> >> > - Changing a ConfigurationContext, e.g. for testing can now be > as > >> >> > flexible as using CDIUnit for CDI. The regarding test support > >> >> flexibility > >> >> > is easy to achieve. > >> >> > - *But most of all (and that was the reason, why I started to > >> think on > >> >> > this enhancements), we can implement automatic configuration > >> updates, > >> >> e.g. > >> >> > based on new files added to a configuration directory or added > to a > >> >> > database table, by implementing the mechanism as part of an > "event" > >> >> module. > >> >> > The event listener can determine from the change event the > affected > >> >> > PropertySource, compare with the PropertySource already > registered > >> in > >> >> the > >> >> > context(s) and remove/add/update new PropertySources as needed > for > >> the > >> >> > affected contexts. *This module currently is in development, > and as > >> >> soon > >> >> > as we would agree on this proposal I can add (and test it > before), > >> so > >> >> we > >> >> > have an initial version for supporting configuration updates. > >> >> > > >> >> > WDYT? > >> >> > > >> >> > Anatole > >> >> > >> > > >> > > >> > > >> > -- > >> > *Anatole Tresch* > >> > Java Engineer & Architect, JSR Spec Lead > >> > Glärnischweg 10 > >> > CH - 8620 Wetzikon > >> > > >> > *Switzerland, Europe Zurich, GMT+1* > >> > *Twitter: @atsticks* > >> > *Blogs: **http://javaremarkables.blogspot.ch/ > >> > <http://javaremarkables.blogspot.ch/>* > >> > > >> > *Google: atsticksMobile +41-76 344 62 79* > >> > > > > > > > > -- > > *Anatole Tresch* > > Java Engineer & Architect, JSR Spec Lead > > Glärnischweg 10 > > CH - 8620 Wetzikon > > > > *Switzerland, Europe Zurich, GMT+1* > > *Twitter: @atsticks* > > *Blogs: **http://javaremarkables.blogspot.ch/ > > <http://javaremarkables.blogspot.ch/>* > > > > *Google: atsticksMobile +41-76 344 62 79* > -- *Anatole Tresch* Java Engineer & Architect, JSR Spec Lead Glärnischweg 10 CH - 8620 Wetzikon *Switzerland, Europe Zurich, GMT+1* *Twitter: @atsticks* *Blogs: **http://javaremarkables.blogspot.ch/ <http://javaremarkables.blogspot.ch/>* *Google: atsticksMobile +41-76 344 62 79*
