From 776c4efa08ccb07f9af92a219797bdc5a6e5bf19 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 21 Feb 2025 18:43:32 -0300 Subject: [PATCH 1/3] RBS storage --- .../LocalhostMySegmentsStorageContainer.java | 5 + .../storage/RolloutDefinitionsCache.java | 2 + .../storage/mysegments/MySegmentsStorage.java | 1 - .../MySegmentsStorageContainerImpl.java | 5 + .../PersistentRuleBasedSegmentStorage.java | 14 ++ .../storage/rbs/RuleBasedSegmentSnapshot.java | 25 +++ .../storage/rbs/RuleBasedSegmentStorage.java | 21 ++ .../rbs/RuleBasedSegmentStorageImpl.java | 96 ++++++++++ .../client/storage/splits/SplitsStorage.java | 1 - .../rbs/RuleBasedSegmentStorageImplTest.java | 181 ++++++++++++++++++ 10 files changed, 349 insertions(+), 2 deletions(-) create mode 100644 src/main/java/io/split/android/client/storage/rbs/PersistentRuleBasedSegmentStorage.java create mode 100644 src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentSnapshot.java create mode 100644 src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorage.java create mode 100644 src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImpl.java create mode 100644 src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java diff --git a/src/main/java/io/split/android/client/localhost/LocalhostMySegmentsStorageContainer.java b/src/main/java/io/split/android/client/localhost/LocalhostMySegmentsStorageContainer.java index 64304916b..a10e387d7 100644 --- a/src/main/java/io/split/android/client/localhost/LocalhostMySegmentsStorageContainer.java +++ b/src/main/java/io/split/android/client/localhost/LocalhostMySegmentsStorageContainer.java @@ -18,6 +18,11 @@ public long getUniqueAmount() { return mEmptyMySegmentsStorage.getAll().size(); } + @Override + public void loadLocal() { + // no-op + } + @Override public void clear() { // No-op diff --git a/src/main/java/io/split/android/client/storage/RolloutDefinitionsCache.java b/src/main/java/io/split/android/client/storage/RolloutDefinitionsCache.java index 912fafa97..bc7dfcb5c 100644 --- a/src/main/java/io/split/android/client/storage/RolloutDefinitionsCache.java +++ b/src/main/java/io/split/android/client/storage/RolloutDefinitionsCache.java @@ -2,5 +2,7 @@ public interface RolloutDefinitionsCache { + void loadLocal(); + void clear(); } diff --git a/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorage.java b/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorage.java index 2675fc5f1..b86e12288 100644 --- a/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorage.java +++ b/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorage.java @@ -6,7 +6,6 @@ import io.split.android.client.storage.RolloutDefinitionsCache; public interface MySegmentsStorage extends RolloutDefinitionsCache { - void loadLocal(); Set getAll(); diff --git a/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageContainerImpl.java b/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageContainerImpl.java index 0a8f51f45..009d3d6f6 100644 --- a/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageContainerImpl.java +++ b/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageContainerImpl.java @@ -42,6 +42,11 @@ public long getUniqueAmount() { return segments.size(); } + @Override + public void loadLocal() { + // no-op + } + @Override public void clear() { synchronized (lock) { diff --git a/src/main/java/io/split/android/client/storage/rbs/PersistentRuleBasedSegmentStorage.java b/src/main/java/io/split/android/client/storage/rbs/PersistentRuleBasedSegmentStorage.java new file mode 100644 index 000000000..f27a9d00d --- /dev/null +++ b/src/main/java/io/split/android/client/storage/rbs/PersistentRuleBasedSegmentStorage.java @@ -0,0 +1,14 @@ +package io.split.android.client.storage.rbs; + +import java.util.Set; + +import io.split.android.client.dtos.RuleBasedSegment; + +public interface PersistentRuleBasedSegmentStorage { + + RuleBasedSegmentSnapshot getSnapshot(); + + boolean update(Set toAdd, Set toRemove, long changeNumber); + + void clear(); +} diff --git a/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentSnapshot.java b/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentSnapshot.java new file mode 100644 index 000000000..fdb618bcd --- /dev/null +++ b/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentSnapshot.java @@ -0,0 +1,25 @@ +package io.split.android.client.storage.rbs; + +import java.util.Set; + +import io.split.android.client.dtos.RuleBasedSegment; + +public class RuleBasedSegmentSnapshot { + + private final Set mSegments; + + private final long mChangeNumber; + + public RuleBasedSegmentSnapshot(Set segments, long changeNumber) { + mSegments = segments; + mChangeNumber = changeNumber; + } + + public Set getSegments() { + return mSegments; + } + + public long getChangeNumber() { + return mChangeNumber; + } +} diff --git a/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorage.java b/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorage.java new file mode 100644 index 000000000..e71b74eec --- /dev/null +++ b/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorage.java @@ -0,0 +1,21 @@ +package io.split.android.client.storage.rbs; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import java.util.Set; + +import io.split.android.client.dtos.RuleBasedSegment; +import io.split.android.client.storage.RolloutDefinitionsCache; + +public interface RuleBasedSegmentStorage extends RolloutDefinitionsCache { + + @Nullable + RuleBasedSegment get(String segmentName); + + boolean update(@NonNull Set toAdd, @NonNull Set toRemove, long changeNumber); + + long getChangeNumber(); + + boolean contains(@NonNull Set segmentNames); +} diff --git a/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImpl.java b/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImpl.java new file mode 100644 index 000000000..013092227 --- /dev/null +++ b/src/main/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImpl.java @@ -0,0 +1,96 @@ +package io.split.android.client.storage.rbs; + +import static io.split.android.client.utils.Utils.checkNotNull; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.annotation.WorkerThread; + +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import io.split.android.client.dtos.RuleBasedSegment; + +public class RuleBasedSegmentStorageImpl implements RuleBasedSegmentStorage { + + private final ConcurrentHashMap mInMemorySegments; + private final PersistentRuleBasedSegmentStorage mPersistentStorage; + private volatile long mChangeNumber; + + public RuleBasedSegmentStorageImpl(@NonNull PersistentRuleBasedSegmentStorage persistentStorage) { + mInMemorySegments = new ConcurrentHashMap<>(); + mPersistentStorage = checkNotNull(persistentStorage); + mChangeNumber = -1; + } + + @Nullable + @Override + public RuleBasedSegment get(String segmentName) { + return mInMemorySegments.get(segmentName); + } + + @Override + public synchronized boolean update(@NonNull Set toAdd, @NonNull Set toRemove, long changeNumber) { + boolean appliedUpdates = false; + + if (toAdd != null) { + if (!toAdd.isEmpty()) { + for (RuleBasedSegment segment : toAdd) { + mInMemorySegments.put(segment.getName(), segment); + } + + appliedUpdates = true; + } + } + + if (toRemove != null) { + if (!toRemove.isEmpty()) { + for (RuleBasedSegment segment : toRemove) { + mInMemorySegments.remove(segment.getName()); + } + } + } + + mChangeNumber = changeNumber; + + return appliedUpdates; + } + + @Override + public long getChangeNumber() { + return mChangeNumber; + } + + @Override + public boolean contains(@NonNull Set segmentNames) { + if (segmentNames == null) { + return false; + } + + for (String name : segmentNames) { + if (!mInMemorySegments.containsKey(name)) { + return false; + } + } + return true; + } + + @WorkerThread + @Override + public void loadLocal() { + RuleBasedSegmentSnapshot snapshot = mPersistentStorage.getSnapshot(); + Set segments = snapshot.getSegments(); + mChangeNumber = snapshot.getChangeNumber(); + for (RuleBasedSegment segment : segments) { + mInMemorySegments.put(segment.getName(), segment); + } + } + + @WorkerThread + @Override + public void clear() { + mInMemorySegments.clear(); + mChangeNumber = -1; + mPersistentStorage.clear(); + } +} 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 81ff4e5a5..b74910f6b 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 @@ -12,7 +12,6 @@ import io.split.android.client.storage.RolloutDefinitionsCache; public interface SplitsStorage extends RolloutDefinitionsCache { - void loadLocal(); Split get(@NonNull String name); diff --git a/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java b/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java new file mode 100644 index 000000000..f128f0588 --- /dev/null +++ b/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java @@ -0,0 +1,181 @@ +package io.split.android.client.storage.rbs; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import io.split.android.client.dtos.Excluded; +import io.split.android.client.dtos.RuleBasedSegment; +import io.split.android.client.dtos.Status; + +public class RuleBasedSegmentStorageImplTest { + + private RuleBasedSegmentStorageImpl storage; + private PersistentRuleBasedSegmentStorage mPersistentStorage; + + @Before + public void setUp() { + mPersistentStorage = mock(PersistentRuleBasedSegmentStorage.class); + storage = new RuleBasedSegmentStorageImpl(mPersistentStorage); + } + + @Test + public void get() { + RuleBasedSegment segment = createRuleBasedSegment("segment1"); + storage.update(Set.of(segment), null, 1); + assertNotNull(storage.get("segment1")); + } + + @Test + public void sequentialUpdate() { + RuleBasedSegment segment1 = createRuleBasedSegment("segment1"); + RuleBasedSegment segment2 = createRuleBasedSegment("segment2"); + + Set toAdd = new HashSet<>(); + toAdd.add(segment1); + toAdd.add(segment2); + + storage.update(toAdd, null, 2); + assertNotNull(storage.get("segment1")); + assertNotNull(storage.get("segment2")); + assertEquals(2, storage.getChangeNumber()); + + Set toRemove = new HashSet<>(); + toRemove.add(segment1); + storage.update(null, toRemove, 3); + assertNull(storage.get("segment1")); + assertNotNull(storage.get("segment2")); + assertEquals(3, storage.getChangeNumber()); + } + + @Test + public void defaultChangeNumberIsMinusOne() { + assertEquals(-1, storage.getChangeNumber()); + } + + @Test + public void updateChangeNumber() { + storage.update(null, null, 5); + assertEquals(5, storage.getChangeNumber()); + } + + @Test + public void contains() { + RuleBasedSegment segment = createRuleBasedSegment("segment1"); + storage.update(Set.of(segment), null, 1); + + Set segmentNames = Collections.singleton("segment1"); + Set segmentNames2 = new HashSet<>(); + segmentNames.add("segment1"); + segmentNames.add("segment2"); + + assertTrue(storage.contains(segmentNames)); + assertFalse(storage.contains(segmentNames2)); + } + + @Test + public void clearRemovesAllSegments() { + RuleBasedSegment segment = createRuleBasedSegment("segment1"); + storage.update(Set.of(segment), null, 1); + storage.clear(); + assertNull(storage.get("segment1")); + } + + @Test + public void clearResetsChangeNumber() { + RuleBasedSegment segment = createRuleBasedSegment("segment1"); + storage.update(Set.of(segment), null, 10); + long preClearChangeNumber = storage.getChangeNumber(); + + storage.clear(); + + assertEquals(10, preClearChangeNumber); + assertEquals(-1, storage.getChangeNumber()); + } + + @Test + public void updateWithNullAddAndRemoveIsSafe() { + long initialChangeNumber = storage.getChangeNumber(); + storage.update(null, null, 10); + assertEquals(10, storage.getChangeNumber()); + assertNotEquals(initialChangeNumber, storage.getChangeNumber()); + } + + @Test + public void segmentRemoval() { + RuleBasedSegment segment1 = createRuleBasedSegment("segment1"); + RuleBasedSegment segment2 = createRuleBasedSegment("segment2"); + + Set toAdd = new HashSet<>(); + toAdd.add(segment1); + toAdd.add(segment2); + storage.update(toAdd, null, 1); + + assertNotNull(storage.get("segment1")); + assertNotNull(storage.get("segment2")); + + Set toRemove = new HashSet<>(); + toRemove.add(createRuleBasedSegment("segment1")); + storage.update(null, toRemove, 2); + + assertNull(storage.get("segment1")); + assertNotNull(storage.get("segment2")); + } + + @Test + public void segmentRemovalOfSameSegment() { + RuleBasedSegment segment1 = createRuleBasedSegment("segment1"); + Set segments = Collections.singleton(segment1); + + storage.update(segments, segments, 1); + assertNull(storage.get("segment1")); + assertEquals(1, storage.getChangeNumber()); + } + + @Test + public void updateReturnsTrueWhenThereWereAddedSegments() { + RuleBasedSegment segment1 = createRuleBasedSegment("segment1"); + RuleBasedSegment segment2 = createRuleBasedSegment("segment2"); + Set toAdd = new HashSet<>(); + toAdd.add(segment1); + toAdd.add(segment2); + + assertTrue(storage.update(toAdd, null, 1)); + } + + @Test + public void loadLocalGetsSnapshotFromPersistentStorage() { + storage.loadLocal(); + + verify(mPersistentStorage).getSnapshot(); + } + + @Test + public void clearCallsClearOnPersistentStorage() { + storage.clear(); + + verify(mPersistentStorage).clear(); + } + + private static RuleBasedSegment createRuleBasedSegment(String name) { + return new RuleBasedSegment(name, + "user", + 1, + Status.ACTIVE, + new ArrayList<>(), + new Excluded()); + } +} From b04a9d5d9920c4ca5ca3035a709d645880e116d9 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 24 Feb 2025 10:34:19 -0300 Subject: [PATCH 2/3] Update tests --- .../rbs/RuleBasedSegmentStorageImplTest.java | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java b/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java index f128f0588..b97d3eca8 100644 --- a/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java +++ b/src/test/java/io/split/android/client/storage/rbs/RuleBasedSegmentStorageImplTest.java @@ -8,6 +8,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.junit.Before; import org.junit.Test; @@ -75,14 +76,14 @@ public void updateChangeNumber() { @Test public void contains() { RuleBasedSegment segment = createRuleBasedSegment("segment1"); - storage.update(Set.of(segment), null, 1); + Set segmentNames = Set.of(segment); + storage.update(segmentNames, null, 1); - Set segmentNames = Collections.singleton("segment1"); Set segmentNames2 = new HashSet<>(); - segmentNames.add("segment1"); - segmentNames.add("segment2"); + segmentNames2.add("segment1"); + segmentNames2.add("segment2"); - assertTrue(storage.contains(segmentNames)); + assertTrue(storage.contains(Set.of("segment1"))); assertFalse(storage.contains(segmentNames2)); } @@ -158,11 +159,33 @@ public void updateReturnsTrueWhenThereWereAddedSegments() { @Test public void loadLocalGetsSnapshotFromPersistentStorage() { + when(mPersistentStorage.getSnapshot()).thenReturn(new RuleBasedSegmentSnapshot(Set.of(), 1)); + storage.loadLocal(); verify(mPersistentStorage).getSnapshot(); } + @Test + public void loadLocalPopulatesValues() { + RuleBasedSegmentSnapshot snapshot = new RuleBasedSegmentSnapshot(Set.of(createRuleBasedSegment("segment1")), + 1); + when(mPersistentStorage.getSnapshot()).thenReturn(snapshot); + + long initialCn = storage.getChangeNumber(); + RuleBasedSegment initialSegment1 = storage.get("segment1"); + + storage.loadLocal(); + + long finalCn = storage.getChangeNumber(); + RuleBasedSegment finalSegment1 = storage.get("segment1"); + + assertEquals(-1, initialCn); + assertEquals(1, finalCn); + assertNull(initialSegment1); + assertNotNull(finalSegment1); + } + @Test public void clearCallsClearOnPersistentStorage() { storage.clear(); From 7e7a33a7a57ddf14b34391e9cb9b121002b15e0c Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 25 Feb 2025 13:03:02 -0300 Subject: [PATCH 3/3] Disable unstable GH run --- .github/workflows/instrumented.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/instrumented.yml b/.github/workflows/instrumented.yml index ad07f29d9..0df019254 100644 --- a/.github/workflows/instrumented.yml +++ b/.github/workflows/instrumented.yml @@ -5,7 +5,6 @@ on: branches: - 'development' - 'master' - - '*_baseline' jobs: test: