Skip to content

Commit 6b7f8dc

Browse files
author
vishalup29
committed
Issue #1754 Implement equals/hashCode for EvaluationContext and add tests.
Signed-off-by: vishalup29 <vishalupadhyay977@gmail.com>
1 parent 79704e4 commit 6b7f8dc

File tree

4 files changed

+159
-43
lines changed

4 files changed

+159
-43
lines changed

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

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import java.util.HashMap;
66
import java.util.Map;
77
import java.util.function.Function;
8-
import lombok.EqualsAndHashCode;
98
import lombok.ToString;
109
import lombok.experimental.Delegate;
1110

@@ -17,7 +16,6 @@
1716
* not be modified after instantiation.
1817
*/
1918
@ToString
20-
@EqualsAndHashCode
2119
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
2220
public final class ImmutableContext implements EvaluationContext {
2321

@@ -26,6 +24,9 @@ public final class ImmutableContext implements EvaluationContext {
2624
@Delegate(excludes = DelegateExclusions.class)
2725
private final ImmutableStructure structure;
2826

27+
// Lazily computed hash code, safe because this class is immutable.
28+
private volatile Integer cachedHashCode;
29+
2930
/**
3031
* Create an immutable context with an empty targeting_key and attributes
3132
* provided.
@@ -96,6 +97,47 @@ public EvaluationContext merge(EvaluationContext overridingContext) {
9697
return new ImmutableContext(attributes);
9798
}
9899

100+
/**
101+
* Equality for EvaluationContext implementations is defined in terms of their resolved
102+
* attribute maps. Two contexts are considered equal if their {@link #asMap()} representations
103+
* contain the same key/value pairs, regardless of how the context was constructed or layered.
104+
*
105+
* @param o the object to compare with this context
106+
* @return true if the other object is an EvaluationContext whose resolved attributes match
107+
*/
108+
@Override
109+
public boolean equals(Object o) {
110+
if (this == o) {
111+
return true;
112+
}
113+
if (!(o instanceof EvaluationContext)) {
114+
return false;
115+
}
116+
EvaluationContext that = (EvaluationContext) o;
117+
return this.asMap().equals(that.asMap());
118+
}
119+
120+
/**
121+
* Computes a hash code consistent with {@link #equals(Object)}. Since this context is immutable,
122+
* the hash code is lazily computed once from its resolved attribute map and then cached.
123+
*
124+
* @return the cached hash code derived from this context's attribute map
125+
*/
126+
@Override
127+
public int hashCode() {
128+
Integer result = cachedHashCode;
129+
if (result == null) {
130+
synchronized (this) {
131+
result = cachedHashCode;
132+
if (result == null) {
133+
result = asMap().hashCode();
134+
cachedHashCode = result;
135+
}
136+
}
137+
}
138+
return result;
139+
}
140+
99141
@SuppressWarnings("all")
100142
private static class DelegateExclusions {
101143
@ExcludeFromGeneratedCoverageReport

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ public class LayeredEvaluationContext implements EvaluationContext {
2121
private ArrayList<EvaluationContext> hookContexts;
2222
private String targetingKey;
2323
private Set<String> keySet = null;
24+
// Lazily computed resolved attribute map for this layered context.
25+
// This must be invalidated whenever the underlying layers change.
26+
private Map<String, Value> cachedMap;
2427

2528
/**
2629
* Constructor for LayeredEvaluationContext.
@@ -174,15 +177,20 @@ public Value getValue(String key) {
174177
return getFromContext(apiContext, key);
175178
}
176179

177-
@Override
178-
public Map<String, Value> asMap() {
180+
private Map<String, Value> getResolvedMap() {
181+
if (cachedMap != null) {
182+
return cachedMap;
183+
}
184+
179185
if (keySet != null && keySet.isEmpty()) {
180-
return new HashMap<>(0);
186+
cachedMap = Collections.emptyMap();
187+
return cachedMap;
181188
}
182189

183190
HashMap<String, Value> map;
184191
if (keySet != null) {
185-
map = new HashMap<>(keySet.size());
192+
// use helper to size the map based on expected entries
193+
map = HashMapUtils.forEntries(keySet.size());
186194
} else {
187195
map = new HashMap<>();
188196
}
@@ -205,7 +213,15 @@ public Map<String, Value> asMap() {
205213
map.putAll(hookContext.asMap());
206214
}
207215
}
208-
return map;
216+
217+
cachedMap = Collections.unmodifiableMap(map);
218+
return cachedMap;
219+
}
220+
221+
@Override
222+
public Map<String, Value> asMap() {
223+
// Return a defensive copy so callers can't mutate our cached map.
224+
return new HashMap<>(getResolvedMap());
209225
}
210226

211227
@Override
@@ -214,41 +230,48 @@ public Map<String, Value> asUnmodifiableMap() {
214230
return Collections.emptyMap();
215231
}
216232

217-
return Collections.unmodifiableMap(asMap());
233+
return getResolvedMap();
218234
}
219235

220236
@Override
221237
public Map<String, Object> asObjectMap() {
222-
if (keySet != null && keySet.isEmpty()) {
238+
// Build the object map directly from the resolved attribute map,
239+
// so this stays consistent with equals/hashCode and asMap().
240+
Map<String, Value> resolved = getResolvedMap();
241+
if (resolved.isEmpty()) {
223242
return new HashMap<>(0);
224243
}
225244

226-
HashMap<String, Object> map;
227-
if (keySet != null) {
228-
map = new HashMap<>(keySet.size());
229-
} else {
230-
map = new HashMap<>();
245+
HashMap<String, Object> map = HashMapUtils.forEntries(resolved.size());
246+
for (Map.Entry<String, Value> entry : resolved.entrySet()) {
247+
Value value = entry.getValue();
248+
// Value is responsible for exposing the underlying Java representation.
249+
map.put(entry.getKey(), value == null ? null : value.asObject());
231250
}
251+
return map;
252+
}
232253

233-
if (apiContext != null) {
234-
map.putAll(apiContext.asObjectMap());
235-
}
236-
if (transactionContext != null) {
237-
map.putAll(transactionContext.asObjectMap());
238-
}
239-
if (clientContext != null) {
240-
map.putAll(clientContext.asObjectMap());
254+
@Override
255+
public boolean equals(Object o) {
256+
if (this == o) {
257+
return true;
241258
}
242-
if (invocationContext != null) {
243-
map.putAll(invocationContext.asObjectMap());
259+
if (!(o instanceof EvaluationContext)) {
260+
return false;
244261
}
245-
if (hookContexts != null) {
246-
for (int i = 0; i < hookContexts.size(); i++) {
247-
EvaluationContext hookContext = hookContexts.get(i);
248-
map.putAll(hookContext.asObjectMap());
249-
}
262+
263+
EvaluationContext that = (EvaluationContext) o;
264+
265+
if (that instanceof LayeredEvaluationContext) {
266+
return this.getResolvedMap().equals(((LayeredEvaluationContext) that).getResolvedMap());
250267
}
251-
return map;
268+
269+
return this.getResolvedMap().equals(that.asMap());
270+
}
271+
272+
@Override
273+
public int hashCode() {
274+
return getResolvedMap().hashCode();
252275
}
253276

254277
void putHookContext(EvaluationContext context) {
@@ -265,5 +288,6 @@ void putHookContext(EvaluationContext context) {
265288
}
266289
this.hookContexts.add(context);
267290
this.keySet = null;
291+
this.cachedMap = null;
268292
}
269293
}

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

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
55
import static org.junit.jupiter.api.Assertions.assertEquals;
66
import static org.junit.jupiter.api.Assertions.assertNotEquals;
7+
import static org.junit.jupiter.api.Assertions.assertNull;
78
import static org.junit.jupiter.api.Assertions.assertTrue;
89

10+
import java.util.Arrays;
911
import java.util.Collections;
1012
import java.util.HashMap;
13+
import java.util.HashSet;
1114
import java.util.Map;
1215
import org.junit.jupiter.api.DisplayName;
1316
import org.junit.jupiter.api.Test;
@@ -50,7 +53,7 @@ void shouldChangeTargetingKeyFromOverridingContext() {
5053
assertEquals("overriding_key", merge.getTargetingKey());
5154
}
5255

53-
@DisplayName("targeting key should not changed from the overriding context if missing")
56+
@DisplayName("targeting key should not be changed from the overriding context if missing")
5457
@Test
5558
void shouldRetainTargetingKeyWhenOverridingContextTargetingKeyValueIsEmpty() {
5659
HashMap<String, Value> attributes = new HashMap<>();
@@ -66,7 +69,7 @@ void shouldRetainTargetingKeyWhenOverridingContextTargetingKeyValueIsEmpty() {
6669
@Test
6770
void missingTargetingKeyShould() {
6871
EvaluationContext ctx = new ImmutableContext();
69-
assertEquals(null, ctx.getTargetingKey());
72+
assertNull(ctx.getTargetingKey());
7073
}
7174

7275
@DisplayName("Merge should retain all the attributes from the existing context when overriding context is null")
@@ -145,10 +148,26 @@ void mergeShouldObtainKeysFromOverridingContextWhenExistingContextIsEmpty() {
145148
EvaluationContext ctx = new ImmutableContext();
146149
EvaluationContext overriding = new ImmutableContext(attributes);
147150
EvaluationContext merge = ctx.merge(overriding);
148-
assertEquals(new java.util.HashSet<>(java.util.Arrays.asList("key1", "key2")), merge.keySet());
151+
assertEquals(new HashSet<>(Arrays.asList("key1", "key2")), merge.keySet());
149152
}
150153

151-
@DisplayName("Two different MutableContext objects with the different contents are not considered equal")
154+
@DisplayName("Two ImmutableContext objects with identical attributes are considered equal")
155+
@Test
156+
void testImmutableContextEquality() {
157+
Map<String, Value> map1 = new HashMap<>();
158+
map1.put("key", new Value("value"));
159+
160+
Map<String, Value> map2 = new HashMap<>();
161+
map2.put("key", new Value("value"));
162+
163+
ImmutableContext a = new ImmutableContext(null, map1);
164+
ImmutableContext b = new ImmutableContext(null, map2);
165+
166+
assertEquals(a, b);
167+
assertEquals(a.hashCode(), b.hashCode());
168+
}
169+
170+
@DisplayName("Two different ImmutableContext objects with different contents are not considered equal")
152171
@Test
153172
void unequalImmutableContextsAreNotEqual() {
154173
final Map<String, Value> attributes = new HashMap<>();
@@ -161,17 +180,16 @@ void unequalImmutableContextsAreNotEqual() {
161180
assertNotEquals(ctx, ctx2);
162181
}
163182

164-
@DisplayName("Two different MutableContext objects with the same content are considered equal")
183+
@DisplayName("ImmutableContext hashCode is stable across multiple invocations")
165184
@Test
166-
void equalImmutableContextsAreEqual() {
167-
final Map<String, Value> attributes = new HashMap<>();
168-
attributes.put("key1", new Value("val1"));
169-
final ImmutableContext ctx = new ImmutableContext(attributes);
185+
void immutableContextHashCodeIsStable() {
186+
Map<String, Value> map = new HashMap<>();
187+
map.put("key", new Value("value"));
170188

171-
final Map<String, Value> attributes2 = new HashMap<>();
172-
attributes2.put("key1", new Value("val1"));
173-
final ImmutableContext ctx2 = new ImmutableContext(attributes2);
189+
ImmutableContext ctx = new ImmutableContext(null, map);
174190

175-
assertEquals(ctx, ctx2);
191+
int first = ctx.hashCode();
192+
int second = ctx.hashCode();
193+
assertEquals(first, second);
176194
}
177195
}

src/test/java/dev/openfeature/sdk/LayeredEvaluationContextTest.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,5 +397,37 @@ void mergesCorrectlyWhenOtherHasNoTargetingKey() {
397397
merged.asMap());
398398
assertEquals(invocationContext.getTargetingKey(), merged.getTargetingKey());
399399
}
400+
401+
@Test
402+
void testLayeredContextEquality() {
403+
Map<String, Value> baseMap = Map.of("k", new Value("v"));
404+
Map<String, Value> layerMap = Map.of("x", new Value("y"));
405+
406+
EvaluationContext base = new MutableContext(null, baseMap);
407+
EvaluationContext layer = new MutableContext(null, layerMap);
408+
409+
LayeredEvaluationContext l1 = new LayeredEvaluationContext(base, layer, null, null);
410+
LayeredEvaluationContext l2 = new LayeredEvaluationContext(base, layer, null, null);
411+
412+
assertEquals(l1, l2);
413+
assertEquals(l1.hashCode(), l2.hashCode());
414+
}
415+
416+
@Test
417+
void testMixedContextEquality() {
418+
Map<String, Value> map = Map.of("foo", new Value("bar"));
419+
420+
EvaluationContext base = new MutableContext(null, map);
421+
LayeredEvaluationContext layered = new LayeredEvaluationContext(null, null, null, base);
422+
423+
// Equality from the layered context's perspective (map-based equality)
424+
assertEquals(layered, base);
425+
426+
// Resolved maps should be identical
427+
assertEquals(base.asMap(), layered.asMap());
428+
429+
// Layered's hashCode must be consistent with its resolved attribute map
430+
assertEquals(base.asMap().hashCode(), layered.hashCode());
431+
}
400432
}
401433
}

0 commit comments

Comments
 (0)