commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Heger <oliver.he...@oliver-heger.de>
Subject Re: [configuration] 1.10 regression / backwards-incompatible change in MapConfiguration.convertPropertiesToMap ?
Date Wed, 13 Jan 2016 10:41:18 GMT
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: user-unsubscribe@commons.apache.org
> For additional commands, e-mail: user-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@commons.apache.org
For additional commands, e-mail: user-help@commons.apache.org


Mime
View raw message