Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.
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
44 changes: 38 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,17 @@ dependencies {
}

jar {
baseName = 'launchdarkly-client'
manifest {
attributes("Implementation-Version": version)
}
baseName = 'launchdarkly-client'
// thin classifier means that the non-shaded non-fat jar is still available
// but is opt-in since users will have to specify it.
classifier = 'thin'
manifest {
attributes("Implementation-Version": version)
}
}

task wrapper(type: Wrapper) {
gradleVersion = '2.0'
gradleVersion = '2.14-rc-3'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the version mentioned here: https://github.com/johnrengelman/shadow

}

buildscript {
Expand Down Expand Up @@ -90,7 +93,36 @@ githubPages {
}

shadowJar {
classifier = 'all'
baseName = 'launchdarkly-client'
//no classifier means that the fat + shaded jar becomes the default artifact
classifier = ''

// Shade all jars except for launchdarkly
relocate('com', 'com.launchdarkly.shaded.com') {
Copy link
Contributor Author

@drichelson drichelson Jun 2, 2016

Choose a reason for hiding this comment

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

The potential problem with shading Gson is that we return a Gson type (JsonElement) from our public interface: https://github.com/launchdarkly/java-client/blob/fa61ceb5f343c53db01f88b417a623575ab9fc58/src/main/java/com/launchdarkly/client/LDClient.java#L298-L298

This means that if a user's code already imports Gson this JsonElement is a different type than they might be expecting.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. I hadn't considered that when I made the suggestion to shade Gson. I think we should not shade it then-- leaking a shaded class into an external interface isn't a good practice.

In addition to just having the wrong type, interacting with the JsonElement is going to be hard-- you'll have to manipulate the shaded API, which exposes more shaded footprint.

Choose a reason for hiding this comment

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

It might be appropriate to reconsider this approach. In my project, SBT (sbt-assembly) got confused by launchdarkly-client depending on com.google.code.gson:gson, but both artifacts included com/google/gson/Gson.class etc.

exclude("com.launchdarkly.client.*")
exclude("com.launchdarkly.eventsource.*")
exclude("com.google.gson.*")
exclude("com.google.gson.annotations.*")
exclude("com.google.gson.internal.*")
exclude("com.google.gson.internal.bind.*")
exclude("com.google.gson.internal.bind.util.*")
exclude("com.google.gson.internal.bind.util.*")
exclude("com.google.gson.reflect.*")
exclude("com.google.gson.stream.*")
}
relocate('okhttp3', 'com.launchdarkly.shaded.okhttp3')
relocate('okio', 'com.launchdarkly.shaded.okio')
relocate('org', 'com.launchdarkly.shaded.org') {
exclude("org.slf4j.*")
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch on excluding the logging facade.

exclude("org.slf4j.event.*")
exclude("org.slf4j.helpers.*")
exclude("org.slf4j.spi.*")
}
relocate('redis', 'com.launchdarkly.shaded.redis')

manifest {
attributes("Implementation-Version": version)
}
}

test {
Expand Down
Binary file modified gradle/wrapper/gradle-wrapper.jar
Binary file not shown.
4 changes: 2 additions & 2 deletions gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#Mon Aug 11 16:15:27 PDT 2014
#Tue Jun 07 10:02:02 PDT 2016
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-2.0-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-2.14-rc-3-all.zip
52 changes: 26 additions & 26 deletions gradlew
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,30 @@
##
##############################################################################

# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS=""
# Attempt to set APP_HOME
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated gradle wrapper

# Resolve links: $0 may be a link
PRG="$0"
# Need this for relative symlinks.
while [ -h "$PRG" ] ; do
ls=`ls -ld "$PRG"`
link=`expr "$ls" : '.*-> \(.*\)$'`
if expr "$link" : '/.*' > /dev/null; then
PRG="$link"
else
PRG=`dirname "$PRG"`"/$link"
fi
done
SAVED="`pwd`"
cd "`dirname \"$PRG\"`/" >/dev/null
APP_HOME="`pwd -P`"
cd "$SAVED" >/dev/null

APP_NAME="Gradle"
APP_BASE_NAME=`basename "$0"`

# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS=""

# Use the maximum available, or set MAX_FD != -1 to use that value.
MAX_FD="maximum"

Expand All @@ -30,6 +48,7 @@ die ( ) {
cygwin=false
msys=false
darwin=false
nonstop=false
case "`uname`" in
CYGWIN* )
cygwin=true
Expand All @@ -40,31 +59,11 @@ case "`uname`" in
MINGW* )
msys=true
;;
NONSTOP* )
nonstop=true
;;
esac

# For Cygwin, ensure paths are in UNIX format before anything is touched.
if $cygwin ; then
[ -n "$JAVA_HOME" ] && JAVA_HOME=`cygpath --unix "$JAVA_HOME"`
fi

