diff --git a/api/src/main/java/io/split/android/client/events/SdkUpdateMetadata.java b/api/src/main/java/io/split/android/client/events/SdkUpdateMetadata.java index 58d6741a5..83dea400a 100644 --- a/api/src/main/java/io/split/android/client/events/SdkUpdateMetadata.java +++ b/api/src/main/java/io/split/android/client/events/SdkUpdateMetadata.java @@ -19,12 +19,16 @@ public final class SdkUpdateMetadata { public enum Type { /** * Feature flags were updated. + *

+ * {@link #getNames()} returns the list of flag names that changed. */ FLAGS_UPDATE, /** - * Rule-based segments were updated. + * Segments were updated (rule-based segments, memberships, or large segments). *

+ * Note: {@link #getNames()} always returns an empty list for this type. + * Segment names are not included in the metadata. */ SEGMENTS_UPDATE } @@ -59,10 +63,10 @@ public Type getType() { /** * Returns the list of entity names that changed in this update. *

- * For {@link Type#FLAGS_UPDATE}, this contains flag names. - * For {@link Type#SEGMENTS_UPDATE}, this contains rule-based segment names. + * For {@link Type#FLAGS_UPDATE}, this contains flag names that were updated. + * For {@link Type#SEGMENTS_UPDATE}, this is always an empty list (segment names are not included). * - * @return the list of updated entity names, never null (empty list if none) + * @return the list of updated entity names, never null (empty list for SEGMENTS_UPDATE or if none) */ @NonNull public List getNames() { diff --git a/build.gradle b/build.gradle index bd6d05d7f..0209da0a1 100644 --- a/build.gradle +++ b/build.gradle @@ -19,7 +19,7 @@ apply plugin: 'com.vanniktech.maven.publish' apply from: "$rootDir/gradle/jacoco-root.gradle" ext { - splitVersion = '5.5.0-rc5' + splitVersion = '5.5.0-rc6' jacocoVersion = '0.8.8' } diff --git a/events-domain/src/main/java/io/split/android/client/events/metadata/EventMetadataHelpers.java b/events-domain/src/main/java/io/split/android/client/events/metadata/EventMetadataHelpers.java index 863799cb2..16018110b 100644 --- a/events-domain/src/main/java/io/split/android/client/events/metadata/EventMetadataHelpers.java +++ b/events-domain/src/main/java/io/split/android/client/events/metadata/EventMetadataHelpers.java @@ -3,6 +3,7 @@ import androidx.annotation.Nullable; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -31,15 +32,16 @@ public static EventMetadata createUpdatedFlagsMetadata(List updatedFlagN } /** - * Creates metadata for SDK_UPDATE events when rule-based segments are updated. + * Creates metadata for SDK_UPDATE events when segments are updated. + *

+ * SEGMENTS_UPDATE always has empty names - segment names are not included in the metadata. * - * @param updatedSegmentNames the list of rule-based segment names that were updated - * @return the event metadata with TYPE=SEGMENTS_UPDATE and NAMES containing the segment names + * @return the event metadata with TYPE=SEGMENTS_UPDATE and empty NAMES list */ - public static EventMetadata createUpdatedSegmentsMetadata(List updatedSegmentNames) { + public static EventMetadata createUpdatedSegmentsMetadata() { return new EventMetadataBuilder() .put(MetadataKeys.TYPE, MetadataKeys.TYPE_SEGMENTS_UPDATE) - .put(MetadataKeys.NAMES, new ArrayList<>(new HashSet<>(updatedSegmentNames))) + .put(MetadataKeys.NAMES, Collections.emptyList()) .build(); } diff --git a/events-domain/src/test/java/io/split/android/client/events/TypedTaskConversionTest.java b/events-domain/src/test/java/io/split/android/client/events/TypedTaskConversionTest.java index fbca03876..926a755c5 100644 --- a/events-domain/src/test/java/io/split/android/client/events/TypedTaskConversionTest.java +++ b/events-domain/src/test/java/io/split/android/client/events/TypedTaskConversionTest.java @@ -36,17 +36,15 @@ public void convertForSdkUpdateConvertsFlagsMetadataCorrectly() { @Test public void convertForSdkUpdateConvertsSegmentsMetadataCorrectly() { - List expectedSegments = Arrays.asList("segment1", "segment2"); - - EventMetadata eventMetadata = EventMetadataHelpers.createUpdatedSegmentsMetadata(expectedSegments); + // SEGMENTS_UPDATE always has empty names + EventMetadata eventMetadata = EventMetadataHelpers.createUpdatedSegmentsMetadata(); // Call conversion method SdkUpdateMetadata converted = TypedTaskConverter.convertForSdkUpdate(eventMetadata); assertNotNull(converted); assertEquals(SdkUpdateMetadata.Type.SEGMENTS_UPDATE, converted.getType()); - assertEquals(expectedSegments.size(), converted.getNames().size()); - assertTrue(converted.getNames().containsAll(expectedSegments)); + assertTrue("Names should be empty for SEGMENTS_UPDATE", converted.getNames().isEmpty()); } @Test diff --git a/events-domain/src/test/java/io/split/android/client/events/metadata/EventMetadataHelpersTest.java b/events-domain/src/test/java/io/split/android/client/events/metadata/EventMetadataHelpersTest.java index 0343c418f..9dca0abcf 100644 --- a/events-domain/src/test/java/io/split/android/client/events/metadata/EventMetadataHelpersTest.java +++ b/events-domain/src/test/java/io/split/android/client/events/metadata/EventMetadataHelpersTest.java @@ -33,19 +33,16 @@ public void createUpdatedFlagsMetadataContainsTypeAndNames() { // Tests for createUpdatedSegmentsMetadata @Test @SuppressWarnings("unchecked") - public void createUpdatedSegmentsMetadataContainsTypeAndNames() { - List segments = Arrays.asList("segment1", "segment2"); - EventMetadata metadata = EventMetadataHelpers.createUpdatedSegmentsMetadata(segments); + public void createUpdatedSegmentsMetadataContainsTypeAndEmptyNames() { + EventMetadata metadata = EventMetadataHelpers.createUpdatedSegmentsMetadata(); assertTrue(metadata.containsKey(MetadataKeys.TYPE)); assertEquals(MetadataKeys.TYPE_SEGMENTS_UPDATE, metadata.get(MetadataKeys.TYPE)); - // Check names + // Check names - should always be empty assertTrue(metadata.containsKey(MetadataKeys.NAMES)); List result = (List) metadata.get(MetadataKeys.NAMES); - assertEquals(2, result.size()); - assertTrue(result.contains("segment1")); - assertTrue(result.contains("segment2")); + assertTrue("Names should be empty for SEGMENTS_UPDATE", result.isEmpty()); } // Tests for createReadyMetadata diff --git a/main/src/androidTest/java/tests/integration/events/SdkEventsIntegrationTest.java b/main/src/androidTest/java/tests/integration/events/SdkEventsIntegrationTest.java index aaf25576c..ccb1eea8b 100644 --- a/main/src/androidTest/java/tests/integration/events/SdkEventsIntegrationTest.java +++ b/main/src/androidTest/java/tests/integration/events/SdkEventsIntegrationTest.java @@ -44,6 +44,7 @@ import io.split.android.client.events.SplitEventTask; import io.split.android.client.network.HttpMethod; import io.split.android.client.storage.db.GeneralInfoEntity; +import io.split.android.client.storage.db.MyLargeSegmentEntity; import io.split.android.client.storage.db.MySegmentEntity; import io.split.android.client.storage.db.SplitEntity; import io.split.android.client.storage.db.SplitRoomDatabase; @@ -391,7 +392,7 @@ public void sdkReadyFiresAfterSdkReadyFromCacheAndRequiresSyncCompletion() throw CountDownLatch readyLatch = new CountDownLatch(1); SplitClient client = factory.client(new Key("key_1")); - + // Register handlers immediately client.on(SplitEvent.SDK_READY_FROM_CACHE, new SplitEventTask() { @Override @@ -422,10 +423,10 @@ public void onPostExecution(SplitClient client) { // Then: sdkReady is emitted exactly once assertTrue("SDK_READY should fire after SDK_READY_FROM_CACHE and sync completion. " + - "Cache fired: " + cacheHandlerCount.get() + ", Ready fired: " + readyHandlerCount.get(), + "Cache fired: " + cacheHandlerCount.get() + ", Ready fired: " + readyHandlerCount.get(), readyFired); assertEquals("Ready handler should be invoked exactly once", 1, readyHandlerCount.get()); - + // Verify both events fired assertEquals("SDK_READY_FROM_CACHE should fire", 1, cacheHandlerCount.get()); assertEquals("SDK_READY should fire after SDK_READY_FROM_CACHE", 1, readyHandlerCount.get()); @@ -618,7 +619,7 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { // When: a second handler H2 is registered for sdkUpdate after one sdkUpdate has already fired CountDownLatch secondUpdateLatch = new CountDownLatch(2); secondUpdateLatchRef.set(secondUpdateLatch); - + fixture.client.addEventListener(new SdkEventListener() { @Override public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { @@ -633,7 +634,7 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { // Ensure handlers are registered and first update is fully processed before pushing second update Thread.sleep(500); - + // Send keep-alive to ensure SSE connection is still active if (fixture.streamingData != null) { TestingHelper.pushKeepAlive(fixture.streamingData); @@ -645,8 +646,8 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { // Then: both H1 and H2 are invoked for that second sdkUpdate boolean secondUpdateFired = secondUpdateLatch.await(15, TimeUnit.SECONDS); - assertTrue("Second SDK_UPDATE should fire. H1 count: " + handler1Count.get() + - ", H2 count: " + handler2Count.get() + + assertTrue("Second SDK_UPDATE should fire. H1 count: " + handler1Count.get() + + ", H2 count: " + handler2Count.get() + ", secondUpdateLatch count: " + secondUpdateLatch.getCount(), secondUpdateFired); // H1 should now have 2 total invocations (1 from first + 1 from second) @@ -916,10 +917,10 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { // are stored in a HashSet which doesn't guarantee iteration order. // The important thing is that all handlers were invoked and H3 was invoked // even though H2 threw an exception (error isolation). - assertTrue("All handlers should have been assigned order numbers", + assertTrue("All handlers should have been assigned order numbers", handler1Order.get() > 0 && handler2Order.get() > 0 && handler3Order.get() > 0); assertEquals("Order counter should be 3 (one for each handler)", 3, orderCounter.get()); - + // Verify error isolation: H3 was invoked even though H2 threw an exception // This is the key assertion - that errors don't prevent subsequent handlers from executing assertTrue("H3 should be invoked even if H2 throws (error isolation)", handler3Count.get() == 1); @@ -1070,7 +1071,7 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { * This test verifies that when a split update notification arrives via SSE, * the SDK_UPDATE event is emitted to all clients in the factory. *

- * Note: True client-scoped events like mySegmentsUpdated require specific streaming + * Note: True client-scoped events like mySegmentsUpdated require specific streaming * notifications targeted at individual user keys. This test demonstrates the difference * by showing that SDK-scoped split updates affect all clients equally. */ @@ -1164,9 +1165,9 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { * When a rule-based segment update notification arrives via SSE * Then sdkUpdate is emitted * And handler H receives metadata with getType() returning Type.SEGMENTS_UPDATE - * And handler H receives metadata with getNames() containing the updated RBS names + * And handler H receives metadata with getNames() returning an empty list *

- * Note: SEGMENTS_UPDATE is for rule-based segments (RBS) ONLY, not for memberships. + * Note: SEGMENTS_UPDATE always has empty names (segment names are not included). */ @Test public void sdkUpdateMetadataContainsTypeForSegmentsUpdate() throws Exception { @@ -1193,13 +1194,309 @@ public void onUpdate(SplitClient client, SdkUpdateMetadata metadata) { SdkUpdateMetadata.Type.SEGMENTS_UPDATE, receivedMetadata.get().getType()); assertNotNull("Names should not be null", receivedMetadata.get().getNames()); - assertFalse("Names should not be empty", receivedMetadata.get().getNames().isEmpty()); - assertTrue("Names should contain rbs_test", - receivedMetadata.get().getNames().contains("rbs_test")); + assertTrue("Names should be empty for SEGMENTS_UPDATE", receivedMetadata.get().getNames().isEmpty()); fixture.destroy(); } + /** + * Scenario: Only FLAGS_UPDATE fires when both flags and RBS change together + *

+ * Given sdkReady has already been emitted + * And a handler H is registered for sdkUpdate + * When a polling sync returns changes to both flags AND rule-based segments + * Then only ONE sdkUpdate is emitted + * And handler H receives metadata with getType() returning Type.FLAGS_UPDATE + * And SEGMENTS_UPDATE is NOT fired (RBS changes are subsumed by FLAGS_UPDATE) + */ + @Test + public void sdkUpdateFiresOnlyOnceWhenBothFlagsAndRbsChange() throws Exception { + // Track number of /splitChanges calls + AtomicInteger splitChangesHitCount = new AtomicInteger(0); + + final Dispatcher pollingDispatcher = new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest request) { + final String path = request.getPath(); + if (path.contains("/" + IntegrationHelper.ServicePath.MEMBERSHIPS)) { + return new MockResponse().setResponseCode(200).setBody(IntegrationHelper.dummyAllSegments()); + } else if (path.contains("/splitChanges")) { + int count = splitChangesHitCount.incrementAndGet(); + if (count <= 1) { + // Initial sync: empty + return new MockResponse().setResponseCode(200) + .setBody(IntegrationHelper.emptyTargetingRulesChanges(1000, 1000)); + } else { + // Polling sync: return BOTH flag and RBS changes + // s and t must be equal to signal end of sync loop + String responseWithBothChanges = "{\"ff\":{\"s\":2000,\"t\":2000,\"d\":[" + + "{\"trafficTypeName\":\"user\",\"name\":\"test_split\",\"status\":\"ACTIVE\"," + + "\"killed\":false,\"defaultTreatment\":\"off\",\"changeNumber\":2000," + + "\"conditions\":[{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\"," + + "\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"ALL_KEYS\",\"negate\":false}]}," + + "\"partitions\":[{\"treatment\":\"on\",\"size\":100}]}]}" + + "]},\"rbs\":{\"s\":2000,\"t\":2000,\"d\":[" + + "{\"name\":\"test_rbs\",\"status\":\"ACTIVE\",\"trafficTypeName\":\"user\"," + + "\"excluded\":{\"keys\":[],\"segments\":[]}," + + "\"conditions\":[{\"matcherGroup\":{\"combiner\":\"AND\"," + + "\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"ALL_KEYS\",\"negate\":false}]}}]}" + + "]}}"; + return new MockResponse().setResponseCode(200).setBody(responseWithBothChanges); + } + } else if (path.contains("/testImpressions/bulk")) { + return new MockResponse().setResponseCode(200); + } + return new MockResponse().setResponseCode(404); + } + }; + mWebServer.setDispatcher(pollingDispatcher); + + // Use polling mode with short refresh rate to trigger sync quickly + SplitClientConfig config = new TestableSplitConfigBuilder() + .serviceEndpoints(endpoints()) + .ready(30000) + .featuresRefreshRate(3) // Poll every 3 seconds + .segmentsRefreshRate(999999) + .impressionsRefreshRate(999999) + .streamingEnabled(false) + .trafficType("account") + .build(); + + SplitFactory factory = buildFactory(config); + SplitClient client = factory.client(); + + // Wait for SDK_READY + CountDownLatch readyLatch = new CountDownLatch(1); + client.on(SplitEvent.SDK_READY, new SplitEventTask() { + @Override + public void onPostExecution(SplitClient c) { + readyLatch.countDown(); + } + }); + assertTrue("SDK_READY should fire", readyLatch.await(10, TimeUnit.SECONDS)); + + // Register handler to count SDK_UPDATE events and capture metadata + List receivedMetadataList = new ArrayList<>(); + CountDownLatch updateLatch = new CountDownLatch(1); + + client.addEventListener(new SdkEventListener() { + @Override + public void onUpdate(SplitClient c, SdkUpdateMetadata metadata) { + synchronized (receivedMetadataList) { + receivedMetadataList.add(metadata); + } + updateLatch.countDown(); + } + }); + + // Wait for SDK_UPDATE (triggered by polling that returns both flag and RBS changes) + boolean updateFired = updateLatch.await(10, TimeUnit.SECONDS); + assertTrue("SDK_UPDATE should fire", updateFired); + + // Wait a bit to ensure no additional events fire + Thread.sleep(1000); + + // Verify only ONE SDK_UPDATE was fired + synchronized (receivedMetadataList) { + assertEquals("Should receive exactly 1 SDK_UPDATE event (not 2)", 1, receivedMetadataList.size()); + + // Verify it's FLAGS_UPDATE (not SEGMENTS_UPDATE) + SdkUpdateMetadata metadata = receivedMetadataList.get(0); + assertNotNull("Metadata should not be null", metadata); + assertEquals("Type should be FLAGS_UPDATE (not SEGMENTS_UPDATE)", + SdkUpdateMetadata.Type.FLAGS_UPDATE, metadata.getType()); + } + + factory.destroy(); + } + + /** + * Scenario: sdkUpdateMetadata contains Type.SEGMENTS_UPDATE for membership segments update (polling) + *

+ * Given sdkReady has already been emitted + * And a handler H is registered for sdkUpdate + * When segments change via polling (server returns different segments) + * Then sdkUpdate is emitted + * And handler H receives metadata with getType() returning Type.SEGMENTS_UPDATE + * And handler H receives metadata with getNames() returning an empty list + */ + @Test + public void sdkUpdateMetadataContainsTypeForMembershipSegmentsUpdate() throws Exception { + verifySdkUpdateForSegmentsPollingWithEmptyNames( + // Initial sync: segment1, segment2 + "{\"ms\":{\"k\":[{\"n\":\"segment1\"},{\"n\":\"segment2\"}],\"cn\":1000},\"ls\":{\"k\":[],\"cn\":1000}}", + // Polling: segment1 removed, segment3 added + "{\"ms\":{\"k\":[{\"n\":\"segment2\"},{\"n\":\"segment3\"}],\"cn\":2000},\"ls\":{\"k\":[],\"cn\":1000}}" + ); + } + + /** + * Scenario: sdkUpdateMetadata contains Type.SEGMENTS_UPDATE for large segments update (polling) + *

+ * Given sdkReady has already been emitted + * And a handler H is registered for sdkUpdate + * When large segments change via polling (server returns different large segments) + * Then sdkUpdate is emitted + * And handler H receives metadata with getType() returning Type.SEGMENTS_UPDATE + * And handler H receives metadata with getNames() returning an empty list + */ + @Test + public void sdkUpdateMetadataContainsTypeForLargeSegmentsUpdate() throws Exception { + verifySdkUpdateForSegmentsPollingWithEmptyNames( + // Initial sync: large_segment1, large_segment2 + "{\"ms\":{\"k\":[],\"cn\":1000},\"ls\":{\"k\":[{\"n\":\"large_segment1\"},{\"n\":\"large_segment2\"}],\"cn\":1000}}", + // Polling: large_segment1 removed, large_segment3 added + "{\"ms\":{\"k\":[],\"cn\":1000},\"ls\":{\"k\":[{\"n\":\"large_segment2\"},{\"n\":\"large_segment3\"}],\"cn\":2000}}" + ); + } + + /** + * Scenario: Two distinct SDK_UPDATE events are fired when both segments and large segments change + *

+ * Given sdkReady has already been emitted + * And a handler H is registered for sdkUpdate + * When a single memberships response contains changes to both segments and large segments + * Then two SDK_UPDATE events are emitted + * And both events have metadata with getType() returning Type.SEGMENTS_UPDATE and empty names + */ + @Test + public void twoDistinctSdkUpdateEventsWhenBothSegmentsAndLargeSegmentsChange() throws Exception { + // Initial sync: segment1, segment2 in ms; large_segment1, large_segment2 in ls + String initialResponse = "{\"ms\":{\"k\":[{\"n\":\"segment1\"},{\"n\":\"segment2\"}],\"cn\":1000},\"ls\":{\"k\":[{\"n\":\"large_segment1\"},{\"n\":\"large_segment2\"}],\"cn\":1000}}"; + // Polling: both ms and ls change + String pollingResponse = "{\"ms\":{\"k\":[{\"n\":\"segment2\"},{\"n\":\"segment3\"}],\"cn\":2000},\"ls\":{\"k\":[{\"n\":\"large_segment2\"},{\"n\":\"large_segment3\"}],\"cn\":2000}}"; + + List metadataList = waitForSegmentsPollingUpdates(initialResponse, pollingResponse, 2); + + // Verify we received 2 distinct SDK_UPDATE events + assertEquals("Should receive 2 SDK_UPDATE events", 2, metadataList.size()); + + // Both events should be SEGMENTS_UPDATE type with empty names + for (SdkUpdateMetadata metadata : metadataList) { + assertNotNull("Metadata should not be null", metadata); + assertEquals("Type should be SEGMENTS_UPDATE", + SdkUpdateMetadata.Type.SEGMENTS_UPDATE, metadata.getType()); + assertNotNull("Names should not be null", metadata.getNames()); + assertTrue("Names should be empty for SEGMENTS_UPDATE", metadata.getNames().isEmpty()); + } + } + + /** + * Helper method to verify SDK_UPDATE with SEGMENTS_UPDATE type is emitted when segments change via polling. + * Verifies that names are always empty for SEGMENTS_UPDATE. + * + * @param initialResponse the memberships response for initial sync + * @param pollingResponse the memberships response for polling (with changed segments) + */ + private void verifySdkUpdateForSegmentsPollingWithEmptyNames(String initialResponse, String pollingResponse) throws Exception { + List metadataList = waitForSegmentsPollingUpdates(initialResponse, pollingResponse, 1); + + assertEquals("Should receive 1 SDK_UPDATE event", 1, metadataList.size()); + + SdkUpdateMetadata metadata = metadataList.get(0); + assertNotNull("Metadata should not be null", metadata); + assertEquals("Type should be SEGMENTS_UPDATE", + SdkUpdateMetadata.Type.SEGMENTS_UPDATE, metadata.getType()); + + assertNotNull("Names should not be null", metadata.getNames()); + assertTrue("Names should be empty for SEGMENTS_UPDATE", metadata.getNames().isEmpty()); + } + + /** + * Helper method that sets up polling for segments and waits for the expected number of SDK_UPDATE events. + * + * @param initialResponse the memberships response for initial sync + * @param pollingResponse the memberships response for polling (with changed segments) + * @param expectedEventCount the number of SDK_UPDATE events to wait for + * @return list of received SdkUpdateMetadata from the events + */ + private List waitForSegmentsPollingUpdates(String initialResponse, String pollingResponse, + int expectedEventCount) throws Exception { + AtomicInteger membershipsHitCount = new AtomicInteger(0); + + final Dispatcher pollingDispatcher = new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest request) { + final String path = request.getPath(); + if (path.contains("/" + IntegrationHelper.ServicePath.MEMBERSHIPS)) { + int count = membershipsHitCount.incrementAndGet(); + if (count <= 1) { + return new MockResponse().setResponseCode(200).setBody(initialResponse); + } else { + return new MockResponse().setResponseCode(200).setBody(pollingResponse); + } + } else if (path.contains("/splitChanges")) { + return new MockResponse().setResponseCode(200) + .setBody(IntegrationHelper.emptyTargetingRulesChanges(1000, 1000)); + } else if (path.contains("/testImpressions/bulk")) { + return new MockResponse().setResponseCode(200); + } + return new MockResponse().setResponseCode(404); + } + }; + mWebServer.setDispatcher(pollingDispatcher); + + SplitClientConfig config = new TestableSplitConfigBuilder() + .serviceEndpoints(endpoints()) + .ready(30000) + .featuresRefreshRate(999999) + .segmentsRefreshRate(3) + .impressionsRefreshRate(999999) + .streamingEnabled(false) + .trafficType("account") + .build(); + + SplitFactory factory = buildFactory(config); + SplitClient client = factory.client(); + + CountDownLatch readyLatch = new CountDownLatch(1); + client.on(SplitEvent.SDK_READY, new SplitEventTask() { + @Override + public void onPostExecution(SplitClient c) { + readyLatch.countDown(); + } + }); + assertTrue("SDK_READY should fire", readyLatch.await(10, TimeUnit.SECONDS)); + + List receivedMetadataList = new ArrayList<>(); + AtomicInteger legacyHandlerCount = new AtomicInteger(0); + // Wait for expectedEventCount events x 2 handlers (new API + legacy) + CountDownLatch updateLatch = new CountDownLatch(expectedEventCount * 2); + + // Register new API handler (addEventListener) + client.addEventListener(new SdkEventListener() { + @Override + public void onUpdate(SplitClient c, SdkUpdateMetadata metadata) { + synchronized (receivedMetadataList) { + receivedMetadataList.add(metadata); + } + updateLatch.countDown(); + } + }); + + // Register legacy API handler (client.on) + client.on(SplitEvent.SDK_UPDATE, new SplitEventTask() { + @Override + public void onPostExecution(SplitClient c) { + legacyHandlerCount.incrementAndGet(); + updateLatch.countDown(); + } + }); + + boolean updateFired = updateLatch.await(10, TimeUnit.SECONDS); + assertTrue("SDK_UPDATE should fire " + expectedEventCount + " time(s). " + + "Hit count: " + membershipsHitCount.get() + ", metadata count: " + receivedMetadataList.size() + + ", legacy count: " + legacyHandlerCount.get(), updateFired); + + // Verify legacy API was triggered the expected number of times + assertEquals("Legacy API (client.on) should be triggered " + expectedEventCount + " time(s)", + expectedEventCount, legacyHandlerCount.get()); + + factory.destroy(); + + return receivedMetadataList; + } + /** * Creates a client and waits for SDK_READY to fire. * Returns a TestClientFixture containing the factory, client, and ready latch. @@ -1571,4 +1868,5 @@ private void populateDatabaseWithRbsData() { // Set RBS change number so streaming notifications trigger in-place updates mDatabase.generalInfoDao().update(new GeneralInfoEntity("rbsChangeNumber", 1000L)); } + } diff --git a/main/src/main/java/io/split/android/client/EvaluationResult.java b/main/src/main/java/io/split/android/client/EvaluationResult.java index 3c50e0428..da529eba6 100644 --- a/main/src/main/java/io/split/android/client/EvaluationResult.java +++ b/main/src/main/java/io/split/android/client/EvaluationResult.java @@ -14,6 +14,7 @@ public EvaluationResult(String treatment, String label) { this(treatment, label, null, null, false); } + @VisibleForTesting public EvaluationResult(String treatment, String label, boolean impressionsDisabled) { this(treatment, label, null, null, impressionsDisabled); } diff --git a/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java b/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java index a5f0759bd..5141b887d 100644 --- a/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java +++ b/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsSyncTask.java @@ -16,9 +16,10 @@ import io.split.android.client.dtos.AllSegmentsChange; import io.split.android.client.dtos.SegmentsChange; -import io.split.android.client.events.SplitEvent; import io.split.android.client.events.SplitEventsManager; import io.split.android.client.events.SplitInternalEvent; +import io.split.android.client.events.metadata.EventMetadata; +import io.split.android.client.events.metadata.EventMetadataHelpers; import io.split.android.client.network.SplitHttpHeadersBuilder; import io.split.android.client.service.ServiceConstants; import io.split.android.client.service.executor.SplitTask; @@ -268,17 +269,18 @@ private void fireMySegmentsUpdatedIfNeeded(UpdateSegmentsResult segmentsResult, // This order is important: if we fire MEMBERSHIPS_SYNC_COMPLETE first, it may trigger SDK_READY, // and then the *_UPDATED events would immediately trigger SDK_UPDATE during initial sync. // By firing *_UPDATED first (while SDK_READY hasn't triggered yet), they won't trigger SDK_UPDATE. - boolean segmentsHaveChanged = mMySegmentsChangeChecker.mySegmentsHaveChanged(segmentsResult.oldSegments, segmentsResult.newSegments); - boolean largeSegmentsHaveChanged = mMySegmentsChangeChecker.mySegmentsHaveChanged(largeSegmentsResult.oldSegments, largeSegmentsResult.newSegments); + List changedSegments = mMySegmentsChangeChecker.getChangedSegments(segmentsResult.oldSegments, segmentsResult.newSegments); + List changedLargeSegments = mMySegmentsChangeChecker.getChangedSegments(largeSegmentsResult.oldSegments, largeSegmentsResult.newSegments); - if (segmentsHaveChanged) { + if (!changedSegments.isEmpty()) { Logger.v("New segments: " + segmentsResult.newSegments); - mEventsManager.notifyInternalEvent(mUpdateEvent); + mEventsManager.notifyInternalEvent(mUpdateEvent, EventMetadataHelpers.createUpdatedSegmentsMetadata()); } - if (largeSegmentsHaveChanged) { + if (!changedLargeSegments.isEmpty()) { Logger.v("New large segments: " + largeSegmentsResult.newSegments); - mEventsManager.notifyInternalEvent(SplitInternalEvent.MY_LARGE_SEGMENTS_UPDATED); + mEventsManager.notifyInternalEvent(SplitInternalEvent.MY_LARGE_SEGMENTS_UPDATED, + EventMetadataHelpers.createUpdatedSegmentsMetadata()); } // Fire sync complete AFTER update events. This ensures SDK_READY triggers after diff --git a/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsUpdateTask.java b/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsUpdateTask.java index 3f8dc1260..cf1257ca4 100644 --- a/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsUpdateTask.java +++ b/main/src/main/java/io/split/android/client/service/mysegments/MySegmentsUpdateTask.java @@ -9,6 +9,7 @@ import io.split.android.client.dtos.SegmentsChange; import io.split.android.client.events.SplitEventsManager; import io.split.android.client.events.SplitInternalEvent; +import io.split.android.client.events.metadata.EventMetadataHelpers; import io.split.android.client.service.executor.SplitTask; import io.split.android.client.service.executor.SplitTaskExecutionInfo; import io.split.android.client.service.executor.SplitTaskType; @@ -96,7 +97,7 @@ public SplitTaskExecutionInfo remove() { private void updateAndNotify(Set segments) { mMySegmentsStorage.set(SegmentsChange.create(segments, mChangeNumber)); - mEventsManager.notifyInternalEvent(mUpdateEvent); + mEventsManager.notifyInternalEvent(mUpdateEvent, EventMetadataHelpers.createUpdatedSegmentsMetadata()); } private void logError(String message) { diff --git a/main/src/main/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTask.java b/main/src/main/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTask.java index 0ca9b88dc..72c05e4a2 100644 --- a/main/src/main/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTask.java +++ b/main/src/main/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTask.java @@ -1,16 +1,12 @@ package io.split.android.client.service.rules; -import static io.split.android.client.service.splits.SplitsSyncHelper.extractRbsNames; import static io.split.android.client.utils.Utils.checkNotNull; import androidx.annotation.NonNull; -import java.util.List; - import io.split.android.client.dtos.RuleBasedSegment; import io.split.android.client.events.ISplitEventsManager; import io.split.android.client.events.SplitInternalEvent; -import io.split.android.client.events.metadata.EventMetadata; import io.split.android.client.events.metadata.EventMetadataHelpers; import io.split.android.client.service.executor.SplitTask; import io.split.android.client.service.executor.SplitTaskExecutionInfo; @@ -46,8 +42,8 @@ public SplitTaskExecutionInfo execute() { boolean triggerSdkUpdate = mRuleBasedSegmentStorage.update(processedChange.getActive(), processedChange.getArchived(), mChangeNumber, null); if (triggerSdkUpdate) { - EventMetadata metadata = createUpdatedRbsMetadata(processedChange); - mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, metadata); + mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, + EventMetadataHelpers.createUpdatedSegmentsMetadata()); } Logger.v("Updated rule based segment"); @@ -58,9 +54,4 @@ public SplitTaskExecutionInfo execute() { return SplitTaskExecutionInfo.error(SplitTaskType.RULE_BASED_SEGMENT_SYNC); } } - - private EventMetadata createUpdatedRbsMetadata(ProcessedRuleBasedSegmentChange processedChange) { - List updatedRbsNames = extractRbsNames(processedChange); - return EventMetadataHelpers.createUpdatedSegmentsMetadata(updatedRbsNames); - } } diff --git a/main/src/main/java/io/split/android/client/service/splits/RuleBasedSegmentInPlaceUpdateTask.java b/main/src/main/java/io/split/android/client/service/splits/RuleBasedSegmentInPlaceUpdateTask.java index 20b261651..6fb8fc8dc 100644 --- a/main/src/main/java/io/split/android/client/service/splits/RuleBasedSegmentInPlaceUpdateTask.java +++ b/main/src/main/java/io/split/android/client/service/splits/RuleBasedSegmentInPlaceUpdateTask.java @@ -4,12 +4,9 @@ import androidx.annotation.NonNull; -import java.util.List; - import io.split.android.client.dtos.RuleBasedSegment; import io.split.android.client.events.ISplitEventsManager; import io.split.android.client.events.SplitInternalEvent; -import io.split.android.client.events.metadata.EventMetadata; import io.split.android.client.events.metadata.EventMetadataHelpers; import io.split.android.client.service.executor.SplitTask; import io.split.android.client.service.executor.SplitTaskExecutionInfo; @@ -47,8 +44,8 @@ public SplitTaskExecutionInfo execute() { boolean triggerSdkUpdate = mRuleBasedSegmentStorage.update(processedChange.getActive(), processedChange.getArchived(), mChangeNumber, null); if (triggerSdkUpdate) { - EventMetadata metadata = createUpdatedRbsMetadata(processedChange); - mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, metadata); + mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, + EventMetadataHelpers.createUpdatedSegmentsMetadata()); } Logger.v("Updated rule based segment"); @@ -59,9 +56,4 @@ public SplitTaskExecutionInfo execute() { return SplitTaskExecutionInfo.error(SplitTaskType.RULE_BASED_SEGMENT_SYNC); } } - - private EventMetadata createUpdatedRbsMetadata(ProcessedRuleBasedSegmentChange processedChange) { - List updatedRbsNames = SplitsSyncHelper.extractRbsNames(processedChange); - return EventMetadataHelpers.createUpdatedSegmentsMetadata(updatedRbsNames); - } } diff --git a/main/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/main/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index 42075e5b2..0ea6127c8 100644 --- a/main/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/main/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -378,52 +378,6 @@ public static List extractFlagNames(@Nullable ProcessedSplitChange proce return updatedNames; } - /** - * Gets the list of updated rule-based segment names from the last sync operation. - * This includes both active (added/modified) and archived (removed) segments. - * - * @return list of updated RBS names, or empty list if no updates occurred - */ - @NonNull - public List getLastUpdatedRbsNames() { - ProcessedRuleBasedSegmentChange lastChange = mLastProcessedRbsChange.get(); - if (lastChange == null) { - return Collections.emptyList(); - } - return extractRbsNames(lastChange); - } - - /** - * Extracts rule-based segment names from a ProcessedRuleBasedSegmentChange. - * This includes both active (added/modified) and archived (removed) segments. - * - * @param processedChange the processed RBS change - * @return list of RBS names, or empty list if change is null - */ - @NonNull - public static List extractRbsNames(@Nullable ProcessedRuleBasedSegmentChange processedChange) { - if (processedChange == null) { - return Collections.emptyList(); - } - - List updatedNames = new ArrayList<>(); - if (processedChange.getActive() != null) { - for (RuleBasedSegment segment : processedChange.getActive()) { - if (segment != null && segment.getName() != null) { - updatedNames.add(segment.getName()); - } - } - } - if (processedChange.getArchived() != null) { - for (RuleBasedSegment segment : processedChange.getArchived()) { - if (segment != null && segment.getName() != null) { - updatedNames.add(segment.getName()); - } - } - } - return updatedNames; - } - private void updateRbsStorage(RuleBasedSegmentChange ruleBasedSegmentChange) { ProcessedRuleBasedSegmentChange change = mRuleBasedSegmentChangeProcessor.process(ruleBasedSegmentChange.getSegments(), ruleBasedSegmentChange.getTill()); mLastProcessedRbsChange.set(change); diff --git a/main/src/main/java/io/split/android/client/service/splits/SplitsSyncTask.java b/main/src/main/java/io/split/android/client/service/splits/SplitsSyncTask.java index 3a9e426e9..dfab23fff 100644 --- a/main/src/main/java/io/split/android/client/service/splits/SplitsSyncTask.java +++ b/main/src/main/java/io/split/android/client/service/splits/SplitsSyncTask.java @@ -106,14 +106,15 @@ private void notifyInternalEvent(long storedChangeNumber) { // if we fire TARGETING_RULES_SYNC_COMPLETE first, it may trigger SDK_READY, // and then the *_UPDATED events would immediately trigger SDK_UPDATE during initial sync. // By firing *_UPDATED first (while SDK_READY hasn't triggered yet), they won't trigger SDK_UPDATE. + // + // Use else-if logic: if splits changed, only fire SPLITS_UPDATED (FLAGS_UPDATE). + // RBS changes are only relevant when flags DIDN'T change. if (mSplitsSyncHelper.splitsHaveChanged()) { EventMetadata metadata = createUpdatedFlagsMetadata(); mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED, metadata); - } - - if (mSplitsSyncHelper.ruleBasedSegmentsHaveChanged()) { - EventMetadata rbsMetadata = createUpdatedRbsMetadata(); - mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, rbsMetadata); + } else if (mSplitsSyncHelper.ruleBasedSegmentsHaveChanged()) { + mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, + EventMetadataHelpers.createUpdatedSegmentsMetadata()); } // Fire sync complete AFTER update events. This ensures SDK_READY triggers after @@ -130,11 +131,6 @@ private EventMetadata createUpdatedFlagsMetadata() { return EventMetadataHelpers.createUpdatedFlagsMetadata(updatedSplitNames); } - private EventMetadata createUpdatedRbsMetadata() { - List updatedRbsNames = mSplitsSyncHelper.getLastUpdatedRbsNames(); - return EventMetadataHelpers.createUpdatedSegmentsMetadata(updatedRbsNames); - } - private boolean splitsFilterHasChanged(String storedSplitsFilterQueryString) { return !sanitizeString(mSplitsFilterQueryStringFromConfig).equals(sanitizeString(storedSplitsFilterQueryString)); } diff --git a/main/src/main/java/io/split/android/client/service/splits/SplitsUpdateTask.java b/main/src/main/java/io/split/android/client/service/splits/SplitsUpdateTask.java index 8ba5da985..d14600725 100644 --- a/main/src/main/java/io/split/android/client/service/splits/SplitsUpdateTask.java +++ b/main/src/main/java/io/split/android/client/service/splits/SplitsUpdateTask.java @@ -75,14 +75,15 @@ public SplitTaskExecutionInfo execute() { // Fire *_UPDATED events BEFORE sync complete. This order is important: // if we fire TARGETING_RULES_SYNC_COMPLETE first, it may trigger SDK_READY, // and then the *_UPDATED events would immediately trigger SDK_UPDATE during initial sync. + // + // Use If splits changed, only fire SPLITS_UPDATED (FLAGS_UPDATE). + // RBS changes are only relevant when flags DIDN'T change. if (mSplitsSyncHelper.splitsHaveChanged()) { EventMetadata metadata = createUpdatedFlagsMetadata(); mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED, metadata); - } - - if (mSplitsSyncHelper.ruleBasedSegmentsHaveChanged()) { - EventMetadata rbsMetadata = createUpdatedRbsMetadata(); - mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, rbsMetadata); + } else if (mSplitsSyncHelper.ruleBasedSegmentsHaveChanged()) { + mEventsManager.notifyInternalEvent(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED, + EventMetadataHelpers.createUpdatedSegmentsMetadata()); } // Fire sync complete AFTER update events. @@ -99,11 +100,6 @@ private EventMetadata createUpdatedFlagsMetadata() { return EventMetadataHelpers.createUpdatedFlagsMetadata(updatedSplitNames); } - private EventMetadata createUpdatedRbsMetadata() { - List updatedRbsNames = mSplitsSyncHelper.getLastUpdatedRbsNames(); - return EventMetadataHelpers.createUpdatedSegmentsMetadata(updatedRbsNames); - } - @VisibleForTesting public void setChangeChecker(SplitsChangeChecker changeChecker) { mChangeChecker = changeChecker; diff --git a/main/src/main/java/io/split/android/client/service/synchronizer/MySegmentsChangeChecker.java b/main/src/main/java/io/split/android/client/service/synchronizer/MySegmentsChangeChecker.java index 16fb4cb73..780b70adb 100644 --- a/main/src/main/java/io/split/android/client/service/synchronizer/MySegmentsChangeChecker.java +++ b/main/src/main/java/io/split/android/client/service/synchronizer/MySegmentsChangeChecker.java @@ -1,12 +1,36 @@ package io.split.android.client.service.synchronizer; -import java.util.Collections; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; public class MySegmentsChangeChecker { - public boolean mySegmentsHaveChanged(final List oldSegments, final List newSegments) { - Collections.sort(oldSegments); - Collections.sort(newSegments); - return !oldSegments.equals(newSegments); + + /** + * Computes and returns the list of changed segment names (added + removed) between old and new segments. + * An empty list means no changes occurred. + * + * @param oldSegments the previous list of segment names + * @param newSegments the new list of segment names + * @return list of segment names that were either added or removed (empty if no changes) + */ + public List getChangedSegments(final List oldSegments, final List newSegments) { + Set oldSet = new HashSet<>(oldSegments); + Set newSet = new HashSet<>(newSegments); + + // Added segments: in new but not in old + Set added = new HashSet<>(newSet); + added.removeAll(oldSet); + + // Removed segments: in old but not in new + Set removed = new HashSet<>(oldSet); + removed.removeAll(newSet); + + // Combined changed segments + Set changed = new HashSet<>(added); + changed.addAll(removed); + + return new ArrayList<>(changed); } } diff --git a/main/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageImpl.java b/main/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageImpl.java index fd0e81a84..4478a9a81 100644 --- a/main/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageImpl.java +++ b/main/src/main/java/io/split/android/client/storage/mysegments/MySegmentsStorageImpl.java @@ -63,6 +63,7 @@ public long getChangeNumber() { public void clear() { mInMemoryMySegments.clear(); mTill.set(DEFAULT_CHANGE_NUMBER); + mPersistentStorage.set(mMatchingKey, SegmentsChange.createEmpty()); } @NonNull diff --git a/main/src/test/java/io/split/android/client/service/MySegmentsChangesCheckerTest.java b/main/src/test/java/io/split/android/client/service/MySegmentsChangesCheckerTest.java index c01b5c279..263d59e6c 100644 --- a/main/src/test/java/io/split/android/client/service/MySegmentsChangesCheckerTest.java +++ b/main/src/test/java/io/split/android/client/service/MySegmentsChangesCheckerTest.java @@ -1,12 +1,13 @@ package io.split.android.client.service; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import io.split.android.client.service.synchronizer.MySegmentsChangeChecker; @@ -16,71 +17,80 @@ public class MySegmentsChangesCheckerTest { @Test public void testChangesArrived() { - List old = Arrays.asList("s1", "s2", "s3"); List newSegments = Arrays.asList("s1"); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); - - Assert.assertTrue(result); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); + + Assert.assertFalse(result.isEmpty()); + // s2 and s3 were removed + Set changedSet = new HashSet<>(result); + Assert.assertTrue(changedSet.contains("s2")); + Assert.assertTrue(changedSet.contains("s3")); + Assert.assertEquals(2, result.size()); } @Test public void testNewChangesArrived() { - List newSegments = Arrays.asList("s1", "s2", "s3"); List old = Arrays.asList("s1"); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); - - Assert.assertTrue(result); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); + + Assert.assertFalse(result.isEmpty()); + // s2 and s3 were added + Set changedSet = new HashSet<>(result); + Assert.assertTrue(changedSet.contains("s2")); + Assert.assertTrue(changedSet.contains("s3")); + Assert.assertEquals(2, result.size()); } @Test public void testNoChangesArrived() { - List old = Arrays.asList("s1", "s2", "s3"); List newSegments = Arrays.asList("s1", "s2", "s3"); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); - Assert.assertFalse(result); + Assert.assertTrue(result.isEmpty()); } @Test public void testNoChangesDifferentOrder() { - List old = Arrays.asList("s1", "s2", "s3"); List newSegments = Arrays.asList("s2", "s1", "s3"); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); - Assert.assertFalse(result); + Assert.assertTrue(result.isEmpty()); } @Test public void testNoChangesDifferentOrderInverted() { - List newSegments = Arrays.asList("s1", "s2", "s3"); List old = Arrays.asList("s2", "s1", "s3"); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); - Assert.assertFalse(result); + Assert.assertTrue(result.isEmpty()); } @Test public void testNoChangesArrivedEmpty() { - List newSegments = new ArrayList<>(); List old = new ArrayList<>(); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); - Assert.assertFalse(result); + Assert.assertTrue(result.isEmpty()); } @Test public void testEmptyChangesArrived() { - List newSegments = new ArrayList<>(); List old = Arrays.asList("s1", "s2", "s3"); - boolean result = mMySegmentsChangeChecker.mySegmentsHaveChanged(old, newSegments); - - Assert.assertTrue(result); + List result = mMySegmentsChangeChecker.getChangedSegments(old, newSegments); + + Assert.assertFalse(result.isEmpty()); + // s1, s2, s3 were all removed + Set changedSet = new HashSet<>(result); + Assert.assertTrue(changedSet.contains("s1")); + Assert.assertTrue(changedSet.contains("s2")); + Assert.assertTrue(changedSet.contains("s3")); + Assert.assertEquals(3, result.size()); } } diff --git a/main/src/test/java/io/split/android/client/service/MySegmentsSyncTaskTest.java b/main/src/test/java/io/split/android/client/service/MySegmentsSyncTaskTest.java index 70faedd68..cf99782c7 100644 --- a/main/src/test/java/io/split/android/client/service/MySegmentsSyncTaskTest.java +++ b/main/src/test/java/io/split/android/client/service/MySegmentsSyncTaskTest.java @@ -29,6 +29,7 @@ import java.util.Set; import io.split.android.client.dtos.AllSegmentsChange; +import io.split.android.client.events.metadata.EventMetadata; import io.split.android.client.dtos.SegmentsChange; import io.split.android.client.events.SplitEventsManager; import io.split.android.client.events.SplitInternalEvent; @@ -70,7 +71,7 @@ public class MySegmentsSyncTaskTest { @Before public void setup() { mAutoCloseable = MockitoAnnotations.openMocks(this); - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(any(), any())).thenReturn(true); + when(mMySegmentsChangeChecker.getChangedSegments(any(), any())).thenReturn(Collections.singletonList("changed_segment")); mTask = new MySegmentsSyncTask(mMySegmentsFetcher, mySegmentsStorage, myLargeSegmentsStorage, false, mEventsManager, mTelemetryRuntimeProducer, MySegmentsSyncTaskConfig.get(), null, null); loadMySegments(); } @@ -223,61 +224,61 @@ public void addTillParameterToRequestWhenResponseCnDoesNotMatchTargetAndRetryLim @Test public void syncCompleteEventIsEmittedWhenNoChangesInSegments() throws HttpFetcherException { - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(any(), any())).thenReturn(false); + when(mMySegmentsChangeChecker.getChangedSegments(any(), any())).thenReturn(Collections.emptyList()); when(mMySegmentsFetcher.execute(noParams, null)).thenReturn(mMySegments); mTask = new MySegmentsSyncTask(mMySegmentsFetcher, mySegmentsStorage, myLargeSegmentsStorage, false, mEventsManager, mMySegmentsChangeChecker, mTelemetryRuntimeProducer, MySegmentsSyncTaskConfig.get(), null, null, mock(BackoffCounter.class), 1); mTask.execute(); verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE); - verify(mEventsManager, never()).notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED); + verify(mEventsManager, never()).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED), any(EventMetadata.class)); } @Test public void membershipsSyncCompleteIsAlwaysFiredOnSuccessfulSync() throws HttpFetcherException { when(mMySegmentsFetcher.execute(noParams, null)).thenReturn(mMySegments); - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(any(), any())).thenReturn(true); + when(mMySegmentsChangeChecker.getChangedSegments(any(), any())).thenReturn(Collections.singletonList("changed_segment")); mTask = new MySegmentsSyncTask(mMySegmentsFetcher, mySegmentsStorage, myLargeSegmentsStorage, false, mEventsManager, mMySegmentsChangeChecker, mTelemetryRuntimeProducer, MySegmentsSyncTaskConfig.get(), null, null, mock(BackoffCounter.class), 1); mTask.execute(); // Verify MEMBERSHIPS_SYNC_COMPLETE is always fired on successful sync, even when segments changed - verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE); - verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED)); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE)); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED), any(EventMetadata.class)); } @Test public void updateEventIsFiredWhenSegmentsHaveChanged() throws HttpFetcherException { when(mMySegmentsFetcher.execute(noParams, null)).thenReturn(mMySegments); - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(any(), any())).thenReturn(true); + when(mMySegmentsChangeChecker.getChangedSegments(any(), any())).thenReturn(Collections.singletonList("changed_segment")); mTask = new MySegmentsSyncTask(mMySegmentsFetcher, mySegmentsStorage, myLargeSegmentsStorage, false, mEventsManager, mMySegmentsChangeChecker, mTelemetryRuntimeProducer, MySegmentsSyncTaskConfig.get(), null, null, mock(BackoffCounter.class), 1); mTask.execute(); - verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED)); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED), any(EventMetadata.class)); } @Test public void updatedEventIsEmittedWhenChangesInSegments() throws HttpFetcherException { - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(any(), any())).thenReturn(true); + when(mMySegmentsChangeChecker.getChangedSegments(any(), any())).thenReturn(Collections.singletonList("changed_segment")); when(mMySegmentsFetcher.execute(noParams, null)).thenReturn(mMySegments); mTask = new MySegmentsSyncTask(mMySegmentsFetcher, mySegmentsStorage, myLargeSegmentsStorage, false, mEventsManager, mMySegmentsChangeChecker, mTelemetryRuntimeProducer, MySegmentsSyncTaskConfig.get(), null, null, mock(BackoffCounter.class), 1); mTask.execute(); - verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED), any(EventMetadata.class)); } @Test public void largeSegmentsUpdatedEventIsEmittedWhenChangesInLargeSegmentsAndNotInSegments() throws HttpFetcherException { - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(any(), any())).thenReturn(false); - when(mMySegmentsChangeChecker.mySegmentsHaveChanged(Collections.emptyList(), Collections.singletonList("largesegment0"))).thenReturn(true); + when(mMySegmentsChangeChecker.getChangedSegments(any(), any())).thenReturn(Collections.emptyList()); + when(mMySegmentsChangeChecker.getChangedSegments(Collections.emptyList(), Collections.singletonList("largesegment0"))).thenReturn(Collections.singletonList("largesegment0")); when(mMySegmentsFetcher.execute(noParams, null)).thenReturn(createChange(1L)); mTask = new MySegmentsSyncTask(mMySegmentsFetcher, mySegmentsStorage, myLargeSegmentsStorage, false, mEventsManager, mMySegmentsChangeChecker, mTelemetryRuntimeProducer, MySegmentsSyncTaskConfig.get(), null, null, mock(BackoffCounter.class), 1); mTask.execute(); - verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.MY_LARGE_SEGMENTS_UPDATED); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_LARGE_SEGMENTS_UPDATED), any(EventMetadata.class)); } @Test diff --git a/main/src/test/java/io/split/android/client/service/MySegmentsUpdateTaskTest.java b/main/src/test/java/io/split/android/client/service/MySegmentsUpdateTaskTest.java index 3e4972765..663602b30 100644 --- a/main/src/test/java/io/split/android/client/service/MySegmentsUpdateTaskTest.java +++ b/main/src/test/java/io/split/android/client/service/MySegmentsUpdateTaskTest.java @@ -1,6 +1,7 @@ package io.split.android.client.service; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -179,7 +180,7 @@ public void removeOperationRemovesOnlyNotifiedSegments() { Assert.assertTrue(captorValue.getNames().contains(mCustomerSegment)); Assert.assertEquals(1, captorValue.getNames().size()); Assert.assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); - verify(mEventsManager).notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.MY_SEGMENTS_UPDATED), any()); } @Test diff --git a/main/src/test/java/io/split/android/client/service/SplitSyncTaskTest.java b/main/src/test/java/io/split/android/client/service/SplitSyncTaskTest.java index 3516c38f1..45bc7b228 100644 --- a/main/src/test/java/io/split/android/client/service/SplitSyncTaskTest.java +++ b/main/src/test/java/io/split/android/client/service/SplitSyncTaskTest.java @@ -329,6 +329,28 @@ public void ruleBasedSegmentsUpdatedIsNotFiredWhenRbsUnchanged() { verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE), any()); } + @Test + public void ruleBasedSegmentsUpdatedIsNotFiredWhenBothSplitsAndRbsChanged() { + // When both splits and RBS change, only SPLITS_UPDATED should fire (else-if logic) + mTask = SplitsSyncTask.build(mSplitsSyncHelper, mSplitsStorage, mRuleBasedSegmentStorage, + mQueryString, mEventsManager, mTelemetryRuntimeProducer); + when(mSplitsStorage.getTill()).thenReturn(-1L).thenReturn(100L); + when(mSplitsStorage.getUpdateTimestamp()).thenReturn(0L); + when(mSplitsStorage.getSplitsFilterQueryString()).thenReturn(mQueryString); + when(mSplitsSyncHelper.sync(any(), anyBoolean(), anyBoolean(), eq(ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES))).thenReturn(SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC)); + when(mSplitsSyncHelper.splitsHaveChanged()).thenReturn(true); + when(mSplitsSyncHelper.ruleBasedSegmentsHaveChanged()).thenReturn(true); + when(mSplitsSyncHelper.getLastUpdatedFlagNames()).thenReturn(Arrays.asList("flag1")); + + mTask.execute(); + + // SPLITS_UPDATED should fire + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.SPLITS_UPDATED), any()); + // RULE_BASED_SEGMENTS_UPDATED should NOT fire (else-if logic) + verify(mEventsManager, never()).notifyInternalEvent(eq(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED), any()); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE), any()); + } + @Test public void syncCompleteMetadataHasInitialCacheLoadFalseWhenCacheAlreadyLoaded() { mTask = SplitsSyncTask.build(mSplitsSyncHelper, mSplitsStorage, mRuleBasedSegmentStorage, diff --git a/main/src/test/java/io/split/android/client/service/SplitUpdateTaskTest.java b/main/src/test/java/io/split/android/client/service/SplitUpdateTaskTest.java index 07089657d..fda1b1a89 100644 --- a/main/src/test/java/io/split/android/client/service/SplitUpdateTaskTest.java +++ b/main/src/test/java/io/split/android/client/service/SplitUpdateTaskTest.java @@ -189,6 +189,28 @@ public void splitsUpdatedIncludesMetadataWithUpdatedFlags() { })); } + @Test + public void ruleBasedSegmentsUpdatedIsNotFiredWhenBothSplitsAndRbsChanged() { + // When both splits and RBS change, only SPLITS_UPDATED should fire (else-if logic) + long storedChangeNumber = 100L; + long storedRbsChangeNumber = 200L; + when(mSplitsStorage.getTill()).thenReturn(storedChangeNumber).thenReturn(150L); + when(mRuleBasedSegmentStorage.getChangeNumber()).thenReturn(storedRbsChangeNumber).thenReturn(250L); + when(mSplitsSyncHelper.sync(any(), eq(ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES))) + .thenReturn(SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC)); + when(mSplitsSyncHelper.splitsHaveChanged()).thenReturn(true); + when(mSplitsSyncHelper.ruleBasedSegmentsHaveChanged()).thenReturn(true); + when(mSplitsSyncHelper.getLastUpdatedFlagNames()).thenReturn(Arrays.asList("flag1")); + + mTask.execute(); + + // SPLITS_UPDATED should fire + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.SPLITS_UPDATED), any()); + // RULE_BASED_SEGMENTS_UPDATED should NOT fire (else-if logic) + verify(mEventsManager, never()).notifyInternalEvent(eq(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED), any()); + verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE), any()); + } + @After public void tearDown() { reset(mSplitsStorage); diff --git a/main/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java b/main/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java index 0f8105c7c..e60147600 100644 --- a/main/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java +++ b/main/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java @@ -780,66 +780,4 @@ public void getLastUpdatedFlagNamesIncludesArchivedSplits() throws HttpFetcherEx assertEquals(1, result.size()); assertTrue(result.contains("archived_split")); } - - @Test - public void getLastUpdatedRbsNamesReturnsSegmentNamesAfterSync() throws HttpFetcherException { - RuleBasedSegment activeSegment = RuleBasedSegmentStorageImplTest.createRuleBasedSegment("active_segment"); - RuleBasedSegment archivedSegment = RuleBasedSegmentStorageImplTest.createRuleBasedSegment("archived_segment"); - SplitChange splitChange = SplitChange.create(-1, 100L, Collections.emptyList()); - RuleBasedSegmentChange rbsChange = RuleBasedSegmentChange.create(-1, 100L, Collections.singletonList(activeSegment)); - // Create ProcessedRuleBasedSegmentChange with both active and archived segments - ProcessedRuleBasedSegmentChange processedChange = new ProcessedRuleBasedSegmentChange( - Set.of(activeSegment), Set.of(archivedSegment), 100L, System.currentTimeMillis()); - - doReturn(processedChange).when(mRuleBasedSegmentChangeProcessor).process(any(List.class), anyLong()); - - when(mSplitsFetcher.execute(any(), any())) - .thenReturn(TargetingRulesChange.create(splitChange, rbsChange)) - .thenReturn(TargetingRulesChange.create(SplitChange.create(100L, 100L, Collections.emptyList()), RuleBasedSegmentChange.create(100L, 100L, Collections.emptyList()))); - when(mSplitsStorage.getTill()).thenReturn(-1L).thenReturn(100L); - when(mRuleBasedSegmentStorageProducer.getChangeNumber()).thenReturn(-1L).thenReturn(100L); - - mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); - - List result = mSplitsSyncHelper.getLastUpdatedRbsNames(); - assertEquals(2, result.size()); - assertTrue(result.contains("active_segment")); - assertTrue(result.contains("archived_segment")); - } - - @Test - public void getLastUpdatedRbsNamesReturnsEmptyListWhenNoSyncPerformed() { - List result = mSplitsSyncHelper.getLastUpdatedRbsNames(); - assertTrue(result.isEmpty()); - } - - @Test - public void extractRbsNamesReturnsActiveAndArchivedSegmentNames() { - RuleBasedSegment activeSegment = RuleBasedSegmentStorageImplTest.createRuleBasedSegment("active_segment"); - RuleBasedSegment archivedSegment = RuleBasedSegmentStorageImplTest.createRuleBasedSegment("archived_segment"); - ProcessedRuleBasedSegmentChange processedChange = new ProcessedRuleBasedSegmentChange( - Set.of(activeSegment), Set.of(archivedSegment), 100L, System.currentTimeMillis()); - - List result = SplitsSyncHelper.extractRbsNames(processedChange); - - assertEquals(2, result.size()); - assertTrue(result.contains("active_segment")); - assertTrue(result.contains("archived_segment")); - } - - @Test - public void extractRbsNamesReturnsEmptyListForNullChange() { - List result = SplitsSyncHelper.extractRbsNames(null); - assertTrue(result.isEmpty()); - } - - @Test - public void extractRbsNamesHandlesNullActiveAndArchivedSets() { - ProcessedRuleBasedSegmentChange processedChange = new ProcessedRuleBasedSegmentChange( - null, null, 100L, System.currentTimeMillis()); - - List result = SplitsSyncHelper.extractRbsNames(processedChange); - - assertTrue(result.isEmpty()); - } } diff --git a/main/src/test/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTaskTest.java b/main/src/test/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTaskTest.java index 713ba5cfa..37d05d47d 100644 --- a/main/src/test/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTaskTest.java +++ b/main/src/test/java/io/split/android/client/service/rules/RuleBasedSegmentInPlaceUpdateTaskTest.java @@ -99,7 +99,7 @@ public void updateIsCalledOnStorage() { } @Test - public void segmentsUpdatedIncludesMetadataWithActiveAndArchivedSegmentNames() { + public void segmentsUpdatedIncludesMetadataWithEmptyNames() { RuleBasedSegment activeSegment = createRuleBasedSegment("active_segment"); RuleBasedSegment archivedSegment = createRuleBasedSegment("archived_segment"); long changeNumber = 123L; @@ -111,14 +111,13 @@ public void segmentsUpdatedIncludesMetadataWithActiveAndArchivedSegmentNames() { mTask = getTask(activeSegment, changeNumber); mTask.execute(); + // SEGMENTS_UPDATE always has empty names verify(mEventsManager).notifyInternalEvent(eq(SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED), argThat(metadata -> { if (metadata == null) return false; SdkUpdateMetadata typedMeta = TypedTaskConverter.convertForSdkUpdate(metadata); List names = typedMeta.getNames(); assertNotNull(names); - assertEquals(2, names.size()); - assertTrue(names.contains("active_segment")); - assertTrue(names.contains("archived_segment")); + assertTrue("Names should be empty for SEGMENTS_UPDATE", names.isEmpty()); assertEquals(SdkUpdateMetadata.Type.SEGMENTS_UPDATE, typedMeta.getType()); return true; }));