On 16/09/21 4:05 am, Roger Riggs wrote:
On Wed, 15 Sep 2021 21:46:59 GMT, Stuart Marks <sma...@openjdk.org> wrote:
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!
One part of what store() does is annoying to replicate, the encoding that
`saveConvert` does is non-trivial.
Other hacks based on returning a different entrySet might be to filter the set
either keep some entries or ignore them.
Both unlikely, but hacks are frequently fragile. And we've been very cautious
in this discussion to avoid
breaking things, in part, because there is so little info about how it is used.
To summarize the options that we have discussed for this entrySet() part:
- Do nothing specific for subclasses that override entrySet() method.
This would mean that the store() method would write out the properties
in the natural sort order, but also has a tiny possibility that it will
break backward compatibility if some code out there that was returning a
differently ordered set. Given how we have tried to prevent backward
compatibility issues in this PR plus the fact that we might have a
possible way to prevent this, I think we can rule out this option.
- Check the Properties object instance type to see if it has been
subclassed. If yes, then don't store the properties in the natural sort
order. I personally think we should rule this option out because this
option prevents this enhancement from being usable by any subclass of
Properties even if those subclasses do nothing related to entrySet() and
could merely have been subclassed for completely unrelated things.
- Detect that the entrySet() method is overridden by the subclass of
Properties and might have done something with the returned
instance/type. It's just a co-incidence that the Properties.entrySet()
already returns a internal private class from that method. This does
allow us to introduce a check to decide whether or not to use the new
natural sort order for writing the properties. It relies on an internal
knowledge plus the internal impl detail of the Property.entrySet() but,
IMO, it might be good enough for us to use to be sure that we don't
break backward compatibility and yet at the same time, introduce this
natural sorted order by default for most of the subclasses out there.
The other aspect to consider here is the future maintainability of such
check that is being proposed here. What this check would effectively
mean is that the implementation in Property.entrySet() and this store()
method will have to be closely "held together" so that if we do change
the implementation of Property.entrySet() to return something else at a
later point, we would still have to return a type which is private to
this Properties class (or some internal marker interface type) so that
the store() methods can continue to employ a check like this one. IMO,
that shouldn't be too hard to do if/when such a change happens in
Properties.entrySet(). So I am in favour of this option to get past this
entrySet() overridding issue.
-Jaikiran
