SLING-9312 - LazyBindings.entrySet: hashCode of binding values is calculated
* switched to collecting the entries into a HashMap
* added a test to track the number of hashCode calls; with the previous
implementation the test fails, with one extra hashCode call
diff --git a/src/main/java/org/apache/sling/api/scripting/LazyBindings.java b/src/main/java/org/apache/sling/api/scripting/LazyBindings.java
index 3dff1a6..4acb4e5 100644
--- a/src/main/java/org/apache/sling/api/scripting/LazyBindings.java
+++ b/src/main/java/org/apache/sling/api/scripting/LazyBindings.java
@@ -24,6 +24,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Stream;
import javax.script.Bindings;
@@ -114,11 +115,10 @@
@NotNull
@Override
public Set<Entry<String, Object>> entrySet() {
- HashSet<Entry<String, Object>> entrySet = new HashSet<>(super.entrySet());
- for (Map.Entry supplierEntry : suppliers.entrySet()) {
- entrySet.add(supplierEntry);
- }
- return Collections.unmodifiableSet(entrySet);
+ Map<String, Object> holder = new HashMap<>();
+ Stream.concat(super.entrySet().stream(), suppliers.entrySet().stream())
+ .forEach(entry -> holder.put(entry.getKey(), entry.getValue()));
+ return holder.entrySet();
}
@Override
diff --git a/src/test/java/org/apache/sling/api/scripting/LazyBindingsTest.java b/src/test/java/org/apache/sling/api/scripting/LazyBindingsTest.java
index 71c2894..f4bcf3d 100644
--- a/src/test/java/org/apache/sling/api/scripting/LazyBindingsTest.java
+++ b/src/test/java/org/apache/sling/api/scripting/LazyBindingsTest.java
@@ -25,6 +25,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;
import org.junit.After;
@@ -41,19 +42,16 @@
private static final int THE_ANSWER = 42;
private Set<String> usedSuppliers;
+ private Map<String, Integer> hashCodeCalls;
private LazyBindings lazyBindings;
- private final LazyBindings.Supplier supplier = new LazyBindings.Supplier() {
- @Override
- public Object get() {
- usedSuppliers.add(THE_QUESTION);
- return THE_ANSWER;
- }
- };
+ private TestSupplier supplier;
@Before
public void setUp() {
+ hashCodeCalls = new HashMap<>();
usedSuppliers = new HashSet<>();
+ supplier = new TestSupplier(THE_QUESTION, () -> THE_ANSWER);
final Map<String, LazyBindings.Supplier> supplierMap = new HashMap<>();
supplierMap.put(THE_QUESTION, supplier);
lazyBindings = new LazyBindings(supplierMap);
@@ -158,7 +156,13 @@
expectedEntrySet.add(new AbstractMap.SimpleEntry<>("a", 0));
expectedEntrySet.add(new AbstractMap.SimpleEntry<>(THE_QUESTION, supplier));
assertFalse(usedSuppliers.contains(THE_QUESTION));
+ supplier.trackHashCodeCalls = true;
+ assertEquals(2, lazyBindings.entrySet().size());
+ assertEquals("Unexpected hashCode call for the test supplier.", 0, (int) hashCodeCalls.get(THE_QUESTION));
assertEquals(expectedEntrySet, lazyBindings.entrySet());
+ // the equals implementation will have to call hashCode on the supplier, but the entrySet method should not, so we expect only
+ // one hashCode call
+ assertEquals(1, (int) hashCodeCalls.get(THE_QUESTION));
assertFalse(usedSuppliers.contains(THE_QUESTION));
}
@@ -203,4 +207,31 @@
assertFalse(usedSuppliers.contains(supplierName));
}
+ private class TestSupplier implements LazyBindings.Supplier {
+
+ private final String name;
+ private final LazyBindings.Supplier wrapped;
+ private boolean trackHashCodeCalls;
+
+ TestSupplier(String name, LazyBindings.Supplier wrapped) {
+ this.name = name;
+ this.wrapped = wrapped;
+ hashCodeCalls.putIfAbsent(name, 0);
+ }
+
+ @Override
+ public Object get() {
+ usedSuppliers.add(name);
+ return wrapped.get();
+ }
+
+ @Override
+ public int hashCode() {
+ if (trackHashCodeCalls) {
+ hashCodeCalls.compute(name, (s, integer) -> integer == null ? 0 : integer + 1);
+ }
+ return super.hashCode();
+ }
+ }
+
}
