From 154a07e88b993652bdeeaeb63361eb1677a081c3 Mon Sep 17 00:00:00 2001 From: John Kodumal Date: Mon, 11 Jan 2016 14:55:11 -0800 Subject: [PATCH 1/5] Provide a no-args ctor for TargetRules. May fix an exception thrown during GSON serialization --- src/main/java/com/launchdarkly/client/Variation.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/launchdarkly/client/Variation.java b/src/main/java/com/launchdarkly/client/Variation.java index 21d0dd939..367460c01 100644 --- a/src/main/java/com/launchdarkly/client/Variation.java +++ b/src/main/java/com/launchdarkly/client/Variation.java @@ -119,6 +119,10 @@ static class TargetRule { private final Logger logger = LoggerFactory.getLogger(TargetRule.class); + public TargetRule() { + + } + TargetRule(String attribute, String operator, List values) { this.attribute = attribute; this.operator = operator; From ee35eee9a58d5f4e5f36b89dbceeff0910a06f3f Mon Sep 17 00:00:00 2001 From: John Kodumal Date: Mon, 11 Jan 2016 14:55:56 -0800 Subject: [PATCH 2/5] Bump version to 0.17.0-SNAPSHOT --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 6f99729df..e449c0c24 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ repositories { allprojects { group = 'com.launchdarkly' - version = "0.16.0-SNAPSHOT" + version = "0.17.0-SNAPSHOT" sourceCompatibility = 1.6 targetCompatibility = 1.6 } From 6529231b8be4e5e2137d4ab107719cade20d3a37 Mon Sep 17 00:00:00 2001 From: John Kodumal Date: Tue, 12 Jan 2016 09:57:08 -0800 Subject: [PATCH 3/5] Fix custom matching for arrays of customs --- .../com/launchdarkly/client/Variation.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/Variation.java b/src/main/java/com/launchdarkly/client/Variation.java index 367460c01..62a905fa1 100644 --- a/src/main/java/com/launchdarkly/client/Variation.java +++ b/src/main/java/com/launchdarkly/client/Variation.java @@ -133,6 +133,16 @@ public TargetRule() { this(attribute, "in", values); } + private boolean matchCustom(JsonPrimitive prim, List values) { + if (prim.isNumber()) { + return values.contains(prim.getAsDouble()); + } else if (prim.isBoolean()) { + return values.contains(prim.getAsBoolean()); + } else { + return values.contains(prim.getAsString()); + } + } + public boolean matchTarget(LDUser user) { Object uValue = null; if (attribute.equals("key")) { @@ -191,21 +201,14 @@ else if (attribute.equals("anonymous")) { logger.error("Invalid custom attribute value in user object: " + elt); return false; } - else if (values.contains(elt.getAsString())) { + else if (matchCustom(elt.getAsJsonPrimitive(), values)) { return true; } } return false; } else if (custom.isJsonPrimitive()) { - JsonPrimitive prim = custom.getAsJsonPrimitive(); - if (prim.isNumber()) { - return values.contains(custom.getAsDouble()); - } else if (prim.isBoolean()) { - return values.contains(custom.getAsBoolean()); - } else { - return values.contains(custom.getAsString()); - } + return matchCustom(custom.getAsJsonPrimitive(), values); } } return false; From 2f7adbc7567dd565f02edbd63704b39aec41abae Mon Sep 17 00:00:00 2001 From: John Kodumal Date: Tue, 12 Jan 2016 11:45:20 -0800 Subject: [PATCH 4/5] Better support for non-string custom attribute values --- .../com/launchdarkly/client/Variation.java | 42 +++++-------- .../launchdarkly/client/FeatureRepTest.java | 63 +++++++++++-------- 2 files changed, 54 insertions(+), 51 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/Variation.java b/src/main/java/com/launchdarkly/client/Variation.java index 62a905fa1..b896e107c 100644 --- a/src/main/java/com/launchdarkly/client/Variation.java +++ b/src/main/java/com/launchdarkly/client/Variation.java @@ -92,7 +92,7 @@ static class Builder { Builder(E value, int weight) { this.value = value; this.weight = weight; - this.userTarget = new TargetRule("key", "in", new ArrayList()); + this.userTarget = new TargetRule("key", "in", new ArrayList()); targets = new ArrayList(); } @@ -115,7 +115,7 @@ Variation build() { static class TargetRule { String attribute; String operator; - List values; + List values; private final Logger logger = LoggerFactory.getLogger(TargetRule.class); @@ -123,71 +123,61 @@ public TargetRule() { } - TargetRule(String attribute, String operator, List values) { + TargetRule(String attribute, String operator, List values) { this.attribute = attribute; this.operator = operator; - this.values = new ArrayList(values); + this.values = new ArrayList(values); } - TargetRule(String attribute, List values) { + TargetRule(String attribute, List values) { this(attribute, "in", values); } - private boolean matchCustom(JsonPrimitive prim, List values) { - if (prim.isNumber()) { - return values.contains(prim.getAsDouble()); - } else if (prim.isBoolean()) { - return values.contains(prim.getAsBoolean()); - } else { - return values.contains(prim.getAsString()); - } - } - public boolean matchTarget(LDUser user) { Object uValue = null; if (attribute.equals("key")) { if (user.getKey() != null) { - uValue = user.getKey(); + uValue = new JsonPrimitive(user.getKey()); } } else if (attribute.equals("ip") && user.getIp() != null) { if (user.getIp() != null) { - uValue = user.getIp(); + uValue = new JsonPrimitive(user.getIp()); } } else if (attribute.equals("country")) { if (user.getCountry() != null) { - uValue = user.getCountry().getAlpha2(); + uValue = new JsonPrimitive(user.getCountry().getAlpha2()); } } else if (attribute.equals("email")) { if (user.getEmail() != null) { - uValue = user.getEmail(); + uValue = new JsonPrimitive(user.getEmail()); } } else if (attribute.equals("firstName")) { if (user.getFirstName() != null ) { - uValue = user.getFirstName(); + uValue = new JsonPrimitive(user.getFirstName()); } } else if (attribute.equals("lastName")) { if (user.getLastName() != null) { - uValue = user.getLastName(); + uValue = new JsonPrimitive(user.getLastName()); } } else if (attribute.equals("avatar")) { if (user.getAvatar() != null) { - uValue = user.getAvatar(); + uValue = new JsonPrimitive(user.getAvatar()); } } else if (attribute.equals("name")) { if (user.getName() != null) { - uValue = user.getName(); + uValue = new JsonPrimitive(user.getName()); } } else if (attribute.equals("anonymous")) { if (user.getAnonymous() != null) { - uValue = user.getAnonymous(); + uValue = new JsonPrimitive(user.getAnonymous()); } } else { // Custom attribute @@ -201,14 +191,14 @@ else if (attribute.equals("anonymous")) { logger.error("Invalid custom attribute value in user object: " + elt); return false; } - else if (matchCustom(elt.getAsJsonPrimitive(), values)) { + else if (values.contains(elt.getAsJsonPrimitive())) { return true; } } return false; } else if (custom.isJsonPrimitive()) { - return matchCustom(custom.getAsJsonPrimitive(), values); + return values.contains(custom.getAsJsonPrimitive()); } } return false; diff --git a/src/test/java/com/launchdarkly/client/FeatureRepTest.java b/src/test/java/com/launchdarkly/client/FeatureRepTest.java index 21b0baa4f..92ff1f62d 100644 --- a/src/test/java/com/launchdarkly/client/FeatureRepTest.java +++ b/src/test/java/com/launchdarkly/client/FeatureRepTest.java @@ -1,5 +1,7 @@ package com.launchdarkly.client; +import com.google.gson.JsonPrimitive; +import org.glassfish.jersey.server.JSONP; import org.junit.Test; import java.util.Arrays; @@ -8,24 +10,24 @@ public class FeatureRepTest { - private final Variation.TargetRule targetUserOn = new Variation.TargetRule("key", Collections.singletonList("targetOn@test.com")); + private final Variation.TargetRule targetUserOn = new Variation.TargetRule("key", Collections.singletonList(new JsonPrimitive("targetOn@test.com"))); - private final Variation.TargetRule targetGroupOn = new Variation.TargetRule("groups", Arrays.asList("google", "microsoft")); + private final Variation.TargetRule targetGroupOn = new Variation.TargetRule("groups", Arrays.asList(new JsonPrimitive("google"), new JsonPrimitive("microsoft"))); // GSON will deserialize numbers as decimals - private final Variation.TargetRule targetFavoriteNumberOn = new Variation.TargetRule("favorite_number", Arrays.asList(42.0)); + private final Variation.TargetRule targetFavoriteNumberOn = new Variation.TargetRule("favorite_number", Arrays.asList(new JsonPrimitive(42))); - private final Variation.TargetRule targetLikesCatsOn = new Variation.TargetRule("likes_cats", Arrays.asList(true)); + private final Variation.TargetRule targetLikesCatsOn = new Variation.TargetRule("likes_cats", Arrays.asList(new JsonPrimitive(true))); - private final Variation.TargetRule targetUserOff = new Variation.TargetRule("key", Collections.singletonList("targetOff@test.com")); + private final Variation.TargetRule targetUserOff = new Variation.TargetRule("key", Collections.singletonList(new JsonPrimitive("targetOff@test.com"))); - private final Variation.TargetRule targetGroupOff = new Variation.TargetRule("groups", Arrays.asList("oracle")); + private final Variation.TargetRule targetGroupOff = new Variation.TargetRule("groups", Arrays.asList(new JsonPrimitive("oracle"))); - private final Variation.TargetRule targetFavoriteNumberOff = new Variation.TargetRule("favorite_number", Arrays.asList(33.0)); + private final Variation.TargetRule targetFavoriteNumberOff = new Variation.TargetRule("favorite_number", Arrays.asList(new JsonPrimitive(33.0))); - private final Variation.TargetRule targetLikesDogsOff = new Variation.TargetRule("likes_dogs", Arrays.asList(false)); + private final Variation.TargetRule targetLikesDogsOff = new Variation.TargetRule("likes_dogs", Arrays.asList(new JsonPrimitive(false))); - private final Variation.TargetRule targetAnonymousOn = new Variation.TargetRule("anonymous", Collections.singletonList(true)); + private final Variation.TargetRule targetAnonymousOn = new Variation.TargetRule("anonymous", Collections.singletonList(new JsonPrimitive(true))); private final Variation trueVariation = new Variation.Builder(true, 80) .target(targetUserOn) @@ -119,16 +121,27 @@ public void testFlagForTargetNumericTestOn() { assertEquals(true, b); } - @Test - public void testFlagForTargetBooleanTestOn() { - LDUser user = new LDUser.Builder("targetOther@test.com") - .custom("likes_cats", true) - .build(); + @Test + public void testFlagForTargetNumericListTestOn() { + LDUser user = new LDUser.Builder("targetOther@test.com") + .customNumber("favorite_number", Arrays.asList(42, 32)) + .build(); - Boolean b = simpleFlag.evaluate(user); + Boolean b = simpleFlag.evaluate(user); - assertEquals(true, b); - } + assertEquals(true, b); + } + + @Test + public void testFlagForTargetBooleanTestOn() { + LDUser user = new LDUser.Builder("targetOther@test.com") + .custom("likes_cats", true) + .build(); + + Boolean b = simpleFlag.evaluate(user); + + assertEquals(true, b); + } @Test public void testFlagForTargetGroupOff() { @@ -152,16 +165,16 @@ public void testFlagForTargetNumericTestOff() { assertEquals(false, b); } - @Test - public void testFlagForTargetBooleanTestOff() { - LDUser user = new LDUser.Builder("targetOther@test.com") - .custom("likes_dogs", false) - .build(); + @Test + public void testFlagForTargetBooleanTestOff() { + LDUser user = new LDUser.Builder("targetOther@test.com") + .custom("likes_dogs", false) + .build(); - Boolean b = simpleFlag.evaluate(user); + Boolean b = simpleFlag.evaluate(user); - assertEquals(false, b); - } + assertEquals(false, b); + } @Test public void testDisabledFlagAlwaysOff() { From 45e664bc51d2fc873c1e8047b07955696be9b5d5 Mon Sep 17 00:00:00 2001 From: John Kodumal Date: Tue, 12 Jan 2016 14:48:01 -0800 Subject: [PATCH 5/5] Eliminate excessive JsonPrimitive construction in an attempt to avoid performance issues with TargetRule matching --- .../com/launchdarkly/client/FeatureRep.java | 2 +- .../launchdarkly/client/IdentifyEvent.java | 2 +- .../java/com/launchdarkly/client/LDUser.java | 64 +++++++++---------- .../com/launchdarkly/client/Variation.java | 18 +++--- .../com/launchdarkly/client/LDUserTest.java | 15 +++-- 5 files changed, 52 insertions(+), 49 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/FeatureRep.java b/src/main/java/com/launchdarkly/client/FeatureRep.java index 1c20631c4..49ef2441f 100644 --- a/src/main/java/com/launchdarkly/client/FeatureRep.java +++ b/src/main/java/com/launchdarkly/client/FeatureRep.java @@ -78,7 +78,7 @@ private Float paramForUser(LDUser user) { String hash; if (user.getKey() != null) { - idHash = user.getKey(); + idHash = user.getKey().getAsString(); } else { return null; diff --git a/src/main/java/com/launchdarkly/client/IdentifyEvent.java b/src/main/java/com/launchdarkly/client/IdentifyEvent.java index b7e599b69..ff36f5049 100644 --- a/src/main/java/com/launchdarkly/client/IdentifyEvent.java +++ b/src/main/java/com/launchdarkly/client/IdentifyEvent.java @@ -5,6 +5,6 @@ class IdentifyEvent extends Event { IdentifyEvent(LDUser user) { - super("identify", user.getKey(), user); + super("identify", user.getKey().getAsString(), user); } } diff --git a/src/main/java/com/launchdarkly/client/LDUser.java b/src/main/java/com/launchdarkly/client/LDUser.java index 7e2e1709d..5ce40ddb0 100644 --- a/src/main/java/com/launchdarkly/client/LDUser.java +++ b/src/main/java/com/launchdarkly/client/LDUser.java @@ -24,17 +24,17 @@ * launch a feature to the top 10% of users on a site. */ public class LDUser { - private String key; - private String secondary; - private String ip; - private String email; - private String name; - private String avatar; - private String firstName; - private String lastName; - private Boolean anonymous; - - private LDCountryCode country; + private JsonPrimitive key; + private JsonPrimitive secondary; + private JsonPrimitive ip; + private JsonPrimitive email; + private JsonPrimitive name; + private JsonPrimitive avatar; + private JsonPrimitive firstName; + private JsonPrimitive lastName; + private JsonPrimitive anonymous; + + private JsonPrimitive country; private Map custom; private static final Logger logger = LoggerFactory.getLogger(LDUser.class); @@ -44,16 +44,16 @@ public class LDUser { } protected LDUser(Builder builder) { - this.key = builder.key; - this.ip = builder.ip; - this.country = builder.country; - this.secondary = builder.secondary; - this.firstName = builder.firstName; - this.lastName = builder.lastName; - this.email = builder.email; - this.name = builder.name; - this.avatar = builder.avatar; - this.anonymous = builder.anonymous; + this.key = builder.key == null? null : new JsonPrimitive(builder.key); + this.ip = builder.ip == null? null : new JsonPrimitive(builder.ip); + this.country = builder.country == null? null : new JsonPrimitive(builder.country.getAlpha2()); + this.secondary = builder.secondary == null ? null : new JsonPrimitive(builder.secondary); + this.firstName = builder.firstName == null ? null : new JsonPrimitive(builder.firstName); + this.lastName = builder.lastName == null ? null : new JsonPrimitive(builder.lastName); + this.email = builder.email == null ? null : new JsonPrimitive(builder.email); + 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 = new HashMap(builder.custom); } @@ -62,31 +62,31 @@ protected LDUser(Builder builder) { * @param key a {@code String} that uniquely identifies a user */ public LDUser(String key) { - this.key = key; + this.key = new JsonPrimitive(key); this.custom = new HashMap(); } - String getKey() { + JsonPrimitive getKey() { return key; } - String getIp() { return ip; } + JsonPrimitive getIp() { return ip; } - LDCountryCode getCountry() { return country; } + JsonPrimitive getCountry() { return country; } - String getSecondary() { return secondary; } + JsonPrimitive getSecondary() { return secondary; } - String getName() { return name; } + JsonPrimitive getName() { return name; } - String getFirstName() { return firstName; } + JsonPrimitive getFirstName() { return firstName; } - String getLastName() { return lastName; } + JsonPrimitive getLastName() { return lastName; } - String getEmail() { return email; } + JsonPrimitive getEmail() { return email; } - String getAvatar() { return avatar; } + JsonPrimitive getAvatar() { return avatar; } - Boolean getAnonymous() { return anonymous; } + JsonPrimitive getAnonymous() { return anonymous; } JsonElement getCustom(String key) { return custom.get(key); diff --git a/src/main/java/com/launchdarkly/client/Variation.java b/src/main/java/com/launchdarkly/client/Variation.java index b896e107c..0d2f7a17d 100644 --- a/src/main/java/com/launchdarkly/client/Variation.java +++ b/src/main/java/com/launchdarkly/client/Variation.java @@ -137,47 +137,47 @@ public boolean matchTarget(LDUser user) { Object uValue = null; if (attribute.equals("key")) { if (user.getKey() != null) { - uValue = new JsonPrimitive(user.getKey()); + uValue = user.getKey(); } } else if (attribute.equals("ip") && user.getIp() != null) { if (user.getIp() != null) { - uValue = new JsonPrimitive(user.getIp()); + uValue = user.getIp(); } } else if (attribute.equals("country")) { if (user.getCountry() != null) { - uValue = new JsonPrimitive(user.getCountry().getAlpha2()); + uValue = user.getCountry(); } } else if (attribute.equals("email")) { if (user.getEmail() != null) { - uValue = new JsonPrimitive(user.getEmail()); + uValue = user.getEmail(); } } else if (attribute.equals("firstName")) { if (user.getFirstName() != null ) { - uValue = new JsonPrimitive(user.getFirstName()); + uValue = user.getFirstName(); } } else if (attribute.equals("lastName")) { if (user.getLastName() != null) { - uValue = new JsonPrimitive(user.getLastName()); + uValue = user.getLastName(); } } else if (attribute.equals("avatar")) { if (user.getAvatar() != null) { - uValue = new JsonPrimitive(user.getAvatar()); + uValue = user.getAvatar(); } } else if (attribute.equals("name")) { if (user.getName() != null) { - uValue = new JsonPrimitive(user.getName()); + uValue = user.getName(); } } else if (attribute.equals("anonymous")) { if (user.getAnonymous() != null) { - uValue = new JsonPrimitive(user.getAnonymous()); + uValue = user.getAnonymous(); } } else { // Custom attribute diff --git a/src/test/java/com/launchdarkly/client/LDUserTest.java b/src/test/java/com/launchdarkly/client/LDUserTest.java index 43a52975f..8aa00c679 100644 --- a/src/test/java/com/launchdarkly/client/LDUserTest.java +++ b/src/test/java/com/launchdarkly/client/LDUserTest.java @@ -1,15 +1,18 @@ package com.launchdarkly.client; import com.google.gson.Gson; +import com.google.gson.JsonPrimitive; import org.junit.Test; public class LDUserTest { + private JsonPrimitive us = new JsonPrimitive(LDCountryCode.US.getAlpha2()); + @Test public void testValidCountryCodeSetsCountry() { LDUser user = new LDUser.Builder("key").country(LDCountryCode.US).build(); - assert(user.getCountry().equals(LDCountryCode.US)); + assert(user.getCountry().equals(us)); } @@ -17,21 +20,21 @@ public void testValidCountryCodeSetsCountry() { public void testValidCountryCodeStringSetsCountry() { LDUser user = new LDUser.Builder("key").country("US").build(); - assert(user.getCountry().equals(LDCountryCode.US)); + assert(user.getCountry().equals(us)); } @Test public void testValidCountryCode3SetsCountry() { LDUser user = new LDUser.Builder("key").country("USA").build(); - assert(user.getCountry().equals(LDCountryCode.US)); + assert(user.getCountry().equals(us)); } @Test public void testAmbiguousCountryNameSetsCountryWithExactMatch() { // "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(LDCountryCode.US)); + assert(user.getCountry().equals(us)); } @Test @@ -45,7 +48,7 @@ public void testAmbiguousCountryNameSetsCountryWithPartialMatch() { @Test public void testPartialUniqueMatchSetsCountry() { LDUser user = new LDUser.Builder("key").country("United States Minor").build(); - assert(user.getCountry().equals(LDCountryCode.UM)); + assert(user.getCountry().equals(new JsonPrimitive(LDCountryCode.UM.getAlpha2()))); } @Test @@ -63,6 +66,6 @@ public void testLDUserJsonSerializationContainsCountryAsTwoDigitCode() { LDUser deserialized = gson.fromJson(jsonStr, LDUser.class); - assert(deserialized.getCountry().equals(LDCountryCode.US)); + assert(deserialized.getCountry().equals(us)); } }