From 7bc7fd9db66e3a49ef582818c2694e124501fc68 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 6 Mar 2025 15:11:24 +0100 Subject: [PATCH 1/5] fix: equals and hashcode of several classes Signed-off-by: christian.lutnik --- .../openfeature/sdk/AbstractStructure.java | 44 ++++++++++- .../dev/openfeature/sdk/ImmutableContext.java | 17 ++++ .../openfeature/sdk/ImmutableMetadata.java | 37 +++++++++ .../openfeature/sdk/ImmutableStructure.java | 4 +- .../dev/openfeature/sdk/MutableContext.java | 19 ++++- .../dev/openfeature/sdk/MutableStructure.java | 2 - src/main/java/dev/openfeature/sdk/Value.java | 20 ++++- .../openfeature/sdk/ImmutableContextTest.java | 28 +++++++ .../sdk/ImmutableMetadataTest.java | 24 ++++++ .../sdk/ImmutableStructureTest.java | 77 ++++++++++++++++++- .../openfeature/sdk/MutableContextTest.java | 28 +++++++ .../java/dev/openfeature/sdk/ValueTest.java | 54 +++++++++---- 12 files changed, 329 insertions(+), 25 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java diff --git a/src/main/java/dev/openfeature/sdk/AbstractStructure.java b/src/main/java/dev/openfeature/sdk/AbstractStructure.java index 6c652114c..c8b203144 100644 --- a/src/main/java/dev/openfeature/sdk/AbstractStructure.java +++ b/src/main/java/dev/openfeature/sdk/AbstractStructure.java @@ -11,7 +11,7 @@ abstract class AbstractStructure implements Structure { @Override public boolean isEmpty() { - return attributes == null || attributes.size() == 0; + return attributes == null || attributes.isEmpty(); } AbstractStructure() { @@ -46,4 +46,46 @@ public Map asObjectMap() { (accumulated, entry) -> accumulated.put(entry.getKey(), convertValue(entry.getValue())), HashMap::putAll); } + + public boolean equals(final Object o) { + if (o == this) { + return true; + } + if (!(o instanceof AbstractStructure)) { + return false; + } + final AbstractStructure other = (AbstractStructure) o; + if (other.attributes == attributes) { + return true; + } + if (attributes == null || other.attributes == null) { + return false; + } + if (other.attributes.size() != attributes.size()) { + return false; + } + + for (Map.Entry thisEntry : attributes.entrySet()) { + Value thisValue = thisEntry.getValue(); + Value otherValue = other.attributes.get(thisEntry.getKey()); + if (thisValue == null && otherValue == null) { + continue; + } + if (thisValue == null || otherValue == null) { + return false; + } + if (!thisValue.equals(otherValue)) { + return false; + } + } + + return true; + } + + public int hashCode() { + if (attributes == null) { + return 0; + } + return attributes.hashCode(); + } } diff --git a/src/main/java/dev/openfeature/sdk/ImmutableContext.java b/src/main/java/dev/openfeature/sdk/ImmutableContext.java index 23a452e08..9a503a33e 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableContext.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableContext.java @@ -91,6 +91,23 @@ public EvaluationContext merge(EvaluationContext overridingContext) { return new ImmutableContext(attributes); } + @Override + public boolean equals(Object object) { + if (object == this) { + return true; + } + if (!(object instanceof ImmutableContext)) { + return false; + } + ImmutableContext other = (ImmutableContext) object; + return this.structure.equals(other.structure); + } + + @Override + public int hashCode() { + return structure.hashCode(); + } + @SuppressWarnings("all") private static class DelegateExclusions { @ExcludeFromGeneratedCoverageReport diff --git a/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java b/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java index f8311a9a5..d646f2f3d 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java @@ -101,6 +101,43 @@ public boolean isEmpty() { return metadata.isEmpty(); } + + public boolean equals(final Object o) { + if (o == this) { + return true; + } + if (!(o instanceof ImmutableMetadata)) { + return false; + } + final ImmutableMetadata other = (ImmutableMetadata) o; + if (other.metadata == metadata) { + return true; + } + if (other.metadata.size() != metadata.size()) { + return false; + } + + for (Map.Entry thisEntry : metadata.entrySet()) { + Object thisValue = thisEntry.getValue(); + Object otherValue = other.metadata.get(thisEntry.getKey()); + if (thisValue == null && otherValue == null) { + continue; + } + if (thisValue == null || otherValue == null) { + return false; + } + if (!thisValue.equals(otherValue)) { + return false; + } + } + + return true; + } + + public int hashCode() { + return metadata.hashCode(); + } + /** * Obtain a builder for {@link ImmutableMetadata}. */ diff --git a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java index c47a49eb3..6bb4ddc93 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java @@ -6,7 +6,6 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.Set; -import lombok.EqualsAndHashCode; import lombok.ToString; /** @@ -18,7 +17,6 @@ * not be modified after instantiation. All references are clones. */ @ToString -@EqualsAndHashCode @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"}) public final class ImmutableStructure extends AbstractStructure { @@ -38,7 +36,7 @@ public ImmutableStructure(Map attributes) { super(copyAttributes(attributes, null)); } - protected ImmutableStructure(String targetingKey, Map attributes) { + ImmutableStructure(String targetingKey, Map attributes) { super(copyAttributes(attributes, targetingKey)); } diff --git a/src/main/java/dev/openfeature/sdk/MutableContext.java b/src/main/java/dev/openfeature/sdk/MutableContext.java index 7fda58065..1679f7429 100644 --- a/src/main/java/dev/openfeature/sdk/MutableContext.java +++ b/src/main/java/dev/openfeature/sdk/MutableContext.java @@ -5,6 +5,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.ToString; @@ -17,7 +18,6 @@ * be modified after instantiation. */ @ToString -@EqualsAndHashCode @SuppressWarnings("PMD.BeanMembersShouldSerialize") public class MutableContext implements EvaluationContext { @@ -125,6 +125,23 @@ public EvaluationContext merge(EvaluationContext overridingContext) { return new MutableContext(attributes); } + @Override + public boolean equals(Object object) { + if (object == this) { + return true; + } + if (!(object instanceof MutableContext)) { + return false; + } + MutableContext other = (MutableContext) object; + return this.structure.equals(other.structure); + } + + @Override + public int hashCode() { + return structure.hashCode(); + } + /** * Hidden class to tell Lombok not to copy these methods over via delegation. */ diff --git a/src/main/java/dev/openfeature/sdk/MutableStructure.java b/src/main/java/dev/openfeature/sdk/MutableStructure.java index a06e2f2d3..7e10f083e 100644 --- a/src/main/java/dev/openfeature/sdk/MutableStructure.java +++ b/src/main/java/dev/openfeature/sdk/MutableStructure.java @@ -5,7 +5,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import lombok.EqualsAndHashCode; import lombok.ToString; /** @@ -15,7 +14,6 @@ * be modified after instantiation. */ @ToString -@EqualsAndHashCode @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"}) public class MutableStructure extends AbstractStructure { diff --git a/src/main/java/dev/openfeature/sdk/Value.java b/src/main/java/dev/openfeature/sdk/Value.java index 05e538e50..7b2d88512 100644 --- a/src/main/java/dev/openfeature/sdk/Value.java +++ b/src/main/java/dev/openfeature/sdk/Value.java @@ -7,7 +7,6 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import lombok.EqualsAndHashCode; import lombok.SneakyThrows; import lombok.ToString; @@ -17,7 +16,6 @@ * This intermediate representation provides a good medium of exchange. */ @ToString -@EqualsAndHashCode @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType", "checkstyle:NoFinalizer"}) public class Value implements Cloneable { @@ -316,4 +314,22 @@ public static Value objectToValue(Object object) { throw new TypeMismatchError("Flag value " + object + " had unexpected type " + object.getClass() + "."); } } + + public boolean equals(final Object o) { + if (o == this) { + return true; + } + if (!(o instanceof Value)) { + return false; + } + final Value other = (Value) o; + return innerObject.equals(other.innerObject); + } + + public int hashCode() { + if (innerObject == null) { + return 0; + } + return innerObject.hashCode(); + } } diff --git a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java index e69a974b3..2b39be741 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java @@ -3,6 +3,7 @@ import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; @@ -133,4 +134,31 @@ void mergeShouldRetainItsSubkeysWhenOverridingContextHasNoTargetingKey() { Structure value = key1.asStructure(); assertArrayEquals(new Object[] {"key1_1"}, value.keySet().toArray()); } + + @DisplayName("Two different MutableContext objects with the different contents are not considered equal") + @Test + void unequalImmutableContextsAreNotEqual() { + final Map attributes = new HashMap<>(); + attributes.put("key1", new Value("val1")); + final ImmutableContext ctx = new ImmutableContext(attributes); + + final Map attributes2 = new HashMap<>(); + final ImmutableContext ctx2 = new ImmutableContext(attributes2); + + assertNotEquals(ctx, ctx2); + } + + @DisplayName("Two different MutableContext objects with the same content are considered equal") + @Test + void equalImmutableContextsAreEqual() { + final Map attributes = new HashMap<>(); + attributes.put("key1", new Value("val1")); + final ImmutableContext ctx = new ImmutableContext(attributes); + + final Map attributes2 = new HashMap<>(); + attributes2.put("key1", new Value("val1")); + final ImmutableContext ctx2 = new ImmutableContext(attributes2); + + assertEquals(ctx, ctx2); + } } diff --git a/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java b/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java new file mode 100644 index 000000000..bea56df41 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java @@ -0,0 +1,24 @@ +package dev.openfeature.sdk; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import org.junit.jupiter.api.Test; + +class ImmutableMetadataTest { + @Test + void unequalImmutableMetadataAreUnequal() { + ImmutableMetadata i1 = ImmutableMetadata.builder().addString("key1", "value1").build(); + ImmutableMetadata i2 = ImmutableMetadata.builder().addString("key1", "value2").build(); + + assertNotEquals(i1, i2); + } + + @Test + void equalImmutableMetadataAreEqual() { + ImmutableMetadata i1 = ImmutableMetadata.builder().addString("key1", "value1").build(); + ImmutableMetadata i2 = ImmutableMetadata.builder().addString("key1", "value1").build(); + + assertEquals(i1, i2); + } +} diff --git a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java index dff95adca..2202d45a9 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java @@ -1,6 +1,11 @@ package dev.openfeature.sdk; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -154,4 +159,74 @@ void constructorHandlesNullValue() { attrs.put("null", null); new ImmutableStructure(attrs); } + + @Test + void unequalImmutableStructuresAreNotEqual() { + Map attrs1 = new HashMap<>(); + attrs1.put("test", new Value(45)); + ImmutableStructure structure1 = new ImmutableStructure(attrs1); + + Map attrs2 = new HashMap<>(); + attrs2.put("test", new Value(2)); + ImmutableStructure structure2 = new ImmutableStructure(attrs2); + + assertNotEquals(structure1, structure2); + } + + @Test + void equalImmutableStructuresAreEqual() { + Map attrs1 = new HashMap<>(); + attrs1.put("test", new Value(45)); + ImmutableStructure structure1 = new ImmutableStructure(attrs1); + + Map attrs2 = new HashMap<>(); + attrs2.put("test", new Value(45)); + ImmutableStructure structure2 = new ImmutableStructure(attrs2); + + assertEquals(structure1, structure2); + } + + @Test + void unequalMutableStructuresAreNotEqual() { + MutableStructure m1 = new MutableStructure(); + m1.add("key1", "val1"); + MutableStructure m2 = new MutableStructure(); + m2.add("key2", "val2"); + assertNotEquals(m1, m2); + } + + @Test + void equalMutableStructuresAreEqual() { + MutableStructure m1 = new MutableStructure(); + m1.add("key1", "val1"); + MutableStructure m2 = new MutableStructure(); + m2.add("key1", "val1"); + assertEquals(m1, m2); + } + + @Test + void equalAbstractStructuresOfDifferentTypesAreEqual() { + MutableStructure m1 = new MutableStructure(); + m1.add("key1", "val1"); + HashMap map = new HashMap<>(); + map.put("key1", new Value("val1")); + AbstractStructure m2 = new AbstractStructure(map) { + @Override + public Set keySet() { + return attributes.keySet(); + } + + @Override + public Value getValue(String key) { + return attributes.get(key); + } + + @Override + public Map asMap() { + return attributes; + } + }; + + assertEquals(m1, m2); + } } diff --git a/src/test/java/dev/openfeature/sdk/MutableContextTest.java b/src/test/java/dev/openfeature/sdk/MutableContextTest.java index 953e3f636..6c471d09a 100644 --- a/src/test/java/dev/openfeature/sdk/MutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableContextTest.java @@ -3,6 +3,7 @@ import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; @@ -137,4 +138,31 @@ void shouldAllowChainingOfMutations() { assertEquals(2, context.getValue("key2").asInteger()); assertEquals(3.0, context.getValue("key3").asDouble()); } + + @DisplayName("Two different MutableContext objects with the different contents are not considered equal") + @Test + void unequalMutableContextsAreNotEqual() { + final Map attributes = new HashMap<>(); + attributes.put("key1", new Value("val1")); + final MutableContext ctx = new MutableContext(attributes); + + final Map attributes2 = new HashMap<>(); + final MutableContext ctx2 = new MutableContext(attributes2); + + assertNotEquals(ctx, ctx2); + } + + @DisplayName("Two different MutableContext objects with the same content are considered equal") + @Test + void equalMutableContextsAreEqual() { + final Map attributes = new HashMap<>(); + attributes.put("key1", new Value("val1")); + final MutableContext ctx = new MutableContext(attributes); + + final Map attributes2 = new HashMap<>(); + attributes2.put("key1", new Value("val1")); + final MutableContext ctx2 = new MutableContext(attributes2); + + assertEquals(ctx, ctx2); + } } diff --git a/src/test/java/dev/openfeature/sdk/ValueTest.java b/src/test/java/dev/openfeature/sdk/ValueTest.java index c25538508..13bda2392 100644 --- a/src/test/java/dev/openfeature/sdk/ValueTest.java +++ b/src/test/java/dev/openfeature/sdk/ValueTest.java @@ -2,24 +2,26 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import java.lang.reflect.Modifier; import java.time.Instant; import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; -public class ValueTest { +class ValueTest { @Test - public void noArgShouldContainNull() { + void noArgShouldContainNull() { Value value = new Value(); assertTrue(value.isNull()); } @Test - public void objectArgShouldContainObject() { + void objectArgShouldContainObject() { try { // int is a special case, see intObjectArgShouldConvertToInt() List list = new ArrayList<>(); @@ -42,7 +44,7 @@ public void objectArgShouldContainObject() { } @Test - public void intObjectArgShouldConvertToInt() { + void intObjectArgShouldConvertToInt() { try { Object innerValue = 1; Value value = new Value(innerValue); @@ -53,7 +55,7 @@ public void intObjectArgShouldConvertToInt() { } @Test - public void invalidObjectArgShouldThrow() { + void invalidObjectArgShouldThrow() { class Something {} @@ -63,7 +65,7 @@ class Something {} } @Test - public void boolArgShouldContainBool() { + void boolArgShouldContainBool() { boolean innerValue = true; Value value = new Value(innerValue); assertTrue(value.isBoolean()); @@ -71,7 +73,7 @@ public void boolArgShouldContainBool() { } @Test - public void numericArgShouldReturnDoubleOrInt() { + void numericArgShouldReturnDoubleOrInt() { double innerDoubleValue = 1.75; Value doubleValue = new Value(innerDoubleValue); assertTrue(doubleValue.isNumber()); @@ -86,7 +88,7 @@ public void numericArgShouldReturnDoubleOrInt() { } @Test - public void stringArgShouldContainString() { + void stringArgShouldContainString() { String innerValue = "hi!"; Value value = new Value(innerValue); assertTrue(value.isString()); @@ -94,7 +96,7 @@ public void stringArgShouldContainString() { } @Test - public void dateShouldContainDate() { + void dateShouldContainDate() { Instant innerValue = Instant.now(); Value value = new Value(innerValue); assertTrue(value.isInstant()); @@ -102,7 +104,7 @@ public void dateShouldContainDate() { } @Test - public void structureShouldContainStructure() { + void structureShouldContainStructure() { String INNER_KEY = "key"; String INNER_VALUE = "val"; MutableStructure innerValue = new MutableStructure().add(INNER_KEY, INNER_VALUE); @@ -112,7 +114,7 @@ public void structureShouldContainStructure() { } @Test - public void listArgShouldContainList() { + void listArgShouldContainList() { String ITEM_VALUE = "val"; List innerValue = new ArrayList(); innerValue.add(new Value(ITEM_VALUE)); @@ -122,7 +124,7 @@ public void listArgShouldContainList() { } @Test - public void listMustBeOfValues() { + void listMustBeOfValues() { String item = "item"; List list = new ArrayList<>(); list.add(item); @@ -135,7 +137,7 @@ public void listMustBeOfValues() { } @Test - public void emptyListAllowed() { + void emptyListAllowed() { List list = new ArrayList<>(); try { Value value = new Value((Object) list); @@ -148,7 +150,7 @@ public void emptyListAllowed() { } @Test - public void valueConstructorValidateListInternals() { + void valueConstructorValidateListInternals() { List list = new ArrayList<>(); list.add(new Value("item")); list.add("item"); @@ -157,8 +159,30 @@ public void valueConstructorValidateListInternals() { } @Test - public void noOpFinalize() { + void noOpFinalize() { Value val = new Value(); assertDoesNotThrow(val::finalize); // does nothing, but we want to defined in and make it final. } + + @Test + void equalValuesShouldBeEqual() { + Value val1 = new Value(12312312); + Value val2 = new Value(12312312); + assertEquals(val1, val2); + } + + @Test + void unequalValuesShouldNotBeEqual() { + Value val1 = new Value("a"); + Value val2 = new Value("b"); + assertNotEquals(val1, val2); + } + + @Test + void hashCodeShouldGiveHashCodeOfInnerObject() { + String innerValue = "val"; + Value val1 = new Value(innerValue); + + assertEquals(innerValue.hashCode(), val1.hashCode()); + } } From 2d28945b4f0c1c50591047da52973dd800f844b6 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 6 Mar 2025 15:23:34 +0100 Subject: [PATCH 2/5] fix: equals and hashcode of several classes Signed-off-by: christian.lutnik --- .../java/dev/openfeature/sdk/ImmutableMetadata.java | 1 - .../java/dev/openfeature/sdk/MutableContext.java | 2 -- src/main/java/dev/openfeature/sdk/Value.java | 9 +++++++++ .../dev/openfeature/sdk/ImmutableMetadataTest.java | 12 ++++++++---- src/test/java/dev/openfeature/sdk/ValueTest.java | 1 - 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java b/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java index d646f2f3d..996876285 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java @@ -101,7 +101,6 @@ public boolean isEmpty() { return metadata.isEmpty(); } - public boolean equals(final Object o) { if (o == this) { return true; diff --git a/src/main/java/dev/openfeature/sdk/MutableContext.java b/src/main/java/dev/openfeature/sdk/MutableContext.java index 1679f7429..11bcc752d 100644 --- a/src/main/java/dev/openfeature/sdk/MutableContext.java +++ b/src/main/java/dev/openfeature/sdk/MutableContext.java @@ -5,9 +5,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.function.Function; -import lombok.EqualsAndHashCode; import lombok.ToString; import lombok.experimental.Delegate; diff --git a/src/main/java/dev/openfeature/sdk/Value.java b/src/main/java/dev/openfeature/sdk/Value.java index 7b2d88512..8df55493e 100644 --- a/src/main/java/dev/openfeature/sdk/Value.java +++ b/src/main/java/dev/openfeature/sdk/Value.java @@ -315,6 +315,11 @@ public static Value objectToValue(Object object) { } } + /** + * Returns true iff {@code this} is equal to {@code o}, or if both objects represent the same data. + * @param o the other object + * @return true iff both objects are equal or represent the same data + */ public boolean equals(final Object o) { if (o == this) { return true; @@ -326,6 +331,10 @@ public boolean equals(final Object o) { return innerObject.equals(other.innerObject); } + /** + * Returns the `hashCode` of the underlying data, or 0 if {@link Value#isNull()} returns true. + * @return the hash code + */ public int hashCode() { if (innerObject == null) { return 0; diff --git a/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java b/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java index bea56df41..e3bd03165 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java @@ -8,16 +8,20 @@ class ImmutableMetadataTest { @Test void unequalImmutableMetadataAreUnequal() { - ImmutableMetadata i1 = ImmutableMetadata.builder().addString("key1", "value1").build(); - ImmutableMetadata i2 = ImmutableMetadata.builder().addString("key1", "value2").build(); + ImmutableMetadata i1 = + ImmutableMetadata.builder().addString("key1", "value1").build(); + ImmutableMetadata i2 = + ImmutableMetadata.builder().addString("key1", "value2").build(); assertNotEquals(i1, i2); } @Test void equalImmutableMetadataAreEqual() { - ImmutableMetadata i1 = ImmutableMetadata.builder().addString("key1", "value1").build(); - ImmutableMetadata i2 = ImmutableMetadata.builder().addString("key1", "value1").build(); + ImmutableMetadata i1 = + ImmutableMetadata.builder().addString("key1", "value1").build(); + ImmutableMetadata i2 = + ImmutableMetadata.builder().addString("key1", "value1").build(); assertEquals(i1, i2); } diff --git a/src/test/java/dev/openfeature/sdk/ValueTest.java b/src/test/java/dev/openfeature/sdk/ValueTest.java index 13bda2392..440a26954 100644 --- a/src/test/java/dev/openfeature/sdk/ValueTest.java +++ b/src/test/java/dev/openfeature/sdk/ValueTest.java @@ -7,7 +7,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import java.lang.reflect.Modifier; import java.time.Instant; import java.util.ArrayList; import java.util.List; From 6be975b8571e85ff5072d73650e8d2c80e63a573 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 10 Mar 2025 11:03:41 +0100 Subject: [PATCH 3/5] fix: equals and hashcode of several classes Signed-off-by: christian.lutnik --- .../openfeature/sdk/AbstractStructure.java | 44 +-------------- .../dev/openfeature/sdk/EventDetails.java | 2 + .../dev/openfeature/sdk/ImmutableContext.java | 19 +------ .../openfeature/sdk/ImmutableMetadata.java | 36 ------------ .../openfeature/sdk/ImmutableStructure.java | 2 + .../dev/openfeature/sdk/MutableContext.java | 19 +------ .../dev/openfeature/sdk/MutableStructure.java | 2 + src/main/java/dev/openfeature/sdk/Value.java | 29 +--------- .../sdk/ImmutableStructureTest.java | 44 --------------- .../openfeature/sdk/MutableStructureTest.java | 56 +++++++++++++++++++ .../java/dev/openfeature/sdk/ValueTest.java | 8 --- 11 files changed, 70 insertions(+), 191 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/MutableStructureTest.java diff --git a/src/main/java/dev/openfeature/sdk/AbstractStructure.java b/src/main/java/dev/openfeature/sdk/AbstractStructure.java index c8b203144..7962705c3 100644 --- a/src/main/java/dev/openfeature/sdk/AbstractStructure.java +++ b/src/main/java/dev/openfeature/sdk/AbstractStructure.java @@ -3,8 +3,10 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import lombok.EqualsAndHashCode; @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"}) +@EqualsAndHashCode abstract class AbstractStructure implements Structure { protected final Map attributes; @@ -46,46 +48,4 @@ public Map asObjectMap() { (accumulated, entry) -> accumulated.put(entry.getKey(), convertValue(entry.getValue())), HashMap::putAll); } - - public boolean equals(final Object o) { - if (o == this) { - return true; - } - if (!(o instanceof AbstractStructure)) { - return false; - } - final AbstractStructure other = (AbstractStructure) o; - if (other.attributes == attributes) { - return true; - } - if (attributes == null || other.attributes == null) { - return false; - } - if (other.attributes.size() != attributes.size()) { - return false; - } - - for (Map.Entry thisEntry : attributes.entrySet()) { - Value thisValue = thisEntry.getValue(); - Value otherValue = other.attributes.get(thisEntry.getKey()); - if (thisValue == null && otherValue == null) { - continue; - } - if (thisValue == null || otherValue == null) { - return false; - } - if (!thisValue.equals(otherValue)) { - return false; - } - } - - return true; - } - - public int hashCode() { - if (attributes == null) { - return 0; - } - return attributes.hashCode(); - } } diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java index e32e61013..c75b046e0 100644 --- a/src/main/java/dev/openfeature/sdk/EventDetails.java +++ b/src/main/java/dev/openfeature/sdk/EventDetails.java @@ -1,11 +1,13 @@ package dev.openfeature.sdk; import lombok.Data; +import lombok.EqualsAndHashCode; import lombok.experimental.SuperBuilder; /** * The details of a particular event. */ +@EqualsAndHashCode(callSuper = true) @Data @SuperBuilder(toBuilder = true) public class EventDetails extends ProviderEventDetails { diff --git a/src/main/java/dev/openfeature/sdk/ImmutableContext.java b/src/main/java/dev/openfeature/sdk/ImmutableContext.java index 9a503a33e..8560c369e 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableContext.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableContext.java @@ -4,6 +4,7 @@ import java.util.HashMap; import java.util.Map; import java.util.function.Function; +import lombok.EqualsAndHashCode; import lombok.ToString; import lombok.experimental.Delegate; @@ -15,6 +16,7 @@ * not be modified after instantiation. */ @ToString +@EqualsAndHashCode @SuppressWarnings("PMD.BeanMembersShouldSerialize") public final class ImmutableContext implements EvaluationContext { @@ -91,23 +93,6 @@ public EvaluationContext merge(EvaluationContext overridingContext) { return new ImmutableContext(attributes); } - @Override - public boolean equals(Object object) { - if (object == this) { - return true; - } - if (!(object instanceof ImmutableContext)) { - return false; - } - ImmutableContext other = (ImmutableContext) object; - return this.structure.equals(other.structure); - } - - @Override - public int hashCode() { - return structure.hashCode(); - } - @SuppressWarnings("all") private static class DelegateExclusions { @ExcludeFromGeneratedCoverageReport diff --git a/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java b/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java index 9330d90e2..7f57a174d 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableMetadata.java @@ -105,42 +105,6 @@ public boolean isNotEmpty() { return !metadata.isEmpty(); } - public boolean equals(final Object o) { - if (o == this) { - return true; - } - if (!(o instanceof ImmutableMetadata)) { - return false; - } - final ImmutableMetadata other = (ImmutableMetadata) o; - if (other.metadata == metadata) { - return true; - } - if (other.metadata.size() != metadata.size()) { - return false; - } - - for (Map.Entry thisEntry : metadata.entrySet()) { - Object thisValue = thisEntry.getValue(); - Object otherValue = other.metadata.get(thisEntry.getKey()); - if (thisValue == null && otherValue == null) { - continue; - } - if (thisValue == null || otherValue == null) { - return false; - } - if (!thisValue.equals(otherValue)) { - return false; - } - } - - return true; - } - - public int hashCode() { - return metadata.hashCode(); - } - /** * Obtain a builder for {@link ImmutableMetadata}. */ diff --git a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java index 6bb4ddc93..948f71a01 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java @@ -6,6 +6,7 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.Set; +import lombok.EqualsAndHashCode; import lombok.ToString; /** @@ -17,6 +18,7 @@ * not be modified after instantiation. All references are clones. */ @ToString +@EqualsAndHashCode(callSuper = true) @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"}) public final class ImmutableStructure extends AbstractStructure { diff --git a/src/main/java/dev/openfeature/sdk/MutableContext.java b/src/main/java/dev/openfeature/sdk/MutableContext.java index 11bcc752d..7fda58065 100644 --- a/src/main/java/dev/openfeature/sdk/MutableContext.java +++ b/src/main/java/dev/openfeature/sdk/MutableContext.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import java.util.function.Function; +import lombok.EqualsAndHashCode; import lombok.ToString; import lombok.experimental.Delegate; @@ -16,6 +17,7 @@ * be modified after instantiation. */ @ToString +@EqualsAndHashCode @SuppressWarnings("PMD.BeanMembersShouldSerialize") public class MutableContext implements EvaluationContext { @@ -123,23 +125,6 @@ public EvaluationContext merge(EvaluationContext overridingContext) { return new MutableContext(attributes); } - @Override - public boolean equals(Object object) { - if (object == this) { - return true; - } - if (!(object instanceof MutableContext)) { - return false; - } - MutableContext other = (MutableContext) object; - return this.structure.equals(other.structure); - } - - @Override - public int hashCode() { - return structure.hashCode(); - } - /** * Hidden class to tell Lombok not to copy these methods over via delegation. */ diff --git a/src/main/java/dev/openfeature/sdk/MutableStructure.java b/src/main/java/dev/openfeature/sdk/MutableStructure.java index 7e10f083e..f3158456d 100644 --- a/src/main/java/dev/openfeature/sdk/MutableStructure.java +++ b/src/main/java/dev/openfeature/sdk/MutableStructure.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import lombok.EqualsAndHashCode; import lombok.ToString; /** @@ -15,6 +16,7 @@ */ @ToString @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"}) +@EqualsAndHashCode(callSuper = true) public class MutableStructure extends AbstractStructure { public MutableStructure() { diff --git a/src/main/java/dev/openfeature/sdk/Value.java b/src/main/java/dev/openfeature/sdk/Value.java index 8df55493e..05e538e50 100644 --- a/src/main/java/dev/openfeature/sdk/Value.java +++ b/src/main/java/dev/openfeature/sdk/Value.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import lombok.EqualsAndHashCode; import lombok.SneakyThrows; import lombok.ToString; @@ -16,6 +17,7 @@ * This intermediate representation provides a good medium of exchange. */ @ToString +@EqualsAndHashCode @SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType", "checkstyle:NoFinalizer"}) public class Value implements Cloneable { @@ -314,31 +316,4 @@ public static Value objectToValue(Object object) { throw new TypeMismatchError("Flag value " + object + " had unexpected type " + object.getClass() + "."); } } - - /** - * Returns true iff {@code this} is equal to {@code o}, or if both objects represent the same data. - * @param o the other object - * @return true iff both objects are equal or represent the same data - */ - public boolean equals(final Object o) { - if (o == this) { - return true; - } - if (!(o instanceof Value)) { - return false; - } - final Value other = (Value) o; - return innerObject.equals(other.innerObject); - } - - /** - * Returns the `hashCode` of the underlying data, or 0 if {@link Value#isNull()} returns true. - * @return the hash code - */ - public int hashCode() { - if (innerObject == null) { - return 0; - } - return innerObject.hashCode(); - } } diff --git a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java index 2202d45a9..9b9a68bd4 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java @@ -185,48 +185,4 @@ void equalImmutableStructuresAreEqual() { assertEquals(structure1, structure2); } - - @Test - void unequalMutableStructuresAreNotEqual() { - MutableStructure m1 = new MutableStructure(); - m1.add("key1", "val1"); - MutableStructure m2 = new MutableStructure(); - m2.add("key2", "val2"); - assertNotEquals(m1, m2); - } - - @Test - void equalMutableStructuresAreEqual() { - MutableStructure m1 = new MutableStructure(); - m1.add("key1", "val1"); - MutableStructure m2 = new MutableStructure(); - m2.add("key1", "val1"); - assertEquals(m1, m2); - } - - @Test - void equalAbstractStructuresOfDifferentTypesAreEqual() { - MutableStructure m1 = new MutableStructure(); - m1.add("key1", "val1"); - HashMap map = new HashMap<>(); - map.put("key1", new Value("val1")); - AbstractStructure m2 = new AbstractStructure(map) { - @Override - public Set keySet() { - return attributes.keySet(); - } - - @Override - public Value getValue(String key) { - return attributes.get(key); - } - - @Override - public Map asMap() { - return attributes; - } - }; - - assertEquals(m1, m2); - } } diff --git a/src/test/java/dev/openfeature/sdk/MutableStructureTest.java b/src/test/java/dev/openfeature/sdk/MutableStructureTest.java new file mode 100644 index 000000000..9fbff54c3 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/MutableStructureTest.java @@ -0,0 +1,56 @@ +package dev.openfeature.sdk; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; + +class MutableStructureTest { + + @Test + void unequalMutableStructuresAreNotEqual() { + MutableStructure m1 = new MutableStructure(); + m1.add("key1", "val1"); + MutableStructure m2 = new MutableStructure(); + m2.add("key2", "val2"); + assertNotEquals(m1, m2); + } + + @Test + void equalMutableStructuresAreEqual() { + MutableStructure m1 = new MutableStructure(); + m1.add("key1", "val1"); + MutableStructure m2 = new MutableStructure(); + m2.add("key1", "val1"); + assertEquals(m1, m2); + } + + @Test + void equalAbstractStructuresOfDifferentTypesAreNotEqual() { + MutableStructure m1 = new MutableStructure(); + m1.add("key1", "val1"); + HashMap map = new HashMap<>(); + map.put("key1", new Value("val1")); + AbstractStructure m2 = new AbstractStructure(map) { + @Override + public Set keySet() { + return attributes.keySet(); + } + + @Override + public Value getValue(String key) { + return attributes.get(key); + } + + @Override + public Map asMap() { + return attributes; + } + }; + + assertNotEquals(m1, m2); + } +} diff --git a/src/test/java/dev/openfeature/sdk/ValueTest.java b/src/test/java/dev/openfeature/sdk/ValueTest.java index 440a26954..697edb7be 100644 --- a/src/test/java/dev/openfeature/sdk/ValueTest.java +++ b/src/test/java/dev/openfeature/sdk/ValueTest.java @@ -176,12 +176,4 @@ void unequalValuesShouldNotBeEqual() { Value val2 = new Value("b"); assertNotEquals(val1, val2); } - - @Test - void hashCodeShouldGiveHashCodeOfInnerObject() { - String innerValue = "val"; - Value val1 = new Value(innerValue); - - assertEquals(innerValue.hashCode(), val1.hashCode()); - } } From 4dba143d698ab59784a4906a7c345529b8b4deb2 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 10 Mar 2025 11:17:32 +0100 Subject: [PATCH 4/5] fix: equals and hashcode of several classes Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/ImmutableStructure.java | 14 ++++++++------ .../openfeature/sdk/ImmutableStructureTest.java | 12 ++++++++++++ .../openfeature/sdk/MutableStructureTest.java | 17 ++++++++++++++--- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java index 948f71a01..849359424 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java @@ -70,12 +70,14 @@ private static Map copyAttributes(Map in) { private static Map copyAttributes(Map in, String targetingKey) { Map copy = new HashMap<>(); - for (Entry entry : in.entrySet()) { - copy.put( - entry.getKey(), - Optional.ofNullable(entry.getValue()) - .map((Value val) -> val.clone()) - .orElse(null)); + if (in != null) { + for (Entry entry : in.entrySet()) { + copy.put( + entry.getKey(), + Optional.ofNullable(entry.getValue()) + .map((Value val) -> val.clone()) + .orElse(null)); + } } if (targetingKey != null) { copy.put(EvaluationContext.TARGETING_KEY, new Value(targetingKey)); diff --git a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java index 9b9a68bd4..6a0eed59b 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java @@ -185,4 +185,16 @@ void equalImmutableStructuresAreEqual() { assertEquals(structure1, structure2); } + + @Test + void emptyImmutableStructureIsEmpty() { + ImmutableStructure m1 = new ImmutableStructure(); + assertTrue(m1.isEmpty()); + } + + @Test + void immutableStructureWithNullAttributesIsEmpty() { + ImmutableStructure m1 = new ImmutableStructure(null); + assertTrue(m1.isEmpty()); + } } diff --git a/src/test/java/dev/openfeature/sdk/MutableStructureTest.java b/src/test/java/dev/openfeature/sdk/MutableStructureTest.java index 9fbff54c3..e9b142209 100644 --- a/src/test/java/dev/openfeature/sdk/MutableStructureTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableStructureTest.java @@ -1,15 +1,26 @@ package dev.openfeature.sdk; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; - import java.util.HashMap; import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + class MutableStructureTest { + @Test + void emptyMutableStructureIsEmpty() { + MutableStructure m1 = new MutableStructure(); + assertTrue(m1.isEmpty()); + } + + @Test + void mutableStructureWithNullBackingStructureIsEmpty() { + MutableStructure m1 = new MutableStructure(null); + assertTrue(m1.isEmpty()); + } + @Test void unequalMutableStructuresAreNotEqual() { MutableStructure m1 = new MutableStructure(); From 96dbc257c662bcdbb34b83af924e63bd8b03b1f2 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 10 Mar 2025 11:23:19 +0100 Subject: [PATCH 5/5] fix: equals and hashcode of several classes Signed-off-by: christian.lutnik --- src/test/java/dev/openfeature/sdk/MutableStructureTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/MutableStructureTest.java b/src/test/java/dev/openfeature/sdk/MutableStructureTest.java index e9b142209..ebd11af0d 100644 --- a/src/test/java/dev/openfeature/sdk/MutableStructureTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableStructureTest.java @@ -1,12 +1,12 @@ package dev.openfeature.sdk; +import static org.junit.jupiter.api.Assertions.*; + import java.util.HashMap; import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; - class MutableStructureTest { @Test