Skip to content

Commit 69b571e

Browse files
chrfwowchrfwow
authored
fix: equals and hashcode of several classes (#1364)
fix: equals and hashcode of several classes
1 parent f8df5fb commit 69b571e

11 files changed

+243
-26
lines changed

‎src/main/java/dev/openfeature/sdk/AbstractStructure.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@
33
import java.util.Collections;
44
import java.util.HashMap;
55
import java.util.Map;
6+
import lombok.EqualsAndHashCode;
67

78
@SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"})
9+
@EqualsAndHashCode
810
abstract class AbstractStructure implements Structure {
911

1012
protected final Map<String, Value> attributes;
1113

1214
@Override
1315
public boolean isEmpty() {
14-
return attributes == null || attributes.size() == 0;
16+
return attributes == null || attributes.isEmpty();
1517
}
1618

1719
AbstractStructure() {

‎src/main/java/dev/openfeature/sdk/EventDetails.java

+2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package dev.openfeature.sdk;
22

33
import lombok.Data;
4+
import lombok.EqualsAndHashCode;
45
import lombok.experimental.SuperBuilder;
56

67
/**
78
* The details of a particular event.
89
*/
10+
@EqualsAndHashCode(callSuper = true)
911
@Data
1012
@SuperBuilder(toBuilder = true)
1113
public class EventDetails extends ProviderEventDetails {

‎src/main/java/dev/openfeature/sdk/ImmutableContext.java

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.HashMap;
55
import java.util.Map;
66
import java.util.function.Function;
7+
import lombok.EqualsAndHashCode;
78
import lombok.ToString;
89
import lombok.experimental.Delegate;
910

@@ -15,6 +16,7 @@
1516
* not be modified after instantiation.
1617
*/
1718
@ToString
19+
@EqualsAndHashCode
1820
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
1921
public final class ImmutableContext implements EvaluationContext {
2022

‎src/main/java/dev/openfeature/sdk/ImmutableStructure.java

+10-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* not be modified after instantiation. All references are clones.
1919
*/
2020
@ToString
21-
@EqualsAndHashCode
21+
@EqualsAndHashCode(callSuper = true)
2222
@SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"})
2323
public final class ImmutableStructure extends AbstractStructure {
2424

@@ -38,7 +38,7 @@ public ImmutableStructure(Map<String, Value> attributes) {
3838
super(copyAttributes(attributes, null));
3939
}
4040

41-
protected ImmutableStructure(String targetingKey, Map<String, Value> attributes) {
41+
ImmutableStructure(String targetingKey, Map<String, Value> attributes) {
4242
super(copyAttributes(attributes, targetingKey));
4343
}
4444

@@ -70,12 +70,14 @@ private static Map<String, Value> copyAttributes(Map<String, Value> in) {
7070

7171
private static Map<String, Value> copyAttributes(Map<String, Value> in, String targetingKey) {
7272
Map<String, Value> copy = new HashMap<>();
73-
for (Entry<String, Value> entry : in.entrySet()) {
74-
copy.put(
75-
entry.getKey(),
76-
Optional.ofNullable(entry.getValue())
77-
.map((Value val) -> val.clone())
78-
.orElse(null));
73+
if (in != null) {
74+
for (Entry<String, Value> entry : in.entrySet()) {
75+
copy.put(
76+
entry.getKey(),
77+
Optional.ofNullable(entry.getValue())
78+
.map((Value val) -> val.clone())
79+
.orElse(null));
80+
}
7981
}
8082
if (targetingKey != null) {
8183
copy.put(EvaluationContext.TARGETING_KEY, new Value(targetingKey));

‎src/main/java/dev/openfeature/sdk/MutableStructure.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
* be modified after instantiation.
1616
*/
1717
@ToString
18-
@EqualsAndHashCode
1918
@SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"})
19+
@EqualsAndHashCode(callSuper = true)
2020
public class MutableStructure extends AbstractStructure {
2121

2222
public MutableStructure() {

‎src/test/java/dev/openfeature/sdk/ImmutableContextTest.java

+28
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY;
44
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
55
import static org.junit.jupiter.api.Assertions.assertEquals;
6+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
67
import static org.junit.jupiter.api.Assertions.assertTrue;
78

89
import java.util.Collections;
@@ -133,4 +134,31 @@ void mergeShouldRetainItsSubkeysWhenOverridingContextHasNoTargetingKey() {
133134
Structure value = key1.asStructure();
134135
assertArrayEquals(new Object[] {"key1_1"}, value.keySet().toArray());
135136
}
137+
138+
@DisplayName("Two different MutableContext objects with the different contents are not considered equal")
139+
@Test
140+
void unequalImmutableContextsAreNotEqual() {
141+
final Map<String, Value> attributes = new HashMap<>();
142+
attributes.put("key1", new Value("val1"));
143+
final ImmutableContext ctx = new ImmutableContext(attributes);
144+
145+
final Map<String, Value> attributes2 = new HashMap<>();
146+
final ImmutableContext ctx2 = new ImmutableContext(attributes2);
147+
148+
assertNotEquals(ctx, ctx2);
149+
}
150+
151+
@DisplayName("Two different MutableContext objects with the same content are considered equal")
152+
@Test
153+
void equalImmutableContextsAreEqual() {
154+
final Map<String, Value> attributes = new HashMap<>();
155+
attributes.put("key1", new Value("val1"));
156+
final ImmutableContext ctx = new ImmutableContext(attributes);
157+
158+
final Map<String, Value> attributes2 = new HashMap<>();
159+
attributes2.put("key1", new Value("val1"));
160+
final ImmutableContext ctx2 = new ImmutableContext(attributes2);
161+
162+
assertEquals(ctx, ctx2);
163+
}
136164
}

‎src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package dev.openfeature.sdk;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
5+
6+
import org.junit.jupiter.api.Test;
7+
8+
class ImmutableMetadataTest {
9+
@Test
10+
void unequalImmutableMetadataAreUnequal() {
11+
ImmutableMetadata i1 =
12+
ImmutableMetadata.builder().addString("key1", "value1").build();
13+
ImmutableMetadata i2 =
14+
ImmutableMetadata.builder().addString("key1", "value2").build();
15+
16+
assertNotEquals(i1, i2);
17+
}
18+
19+
@Test
20+
void equalImmutableMetadataAreEqual() {
21+
ImmutableMetadata i1 =
22+
ImmutableMetadata.builder().addString("key1", "value1").build();
23+
ImmutableMetadata i2 =
24+
ImmutableMetadata.builder().addString("key1", "value1").build();
25+
26+
assertEquals(i1, i2);
27+
}
28+
}

‎src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java

+44-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
package dev.openfeature.sdk;
22

3-
import static org.junit.jupiter.api.Assertions.*;
3+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
4+
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
6+
import static org.junit.jupiter.api.Assertions.assertNotSame;
7+
import static org.junit.jupiter.api.Assertions.assertNull;
8+
import static org.junit.jupiter.api.Assertions.assertTrue;
49

510
import java.time.Instant;
611
import java.time.temporal.ChronoUnit;
@@ -154,4 +159,42 @@ void constructorHandlesNullValue() {
154159
attrs.put("null", null);
155160
new ImmutableStructure(attrs);
156161
}
162+
163+
@Test
164+
void unequalImmutableStructuresAreNotEqual() {
165+
Map<String, Value> attrs1 = new HashMap<>();
166+
attrs1.put("test", new Value(45));
167+
ImmutableStructure structure1 = new ImmutableStructure(attrs1);
168+
169+
Map<String, Value> attrs2 = new HashMap<>();
170+
attrs2.put("test", new Value(2));
171+
ImmutableStructure structure2 = new ImmutableStructure(attrs2);
172+
173+
assertNotEquals(structure1, structure2);
174+
}
175+
176+
@Test
177+
void equalImmutableStructuresAreEqual() {
178+
Map<String, Value> attrs1 = new HashMap<>();
179+
attrs1.put("test", new Value(45));
180+
ImmutableStructure structure1 = new ImmutableStructure(attrs1);
181+
182+
Map<String, Value> attrs2 = new HashMap<>();
183+
attrs2.put("test", new Value(45));
184+
ImmutableStructure structure2 = new ImmutableStructure(attrs2);
185+
186+
assertEquals(structure1, structure2);
187+
}
188+
189+
@Test
190+
void emptyImmutableStructureIsEmpty() {
191+
ImmutableStructure m1 = new ImmutableStructure();
192+
assertTrue(m1.isEmpty());
193+
}
194+
195+
@Test
196+
void immutableStructureWithNullAttributesIsEmpty() {
197+
ImmutableStructure m1 = new ImmutableStructure(null);
198+
assertTrue(m1.isEmpty());
199+
}
157200
}

‎src/test/java/dev/openfeature/sdk/MutableContextTest.java

+28
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY;
44
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
55
import static org.junit.jupiter.api.Assertions.assertEquals;
6+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
67
import static org.junit.jupiter.api.Assertions.assertTrue;
78

89
import java.util.Collections;
@@ -137,4 +138,31 @@ void shouldAllowChainingOfMutations() {
137138
assertEquals(2, context.getValue("key2").asInteger());
138139
assertEquals(3.0, context.getValue("key3").asDouble());
139140
}
141+
142+
@DisplayName("Two different MutableContext objects with the different contents are not considered equal")
143+
@Test
144+
void unequalMutableContextsAreNotEqual() {
145+
final Map<String, Value> attributes = new HashMap<>();
146+
attributes.put("key1", new Value("val1"));
147+
final MutableContext ctx = new MutableContext(attributes);
148+
149+
final Map<String, Value> attributes2 = new HashMap<>();
150+
final MutableContext ctx2 = new MutableContext(attributes2);
151+
152+
assertNotEquals(ctx, ctx2);
153+
}
154+
155+
@DisplayName("Two different MutableContext objects with the same content are considered equal")
156+
@Test
157+
void equalMutableContextsAreEqual() {
158+
final Map<String, Value> attributes = new HashMap<>();
159+
attributes.put("key1", new Value("val1"));
160+
final MutableContext ctx = new MutableContext(attributes);
161+
162+
final Map<String, Value> attributes2 = new HashMap<>();
163+
attributes2.put("key1", new Value("val1"));
164+
final MutableContext ctx2 = new MutableContext(attributes2);
165+
166+
assertEquals(ctx, ctx2);
167+
}
140168
}

‎src/test/java/dev/openfeature/sdk/MutableStructureTest.java

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package dev.openfeature.sdk;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import java.util.HashMap;
6+
import java.util.Map;
7+
import java.util.Set;
8+
import org.junit.jupiter.api.Test;
9+
10+
class MutableStructureTest {
11+
12+
@Test
13+
void emptyMutableStructureIsEmpty() {
14+
MutableStructure m1 = new MutableStructure();
15+
assertTrue(m1.isEmpty());
16+
}
17+
18+
@Test
19+
void mutableStructureWithNullBackingStructureIsEmpty() {
20+
MutableStructure m1 = new MutableStructure(null);
21+
assertTrue(m1.isEmpty());
22+
}
23+
24+
@Test
25+
void unequalMutableStructuresAreNotEqual() {
26+
MutableStructure m1 = new MutableStructure();
27+
m1.add("key1", "val1");
28+
MutableStructure m2 = new MutableStructure();
29+
m2.add("key2", "val2");
30+
assertNotEquals(m1, m2);
31+
}
32+
33+
@Test
34+
void equalMutableStructuresAreEqual() {
35+
MutableStructure m1 = new MutableStructure();
36+
m1.add("key1", "val1");
37+
MutableStructure m2 = new MutableStructure();
38+
m2.add("key1", "val1");
39+
assertEquals(m1, m2);
40+
}
41+
42+
@Test
43+
void equalAbstractStructuresOfDifferentTypesAreNotEqual() {
44+
MutableStructure m1 = new MutableStructure();
45+
m1.add("key1", "val1");
46+
HashMap<String, Value> map = new HashMap<>();
47+
map.put("key1", new Value("val1"));
48+
AbstractStructure m2 = new AbstractStructure(map) {
49+
@Override
50+
public Set<String> keySet() {
51+
return attributes.keySet();
52+
}
53+
54+
@Override
55+
public Value getValue(String key) {
56+
return attributes.get(key);
57+
}
58+
59+
@Override
60+
public Map<String, Value> asMap() {
61+
return attributes;
62+
}
63+
};
64+
65+
assertNotEquals(m1, m2);
66+
}
67+
}

0 commit comments

Comments
 (0)