On Wed, 15 Sep 2021 15:59:53 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> - Clarify how overriden Properties#entrySet() method impacts the order of
>> stored properties
>> - Tests to verify subclasses of Properties
>
> src/java.base/share/classes/java/util/Properties.java line 819:
>
>> 817: * <p>
>> 818: * If the {@systemProperty java.util.Properties.storeDate} is set
>> and
>> 819: * is non-empty (as determined by {@link String#isEmpty()
>> String.isEmpty}),
>
> "is set **on the command line** and non-empty"...
>
> Following from a comment on the CSR, it should be clear that the property
> value used can only be set on the command line.
This is a clever way to detect whether the `entrySet()` method has been
overridden to return something other than the entry-set provided by the
Properties class... but I'm wondering if it's too clever. Does anyone who
overrides entrySet() care about a specific order, or do they simply sort it in
order to get reproducible output? If the latter, then sorting by default hasn't
really broken anything.
Also, it was never specified that the properties are written based on what's
returned by a self-call to `entrySet()`. So this was never guaranteed, though
we do want to avoid gratuitous breakage.
I would also wager that anybody who overrides entrySet() so that they can
control the ordering of the entries is probably breaking the contract of
Map::entrySet, which says that it's mutable (a mapping can be removed by
removing its entry from the entry-set, or the underlying value can be changed
by calling setValue on an entry). This is admittedly pretty obscure, but it
tells me that trying to customize the output of Properties::store by overriding
entrySet() is a pretty fragile hack.
If people really need to control the order of output, they need to iterate the
properties themselves instead of overriding entrySet(). I think the only
difficulty in doing so is properly escaping the keys and values, as performed
by saveConvert(). If this is a use case we want to support, then maybe we
should expose a suitable API for escaping properties keys and values. That
would be a separate enhancement, though.
Note some history in this Stack Overflow question and answers:
https://stackoverflow.com/questions/10275862/how-to-sort-properties-in-java
Basically they mostly describe overriding `keys()`, but we broke that in Java
9, and now the advice is to override `entrySet()`. But the goal is to sort the
properties, which we're doing anyway!
-------------
PR: https://git.openjdk.java.net/jdk/pull/5372
