RFR: 8231640: (prop) Canonical property storage [v17]

Jaikiran Pai jpai at openjdk.java.net
Thu Sep 16 12:06:50 UTC 2021


On Wed, 15 Sep 2021 22:32:33 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> 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.

> "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.

Done. The PR has been updated accordingly.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5372


More information about the core-libs-dev mailing list