Feat/confirm allocations individually (#7)

FrazerBayley · web-flow · commit d6ec5f509e45 · 2019-10-02T12:34:37.000-07:00
* feat: send confirmation events ony for touched allocations

* fix: tests and allocations bug

* chore: updating changelog and readme

* fix: checkstyle
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,11 @@
 
 All updates that affect the released version of the Ascend Java Client will be detailed here. This project adheres to [Semantic Versioning](http://semver.org).
 
+## [0.7.1-beta] - 2019-10-02
+### Added
+- confirmations/contaminations are now only sent when the allocation is used
+- improved logging messages
+
 ## [0.7.0-beta] - 2019-05-29
 ### Fixed 
 - Participant's now initialize with the client instead of getting reused like before.
diff --git a/README.md b/README.md
@@ -15,7 +15,7 @@
         <dependency>
           <groupId>ai.evolv</groupId>
           <artifactId>ascend-sdk</artifactId>
-          <version>0.7.0</version>
+          <version>0.7.1</version>
         </dependency>
     ```
 
@@ -40,8 +40,7 @@
     ```java
        ascendClient.confirm();
     ```
-    *Note: After the AscendClient has initialized, it is important to confirm the participant into the experiment. This action
-     records the participant's allocation and sends the info back to Ascend.*
+    *Note: This action records the participant's allocation and sends the info back to Ascend.*
 
 ### Value Retrieval
 
diff --git a/build.gradle b/build.gradle
@@ -7,7 +7,7 @@ plugins {
 
 group = "ai.evolv"
 archivesBaseName = "ascend-sdk"
-version = "0.7.0"
+version = "0.7.1"
 
 sourceCompatibility = 1.8
 
diff --git a/src/main/java/ai/evolv/Allocations.java b/src/main/java/ai/evolv/Allocations.java
@@ -18,14 +18,20 @@
 
 class Allocations {
 
+    private static final String TOUCHED = "touched";
+    private static final String CONFIRMED = "confirmed";
+    private static final String CONTAMINATED = "contaminated";
+
     private static final Logger LOGGER = LoggerFactory.getLogger(Allocations.class);
 
     private final JsonArray allocations;
+    private final AscendAllocationStore store;
 
     private final Audience audience = new Audience();
 
-    Allocations(JsonArray allocations) {
+    Allocations(JsonArray allocations, AscendAllocationStore store) {
         this.allocations = allocations;
+        this.store = store;
     }
 
     <T> T getValueFromAllocations(String key, Class<T> cls, AscendParticipant participant)
@@ -40,10 +46,17 @@ <T> T getValueFromAllocations(String key, Class<T> cls, AscendParticipant partic
             if (!audience.filter(participant.getUserAttributes(), allocation)) {
                 try {
                     JsonElement element = getElementFromGenome(allocation.get("genome"), keyParts);
-                    return new Gson().fromJson(element, cls);
+                    T value = new Gson().fromJson(element, cls);
+                    if (value != null) {
+                        LOGGER.debug(String.format("Found value for key '%s' in experiment %s",
+                                key, allocation.get("eid").getAsString()));
+                        markTouched(allocation);
+                        store.put(participant.getUserId(), allocations);
+                    }
+                    return value;
                 } catch (AscendKeyError e) {
-                    LOGGER.debug(String.format("Unable to find key %s in experiment %s.",
-                            keyParts.toString(), allocation.get("eid").getAsString()), e);
+                    LOGGER.debug(String.format("Unable to find key '%s' in experiment %s.",
+                            key, allocation.get("eid").getAsString()));
                     continue;
                 }
             }
@@ -123,4 +136,34 @@ Set<String> getActiveExperiments() {
         }
         return activeExperiments;
     }
+
+    static JsonObject markTouched(JsonObject allocation) {
+        allocation.addProperty(TOUCHED, true);
+        return allocation;
+    }
+
+    static boolean isTouched(JsonObject allocation) {
+        return allocation.has(TOUCHED) &&
+                allocation.get(TOUCHED).getAsBoolean();
+    }
+
+    static JsonObject markConfirmed(JsonObject allocation) {
+        allocation.addProperty(CONFIRMED, true);
+        return allocation;
+    }
+
+    static boolean isConfirmed(JsonObject allocation) {
+        return allocation.has(CONFIRMED) &&
+                allocation.get(CONFIRMED).getAsBoolean();
+    }
+
+    static JsonObject markContaminated(JsonObject allocation) {
+        allocation.addProperty(CONTAMINATED, true);
+        return allocation;
+    }
+
+    static boolean isContaminated(JsonObject allocation) {
+        return allocation.has(CONTAMINATED) &&
+                allocation.get(CONTAMINATED).getAsBoolean();
+    }
 }
diff --git a/src/main/java/ai/evolv/Allocator.java b/src/main/java/ai/evolv/Allocator.java
@@ -38,7 +38,7 @@ enum AllocationStatus {
         this.participant = participant;
         this.httpClient = config.getHttpClient();
         this.allocationStatus = AllocationStatus.FETCHING;
-        this.eventEmitter = new EventEmitter(config, participant);
+        this.eventEmitter = new EventEmitter(config, participant, this.store);
 
     }
 
@@ -86,15 +86,8 @@ CompletableFuture<JsonArray> fetchAllocations() {
             store.put(participant.getUserId(), allocations);
             allocationStatus = AllocationStatus.RETRIEVED;
 
-            if (confirmationSandbagged) {
-                eventEmitter.confirm(allocations);
-            }
-
-            if (contaminationSandbagged) {
-                eventEmitter.contaminate(allocations);
-            }
-
-            executionQueue.executeAllWithValuesFromAllocations(allocations);
+            executionQueue.executeAllWithValuesFromAllocations(allocations,
+                eventEmitter, confirmationSandbagged, contaminationSandbagged);
 
             return allocations;
         }).exceptionally(e -> {
@@ -107,16 +100,9 @@ JsonArray resolveAllocationFailure() {
         JsonArray previousAllocations = store.get(participant.getUserId());
         if (allocationsNotEmpty(previousAllocations)) {
             LOGGER.debug("Falling back to participant's previous allocation.");
-            if (confirmationSandbagged) {
-                eventEmitter.confirm(previousAllocations);
-            }
-
-            if (contaminationSandbagged) {
-                eventEmitter.contaminate(previousAllocations);
-            }
-
             allocationStatus = AllocationStatus.RETRIEVED;
-            executionQueue.executeAllWithValuesFromAllocations(previousAllocations);
+            executionQueue.executeAllWithValuesFromAllocations(previousAllocations,
+                    eventEmitter, confirmationSandbagged, contaminationSandbagged);
         } else {
             LOGGER.debug("Falling back to the supplied defaults.");
             allocationStatus = AllocationStatus.FAILED;
diff --git a/src/main/java/ai/evolv/AscendClientFactory.java b/src/main/java/ai/evolv/AscendClientFactory.java
@@ -53,7 +53,7 @@ private static AscendClient createClient(AscendConfig config, AscendParticipant
         CompletableFuture<JsonArray> futureAllocations = allocator.fetchAllocations();
 
         return new AscendClientImpl(config,
-                new EventEmitter(config, participant),
+                new EventEmitter(config, participant, store),
                 futureAllocations,
                 allocator,
                 reconciliationNeeded,
diff --git a/src/main/java/ai/evolv/AscendClientImpl.java b/src/main/java/ai/evolv/AscendClientImpl.java
@@ -50,11 +50,12 @@ public <T> T get(String key, T defaultValue) {
             }
 
             GenericClass<T> cls = new GenericClass(defaultValue.getClass());
-            T value = new Allocations(allocations).getValueFromAllocations(key, cls.getMyType(),
-                    participant);
+            T value = new Allocations(allocations, store).getValueFromAllocations(key,
+                    cls.getMyType(), participant);
 
             if (value == null) {
-                throw new AscendKeyError("Got null when retrieving key from allocations.");
+                throw new AscendKeyError("Got null when retrieving key " +
+                        "from allocations.");
             }
 
             return value;
@@ -71,7 +72,7 @@ public <T> T get(String key, T defaultValue) {
 
     @Override
     public <T> void subscribe(String key, T defaultValue, AscendAction<T> function) {
-        Execution execution = new Execution<>(key, defaultValue, function, participant);
+        Execution execution = new Execution<>(key, defaultValue, function, participant, store);
         if (previousAllocations) {
             try {
                 JsonArray allocations = store.get(participant.getUserId());
diff --git a/src/main/java/ai/evolv/DefaultAllocationStore.java b/src/main/java/ai/evolv/DefaultAllocationStore.java
@@ -6,7 +6,7 @@ public class DefaultAllocationStore implements AscendAllocationStore {
 
     private LruCache cache;
 
-    DefaultAllocationStore(int size) {
+    public DefaultAllocationStore(int size) {
         this.cache = new LruCache(size);
     }
 
diff --git a/src/main/java/ai/evolv/EventEmitter.java b/src/main/java/ai/evolv/EventEmitter.java
@@ -20,13 +20,15 @@ class EventEmitter {
     private final HttpClient httpClient;
     private final AscendConfig config;
     private final AscendParticipant participant;
+    private final AscendAllocationStore store;
 
     private final Audience audience = new Audience();
 
-    EventEmitter(AscendConfig config, AscendParticipant participant) {
+    EventEmitter(AscendConfig config, AscendParticipant participant, AscendAllocationStore store) {
         this.httpClient = config.getHttpClient();
         this.config = config;
         this.participant = participant;
+        this.store = store;
     }
 
     void emit(String key) {
@@ -49,17 +51,29 @@ void contaminate(JsonArray allocations)  {
 
     void sendAllocationEvents(String key, JsonArray allocations) {
         for (JsonElement a : allocations) {
-            if (!audience.filter(participant.getUserAttributes(), a.getAsJsonObject())) {
-                JsonObject allocation = a.getAsJsonObject();
+            JsonObject allocation = a.getAsJsonObject();
+            if (!audience.filter(participant.getUserAttributes(), allocation)
+                    && Allocations.isTouched(allocation)
+                    && !Allocations.isConfirmed(allocation)
+                    && !Allocations.isContaminated(allocation)) {
                 String experimentId = allocation.get("eid").getAsString();
                 String candidateId = allocation.get("cid").getAsString();
 
                 String url = getEventUrl(key, experimentId, candidateId);
                 makeEventRequest(url);
+
+                if (key.equals(CONFIRM_KEY)) {
+                    Allocations.markConfirmed(allocation);
+                } else if (key.equals(CONTAMINATE_KEY)) {
+                    Allocations.markContaminated(allocation);
+                }
+
                 continue;
             }
-            LOGGER.debug(String.format("%s event filtered.", key));
+            LOGGER.debug(String.format("%s event filtered for experiment %s.", key,
+                    allocation.get("eid").getAsString()));
         }
+        store.put(this.participant.getUserId(), allocations);
     }
 
     String getEventUrl(String type, Double score) {
diff --git a/src/main/java/ai/evolv/Execution.java b/src/main/java/ai/evolv/Execution.java
@@ -14,14 +14,17 @@ class Execution<T> {
     private final T defaultValue;
     private final AscendAction function;
     private final AscendParticipant participant;
+    private final AscendAllocationStore store;
 
     private Set<String> alreadyExecuted = new HashSet<>();
 
-    Execution(String key, T defaultValue, AscendAction<T> function, AscendParticipant participant) {
+    Execution(String key, T defaultValue, AscendAction<T> function, AscendParticipant participant,
+              AscendAllocationStore store) {
         this.key = key;
         this.defaultValue = defaultValue;
         this.function = function;
         this.participant = participant;
+        this.store = store;
     }
 
     String getKey() {
@@ -30,7 +33,7 @@ String getKey() {
 
     void executeWithAllocation(JsonArray rawAllocations) throws AscendKeyError {
         GenericClass<T> cls = new GenericClass(defaultValue.getClass());
-        Allocations allocations = new Allocations(rawAllocations);
+        Allocations allocations = new Allocations(rawAllocations, store);
         T value = allocations.getValueFromAllocations(key, cls.getMyType(), participant);
 
         if (value == null) {
diff --git a/src/main/java/ai/evolv/ExecutionQueue.java b/src/main/java/ai/evolv/ExecutionQueue.java
@@ -23,7 +23,10 @@ void enqueue(Execution execution) {
         this.queue.add(execution);
     }
 
-    void executeAllWithValuesFromAllocations(JsonArray allocations) {
+    void executeAllWithValuesFromAllocations(JsonArray allocations,
+                                             EventEmitter eventEmitter,
+                                             boolean confirmationSandbagged,
+                                             boolean contaminationSandbagged) {
         while (!queue.isEmpty()) {
             Execution execution = queue.remove();
             try {
@@ -37,6 +40,14 @@ void executeAllWithValuesFromAllocations(JsonArray allocations) {
                         " the stored actions.", e);
             }
         }
+
+        if (confirmationSandbagged) {
+            eventEmitter.confirm(allocations);
+        }
+
+        if (contaminationSandbagged) {
+            eventEmitter.contaminate(allocations);
+        }
     }
 
     void executeAllWithValuesFromDefaults() {
diff --git a/src/test/java/ai/evolv/AllocationsTest.java b/src/test/java/ai/evolv/AllocationsTest.java
@@ -29,7 +29,8 @@ JsonArray parseRawAllocations(String raw) {
     public void testGetValueFromAllocationGenome() {
         try {
             AscendParticipant participant = AscendParticipant.builder().build();
-            Allocations allocations = new Allocations(parseRawAllocations(rawAllocation));
+            Allocations allocations = new Allocations(parseRawAllocations(rawAllocation),
+                    new DefaultAllocationStore(10));
             Boolean featureImportance = allocations.getValueFromAllocations("algorithms.feature_importance",
                     Boolean.class, participant);
             Assert.assertEquals(featureImportance, false);
@@ -45,7 +46,8 @@ public void testGetValueFromAllocationGenome() {
     public void testGetValueFromMultiAllocationGenome() {
         try {
             AscendParticipant participant = AscendParticipant.builder().build();
-            Allocations allocations = new Allocations(parseRawAllocations(rawMultiAllocation));
+            Allocations allocations = new Allocations(parseRawAllocations(rawMultiAllocation),
+                    new DefaultAllocationStore(10));
             Boolean featureImportance = allocations.getValueFromAllocations("algorithms.feature_importance",
                     Boolean.class, participant);
             Assert.assertEquals(featureImportance, false);
@@ -61,7 +63,8 @@ public void testGetValueFromMultiAllocationGenome() {
     public void testGetValueFromMultiAllocationWithDupsGenome() {
         try{
             AscendParticipant participant = AscendParticipant.builder().build();
-            Allocations allocations = new Allocations(parseRawAllocations(rawMultiAllocationWithDups));
+            Allocations allocations = new Allocations(parseRawAllocations(rawMultiAllocationWithDups),
+                    new DefaultAllocationStore(10));
             Boolean featureImportance = allocations.getValueFromAllocations("algorithms.feature_importance",
                     Boolean.class, participant);
             Assert.assertFalse(featureImportance);
@@ -75,7 +78,8 @@ public void testGetValueFromMultiAllocationWithDupsGenome() {
 
     @Test
     public void testGetActiveExperiments() {
-        Allocations allocations = new Allocations(parseRawAllocations(rawMultiAllocation));
+        Allocations allocations = new Allocations(parseRawAllocations(rawMultiAllocation),
+                new DefaultAllocationStore(10));
         Set<String> activeExperiments = allocations.getActiveExperiments();
         Set<String> expected = new HashSet<>();
         expected.add("test_eid");
diff --git a/src/test/java/ai/evolv/AllocatorTest.java b/src/test/java/ai/evolv/AllocatorTest.java
diff --git a/src/test/java/ai/evolv/EventEmitterTest.java b/src/test/java/ai/evolv/EventEmitterTest.java

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ public class DefaultAllocationStore implements AscendAllocationStore {`
`6`	`6`
`7`	`7`	`private LruCache cache;`
`8`	`8`
`9`		`- DefaultAllocationStore(int size) {`
	`9`	`+ public DefaultAllocationStore(int size) {`
`10`	`10`	`this.cache = new LruCache(size);`
`11`	`11`	`}`
`12`	`12`