diff --git a/lib/shared/common/src/main/java/com/launchdarkly/sdk/AttributeMap.java b/lib/shared/common/src/main/java/com/launchdarkly/sdk/AttributeMap.java new file mode 100644 index 0000000..9ca1c2d --- /dev/null +++ b/lib/shared/common/src/main/java/com/launchdarkly/sdk/AttributeMap.java @@ -0,0 +1,87 @@ +package com.launchdarkly.sdk; + +import java.util.HashMap; +import java.util.Map; + +final class AttributeMap { + private final AttributeMap parent; + private final Map map; + + AttributeMap() { + this(null); + } + + AttributeMap(AttributeMap parent) { + this.parent = parent; + this.map = new HashMap<>(); + } + + LDValue get(String key) { + AttributeMap current = this; + while (current != null) { + LDValue value = current.map.get(key); + if (value != null) { + if (value.isNull()) { + break; + } + return value; + } + current = current.parent; + } + return null; + } + + void put(String key, LDValue value) { + map.put(key, value); + } + + @Override + public int hashCode() { + return flatten().hashCode(); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof AttributeMap)) { + return false; + } + AttributeMap o = (AttributeMap) other; + return flatten().equals(o.flatten()); + } + + Map flatten() { + if (parent == null) { + return map; + } + Map out = new HashMap<>(); + flattenRecursive(out); + return out; + } + + private void flattenRecursive(Map out) { + if (parent != null) { + parent.flattenRecursive(out); + } + for (Map.Entry entry : map.entrySet()) { + String key = entry.getKey(); + LDValue value = entry.getValue(); + if (value.isNull()) { + out.remove(key); + } else { + out.put(key, value); + } + } + } + + void remove(String key) { + if (parent == null) { + map.remove(key); + return; + } + // we need to hide the value from the parents + map.put(key, LDValue.ofNull()); + } +} \ No newline at end of file diff --git a/lib/shared/common/src/main/java/com/launchdarkly/sdk/ContextBuilder.java b/lib/shared/common/src/main/java/com/launchdarkly/sdk/ContextBuilder.java index ac220c2..d9e5fa5 100644 --- a/lib/shared/common/src/main/java/com/launchdarkly/sdk/ContextBuilder.java +++ b/lib/shared/common/src/main/java/com/launchdarkly/sdk/ContextBuilder.java @@ -1,9 +1,7 @@ package com.launchdarkly.sdk; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** * A mutable object that uses the builder pattern to specify properties for {@link LDContext}. @@ -34,7 +32,7 @@ public final class ContextBuilder { private ContextKind kind; private String key; private String name; - private Map attributes; + private AttributeMap attributes; private boolean anonymous; private List privateAttributes; private boolean copyOnWriteAttributes; @@ -303,7 +301,7 @@ public boolean trySet(String attributeName, LDValue value) { return false; default: if (copyOnWriteAttributes) { - attributes = new HashMap<>(attributes); + attributes = new AttributeMap(attributes); copyOnWriteAttributes = false; } if (value == null || value.isNull()) { @@ -312,7 +310,7 @@ public boolean trySet(String attributeName, LDValue value) { } } else { if (attributes == null) { - attributes = new HashMap<>(); + attributes = new AttributeMap(); } attributes.put(attributeName, value); } diff --git a/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContext.java b/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContext.java index cb01c39..986952f 100644 --- a/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContext.java +++ b/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContext.java @@ -58,7 +58,7 @@ public final class LDContext implements JsonSerializable { final String key; final String fullyQualifiedKey; final String name; - final Map attributes; + final AttributeMap attributes; final boolean anonymous; final List privateAttributes; @@ -68,7 +68,7 @@ private LDContext( String key, String fullyQualifiedKey, String name, - Map attributes, + AttributeMap attributes, boolean anonymous, List privateAttributes ) { @@ -100,7 +100,7 @@ static LDContext createSingle( ContextKind kind, String key, String name, - Map attributes, + AttributeMap attributes, boolean anonymous, List privateAttributes, boolean allowEmptyKey // allowEmptyKey is true only when deserializing old-style user JSON @@ -300,7 +300,7 @@ public static LDContext fromUser(LDUser user) { return failed(Errors.CONTEXT_NO_KEY); } } - Map attributes = null; + AttributeMap attributes = null; for (UserAttribute a: UserAttribute.OPTIONAL_STRING_ATTRIBUTES) { if (a == UserAttribute.NAME) { continue; @@ -308,14 +308,14 @@ public static LDContext fromUser(LDUser user) { LDValue value = user.getAttribute(a); if (!value.isNull()) { if (attributes == null) { - attributes = new HashMap<>(); + attributes = new AttributeMap(); } attributes.put(a.getName(), value); } } if (user.custom != null && !user.custom.isEmpty()) { if (attributes == null) { - attributes = new HashMap<>(); + attributes = new AttributeMap(); } for (Map.Entry kv: user.custom.entrySet()) { attributes.put(kv.getKey().getName(), kv.getValue()); @@ -671,7 +671,7 @@ public LDValue getValue(AttributeRef attributeRef) { * @return an iterable of strings (may be empty, but will never be null) */ public Iterable getCustomAttributeNames() { - return attributes == null ? Collections.emptyList() : attributes.keySet(); + return attributes == null ? Collections.emptyList() : attributes.flatten().keySet(); } /** @@ -853,17 +853,9 @@ public boolean equals(Object other) { if (!Objects.equals(key, o.key) || !Objects.equals(name, o.name) || anonymous != o.anonymous) { return false; } - if ((attributes == null ? 0 : attributes.size()) != - (o.attributes == null ? 0 : o.attributes.size())) { + if (!Objects.equals(attributes, o.attributes)) { return false; } - if (attributes != null) { - for (Map.Entry kv: attributes.entrySet()) { - if (!Objects.equals(o.attributes.get(kv.getKey()), kv.getValue())) { - return false; - } - } - } if (getPrivateAttributeCount() != o.getPrivateAttributeCount()) { return false; } @@ -886,10 +878,8 @@ public boolean equals(Object other) { @Override public int hashCode() { - // This implementation of hashCode() is inefficient due to the need to create a predictable ordering - // of attribute names. That's necessary just for the sake of aligning with the behavior of equals(), - // which is insensitive to ordering. However, using an LDContext as a map key is not an anticipated - // or recommended use case. + // This implementation of hashCode() is inefficient due to the need to flatten the attributes map. + // However, using an LDContext as a map key is not an anticipated or recommended use case. int h = Objects.hash(error, kind, key, name, anonymous); if (multiContexts != null) { for (LDContext c: multiContexts) { @@ -897,11 +887,7 @@ public int hashCode() { } } if (attributes != null) { - String[] names = attributes.keySet().toArray(new String[attributes.size()]); - Arrays.sort(names); - for (String name: names) { - h = (h * 17 + name.hashCode()) * 17 + attributes.get(name).hashCode(); - } + h = h * 17 + attributes.hashCode(); } if (privateAttributes != null) { AttributeRef[] refs = privateAttributes.toArray(new AttributeRef[privateAttributes.size()]); diff --git a/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContextTypeAdapter.java b/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContextTypeAdapter.java index f0ca4d1..7b6ba1c 100644 --- a/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContextTypeAdapter.java +++ b/lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContextTypeAdapter.java @@ -51,7 +51,7 @@ private void writeSingleKind(JsonWriter out, LDContext c, boolean includeKind) t out.name(ATTR_ANONYMOUS).value(c.isAnonymous()); } if (c.attributes != null) { - for (Map.Entry kv: c.attributes.entrySet()) { + for (Map.Entry kv: c.attributes.flatten().entrySet()) { out.name(kv.getKey()); LDValueTypeAdapter.INSTANCE.write(out, kv.getValue()); } diff --git a/lib/shared/common/src/test/java/com/launchdarkly/sdk/ContextBuilderTest.java b/lib/shared/common/src/test/java/com/launchdarkly/sdk/ContextBuilderTest.java index 2d8e491..65b84ba 100644 --- a/lib/shared/common/src/test/java/com/launchdarkly/sdk/ContextBuilderTest.java +++ b/lib/shared/common/src/test/java/com/launchdarkly/sdk/ContextBuilderTest.java @@ -2,7 +2,9 @@ import org.junit.Test; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static com.launchdarkly.sdk.LDContextTest.kind1; import static com.launchdarkly.sdk.LDContextTest.kind2; @@ -75,6 +77,35 @@ public void copyOnWriteAttributes() { assertThat(c1.getValue("b"), equalTo(LDValue.ofNull())); assertThat(c2.getValue("a"), equalTo(LDValue.of(2))); assertThat(c2.getValue("b"), equalTo(LDValue.of(3))); + Map c1Map = new HashMap<>(); + c1Map.put("a", LDValue.of(1)); + assertThat(c1.attributes.flatten(), equalTo(c1Map)); + Map c2Map = new HashMap<>(); + c2Map.put("a", LDValue.of(2)); + c2Map.put("b", LDValue.of(3)); + assertThat(c2.attributes.flatten(), equalTo(c2Map)); + } + + + @Test + public void copyOnWriteAttributes2() { + LDContext c1 = LDContext.builder("key").set("a", 1).set("c", LDValue.ofNull()).set("fsdf", 12).build(); + LDContext c2 = LDContext.builderFromContext(c1).set("a", 2).set("b", 3).set("fsdf", LDValue.ofNull()).build(); + + assertThat(c1.getValue("a"), equalTo(LDValue.of(1))); + assertThat(c1.getValue("b"), equalTo(LDValue.ofNull())); + assertThat(c1.getValue("fsdf"), equalTo(LDValue.of(12))); + assertThat(c2.getValue("a"), equalTo(LDValue.of(2))); + assertThat(c2.getValue("b"), equalTo(LDValue.of(3))); + assertThat(c2.getValue("fsdf"), equalTo(LDValue.ofNull())); + Map c1Map = new HashMap<>(); + c1Map.put("a", LDValue.of(1)); + c1Map.put("fsdf", LDValue.of(12)); + assertThat(c1.attributes.flatten(), equalTo(c1Map)); + Map c2Map = new HashMap<>(); + c2Map.put("a", LDValue.of(2)); + c2Map.put("b", LDValue.of(3)); + assertThat(c2.attributes.flatten(), equalTo(c2Map)); } @Test