RFR(xs): 8059361: Properties.stringPropertyNames() returns a set inconsistent with the assertions from the spec

[Stuart Marks]

On 5/26/16 2:28 PM, Mandy Chung wrote:
>> -        return h.keySet();
>> +        return Set.of(h.keySet().toArray(new String[0]));
>>     }
>
> The patch looks fine.  It’d be good to add a test case.
>
> If you use Collections.unmodifiableSet, you would not need to convert the key sets to an array. Any benefit of using Set::of instead of Collections.unmodifiableSet?

There are several minor tradeoffs.

Collections.unmodifiableSet() simply creates and returns a wrapper object. It 
contains the keySet, which is backed by the HashMap created within the 
stringPropertyNames() method. This keeps references to all the string keys and 
values around, even though the values aren't used. But the strings themselves 
are also referenced from the original Properties object and its default chain.

The toArray() call copies the key string refs to an array, then Set.of() hashes 
them into its internal array. But the resulting data structure holds only 
references to the keys, so it's more compact.

Given that most uses seem to iterate the set's elements and then throw it away, 
it's probably not worth the creation overhead of for an immutable Set. Drat. I 
guess I was too eager to use the new API. :-)

I've changed this to use Collections.unmodifiableSet(); see below.

I'll also add test cases to the (internal) test suite for this class.

s'marks


diff -r 4d9388b1ae27 src/java.base/share/classes/java/util/Properties.java
--- a/src/java.base/share/classes/java/util/Properties.java	Wed May 25 13:38:35 
2016 -0700
+++ b/src/java.base/share/classes/java/util/Properties.java	Thu May 26 15:48:20 
2016 -0700
@@ -1037,18 +1037,18 @@
      }

      /**
-     * Returns a set of keys in this property list where
-     * the key and its corresponding value are strings,
+     * Returns an unmodifiable set of keys from this property list
+     * where the key and its corresponding value are strings,
       * including distinct keys in the default property list if a key
       * of the same name has not already been found from the main
       * properties list.  Properties whose key or value is not
       * of type {@code String} are omitted.
       * <p>
-     * The returned set is not backed by the {@code Properties} object.
-     * Changes to this {@code Properties} are not reflected in the set,
-     * or vice versa.
+     * The returned set is not backed by this {@code Properties} object.
+     * Changes to this {@code Properties} object are not reflected in the
+     * returned set.
       *
-     * @return  a set of keys in this property list where
+     * @return  an unmodifiable set of keys in this property list where
       *          the key and its corresponding value are strings,
       *          including the keys in the default property list.
       * @see     java.util.Properties#defaults
@@ -1057,7 +1057,7 @@
      public Set<String> stringPropertyNames() {
          Map<String, String> h = new HashMap<>();
          enumerateStringProperties(h);
-        return h.keySet();
+        return Collections.unmodifiableSet(h.keySet());
      }

      /**