On Wed, 15 Sep 2021 22:32:33 GMT, Roger Riggs <rri...@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
