hi anatole, i guess everybody agrees that overriding based on ordinals wouldn't be enough for every possible use-case. back then we had a long discussion about it. we ended up with adding a simple (and optional) ConfigFilter interface.
@aggregation: aggregation of key/value pairs is supported implicitly by an ordinal based approach. aggregation of values >per key< can be done manually with an own config-source (which delegates to the others and only aggregates those results). yes - that way this use-case is more complicated out-of-the-box. however, imo if users (think they) need such a complicated config, they should also need to do a bit more for it. (i guess in several cases they might question the requirement before they continue...) in general about possible use-cases (and features): we can never address everything out-of-the-box. a nice example is bean-validation. i could list a lot of cases which aren't possible with that api/spi out-of-the-box, however, for the majority it's good enough. i saw several users who couldn't replace their existing validation-approach (as it was). however, several of them just simplified their validation-logic and were still happy about the result. others extended bv e.g. via the spi and only few kept what they used before (e.g. because it wasn't possible with bv or the workaround was too complicated and they insisted on what they had). regards, gerhard 2014-12-27 10:56 GMT+01:00 Anatole Tresch <[email protected]>: > *See inline ...* > > Mark Struberg <[email protected]> schrieb am Sa., 27. Dez. 2014 um 09:58: > > > > *In the existing code there was an "internal" interface called > > PropertySource > > > > That one is ok. I don't get the DEFAULT (not needed imo) and I don't like > > the tight coupling to the typing (toConfiguration). I would completely > > detach the configuration aspect (String/String) from the conversion > aspect. > > Actually having some sane String representation for converting values is > a > > feature which is desperately needed in many other places as well. > > > > *Basically application/user code bde fault simply calls > Configuration.current() to access the current Configuration. But for > special cases, additional isolation is required. In CDI this is done with > qualifiers. But qualifiers also have disadvantages:* > *1) it duplicates logic from CDI* > *2) Qualifiers are code, so they are not as easy portable over the network > as the rest of the config* > > *That's why I suggested to take simple, old styled, ugly names to be able > to have multiple configurations in place. E.g. a third party product > assembled as part can have its own configuration, a EJB container > configuration for admin resources and the config used to setup CDI during > boot time may also have different names (some of them should be defined of > course).* > > > In the implementation I also don't like that for each and every operation > > this currently does > > PropertyAdapterProviderSpi as = > ServiceContext.getInstance().getSingleton( > > PropertyAdapterProviderSpi.class); > > over and over again. This really slows down the operation. > > Imo it should be more like BeanManager. To have a getInstance() method > but > > it's perfectly fine to store this reference once and after that just use > it > > without having to go through the expensive parts over and over again. > > > > > *-> +1 That is exactly how it was written before the discussion around > ServiceContext;) Romain thaught he wanted to have a ServiceContext, which > is doing the caching so the singleton does not hold any reference to its > spi, which currently is the case. The overhead currently that can be > avoided is the Map access on each call.* > *I am happy to change that back to how it was originally ;)* > > > > > > > Aggregation gets as simple as a simple > > > > > BiFunction<String,String,String> > > > > If you have a hammer... > > I don't see why we need the aggregation to be a function at all? There is > > one single fixed algorithm: Take the ordinal value of each PropertySource > > (oh, this is missing btw! Please read up on that trick, this is actually > > the CORE of the whole mechanism.) and apply all of them on top of each > > other. Effectively merging them - with the more important values > 'winning'. > > > > *Because:* > *1) Aggregation IS a function (a mapping function)* > *2) because your ordinal system is much too constraint (this is IMO YOUR > hammer ;) ), it only supports override. But overriding os only one of > possible several strategies how things can be combined. Sometimes I want > also to add additional functionality during the mapping, e.g. for auditing > purposes, so a fixed ordinal system is completely insufficient IMO.* > *3) Its much easier to understand and to test, I think.* > > > > > > *- one convenient accessor for each basic wrapper type like short, int, > > > boolean etc.* > > > > That's fine, but it must not be bound to the configuration. See my > > arguments above. > > > > *??? Where when not the configuration (not the property source ;) ). That > is the location, where users expect it, and it should be as transparent as > possible, with the option of tweaking it for special cases.... perhaps an > example would help here to clarify your idea...* > > > > *- one generic accessor where no adapter was passed. In this case > > > PropertyAdapter itself provides a service interface, where you can > access > > > one by calling PropertyAdapter.getInstance(Class<T> type).* > > > > I like the idea of having an extensible converter system. > > > > I've not looked at this in detail but from doing a few unit tests with it > > it feels a slight bit clumsy yet (too many method calls involved). I bet > > together we can find a solution which is as flexible but more elegant. > But > > have not wrapped around my head about this topic. > > > > *So I propose with live with it. If someone else has a better idea, we > can take that up again. We have enough other topics to discuss ;)* > > *Best* > *Anatole* > > > > > > LieGrue, > > strub > > > > >
