Skip to content

Commit 82cf13f

Browse files
gtheagthea
authored
Remove synthetic sync events (#836)
1 parent c75832c commit 82cf13f

20 files changed

+387
-154
lines changed

‎events-domain/src/main/java/io/split/android/client/events/EventsManagerCoordinator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public class EventsManagerCoordinator implements ISplitEventsManager, EventsMana
2626
*/
2727
private static final Set<SplitInternalEvent> SDK_SCOPED_EVENTS = EnumSet.of(
2828
SplitInternalEvent.SPLITS_UPDATED,
29-
SplitInternalEvent.SPLITS_FETCHED,
29+
SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE,
3030
SplitInternalEvent.SPLITS_LOADED_FROM_STORAGE,
3131
SplitInternalEvent.SPLIT_KILLED_NOTIFICATION,
3232
SplitInternalEvent.RULE_BASED_SEGMENTS_UPDATED,

‎events-domain/src/main/java/io/split/android/client/events/SplitEventsManager.java‎

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -80,20 +80,7 @@ public SplitEventExecutorResources getExecutorResources() {
8080
@Override
8181
public void notifyInternalEvent(SplitInternalEvent internalEvent) {
8282
requireNonNull(internalEvent);
83-
84-
// Skip FETCHED events after SDK_READY to prevent unnecessary SDK_UPDATE triggers.
85-
// TODO: This is temporary until *_FETCHED and *_UPDATED events are unified.
86-
if ((internalEvent == SplitInternalEvent.SPLITS_FETCHED
87-
|| internalEvent == SplitInternalEvent.MY_SEGMENTS_FETCHED)
88-
&& eventAlreadyTriggered(SplitEvent.SDK_READY)) {
89-
return;
90-
}
91-
92-
// Notify the actual internal event
9383
mEventsManager.notifyInternalEvent(internalEvent, null);
94-
95-
// Also notify the synthetic composite events for SDK_READY evaluation
96-
notifySyntheticEventsIfNeeded(internalEvent);
9784
}
9885

9986
/**
@@ -104,16 +91,7 @@ && eventAlreadyTriggered(SplitEvent.SDK_READY)) {
10491
*/
10592
public void notifyInternalEvent(SplitInternalEvent internalEvent, EventMetadata metadata) {
10693
requireNonNull(internalEvent);
107-
108-
// Skip FETCHED events after SDK_READY
109-
if ((internalEvent == SplitInternalEvent.SPLITS_FETCHED
110-
|| internalEvent == SplitInternalEvent.MY_SEGMENTS_FETCHED)
111-
&& eventAlreadyTriggered(SplitEvent.SDK_READY)) {
112-
return;
113-
}
114-
11594
mEventsManager.notifyInternalEvent(internalEvent, metadata);
116-
notifySyntheticEventsIfNeeded(internalEvent);
11795
}
11896

11997
@Override
@@ -143,32 +121,6 @@ public void destroy() {
143121
mEventsManager.destroy();
144122
}
145123

146-
/**
147-
* Notifies the synthetic composite events based on the actual internal event.
148-
* These synthetic events simplify the SDK_READY condition evaluation.
149-
* The prerequisite configuration ensures SDK_READY_FROM_CACHE always fires before SDK_READY.
150-
* <p>
151-
* TODO: Remove this method once EventsManagerConfig is updated.
152-
*/
153-
private void notifySyntheticEventsIfNeeded(SplitInternalEvent internalEvent) {
154-
switch (internalEvent) {
155-
case SPLITS_UPDATED:
156-
case SPLITS_FETCHED:
157-
mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_SYNC_COMPLETE, null);
158-
break;
159-
160-
case MY_SEGMENTS_UPDATED:
161-
case MY_SEGMENTS_FETCHED:
162-
case MY_LARGE_SEGMENTS_UPDATED:
163-
mEventsManager.notifyInternalEvent(SplitInternalEvent.SEGMENTS_SYNC_COMPLETE, null);
164-
break;
165-
166-
default:
167-
// No synthetic event needed for other internal events
168-
break;
169-
}
170-
}
171-
172124
private void startTimeoutThread(final int blockUntilReady) {
173125
Thread timeoutThread = new Thread(new Runnable() {
174126
@Override

‎events-domain/src/main/java/io/split/android/client/events/SplitEventsManagerConfigFactory.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ static EventsManagerConfig<SplitEvent, SplitInternalEvent> create() {
4141
cacheGroup.add(SplitInternalEvent.ENCRYPTION_MIGRATION_DONE);
4242

4343
Set<SplitInternalEvent> syncGroup = new HashSet<>();
44-
syncGroup.add(SplitInternalEvent.SPLITS_SYNC_COMPLETE);
45-
syncGroup.add(SplitInternalEvent.SEGMENTS_SYNC_COMPLETE);
44+
syncGroup.add(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE);
45+
syncGroup.add(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE);
4646

4747
return EventsManagerConfig.<SplitEvent, SplitInternalEvent>builder()
4848
.requireAll(SplitEvent.SDK_READY,
49-
SplitInternalEvent.SPLITS_SYNC_COMPLETE,
50-
SplitInternalEvent.SEGMENTS_SYNC_COMPLETE)
49+
SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE,
50+
SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE)
5151

5252
// SDK_READY_FROM_CACHE: OR of ANDs
5353
// Fires when (cache group all done) OR (sync group all done)

‎events-domain/src/main/java/io/split/android/client/events/SplitInternalEvent.java‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,26 @@
11
package io.split.android.client.events;
22

33
/**
4-
* Created by sarrubia on 4/6/18.
4+
* Internal events used to track SDK initialization and data updates.
55
*/
6-
76
public enum SplitInternalEvent {
7+
// Cache loading events
88
MY_SEGMENTS_LOADED_FROM_STORAGE,
99
SPLITS_LOADED_FROM_STORAGE,
10-
MY_SEGMENTS_FETCHED,
11-
MY_SEGMENTS_UPDATED,
12-
SPLITS_FETCHED,
13-
SPLITS_UPDATED,
14-
SDK_READY_TIMEOUT_REACHED,
15-
SPLIT_KILLED_NOTIFICATION,
1610
ATTRIBUTES_LOADED_FROM_STORAGE,
1711
ENCRYPTION_MIGRATION_DONE,
12+
13+
// Data update events (fired only when data actually changed)
14+
MY_SEGMENTS_UPDATED,
15+
SPLITS_UPDATED,
1816
MY_LARGE_SEGMENTS_UPDATED,
1917
RULE_BASED_SEGMENTS_UPDATED,
18+
SPLIT_KILLED_NOTIFICATION,
2019

21-
/**
22-
* Synthetic event: fired when splits sync completes (either SPLITS_FETCHED or SPLITS_UPDATED).
23-
* Used internally to simplify SDK_READY and SDK_READY_FROM_CACHE condition evaluation.
24-
*/
25-
SPLITS_SYNC_COMPLETE,
20+
// Sync completion events (fired when sync completes, regardless of data change)
21+
TARGETING_RULES_SYNC_COMPLETE,
22+
MEMBERSHIPS_SYNC_COMPLETE,
2623

27-
/**
28-
* Synthetic event: fired when segments sync completes (any of MY_SEGMENTS_FETCHED,
29-
* MY_SEGMENTS_UPDATED, or MY_LARGE_SEGMENTS_UPDATED).
30-
* Used internally to simplify SDK_READY and SDK_READY_FROM_CACHE condition evaluation.
31-
*/
32-
SEGMENTS_SYNC_COMPLETE,
24+
// Other events
25+
SDK_READY_TIMEOUT_REACHED,
3326
}

‎main/src/androidTest/java/tests/service/EventsManagerTest.java‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import io.split.android.client.events.SplitEventsManager;
1414
import io.split.android.client.events.SplitInternalEvent;
1515
import io.split.android.client.service.executor.SplitTaskExecutorImpl;
16-
import io.split.android.client.service.synchronizer.ThreadUtils;
1716

1817
public class EventsManagerTest {
1918
@Test
@@ -37,7 +36,7 @@ public void testSdkUpdateSplits() throws InterruptedException {
3736
}
3837

3938
@Test
40-
public void testSdkFetchedUpdatedSplits() throws InterruptedException {
39+
public void testSdkUpdateTriggersAfterReady() throws InterruptedException {
4140

4241
SplitClientConfig cfg = SplitClientConfig.builder().build();
4342
SplitEventsManager eventManager = new SplitEventsManager(new SplitTaskExecutorImpl(), cfg.blockUntilReady());
@@ -47,8 +46,10 @@ public void testSdkFetchedUpdatedSplits() throws InterruptedException {
4746
TestingHelper.TestEventTask updateTask = TestingHelper.testTask(updateLatch);
4847
eventManager.register(SplitEvent.SDK_UPDATE, updateTask);
4948

50-
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED);
51-
eventManager.notifyInternalEvent(SplitInternalEvent.SPLITS_FETCHED);
49+
// First make SDK_READY fire by completing sync
50+
eventManager.notifyInternalEvent(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE);
51+
eventManager.notifyInternalEvent(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE);
52+
// Then trigger SDK_UPDATE with a data change
5253
eventManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
5354

5455
updateLatch.await(5, TimeUnit.SECONDS);
@@ -57,7 +58,7 @@ public void testSdkFetchedUpdatedSplits() throws InterruptedException {
5758
}
5859

5960
@Test
60-
public void testSdkUpdatedFetchedSplits() throws InterruptedException {
61+
public void testSdkUpdateDoesNotTriggerBeforeReady() throws InterruptedException {
6162

6263
SplitClientConfig cfg = SplitClientConfig.builder().build();
6364
SplitEventsManager eventManager = new SplitEventsManager(new SplitTaskExecutorImpl(), cfg.blockUntilReady());
@@ -67,12 +68,11 @@ public void testSdkUpdatedFetchedSplits() throws InterruptedException {
6768
TestingHelper.TestEventTask updateTask = TestingHelper.testTask(updateLatch);
6869
eventManager.register(SplitEvent.SDK_UPDATE, updateTask);
6970

70-
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED);
71+
// Fire UPDATED before SDK_READY - should NOT trigger SDK_UPDATE due to prerequisite
7172
eventManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
72-
eventManager.notifyInternalEvent(SplitInternalEvent.SPLITS_FETCHED);
73-
73+
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED);
7474

75-
updateLatch.await(5, TimeUnit.SECONDS);
75+
updateLatch.await(2, TimeUnit.SECONDS);
7676

7777
Assert.assertFalse(updateTask.onExecutedCalled);
7878
}
@@ -98,7 +98,7 @@ public void testSdkUpdateSegments() throws InterruptedException {
9898
}
9999

100100
@Test
101-
public void testSdkFetchedUpdatedSegments() throws InterruptedException {
101+
public void testSdkUpdateTriggersOnSegmentChange() throws InterruptedException {
102102

103103
SplitClientConfig cfg = SplitClientConfig.builder().build();
104104
SplitEventsManager eventManager = new SplitEventsManager(new SplitTaskExecutorImpl(), cfg.blockUntilReady());
@@ -108,8 +108,10 @@ public void testSdkFetchedUpdatedSegments() throws InterruptedException {
108108
TestingHelper.TestEventTask updateTask = TestingHelper.testTask(updateLatch);
109109
eventManager.register(SplitEvent.SDK_UPDATE, updateTask);
110110

111-
eventManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
112-
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_FETCHED);
111+
// Make SDK_READY fire
112+
eventManager.notifyInternalEvent(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE);
113+
eventManager.notifyInternalEvent(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE);
114+
// Then trigger SDK_UPDATE with a segment change
113115
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED);
114116

115117
updateLatch.await(5, TimeUnit.SECONDS);
@@ -118,7 +120,7 @@ public void testSdkFetchedUpdatedSegments() throws InterruptedException {
118120
}
119121

120122
@Test
121-
public void testSdkUpdatedFetchedSegments() throws InterruptedException {
123+
public void testSdkUpdateRequiresDataChange() throws InterruptedException {
122124

123125
SplitClientConfig cfg = SplitClientConfig.builder().build();
124126
SplitEventsManager eventManager = new SplitEventsManager(new SplitTaskExecutorImpl(), cfg.blockUntilReady());
@@ -128,13 +130,14 @@ public void testSdkUpdatedFetchedSegments() throws InterruptedException {
128130
TestingHelper.TestEventTask updateTask = TestingHelper.testTask(updateLatch);
129131
eventManager.register(SplitEvent.SDK_UPDATE, updateTask);
130132

131-
eventManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
132-
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED);
133-
eventManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_FETCHED);
133+
// Make SDK_READY fire with only SYNC_COMPLETE events (no UPDATED)
134+
eventManager.notifyInternalEvent(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE);
135+
eventManager.notifyInternalEvent(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE);
136+
// No UPDATED events fired
134137

138+
updateLatch.await(2, TimeUnit.SECONDS);
135139

136-
updateLatch.await(5, TimeUnit.SECONDS);
137-
140+
// SDK_UPDATE should NOT fire because no data actually changed
138141
Assert.assertFalse(updateTask.onExecutedCalled);
139142
}
140143

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ private void loadSplits() {
216216
}
217217
if (!content.equals(mLastContentLoaded)) {
218218
mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_LOADED_FROM_STORAGE);
219-
mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_FETCHED);
219+
mEventsManager.notifyInternalEvent(SplitInternalEvent.TARGETING_RULES_SYNC_COMPLETE);
220220
mEventsManager.notifyInternalEvent(SplitInternalEvent.SPLITS_UPDATED);
221221
}
222222
mLastContentLoaded = content;

‎main/src/main/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImpl.java‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.split.android.client.localhost.shared;
22

3+
import androidx.annotation.VisibleForTesting;
4+
35
import io.split.android.client.FlagSetsFilter;
46
import io.split.android.client.SplitClient;
57
import io.split.android.client.SplitClientConfig;
@@ -31,6 +33,7 @@ public class LocalhostSplitClientContainerImpl extends BaseSplitClientContainer
3133
private final EventsManagerCoordinator mEventsManagerCoordinator;
3234
private final SplitTaskExecutor mSplitTaskExecutor;
3335
private final FlagSetsFilter mFlagSetsFilter;
36+
private final SplitEventsManagerFactory mEventsManagerFactory;
3437

3538
public LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory,
3639
SplitClientConfig config,
@@ -42,6 +45,24 @@ public LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory,
4245
EventsManagerCoordinator eventsManagerCoordinator,
4346
SplitTaskExecutor taskExecutor,
4447
FlagSetsFilter flagSetsFilter) {
48+
this(splitFactory, config, splitsStorage, splitParser, attributesManagerFactory,
49+
attributesMerger, telemetryStorageProducer, eventsManagerCoordinator,
50+
taskExecutor, flagSetsFilter,
51+
new DefaultSplitEventsManagerFactory(taskExecutor, config));
52+
}
53+
54+
@VisibleForTesting
55+
LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory,
56+
SplitClientConfig config,
57+
SplitsStorage splitsStorage,
58+
SplitParser splitParser,
59+
AttributesManagerFactory attributesManagerFactory,
60+
AttributesMerger attributesMerger,
61+
TelemetryStorageProducer telemetryStorageProducer,
62+
EventsManagerCoordinator eventsManagerCoordinator,
63+
SplitTaskExecutor taskExecutor,
64+
FlagSetsFilter flagSetsFilter,
65+
SplitEventsManagerFactory eventsManagerFactory) {
4566
mSplitFactory = splitFactory;
4667
mConfig = config;
4768
mSplitStorage = splitsStorage;
@@ -52,13 +73,14 @@ public LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory,
5273
mEventsManagerCoordinator = eventsManagerCoordinator;
5374
mSplitTaskExecutor = taskExecutor;
5475
mFlagSetsFilter = flagSetsFilter;
76+
mEventsManagerFactory = eventsManagerFactory;
5577
}
5678

5779
@Override
5880
protected void createNewClient(Key key) {
59-
SplitEventsManager eventsManager = new SplitEventsManager(mSplitTaskExecutor, mConfig.blockUntilReady());
81+
SplitEventsManager eventsManager = mEventsManagerFactory.create();
6082
eventsManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_LOADED_FROM_STORAGE);
61-
eventsManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_FETCHED);
83+
eventsManager.notifyInternalEvent(SplitInternalEvent.MEMBERSHIPS_SYNC_COMPLETE);
6284
eventsManager.notifyInternalEvent(SplitInternalEvent.MY_SEGMENTS_UPDATED);
6385

6486
AttributesStorageImpl attributesStorage = new AttributesStorageImpl();
@@ -88,4 +110,19 @@ protected void createNewClient(Key key) {
88110
public void destroy() {
89111
// No-op
90112
}
113+
114+
private static class DefaultSplitEventsManagerFactory implements SplitEventsManagerFactory {
115+
private final SplitTaskExecutor mTaskExecutor;
116+
private final int mBlockUntilReady;
117+
118+
DefaultSplitEventsManagerFactory(SplitTaskExecutor taskExecutor, SplitClientConfig config) {
119+
mTaskExecutor = taskExecutor;
120+
mBlockUntilReady = config.blockUntilReady();
121+
}
122+
123+
@Override
124+
public SplitEventsManager create() {
125+
return new SplitEventsManager(mTaskExecutor, mBlockUntilReady);
126+
}
127+
}
91128
}

‎main/src/main/java/io/split/android/client/localhost/shared/SplitEventsManagerFactory.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.split.android.client.localhost.shared;
2+
3+
import io.split.android.client.events.SplitEventsManager;
4+
5+
/**
6+
* Factory interface for creating SplitEventsManager instances.
7+
* Package-local interface to allow testing by injecting mock implementations.
8+
*/
9+
interface SplitEventsManagerFactory {
10+
/**
11+
* Creates a new SplitEventsManager instance.
12+
*
13+
* @return a new SplitEventsManager instance
14+
*/
15+
SplitEventsManager create();
16+
}
17+

0 commit comments

Comments
 (0)