Bug 576333 - Simplify ConcurrentNeutralValueMap Fixed and added test for toString, hashCode, equals just in case it would ever be used. Change-Id: I4a55101530c3ce813dd80ebdba05d4b2dd867c6f Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de> Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.runtime/+/186070 Reviewed-by: Sebastian Zarnekow <sebastian.zarnekow@gmail.com> Tested-by: Platform Bot <platform-bot@eclipse.org>
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java index f588fac..01e898e 100644 --- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java +++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java
@@ -27,6 +27,7 @@ final ConcurrentHashMap<K, V> delegate = new ConcurrentHashMap<>(); /** a value that is used for null elements **/ final private V neutralValue; + final private static NullValue NULL = new NullValue(); /** * @param neutralValue a Element that does not equal any other Value @@ -36,6 +37,11 @@ this.neutralValue = neutralValue; } + @SuppressWarnings("unchecked") + public ConcurrentNeutralValueMap() { + this((V) NULL); + } + private V wrapValue(V value) { return value == null ? neutralValue : value; } @@ -49,6 +55,7 @@ } private static final class NullValue { + /** for {@link ConcurrentNeutralValueMap#toString()} **/ @Override public String toString() { return "null"; //$NON-NLS-1$ @@ -56,34 +63,42 @@ @Override public boolean equals(Object obj) { - return false; + // currently not called since ConcurrentHashMap checks for value1==value2 + // but just in case the Implementation changes: + // in means of ConcurrentNeutralValueMap#equals() every NullValue equals + return obj instanceof NullValue; + // like java.util.Objects.equals(null, null) would return true, } + /** for {@link ConcurrentNeutralValueMap#hashCode()} **/ @Override public int hashCode() { + // in means of ConcurrentNeutralValueMap#hashCode() we need a hashCode + // even tough null.hashCode() would normally throw NullPointerException. return 0x55555555; + // like java.util.Objects.hashCode(null) would return an int, } } - /** returns a neutral Element that does not equals any other **/ - public static Object neutralObject() { - return new NullValue(); - } - /* * Method overloads from java.lang.Object: */ + /** return uses hashCode() of the neutralValue **/ @Override public int hashCode() { return delegate.hashCode(); } + /** return does NOT use equals() of the neutralValue **/ @Override public boolean equals(Object obj) { - return delegate.equals(obj); + if (!(obj instanceof ConcurrentNeutralValueMap)) + return false; + return delegate.equals(((ConcurrentNeutralValueMap<?, ?>) obj).delegate); } + /** return uses toString() of the neutralValue **/ @Override public String toString() { return delegate.toString(); @@ -186,9 +201,12 @@ boolean isPresent(); /** - * @return the result of map.get(key) + * Callers are supposed to check #isPresent first to determine whether a + * returned null was a null value or the key was not present. + * + * @return the unwrapped result of map.get(key). May return null. */ - V unwraped(); + V unwrapped(); } private final class Wrapped implements Value<V> { @@ -204,7 +222,7 @@ } @Override - public V unwraped() { + public V unwrapped() { return unwrapValue(wrappedValue); } } @@ -212,7 +230,7 @@ /** * @return a value that is wrapped into a Value Object. * @see Value#isPresent() - * @see Value#unwraped() + * @see Value#unwrapped() */ public Value<V> getValue(K key) { return new Wrapped(delegate.get(key));
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java index e8b7311..c777db8 100644 --- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java +++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
@@ -95,7 +95,7 @@ private Map<String, ValueComputation> localValueComputations = new ConcurrentHashMap<>(); final protected ConcurrentNeutralValueMap<String, Object> localValues = // null values allowed - new ConcurrentNeutralValueMap<>(ConcurrentNeutralValueMap.neutralObject()); + new ConcurrentNeutralValueMap<>(); private Set<String> modifiable; @@ -249,7 +249,7 @@ // 1. try for local value Value<Object> value = localValues.getValue(name); if (value.isPresent()) { - result = value.unwraped(); + result = value.unwrapped(); if (result == null) return null; } else @@ -314,7 +314,7 @@ Value<Object> value = localValues.getValue(name); if (!value.isPresent()) return false; - return (value.unwraped() == newValue); + return (value.unwrapped() == newValue); } private boolean isSetLocally(String name) { @@ -365,7 +365,7 @@ } Value<Object> old = localValues.putAndGetOld(name, value); boolean containsKey = old.isPresent(); - Object oldValue = old.unwraped(); + Object oldValue = old.unwrapped(); if (!containsKey || oldValue != value) { Set<Scheduled> scheduled = new LinkedHashSet<>(); invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled); @@ -400,7 +400,7 @@ String tmp = "Variable " + name + " is not modifiable in the context " + this; //$NON-NLS-1$ //$NON-NLS-2$ throw new IllegalArgumentException(tmp); } - Object oldValue = localValues.putAndGetOld(name, value).unwraped(); + Object oldValue = localValues.putAndGetOld(name, value).unwrapped(); if (oldValue != value) invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled); return true;
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java index dccb2c3..0217012 100644 --- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java +++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java
@@ -28,14 +28,6 @@ public class NeutralValueTest { @Test - public void testNeutralObject() { - assertFalse(new Object().equals(ConcurrentNeutralValueMap.neutralObject())); - assertFalse(ConcurrentNeutralValueMap.neutralObject().equals(new Object())); - assertNotEquals(ConcurrentNeutralValueMap.neutralObject(), ConcurrentNeutralValueMap.neutralObject()); - assertEquals("" + null, ConcurrentNeutralValueMap.neutralObject().toString()); - } - - @Test public void testConcurrentNeutralValueMap() { ConcurrentNeutralValueMap<String, Double> map = new ConcurrentNeutralValueMap<>(Double.NaN); map.put("nix", null); // modify @@ -78,29 +70,29 @@ { Value<Double> v = map.getValue("nix"); assertTrue(v.isPresent()); - assertEquals(null, v.unwraped()); + assertEquals(null, v.unwrapped()); } { Value<Double> v = map.getValue("1"); assertFalse(v.isPresent()); - assertEquals(null, v.unwraped()); + assertEquals(null, v.unwrapped()); } { Value<Double> v = map.getValue("2"); assertTrue(v.isPresent()); - assertEquals(Double.valueOf(2.0), v.unwraped()); + assertEquals(Double.valueOf(2.0), v.unwrapped()); } { Value<Double> v = map.putAndGetOld("5", 5555.0); // modify assertFalse(v.isPresent()); - assertEquals(null, v.unwraped()); + assertEquals(null, v.unwrapped()); assertEquals(Double.valueOf(5555.0), map.get("5")); } { Value<Double> v = map.putAndGetOld("5", 5.0); // modify assertTrue(v.isPresent()); - assertEquals(Double.valueOf(5555.0), v.unwraped()); + assertEquals(Double.valueOf(5555.0), v.unwrapped()); assertEquals(Double.valueOf(5.0), map.get("5")); } map.putIfAbsent("5", 5555.0); // modify @@ -111,27 +103,27 @@ { Value<Double> v = map.putAndGetOld("five", null); // modify assertFalse(v.isPresent()); - assertEquals(null, v.unwraped()); + assertEquals(null, v.unwrapped()); assertEquals(null, map.get("five")); } { Value<Double> v = map.putAndGetOld("five", 5.0); // modify assertTrue(v.isPresent()); - assertEquals(null, v.unwraped()); + assertEquals(null, v.unwrapped()); assertEquals(Double.valueOf(5.0), map.get("five")); } { map.putIfAbsent("five", null); // modify Value<Double> v = map.getValue("five"); assertTrue(v.isPresent()); - assertEquals(Double.valueOf(5.0), v.unwraped()); + assertEquals(Double.valueOf(5.0), v.unwrapped()); } map.remove("five"); // modify { map.putIfAbsent("five", null); // modify Value<Double> v = map.getValue("five"); assertTrue(v.isPresent()); - assertEquals(null, v.unwraped()); + assertEquals(null, v.unwrapped()); } map.remove("five"); // modify assertFalse(map.containsKey("five")); @@ -141,4 +133,73 @@ assertTrue(map.isEmpty()); } + @Test + public void testToString() { + ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(); + map1.put("0", 0f); + map1.put("NULL", null); + map1.put("nothing", null); + map1.put("1", 1f); + map1.put("~2", 2.1f); + assertTrue(map1.toString().contains("0=0.0")); + assertTrue(map1.toString().contains("1=1.0")); + assertTrue(map1.toString().contains("~2=2.1")); + assertTrue(map1.toString().contains("NULL=null")); + assertTrue(map1.toString().contains("nothing=null")); + } + + @Test + public void testCustomToString() { + ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(Float.NaN); + map1.put("0", 0f); + map1.put("NULL", null); + assertTrue(map1.toString().contains("0=0.0")); + assertTrue(map1.toString().contains("NULL=NaN")); + } + + @Test + public void testEquals() { + { + ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(); + ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>(); + map1.put("0", 0f); + map1.put("NULL", null); + map2.put("NULL", null); + map2.put("0", 0f); + assertEquals(map1.hashCode(), map2.hashCode()); + assertEquals(map1, map2); + } + { + ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(); + ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>(); + map1.put("0", 0f); + map1.put("1", 1f); + map1.put("NULL", null); + map2.put("NULL", null); + map2.put("0", 0f); + assertNotEquals(map1.hashCode(), map2.hashCode()); + assertNotEquals(map1, map2); + } + { + ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(Float.NaN); + ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>(Float.NaN); + map1.put("0", 0f); + map1.put("NULL", null); + map2.put("NULL", null); + map2.put("0", 0f); + assertEquals(map1.hashCode(), map2.hashCode()); + assertEquals(map1, map2); + } + { + ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(Float.NaN); + ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>(Float.NaN); + map1.put("0", 0f); + map1.put("1", 1f); + map1.put("NULL", null); + map2.put("NULL", null); + map2.put("0", 0f); + assertNotEquals(map1.hashCode(), map2.hashCode()); + assertNotEquals(map1, map2); + } + } }