Skip to content

fix: Define equality behavior for EvaluationContext implementations #1756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Define equality behavior for EvaluationContext implementations #1756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/main/java/dev/openfeature/sdk/ImmutableContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,38 @@ public EvaluationContext merge(EvaluationContext overridingContext) {
return new ImmutableContext(attributes);
}

/**
* Equality for EvaluationContext implementations is defined in terms of their resolved
* attribute maps. Two contexts are considered equal if their {@link #asMap()} representations
* contain the same key/value pairs, regardless of how the context was constructed or layered.
*
* @param o the object to compare with this context
* @return true if the other object is an EvaluationContext whose resolved attributes match
*/
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof EvaluationContext)) {
return false;
}
EvaluationContext that = (EvaluationContext) o;
return this.asMap().equals(that.asMap());
}

/**
* Computes a hash code consistent with {@link #equals(Object)} by delegating to the
* hash code of this context's resolved attribute map. This ensures that contexts with
* identical effective attributes will have the same hash code.
*
* @return the hash code derived from this context's attribute map
*/
@Override
public int hashCode() {
return asMap().hashCode();
}
Comment on lines +127 to +129
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This implementation is correct, but asMap() may be an expensive call if this object is used frequently in hash-based collections. Since ImmutableContext is immutable, you could cache the hash code after its first computation to improve performance. This is a common optimization for immutable objects.

For example, you could introduce a private volatile Integer cachedHashCode; field and use lazy initialization in a thread-safe way:

private volatile Integer cachedHashCode;

@Override
public int hashCode() {
    if (cachedHashCode == null) {
        synchronized (this) {
            if (cachedHashCode == null) {
                cachedHashCode = asMap().hashCode();
            }
        }
    }
    return cachedHashCode;
}


@SuppressWarnings("all")
private static class DelegateExclusions {
@ExcludeFromGeneratedCoverageReport
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,24 @@ public Map<String, Object> asObjectMap() {
return map;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof EvaluationContext)) {
return false;
}

EvaluationContext that = (EvaluationContext) o;
return this.asMap().equals(that.asMap());
}

@Override
public int hashCode() {
return asMap().hashCode();
}
Comment on lines +255 to +270
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The logic here is correct, but it has a significant performance implication. The asMap() method on LayeredEvaluationContext is expensive, as it constructs a new map by merging all context layers every time it's called. Calling this in equals() and hashCode() can lead to performance bottlenecks, especially if these objects are used in collections.

I recommend caching the resolved map. The cache can be invalidated in putHookContext(), similar to how keySet is handled.

Here's a potential refactoring:

  1. Introduce a private Map<String, Value> cachedMap; field.
  2. Create a private helper method to get or build the map.
  3. Update asMap(), equals(), and hashCode() to use the helper.
  4. Invalidate cachedMap in putHookContext().

Example:

// Add this field
private Map<String, Value> cachedMap;

// New helper method
private Map<String, Value> getResolvedMap() {
    if (cachedMap == null) {
        // logic from asMap() to build the map
        HashMap<String, Value> map = new HashMap<>();
        // ... build map ...
        cachedMap = map;
    }
    return cachedMap;
}

@Override
public Map<String, Value> asMap() {
    // Return a copy to prevent mutation of the cached map
    return new HashMap<>(getResolvedMap());
}

@Override
public boolean equals(Object o) {
    if (this == o) {
        return true;
    }
    if (!(o instanceof EvaluationContext)) {
        return false;
    }
    EvaluationContext that = (EvaluationContext) o;
    return this.getResolvedMap().equals(that.asMap());
}

@Override
public int hashCode() {
    return getResolvedMap().hashCode();
}

void putHookContext(EvaluationContext context) {
    // ... existing logic
    this.cachedMap = null; // Invalidate cache
    this.keySet = null;
}

This change would greatly improve the performance of equals and hashCode.


void putHookContext(EvaluationContext context) {
if (context == null || context.isEmpty()) {
return;
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/dev/openfeature/sdk/ImmutableContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,22 @@ void mergeShouldObtainKeysFromOverridingContextWhenExistingContextIsEmpty() {
assertEquals(new java.util.HashSet<>(java.util.Arrays.asList("key1", "key2")), merge.keySet());
}

@DisplayName("Two ImmutableContext objects with identical attributes are considered equal")
@Test
void testImmutableContextEquality() {
Map<String, Value> map1 = new HashMap<>();
map1.put("key", new Value("value"));

Map<String, Value> map2 = new HashMap<>();
map2.put("key", new Value("value"));

ImmutableContext a = new ImmutableContext(null, map1);
ImmutableContext b = new ImmutableContext(null, map2);

assertEquals(a, b);
assertEquals(a.hashCode(), b.hashCode());
}

@DisplayName("Two different MutableContext objects with the different contents are not considered equal")
@Test
void unequalImmutableContextsAreNotEqual() {
Expand Down
27 changes: 27 additions & 0 deletions src/test/java/dev/openfeature/sdk/LayeredEvaluationContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,5 +397,32 @@ void mergesCorrectlyWhenOtherHasNoTargetingKey() {
merged.asMap());
assertEquals(invocationContext.getTargetingKey(), merged.getTargetingKey());
}

@Test
void testLayeredContextEquality() {
Map<String, Value> baseMap = Map.of("k", new Value("v"));
Map<String, Value> layerMap = Map.of("x", new Value("y"));

EvaluationContext base = new MutableContext(null, baseMap);
EvaluationContext layer = new MutableContext(null, layerMap);

LayeredEvaluationContext l1 = new LayeredEvaluationContext(base, layer, null, null);
LayeredEvaluationContext l2 = new LayeredEvaluationContext(base, layer, null, null);

assertEquals(l1, l2);
assertEquals(l1.hashCode(), l2.hashCode());
}

@Test
void testMixedContextEquality() {
Map<String, Value> map = Map.of("foo", new Value("bar"));

EvaluationContext base = new MutableContext(null, map);
LayeredEvaluationContext layered = new LayeredEvaluationContext(null, null, null, base);

assertEquals(base, layered);
assertEquals(layered, base);
assertEquals(base.hashCode(), layered.hashCode());
}
}
}
Loading