Revert "Merge pull request #116 from launchdarkly/eb/ch32302/experimentation-events"

This reverts commit 29716e6, reversing
changes made to 67beb27.

Author: eli-darkly

src/main/java/com/launchdarkly/client/EventFactory.java

@@ -9,29 +9,11 @@ abstract class EventFactory {
   protected abstract long getTimestamp();
   protected abstract boolean isIncludeReasons();
 
-  public Event.FeatureRequest newFeatureRequestEvent(FeatureFlag flag, LDUser user, JsonElement value,
-      Integer variationIndex, EvaluationReason reason, JsonElement defaultValue, String prereqOf) {
-    boolean requireExperimentData = isExperiment(flag, reason);
-    return new Event.FeatureRequest(
-        getTimestamp(),
-        flag.getKey(),
-        user,
-        flag.getVersion(),
-        variationIndex,
-        value,
-        defaultValue,
-        prereqOf,
-        requireExperimentData || flag.isTrackEvents(),
-        flag.getDebugEventsUntilDate(),
-        (requireExperimentData || isIncludeReasons()) ? reason : null,
-        false
-    );
-  }
-  
   public Event.FeatureRequest newFeatureRequestEvent(FeatureFlag flag, LDUser user, EvaluationDetail<JsonElement> result, JsonElement defaultVal) {
-    return newFeatureRequestEvent(flag, user, result == null ? null : result.getValue(),
-        result == null ? null : result.getVariationIndex(), result == null ? null : result.getReason(),
-        defaultVal, null);
+    return new Event.FeatureRequest(getTimestamp(), flag.getKey(), user, flag.getVersion(),
+        result == null ? null : result.getVariationIndex(), result == null ? null : result.getValue(),
+        defaultVal, null, flag.isTrackEvents(), flag.getDebugEventsUntilDate(),
+        isIncludeReasons() ? result.getReason() : null, false);
   }
 
   public Event.FeatureRequest newDefaultFeatureRequestEvent(FeatureFlag flag, LDUser user, JsonElement defaultValue,
@@ -49,9 +31,10 @@ public Event.FeatureRequest newUnknownFeatureRequestEvent(String key, LDUser use
 
   public Event.FeatureRequest newPrerequisiteFeatureRequestEvent(FeatureFlag prereqFlag, LDUser user, EvaluationDetail<JsonElement> result,
       FeatureFlag prereqOf) {
-    return newFeatureRequestEvent(prereqFlag, user, result == null ? null : result.getValue(),
-        result == null ? null : result.getVariationIndex(), result == null ? null : result.getReason(),
-        null, prereqOf.getKey());
+    return new Event.FeatureRequest(getTimestamp(), prereqFlag.getKey(), user, prereqFlag.getVersion(),
+        result == null ? null : result.getVariationIndex(), result == null ? null : result.getValue(),
+        null, prereqOf.getKey(), prereqFlag.isTrackEvents(), prereqFlag.getDebugEventsUntilDate(),
+        isIncludeReasons() ? result.getReason() : null, false);
   }
 
   public Event.FeatureRequest newDebugEvent(Event.FeatureRequest from) {
@@ -67,34 +50,6 @@ public Event.Identify newIdentifyEvent(LDUser user) {
     return new Event.Identify(getTimestamp(), user);
   }
 
-  private boolean isExperiment(FeatureFlag flag, EvaluationReason reason) {
-    if (reason == null) {
-      // doesn't happen in real life, but possible in testing
-      return false;
-    }
-    switch (reason.getKind()) { 
-    case FALLTHROUGH:
-      return flag.isTrackEventsFallthrough();
-    case RULE_MATCH:
-      if (!(reason instanceof EvaluationReason.RuleMatch)) {
-        // shouldn't be possible
-        return false;
-      }
-      EvaluationReason.RuleMatch rm = (EvaluationReason.RuleMatch)reason;
-      int ruleIndex = rm.getRuleIndex();
-      // Note, it is OK to rely on the rule index rather than the unique ID in this context, because the
-      // FeatureFlag that is passed to us here *is* necessarily the same version of the flag that was just
-      // evaluated, so we cannot be out of sync with its rule list.
-      if (ruleIndex >= 0 && ruleIndex < flag.getRules().size()) {
-        Rule rule = flag.getRules().get(ruleIndex);
-        return rule.isTrackEvents();
-      }
-      return false;
-    default:
-      return false;
-    }
-  }
-
   public static class DefaultEventFactory extends EventFactory {
     private final boolean includeReasons;
 

src/main/java/com/launchdarkly/client/FeatureFlag.java

@@ -32,7 +32,6 @@ class FeatureFlag implements VersionedData {
   private List<JsonElement> variations;
   private boolean clientSide;
   private boolean trackEvents;
-  private boolean trackEventsFallthrough;
   private Long debugEventsUntilDate;
   private boolean deleted;
 
@@ -49,8 +48,7 @@ static Map<String, FeatureFlag> fromJsonMap(LDConfig config, String json) {
 
   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 clientSide, boolean trackEvents, boolean trackEventsFallthrough,
-      Long debugEventsUntilDate, boolean deleted) {
+      boolean clientSide, boolean trackEvents, Long debugEventsUntilDate, boolean deleted) {
     this.key = key;
     this.version = version;
     this.on = on;
@@ -63,7 +61,6 @@ static Map<String, FeatureFlag> fromJsonMap(LDConfig config, String json) {
     this.variations = variations;
     this.clientSide = clientSide;
     this.trackEvents = trackEvents;
-    this.trackEventsFallthrough = trackEventsFallthrough;
     this.debugEventsUntilDate = debugEventsUntilDate;
     this.deleted = deleted;
   }
@@ -181,10 +178,6 @@ public boolean isTrackEvents() {
     return trackEvents;
   }
 
-  public boolean isTrackEventsFallthrough() {
-    return trackEventsFallthrough;
-  }
-  
   public Long getDebugEventsUntilDate() {
     return debugEventsUntilDate;
   }

src/main/java/com/launchdarkly/client/FeatureFlagBuilder.java

@@ -19,7 +19,6 @@ class FeatureFlagBuilder {
   private List<JsonElement> variations = new ArrayList<>();
   private boolean clientSide;
   private boolean trackEvents;
-  private boolean trackEventsFallthrough;
   private Long debugEventsUntilDate;
   private boolean deleted;
 
@@ -41,7 +40,6 @@ class FeatureFlagBuilder {
       this.variations = f.getVariations();
       this.clientSide = f.isClientSide();
       this.trackEvents = f.isTrackEvents();
-      this.trackEventsFallthrough = f.isTrackEventsFallthrough();
       this.debugEventsUntilDate = f.getDebugEventsUntilDate();
       this.deleted = f.isDeleted();
     }
@@ -105,11 +103,6 @@ FeatureFlagBuilder trackEvents(boolean trackEvents) {
     this.trackEvents = trackEvents;
     return this;
   }
-
-  FeatureFlagBuilder trackEventsFallthrough(boolean trackEventsFallthrough) {
-    this.trackEventsFallthrough = trackEventsFallthrough;
-    return this;
-  }
 
   FeatureFlagBuilder debugEventsUntilDate(Long debugEventsUntilDate) {
     this.debugEventsUntilDate = debugEventsUntilDate;
@@ -123,6 +116,6 @@ FeatureFlagBuilder deleted(boolean deleted) {
 
   FeatureFlag build() {
     return new FeatureFlag(key, version, on, prerequisites, salt, targets, rules, fallthrough, offVariation, variations,
-        clientSide, trackEvents, trackEventsFallthrough, debugEventsUntilDate, deleted);
+        clientSide, trackEvents, debugEventsUntilDate, deleted);
   }
 }

src/main/java/com/launchdarkly/client/Rule.java

@@ -10,36 +10,22 @@
 class Rule extends VariationOrRollout {
   private String id;
   private List<Clause> clauses;
-  private boolean trackEvents;
 
   // We need this so Gson doesn't complain in certain java environments that restrict unsafe allocation
   Rule() {
     super();
   }
 
-  Rule(String id, List<Clause> clauses, Integer variation, Rollout rollout, boolean trackEvents) {
+  Rule(String id, List<Clause> clauses, Integer variation, Rollout rollout) {
     super(variation, rollout);
     this.id = id;
     this.clauses = clauses;
-    this.trackEvents = trackEvents;
-  }
-  
-  Rule(String id, List<Clause> clauses, Integer variation, Rollout rollout) {
-    this(id, clauses, variation, rollout, false);
   }
 
   String getId() {
     return id;
   }
 
-  Iterable<Clause> getClauses() {
-    return clauses;
-  }
-  
-  boolean isTrackEvents() {
-    return trackEvents;
-  }
-  
   boolean matchesUser(FeatureStore store, LDUser user) {
     for (Clause clause : clauses) {
       if (!clause.matchesUser(store, user)) {

src/test/java/com/launchdarkly/client/LDClientEventTest.java

@@ -15,15 +15,11 @@
 import static com.launchdarkly.client.TestUtil.jdouble;
 import static com.launchdarkly.client.TestUtil.jint;
 import static com.launchdarkly.client.TestUtil.js;
-import static com.launchdarkly.client.TestUtil.makeClauseToMatchUser;
-import static com.launchdarkly.client.TestUtil.makeClauseToNotMatchUser;
 import static com.launchdarkly.client.TestUtil.specificEventProcessor;
 import static com.launchdarkly.client.TestUtil.specificFeatureStore;
 import static com.launchdarkly.client.VersionedDataKind.FEATURES;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 
 public class LDClientEventTest {
   private static final LDUser user = new LDUser("userkey");
@@ -261,111 +257,6 @@ public void jsonVariationDetailSendsEventForUnknownFlag() throws Exception {
         EvaluationReason.error(ErrorKind.FLAG_NOT_FOUND));
   }
 
-  @Test
-  public void eventTrackingAndReasonCanBeForcedForRule() throws Exception {
-    Clause clause = makeClauseToMatchUser(user);
-    Rule rule = new RuleBuilder().id("id").clauses(clause).variation(1).trackEvents(true).build();
-    FeatureFlag flag = new FeatureFlagBuilder("flag")
-        .on(true)
-        .rules(Arrays.asList(rule))
-        .offVariation(0)
-        .variations(js("off"), js("on"))
-        .build();
-    featureStore.upsert(FEATURES, flag);
-
-    client.stringVariation("flag", user, "default");
-    
-    // Note, we did not call stringVariationDetail and the flag is not tracked, but we should still get
-    // tracking and a reason, because the rule-level trackEvents flag is on for the matched rule.
-    
-    assertEquals(1, eventSink.events.size());
-    Event.FeatureRequest event = (Event.FeatureRequest)eventSink.events.get(0);
-    assertTrue(event.trackEvents);
-    assertEquals(EvaluationReason.ruleMatch(0, "id"), event.reason);
-  }
-
-  @Test
-  public void eventTrackingAndReasonAreNotForcedIfFlagIsNotSetForMatchingRule() throws Exception {
-    Clause clause0 = makeClauseToNotMatchUser(user);
-    Clause clause1 = makeClauseToMatchUser(user);
-    Rule rule0 = new RuleBuilder().id("id0").clauses(clause0).variation(1).trackEvents(true).build();
-    Rule rule1 = new RuleBuilder().id("id1").clauses(clause1).variation(1).trackEvents(false).build();
-    FeatureFlag flag = new FeatureFlagBuilder("flag")
-        .on(true)
-        .rules(Arrays.asList(rule0, rule1))
-        .offVariation(0)
-        .variations(js("off"), js("on"))
-        .build();
-    featureStore.upsert(FEATURES, flag);
-
-    client.stringVariation("flag", user, "default");
-    
-    // It matched rule1, which has trackEvents: false, so we don't get the override behavior
-    
-    assertEquals(1, eventSink.events.size());
-    Event.FeatureRequest event = (Event.FeatureRequest)eventSink.events.get(0);
-    assertFalse(event.trackEvents);
-    assertNull(event.reason);
-  }
-
-  @Test
-  public void eventTrackingAndReasonCanBeForcedForFallthrough() throws Exception {
-    FeatureFlag flag = new FeatureFlagBuilder("flag")
-        .on(true)
-        .fallthrough(new VariationOrRollout(0, null))
-        .variations(js("fall"), js("off"), js("on"))
-        .trackEventsFallthrough(true)
-        .build();
-    featureStore.upsert(FEATURES, flag);
-
-    client.stringVariation("flag", user, "default");
-    
-    // Note, we did not call stringVariationDetail and the flag is not tracked, but we should still get
-    // tracking and a reason, because trackEventsFallthrough is on and the evaluation fell through.
-    
-    assertEquals(1, eventSink.events.size());
-    Event.FeatureRequest event = (Event.FeatureRequest)eventSink.events.get(0);
-    assertTrue(event.trackEvents);
-    assertEquals(EvaluationReason.fallthrough(), event.reason);
-  }
-
-  @Test
-  public void eventTrackingAndReasonAreNotForcedForFallthroughIfFlagIsNotSet() throws Exception {
-    FeatureFlag flag = new FeatureFlagBuilder("flag")
-        .on(true)
-        .fallthrough(new VariationOrRollout(0, null))
-        .variations(js("fall"), js("off"), js("on"))
-        .trackEventsFallthrough(false)
-        .build();
-    featureStore.upsert(FEATURES, flag);
-
-    client.stringVariation("flag", user, "default");
-    
-    assertEquals(1, eventSink.events.size());
-    Event.FeatureRequest event = (Event.FeatureRequest)eventSink.events.get(0);
-    assertFalse(event.trackEvents);
-    assertNull(event.reason);
-  }
-
-  @Test
-  public void eventTrackingAndReasonAreNotForcedForFallthroughIfReasonIsNotFallthrough() throws Exception {
-    FeatureFlag flag = new FeatureFlagBuilder("flag")
-        .on(false) // so the evaluation reason will be OFF, not FALLTHROUGH
-        .offVariation(1)
-        .fallthrough(new VariationOrRollout(0, null))
-        .variations(js("fall"), js("off"), js("on"))
-        .trackEventsFallthrough(true)
-        .build();
-    featureStore.upsert(FEATURES, flag);
-
-    client.stringVariation("flag", user, "default");
-    
-    assertEquals(1, eventSink.events.size());
-    Event.FeatureRequest event = (Event.FeatureRequest)eventSink.events.get(0);
-    assertFalse(event.trackEvents);
-    assertNull(event.reason);
-  }
-  
   @Test
   public void eventIsSentForExistingPrererequisiteFlag() throws Exception {
     FeatureFlag f0 = new FeatureFlagBuilder("feature0")
@@ -466,8 +357,6 @@ private void checkFeatureEvent(Event e, FeatureFlag flag, JsonElement value, Jso
     assertEquals(defaultVal, fe.defaultVal);
     assertEquals(prereqOf, fe.prereqOf);
     assertEquals(reason, fe.reason);
-    assertEquals(flag.isTrackEvents(), fe.trackEvents);
-    assertEquals(flag.getDebugEventsUntilDate(), fe.debugEventsUntilDate);
   }
 
   private void checkUnknownFeatureEvent(Event e, String key, JsonElement defaultVal, String prereqOf,
@@ -481,7 +370,5 @@ private void checkUnknownFeatureEvent(Event e, String key, JsonElement defaultVa
     assertEquals(defaultVal, fe.defaultVal);
     assertEquals(prereqOf, fe.prereqOf);
     assertEquals(reason, fe.reason);
-    assertFalse(fe.trackEvents);
-    assertNull(fe.debugEventsUntilDate);
   }
 }

src/test/java/com/launchdarkly/client/RuleBuilder.java

@@ -176,14 +176,6 @@ public static FeatureFlag flagWithValue(String key, JsonElement value) {
         .build();
   }
 
-  public static Clause makeClauseToMatchUser(LDUser user) {
-    return new Clause("key", Operator.in, Arrays.asList(user.getKey()), false);
-  }
-
-  public static Clause makeClauseToNotMatchUser(LDUser user) {
-    return new Clause("key", Operator.in, Arrays.asList(js("not-" + user.getKeyAsString())), false);
-  }
-  
   public static class DataBuilder {
     private Map<VersionedDataKind<?>, Map<String, ? extends VersionedData>> data = new HashMap<>();