Verify remote-data info is the same when it was prepared (#1332)

Author: rlepinski

urbanairship-automation/src/main/java/com/urbanairship/automation/InAppAutomation.java

@@ -79,6 +79,7 @@ public class InAppAutomation extends AirshipComponent implements InAppAutomation
 
     private final Map<String, ScheduleDelegate<?>> scheduleDelegateMap = new HashMap<>();
     private final Map<String, FrequencyChecker> frequencyCheckerMap = new HashMap<>();
+    private final Map<String, RemoteDataInfo> remoteDataInfoMap = new HashMap<>();
 
     private final Map<String, Uri> redirectURLs = new HashMap<>();
 
@@ -543,6 +544,7 @@ private void onPrepareSchedule(final @NonNull Schedule<? extends ScheduleData> s
         final AutomationDriver.PrepareScheduleCallback callbackWrapper = result -> {
             if (result != AutomationDriver.PREPARE_RESULT_CONTINUE) {
                 frequencyCheckerMap.remove(schedule.getId());
+                remoteDataInfoMap.remove(schedule.getId());
             }
             callback.onFinish(result);
         };
@@ -560,6 +562,12 @@ private void onPrepareSchedule(final @NonNull Schedule<? extends ScheduleData> s
                 return RetryingExecutor.cancelResult();
             }
 
+            RemoteDataInfo info = remoteDataSubscriber.parseRemoteDataInfo(schedule);
+            if (info != null) {
+                remoteDataInfoMap.put(schedule.getId(), info);
+            }
+
+
             return RetryingExecutor.finishedResult();
         };
 
@@ -765,7 +773,9 @@ private int onCheckExecutionReadiness(@NonNull Schedule<? extends ScheduleData>
             return AutomationDriver.READY_RESULT_NOT_READY;
         }
 
-        if (!this.remoteDataSubscriber.isScheduleValid(schedule)) {
+        RemoteDataInfo info = remoteDataSubscriber.parseRemoteDataInfo(schedule);
+
+        if ((info != null && !info.equals(remoteDataInfoMap.get(schedule.getId()))) || !this.remoteDataSubscriber.isScheduleValid(schedule)) {
             ScheduleDelegate<?> delegate = scheduleDelegateMap.remove(schedule.getId());
             if (delegate != null) {
                 delegate.onExecutionInvalidated(schedule);
@@ -797,6 +807,7 @@ private void onExecuteTriggeredSchedule(@NonNull Schedule<? extends
             ScheduleData> schedule, @NonNull AutomationDriver.ExecutionCallback callback) {
         UALog.v("onExecuteTriggeredSchedule schedule: %s", schedule.getId());
         frequencyCheckerMap.remove(schedule.getId());
+        remoteDataInfoMap.remove(schedule.getId());
 
         ScheduleDelegate<?> delegate = scheduleDelegateMap.remove(schedule.getId());
         if (delegate != null) {

urbanairship-automation/src/test/java/com/urbanairship/automation/InAppAutomationTest.java

@@ -1185,6 +1185,59 @@ public void testOnExecuteControlEventIsSentForHoldoutGroup() {
         verify(mockMessageScheduleDelegate).onExecute(schedule, executeCallback);
     }
 
+    @Test
+    public void testRemoteDataInfoChangesBeforeExecute() {
+        InAppMessage message = InAppMessage.newBuilder()
+                                           .setDisplayContent(new CustomDisplayContent(JsonValue.NULL))
+                                           .build();
+
+        Schedule<InAppMessage> schedule = Schedule.newBuilder(message)
+                                                  .setId("test-id")
+                                                  .addTrigger(Triggers.newAppInitTriggerBuilder().setGoal(1).build())
+                                                  .build();
+
+        when(mockObserver.requiresRefresh(eq(schedule))).thenReturn(false);
+        when(mockObserver.bestEffortRefresh(eq(schedule))).thenReturn(true);
+        when(mockObserver.isScheduleValid(eq(schedule))).thenReturn(true);
+
+        MessageInfo expectedInfo = new MessageInfo("transactional", null);
+        JsonMap experimentMetadata = JsonMap
+                .newBuilder()
+                .put("report", "metadata")
+                .build();
+
+        List<JsonMap> metadata = Collections.singletonList(experimentMetadata);
+        ExperimentResult results = new ExperimentResult("cid", "coid", "eid", true, metadata);
+
+        PendingResult<ExperimentResult> pendingResult = new PendingResult<>();
+        pendingResult.setResult(results);
+        doReturn(pendingResult)
+                .when(mockExperimentManager)
+                .evaluateGlobalHoldoutsPendingResult(eq(expectedInfo), nullable(String.class));
+
+        doReturn(new RemoteDataInfo("url", null, RemoteDataSource.APP, null))
+                .when(mockObserver).parseRemoteDataInfo(eq(schedule));
+
+        AutomationDriver.PrepareScheduleCallback callback = mock(AutomationDriver.PrepareScheduleCallback.class);
+        driver.onPrepareSchedule(schedule, null, callback);
+
+        ArgumentCaptor<AutomationDriver.PrepareScheduleCallback> argumentCaptor = ArgumentCaptor.forClass(AutomationDriver.PrepareScheduleCallback.class);
+        verify(mockMessageScheduleDelegate, times(1)).onPrepareSchedule(eq(schedule), eq(schedule.getData()), eq(results), argumentCaptor.capture());
+        argumentCaptor.getValue().onFinish(AutomationDriver.PREPARE_RESULT_CONTINUE);
+        verify(callback).onFinish(AutomationDriver.PREPARE_RESULT_CONTINUE);
+
+        // Verify its not invalid before the remote-data info changes
+        assertEquals(AutomationDriver.READY_RESULT_NOT_READY, driver.onCheckExecutionReadiness(schedule));
+
+        // Change remote data
+        doReturn(new RemoteDataInfo("some other url", null, RemoteDataSource.APP, null))
+                .when(mockObserver).parseRemoteDataInfo(eq(schedule));
+
+        // Should be invalid since the remote-data info is not longer the same as when it was prepared
+        assertEquals(AutomationDriver.READY_RESULT_INVALIDATE, driver.onCheckExecutionReadiness(schedule));
+
+    }
+
     /**
      * Helper method to run all the looper tasks.
      */