From dfe5321a578342d911de29541c1fdd3e9bd69396 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 30 Oct 2023 21:57:22 -0300 Subject: [PATCH 1/4] Version 3.4.0-rc1 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 78ec8cda1..e6455b15a 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ apply plugin: 'signing' apply plugin: 'kotlin-android' ext { - splitVersion = '3.4.0-alpha-1' + splitVersion = '3.4.0-rc1' } android { From 4089e3f3997692350ab52b50288c3d1df1c3c818 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 31 Oct 2023 16:03:25 -0300 Subject: [PATCH 2/4] More performant --- .../client/localhost/LocalhostSplitsStorage.java | 3 ++- .../service/splits/SplitInPlaceUpdateTask.java | 6 ++++-- .../client/storage/splits/SplitsStorage.java | 3 ++- .../client/storage/splits/SplitsStorageImpl.java | 14 ++++++++++++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java index 989d9a16c..de3f5e085 100644 --- a/src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java +++ b/src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java @@ -89,7 +89,8 @@ public Map getAll() { } @Override - public void update(ProcessedSplitChange splitChange) { + public boolean update(ProcessedSplitChange splitChange) { + return false; } @Override diff --git a/src/main/java/io/split/android/client/service/splits/SplitInPlaceUpdateTask.java b/src/main/java/io/split/android/client/service/splits/SplitInPlaceUpdateTask.java index c34dd6539..bf61a1ca5 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitInPlaceUpdateTask.java +++ b/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"); diff --git a/src/main/java/io/split/android/client/storage/splits/SplitsStorage.java b/src/main/java/io/split/android/client/storage/splits/SplitsStorage.java index 62af70e7b..1e6fa533e 100644 --- a/src/main/java/io/split/android/client/storage/splits/SplitsStorage.java +++ b/src/main/java/io/split/android/client/storage/splits/SplitsStorage.java @@ -19,7 +19,8 @@ public interface SplitsStorage { Map getAll(); - void update(ProcessedSplitChange splitChange); + // Returns true if at least one split was updated + boolean update(ProcessedSplitChange splitChange); void updateWithoutChecks(Split split); diff --git a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java index b143c209c..d79e4a59e 100644 --- a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java +++ b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java @@ -77,14 +77,20 @@ public Map getAll() { @Override @WorkerThread - public void update(ProcessedSplitChange splitChange) { + public boolean update(ProcessedSplitChange splitChange) { if (splitChange == null) { - return; + return false; } + // return true if there is at least one split change; otherwise false + boolean result = false; + List activeSplits = splitChange.getActiveSplits(); List archivedSplits = splitChange.getArchivedSplits(); if (activeSplits != null) { + if (!activeSplits.isEmpty()) { + result = 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 + result = 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 result; } @Override From 1ff6a3ab2d9252d3aa9e873db96a9525a12a5451 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 31 Oct 2023 16:54:43 -0300 Subject: [PATCH 3/4] More tests --- .../sets/FlagSetsStreamingTest.java | 2 +- .../java/tests/storage/SplitsStorageTest.java | 83 +++++++++++++++---- .../storage/splits/SplitsStorageImpl.java | 2 +- .../service/SplitInPlaceUpdateTaskTest.java | 31 ++++++- 4 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/androidTest/java/tests/integration/sets/FlagSetsStreamingTest.java b/src/androidTest/java/tests/integration/sets/FlagSetsStreamingTest.java index d61a46e06..20d3afa06 100644 --- a/src/androidTest/java/tests/integration/sets/FlagSetsStreamingTest.java +++ b/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 diff --git a/src/androidTest/java/tests/storage/SplitsStorageTest.java b/src/androidTest/java/tests/storage/SplitsStorageTest.java index 1669c1660..e818c5ee3 100644 --- a/src/androidTest/java/tests/storage/SplitsStorageTest.java +++ b/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 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 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 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 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()); } diff --git a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java index d79e4a59e..d18012b27 100644 --- a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java +++ b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java @@ -82,7 +82,7 @@ public boolean update(ProcessedSplitChange splitChange) { return false; } - // return true if there is at least one split change; otherwise false + // this will be true if there is at least one split change; otherwise false boolean result = false; List activeSplits = splitChange.getActiveSplits(); diff --git a/src/test/java/io/split/android/client/service/SplitInPlaceUpdateTaskTest.java b/src/test/java/io/split/android/client/service/SplitInPlaceUpdateTaskTest.java index 1ba725bbb..90ecd75b5 100644 --- a/src/test/java/io/split/android/client/service/SplitInPlaceUpdateTaskTest.java +++ b/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); + } } From 5756fa2e46775631f2753be03dbe96ee9253f5f8 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 31 Oct 2023 16:58:26 -0300 Subject: [PATCH 4/4] Changed version --- build.gradle | 2 +- .../client/storage/splits/SplitsStorageImpl.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index e6455b15a..a5245952a 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ apply plugin: 'signing' apply plugin: 'kotlin-android' ext { - splitVersion = '3.4.0-rc1' + splitVersion = '3.4.0-alpha-2' } android { diff --git a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java index d18012b27..2d8823356 100644 --- a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java +++ b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java @@ -82,14 +82,14 @@ public boolean update(ProcessedSplitChange splitChange) { return false; } - // this will be true if there is at least one split change; otherwise false - boolean result = false; + boolean appliedUpdates = false; List activeSplits = splitChange.getActiveSplits(); List archivedSplits = splitChange.getArchivedSplits(); if (activeSplits != null) { if (!activeSplits.isEmpty()) { - result = true; + // There is at least one added or modified feature flag + appliedUpdates = true; } for (Split split : activeSplits) { Split loadedSplit = mInMemorySplits.get(split.name); @@ -106,7 +106,7 @@ public boolean update(ProcessedSplitChange splitChange) { for (Split split : archivedSplits) { if (mInMemorySplits.remove(split.name) != null) { // The flag was in memory, so it will be updated - result = true; + appliedUpdates = true; decreaseTrafficTypeCount(split.trafficTypeName); deleteFromFlagSetsIfNecessary(split); } @@ -117,7 +117,7 @@ public boolean update(ProcessedSplitChange splitChange) { mUpdateTimestamp = splitChange.getUpdateTimestamp(); mPersistentStorage.update(splitChange); - return result; + return appliedUpdates; } @Override