# Attempt to set APP_HOME
# Resolve links: $0 may be a link
PRG="$0"
# Need this for relative symlinks.
while [ -h "$PRG" ] ; do
ls=`ls -ld "$PRG"`
link=`expr "$ls" : '.*-> \(.*\)$'`
if expr "$link" : '/.*' > /dev/null; then
PRG="$link"
else
PRG=`dirname "$PRG"`"/$link"
fi
done
SAVED="`pwd`"
cd "`dirname \"$PRG\"`/" >&-
APP_HOME="`pwd -P`"
cd "$SAVED" >&-

CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar

# Determine the Java command to use to start the JVM.
Expand All @@ -90,7 +89,7 @@ location of your Java installation."
fi

# Increase the maximum file descriptors if we can.
if [ "$cygwin" = "false" -a "$darwin" = "false" ] ; then
if [ "$cygwin" = "false" -a "$darwin" = "false" -a "$nonstop" = "false" ] ; then
MAX_FD_LIMIT=`ulimit -H -n`
if [ $? -eq 0 ] ; then
if [ "$MAX_FD" = "maximum" -o "$MAX_FD" = "max" ] ; then
Expand All @@ -114,6 +113,7 @@ fi
if $cygwin ; then
APP_HOME=`cygpath --path --mixed "$APP_HOME"`
CLASSPATH=`cygpath --path --mixed "$CLASSPATH"`
JAVACMD=`cygpath --unix "$JAVACMD"`

# We build the pattern for arguments to be converted via cygpath
ROOTDIRSRAW=`find -L / -maxdepth 1 -mindepth 1 -type d 2>/dev/null`
Expand Down
8 changes: 4 additions & 4 deletions gradlew.bat
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
@rem Set local scope for the variables with windows NT shell
if "%OS%"=="Windows_NT" setlocal

@rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
set DEFAULT_JVM_OPTS=

set DIRNAME=%~dp0
if "%DIRNAME%" == "" set DIRNAME=.
set APP_BASE_NAME=%~n0
set APP_HOME=%DIRNAME%

@rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
set DEFAULT_JVM_OPTS=

@rem Find java.exe
if defined JAVA_HOME goto findJavaFromJavaHome

Expand Down Expand Up @@ -46,7 +46,7 @@ echo location of your Java installation.
goto fail

:init
@rem Get command-line arguments, handling Windowz variants
@rem Get command-line arguments, handling Windows variants

