Update enhancements

build.gradle

@@ -16,7 +16,7 @@ apply plugin: 'signing'
 apply plugin: 'kotlin-android'
 
 ext {
-    splitVersion = '3.4.0-alpha-1'
+    splitVersion = '3.4.0-alpha-2'
 }
 
 android {

src/androidTest/java/tests/integration/sets/FlagSetsStreamingTest.java

@@ -134,7 +134,7 @@ public void sdkWithSetsConfiguredDeletedDueToNonMatchingSets() throws IOExceptio
         assertTrue(firstChange);
         assertTrue(secondChange);
         assertTrue(thirdChange);
-        assertTrue(fourthChange);
+        assertFalse(fourthChange); // SDK_UPDATE should not be triggered since there were no changes to storage
     }
 
     @Test

src/androidTest/java/tests/storage/SplitsStorageTest.java

@@ -1,5 +1,8 @@
 package tests.storage;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import android.content.Context;
 
 import androidx.test.platform.app.InstrumentationRegistry;
@@ -282,8 +285,8 @@ public void updatedSplitTrafficType() {
         mSplitsStorage.update(new ProcessedSplitChange(Arrays.asList(s2), empty, 1L, 0L));
         mSplitsStorage.update(new ProcessedSplitChange(empty, Arrays.asList(s2ar), 1L, 0L));
 
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("tt"));
-        Assert.assertFalse(mSplitsStorage.isValidTrafficType("mytt"));
+        assertTrue(mSplitsStorage.isValidTrafficType("tt"));
+        assertFalse(mSplitsStorage.isValidTrafficType("mytt"));
     }
 
     @Test
@@ -300,8 +303,8 @@ public void changedTrafficTypeForSplit() {
         mSplitsStorage.update(new ProcessedSplitChange(Arrays.asList(s1t1), empty, 1L, 0L));
         mSplitsStorage.update(new ProcessedSplitChange(Arrays.asList(s1t2), empty, 1L, 0L));
 
-        Assert.assertFalse(mSplitsStorage.isValidTrafficType("tt"));
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("mytt"));
+        assertFalse(mSplitsStorage.isValidTrafficType("tt"));
+        assertTrue(mSplitsStorage.isValidTrafficType("mytt"));
     }
 
     @Test
@@ -320,8 +323,8 @@ public void existingChangedTrafficTypeForSplit() {
         mSplitsStorage.update(new ProcessedSplitChange(Arrays.asList(s1t1), empty, 1L, 0L));
         mSplitsStorage.update(new ProcessedSplitChange(Arrays.asList(s1t2), empty, 1L, 0L));
 
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("tt"));
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("mytt"));
+        assertTrue(mSplitsStorage.isValidTrafficType("tt"));
+        assertTrue(mSplitsStorage.isValidTrafficType("mytt"));
     }
 
     @Test
@@ -331,9 +334,9 @@ public void trafficTypesAreLoadedInMemoryWhenLoadingLocalSplits() {
 
         mSplitsStorage.loadLocal();
 
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("test_type"));
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("test_type_2"));
-        Assert.assertFalse(mSplitsStorage.isValidTrafficType("invalid_type"));
+        assertTrue(mSplitsStorage.isValidTrafficType("test_type"));
+        assertTrue(mSplitsStorage.isValidTrafficType("test_type_2"));
+        assertFalse(mSplitsStorage.isValidTrafficType("invalid_type"));
     }
 
     @Test
@@ -346,9 +349,9 @@ public void loadedFromStorageTrafficTypesAreCorrectlyUpdated() {
         Split updatedSplit = newSplit("split_test", Status.ACTIVE, "new_type");
         mSplitsStorage.update(new ProcessedSplitChange(Collections.singletonList(updatedSplit), Collections.emptyList(), 1L, 0L));
 
-        Assert.assertFalse(mSplitsStorage.isValidTrafficType("test_type"));
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("new_type"));
-        Assert.assertTrue(mSplitsStorage.isValidTrafficType("test_type_2"));
+        assertFalse(mSplitsStorage.isValidTrafficType("test_type"));
+        assertTrue(mSplitsStorage.isValidTrafficType("new_type"));
+        assertTrue(mSplitsStorage.isValidTrafficType("test_type_2"));
     }
 
     @Test
@@ -382,7 +385,7 @@ public void flagSetsAreRemovedWhenUpdating() {
                 Collections.singletonList(newSplit("split_test", Status.ACTIVE, "test_type")), Collections.emptyList(),
                 1L, 0L));
 
