[PATCH] 8245694 : java.util.Properties.entrySet() does not override java.lang.Object methods
Lisa Li
liyu.italy at gmail.com
Wed Jun 3 05:37:30 UTC 2020
Hi，
Thanks a lot Julia! I have signed OCA and emailed it to Oracle last week.
But I haven't seen my name on the contributor list yet. I guess I'll have
to wait.
And thanks a lot for all the advice, Brent! It's really useful. I'll do as
you said.
Best regards,
Yu
On Wed, Jun 3, 2020 at 7:51 AM Brent Christian <brent.christian at oracle.com>
wrote:
> Thank you for submitting the fix, Yu Li. (And thanks for the sponsor,
> Julia). This will be good to fix. I have a few changes to recommend.
>
> src/java.base/share/classes/java/util/Properties.java
>
> The code change looks fine to me.
>
> The ending Copyright year just needs to be changed from 2019 -> 2020.
>
>
> test/jdk/java/util/Properties/PropertiesEntrySetTest.java
>
> + * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights
>
> For a newly added file, only the current year is listed, so:
>
> Copyright (c) 2020, Oracle and/or its affiliates. All rights
> reserved.
>
> +/*
> + * @test
> + * @summary tests the entrySet() method of Properties class
> + * @author Yu Li
> + * @bug 8245694
> + */
>
> Please also add an @run tag. Using testng would prevent needing to add
> assert[True|False] methods. So that would be:
>
> @run testng PropertiesEntrySetTest
>
> Then you should be able to:
>
> import org.testng.Assert;
>
> and use the testng Assert.assertTrue() and Assert.assertFalse().
>
>
> + Set<Map.Entry<Object, Object>> aEntrySet = a.entrySet();
>
> You might consider using 'var' to simplify declarations of aEntrySet,
> bEntrySet, etc
>
>
> Please also add testing of EntrySet.toString().
>
> Along with testing equals/hashCode/toString after properties are added,
> I think it would be good to also test after properties are removed.
> There's Properties.remove()/clear(), and also maybe some of the removal
> methods listed in Hashtable.entrySet(): Iterator.remove(), Set.remove(),
> etc.
>
>
> (And just as a point of interest, there is a test,
> test/jdk/java/util/Properties/CheckOverrides.java,
> to ensure that the Properties class overrides all necessary methods of
> its superclasses. Checks were not included for Properties.EntrySet
> (hence this bug), though I don't think it makes sense to add any at this
> point.)
>
> Thanks a lot,
> -Brent
>
> On 6/2/20 8:43 AM, Julia Boes wrote:
> > Hi Yu Li,
> >
> > I'm happy to sponsor your fix once it's reviewed. Could you just confirm
> > that you have signed the Oracle Contributor Agreement?
> >
> > https://www.oracle.com/technetwork/community/oca-486395.html
> >
> > Cheers,
> >
> > Julia
> >
> > On 30/05/2020 21:00, Rob Spoor wrote:
> >> There's still a little regression there since Java 8, if you add the
> >> keySet(), values() or entrySet() to the Properties object as either
> >> key or value; in Java 8 this would cause "(this Collection)" to be
> >> included, while even with this fix that would cause a
> >> StackOverflowError. However, given that it's only a corner case for
> >> most Collections and Maps in the first place, and with Properties
> >> objects even more so, I don't think anyone would mind this regression.
> >>
> >> Rob
> >>
> >> On 30/05/2020 20:31, Lisa Li wrote:
> >>> Hi,
> >>>
> >>> Please help review the fix for JDK-8245694
> >>> <https://bugs.openjdk.java.net/browse/JDK-8245694>. And this is my
> very
> >>> first patch submission. I know it's not perfect.
> >>> <https://bugs.openjdk.java.net/browse/JDK-8245694>
> >>>
> >>>
> >>> *BUG DESCRIPTION*:
> >>>
> >>> JDK-8245694 <https://bugs.openjdk.java.net/browse/JDK-8245694> :
> >>> java.util.Properties.entrySet()
> >>> does not override java.lang.Object methods since java 9.
> >>>
> >>>
> >>> *PROPOSED FIX*:
> >>>
> >>> Add delegating overrides for equals(), hashCode(), and toString().
> >>>
> >>>
> >>> *PATCH*:
> >>>
> >>> # HG changeset patch
> >>> # User yuli
> >>> # Date 1590841711 -28800
> >>> # Sat May 30 20:28:31 2020 +0800
> >>> # Node ID 7dc946e5b5863291fa070bfdbdce48aefbe87b7b
> >>> # Parent 8e28aae5068069e853673148e4d3f44cb50350a7
> >>> 8245694: java.util.Properties.entrySet() does not override Object
> >>> methods
> >>> Summary: Add missing override methods
> >>> Contributed-by: Yu Li <liyu.italy at gmail.com>
> >>>
> >>> diff --git a/src/java.base/share/classes/java/util/Properties.java
> >>> b/src/java.base/share/classes/java/util/Properties.java
> >>> --- a/src/java.base/share/classes/java/util/Properties.java
> >>> +++ b/src/java.base/share/classes/java/util/Properties.java
> >>> @@ -1414,6 +1414,21 @@
> >>> return entrySet.retainAll(c);
> >>> }
> >>>
> >>> + @Override
> >>> + public boolean equals(Object o) {
> >>> + return entrySet.equals(o);
> >>> + }
> >>> +
> >>> + @Override
> >>> + public int hashCode() {
> >>> + return entrySet.hashCode();
> >>> + }
> >>> +
> >>> + @Override
> >>> + public String toString() {
> >>> + return entrySet.toString();
> >>> + }
> >>> +
> >>> @Override
> >>> public Iterator<Map.Entry<Object, Object>> iterator() {
> >>> return entrySet.iterator();
> >>> diff --git a/test/jdk/java/util/Properties/PropertiesEntrySetTest.java
> >>> b/test/jdk/java/util/Properties/PropertiesEntrySetTest.java
> >>> new file mode 100644
> >>> --- /dev/null
> >>> +++ b/test/jdk/java/util/Properties/PropertiesEntrySetTest.java
> >>> @@ -0,0 +1,90 @@
> >>> +/*
> >>> + * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights
> >>> reserved.
> >>> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> >>> + *
> >>> + * This code is free software; you can redistribute it and/or modify
> it
> >>> + * under the terms of the GNU General Public License version 2 only,
> as
> >>> + * published by the Free Software Foundation.
> >>> + *
> >>> + * This code is distributed in the hope that it will be useful, but
> >>> WITHOUT
> >>> + * ANY WARRANTY; without even the implied warranty of
> >>> MERCHANTABILITY or
> >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
> >>> License
> >>> + * version 2 for more details (a copy is included in the LICENSE
> >>> file that
> >>> + * accompanied this code).
> >>> + *
> >>> + * You should have received a copy of the GNU General Public License
> >>> version
> >>> + * 2 along with this work; if not, write to the Free Software
> >>> Foundation,
> >>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> >>> + *
> >>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA
> >>> 94065 USA
> >>> + * or visit www.oracle.com if you need additional information or
> >>> have any
> >>> + * questions.
> >>> + */
> >>> +
> >>> +/*
> >>> + * @test
> >>> + * @summary tests the entrySet() method of Properties class
> >>> + * @author Yu Li
> >>> + * @bug 8245694
> >>> + */
> >>> +
> >>> +import java.util.Map;
> >>> +import java.util.Properties;
> >>> +import java.util.Set;
> >>> +
> >>> +public class PropertiesEntrySetTest {
> >>> +
> >>> + public static void main(String[] args) {
> >>> +
> >>> + testEntrySet();
> >>> +
> >>> + }
> >>> +
> >>> + private static void assertTrue(boolean value) {
> >>> + if (!value) {
> >>> + throw new RuntimeException("Failure in Properties
> >>> testing.");
> >>> + }
> >>> + }
> >>> +
> >>> + private static void assertFalse(boolean value) {
> >>> + if (value) {
> >>> + throw new RuntimeException("Failure in Properties
> >>> testing.");
> >>> + }
> >>> + }
> >>> +
> >>> + private static void testEntrySet() {
> >>> + Properties a = new Properties();
> >>> + Set<Map.Entry<Object, Object>> aEntrySet = a.entrySet();
> >>> + assertFalse(aEntrySet.equals(null));
> >>> + assertTrue(aEntrySet.equals(aEntrySet));
> >>> +
> >>> + Properties b = new Properties();
> >>> + Set<Map.Entry<Object, Object>> bEntrySet = b.entrySet();
> >>> + assertTrue(aEntrySet.equals(bEntrySet));
> >>> + assertTrue(aEntrySet.hashCode() == bEntrySet.hashCode());
> >>> +
> >>> + a.setProperty("p1", "1");
> >>> + assertFalse(aEntrySet.equals(bEntrySet));
> >>> + assertFalse(bEntrySet.equals(aEntrySet));
> >>> + assertFalse(aEntrySet.hashCode() == bEntrySet.hashCode());
> >>> +
> >>> + b.setProperty("p1", "1");
> >>> + assertTrue(aEntrySet.equals(bEntrySet));
> >>> + assertTrue(aEntrySet.hashCode() == bEntrySet.hashCode());
> >>> +
> >>> + Properties c = new Properties();
> >>> + c.setProperty("p1", "2");
> >>> + Set<Map.Entry<Object, Object>> cEntrySet = c.entrySet();
> >>> + assertFalse(cEntrySet.equals(bEntrySet));
> >>> + assertFalse(cEntrySet.hashCode() == bEntrySet.hashCode());
> >>> + assertFalse(cEntrySet.equals(aEntrySet));
> >>> + assertFalse(cEntrySet.hashCode() == aEntrySet.hashCode());
> >>> +
> >>> + a.setProperty("p2", "2");
> >>> + Properties d = new Properties();
> >>> + d.setProperty("p2", "2");
> >>> + d.setProperty("p1", "1");
> >>> + Set<Map.Entry<Object, Object>> dEntrySet = d.entrySet();
> >>> + assertTrue(dEntrySet.equals(aEntrySet));
> >>> + }
> >>> +}
> >>>
> >>> Best regards,
> >>> Yu Li
>
More information about the core-libs-dev
mailing list