diff --git a/CHANGELOG.md b/CHANGELOG.md index 1309ba075..a5dca8328 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ All notable changes to the LaunchDarkly Java SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [4.3.1] - 2018-09-04 +### Fixed: +- When evaluating a prerequisite feature flag, the analytics event for the evaluation did not include the result value if the prerequisite flag was off. +- The default Gson serialization for `LDUser` now includes all user properties. Previously, it omitted `privateAttributeNames`. + ## [4.3.0] - 2018-08-27 ### Added: - The new `LDClient` method `allFlagsState()` should be used instead of `allFlags()` if you are passing flag data to the front end for use with the JavaScript SDK. It preserves some flag metadata that the front end requires in order to send analytics events correctly. Versions 2.5.0 and above of the JavaScript SDK are able to use this metadata, but the output of `allFlagsState()` will still work with older versions. diff --git a/gradle.properties b/gradle.properties index 7a06d1b5c..37967c5fb 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=4.3.0 +version=4.3.1 ossrhUsername= ossrhPassword= diff --git a/src/main/java/com/launchdarkly/client/FeatureFlag.java b/src/main/java/com/launchdarkly/client/FeatureFlag.java index c13464c7a..7cc7dde91 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlag.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlag.java @@ -74,16 +74,16 @@ EvalResult evaluate(LDUser user, FeatureStore featureStore, EventFactory eventFa return new EvalResult(EvaluationDetail.error(EvaluationReason.ErrorKind.USER_NOT_SPECIFIED, null), prereqEvents); } - if (isOn()) { - EvaluationDetail details = evaluate(user, featureStore, prereqEvents, eventFactory); - return new EvalResult(details, prereqEvents); - } - - return new EvalResult(getOffValue(EvaluationReason.off()), prereqEvents); + EvaluationDetail details = evaluate(user, featureStore, prereqEvents, eventFactory); + return new EvalResult(details, prereqEvents); } private EvaluationDetail evaluate(LDUser user, FeatureStore featureStore, List events, EventFactory eventFactory) { + if (!isOn()) { + return getOffValue(EvaluationReason.off()); + } + EvaluationReason prereqFailureReason = checkPrerequisites(user, featureStore, events, eventFactory); if (prereqFailureReason != null) { return getOffValue(prereqFailureReason); @@ -123,20 +123,16 @@ private EvaluationReason checkPrerequisites(LDUser user, FeatureStore featureSto boolean prereqOk = true; Prerequisite prereq = prerequisites.get(i); FeatureFlag prereqFeatureFlag = featureStore.get(FEATURES, prereq.getKey()); - EvaluationDetail prereqEvalResult = null; if (prereqFeatureFlag == null) { logger.error("Could not retrieve prerequisite flag \"{}\" when evaluating \"{}\"", prereq.getKey(), key); prereqOk = false; - } else if (prereqFeatureFlag.isOn()) { - prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, events, eventFactory); - if (prereqEvalResult == null || prereqEvalResult.getVariationIndex() != prereq.getVariation()) { + } else { + EvaluationDetail prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, events, eventFactory); + // Note that if the prerequisite flag is off, we don't consider it a match no matter what its + // off variation was. But we still need to evaluate it in order to generate an event. + if (!prereqFeatureFlag.isOn() || prereqEvalResult == null || prereqEvalResult.getVariationIndex() != prereq.getVariation()) { prereqOk = false; } - } else { - prereqOk = false; - } - // We continue to evaluate all prerequisites even if one failed. - if (prereqFeatureFlag != null) { events.add(eventFactory.newPrerequisiteFeatureRequestEvent(prereqFeatureFlag, user, prereqEvalResult, this)); } if (!prereqOk) { diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index cc753e6e8..32409a07f 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -29,7 +29,7 @@ */ public final class LDConfig { private static final Logger logger = LoggerFactory.getLogger(LDConfig.class); - final Gson gson = new GsonBuilder().registerTypeAdapter(LDUser.class, new LDUser.UserAdapter(this)).create(); + final Gson gson = new GsonBuilder().registerTypeAdapter(LDUser.class, new LDUser.UserAdapterWithPrivateAttributeBehavior(this)).create(); private static final URI DEFAULT_BASE_URI = URI.create("https://app.launchdarkly.com"); private static final URI DEFAULT_EVENTS_URI = URI.create("https://events.launchdarkly.com"); diff --git a/src/main/java/com/launchdarkly/client/LDUser.java b/src/main/java/com/launchdarkly/client/LDUser.java index 6dc26e836..8fcada638 100644 --- a/src/main/java/com/launchdarkly/client/LDUser.java +++ b/src/main/java/com/launchdarkly/client/LDUser.java @@ -2,11 +2,11 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonPrimitive; import com.google.gson.TypeAdapter; -import com.google.gson.internal.Streams; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; @@ -18,6 +18,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; @@ -32,8 +33,15 @@ *

* Custom attributes are not parsed by LaunchDarkly. They can be used in custom rules-- for example, a custom attribute such as "customer_ranking" can be used to * launch a feature to the top 10% of users on a site. + *

