Skip to content
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.

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

fix: equals and hashcode of several classes #1364

Merged
merged 7 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/main/java/dev/openfeature/sdk/AbstractStructure.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
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<String, Value> attributes;

@Override
public boolean isEmpty() {
return attributes == null || attributes.size() == 0;
return attributes == null || attributes.isEmpty();
}

AbstractStructure() {
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/dev/openfeature/sdk/EventDetails.java
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/dev/openfeature/sdk/ImmutableContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -15,6 +16,7 @@
* not be modified after instantiation.
*/
@ToString
@EqualsAndHashCode
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
public final class ImmutableContext implements EvaluationContext {

Expand Down
18 changes: 10 additions & 8 deletions src/main/java/dev/openfeature/sdk/ImmutableStructure.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* not be modified after instantiation. All references are clones.
*/
@ToString
@EqualsAndHashCode
@EqualsAndHashCode(callSuper = true)
@SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"})
public final class ImmutableStructure extends AbstractStructure {

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

protected ImmutableStructure(String targetingKey, Map<String, Value> attributes) {
ImmutableStructure(String targetingKey, Map<String, Value> attributes) {
super(copyAttributes(attributes, targetingKey));
}

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

private static Map<String, Value> copyAttributes(Map<String, Value> in, String targetingKey) {
Map<String, Value> copy = new HashMap<>();
for (Entry<String, Value> entry : in.entrySet()) {
copy.put(
entry.getKey(),
Optional.ofNullable(entry.getValue())
.map((Value val) -> val.clone())
.orElse(null));
if (in != null) {
for (Entry<String, Value> 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));
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/dev/openfeature/sdk/MutableStructure.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
* be modified after instantiation.
*/
@ToString
@EqualsAndHashCode
@SuppressWarnings({"PMD.BeanMembersShouldSerialize", "checkstyle:MissingJavadocType"})
@EqualsAndHashCode(callSuper = true)
public class MutableStructure extends AbstractStructure {

public MutableStructure() {
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/dev/openfeature/sdk/ImmutableContextTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, Value> attributes = new HashMap<>();
attributes.put("key1", new Value("val1"));
final ImmutableContext ctx = new ImmutableContext(attributes);

final Map<String, Value> 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<String, Value> attributes = new HashMap<>();
attributes.put("key1", new Value("val1"));
final ImmutableContext ctx = new ImmutableContext(attributes);

final Map<String, Value> attributes2 = new HashMap<>();
attributes2.put("key1", new Value("val1"));
final ImmutableContext ctx2 = new ImmutableContext(attributes2);

assertEquals(ctx, ctx2);
}
}
28 changes: 28 additions & 0 deletions src/test/java/dev/openfeature/sdk/ImmutableMetadataTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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);
}
}
45 changes: 44 additions & 1 deletion src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -154,4 +159,42 @@ void constructorHandlesNullValue() {
attrs.put("null", null);
new ImmutableStructure(attrs);
}

@Test
void unequalImmutableStructuresAreNotEqual() {
Map<String, Value> attrs1 = new HashMap<>();
attrs1.put("test", new Value(45));
ImmutableStructure structure1 = new ImmutableStructure(attrs1);

Map<String, Value> attrs2 = new HashMap<>();
attrs2.put("test", new Value(2));
ImmutableStructure structure2 = new ImmutableStructure(attrs2);

assertNotEquals(structure1, structure2);
}

@Test
void equalImmutableStructuresAreEqual() {
Map<String, Value> attrs1 = new HashMap<>();
attrs1.put("test", new Value(45));
ImmutableStructure structure1 = new ImmutableStructure(attrs1);

Map<String, Value> attrs2 = new HashMap<>();
attrs2.put("test", new Value(45));
ImmutableStructure structure2 = new ImmutableStructure(attrs2);

assertEquals(structure1, structure2);
}

@Test
void emptyImmutableStructureIsEmpty() {
ImmutableStructure m1 = new ImmutableStructure();
assertTrue(m1.isEmpty());
}

@Test
void immutableStructureWithNullAttributesIsEmpty() {
ImmutableStructure m1 = new ImmutableStructure(null);
assertTrue(m1.isEmpty());
}
}
28 changes: 28 additions & 0 deletions src/test/java/dev/openfeature/sdk/MutableContextTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, Value> attributes = new HashMap<>();
attributes.put("key1", new Value("val1"));
final MutableContext ctx = new MutableContext(attributes);

final Map<String, Value> 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<String, Value> attributes = new HashMap<>();
attributes.put("key1", new Value("val1"));
final MutableContext ctx = new MutableContext(attributes);

final Map<String, Value> attributes2 = new HashMap<>();
attributes2.put("key1", new Value("val1"));
final MutableContext ctx2 = new MutableContext(attributes2);

assertEquals(ctx, ctx2);
}
}
67 changes: 67 additions & 0 deletions src/test/java/dev/openfeature/sdk/MutableStructureTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
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;

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();
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<String, Value> map = new HashMap<>();
map.put("key1", new Value("val1"));
AbstractStructure m2 = new AbstractStructure(map) {
@Override
public Set<String> keySet() {
return attributes.keySet();
}

@Override
public Value getValue(String key) {
return attributes.get(key);
}

@Override
public Map<String, Value> asMap() {
return attributes;
}
};

assertNotEquals(m1, m2);
}
}
Loading
Loading