-        Assert.assertFalse(initialSet1.isEmpty());
+        assertFalse(initialSet1.isEmpty());
         Assert.assertEquals(Collections.emptySet(), mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1")));
         Assert.assertEquals(initialSet2, mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_2")));
     }
@@ -398,11 +401,63 @@ public void updateWithoutChecksRemovesFromFlagSet() {
 
         mSplitsStorage.updateWithoutChecks(newSplit("split_test", Status.ACTIVE, "test_type"));
 
-        Assert.assertFalse(initialSet1.isEmpty());
+        assertFalse(initialSet1.isEmpty());
         Assert.assertEquals(Collections.emptySet(), mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1")));
         Assert.assertEquals(initialSet2, mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_2")));
     }
 
+    @Test
+    public void updateReturnsTrueWhenFlagsHaveBeenRemovedFromStorage() {
+        mRoomDb.clearAllTables();
+        mRoomDb.splitDao().insert(Arrays.asList(newSplitEntity("split_test", "test_type", Collections.singleton("set_1")), newSplitEntity("split_test_2", "test_type_2", Collections.singleton("set_2"))));
+        mSplitsStorage.loadLocal();
+
+        ArrayList<Split> archivedSplits = new ArrayList<>();
+        archivedSplits.add(newSplit("split_test", Status.ARCHIVED, "test_type"));
+        boolean update = mSplitsStorage.update(new ProcessedSplitChange(new ArrayList<>(), archivedSplits, 1L, 0L));
+
+        assertTrue(update);
+    }
+
+    @Test
+    public void updateReturnsTrueWhenFlagsWereAddedToStorage() {
+        mRoomDb.clearAllTables();
+        mRoomDb.splitDao().insert(Arrays.asList(newSplitEntity("split_test", "test_type", Collections.singleton("set_1")), newSplitEntity("split_test_2", "test_type_2", Collections.singleton("set_2"))));
+        mSplitsStorage.loadLocal();
+
+        ArrayList<Split> activeSplits = new ArrayList<>();
+        activeSplits.add(newSplit("split_test_3", Status.ACTIVE, "test_type_2", Collections.singleton("set_2")));
+        boolean update = mSplitsStorage.update(new ProcessedSplitChange(activeSplits, new ArrayList<>(), 1L, 0L));
+
+        assertTrue(update);
+    }
+
+    @Test
+    public void updateReturnsTrueWhenFlagsWereUpdatedInStorage() {
+        mRoomDb.clearAllTables();
+        mRoomDb.splitDao().insert(Arrays.asList(newSplitEntity("split_test", "test_type", Collections.singleton("set_1")), newSplitEntity("split_test_2", "test_type_2", Collections.singleton("set_2"))));
+        mSplitsStorage.loadLocal();
+
+        ArrayList<Split> activeSplits = new ArrayList<>();
+        activeSplits.add(newSplit("split_test", Status.ACTIVE, "test_type", Collections.singleton("set_2")));
+        boolean update = mSplitsStorage.update(new ProcessedSplitChange(activeSplits, new ArrayList<>(), 1L, 0L));
+
+        assertTrue(update);
+    }
+
+    @Test
+    public void updateReturnsFalseWhenFlagsThatAreNotInStorageAreAttemptedToBeRemoved() {
+        mRoomDb.clearAllTables();
+        mRoomDb.splitDao().insert(Arrays.asList(newSplitEntity("split_test", "test_type", Collections.singleton("set_1")), newSplitEntity("split_test_2", "test_type_2", Collections.singleton("set_2"))));
+        mSplitsStorage.loadLocal();
+
+        ArrayList<Split> archivedSplits = new ArrayList<>();
+        archivedSplits.add(newSplit("split_test_3", Status.ACTIVE, "test_type_2", Collections.singleton("set_2")));
+        boolean update = mSplitsStorage.update(new ProcessedSplitChange(new ArrayList<>(), archivedSplits, 1L, 0L));
+
+        assertFalse(update);
+    }
+
     private Split newSplit(String name, Status status, String trafficType) {
         return newSplit(name, status, trafficType, Collections.emptySet());
     }

src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java

@@ -89,7 +89,8 @@ public Map<String, Split> getAll() {
     }
 
     @Override
-    public void update(ProcessedSplitChange splitChange) {
+    public boolean update(ProcessedSplitChange splitChange) {
+        return false;
     }
 
     @Override

src/main/java/io/split/android/client/service/splits/SplitInPlaceUpdateTask.java

@@ -44,9 +44,11 @@ public SplitInPlaceUpdateTask(@NonNull SplitsStorage splitsStorage,
     public SplitTaskExecutionInfo execute() {
         try {
             ProcessedSplitChange processedSplitChange = mSplitChangeProcessor.process(mSplit, mChangeNumber);
-            mSplitsStorage.update(processedSplitChange);
+            boolean triggerSdkUpdate = mSplitsStorage.update(processedSplitChange);
 
-            mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
+            if (triggerSdkUpdate) {
+                mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
+            }
             mTelemetryRuntimeProducer.recordUpdatesFromSSE(UpdatesFromSSEEnum.SPLITS);
 
             Logger.v("Updated feature flag");

src/main/java/io/split/android/client/storage/splits/SplitsStorage.java

@@ -19,7 +19,8 @@ public interface SplitsStorage {
 
     Map<String, Split> getAll();
 
-    void update(ProcessedSplitChange splitChange);
+    // Returns true if at least one split was updated
+    boolean update(ProcessedSplitChange splitChange);
 
     void updateWithoutChecks(Split split);
 

src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java

@@ -77,14 +77,20 @@ public Map<String, Split> getAll() {
 
     @Override
     @WorkerThread
-    public void update(ProcessedSplitChange splitChange) {
+    public boolean update(ProcessedSplitChange splitChange) {
         if (splitChange == null) {
-            return;
+            return false;
         }
 
+        boolean appliedUpdates = false;
+
         List<Split> activeSplits = splitChange.getActiveSplits();
         List<Split> archivedSplits = splitChange.getArchivedSplits();
         if (activeSplits != null) {
+            if (!activeSplits.isEmpty()) {
+                // There is at least one added or modified feature flag
+                appliedUpdates = true;
+            }
             for (Split split : activeSplits) {
                 Split loadedSplit = mInMemorySplits.get(split.name);
                 if (loadedSplit != null && loadedSplit.trafficTypeName != null) {
@@ -99,6 +105,8 @@ public void update(ProcessedSplitChange splitChange) {
         if (archivedSplits != null) {
             for (Split split : archivedSplits) {
                 if (mInMemorySplits.remove(split.name) != null) {
+                    // The flag was in memory, so it will be updated
+                    appliedUpdates = true;
                     decreaseTrafficTypeCount(split.trafficTypeName);
                     deleteFromFlagSetsIfNecessary(split);
                 }
@@ -108,6 +116,8 @@ public void update(ProcessedSplitChange splitChange) {
         mChangeNumber = splitChange.getChangeNumber();
         mUpdateTimestamp = splitChange.getUpdateTimestamp();
         mPersistentStorage.update(splitChange);
+
+        return appliedUpdates;
     }
 
     @Override

src/test/java/io/split/android/client/service/SplitInPlaceUpdateTaskTest.java

@@ -61,7 +61,6 @@ public void sseUpdateIsRecordedInTelemetryWhenOperationIsSuccessful() {
 
         verify(mSplitChangeProcessor).process(mSplit, 123L);
         verify(mSplitsStorage).update(processedSplitChange);
-        verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
         verify(mTelemetryRuntimeProducer).recordUpdatesFromSSE(UpdatesFromSSEEnum.SPLITS);
 
         assertEquals(result.getStatus(), SplitTaskExecutionStatus.SUCCESS);
@@ -98,4 +97,34 @@ public void exceptionDuringStorageUpdateReturnsErrorExecutionInfo() {
 
         assertEquals(result.getStatus(), SplitTaskExecutionStatus.ERROR);
     }
+
+    @Test
+    public void sdkUpdateIsNotTriggeredWhenStorageUpdateReturnsFalse() {
+        ProcessedSplitChange processedSplitChange = new ProcessedSplitChange(new ArrayList<>(), new ArrayList<>(), 0L, 0);
+
+        when(mSplitChangeProcessor.process(mSplit, 123L)).thenReturn(processedSplitChange);
+        when(mSplitsStorage.update(processedSplitChange)).thenReturn(false);
+
+        SplitTaskExecutionInfo result = mSplitInPlaceUpdateTask.execute();
+
+        verify(mSplitChangeProcessor).process(mSplit, 123L);
+        verify(mSplitsStorage).update(processedSplitChange);
+        verify(mEventsManager, never()).notifyInternalEvent(any());
+        verify(mTelemetryRuntimeProducer).recordUpdatesFromSSE(UpdatesFromSSEEnum.SPLITS);
+    }
+
+    @Test
+    public void sdkUpdateIsTriggeredWhenStorageUpdateReturnsTrue() {
+        ProcessedSplitChange processedSplitChange = new ProcessedSplitChange(new ArrayList<>(), new ArrayList<>(), 0L, 0);
+
+        when(mSplitChangeProcessor.process(mSplit, 123L)).thenReturn(processedSplitChange);
+        when(mSplitsStorage.update(processedSplitChange)).thenReturn(true);
+
+        SplitTaskExecutionInfo result = mSplitInPlaceUpdateTask.execute();
+
+        verify(mSplitChangeProcessor).process(mSplit, 123L);
+        verify(mSplitsStorage).update(processedSplitChange);
+        verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
+        verify(mTelemetryRuntimeProducer).recordUpdatesFromSSE(UpdatesFromSSEEnum.SPLITS);
+    }
 }