Hi Norbert, due to lack of time, I recently only focused on Configuration 2.0 and intended to let the 1.x series slowly die. Therefore, my priority is to get 2.0 ready and push the release out. If I understand correctly, the implementation in 2.0 satisfies your needs, except that some generic types still have to be adapted (passing a Map<String, ?> to the constructor rather than a Map<String, Object>). Is this correct?
Patches for a 1.11 fix release are of course welcome, but I cannot promise that I will be able to actually do a 1.11 release in the near future. If somebody else steps up and volunteers to do this, this would of course be another story. Some more comments below. Am 13.01.2016 um 08:16 schrieb Norbert Kiesel: > Hi Oliver, > >> >> Hi Norbert, >> >> Am 11.01.2016 um 23:37 schrieb Norbert Kiesel: >>> Attached is a patch with an extended unit test (which the patched code >>> passes, but the current 1.10 fails). >>> >>> Just using the 2.0 code does not work, because >>> - the 1.10 API uses Map<String, Object> and not Map<String, String> >>> - the 1.10 MapConfiguration(Properties props) constructor mandates that >>> the implementation has to ignore property keys which are not strings. >>> >> >> The goal should be that the 2.0 code satisfies all requirements. If this >> is not the case, we should adapt it. It seems that there have been >> slight API changes in the constructors of MapConfiguration in the 1.10 >> release which have not been merged back to the main branch. This should >> be aligned. > > The last 3 implementations differ significantly: > - 1.9 copies Properties entries with String keys into a new map (similar to > my > patch) > - 1.10 creates an immutable map backed by the Properties object (limited to > elements with String key) > - 2.0 simply adopts the Properties object (and says in constructor > description > that it expects all keys to be strings) > > The problem with 1.10 is that this breaks it's public API (e.g. setProperty > and > clearProperty inherited from AbstractConfiguration), while claiming to be > binary > compatible with 1.9. > >> >>> So instead the patch simply creates a new HashMap from the Properties >>> entries with String keys. >>> >>> However, this makes TestSystemConfigurationRegression fail now. I >>> understand why, but I don't understand why this is a valid test case. >>> >>> TestSystemConfigurationRegression assumes that a SystemConfiguration >>> instances changes when a System property is added. The old code did that, >>> and the patched code does not. However, MapConfiguration(Properties props) >>> javadoc for 1.10 clearly states: "The resulting configuration is not >>> connected to the Properties object". So why is this a valid test case? >> >> There have been multiple iterations with SystemConfiguration regarding >> life-access to the underlying system properties. I think, most >> developers expect that this configuration is just a thin wrapper over >> System.getProperties(). Hence, updates of system properties should be >> immediately reflected by the configuration. The fact that >> SystemConfiguration extends MapConfiguration is merely an implementation >> detail; this does not mean that it only represents a snapshot of system >> properties at creation time. > > The way out for a potential 1.11 would be to override more of the the > AbstractMap API to make that a mutable map backed by the Properties object. > Do > you want me to provide a patch along these lines? This approach would probably work, but it seems like unnecessary complexity. Accessing the passed in Properties object directly - as done in 2.0 - is more straight-forward, isn't it? Oliver > Confidentiality Notice:This email and any files transmitted with it are > confidential and intended solely for the use of the individual or entity to > whom they are addressed. This message contains confidential information and > is intended only for the individual named. If you are not the named addressee > you should not disseminate, distribute or copy this e-mail. Please notify the > sender immediately by e-mail if you have received this e-mail by mistake and > delete this e-mail from your system. If you are not the intended recipient > you are notified that disclosing, copying, distributing or taking any action > in reliance on the contents of this information is strictly prohibited > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