if not "%OS%" == "Windows_NT" goto win9xME_args
if "%@eval[2+2]" == "4" goto 4NT_args
Expand Down
26 changes: 2 additions & 24 deletions src/main/java/com/launchdarkly/client/Clause.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Clause {
private boolean negate;

boolean matchesUser(LDUser user) {
JsonElement userValue = valueOf(user, attribute);
JsonElement userValue = user.getValueForEvaluation(attribute);
if (userValue == null) {
return false;
}
Expand Down Expand Up @@ -58,27 +58,5 @@ private boolean maybeNegate(boolean b) {
return b;
}

static JsonElement valueOf(LDUser user, String attribute) {
switch (attribute) {
case "key":
return user.getKey();
case "ip":
return user.getIp();
case "country":
return user.getCountry();
case "email":
return user.getEmail();
case "firstName":
return user.getFirstName();
case "lastName":
return user.getLastName();
case "avatar":
return user.getAvatar();
case "name":
return user.getName();
case "anonymous":
return user.getAnonymous();
}
return user.getCustom(attribute);
}

Copy link
Contributor Author

@drichelson drichelson Jun 2, 2016

Choose a reason for hiding this comment

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

Moved this behavior to the UserAttribute enum + LDUser class

}
41 changes: 21 additions & 20 deletions src/main/java/com/launchdarkly/client/FeatureFlag.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class FeatureFlag {
private final String salt;
private final List<Target> targets;
private final List<Rule> rules;
private final Rule fallthrough;
private final VariationOrRollout fallthrough;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because fallthrough isn't a rule. It should never have clauses

private final Integer offVariation; //optional
private final List<JsonElement> variations;
private final boolean deleted;
Expand All @@ -36,7 +36,7 @@ static Map<String, FeatureFlag> fromJsonMap(String json) {
return gson.fromJson(json, mapType);
}

FeatureFlag(String key, int version, boolean on, List<Prerequisite> prerequisites, String salt, List<Target> targets, List<Rule> rules, Rule fallthrough, Integer offVariation, List<JsonElement> variations, boolean deleted) {
FeatureFlag(String key, int version, boolean on, List<Prerequisite> prerequisites, String salt, List<Target> targets, List<Rule> rules, VariationOrRollout fallthrough, Integer offVariation, List<JsonElement> variations, boolean deleted) {
this.key = key;
this.version = version;
this.on = on;
Expand Down Expand Up @@ -70,36 +70,37 @@ EvalResult evaluate(LDUser user, FeatureStore featureStore) {
return evaluate(user, featureStore, prereqEvents, visited);
}

// Returning either a nil EvalResult or EvalResult.value indicates prereq failure/error.
private EvalResult evaluate(LDUser user, FeatureStore featureStore, List<FeatureRequestEvent> events, Set<String> visited) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed logic a bit so we always send feature request events for all prereqs unless there is a data integrity problem: ie prereq cycle, or a prereq is not found in the feature store.

boolean prereqOk = true;
EvalResult evalResult = new EvalResult(null, events, visited);
for (Prerequisite prereq : prerequisites) {
visited.add(key);
if (visited.contains(prereq.getKey())) {
evalResult.visitedFeatureKeys.add(key);
if (evalResult.visitedFeatureKeys.contains(prereq.getKey())) {
logger.error("Prerequisite cycle detected when evaluating feature flag: " + key);
return null;
}
JsonElement prereqEvalResultValue = null;
FeatureFlag prereqFeatureFlag = featureStore.get(prereq.getKey());
if (prereqFeatureFlag == null) {
logger.error("Could not retrieve prerequisite flag: " + prereq.getKey() + " when evaluating: " + key);
return null;
}
JsonElement prereqValue;
if (prereqFeatureFlag.isOn()) {
EvalResult prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, events, visited);
if (prereqEvalResult == null) {
return null;
}
prereqValue = prereqEvalResult.value;
visited = prereqEvalResult.visitedFeatureKeys;
events = prereqEvalResult.prerequisiteEvents;
events.add(new FeatureRequestEvent(prereqFeatureFlag.getKey(), user, prereqValue, null));
if (prereqValue == null || !prereqValue.equals(prereqFeatureFlag.getVariation(prereq.getVariation()))) {
return new EvalResult(null, events, visited);
} else if (prereqFeatureFlag.isOn()) {
EvalResult prereqEvalResult = prereqFeatureFlag.evaluate(user, featureStore, evalResult.prerequisiteEvents, evalResult.visitedFeatureKeys);
if (prereqEvalResult == null || prereqEvalResult.getValue() == null || !prereqEvalResult.value.equals(prereqFeatureFlag.getVariation(prereq.getVariation()))) {
prereqOk = false;
}
prereqEvalResultValue = prereqEvalResult != null ? prereqEvalResult.getValue() : null;
} else {
return null;
prereqOk = false;
}
//We don't short circuit and also send events for each prereq.
evalResult.prerequisiteEvents.add(new FeatureRequestEvent(prereqFeatureFlag.getKey(), user, prereqEvalResultValue, null));
}
if (prereqOk) {
evalResult.value = getVariation(evaluateIndex(user));
}
return new EvalResult(getVariation(evaluateIndex(user)), events, visited);
return evalResult;
}

private Integer evaluateIndex(LDUser user) {
Expand Down Expand Up @@ -163,7 +164,7 @@ List<Rule> getRules() {
return rules;
}

Rule getFallthrough() {
VariationOrRollout getFallthrough() {
return fallthrough;
}

Expand Down
13 changes: 7 additions & 6 deletions src/main/java/com/launchdarkly/client/FeatureFlagBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@

import com.google.gson.JsonElement;

import java.util.ArrayList;
import java.util.List;

class FeatureFlagBuilder {
private String key;
private int version;
private boolean on;
private List<Prerequisite> prerequisites;
private List<Prerequisite> prerequisites = new ArrayList<>();
private String salt;
private List<Target> targets;
private List<Rule> rules;
private Rule fallthrough;
private List<Target> targets = new ArrayList<>();
private List<Rule> rules = new ArrayList<>();
private VariationOrRollout fallthrough;
private Integer offVariation;
private List<JsonElement> variations;
private List<JsonElement> variations = new ArrayList<>();
private boolean deleted;

FeatureFlagBuilder(String key) {
Expand Down Expand Up @@ -68,7 +69,7 @@ FeatureFlagBuilder rules(List<Rule> rules) {
return this;
}

FeatureFlagBuilder fallthrough(Rule fallthrough) {
FeatureFlagBuilder fallthrough(VariationOrRollout fallthrough) {
this.fallthrough = fallthrough;
return this;
}
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/launchdarkly/client/LDUser.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ public LDUser(String key) {
this.custom = new HashMap<>();
}

protected JsonElement getValueForEvaluation(String attribute) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this to LDUser since it is user behavior

try {
return UserAttribute.valueOf(attribute).get(this);
} catch (IllegalArgumentException expected) {
return getCustom(attribute);
}
}

JsonPrimitive getKey() {
return key;
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/launchdarkly/client/Operator.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ enum Operator {
in {
@Override
public boolean apply(JsonPrimitive uValue, JsonPrimitive cValue) {
if (uValue.equals(cValue)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this we were failing to match on basic equality for boolean values.

return true;
}

if (uValue.isString() && cValue.isString()) {
if (uValue.getAsString().equals(cValue.getAsString()))
return true;
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/com/launchdarkly/client/Prerequisite.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package com.launchdarkly.client;

class Prerequisite {
private String key;
private int variation;
private final String key;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

made this and many more things private + final

private final int variation;

Prerequisite(String key, int variation) {
this.key = key;
this.variation = variation;
}

String getKey() {
return key;
Expand Down
Loading