prepare 5.5.0 release (#237)

Author: LaunchDarklyCI

build.gradle

@@ -72,7 +72,7 @@ ext.versions = [
     "gson": "2.7",
     "guava": "30.1-jre",
     "jackson": "2.11.2", 
-    "launchdarklyJavaSdkCommon": "1.1.1",
+    "launchdarklyJavaSdkCommon": "1.2.0",
     "okhttp": "4.8.1", // specify this for the SDK build instead of relying on the transitive dependency from okhttp-eventsource
     "okhttpEventsource": "2.3.1",
     "slf4j": "1.7.21",
@@ -83,6 +83,16 @@ ext.versions = [
 // Add dependencies to "libraries.internal" that are not exposed in our public API. These
 // will be completely omitted from the "thin" jar, and will be embedded with shaded names
 // in the other two SDK jars.
+//
+// Note that Gson is included here but Jackson is not, even though there is some Jackson
+// helper code in java-sdk-common. The reason is that the SDK always needs to use Gson for
+// its own usual business, so (except in the "thin" jar) we will be embedding a shaded
+// copy of Gson; but we do not use Jackson normally, we just provide those helpers for use
+// by applications that are already using Jackson. So we do not want to embed it and we do
+// not want it to show up as a dependency at all in our pom (and it's been excluded from
+// the launchdarkly-java-sdk-common pom for the same reason). However, we do include
+// Jackson in "libraries.optional" because we need to generate OSGi optional import
+// headers for it.
 libraries.internal = [
     "com.launchdarkly:launchdarkly-java-sdk-common:${versions.launchdarklyJavaSdkCommon}",
     "commons-codec:commons-codec:${versions.commonsCodec}",
@@ -96,7 +106,15 @@ libraries.internal = [
 // Add dependencies to "libraries.external" that are exposed in our public API, or that have
 // global state that must be shared between the SDK and the caller.
 libraries.external = [
-    "org.slf4j:slf4j-api:${versions.slf4j}",
+    "org.slf4j:slf4j-api:${versions.slf4j}"
+]
+
+// Add dependencies to "libraries.optional" that are not exposed in our public API and are
+// *not* embedded in the SDK jar. These are for optional things that will only work if
+// they are already in the application classpath; we do not want show them as a dependency
+// because that would cause them to be pulled in automatically in all builds. The reason
+// we need to even mention them here at all is for the sake of OSGi optional import headers.
+libraries.optional = [
     "com.fasterxml.jackson.core:jackson-core:${versions.jackson}",
     "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
 ]
@@ -114,24 +132,29 @@ libraries.test = [
     "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
 ]
 
+configurations {
+    // We need to define "internal" as a custom configuration that contains the same things as
+    // "implementation", because "implementation" has special behavior in Gradle that prevents us
+    // from referencing it the way we do in shadeDependencies().
+    internal.extendsFrom implementation
+    optional
+    imports
+}
+
 dependencies {
     implementation libraries.internal
     api libraries.external
     testImplementation libraries.test, libraries.internal, libraries.external
+    optional libraries.optional
 
     commonClasses "com.launchdarkly:launchdarkly-java-sdk-common:${versions.launchdarklyJavaSdkCommon}"
     commonDoc "com.launchdarkly:launchdarkly-java-sdk-common:${versions.launchdarklyJavaSdkCommon}:sources"
 
     // Unlike what the name might suggest, the "shadow" configuration specifies dependencies that
     // should *not* be shaded by the Shadow plugin when we build our shaded jars.
-    shadow libraries.external
-}
+    shadow libraries.external, libraries.optional
 
-configurations {
-    // We need to define "internal" as a custom configuration that contains the same things as
-    // "implementation", because "implementation" has special behavior in Gradle that prevents us
-    // from referencing it the way we do in shadeDependencies().
-    internal.extendsFrom implementation
+    imports libraries.external
 }
 
 checkstyle {
@@ -171,7 +194,6 @@ shadowJar {
 
     dependencies {
         exclude(dependency('org.slf4j:.*:.*'))
-        exclude(dependency('com.fasterxml.jackson.core:.*:.*'))
     }
 
     // Kotlin metadata for shaded classes should not be included - it confuses IDEs
@@ -185,7 +207,7 @@ shadowJar {
         shadeDependencies(project.tasks.shadowJar)
         // Note that "configurations.shadow" is the same as "libraries.external", except it contains
         // objects with detailed information about the resolved dependencies.
-        addOsgiManifest(project.tasks.shadowJar, [ project.configurations.shadow ], [])
+        addOsgiManifest(project.tasks.shadowJar, [ project.configurations.imports ], [])
     }
 
     doLast {
@@ -208,17 +230,17 @@ task shadowJarAll(type: com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJ
     exclude '**/*.kotlin_builtins'
 
     dependencies {
-        exclude(dependency('com.fasterxml.jackson.core:.*:.*'))
+        // Currently we don't need to exclude anything - SLF4J will be embedded, unshaded
     }
 
     // doFirst causes the following steps to be run during Gradle's execution phase rather than the
     // configuration phase; this is necessary because they access the build products
     doFirst {
         shadeDependencies(project.tasks.shadowJarAll)
-        // The "all" jar exposes its bundled Gson and SLF4j dependencies as exports - but, like the
-        // default jar, it *also* imports them ("self-wiring"), which allows the bundle to use a
+        // The "all" jar exposes its bundled SLF4j dependency as an export - but, like the
+        // default jar, it *also* imports it ("self-wiring"), which allows the bundle to use a
         // higher version if one is provided by another bundle.
-        addOsgiManifest(project.tasks.shadowJarAll, [ project.configurations.shadow ], [ project.configurations.shadow ])
+        addOsgiManifest(project.tasks.shadowJarAll, [ project.configurations.imports ], [ project.configurations.imports ])
     }
 
     doLast {
@@ -370,14 +392,18 @@ def addOsgiManifest(jarTask, List<Configuration> importConfigs, List<Configurati
         // Since we're not currently able to use bnd or the Gradle OSGi plugin, we're not discovering
         // imports by looking at the actual code; instead, we're just importing whatever packages each
         // dependency is exporting (if it has an OSGi manifest) or every package in the dependency (if
-        // it doesn't). We also always add *optional* imports for Gson, so that GsonTypeAdapters will
-        // work *if* Gson is present externally.
+        // it doesn't).
         def imports = forEachArtifactAndVisiblePackage(importConfigs, { a, p ->
             bundleImport(p, a.moduleVersion.id.version, nextMajorVersion(a.moduleVersion.id.version))
         }) + systemPackageImports
-        imports += "com.google.gson;resolution:=optional"	
-        	imports += "com.google.gson.reflect;resolution:=optional"
-        	imports += "com.google.gson.stream;resolution:=optional"
+
+        // We also always add *optional* imports for Gson and Jackson, so that GsonTypeAdapters and
+        // JacksonTypeAdapters will work *if* Gson or Jackson is present externally. Currently we
+        // are hard-coding the Gson packages but there is probably a better way.
+        def optImports = [ "com.google.gson", "com.google.gson.reflect", "com.google.gson.stream" ]
+        forEachArtifactAndVisiblePackage([ configurations.optional ]) { a, p -> optImports += p }
+        imports += (optImports.join(";") + ";resolution:=optional" )
+
         attributes("Import-Package": imports.join(","))
 
         // Similarly, we're adding package exports for every package in whatever libraries we're
@@ -391,9 +417,7 @@ def addOsgiManifest(jarTask, List<Configuration> importConfigs, List<Configurati
 }
 
 def bundleImport(packageName, importVersion, versionLimit) {
-    def optional = packageName.startsWith("com.fasterxml.jackson")
-    packageName + ";version=\"[" + importVersion + "," + versionLimit + ")\"" +
-        (optional ? ";resolution:=optional" : "")
+    packageName + ";version=\"[" + importVersion + "," + versionLimit + ")\""
 }
 
 def bundleExport(packageName, exportVersion) {

packaging-test/Makefile

@@ -67,7 +67,7 @@ clean:
 # SECONDEXPANSION is needed so we can use "$@" inside a variable in the prerequisite list of the test targets
 .SECONDEXPANSION:
 
-test-all-jar test-default-jar test-thin-jar: $$@-classes get-sdk-dependencies $$(RUN_JARS_$$@) $(TEST_APP_JAR) $(FELIX_DIR)
+test-all-jar test-default-jar test-thin-jar: $$@-classes $(TEST_APP_JAR) get-sdk-dependencies $$(RUN_JARS_$$@) $(FELIX_DIR)
 	@$(call caption,$@)
 	@./run-non-osgi-test.sh $(RUN_JARS_$@)
 	@./run-osgi-test.sh $(RUN_JARS_$@)
@@ -113,6 +113,7 @@ $(SDK_THIN_JAR):
 	cd .. && ./gradlew jar
 
 $(TEST_APP_JAR): $(SDK_THIN_JAR) $(TEST_APP_SOURCES) | $(TEMP_DIR)
+	mkdir -p $(TEMP_DIR)/dependencies-app
 	cd test-app && ../../gradlew jar
 	cp $(BASE_DIR)/test-app/build/libs/test-app-*.jar $@
 
@@ -121,6 +122,7 @@ get-sdk-dependencies: $(TEMP_DIR)/dependencies-all $(TEMP_DIR)/dependencies-exte
 $(TEMP_DIR)/dependencies-all: | $(TEMP_DIR)
 	[ -d $@ ] || mkdir -p $@
 	cd .. && ./gradlew exportDependencies
+	cp $(TEMP_DIR)/dependencies-app/*.jar $@
 
 $(TEMP_DIR)/dependencies-external: $(TEMP_DIR)/dependencies-all
 	[ -d $@ ] || mkdir -p $@
@@ -132,6 +134,7 @@ $(TEMP_DIR)/dependencies-internal: $(TEMP_DIR)/dependencies-all
 	[ -d $@ ] || mkdir -p $@
 	cp $(TEMP_DIR)/dependencies-all/*.jar $@
 	rm $@/slf4j*.jar
+	rm $@/jackson*.jar
 
 $(FELIX_JAR): $(FELIX_DIR)
 

packaging-test/test-app/build.gradle

@@ -41,6 +41,11 @@ dependencies {
     osgiRuntime "org.slf4j:slf4j-simple:1.7.22"
 }
 
+task exportDependencies(type: Copy, dependsOn: compileJava) {
+    into "../temp/dependencies-app"
+    from configurations.runtimeClasspath.resolvedConfiguration.resolvedArtifacts.collect { it.file }
+}
+
 jar {
     bnd(
         // This consumer-policy directive completely turns off version checking for the test app's
@@ -57,6 +62,8 @@ jar {
             ',com.google.gson;resolution:=optional' +
             ',com.fasterxml.jackson.*;resolution:=optional'
     )
+
+    finalizedBy(exportDependencies)
 }
 
 runOsgi {

src/main/java/com/launchdarkly/sdk/server/DataModel.java

@@ -344,12 +344,23 @@ void setPreprocessed(EvaluatorPreprocessing.ClauseExtra preprocessed) {
   static final class Rollout {
     private List<WeightedVariation> variations;
     private UserAttribute bucketBy;
+    private RolloutKind kind;
+    private Integer seed;
 
     Rollout() {}
 
-    Rollout(List<WeightedVariation> variations, UserAttribute bucketBy) {
+    Rollout(List<WeightedVariation> variations, UserAttribute bucketBy, RolloutKind kind) {
       this.variations = variations;
       this.bucketBy = bucketBy;
+      this.kind = kind;
+      this.seed = null;
+    }
+    
+    Rollout(List<WeightedVariation> variations, UserAttribute bucketBy, RolloutKind kind, Integer seed) {
+      this.variations = variations;
+      this.bucketBy = bucketBy;
+      this.kind = kind;
+      this.seed = seed;
     }
 
     // Guaranteed non-null
@@ -360,6 +371,18 @@ List<WeightedVariation> getVariations() {
     UserAttribute getBucketBy() {
       return bucketBy;
     }
+
+    RolloutKind getKind() {
+      return this.kind;
+    }
+
+    Integer getSeed() {
+      return this.seed;
+    }
+
+    boolean isExperiment() {
+      return kind == RolloutKind.experiment;
+    }
   }
 
   /**
@@ -389,12 +412,14 @@ Rollout getRollout() {
   static final class WeightedVariation {
     private int variation;
     private int weight;
+    private boolean untracked;
 
     WeightedVariation() {}
 
-    WeightedVariation(int variation, int weight) {
+    WeightedVariation(int variation, int weight, boolean untracked) {
       this.variation = variation;
       this.weight = weight;
+      this.untracked = untracked;
     }
 
     int getVariation() {
@@ -404,6 +429,10 @@ int getVariation() {
     int getWeight() {
       return weight;
     }
+
+    boolean isUntracked() {
+      return untracked;
+    }
   }
 
   @JsonAdapter(JsonHelpers.PostProcessingDeserializableTypeAdapterFactory.class)
@@ -511,4 +540,13 @@ static enum Operator {
     semVerGreaterThan,
     segmentMatch
   }
+
+  /**
+   * This enum is all lowercase so that when it is automatically deserialized from JSON, 
+   * the lowercase properties properly map to these enumerations.
+   */
+  static enum RolloutKind {
+    rollout,
+    experiment
+  }
 }

src/main/java/com/launchdarkly/sdk/server/Evaluator.java

@@ -6,6 +6,8 @@
 import com.launchdarkly.sdk.LDUser;
 import com.launchdarkly.sdk.LDValue;
 import com.launchdarkly.sdk.LDValueType;
+import com.launchdarkly.sdk.EvaluationReason.Kind;
+import com.launchdarkly.sdk.server.DataModel.WeightedVariation;
 import com.launchdarkly.sdk.server.interfaces.Event;
 
 import org.slf4j.Logger;
@@ -15,6 +17,7 @@
 import java.util.Set;
 
 import static com.launchdarkly.sdk.EvaluationDetail.NO_VARIATION;
+import static com.launchdarkly.sdk.server.EvaluatorBucketing.bucketUser;
 
 /**
  * Encapsulates the feature flag evaluation logic. The Evaluator has no knowledge of the rest of the SDK environment;
@@ -221,13 +224,52 @@ private EvalResult getOffValue(DataModel.FeatureFlag flag, EvaluationReason reas
   }
 
   private EvalResult getValueForVariationOrRollout(DataModel.FeatureFlag flag, DataModel.VariationOrRollout vr, LDUser user, EvaluationReason reason) {
-    Integer index = EvaluatorBucketing.variationIndexForUser(vr, user, flag.getKey(), flag.getSalt());
-    if (index == null) {
+    int variation = -1;
+    boolean inExperiment = false;
+    Integer maybeVariation = vr.getVariation();
+    if (maybeVariation != null) {
+      variation = maybeVariation.intValue();
+    } else {
+      DataModel.Rollout rollout = vr.getRollout();
+      if (rollout != null && !rollout.getVariations().isEmpty()) {
+        float bucket = bucketUser(rollout.getSeed(), user, flag.getKey(), rollout.getBucketBy(), flag.getSalt());
+        float sum = 0F;
+        for (DataModel.WeightedVariation wv : rollout.getVariations()) {
+          sum += (float) wv.getWeight() / 100000F;
+          if (bucket < sum) {
+            variation = wv.getVariation();
+            inExperiment = vr.getRollout().isExperiment() && !wv.isUntracked();
+            break;
+          }
+        }
+        if (variation < 0) {
+          // The user's bucket value was greater than or equal to the end of the last bucket. This could happen due
+          // to a rounding error, or due to the fact that we are scaling to 100000 rather than 99999, or the flag
+          // data could contain buckets that don't actually add up to 100000. Rather than returning an error in
+          // this case (or changing the scaling, which would potentially change the results for *all* users), we
+          // will simply put the user in the last bucket.
+          WeightedVariation lastVariation = rollout.getVariations().get(rollout.getVariations().size() - 1);
+          variation = lastVariation.getVariation();
+          inExperiment = vr.getRollout().isExperiment() && !lastVariation.isUntracked();
+        }
+      }
+    }
+    
+    if (variation < 0) {
       logger.error("Data inconsistency in feature flag \"{}\": variation/rollout object with no variation or rollout", flag.getKey());
       return EvalResult.error(EvaluationReason.ErrorKind.MALFORMED_FLAG); 
     } else {
-      return getVariation(flag, index, reason);
+      return getVariation(flag, variation, inExperiment ? experimentize(reason) : reason);
+    }
+  }
+
+  private EvaluationReason experimentize(EvaluationReason reason) {
+    if (reason.getKind() == Kind.FALLTHROUGH) {
+      return EvaluationReason.fallthrough(true);
+    } else if (reason.getKind() == Kind.RULE_MATCH) {
+     return EvaluationReason.ruleMatch(reason.getRuleIndex(), reason.getRuleId(), true);
     }
+    return reason;
   }
 
   private boolean ruleMatchesUser(DataModel.FeatureFlag flag, DataModel.Rule rule, LDUser user) {
@@ -344,7 +386,7 @@ private boolean segmentRuleMatchesUser(DataModel.SegmentRule segmentRule, LDUser
     }
 
     // All of the clauses are met. See if the user buckets in
-    double bucket = EvaluatorBucketing.bucketUser(user, segmentKey, segmentRule.getBucketBy(), salt);
+    double bucket = EvaluatorBucketing.bucketUser(null, user, segmentKey, segmentRule.getBucketBy(), salt);
     double weight = (double)segmentRule.getWeight() / 100000.0;
     return bucket < weight;
   }