+ * If you want to pass an LDUser object to the front end to be used with the JavaScript SDK, simply call Gson.toJson() or + * Gson.toJsonTree() on it. */ public class LDUser { + private static final Logger logger = LoggerFactory.getLogger(LDUser.class); + + // Note that these fields are all stored internally as JsonPrimitive rather than String so that + // we don't waste time repeatedly converting them to JsonPrimitive in the rule evaluation logic. private final JsonPrimitive key; private JsonPrimitive secondary; private JsonPrimitive ip; @@ -44,10 +52,8 @@ public class LDUser { private JsonPrimitive lastName; private JsonPrimitive anonymous; private JsonPrimitive country; - protected Map custom; - // This is set as transient as we'll use a custom serializer to marshal it - protected transient Set privateAttributeNames; - private static final Logger logger = LoggerFactory.getLogger(LDUser.class); + private Map custom; + Set privateAttributeNames; protected LDUser(Builder builder) { if (builder.key == null || builder.key.equals("")) { @@ -63,8 +69,8 @@ protected LDUser(Builder builder) { this.name = builder.name == null ? null : new JsonPrimitive(builder.name); this.avatar = builder.avatar == null ? null : new JsonPrimitive(builder.avatar); this.anonymous = builder.anonymous == null ? null : new JsonPrimitive(builder.anonymous); - this.custom = ImmutableMap.copyOf(builder.custom); - this.privateAttributeNames = ImmutableSet.copyOf(builder.privateAttrNames); + this.custom = builder.custom == null ? null : ImmutableMap.copyOf(builder.custom); + this.privateAttributeNames = builder.privateAttrNames == null ? null : ImmutableSet.copyOf(builder.privateAttrNames); } /** @@ -74,8 +80,8 @@ protected LDUser(Builder builder) { */ public LDUser(String key) { this.key = new JsonPrimitive(key); - this.custom = new HashMap<>(); - this.privateAttributeNames = new HashSet(); + this.custom = null; + this.privateAttributeNames = null; } protected JsonElement getValueForEvaluation(String attribute) { @@ -142,7 +148,7 @@ JsonElement getCustom(String key) { } return null; } - + @Override public boolean equals(Object o) { if (this == o) return true; @@ -150,41 +156,31 @@ public boolean equals(Object o) { LDUser ldUser = (LDUser) o; - if (key != null ? !key.equals(ldUser.key) : ldUser.key != null) return false; - if (secondary != null ? !secondary.equals(ldUser.secondary) : ldUser.secondary != null) return false; - if (ip != null ? !ip.equals(ldUser.ip) : ldUser.ip != null) return false; - if (email != null ? !email.equals(ldUser.email) : ldUser.email != null) return false; - if (name != null ? !name.equals(ldUser.name) : ldUser.name != null) return false; - if (avatar != null ? !avatar.equals(ldUser.avatar) : ldUser.avatar != null) return false; - if (firstName != null ? !firstName.equals(ldUser.firstName) : ldUser.firstName != null) return false; - if (lastName != null ? !lastName.equals(ldUser.lastName) : ldUser.lastName != null) return false; - if (anonymous != null ? !anonymous.equals(ldUser.anonymous) : ldUser.anonymous != null) return false; - if (country != null ? !country.equals(ldUser.country) : ldUser.country != null) return false; - if (custom != null ? !custom.equals(ldUser.custom) : ldUser.custom != null) return false; - return privateAttributeNames != null ? privateAttributeNames.equals(ldUser.privateAttributeNames) : ldUser.privateAttributeNames == null; + return Objects.equals(key, ldUser.key) && + Objects.equals(secondary, ldUser.secondary) && + Objects.equals(ip, ldUser.ip) && + Objects.equals(email, ldUser.email) && + Objects.equals(name, ldUser.name) && + Objects.equals(avatar, ldUser.avatar) && + Objects.equals(firstName, ldUser.firstName) && + Objects.equals(lastName, ldUser.lastName) && + Objects.equals(anonymous, ldUser.anonymous) && + Objects.equals(country, ldUser.country) && + Objects.equals(custom, ldUser.custom) && + Objects.equals(privateAttributeNames, ldUser.privateAttributeNames); } @Override public int hashCode() { - int result = key != null ? key.hashCode() : 0; - result = 31 * result + (secondary != null ? secondary.hashCode() : 0); - result = 31 * result + (ip != null ? ip.hashCode() : 0); - result = 31 * result + (email != null ? email.hashCode() : 0); - result = 31 * result + (name != null ? name.hashCode() : 0); - result = 31 * result + (avatar != null ? avatar.hashCode() : 0); - result = 31 * result + (firstName != null ? firstName.hashCode() : 0); - result = 31 * result + (lastName != null ? lastName.hashCode() : 0); - result = 31 * result + (anonymous != null ? anonymous.hashCode() : 0); - result = 31 * result + (country != null ? country.hashCode() : 0); - result = 31 * result + (custom != null ? custom.hashCode() : 0); - result = 31 * result + (privateAttributeNames != null ? privateAttributeNames.hashCode() : 0); - return result; + return Objects.hash(key, secondary, ip, email, name, avatar, firstName, lastName, anonymous, country, custom, privateAttributeNames); } - static class UserAdapter extends TypeAdapter { + // Used internally when including users in analytics events, to ensure that private attributes are stripped out. + static class UserAdapterWithPrivateAttributeBehavior extends TypeAdapter { + private static final Gson gson = new Gson(); private final LDConfig config; - public UserAdapter(LDConfig config) { + public UserAdapterWithPrivateAttributeBehavior(LDConfig config) { this.config = config; } @@ -284,10 +280,7 @@ private void writeCustomAttrs(JsonWriter out, LDUser user, Set privateAt beganObject = true; } out.name(entry.getKey()); - // NB: this accesses part of the internal GSON api. However, it's likely - // the only way to write a JsonElement directly: - // https://groups.google.com/forum/#!topic/google-gson/JpHbpZ9mTOk - Streams.write(entry.getValue(), out); + gson.toJson(entry.getValue(), JsonElement.class, out); } } if (beganObject) { @@ -333,8 +326,6 @@ public static class Builder { */ public Builder(String key) { this.key = key; - this.custom = new HashMap<>(); - this.privateAttrNames = new HashSet<>(); } /** @@ -358,8 +349,8 @@ public Builder(LDUser user) { this.avatar = user.getAvatar() != null ? user.getAvatar().getAsString() : null; this.anonymous = user.getAnonymous() != null ? user.getAnonymous().getAsBoolean() : null; this.country = user.getCountry() != null ? LDCountryCode.valueOf(user.getCountry().getAsString()) : null; - this.custom = new HashMap<>(user.custom); - this.privateAttrNames = new HashSet<>(user.privateAttributeNames); + this.custom = user.custom == null ? null : new HashMap<>(user.custom); + this.privateAttrNames = user.privateAttributeNames == null ? null : new HashSet<>(user.privateAttributeNames); } /** @@ -380,7 +371,7 @@ public Builder ip(String s) { * @return the builder */ public Builder privateIp(String s) { - privateAttrNames.add("ip"); + addPrivate("ip"); return ip(s); } @@ -404,7 +395,7 @@ public Builder secondary(String s) { * @return the builder */ public Builder privateSecondary(String s) { - privateAttrNames.add("secondary"); + addPrivate("secondary"); return secondary(s); } @@ -455,7 +446,7 @@ public Builder country(String s) { * @return the builder */ public Builder privateCountry(String s) { - privateAttrNames.add("country"); + addPrivate("country"); return country(s); } @@ -477,7 +468,7 @@ public Builder country(LDCountryCode country) { * @return the builder */ public Builder privateCountry(LDCountryCode country) { - privateAttrNames.add("country"); + addPrivate("country"); return country(country); } @@ -500,7 +491,7 @@ public Builder firstName(String firstName) { * @return the builder */ public Builder privateFirstName(String firstName) { - privateAttrNames.add("firstName"); + addPrivate("firstName"); return firstName(firstName); } @@ -534,7 +525,7 @@ public Builder lastName(String lastName) { * @return the builder */ public Builder privateLastName(String lastName) { - privateAttrNames.add("lastName"); + addPrivate("lastName"); return lastName(lastName); } @@ -557,7 +548,7 @@ public Builder name(String name) { * @return the builder */ public Builder privateName(String name) { - privateAttrNames.add("name"); + addPrivate("name"); return name(name); } @@ -579,7 +570,7 @@ public Builder avatar(String avatar) { * @return the builder */ public Builder privateAvatar(String avatar) { - privateAttrNames.add("avatar"); + addPrivate("avatar"); return avatar(avatar); } @@ -602,7 +593,7 @@ public Builder email(String email) { * @return the builder */ public Builder privateEmail(String email) { - privateAttrNames.add("email"); + addPrivate("email"); return email(email); } @@ -657,6 +648,9 @@ public Builder custom(String k, Boolean b) { public Builder custom(String k, JsonElement v) { checkCustomAttribute(k); if (k != null && v != null) { + if (custom == null) { + custom = new HashMap<>(); + } custom.put(k, v); } return this; @@ -672,15 +666,13 @@ public Builder custom(String k, JsonElement v) { * @return the builder */ public Builder customString(String k, List vs) { - checkCustomAttribute(k); JsonArray array = new JsonArray(); for (String v : vs) { if (v != null) { array.add(new JsonPrimitive(v)); } } - custom.put(k, array); - return this; + return custom(k, array); } /** @@ -693,15 +685,13 @@ public Builder customString(String k, List vs) { * @return the builder */ public Builder customNumber(String k, List vs) { - checkCustomAttribute(k); JsonArray array = new JsonArray(); for (Number v : vs) { if (v != null) { array.add(new JsonPrimitive(v)); } } - custom.put(k, array); - return this; + return custom(k, array); } /** @@ -714,15 +704,13 @@ public Builder customNumber(String k, List vs) { * @return the builder */ public Builder customValues(String k, List vs) { - checkCustomAttribute(k); JsonArray array = new JsonArray(); for (JsonElement v : vs) { if (v != null) { array.add(v); } } - custom.put(k, array); - return this; + return custom(k, array); } /** @@ -736,7 +724,7 @@ public Builder customValues(String k, List vs) { * @return the builder */ public Builder privateCustom(String k, String v) { - privateAttrNames.add(k); + addPrivate(k); return custom(k, v); } @@ -751,7 +739,7 @@ public Builder privateCustom(String k, String v) { * @return the builder */ public Builder privateCustom(String k, Number n) { - privateAttrNames.add(k); + addPrivate(k); return custom(k, n); } @@ -766,7 +754,7 @@ public Builder privateCustom(String k, Number n) { * @return the builder */ public Builder privateCustom(String k, Boolean b) { - privateAttrNames.add(k); + addPrivate(k); return custom(k, b); } @@ -781,7 +769,7 @@ public Builder privateCustom(String k, Boolean b) { * @return the builder */ public Builder privateCustom(String k, JsonElement v) { - privateAttrNames.add(k); + addPrivate(k); return custom(k, v); } @@ -796,7 +784,7 @@ public Builder privateCustom(String k, JsonElement v) { * @return the builder */ public Builder privateCustomString(String k, List vs) { - privateAttrNames.add(k); + addPrivate(k); return customString(k, vs); } @@ -811,7 +799,7 @@ public Builder privateCustomString(String k, List vs) { * @return the builder */ public Builder privateCustomNumber(String k, List vs) { - privateAttrNames.add(k); + addPrivate(k); return customNumber(k, vs); } @@ -826,7 +814,7 @@ public Builder privateCustomNumber(String k, List vs) { * @return the builder */ public Builder privateCustomValues(String k, List vs) { - privateAttrNames.add(k); + addPrivate(k); return customValues(k, vs); } @@ -839,6 +827,13 @@ private void checkCustomAttribute(String key) { } } + private void addPrivate(String key) { + if (privateAttrNames == null) { + privateAttrNames = new HashSet<>(); + } + privateAttrNames.add(key); + } + /** * Builds the configured {@link com.launchdarkly.client.LDUser} object. * diff --git a/src/test/java/com/launchdarkly/client/FeatureFlagTest.java b/src/test/java/com/launchdarkly/client/FeatureFlagTest.java index 6ba5317b5..1d3c800c5 100644 --- a/src/test/java/com/launchdarkly/client/FeatureFlagTest.java +++ b/src/test/java/com/launchdarkly/client/FeatureFlagTest.java @@ -171,7 +171,39 @@ public void flagReturnsOffVariationIfPrerequisiteIsNotFound() throws Exception { assertEquals(new EvaluationDetail<>(expectedReason, 1, js("off")), result.getDetails()); assertEquals(0, result.getPrerequisiteEvents().size()); } - + + @Test + public void flagReturnsOffVariationAndEventIfPrerequisiteIsOff() throws Exception { + FeatureFlag f0 = new FeatureFlagBuilder("feature0") + .on(true) + .prerequisites(Arrays.asList(new Prerequisite("feature1", 1))) + .fallthrough(fallthroughVariation(0)) + .offVariation(1) + .variations(js("fall"), js("off"), js("on")) + .version(1) + .build(); + FeatureFlag f1 = new FeatureFlagBuilder("feature1") + .on(false) + .offVariation(1) + // note that even though it returns the desired variation, it is still off and therefore not a match + .fallthrough(fallthroughVariation(0)) + .variations(js("nogo"), js("go")) + .version(2) + .build(); + featureStore.upsert(FEATURES, f1); + FeatureFlag.EvalResult result = f0.evaluate(BASE_USER, featureStore, EventFactory.DEFAULT); + + EvaluationReason expectedReason = EvaluationReason.prerequisiteFailed("feature1"); + assertEquals(new EvaluationDetail<>(expectedReason, 1, js("off")), result.getDetails()); + + assertEquals(1, result.getPrerequisiteEvents().size()); + Event.FeatureRequest event = result.getPrerequisiteEvents().get(0); + assertEquals(f1.getKey(), event.key); + assertEquals(js("go"), event.value); + assertEquals(f1.getVersion(), event.version.intValue()); + assertEquals(f0.getKey(), event.prereqOf); + } + @Test public void flagReturnsOffVariationAndEventIfPrerequisiteIsNotMet() throws Exception { FeatureFlag f0 = new FeatureFlagBuilder("feature0") diff --git a/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java b/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java index 201978b9f..e07777647 100644 --- a/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java @@ -54,6 +54,13 @@ public void boolVariationReturnsDefaultValueForUnknownFlag() throws Exception { assertFalse(client.boolVariation("key", user, false)); } + @Test + public void boolVariationReturnsDefaultValueForWrongType() throws Exception { + featureStore.upsert(FEATURES, flagWithValue("key", js("wrong"))); + + assertFalse(client.boolVariation("key", user, false)); + } + @Test public void intVariationReturnsFlagValue() throws Exception { featureStore.upsert(FEATURES, flagWithValue("key", jint(2))); @@ -61,11 +68,25 @@ public void intVariationReturnsFlagValue() throws Exception { assertEquals(new Integer(2), client.intVariation("key", user, 1)); } + @Test + public void intVariationReturnsFlagValueEvenIfEncodedAsDouble() throws Exception { + featureStore.upsert(FEATURES, flagWithValue("key", jdouble(2.0))); + + assertEquals(new Integer(2), client.intVariation("key", user, 1)); + } + @Test public void intVariationReturnsDefaultValueForUnknownFlag() throws Exception { assertEquals(new Integer(1), client.intVariation("key", user, 1)); } + @Test + public void intVariationReturnsDefaultValueForWrongType() throws Exception { + featureStore.upsert(FEATURES, flagWithValue("key", js("wrong"))); + + assertEquals(new Integer(1), client.intVariation("key", user, 1)); + } + @Test public void doubleVariationReturnsFlagValue() throws Exception { featureStore.upsert(FEATURES, flagWithValue("key", jdouble(2.5d))); @@ -73,11 +94,25 @@ public void doubleVariationReturnsFlagValue() throws Exception { assertEquals(new Double(2.5d), client.doubleVariation("key", user, 1.0d)); } + @Test + public void doubleVariationReturnsFlagValueEvenIfEncodedAsInt() throws Exception { + featureStore.upsert(FEATURES, flagWithValue("key", jint(2))); + + assertEquals(new Double(2.0d), client.doubleVariation("key", user, 1.0d)); + } + @Test public void doubleVariationReturnsDefaultValueForUnknownFlag() throws Exception { assertEquals(new Double(1.0d), client.doubleVariation("key", user, 1.0d)); } + @Test + public void doubleVariationReturnsDefaultValueForWrongType() throws Exception { + featureStore.upsert(FEATURES, flagWithValue("key", js("wrong"))); + + assertEquals(new Double(1.0d), client.doubleVariation("key", user, 1.0d)); + } + @Test public void stringVariationReturnsFlagValue() throws Exception { featureStore.upsert(FEATURES, flagWithValue("key", js("b"))); @@ -90,6 +125,13 @@ public void stringVariationReturnsDefaultValueForUnknownFlag() throws Exception assertEquals("a", client.stringVariation("key", user, "a")); } + @Test + public void stringVariationReturnsDefaultValueForWrongType() throws Exception { + featureStore.upsert(FEATURES, flagWithValue("key", jbool(true))); + + assertEquals("a", client.stringVariation("key", user, "a")); + } + @Test public void jsonVariationReturnsFlagValue() throws Exception { JsonObject data = new JsonObject(); diff --git a/src/test/java/com/launchdarkly/client/LDUserTest.java b/src/test/java/com/launchdarkly/client/LDUserTest.java index ca8a9f7f4..ccbfe8c95 100644 --- a/src/test/java/com/launchdarkly/client/LDUserTest.java +++ b/src/test/java/com/launchdarkly/client/LDUserTest.java @@ -1,6 +1,8 @@ package com.launchdarkly.client; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonElement; @@ -11,20 +13,22 @@ import org.junit.Test; import java.lang.reflect.Type; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; import static com.launchdarkly.client.TestUtil.jbool; import static com.launchdarkly.client.TestUtil.jdouble; import static com.launchdarkly.client.TestUtil.jint; import static com.launchdarkly.client.TestUtil.js; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; public class LDUserTest { - - private JsonPrimitive us = new JsonPrimitive(LDCountryCode.US.getAlpha2()); - + private static final Gson defaultGson = new Gson(); + @Test public void testLDUserConstructor() { LDUser user = new LDUser.Builder("key") @@ -44,169 +48,324 @@ public void testLDUserConstructor() { } @Test - public void testValidCountryCodeSetsCountry() { - LDUser user = new LDUser.Builder("key").country(LDCountryCode.US).build(); + public void canSetKey() { + LDUser user = new LDUser.Builder("k").build(); + assertEquals("k", user.getKeyAsString()); + } + + @Test + public void canSetSecondary() { + LDUser user = new LDUser.Builder("key").secondary("s").build(); + assertEquals("s", user.getSecondary().getAsString()); + } - assert(user.getCountry().equals(us)); + @Test + public void canSetPrivateSecondary() { + LDUser user = new LDUser.Builder("key").privateSecondary("s").build(); + assertEquals("s", user.getSecondary().getAsString()); + assertEquals(ImmutableSet.of("secondary"), user.privateAttributeNames); + } + + @Test + public void canSetIp() { + LDUser user = new LDUser.Builder("key").ip("i").build(); + assertEquals("i", user.getIp().getAsString()); + } + + @Test + public void canSetPrivateIp() { + LDUser user = new LDUser.Builder("key").privateIp("i").build(); + assertEquals("i", user.getIp().getAsString()); + assertEquals(ImmutableSet.of("ip"), user.privateAttributeNames); } + @Test + public void canSetEmail() { + LDUser user = new LDUser.Builder("key").email("e").build(); + assertEquals("e", user.getEmail().getAsString()); + } + + @Test + public void canSetPrivateEmail() { + LDUser user = new LDUser.Builder("key").privateEmail("e").build(); + assertEquals("e", user.getEmail().getAsString()); + assertEquals(ImmutableSet.of("email"), user.privateAttributeNames); + } @Test - public void testValidCountryCodeStringSetsCountry() { - LDUser user = new LDUser.Builder("key").country("US").build(); + public void canSetName() { + LDUser user = new LDUser.Builder("key").name("n").build(); + assertEquals("n", user.getName().getAsString()); + } + + @Test + public void canSetPrivateName() { + LDUser user = new LDUser.Builder("key").privateName("n").build(); + assertEquals("n", user.getName().getAsString()); + assertEquals(ImmutableSet.of("name"), user.privateAttributeNames); + } - assert(user.getCountry().equals(us)); + @Test + public void canSetAvatar() { + LDUser user = new LDUser.Builder("key").avatar("a").build(); + assertEquals("a", user.getAvatar().getAsString()); + } + + @Test + public void canSetPrivateAvatar() { + LDUser user = new LDUser.Builder("key").privateAvatar("a").build(); + assertEquals("a", user.getAvatar().getAsString()); + assertEquals(ImmutableSet.of("avatar"), user.privateAttributeNames); } @Test - public void testValidCountryCode3SetsCountry() { - LDUser user = new LDUser.Builder("key").country("USA").build(); + public void canSetFirstName() { + LDUser user = new LDUser.Builder("key").firstName("f").build(); + assertEquals("f", user.getFirstName().getAsString()); + } + + @Test + public void canSetPrivateFirstName() { + LDUser user = new LDUser.Builder("key").privateFirstName("f").build(); + assertEquals("f", user.getFirstName().getAsString()); + assertEquals(ImmutableSet.of("firstName"), user.privateAttributeNames); + } + + @Test + public void canSetLastName() { + LDUser user = new LDUser.Builder("key").lastName("l").build(); + assertEquals("l", user.getLastName().getAsString()); + } + + @Test + public void canSetPrivateLastName() { + LDUser user = new LDUser.Builder("key").privateLastName("l").build(); + assertEquals("l", user.getLastName().getAsString()); + assertEquals(ImmutableSet.of("lastName"), user.privateAttributeNames); + } + + @Test + public void canSetAnonymous() { + LDUser user = new LDUser.Builder("key").anonymous(true).build(); + assertEquals(true, user.getAnonymous().getAsBoolean()); + } + + @Test + public void canSetCountry() { + LDUser user = new LDUser.Builder("key").country(LDCountryCode.US).build(); + assertEquals("US", user.getCountry().getAsString()); + } + + @Test + public void canSetCountryAsString() { + LDUser user = new LDUser.Builder("key").country("US").build(); + assertEquals("US", user.getCountry().getAsString()); + } - assert(user.getCountry().equals(us)); + @Test + public void canSetCountryAs3CharacterString() { + LDUser user = new LDUser.Builder("key").country("USA").build(); + assertEquals("US", user.getCountry().getAsString()); } @Test - public void testAmbiguousCountryNameSetsCountryWithExactMatch() { + public void ambiguousCountryNameSetsCountryWithExactMatch() { // "United States" is ambiguous: can also match "United States Minor Outlying Islands" LDUser user = new LDUser.Builder("key").country("United States").build(); - assert(user.getCountry().equals(us)); + assertEquals("US", user.getCountry().getAsString()); } @Test - public void testAmbiguousCountryNameSetsCountryWithPartialMatch() { + public void ambiguousCountryNameSetsCountryWithPartialMatch() { // For an ambiguous match, we return the first match LDUser user = new LDUser.Builder("key").country("United St").build(); - assert(user.getCountry() != null); + assertNotNull(user.getCountry()); } - @Test - public void testPartialUniqueMatchSetsCountry() { + public void partialUniqueMatchSetsCountry() { LDUser user = new LDUser.Builder("key").country("United States Minor").build(); - assert(user.getCountry().equals(new JsonPrimitive(LDCountryCode.UM.getAlpha2()))); + assertEquals("UM", user.getCountry().getAsString()); } @Test - public void testInvalidCountryNameDoesNotSetCountry() { + public void invalidCountryNameDoesNotSetCountry() { LDUser user = new LDUser.Builder("key").country("East Jibip").build(); - assert(user.getCountry() == null); + assertNull(user.getCountry()); } @Test - public void testLDUserJsonSerializationContainsCountryAsTwoDigitCode() { - LDConfig config = LDConfig.DEFAULT; - Gson gson = config.gson; - LDUser user = new LDUser.Builder("key").country(LDCountryCode.US).build(); - - String jsonStr = gson.toJson(user); - Type type = new TypeToken>(){}.getType(); - Map json = gson.fromJson(jsonStr, type); - - assert(json.get("country").equals(us)); + public void canSetPrivateCountry() { + LDUser user = new LDUser.Builder("key").privateCountry(LDCountryCode.US).build(); + assertEquals("US", user.getCountry().getAsString()); + assertEquals(ImmutableSet.of("country"), user.privateAttributeNames); } @Test - public void testLDUserCustomMarshalWithPrivateAttrsProducesEquivalentLDUserIfNoAttrsArePrivate() { - LDConfig config = LDConfig.DEFAULT; - LDUser user = new LDUser.Builder("key") - .anonymous(true) - .avatar("avatar") - .country(LDCountryCode.AC) - .ip("127.0.0.1") - .firstName("bob") - .lastName("loblaw") - .email("bob@example.com") - .custom("foo", 42) - .build(); - - String jsonStr = new Gson().toJson(user); - Type type = new TypeToken>(){}.getType(); - Map json = config.gson.fromJson(jsonStr, type); - Map privateJson = config.gson.fromJson(config.gson.toJson(user), type); - - assertEquals(json, privateJson); + public void canSetCustomString() { + LDUser user = new LDUser.Builder("key").custom("thing", "value").build(); + assertEquals("value", user.getCustom("thing").getAsString()); } - + @Test - public void testLDUserCustomMarshalWithAllPrivateAttributesReturnsKey() { - LDConfig config = new LDConfig.Builder().allAttributesPrivate(true).build(); - LDUser user = new LDUser.Builder("key") - .email("foo@bar.com") - .custom("bar", 43) - .build(); + public void canSetPrivateCustomString() { + LDUser user = new LDUser.Builder("key").privateCustom("thing", "value").build(); + assertEquals("value", user.getCustom("thing").getAsString()); + assertEquals(ImmutableSet.of("thing"), user.privateAttributeNames); + } - Type type = new TypeToken>(){}.getType(); - Map privateJson = config.gson.fromJson(config.gson.toJson(user), type); + @Test + public void canSetCustomInt() { + LDUser user = new LDUser.Builder("key").custom("thing", 1).build(); + assertEquals(1, user.getCustom("thing").getAsInt()); + } + + @Test + public void canSetPrivateCustomInt() { + LDUser user = new LDUser.Builder("key").privateCustom("thing", 1).build(); + assertEquals(1, user.getCustom("thing").getAsInt()); + assertEquals(ImmutableSet.of("thing"), user.privateAttributeNames); + } + + @Test + public void canSetCustomBoolean() { + LDUser user = new LDUser.Builder("key").custom("thing", true).build(); + assertEquals(true, user.getCustom("thing").getAsBoolean()); + } + + @Test + public void canSetPrivateCustomBoolean() { + LDUser user = new LDUser.Builder("key").privateCustom("thing", true).build(); + assertEquals(true, user.getCustom("thing").getAsBoolean()); + assertEquals(ImmutableSet.of("thing"), user.privateAttributeNames); + } + + @Test + public void canSetCustomJsonValue() { + JsonObject value = new JsonObject(); + LDUser user = new LDUser.Builder("key").custom("thing", value).build(); + assertEquals(value, user.getCustom("thing")); + } - assertNull(privateJson.get("custom")); - assertEquals(privateJson.get("key").getAsString(), "key"); + @Test + public void canSetPrivateCustomJsonValue() { + JsonObject value = new JsonObject(); + LDUser user = new LDUser.Builder("key").privateCustom("thing", value).build(); + assertEquals(value, user.getCustom("thing")); + assertEquals(ImmutableSet.of("thing"), user.privateAttributeNames); + } - // email and custom are private - assert(privateJson.get("privateAttrs").getAsJsonArray().size() == 2); - assertNull(privateJson.get("email")); + @Test + public void testAllPropertiesInDefaultEncoding() { + for (Map.Entry e: getUserPropertiesJsonMap().entrySet()) { + JsonElement expected = defaultGson.fromJson(e.getValue(), JsonElement.class); + JsonElement actual = defaultGson.toJsonTree(e.getKey()); + assertEquals(expected, actual); + } + } + + @Test + public void testAllPropertiesInPrivateAttributeEncoding() { + for (Map.Entry e: getUserPropertiesJsonMap().entrySet()) { + JsonElement expected = defaultGson.fromJson(e.getValue(), JsonElement.class); + JsonElement actual = LDConfig.DEFAULT.gson.toJsonTree(e.getKey()); + assertEquals(expected, actual); + } } + private Map getUserPropertiesJsonMap() { + ImmutableMap.Builder builder = ImmutableMap.builder(); + builder.put(new LDUser.Builder("userkey").build(), "{\"key\":\"userkey\"}"); + builder.put(new LDUser.Builder("userkey").secondary("value").build(), + "{\"key\":\"userkey\",\"secondary\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").ip("value").build(), + "{\"key\":\"userkey\",\"ip\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").email("value").build(), + "{\"key\":\"userkey\",\"email\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").name("value").build(), + "{\"key\":\"userkey\",\"name\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").avatar("value").build(), + "{\"key\":\"userkey\",\"avatar\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").firstName("value").build(), + "{\"key\":\"userkey\",\"firstName\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").lastName("value").build(), + "{\"key\":\"userkey\",\"lastName\":\"value\"}"); + builder.put(new LDUser.Builder("userkey").anonymous(true).build(), + "{\"key\":\"userkey\",\"anonymous\":true}"); + builder.put(new LDUser.Builder("userkey").country(LDCountryCode.US).build(), + "{\"key\":\"userkey\",\"country\":\"US\"}"); + builder.put(new LDUser.Builder("userkey").custom("thing", "value").build(), + "{\"key\":\"userkey\",\"custom\":{\"thing\":\"value\"}}"); + return builder.build(); + } + + @Test + public void defaultJsonEncodingHasPrivateAttributeNames() { + LDUser user = new LDUser.Builder("userkey").privateName("x").privateEmail("y").build(); + String expected = "{\"key\":\"userkey\",\"name\":\"x\",\"email\":\"y\",\"privateAttributeNames\":[\"name\",\"email\"]}"; + assertEquals(defaultGson.fromJson(expected, JsonElement.class), defaultGson.toJsonTree(user)); + } + @Test - public void testLDUserAnonymousAttributeIsNeverPrivate() { + public void privateAttributeEncodingRedactsAllPrivateAttributes() { LDConfig config = new LDConfig.Builder().allAttributesPrivate(true).build(); - LDUser user = new LDUser.Builder("key") + LDUser user = new LDUser.Builder("userkey") + .secondary("s") + .ip("i") + .email("e") + .name("n") + .avatar("a") + .firstName("f") + .lastName("l") .anonymous(true) + .country(LDCountryCode.US) + .custom("thing", "value") .build(); + Set redacted = ImmutableSet.of("secondary", "ip", "email", "name", "avatar", "firstName", "lastName", "country", "thing"); - Type type = new TypeToken>(){}.getType(); - Map privateJson = config.gson.fromJson(config.gson.toJson(user), type); - - assertEquals(privateJson.get("anonymous").getAsBoolean(), true); - assertNull(privateJson.get("privateAttrs")); + JsonObject o = config.gson.toJsonTree(user).getAsJsonObject(); + assertEquals("userkey", o.get("key").getAsString()); + assertEquals(true, o.get("anonymous").getAsBoolean()); + for (String attr: redacted) { + assertNull(o.get(attr)); + } + assertNull(o.get("custom")); + assertEquals(redacted, getPrivateAttrs(o)); } - + @Test - public void testLDUserCustomMarshalWithPrivateAttrsRedactsCorrectAttrs() { - LDConfig config = LDConfig.DEFAULT; - LDUser user = new LDUser.Builder("key") - .privateCustom("foo", 42) + public void privateAttributeEncodingRedactsSpecificPerUserPrivateAttributes() { + LDUser user = new LDUser.Builder("userkey") + .email("e") + .privateName("n") .custom("bar", 43) - .build(); - - Type type = new TypeToken>(){}.getType(); - Map privateJson = config.gson.fromJson(config.gson.toJson(user), type); - - assertNull(privateJson.get("custom").getAsJsonObject().get("foo")); - assertEquals(privateJson.get("key").getAsString(), "key"); - assertEquals(privateJson.get("custom").getAsJsonObject().get("bar"), new JsonPrimitive(43)); - } - - @Test - public void testLDUserCustomMarshalWithPrivateGlobalAttributesRedactsCorrectAttrs() { - LDConfig config = new LDConfig.Builder().privateAttributeNames("foo", "bar").build(); - - LDUser user = new LDUser.Builder("key") .privateCustom("foo", 42) - .custom("bar", 43) - .custom("baz", 44) - .privateCustom("bum", 45) .build(); - - Type type = new TypeToken>(){}.getType(); - Map privateJson = config.gson.fromJson(config.gson.toJson(user), type); - - assertNull(privateJson.get("custom").getAsJsonObject().get("foo")); - assertNull(privateJson.get("custom").getAsJsonObject().get("bar")); - assertNull(privateJson.get("custom").getAsJsonObject().get("bum")); - assertEquals(privateJson.get("custom").getAsJsonObject().get("baz"), new JsonPrimitive(44)); + + JsonObject o = LDConfig.DEFAULT.gson.toJsonTree(user).getAsJsonObject(); + assertEquals("e", o.get("email").getAsString()); + assertNull(o.get("name")); + assertEquals(43, o.get("custom").getAsJsonObject().get("bar").getAsInt()); + assertNull(o.get("custom").getAsJsonObject().get("foo")); + assertEquals(ImmutableSet.of("name", "foo"), getPrivateAttrs(o)); } @Test - public void testLDUserCustomMarshalWithBuiltInAttributesRedactsCorrectAttrs() { - LDConfig config = LDConfig.DEFAULT; - LDUser user = new LDUser.Builder("key") - .privateEmail("foo@bar.com") + public void privateAttributeEncodingRedactsSpecificGlobalPrivateAttributes() { + LDConfig config = new LDConfig.Builder().privateAttributeNames("name", "foo").build(); + LDUser user = new LDUser.Builder("userkey") + .email("e") + .name("n") .custom("bar", 43) + .custom("foo", 42) .build(); - - Type type = new TypeToken>(){}.getType(); - Map privateJson = config.gson.fromJson(config.gson.toJson(user), type); - assertNull(privateJson.get("email")); + + JsonObject o = config.gson.toJsonTree(user).getAsJsonObject(); + assertEquals("e", o.get("email").getAsString()); + assertNull(o.get("name")); + assertEquals(43, o.get("custom").getAsJsonObject().get("bar").getAsInt()); + assertNull(o.get("custom").getAsJsonObject().get("foo")); + assertEquals(ImmutableSet.of("name", "foo"), getPrivateAttrs(o)); } @Test @@ -242,25 +401,6 @@ public void getValueReturnsNullIfNotFound() { assertNull(user.getValueForEvaluation("height")); } - @Test - public void canAddCustomAttrWithJsonValue() { - JsonElement value = new JsonPrimitive("x"); - LDUser user = new LDUser.Builder("key") - .custom("foo", value) - .build(); - assertEquals(value, user.getCustom("foo")); - } - - @Test - public void canAddPrivateCustomAttrWithJsonValue() { - JsonElement value = new JsonPrimitive("x"); - LDUser user = new LDUser.Builder("key") - .privateCustom("foo", value) - .build(); - assertEquals(value, user.getCustom("foo")); - assertTrue(user.privateAttributeNames.contains("foo")); - } - @Test public void canAddCustomAttrWithListOfStrings() { LDUser user = new LDUser.Builder("key") @@ -300,4 +440,9 @@ private JsonElement makeCustomAttrWithListOfValues(String name, JsonElement... v ret.add(name, a); return ret; } + + private Set getPrivateAttrs(JsonObject o) { + Type type = new TypeToken>(){}.getType(); + return new HashSet(defaultGson.>fromJson(o.get("privateAttrs"), type)); + } }