From d297476cbfcf259dd4f28732f2132f9e5e0aefbe Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 6 Mar 2026 20:53:09 -0300 Subject: [PATCH 01/15] Module creation --- settings.gradle | 1 + tracker/.gitignore | 1 + tracker/build.gradle | 19 +++++++++++++++++++ .../client/tracker/DefaultTracker.java | 0 .../split/android/client/tracker/Tracker.java | 0 5 files changed, 21 insertions(+) create mode 100644 tracker/.gitignore create mode 100644 tracker/build.gradle rename main/src/main/java/io/split/android/client/EventsTrackerImpl.java => tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java (100%) rename main/src/main/java/io/split/android/client/EventsTracker.java => tracker/src/main/java/io/split/android/client/tracker/Tracker.java (100%) diff --git a/settings.gradle b/settings.gradle index f8bc17da1..4b3af1a51 100644 --- a/settings.gradle +++ b/settings.gradle @@ -9,3 +9,4 @@ include ':main' include ':events' include ':events-domain' include ':backoff' +include ':tracker' diff --git a/tracker/.gitignore b/tracker/.gitignore new file mode 100644 index 000000000..796b96d1c --- /dev/null +++ b/tracker/.gitignore @@ -0,0 +1 @@ +/build diff --git a/tracker/build.gradle b/tracker/build.gradle new file mode 100644 index 000000000..aa8bb68d5 --- /dev/null +++ b/tracker/build.gradle @@ -0,0 +1,19 @@ +plugins { + id 'com.android.library' +} + +apply from: "$projectDir/../gradle/common-android-library.gradle" + +android { + namespace 'io.split.android.client.tracker' + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + targetCompatibility JavaVersion.VERSION_1_8 + } +} + +dependencies { + testImplementation libs.junit4 + testImplementation libs.mockitoCore +} diff --git a/main/src/main/java/io/split/android/client/EventsTrackerImpl.java b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java similarity index 100% rename from main/src/main/java/io/split/android/client/EventsTrackerImpl.java rename to tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java diff --git a/main/src/main/java/io/split/android/client/EventsTracker.java b/tracker/src/main/java/io/split/android/client/tracker/Tracker.java similarity index 100% rename from main/src/main/java/io/split/android/client/EventsTracker.java rename to tracker/src/main/java/io/split/android/client/tracker/Tracker.java From e4dd525c6711d1e1932ddd067f32e5cb8ae77403 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 6 Mar 2026 21:25:00 -0300 Subject: [PATCH 02/15] Tracker module --- main/build.gradle | 1 + .../android/client/PropertyValidatorImpl.java | 15 +- .../split/android/client/SplitClientImpl.java | 5 +- .../android/client/SplitFactoryImpl.java | 29 +++- .../client/validators/EventValidatorImpl.java | 31 +++- .../ValidationMessageLoggerImpl.java | 20 ++- .../client/SplitClientImplBaseTest.java | 3 +- .../SplitClientImplEventRegistrationTest.java | 3 +- .../client/UserConsentManagerTest.java | 3 +- .../service/events/EventsTrackerTest.java | 100 ----------- .../shared/SplitClientContainerImplTest.java | 4 +- .../client/utils/SplitClientImplFactory.java | 8 +- tracker/README.md | 44 +++++ .../client/tracker/DefaultTracker.java | 109 ++++++------ .../split/android/client/tracker/Tracker.java | 10 +- .../android/client/tracker/TrackerEvent.java | 17 ++ .../client/tracker/TrackerEventValidator.java | 12 ++ .../android/client/tracker/TrackerLogger.java | 15 ++ .../tracker/TrackerPropertyValidator.java | 61 +++++++ .../tracker/TrackerValidationError.java | 22 +++ .../client/tracker/DefaultTrackerTest.java | 164 ++++++++++++++++++ 21 files changed, 496 insertions(+), 180 deletions(-) delete mode 100644 main/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java create mode 100644 tracker/README.md create mode 100644 tracker/src/main/java/io/split/android/client/tracker/TrackerEvent.java create mode 100644 tracker/src/main/java/io/split/android/client/tracker/TrackerEventValidator.java create mode 100644 tracker/src/main/java/io/split/android/client/tracker/TrackerLogger.java create mode 100644 tracker/src/main/java/io/split/android/client/tracker/TrackerPropertyValidator.java create mode 100644 tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java create mode 100644 tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java diff --git a/main/build.gradle b/main/build.gradle index c8477731f..45f456bdd 100644 --- a/main/build.gradle +++ b/main/build.gradle @@ -55,6 +55,7 @@ dependencies { api clientModuleProject('http-api') api clientModuleProject('fallback') api clientModuleProject('backoff') + api clientModuleProject('tracker') // Internal module dependencies implementation clientModuleProject('http') implementation clientModuleProject('events-domain') diff --git a/main/src/main/java/io/split/android/client/PropertyValidatorImpl.java b/main/src/main/java/io/split/android/client/PropertyValidatorImpl.java index 01cc06ef6..1c4519bf8 100644 --- a/main/src/main/java/io/split/android/client/PropertyValidatorImpl.java +++ b/main/src/main/java/io/split/android/client/PropertyValidatorImpl.java @@ -3,12 +3,13 @@ import java.util.HashMap; import java.util.Map; +import io.split.android.client.tracker.TrackerPropertyValidator; import io.split.android.client.utils.logger.Logger; import io.split.android.client.validators.PropertyValidator; import io.split.android.client.validators.ValidationConfig; -public class PropertyValidatorImpl implements PropertyValidator { +public class PropertyValidatorImpl implements PropertyValidator, TrackerPropertyValidator { private final static int MAX_PROPS_COUNT = 300; private final static int MAXIMUM_EVENT_PROPERTY_BYTES = @@ -60,4 +61,16 @@ private static int calculateEventSizeInBytes(String key, Object value) { } return valueSize + key.getBytes().length; } + + @Override + public TrackerPropertyResult validate(Map properties, int initialSizeInBytes, + String validationTag) { + Result result = validate(properties, validationTag); + int totalSize = initialSizeInBytes + result.getSizeInBytes(); + if (result.isValid()) { + return TrackerPropertyResult.valid(result.getProperties(), totalSize); + } else { + return TrackerPropertyResult.invalid(result.getErrorMessage(), totalSize); + } + } } diff --git a/main/src/main/java/io/split/android/client/SplitClientImpl.java b/main/src/main/java/io/split/android/client/SplitClientImpl.java index 571efa169..c3795a416 100644 --- a/main/src/main/java/io/split/android/client/SplitClientImpl.java +++ b/main/src/main/java/io/split/android/client/SplitClientImpl.java @@ -18,6 +18,7 @@ import io.split.android.client.events.SplitEventsManager; import io.split.android.client.impressions.ImpressionListener; import io.split.android.client.shared.SplitClientContainer; +import io.split.android.client.tracker.Tracker; import io.split.android.client.utils.logger.Logger; import io.split.android.client.validators.SplitValidator; import io.split.android.client.validators.TreatmentManager; @@ -35,7 +36,7 @@ public final class SplitClientImpl implements SplitClient { private final TreatmentManager mTreatmentManager; private final ValidationMessageLogger mValidationLogger; private final AttributesManager mAttributesManager; - private final EventsTracker mEventsTracker; + private final Tracker mEventsTracker; private static final double TRACK_DEFAULT_VALUE = 0.0; @@ -48,7 +49,7 @@ public SplitClientImpl(SplitFactory container, ImpressionListener impressionListener, SplitClientConfig config, SplitEventsManager eventsManager, - EventsTracker eventsTracker, + Tracker eventsTracker, AttributesManager attributesManager, SplitValidator splitValidator, TreatmentManager treatmentManager) { diff --git a/main/src/main/java/io/split/android/client/SplitFactoryImpl.java b/main/src/main/java/io/split/android/client/SplitFactoryImpl.java index 3cd4f501b..a79ae9376 100644 --- a/main/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/main/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -70,11 +70,15 @@ import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.TelemetrySynchronizer; +import io.split.android.client.dtos.Event; +import io.split.android.client.telemetry.model.Method; import io.split.android.client.telemetry.storage.TelemetryStorage; +import io.split.android.client.tracker.DefaultTracker; +import io.split.android.client.tracker.Tracker; +import io.split.android.client.tracker.TrackerEvent; import io.split.android.client.utils.logger.Logger; import io.split.android.client.validators.ApiKeyValidator; import io.split.android.client.validators.ApiKeyValidatorImpl; -import io.split.android.client.validators.EventValidator; import io.split.android.client.validators.EventValidatorImpl; import io.split.android.client.validators.KeyValidator; import io.split.android.client.validators.KeyValidatorImpl; @@ -545,7 +549,7 @@ public static class EventsTrackerProvider { private final SplitsStorage mSplitsStorage; private final TelemetryStorage mTelemetryStorage; private final SyncManager mSyncManager; - private volatile EventsTracker mEventsTracker; + private volatile Tracker mEventsTracker; public EventsTrackerProvider(SplitsStorage splitsStorage, TelemetryStorage telemetryStorage, SyncManager syncManager) { mSplitsStorage = splitsStorage; @@ -553,13 +557,26 @@ public EventsTrackerProvider(SplitsStorage splitsStorage, TelemetryStorage telem mSyncManager = syncManager; } - public EventsTracker getEventsTracker() { + public Tracker getEventsTracker() { if (mEventsTracker == null) { synchronized (this) { if (mEventsTracker == null) { - EventValidator eventsValidator = new EventValidatorImpl(new KeyValidatorImpl(), mSplitsStorage); - mEventsTracker = new EventsTrackerImpl(eventsValidator, new ValidationMessageLoggerImpl(), mTelemetryStorage, - new PropertyValidatorImpl(), mSyncManager); + mEventsTracker = new DefaultTracker( + new EventValidatorImpl(new KeyValidatorImpl(), mSplitsStorage), + new ValidationMessageLoggerImpl(), + new PropertyValidatorImpl(), + trackerEvent -> { + Event event = new Event(); + event.eventTypeId = trackerEvent.eventType; + event.trafficTypeName = trackerEvent.trafficType; + event.key = trackerEvent.key; + event.value = trackerEvent.value; + event.timestamp = trackerEvent.timestamp; + event.properties = trackerEvent.properties; + event.setSizeInBytes(trackerEvent.sizeInBytes); + mSyncManager.pushEvent(event); + }, + latencyMs -> mTelemetryStorage.recordLatency(Method.TRACK, latencyMs)); } } } diff --git a/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java b/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java index a189a3a02..61e0b8779 100644 --- a/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java +++ b/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java @@ -1,13 +1,17 @@ package io.split.android.client.validators; +import java.util.Map; + import io.split.android.client.dtos.Event; import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.client.tracker.TrackerEventValidator; +import io.split.android.client.tracker.TrackerValidationError; import io.split.android.client.utils.Utils; /** * Contains func an instance of Event class. */ -public class EventValidatorImpl implements EventValidator { +public class EventValidatorImpl implements EventValidator, TrackerEventValidator { private final String TYPE_REGEX = ValidationConfig.getInstance().getTrackEventNamePattern(); private KeyValidator mKeyValidator; @@ -69,4 +73,29 @@ public ValidationErrorInfo validate(Event event, boolean validateTrafficType) { return errorInfo; } + + @Override + public TrackerValidationError validate(String key, String trafficTypeName, String eventTypeId, + Double value, Map properties, boolean isSdkReady) { + Event event = new Event(); + event.key = key; + event.trafficTypeName = trafficTypeName; + event.eventTypeId = eventTypeId; + event.value = (value != null) ? value : 0.0; + event.properties = properties; + + ValidationErrorInfo result = validate(event, isSdkReady); + if (result == null) { + return null; + } + if (result.isError()) { + return new TrackerValidationError(true, result.getErrorMessage()); + } + StringBuilder warnings = new StringBuilder(); + for (String warning : result.getWarnings().values()) { + if (warnings.length() > 0) warnings.append("; "); + warnings.append(warning); + } + return new TrackerValidationError(false, warnings.toString()); + } } diff --git a/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java b/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java index c6678b276..b0ff8fa16 100644 --- a/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java +++ b/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java @@ -3,12 +3,14 @@ import java.util.ArrayList; import java.util.List; +import io.split.android.client.tracker.TrackerLogger; +import io.split.android.client.tracker.TrackerValidationError; import io.split.android.client.utils.logger.Logger; /** * Default implementation of ValidationMessageLogger interface */ -public class ValidationMessageLoggerImpl implements ValidationMessageLogger { +public class ValidationMessageLoggerImpl implements ValidationMessageLogger, TrackerLogger { @Override public void log(ValidationErrorInfo errorInfo, String tag) { @@ -52,4 +54,20 @@ private String sanitizeTag(String tag) { return (tag != null ? tag : ""); } + // TrackerLogger implementation + + @Override + public void log(TrackerValidationError errorInfo, String tag) { + if (errorInfo.isError()) { + logError(errorInfo.getMessage(), tag); + } else { + logWarning(errorInfo.getMessage(), tag); + } + } + + @Override + public void v(String message) { + Logger.v(message); + } + } diff --git a/main/src/test/java/io/split/android/client/SplitClientImplBaseTest.java b/main/src/test/java/io/split/android/client/SplitClientImplBaseTest.java index 88cd686ee..b89a4f6e4 100644 --- a/main/src/test/java/io/split/android/client/SplitClientImplBaseTest.java +++ b/main/src/test/java/io/split/android/client/SplitClientImplBaseTest.java @@ -14,6 +14,7 @@ import io.split.android.client.storage.mysegments.MySegmentsStorageContainer; import io.split.android.client.storage.rbs.RuleBasedSegmentStorage; import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.client.tracker.Tracker; import io.split.android.client.validators.SplitValidator; import io.split.android.client.validators.TreatmentManager; import io.split.android.engine.experiments.ParserCommons; @@ -41,7 +42,7 @@ public abstract class SplitClientImplBaseTest { @Mock protected SplitsStorage splitsStorage; @Mock - protected EventsTracker eventsTracker; + protected Tracker eventsTracker; @Mock protected SyncManager syncManager; @Mock diff --git a/main/src/test/java/io/split/android/client/SplitClientImplEventRegistrationTest.java b/main/src/test/java/io/split/android/client/SplitClientImplEventRegistrationTest.java index 16d40a060..539982fc0 100644 --- a/main/src/test/java/io/split/android/client/SplitClientImplEventRegistrationTest.java +++ b/main/src/test/java/io/split/android/client/SplitClientImplEventRegistrationTest.java @@ -22,6 +22,7 @@ import io.split.android.client.events.SplitEventsManager; import io.split.android.client.impressions.ImpressionListener; import io.split.android.client.shared.SplitClientContainer; +import io.split.android.client.tracker.Tracker; import io.split.android.client.utils.logger.Logger; import io.split.android.client.validators.SplitValidator; import io.split.android.client.validators.TreatmentManager; @@ -38,7 +39,7 @@ public class SplitClientImplEventRegistrationTest { @Mock private ImpressionListener impressionListener; @Mock - private EventsTracker eventsTracker; + private Tracker eventsTracker; @Mock private AttributesManager attributesManager; @Mock diff --git a/main/src/test/java/io/split/android/client/UserConsentManagerTest.java b/main/src/test/java/io/split/android/client/UserConsentManagerTest.java index 8dc3a2194..0d133342f 100644 --- a/main/src/test/java/io/split/android/client/UserConsentManagerTest.java +++ b/main/src/test/java/io/split/android/client/UserConsentManagerTest.java @@ -17,6 +17,7 @@ import io.split.android.client.shared.UserConsent; import io.split.android.client.storage.events.EventsStorage; import io.split.android.client.storage.impressions.ImpressionsStorage; +import io.split.android.client.tracker.Tracker; import io.split.android.fake.SplitTaskExecutorStub; public class UserConsentManagerTest { @@ -30,7 +31,7 @@ public class UserConsentManagerTest { @Mock private SyncManager mSyncManager; @Mock - private EventsTracker mEventsTracker; + private Tracker mEventsTracker; @Mock private SplitFactoryImpl.EventsTrackerProvider mEventsTrackerProvider; @Mock diff --git a/main/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java b/main/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java deleted file mode 100644 index bf0b601e6..000000000 --- a/main/src/test/java/io/split/android/client/service/events/EventsTrackerTest.java +++ /dev/null @@ -1,100 +0,0 @@ -package io.split.android.client.service.events; - -import static org.junit.Assert.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - -import io.split.android.client.EventsTracker; -import io.split.android.client.EventsTrackerImpl; -import io.split.android.client.ProcessedEventProperties; -import io.split.android.client.events.SplitEventsManager; -import io.split.android.client.service.synchronizer.SyncManager; -import io.split.android.client.telemetry.model.Method; -import io.split.android.client.telemetry.storage.TelemetryStorageProducer; -import io.split.android.client.validators.EventValidator; -import io.split.android.client.validators.PropertyValidator; -import io.split.android.client.validators.ValidationMessageLogger; - -public class EventsTrackerTest { - @Mock - private SplitEventsManager mEventsManager; - @Mock - private EventValidator mEventValidator; - @Mock - private ValidationMessageLogger mValidationLogger; - @Mock - private TelemetryStorageProducer mTelemetryStorageProducer; - @Mock - private PropertyValidator mPropertyValidator; - @Mock - private SyncManager mSyncManager; - - private EventsTracker mEventsTracker; - - @Before - public void setup() { - MockitoAnnotations.openMocks(this); - when(mEventValidator.validate(any(), anyBoolean())).thenReturn(null); - when(mEventsManager.eventAlreadyTriggered(any())).thenReturn(true); - when(mPropertyValidator.validate(any(), any())).thenReturn(PropertyValidator.Result.valid(null, 0)); - - mEventsTracker = new EventsTrackerImpl(mEventValidator, mValidationLogger, mTelemetryStorageProducer, - mPropertyValidator, mSyncManager); - } - - @Test - public void testTrackEnabled() throws InterruptedException { - trackingEnabledTest(true); - } - - @Test - public void testTrackDisabled() throws InterruptedException { - trackingEnabledTest(false); - } - - private void trackingEnabledTest(boolean enabled) throws InterruptedException { - mEventsTracker.enableTracking(enabled); - boolean res = mEventsTracker.track("pepe", "tt", null, 1.0, null, true); - Thread.sleep(500); - assertEquals(enabled, res); - if (enabled) { - verify(mSyncManager, times(1)).pushEvent(any()); - verify(mTelemetryStorageProducer, times(1)).recordLatency(Method.TRACK, 0L); - } else { - verify(mSyncManager, never()).pushEvent(any()); - verify(mTelemetryStorageProducer, never()).recordLatency(Method.TRACK, 0L); - } - } - - @Test - public void trackRecordsLatencyInEvaluationProducer() { - ProcessedEventProperties processedEventProperties = mock(ProcessedEventProperties.class); - when(processedEventProperties.isValid()).thenReturn(true); - mEventsTracker.track("any", "tt", "ev", 1, null, true); - - verify(mTelemetryStorageProducer).recordLatency(eq(Method.TRACK), anyLong()); - } - - @Test - public void trackRecordsExceptionInCaseThereIsOne() { - when(mPropertyValidator.validate(any(), any())).thenAnswer(invocation -> { - throw new Exception("test exception"); - }); - - mEventsTracker.track("event", "tt", "ev", 0, null, true); - - verify(mTelemetryStorageProducer).recordException(Method.TRACK); - } -} diff --git a/main/src/test/java/io/split/android/client/shared/SplitClientContainerImplTest.java b/main/src/test/java/io/split/android/client/shared/SplitClientContainerImplTest.java index 7ee312106..db0f905c3 100644 --- a/main/src/test/java/io/split/android/client/shared/SplitClientContainerImplTest.java +++ b/main/src/test/java/io/split/android/client/shared/SplitClientContainerImplTest.java @@ -28,7 +28,7 @@ import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; -import io.split.android.client.EventsTracker; +import io.split.android.client.tracker.Tracker; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitClientFactory; @@ -68,7 +68,7 @@ public class SplitClientContainerImplTest { private MySegmentsWorkManagerWrapper mWorkManagerWrapper; @Mock - private EventsTracker mEventsTracker; + private Tracker mEventsTracker; private final String mDefaultMatchingKey = "matching_key"; private SplitClientContainer mClientContainer; diff --git a/main/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java b/main/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java index 50fec3e8f..e2857760b 100644 --- a/main/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java +++ b/main/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java @@ -6,7 +6,7 @@ import java.util.Collections; -import io.split.android.client.EventsTracker; +import io.split.android.client.tracker.Tracker; import io.split.android.client.FlagSetsFilterImpl; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitClientImpl; @@ -53,7 +53,7 @@ false, new AttributesMergerImpl(), telemetryStorage, splitParser, new ImpressionListener.NoopImpressionListener(), cfg, eventsManager, - mock(EventsTracker.class), + mock(Tracker.class), attributesManager, mock(SplitValidator.class), treatmentManagerFactory.getTreatmentManager(key, eventsManager, attributesManager) @@ -74,7 +74,7 @@ public static SplitClientImpl get(Key key, ImpressionListener impressionListener impressionListener, cfg, new SplitEventsManager(new SplitTaskExecutorStub(), cfg.blockUntilReady()), - mock(EventsTracker.class), + mock(Tracker.class), mock(AttributesManager.class), mock(SplitValidator.class), mock(TreatmentManager.class) @@ -91,7 +91,7 @@ public static SplitClientImpl get(Key key, SplitEventsManager eventsManager) { new ImpressionListener.NoopImpressionListener(), SplitClientConfig.builder().build(), eventsManager, - mock(EventsTracker.class), + mock(Tracker.class), mock(AttributesManager.class), mock(SplitValidator.class), mock(TreatmentManager.class) diff --git a/tracker/README.md b/tracker/README.md new file mode 100644 index 000000000..a9e30cb63 --- /dev/null +++ b/tracker/README.md @@ -0,0 +1,44 @@ +# tracker + +Self-contained event-tracking module for the Split Android SDK. + +## Purpose + +Encapsulates the logic for validating and dispatching track events. It is intentionally decoupled from the SDK's internal networking, storage, and telemetry layers — dependencies are injected via callbacks. + +## Public API + +| Class / Interface | Role | +|---|---| +| `Tracker` | Primary interface. `enableTracking(boolean)` / `track(...)` | +| `DefaultTracker` | Default implementation | +| `TrackerEvent` | Domain object representing a validated event (no serialization concerns) | +| `TrackerEventValidator` | Validates key, traffic type, event type, value | +| `TrackerPropertyValidator` | Validates event properties; returns `TrackerPropertyResult` | +| `TrackerLogger` | Logging abstraction (`log`, `e`, `v`) | +| `TrackerValidationError` | Simple error/warning result (`isError`, `getMessage`) | + +## Wiring (in `main/`) + +`DefaultTracker` is wired in `SplitFactoryImpl.EventsTrackerProvider`: + +```java +new DefaultTracker( + new EventValidatorImpl(keyValidator, splitsStorage), // implements TrackerEventValidator + new ValidationMessageLoggerImpl(), // implements TrackerLogger + new PropertyValidatorImpl(), // implements TrackerPropertyValidator + trackerEvent -> { + // convert TrackerEvent → Event DTO, then push + mSyncManager.pushEvent(toEvent(trackerEvent)); + }, + latencyMs -> mTelemetryStorage.recordLatency(Method.TRACK, latencyMs) +); +``` + +The `onTrackLatency` callback is optional (pass `null` to skip telemetry). + +## Design notes + +- `TrackerEvent` is a plain domain object separate from the networking DTO (`Event` in `main/dtos/`). The caller converts between them in the `onEventPush` callback. +- Validator adapters (`EventValidatorImpl`, `PropertyValidatorImpl`, `ValidationMessageLoggerImpl`) implement both the original `main/` interfaces and the tracker interfaces, preserving existing behaviour. +- No dependency on `SyncManager`, `TelemetryStorageProducer`, or any `main/` internals. diff --git a/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java index 0b8d18982..b04317873 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java +++ b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java @@ -1,97 +1,94 @@ -package io.split.android.client; - -import static io.split.android.client.utils.Utils.checkNotNull; - -import androidx.annotation.NonNull; +package io.split.android.client.tracker; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import io.split.android.client.dtos.Event; -import io.split.android.client.service.synchronizer.SyncManager; -import io.split.android.client.telemetry.model.Method; -import io.split.android.client.telemetry.storage.TelemetryStorageProducer; -import io.split.android.client.utils.logger.Logger; -import io.split.android.client.validators.EventValidator; -import io.split.android.client.validators.PropertyValidator; -import io.split.android.client.validators.ValidationErrorInfo; -import io.split.android.client.validators.ValidationMessageLogger; - -public class EventsTrackerImpl implements EventsTracker { - // Estimated event size without properties - private final static int ESTIMATED_EVENT_SIZE_WITHOUT_PROPS = 1024; - - private final EventValidator mEventValidator; - private final ValidationMessageLogger mValidationLogger; - private final TelemetryStorageProducer mTelemetryStorageProducer; - private final PropertyValidator mPropertyValidator; - private final SyncManager mSyncManager; +public class DefaultTracker implements Tracker { + + // Estimated event size in bytes without properties + private static final int ESTIMATED_EVENT_SIZE_WITHOUT_PROPS = 1024; + + /** Callback invoked with the validated event when tracking succeeds. */ + public interface OnEventPush { + void accept(TrackerEvent event); + } + + /** Callback invoked with the track latency in milliseconds. May be null to skip telemetry. */ + public interface OnTrackLatency { + void accept(long latencyMs); + } + + private final TrackerEventValidator mEventValidator; + private final TrackerLogger mTrackerLogger; + private final TrackerPropertyValidator mPropertyValidator; + private final OnEventPush mOnEventPush; + private final OnTrackLatency mOnTrackLatency; private final AtomicBoolean isTrackingEnabled = new AtomicBoolean(true); - public EventsTrackerImpl(@NonNull EventValidator eventValidator, - @NonNull ValidationMessageLogger validationLogger, - @NonNull TelemetryStorageProducer telemetryStorageProducer, - @NonNull PropertyValidator eventPropertiesProcessor, - @NonNull SyncManager syncManager) { - - mEventValidator = checkNotNull(eventValidator); - mValidationLogger = checkNotNull(validationLogger); - mTelemetryStorageProducer = checkNotNull(telemetryStorageProducer); - mPropertyValidator = checkNotNull(eventPropertiesProcessor); - mSyncManager = checkNotNull(syncManager); + public DefaultTracker(TrackerEventValidator eventValidator, + TrackerLogger trackerLogger, + TrackerPropertyValidator propertyValidator, + OnEventPush onEventPush, + OnTrackLatency onTrackLatency) { + mEventValidator = eventValidator; + mTrackerLogger = trackerLogger; + mPropertyValidator = propertyValidator; + mOnEventPush = onEventPush; + mOnTrackLatency = onTrackLatency; } + @Override public void enableTracking(boolean enable) { isTrackingEnabled.set(enable); } + @Override public boolean track(String key, String trafficType, String eventType, double value, Map properties, boolean isSdkReady) { - if (!isTrackingEnabled.get()) { - Logger.v("Event not tracked because tracking is disabled"); + mTrackerLogger.v("Event not tracked because tracking is disabled"); return false; } try { final String validationTag = "track"; - Event event = new Event(); - event.eventTypeId = eventType; - event.trafficTypeName = trafficType; - event.key = key; - event.value = value; - event.timestamp = System.currentTimeMillis(); - event.properties = properties; - - ValidationErrorInfo errorInfo = mEventValidator.validate(event, isSdkReady); + TrackerValidationError errorInfo = mEventValidator.validate( + key, trafficType, eventType, value, properties, isSdkReady); if (errorInfo != null) { - if (errorInfo.isError()) { - mValidationLogger.e(errorInfo, validationTag); + mTrackerLogger.e(errorInfo.getMessage(), validationTag); return false; } - mValidationLogger.w(errorInfo, validationTag); - event.trafficTypeName = event.trafficTypeName.toLowerCase(); + mTrackerLogger.log(errorInfo, validationTag); + trafficType = trafficType.toLowerCase(); } - PropertyValidator.Result processedProperties = - mPropertyValidator.validate(event.properties, validationTag); + TrackerPropertyValidator.TrackerPropertyResult processedProperties = + mPropertyValidator.validate(properties, ESTIMATED_EVENT_SIZE_WITHOUT_PROPS, validationTag); if (!processedProperties.isValid()) { return false; } long startTime = System.currentTimeMillis(); + TrackerEvent event = new TrackerEvent(); + event.eventType = eventType; + event.trafficType = trafficType; + event.key = key; + event.value = value; + event.timestamp = System.currentTimeMillis(); event.properties = processedProperties.getProperties(); - event.setSizeInBytes(ESTIMATED_EVENT_SIZE_WITHOUT_PROPS + processedProperties.getSizeInBytes()); - mSyncManager.pushEvent(event); + event.sizeInBytes = processedProperties.getSizeInBytes(); + mOnEventPush.accept(event); - mTelemetryStorageProducer.recordLatency(Method.TRACK, System.currentTimeMillis() - startTime); + if (mOnTrackLatency != null) { + mOnTrackLatency.accept(System.currentTimeMillis() - startTime); + } return true; } catch (Exception exception) { - mTelemetryStorageProducer.recordException(Method.TRACK); + mTrackerLogger.e("Exception while tracking event: " + exception.getMessage(), "track"); } return false; } diff --git a/tracker/src/main/java/io/split/android/client/tracker/Tracker.java b/tracker/src/main/java/io/split/android/client/tracker/Tracker.java index 800b8c0c2..aa2d2401f 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/Tracker.java +++ b/tracker/src/main/java/io/split/android/client/tracker/Tracker.java @@ -1,8 +1,10 @@ -package io.split.android.client; +package io.split.android.client.tracker; import java.util.Map; -public interface EventsTracker { +public interface Tracker { void enableTracking(boolean enable); - boolean track(String key, String trafficType, String eventType, double value, Map properties, boolean isSdkReady); -} \ No newline at end of file + + boolean track(String key, String trafficType, String eventType, double value, + Map properties, boolean isSdkReady); +} diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerEvent.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerEvent.java new file mode 100644 index 000000000..dcdade61a --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerEvent.java @@ -0,0 +1,17 @@ +package io.split.android.client.tracker; + +import java.util.Map; + +/** + * Domain object representing a track event inside the tracker module. + * This is intentionally separate from the networking DTO (Event) used in main/. + */ +public class TrackerEvent { + public String trafficType; + public String eventType; + public String key; + public double value; + public long timestamp; + public Map properties; + public int sizeInBytes; +} diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerEventValidator.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerEventValidator.java new file mode 100644 index 000000000..a9d6285e3 --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerEventValidator.java @@ -0,0 +1,12 @@ +package io.split.android.client.tracker; + +import java.util.Map; + +/** + * Validates event parameters before tracking. + * Returns null if valid, or a {@link TrackerValidationError} with error/warning info. + */ +public interface TrackerEventValidator { + TrackerValidationError validate(String key, String trafficTypeName, String eventTypeId, + Double value, Map properties, boolean isSdkReady); +} diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerLogger.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerLogger.java new file mode 100644 index 000000000..bc8a46873 --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerLogger.java @@ -0,0 +1,15 @@ +package io.split.android.client.tracker; + +/** + * Logging abstraction for the tracker module. + */ +public interface TrackerLogger { + /** Log a validation result (error or warning) with a tag. */ + void log(TrackerValidationError errorInfo, String tag); + + /** Log an error message with a tag. */ + void e(String message, String tag); + + /** Log a verbose message. */ + void v(String message); +} diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerPropertyValidator.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerPropertyValidator.java new file mode 100644 index 000000000..2246109da --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerPropertyValidator.java @@ -0,0 +1,61 @@ +package io.split.android.client.tracker; + +import java.util.Map; + +/** + * Validates and processes event properties. + */ +public interface TrackerPropertyValidator { + + /** + * Validates event properties. + * + * @param properties raw properties map (may be null) + * @param initialSizeInBytes base event size in bytes (before properties), added to computed + * property size to produce the total in {@link TrackerPropertyResult#getSizeInBytes()} + * @param validationTag tag used for log messages + * @return validation result containing processed properties and total size + */ + TrackerPropertyResult validate(Map properties, int initialSizeInBytes, + String validationTag); + + class TrackerPropertyResult { + private final boolean mIsValid; + private final Map mProperties; + private final int mSizeInBytes; + private final String mErrorMessage; + + private TrackerPropertyResult(boolean isValid, Map properties, + int sizeInBytes, String errorMessage) { + mIsValid = isValid; + mProperties = properties; + mSizeInBytes = sizeInBytes; + mErrorMessage = errorMessage; + } + + public static TrackerPropertyResult valid(Map properties, int sizeInBytes) { + return new TrackerPropertyResult(true, properties, sizeInBytes, null); + } + + public static TrackerPropertyResult invalid(String errorMessage, int sizeInBytes) { + return new TrackerPropertyResult(false, null, sizeInBytes, errorMessage); + } + + public boolean isValid() { + return mIsValid; + } + + public Map getProperties() { + return mProperties; + } + + /** Total event size in bytes (initial base size + properties size). */ + public int getSizeInBytes() { + return mSizeInBytes; + } + + public String getErrorMessage() { + return mErrorMessage; + } + } +} diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java new file mode 100644 index 000000000..b8b25c2bd --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java @@ -0,0 +1,22 @@ +package io.split.android.client.tracker; + +/** + * Simple error/warning result from tracker validation. + */ +public class TrackerValidationError { + private final boolean mIsError; + private final String mMessage; + + public TrackerValidationError(boolean isError, String message) { + mIsError = isError; + mMessage = message; + } + + public boolean isError() { + return mIsError; + } + + public String getMessage() { + return mMessage; + } +} diff --git a/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java b/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java new file mode 100644 index 000000000..f8bb91b4c --- /dev/null +++ b/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java @@ -0,0 +1,164 @@ +package io.split.android.client.tracker; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyDouble; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.HashMap; +import java.util.Map; + +public class DefaultTrackerTest { + + @Mock + private TrackerEventValidator mEventValidator; + @Mock + private TrackerLogger mTrackerLogger; + @Mock + private TrackerPropertyValidator mPropertyValidator; + @Mock + private DefaultTracker.OnEventPush mOnEventPush; + @Mock + private DefaultTracker.OnTrackLatency mOnTrackLatency; + + private DefaultTracker mTracker; + + @Before + public void setUp() { + MockitoAnnotations.openMocks(this); + when(mEventValidator.validate(anyString(), anyString(), anyString(), anyDouble(), any(), anyBoolean())) + .thenReturn(null); + when(mPropertyValidator.validate(any(), anyInt(), anyString())) + .thenReturn(TrackerPropertyValidator.TrackerPropertyResult.valid(null, 0)); + + mTracker = new DefaultTracker(mEventValidator, mTrackerLogger, mPropertyValidator, + mOnEventPush, mOnTrackLatency); + } + + @Test + public void trackingEnabledByDefault() { + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertTrue(result); + verify(mOnEventPush).accept(any()); + } + + @Test + public void trackDisabledReturnsFalse() { + mTracker.enableTracking(false); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertFalse(result); + verify(mOnEventPush, never()).accept(any()); + } + + @Test + public void trackDisabledLogsVerbose() { + mTracker.enableTracking(false); + + mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + verify(mTrackerLogger).v("Event not tracked because tracking is disabled"); + } + + @Test + public void validationErrorBlocksTracking() { + when(mEventValidator.validate(anyString(), anyString(), anyString(), anyDouble(), any(), anyBoolean())) + .thenReturn(new TrackerValidationError(true, "bad event")); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertFalse(result); + verify(mTrackerLogger).e(eq("bad event"), anyString()); + verify(mOnEventPush, never()).accept(any()); + } + + @Test + public void validationWarningAllowsTracking() { + when(mEventValidator.validate(anyString(), anyString(), anyString(), anyDouble(), any(), anyBoolean())) + .thenReturn(new TrackerValidationError(false, "traffic type uppercase")); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertTrue(result); + verify(mTrackerLogger).log(any(TrackerValidationError.class), anyString()); + verify(mOnEventPush).accept(any()); + } + + @Test + public void validationWarningLowercasesTrafficType() { + when(mEventValidator.validate(anyString(), anyString(), anyString(), anyDouble(), any(), anyBoolean())) + .thenReturn(new TrackerValidationError(false, "traffic type has uppercase chars")); + + mTracker.track("key", "TRAFFIC", "eventType", 1.0, null, true); + + verify(mOnEventPush).accept(argThat(event -> "traffic".equals(event.trafficType))); + } + + @Test + public void propertyValidationErrorBlocksTracking() { + when(mPropertyValidator.validate(any(), anyInt(), anyString())) + .thenReturn(TrackerPropertyValidator.TrackerPropertyResult.invalid("too large", 0)); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, new HashMap<>(), true); + + assertFalse(result); + verify(mOnEventPush, never()).accept(any()); + } + + @Test + public void successfulTrackInvokesOnEventPush() { + Map props = new HashMap<>(); + props.put("k", "v"); + when(mPropertyValidator.validate(any(), anyInt(), anyString())) + .thenReturn(TrackerPropertyValidator.TrackerPropertyResult.valid(props, 1024)); + + boolean result = mTracker.track("key", "traffic", "eventType", 2.0, props, true); + + assertTrue(result); + verify(mOnEventPush).accept(any()); + } + + @Test + public void successfulTrackInvokesLatencyCallback() { + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertTrue(result); + verify(mOnTrackLatency).accept(any(Long.class)); + } + + @Test + public void nullLatencyCallbackDoesNotCrash() { + mTracker = new DefaultTracker(mEventValidator, mTrackerLogger, mPropertyValidator, + mOnEventPush, null); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertTrue(result); + verify(mOnEventPush).accept(any()); + } + + // Helper matcher for verifying TrackerEvent fields + private static T argThat(ArgumentMatcherWithReturn matcher) { + return org.mockito.ArgumentMatchers.argThat(matcher::matches); + } + + @FunctionalInterface + interface ArgumentMatcherWithReturn { + boolean matches(T argument); + } +} From 52cb0c9c93af4e18e79b0b40650bb6b56d0f1287 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Sat, 7 Mar 2026 19:58:25 -0300 Subject: [PATCH 03/15] Fix telemetry exception tracking --- .../android/client/SplitFactoryImpl.java | 3 +- .../client/tracker/DefaultTracker.java | 13 ++++++++- .../client/tracker/DefaultTrackerTest.java | 28 +++++++++++++++++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/main/src/main/java/io/split/android/client/SplitFactoryImpl.java b/main/src/main/java/io/split/android/client/SplitFactoryImpl.java index a79ae9376..e1898e6c2 100644 --- a/main/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/main/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -576,7 +576,8 @@ public Tracker getEventsTracker() { event.setSizeInBytes(trackerEvent.sizeInBytes); mSyncManager.pushEvent(event); }, - latencyMs -> mTelemetryStorage.recordLatency(Method.TRACK, latencyMs)); + latencyMs -> mTelemetryStorage.recordLatency(Method.TRACK, latencyMs), + () -> mTelemetryStorage.recordException(Method.TRACK)); } } } diff --git a/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java index b04317873..afca35830 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java +++ b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java @@ -18,23 +18,31 @@ public interface OnTrackLatency { void accept(long latencyMs); } + /** Callback invoked when an exception occurs during tracking. May be null to skip telemetry. */ + public interface OnTrackException { + void accept(); + } + private final TrackerEventValidator mEventValidator; private final TrackerLogger mTrackerLogger; private final TrackerPropertyValidator mPropertyValidator; private final OnEventPush mOnEventPush; private final OnTrackLatency mOnTrackLatency; + private final OnTrackException mOnTrackException; private final AtomicBoolean isTrackingEnabled = new AtomicBoolean(true); public DefaultTracker(TrackerEventValidator eventValidator, TrackerLogger trackerLogger, TrackerPropertyValidator propertyValidator, OnEventPush onEventPush, - OnTrackLatency onTrackLatency) { + OnTrackLatency onTrackLatency, + OnTrackException onTrackException) { mEventValidator = eventValidator; mTrackerLogger = trackerLogger; mPropertyValidator = propertyValidator; mOnEventPush = onEventPush; mOnTrackLatency = onTrackLatency; + mOnTrackException = onTrackException; } @Override @@ -89,6 +97,9 @@ public boolean track(String key, String trafficType, String eventType, return true; } catch (Exception exception) { mTrackerLogger.e("Exception while tracking event: " + exception.getMessage(), "track"); + if (mOnTrackException != null) { + mOnTrackException.accept(); + } } return false; } diff --git a/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java b/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java index f8bb91b4c..22195d4cf 100644 --- a/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java +++ b/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java @@ -8,6 +8,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -33,6 +34,8 @@ public class DefaultTrackerTest { private DefaultTracker.OnEventPush mOnEventPush; @Mock private DefaultTracker.OnTrackLatency mOnTrackLatency; + @Mock + private DefaultTracker.OnTrackException mOnTrackException; private DefaultTracker mTracker; @@ -45,7 +48,7 @@ public void setUp() { .thenReturn(TrackerPropertyValidator.TrackerPropertyResult.valid(null, 0)); mTracker = new DefaultTracker(mEventValidator, mTrackerLogger, mPropertyValidator, - mOnEventPush, mOnTrackLatency); + mOnEventPush, mOnTrackLatency, mOnTrackException); } @Test @@ -144,7 +147,7 @@ public void successfulTrackInvokesLatencyCallback() { @Test public void nullLatencyCallbackDoesNotCrash() { mTracker = new DefaultTracker(mEventValidator, mTrackerLogger, mPropertyValidator, - mOnEventPush, null); + mOnEventPush, null, null); boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); @@ -152,6 +155,27 @@ public void nullLatencyCallbackDoesNotCrash() { verify(mOnEventPush).accept(any()); } + @Test + public void exceptionDuringTrackingInvokesOnTrackException() { + doThrow(new RuntimeException("push failed")).when(mOnEventPush).accept(any()); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertFalse(result); + verify(mOnTrackException).accept(); + } + + @Test + public void nullExceptionCallbackDoesNotCrashOnException() { + mTracker = new DefaultTracker(mEventValidator, mTrackerLogger, mPropertyValidator, + mOnEventPush, null, null); + doThrow(new RuntimeException("push failed")).when(mOnEventPush).accept(any()); + + boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); + + assertFalse(result); + } + // Helper matcher for verifying TrackerEvent fields private static T argThat(ArgumentMatcherWithReturn matcher) { return org.mockito.ArgumentMatchers.argThat(matcher::matches); From db95576f67ba8b7d0db1caee84fe8b85623b587a Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Sat, 7 Mar 2026 20:18:37 -0300 Subject: [PATCH 04/15] Fix remaining bugs --- .../client/validators/EventValidatorImpl.java | 10 +++--- .../ValidationMessageLoggerImpl.java | 4 ++- tracker/build.gradle | 1 + .../client/tracker/DefaultTracker.java | 36 ++++++++++--------- .../tracker/TrackerValidationError.java | 17 +++++++++ .../client/tracker/DefaultTrackerTest.java | 33 +++++++++++++++-- 6 files changed, 76 insertions(+), 25 deletions(-) diff --git a/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java b/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java index 61e0b8779..88ab9404d 100644 --- a/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java +++ b/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java @@ -1,5 +1,7 @@ package io.split.android.client.validators; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import io.split.android.client.dtos.Event; @@ -91,11 +93,7 @@ public TrackerValidationError validate(String key, String trafficTypeName, Strin if (result.isError()) { return new TrackerValidationError(true, result.getErrorMessage()); } - StringBuilder warnings = new StringBuilder(); - for (String warning : result.getWarnings().values()) { - if (warnings.length() > 0) warnings.append("; "); - warnings.append(warning); - } - return new TrackerValidationError(false, warnings.toString()); + List warnings = new ArrayList<>(result.getWarnings().values()); + return new TrackerValidationError(warnings); } } diff --git a/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java b/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java index b0ff8fa16..a56866a28 100644 --- a/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java +++ b/main/src/main/java/io/split/android/client/validators/ValidationMessageLoggerImpl.java @@ -61,7 +61,9 @@ public void log(TrackerValidationError errorInfo, String tag) { if (errorInfo.isError()) { logError(errorInfo.getMessage(), tag); } else { - logWarning(errorInfo.getMessage(), tag); + for (String warning : errorInfo.getWarnings()) { + logWarning(warning, tag); + } } } diff --git a/tracker/build.gradle b/tracker/build.gradle index aa8bb68d5..0ba030834 100644 --- a/tracker/build.gradle +++ b/tracker/build.gradle @@ -14,6 +14,7 @@ android { } dependencies { + implementation libs.annotation testImplementation libs.junit4 testImplementation libs.mockitoCore } diff --git a/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java index afca35830..8e06a1e26 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java +++ b/tracker/src/main/java/io/split/android/client/tracker/DefaultTracker.java @@ -1,6 +1,10 @@ package io.split.android.client.tracker; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; public class DefaultTracker implements Tracker { @@ -23,24 +27,24 @@ public interface OnTrackException { void accept(); } - private final TrackerEventValidator mEventValidator; - private final TrackerLogger mTrackerLogger; - private final TrackerPropertyValidator mPropertyValidator; - private final OnEventPush mOnEventPush; - private final OnTrackLatency mOnTrackLatency; - private final OnTrackException mOnTrackException; + @NonNull private final TrackerEventValidator mEventValidator; + @NonNull private final TrackerLogger mTrackerLogger; + @NonNull private final TrackerPropertyValidator mPropertyValidator; + @NonNull private final OnEventPush mOnEventPush; + @Nullable private final OnTrackLatency mOnTrackLatency; + @Nullable private final OnTrackException mOnTrackException; private final AtomicBoolean isTrackingEnabled = new AtomicBoolean(true); - public DefaultTracker(TrackerEventValidator eventValidator, - TrackerLogger trackerLogger, - TrackerPropertyValidator propertyValidator, - OnEventPush onEventPush, - OnTrackLatency onTrackLatency, - OnTrackException onTrackException) { - mEventValidator = eventValidator; - mTrackerLogger = trackerLogger; - mPropertyValidator = propertyValidator; - mOnEventPush = onEventPush; + public DefaultTracker(@NonNull TrackerEventValidator eventValidator, + @NonNull TrackerLogger trackerLogger, + @NonNull TrackerPropertyValidator propertyValidator, + @NonNull OnEventPush onEventPush, + @Nullable OnTrackLatency onTrackLatency, + @Nullable OnTrackException onTrackException) { + mEventValidator = Objects.requireNonNull(eventValidator, "eventValidator must not be null"); + mTrackerLogger = Objects.requireNonNull(trackerLogger, "trackerLogger must not be null"); + mPropertyValidator = Objects.requireNonNull(propertyValidator, "propertyValidator must not be null"); + mOnEventPush = Objects.requireNonNull(onEventPush, "onEventPush must not be null"); mOnTrackLatency = onTrackLatency; mOnTrackException = onTrackException; } diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java index b8b25c2bd..1aa561991 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java @@ -1,15 +1,28 @@ package io.split.android.client.tracker; +import java.util.Collections; +import java.util.List; + /** * Simple error/warning result from tracker validation. */ public class TrackerValidationError { private final boolean mIsError; private final String mMessage; + private final List mWarnings; + /** Constructor for error results. */ public TrackerValidationError(boolean isError, String message) { mIsError = isError; mMessage = message; + mWarnings = Collections.emptyList(); + } + + /** Constructor for warning results with multiple individual warnings. */ + public TrackerValidationError(List warnings) { + mIsError = false; + mMessage = null; + mWarnings = (warnings != null) ? warnings : Collections.emptyList(); } public boolean isError() { @@ -19,4 +32,8 @@ public boolean isError() { public String getMessage() { return mMessage; } + + public List getWarnings() { + return mWarnings; + } } diff --git a/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java b/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java index 22195d4cf..4c2a9fb62 100644 --- a/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java +++ b/tracker/src/test/java/io/split/android/client/tracker/DefaultTrackerTest.java @@ -1,6 +1,8 @@ package io.split.android.client.tracker; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -19,6 +21,9 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.ArgumentCaptor; + +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -93,7 +98,7 @@ public void validationErrorBlocksTracking() { @Test public void validationWarningAllowsTracking() { when(mEventValidator.validate(anyString(), anyString(), anyString(), anyDouble(), any(), anyBoolean())) - .thenReturn(new TrackerValidationError(false, "traffic type uppercase")); + .thenReturn(new TrackerValidationError(Collections.singletonList("traffic type uppercase"))); boolean result = mTracker.track("key", "traffic", "eventType", 1.0, null, true); @@ -105,7 +110,7 @@ public void validationWarningAllowsTracking() { @Test public void validationWarningLowercasesTrafficType() { when(mEventValidator.validate(anyString(), anyString(), anyString(), anyDouble(), any(), anyBoolean())) - .thenReturn(new TrackerValidationError(false, "traffic type has uppercase chars")); + .thenReturn(new TrackerValidationError(Collections.singletonList("traffic type has uppercase chars"))); mTracker.track("key", "TRAFFIC", "eventType", 1.0, null, true); @@ -176,6 +181,30 @@ public void nullExceptionCallbackDoesNotCrashOnException() { assertFalse(result); } + @Test + public void successfulTrackPopulatesEventFieldsCorrectly() { + Map props = new HashMap<>(); + props.put("k", "v"); + when(mPropertyValidator.validate(any(), anyInt(), anyString())) + .thenReturn(TrackerPropertyValidator.TrackerPropertyResult.valid(props, 512)); + + long beforeTrack = System.currentTimeMillis(); + mTracker.track("myKey", "myTraffic", "myEventType", 3.14, props, true); + long afterTrack = System.currentTimeMillis(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(TrackerEvent.class); + verify(mOnEventPush).accept(captor.capture()); + + TrackerEvent captured = captor.getValue(); + assertNotNull(captured); + assertEquals("myKey", captured.key); + assertEquals("myTraffic", captured.trafficType); + assertEquals("myEventType", captured.eventType); + assertEquals(3.14, captured.value, 0.0001); + assertTrue(captured.timestamp >= beforeTrack && captured.timestamp <= afterTrack); + assertEquals(512, captured.sizeInBytes); + } + // Helper matcher for verifying TrackerEvent fields private static T argThat(ArgumentMatcherWithReturn matcher) { return org.mockito.ArgumentMatchers.argThat(matcher::matches); From b874994b0ad2915c678edbed2a1c3d923742039c Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 16 Mar 2026 15:05:14 -0300 Subject: [PATCH 05/15] Correct dependency --- main/build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main/build.gradle b/main/build.gradle index 45f456bdd..a0325264f 100644 --- a/main/build.gradle +++ b/main/build.gradle @@ -54,8 +54,8 @@ dependencies { api clientModuleProject('api') api clientModuleProject('http-api') api clientModuleProject('fallback') - api clientModuleProject('backoff') - api clientModuleProject('tracker') + implementation clientModuleProject('backoff') + implementation clientModuleProject('tracker') // Internal module dependencies implementation clientModuleProject('http') implementation clientModuleProject('events-domain') From 5f173854db8747a66a0550620d684b3c321b0cdd Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 12:03:38 -0300 Subject: [PATCH 06/15] Extracted TT validator --- .../android/client/SplitFactoryImpl.java | 11 +- .../localhost/LocalhostSplitClient.java | 7 +- .../LocalhostTrafficTypeValidator.java | 18 ++ .../client/validators/EventValidator.java | 17 - .../client/validators/EventValidatorImpl.java | 99 ------ .../validators/PropertyValidatorAdapter.java | 32 ++ .../validators/TrafficTypeValidatorImpl.java | 23 ++ .../TreatmentManagerFactoryImpl.java | 5 +- .../TreatmentManagerExceptionsTest.java | 4 +- .../client/TreatmentManagerTelemetryTest.java | 4 +- .../android/client/TreatmentManagerTest.java | 6 +- .../TreatmentManagerWithFlagSetsTest.java | 4 +- .../client/validators/EventValidatorTest.java | 305 ------------------ .../client/tracker/TrafficTypeValidator.java | 19 ++ .../client/validators/EventValidatorImpl.java | 73 +++++ .../client/validators/KeyValidator.java | 0 .../client/validators/KeyValidatorImpl.java | 6 +- .../validators}/PropertyValidatorImpl.java | 57 +++- .../client/validators/ValidationConfig.java | 0 .../validators/ValidationErrorInfo.java | 4 +- .../client/validators/ValidationUtils.java | 26 ++ .../validators/EventTypeNameHelper.java | 0 .../client/validators/EventValidatorTest.java | 198 ++++++++++++ .../client/validators/KeyValidatorTest.java | 14 +- .../validators}/PropertyValidatorTest.java | 38 ++- 25 files changed, 496 insertions(+), 474 deletions(-) create mode 100644 main/src/main/java/io/split/android/client/localhost/LocalhostTrafficTypeValidator.java delete mode 100644 main/src/main/java/io/split/android/client/validators/EventValidator.java delete mode 100644 main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java create mode 100644 main/src/main/java/io/split/android/client/validators/PropertyValidatorAdapter.java create mode 100644 main/src/main/java/io/split/android/client/validators/TrafficTypeValidatorImpl.java delete mode 100644 main/src/test/java/io/split/android/client/validators/EventValidatorTest.java create mode 100644 tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java create mode 100644 tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java rename {main => tracker}/src/main/java/io/split/android/client/validators/KeyValidator.java (100%) rename {main => tracker}/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java (90%) rename {main/src/main/java/io/split/android/client => tracker/src/main/java/io/split/android/client/validators}/PropertyValidatorImpl.java (51%) rename {main => tracker}/src/main/java/io/split/android/client/validators/ValidationConfig.java (100%) rename {main => tracker}/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java (91%) create mode 100644 tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java rename {main => tracker}/src/test/java/io/split/android/client/validators/EventTypeNameHelper.java (100%) create mode 100644 tracker/src/test/java/io/split/android/client/validators/EventValidatorTest.java rename {main => tracker}/src/test/java/io/split/android/client/validators/KeyValidatorTest.java (90%) rename {main/src/test/java/io/split/android/client/events => tracker/src/test/java/io/split/android/client/validators}/PropertyValidatorTest.java (57%) diff --git a/main/src/main/java/io/split/android/client/SplitFactoryImpl.java b/main/src/main/java/io/split/android/client/SplitFactoryImpl.java index e1898e6c2..e37792172 100644 --- a/main/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/main/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -82,7 +82,9 @@ import io.split.android.client.validators.EventValidatorImpl; import io.split.android.client.validators.KeyValidator; import io.split.android.client.validators.KeyValidatorImpl; +import io.split.android.client.validators.PropertyValidatorImpl; import io.split.android.client.validators.SplitValidatorImpl; +import io.split.android.client.validators.TrafficTypeValidatorImpl; import io.split.android.client.validators.ValidationConfig; import io.split.android.client.validators.ValidationErrorInfo; import io.split.android.client.validators.ValidationMessageLogger; @@ -562,9 +564,14 @@ public Tracker getEventsTracker() { synchronized (this) { if (mEventsTracker == null) { mEventsTracker = new DefaultTracker( - new EventValidatorImpl(new KeyValidatorImpl(), mSplitsStorage), + new EventValidatorImpl( + new KeyValidatorImpl(), + new TrafficTypeValidatorImpl(mSplitsStorage) + ), new ValidationMessageLoggerImpl(), - new PropertyValidatorImpl(), + new PropertyValidatorImpl( + new ValidationMessageLoggerImpl() + ), trackerEvent -> { Event event = new Event(); event.eventTypeId = trackerEvent.eventType; diff --git a/main/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java b/main/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java index 1b5e58499..5fb309e76 100644 --- a/main/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java +++ b/main/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java @@ -16,7 +16,8 @@ import io.split.android.client.EvaluationOptions; import io.split.android.client.EvaluatorImpl; import io.split.android.client.FlagSetsFilter; -import io.split.android.client.PropertyValidatorImpl; +import io.split.android.client.validators.PropertyValidatorImpl; +import io.split.android.client.validators.PropertyValidatorAdapter; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; @@ -87,7 +88,9 @@ public LocalhostSplitClient(@NonNull LocalhostSplitFactory container, new SplitValidatorImpl(), getImpressionsListener(splitClientConfig), splitClientConfig.labelsEnabled(), eventsManager, attributesManager, attributesMerger, telemetryStorageProducer, flagSetsFilter, splitsStorage, new ValidationMessageLoggerImpl(), new FlagSetsValidatorImpl(), - new PropertyValidatorImpl(), mFallbackTreatmentsCalculator); + new PropertyValidatorAdapter( + new PropertyValidatorImpl(new ValidationMessageLoggerImpl())), + mFallbackTreatmentsCalculator); } @Override diff --git a/main/src/main/java/io/split/android/client/localhost/LocalhostTrafficTypeValidator.java b/main/src/main/java/io/split/android/client/localhost/LocalhostTrafficTypeValidator.java new file mode 100644 index 000000000..2b4ed2010 --- /dev/null +++ b/main/src/main/java/io/split/android/client/localhost/LocalhostTrafficTypeValidator.java @@ -0,0 +1,18 @@ +package io.split.android.client.localhost; + +import io.split.android.client.tracker.TrafficTypeValidator; + +/** + * Traffic type validator for localhost mode. + *

+ * In localhost mode, all traffic types are considered valid since we're not + * connected to the Split backend and can't validate against real feature flags. + */ +public class LocalhostTrafficTypeValidator implements TrafficTypeValidator { + + @Override + public boolean isValid(String trafficTypeName) { + // In localhost mode, accept all traffic types + return true; + } +} diff --git a/main/src/main/java/io/split/android/client/validators/EventValidator.java b/main/src/main/java/io/split/android/client/validators/EventValidator.java deleted file mode 100644 index a1bd81220..000000000 --- a/main/src/main/java/io/split/android/client/validators/EventValidator.java +++ /dev/null @@ -1,17 +0,0 @@ -package io.split.android.client.validators; - -import io.split.android.client.dtos.Event; - -/** - * Interface to implement by Track Events validators - */ -public interface EventValidator { - - /** - * Checks that a Track event is valid - * @param event: Event instance - * @return true when the key is valid, false when it is not - */ - ValidationErrorInfo validate(Event event, boolean validateTrafficType); - -} diff --git a/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java b/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java deleted file mode 100644 index 88ab9404d..000000000 --- a/main/src/main/java/io/split/android/client/validators/EventValidatorImpl.java +++ /dev/null @@ -1,99 +0,0 @@ -package io.split.android.client.validators; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -import io.split.android.client.dtos.Event; -import io.split.android.client.storage.splits.SplitsStorage; -import io.split.android.client.tracker.TrackerEventValidator; -import io.split.android.client.tracker.TrackerValidationError; -import io.split.android.client.utils.Utils; - -/** - * Contains func an instance of Event class. - */ -public class EventValidatorImpl implements EventValidator, TrackerEventValidator { - - private final String TYPE_REGEX = ValidationConfig.getInstance().getTrackEventNamePattern(); - private KeyValidator mKeyValidator; - private final SplitsStorage mSplitsStorage; - - public EventValidatorImpl(KeyValidator keyValidator, SplitsStorage splitsStorage) { - mKeyValidator = keyValidator; - mSplitsStorage = splitsStorage; - } - - @Override - public ValidationErrorInfo validate(Event event, boolean validateTrafficType) { - - if(event == null) { - return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "Event could not be null"); - } - - ValidationErrorInfo errorInfo = mKeyValidator.validate(event.key, null); - if(errorInfo != null){ - return errorInfo; - } - - if (event.trafficTypeName == null) { - return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed a null or undefined traffic_type_name, traffic_type_name must be a non-empty string"); - } - - if (Utils.isNullOrEmpty(event.trafficTypeName.trim())) { - return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed an empty traffic_type_name, traffic_type_name must be a non-empty string"); - } - - if (event.eventTypeId == null) { - return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed a null or undefined event_type, event_type must be a non-empty String"); - } - - if (Utils.isNullOrEmpty(event.eventTypeId.trim())) { - return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed an empty event_type, event_type must be a non-empty String"); - } - - if (!event.eventTypeId.matches(TYPE_REGEX)) { - return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed " + event.eventTypeId - + ", event name must adhere to the regular expression " + TYPE_REGEX - + ". This means an event name must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, " - + " underscore, period, or colon as separators of alphanumeric characters."); - } - - if(!event.trafficTypeName.toLowerCase().equals(event.trafficTypeName)) { - errorInfo = new ValidationErrorInfo(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_HAS_UPPERCASE_CHARS, "traffic_type_name should be all lowercase - converting string to lowercase", true); - } - - if (validateTrafficType && !mSplitsStorage.isValidTrafficType(event.trafficTypeName)) { - String message = "Traffic Type " + event.trafficTypeName + " does not have any corresponding feature flags in this environment, " - + "make sure you’re tracking your events to a valid traffic type defined in the Split user interface"; - if(errorInfo == null) { - errorInfo = new ValidationErrorInfo(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_WITHOUT_SPLIT_IN_ENVIRONMENT, message, true); - } else { - errorInfo.addWarning(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_WITHOUT_SPLIT_IN_ENVIRONMENT, message); - } - } - - return errorInfo; - } - - @Override - public TrackerValidationError validate(String key, String trafficTypeName, String eventTypeId, - Double value, Map properties, boolean isSdkReady) { - Event event = new Event(); - event.key = key; - event.trafficTypeName = trafficTypeName; - event.eventTypeId = eventTypeId; - event.value = (value != null) ? value : 0.0; - event.properties = properties; - - ValidationErrorInfo result = validate(event, isSdkReady); - if (result == null) { - return null; - } - if (result.isError()) { - return new TrackerValidationError(true, result.getErrorMessage()); - } - List warnings = new ArrayList<>(result.getWarnings().values()); - return new TrackerValidationError(warnings); - } -} diff --git a/main/src/main/java/io/split/android/client/validators/PropertyValidatorAdapter.java b/main/src/main/java/io/split/android/client/validators/PropertyValidatorAdapter.java new file mode 100644 index 000000000..4406cd4ee --- /dev/null +++ b/main/src/main/java/io/split/android/client/validators/PropertyValidatorAdapter.java @@ -0,0 +1,32 @@ +package io.split.android.client.validators; + +import java.util.Map; + +import io.split.android.client.tracker.TrackerLogger; +import io.split.android.client.tracker.TrackerPropertyValidator; + +/** + * Adapter that bridges the main module's PropertyValidator interface with + * the tracker module's TrackerPropertyValidator implementation. + */ +public class PropertyValidatorAdapter implements PropertyValidator { + + private final TrackerPropertyValidator mDelegate; + + public PropertyValidatorAdapter(TrackerPropertyValidator delegate) { + mDelegate = delegate; + } + + @Override + public Result validate(Map properties, String validationTag) { + // Call the tracker validator with initialSizeInBytes=0 since we're not tracking + TrackerPropertyValidator.TrackerPropertyResult trackerResult = + mDelegate.validate(properties, 0, validationTag); + + if (trackerResult.isValid()) { + return Result.valid(trackerResult.getProperties(), trackerResult.getSizeInBytes()); + } else { + return Result.invalid(trackerResult.getErrorMessage(), trackerResult.getSizeInBytes()); + } + } +} diff --git a/main/src/main/java/io/split/android/client/validators/TrafficTypeValidatorImpl.java b/main/src/main/java/io/split/android/client/validators/TrafficTypeValidatorImpl.java new file mode 100644 index 000000000..a46d998b8 --- /dev/null +++ b/main/src/main/java/io/split/android/client/validators/TrafficTypeValidatorImpl.java @@ -0,0 +1,23 @@ +package io.split.android.client.validators; + +import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.client.tracker.TrafficTypeValidator; + +/** + * Implementation of {@link TrafficTypeValidator} that delegates to {@link SplitsStorage}. + *

+ * This implementation validates traffic type names by checking if they exist in the + * Split storage. It bridges the tracker module's abstraction with the SDK's storage layer. + */ +public class TrafficTypeValidatorImpl implements TrafficTypeValidator { + private final SplitsStorage mSplitsStorage; + + public TrafficTypeValidatorImpl(SplitsStorage splitsStorage) { + mSplitsStorage = splitsStorage; + } + + @Override + public boolean isValid(String trafficTypeName) { + return mSplitsStorage.isValidTrafficType(trafficTypeName); + } +} diff --git a/main/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java b/main/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java index 287fb94b4..28d54a578 100644 --- a/main/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java +++ b/main/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java @@ -8,7 +8,7 @@ import io.split.android.client.Evaluator; import io.split.android.client.EvaluatorImpl; import io.split.android.client.FlagSetsFilter; -import io.split.android.client.PropertyValidatorImpl; +import io.split.android.client.validators.PropertyValidatorImpl; import io.split.android.client.api.Key; import io.split.android.client.attributes.AttributesManager; import io.split.android.client.attributes.AttributesMerger; @@ -65,7 +65,8 @@ public TreatmentManagerFactoryImpl(@NonNull KeyValidator keyValidator, mSplitsStorage = checkNotNull(splitsStorage); mValidationMessageLogger = new ValidationMessageLoggerImpl(); mFlagSetsValidator = new FlagSetsValidatorImpl(); - mPropertyValidator = new PropertyValidatorImpl(); + mPropertyValidator = new PropertyValidatorAdapter( + new PropertyValidatorImpl(new ValidationMessageLoggerImpl())); } @Override diff --git a/main/src/test/java/io/split/android/client/TreatmentManagerExceptionsTest.java b/main/src/test/java/io/split/android/client/TreatmentManagerExceptionsTest.java index 4f8432e18..3d06f2f89 100644 --- a/main/src/test/java/io/split/android/client/TreatmentManagerExceptionsTest.java +++ b/main/src/test/java/io/split/android/client/TreatmentManagerExceptionsTest.java @@ -1,5 +1,7 @@ package io.split.android.client; +import io.split.android.client.validators.PropertyValidatorAdapter; +import io.split.android.client.validators.PropertyValidatorImpl; import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; @@ -85,7 +87,7 @@ public void setUp() { mSplitsStorage, new ValidationMessageLoggerImpl(), mFlagSetsValidator, - new PropertyValidatorImpl(), + new PropertyValidatorAdapter(new PropertyValidatorImpl(new ValidationMessageLoggerImpl())), new FallbackTreatmentsCalculatorImpl(FallbackTreatmentsConfiguration.builder().build())); when(evaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); diff --git a/main/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java b/main/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java index 222de7750..c7a3e0ec6 100644 --- a/main/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java +++ b/main/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java @@ -1,5 +1,7 @@ package io.split.android.client; +import io.split.android.client.validators.PropertyValidatorAdapter; +import io.split.android.client.validators.PropertyValidatorImpl; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; @@ -78,7 +80,7 @@ public void setUp() { mFlagSetsFilter, mSplitsStorage, new ValidationMessageLoggerImpl(), new FlagSetsValidatorImpl(), - new PropertyValidatorImpl(), + new PropertyValidatorAdapter(new PropertyValidatorImpl(new ValidationMessageLoggerImpl())), new FallbackTreatmentsCalculatorImpl(FallbackTreatmentsConfiguration.builder().build())); when(evaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); diff --git a/main/src/test/java/io/split/android/client/TreatmentManagerTest.java b/main/src/test/java/io/split/android/client/TreatmentManagerTest.java index ce889d69d..6a2dd7988 100644 --- a/main/src/test/java/io/split/android/client/TreatmentManagerTest.java +++ b/main/src/test/java/io/split/android/client/TreatmentManagerTest.java @@ -1,5 +1,7 @@ package io.split.android.client; +import io.split.android.client.validators.PropertyValidatorAdapter; +import io.split.android.client.validators.PropertyValidatorImpl; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.argThat; @@ -372,7 +374,7 @@ private TreatmentManager createTreatmentManager(String matchingKey, String bucke new KeyValidatorImpl(), splitValidator, mock(ImpressionListener.FederatedImpressionListener.class), config.labelsEnabled(), eventsManager, mock(AttributesManager.class), mock(AttributesMerger.class), - mock(TelemetryStorageProducer.class), mFlagSetsFilter, mSplitsStorage, validationLogger, new FlagSetsValidatorImpl(), new PropertyValidatorImpl(), + mock(TelemetryStorageProducer.class), mFlagSetsFilter, mSplitsStorage, validationLogger, new FlagSetsValidatorImpl(), new PropertyValidatorAdapter(new PropertyValidatorImpl(new ValidationMessageLoggerImpl())), new FallbackTreatmentsCalculatorImpl(FallbackTreatmentsConfiguration.builder().build())); } @@ -403,7 +405,7 @@ private TreatmentManagerImpl initializeTreatmentManager(Evaluator evaluator) { telemetryStorageProducer, mFlagSetsFilter, mSplitsStorage, - new ValidationMessageLoggerImpl(), new FlagSetsValidatorImpl(), new PropertyValidatorImpl(), + new ValidationMessageLoggerImpl(), new FlagSetsValidatorImpl(), new PropertyValidatorAdapter(new PropertyValidatorImpl(new ValidationMessageLoggerImpl())), new FallbackTreatmentsCalculatorImpl(FallbackTreatmentsConfiguration.builder().build())); } diff --git a/main/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java b/main/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java index aa12c3d5e..8d51f2263 100644 --- a/main/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java +++ b/main/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java @@ -1,5 +1,7 @@ package io.split.android.client; +import io.split.android.client.validators.PropertyValidatorAdapter; +import io.split.android.client.validators.PropertyValidatorImpl; import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -157,7 +159,7 @@ private void initializeTreatmentManager() { mAttributesMerger, mTelemetryStorageProducer, mFlagSetsFilter, - mSplitsStorage, new ValidationMessageLoggerImpl(), new FlagSetsValidatorImpl(), new PropertyValidatorImpl(), + mSplitsStorage, new ValidationMessageLoggerImpl(), new FlagSetsValidatorImpl(), new PropertyValidatorAdapter(new PropertyValidatorImpl(new ValidationMessageLoggerImpl())), new FallbackTreatmentsCalculatorImpl(FallbackTreatmentsConfiguration.builder().build())); } diff --git a/main/src/test/java/io/split/android/client/validators/EventValidatorTest.java b/main/src/test/java/io/split/android/client/validators/EventValidatorTest.java deleted file mode 100644 index 7f1e033da..000000000 --- a/main/src/test/java/io/split/android/client/validators/EventValidatorTest.java +++ /dev/null @@ -1,305 +0,0 @@ -package io.split.android.client.validators; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - -import io.split.android.client.dtos.Event; -import io.split.android.client.dtos.Split; -import io.split.android.client.dtos.Status; -import io.split.android.client.storage.splits.SplitsStorage; -import io.split.android.client.utils.Utils; - -public class EventValidatorTest { - - private EventValidator validator; - - @Before - public void setUp() { - - SplitsStorage splitsStorage = mock(SplitsStorage.class); - - when(splitsStorage.isValidTrafficType("traffic1")).thenReturn(true); - when(splitsStorage.isValidTrafficType("trafficType1")).thenReturn(true); - when(splitsStorage.isValidTrafficType("custom")).thenReturn(true); - - validator = new EventValidatorImpl(new KeyValidatorImpl(), splitsStorage); - } - - @Test - public void testValidEventAllValues() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "traffic1"; - event.key = "pepe"; - event.value = 1.0; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNull(errorInfo); - } - - @Test - public void testValidEventNullValue() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "traffic1"; - event.key = "pepe"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNull(errorInfo); - } - - @Test - public void testNullKey() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "traffic1"; - event.key = null; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed a null key, matching key must be a non-empty string", errorInfo.getErrorMessage()); - } - - @Test - public void testEmptyKey() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "traffic1"; - event.key = ""; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed an empty string, matching key must be a non-empty string", errorInfo.getErrorMessage()); - } - - @Test - public void testAllSpacesInKey() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "traffic1"; - event.key = " "; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed an empty string, matching key must be a non-empty string", errorInfo.getErrorMessage()); - } - - @Test - public void testLongKey() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "traffic1"; - event.key = Utils.repeat("p", 300); - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("matching key too long - must be " + ValidationConfig.getInstance().getMaximumKeyLength() + " characters or less", errorInfo.getErrorMessage()); - } - - @Test - public void testNullType() { - Event event = new Event(); - event.eventTypeId = null; - event.trafficTypeName = "traffic1"; - event.key = "key1"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed a null or undefined event_type, event_type must be a non-empty String", errorInfo.getErrorMessage()); - } - - @Test - public void testEmptyType() { - Event event = new Event(); - event.eventTypeId = ""; - event.trafficTypeName = "traffic1"; - event.key = "key1"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed an empty event_type, event_type must be a non-empty String", errorInfo.getErrorMessage()); - } - - @Test - public void testAllSpacesInType() { - Event event = new Event(); - event.eventTypeId = " "; - event.trafficTypeName = "traffic1"; - event.key = "key1"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed an empty event_type, event_type must be a non-empty String", errorInfo.getErrorMessage()); - } - - @Test - public void testTypeName() { - - EventTypeNameHelper nameHelper = new EventTypeNameHelper(); - Event event1 = newEventTypeName(); - Event event2 = newEventTypeName(); - Event event3 = newEventTypeName(); - Event event4 = newEventTypeName(); - Event event5 = newEventTypeName(); - - event1.eventTypeId = nameHelper.getValidAllValidChars(); - event2.eventTypeId = nameHelper.getValidStartNumber(); - event3.eventTypeId = nameHelper.getInvalidChars(); - event4.eventTypeId = nameHelper.getInvalidUndercoreStart(); - event5.eventTypeId = nameHelper.getInvalidHypenStart(); - - ValidationErrorInfo errorInfo1 = validator.validate(event1, true); - ValidationErrorInfo errorInfo2 = validator.validate(event2, true); - ValidationErrorInfo errorInfo3 = validator.validate(event3, true); - ValidationErrorInfo errorInfo4 = validator.validate(event4, true); - ValidationErrorInfo errorInfo5 = validator.validate(event5, true); - - Assert.assertNull(errorInfo1); - - Assert.assertNull(errorInfo2); - - Assert.assertNotNull(errorInfo3); - Assert.assertTrue(errorInfo3.isError()); - Assert.assertEquals(buildEventTypeValidationMessage(event3.eventTypeId), errorInfo3.getErrorMessage()); - - Assert.assertNotNull(errorInfo4); - Assert.assertTrue(errorInfo4.isError()); - Assert.assertEquals(buildEventTypeValidationMessage(event4.eventTypeId), errorInfo4.getErrorMessage()); - - Assert.assertNotNull(errorInfo5); - Assert.assertTrue(errorInfo5.isError()); - Assert.assertEquals(buildEventTypeValidationMessage(event5.eventTypeId), errorInfo5.getErrorMessage()); - } - - @Test - public void testNullTrafficType() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = null; - event.key = "key1"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed a null or undefined traffic_type_name, traffic_type_name must be a non-empty string", errorInfo.getErrorMessage()); - } - - @Test - public void testEmptyTrafficType() { - - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = ""; - event.key = "key1"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed an empty traffic_type_name, traffic_type_name must be a non-empty string", errorInfo.getErrorMessage()); - } - - @Test - public void testAllSpacesInTrafficType() { - - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = " "; - event.key = "key1"; - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertTrue(errorInfo.isError()); - Assert.assertEquals("you passed an empty traffic_type_name, traffic_type_name must be a non-empty string", errorInfo.getErrorMessage()); - } - - @Test - public void testUppercaseCharsInTrafficType() { - - Event event0 = newEventUppercase(); - Event event1 = newEventUppercase(); - Event event2 = newEventUppercase(); - Event event3 = newEventUppercase(); - - final String uppercaseMessage = "traffic_type_name should be all lowercase - converting string to lowercase"; - - event0.trafficTypeName = "custom"; - event1.trafficTypeName = "Custom"; - event2.trafficTypeName = "cUSTom"; - event3.trafficTypeName = "custoM"; - - ValidationErrorInfo errorInfo0 = validator.validate(event0, true); - ValidationErrorInfo errorInfo1 = validator.validate(event1, true); - ValidationErrorInfo errorInfo2 = validator.validate(event2, true); - ValidationErrorInfo errorInfo3 = validator.validate(event3, true); - - - Assert.assertNull(errorInfo0); - - Assert.assertNotNull(errorInfo1); - Assert.assertFalse(errorInfo1.isError()); - Assert.assertEquals(uppercaseMessage, errorInfo1.getWarnings().get(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_HAS_UPPERCASE_CHARS)); - - Assert.assertNotNull(errorInfo2); - Assert.assertFalse(errorInfo2.isError()); - Assert.assertEquals(uppercaseMessage, errorInfo2.getWarnings().get(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_HAS_UPPERCASE_CHARS)); - - Assert.assertNotNull(errorInfo3); - Assert.assertFalse(errorInfo3.isError()); - Assert.assertEquals(uppercaseMessage, errorInfo3.getWarnings().get(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_HAS_UPPERCASE_CHARS)); - } - - @Test - public void noChachedServerTrafficType() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.trafficTypeName = "nocached"; - event.key = "key1"; - - ValidationErrorInfo errorInfo = validator.validate(event, true); - - Assert.assertNotNull(errorInfo); - Assert.assertFalse(errorInfo.isError()); - Assert.assertEquals("Traffic Type nocached does not have any corresponding feature flags in this environment, " - + "make sure you’re tracking your events to a valid traffic type defined in the Split user interface", errorInfo.getWarnings().get(ValidationErrorInfo.WARNING_TRAFFIC_TYPE_WITHOUT_SPLIT_IN_ENVIRONMENT)); - } - - private Event newEventTypeName() { - Event event = new Event(); - event.trafficTypeName = "traffic1"; - event.key = "key1"; - return event; - } - - private Event newEventUppercase() { - Event event = new Event(); - event.eventTypeId = "type1"; - event.key = "key1"; - return event; - } - - private String buildEventTypeValidationMessage(String eventType) { - return "you passed " + eventType - + ", event name must adhere to the regular expression " + ValidationConfig.getInstance().getTrackEventNamePattern() - + ". This means an event name must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, " - + " underscore, period, or colon as separators of alphanumeric characters."; - } - - private Split newSplit(String name, String trafficType) { - Split split = new Split(); - split.name = name; - split.trafficTypeName = trafficType; - split.status = Status.ACTIVE; - return split; - } -} diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java b/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java new file mode 100644 index 000000000..f004ca4b5 --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java @@ -0,0 +1,19 @@ +package io.split.android.client.tracker; + +/** + * Interface for validating traffic type names. + *

+ * This abstraction allows different implementations: + * - Production: delegates to SplitsStorage to check if traffic type exists + * - Localhost: always returns true (any traffic type is valid) + * - Testing: fully mockable + */ +public interface TrafficTypeValidator { + /** + * Checks if the given traffic type name is valid. + * + * @param trafficTypeName the traffic type name to validate + * @return true if the traffic type is valid, false otherwise + */ + boolean isValid(String trafficTypeName); +} diff --git a/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java b/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java new file mode 100644 index 000000000..9b61748e6 --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java @@ -0,0 +1,73 @@ +package io.split.android.client.validators; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import io.split.android.client.tracker.TrafficTypeValidator; +import io.split.android.client.tracker.TrackerEventValidator; +import io.split.android.client.tracker.TrackerValidationError; + +/** + * Event validator implementation for the tracker module. + */ +public class EventValidatorImpl implements TrackerEventValidator { + + private final String TYPE_REGEX = ValidationConfig.getInstance().getTrackEventNamePattern(); + private KeyValidator mKeyValidator; + private final TrafficTypeValidator mTrafficTypeValidator; + + public EventValidatorImpl(KeyValidator keyValidator, TrafficTypeValidator trafficTypeValidator) { + mKeyValidator = keyValidator; + mTrafficTypeValidator = trafficTypeValidator; + } + + @Override + public TrackerValidationError validate(String key, String trafficTypeName, String eventTypeId, + Double value, Map properties, boolean isSdkReady) { + ValidationErrorInfo errorInfo = mKeyValidator.validate(key, null); + if(errorInfo != null){ + return new TrackerValidationError(true, errorInfo.getErrorMessage()); + } + + if (trafficTypeName == null) { + return new TrackerValidationError(true, "you passed a null or undefined traffic_type_name, traffic_type_name must be a non-empty string"); + } + + if (ValidationUtils.isNullOrEmpty(trafficTypeName.trim())) { + return new TrackerValidationError(true, "you passed an empty traffic_type_name, traffic_type_name must be a non-empty string"); + } + + if (eventTypeId == null) { + return new TrackerValidationError(true, "you passed a null or undefined event_type, event_type must be a non-empty String"); + } + + if (ValidationUtils.isNullOrEmpty(eventTypeId.trim())) { + return new TrackerValidationError(true, "you passed an empty event_type, event_type must be a non-empty String"); + } + + if (!eventTypeId.matches(TYPE_REGEX)) { + return new TrackerValidationError(true, "you passed " + eventTypeId + + ", event name must adhere to the regular expression " + TYPE_REGEX + + ". This means an event name must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, " + + " underscore, period, or colon as separators of alphanumeric characters."); + } + + List warnings = new ArrayList<>(); + + if(!trafficTypeName.toLowerCase().equals(trafficTypeName)) { + warnings.add("traffic_type_name should be all lowercase - converting string to lowercase"); + } + + if (isSdkReady && !mTrafficTypeValidator.isValid(trafficTypeName)) { + String message = "Traffic Type " + trafficTypeName + " does not have any corresponding feature flags in this environment, " + + "make sure you’re tracking your events to a valid traffic type defined in the Split user interface"; + warnings.add(message); + } + + if (warnings.isEmpty()) { + return null; + } + return new TrackerValidationError(warnings); + } +} diff --git a/main/src/main/java/io/split/android/client/validators/KeyValidator.java b/tracker/src/main/java/io/split/android/client/validators/KeyValidator.java similarity index 100% rename from main/src/main/java/io/split/android/client/validators/KeyValidator.java rename to tracker/src/main/java/io/split/android/client/validators/KeyValidator.java diff --git a/main/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java b/tracker/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java similarity index 90% rename from main/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java rename to tracker/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java index c22a8daa5..fcdf0d931 100644 --- a/main/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java +++ b/tracker/src/main/java/io/split/android/client/validators/KeyValidatorImpl.java @@ -1,7 +1,5 @@ package io.split.android.client.validators; -import io.split.android.client.utils.Utils; - /** * Validates an instance of Key class. */ @@ -17,7 +15,7 @@ public ValidationErrorInfo validate(String matchingKey, String bucketingKey) { return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed a null key, matching key must be a non-empty string"); } - if (Utils.isNullOrEmpty(matchingKey.trim())) { + if (ValidationUtils.isNullOrEmpty(matchingKey.trim())) { return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME,"you passed an empty string, matching key must be a non-empty string"); } @@ -26,7 +24,7 @@ public ValidationErrorInfo validate(String matchingKey, String bucketingKey) { } if (bucketingKey != null) { - if (Utils.isNullOrEmpty(bucketingKey.trim())) { + if (ValidationUtils.isNullOrEmpty(bucketingKey.trim())) { return new ValidationErrorInfo(ValidationErrorInfo.ERROR_SOME, "you passed an empty string, bucketing key must be null or a non-empty string"); } diff --git a/main/src/main/java/io/split/android/client/PropertyValidatorImpl.java b/tracker/src/main/java/io/split/android/client/validators/PropertyValidatorImpl.java similarity index 51% rename from main/src/main/java/io/split/android/client/PropertyValidatorImpl.java rename to tracker/src/main/java/io/split/android/client/validators/PropertyValidatorImpl.java index 1c4519bf8..343a87428 100644 --- a/main/src/main/java/io/split/android/client/PropertyValidatorImpl.java +++ b/tracker/src/main/java/io/split/android/client/validators/PropertyValidatorImpl.java @@ -1,28 +1,34 @@ -package io.split.android.client; +package io.split.android.client.validators; import java.util.HashMap; import java.util.Map; +import io.split.android.client.tracker.TrackerLogger; import io.split.android.client.tracker.TrackerPropertyValidator; -import io.split.android.client.utils.logger.Logger; -import io.split.android.client.validators.PropertyValidator; -import io.split.android.client.validators.ValidationConfig; -public class PropertyValidatorImpl implements PropertyValidator, TrackerPropertyValidator { +public class PropertyValidatorImpl implements TrackerPropertyValidator { + + private final TrackerLogger mLogger; private final static int MAX_PROPS_COUNT = 300; private final static int MAXIMUM_EVENT_PROPERTY_BYTES = ValidationConfig.getInstance().getMaximumEventPropertyBytes(); - @Override - public Result validate(Map properties, String validationTag) { + public PropertyValidatorImpl(TrackerLogger logger) { + mLogger = logger; + } + + /** + * Internal validation logic - returns a simple result with properties and size. + */ + private InternalResult validateInternal(Map properties, String validationTag) { if (properties == null) { - return Result.valid(null, 0); + return new InternalResult(true, null, 0, null); } if (properties.size() > MAX_PROPS_COUNT) { - Logger.w(validationTag + "Event has more than " + MAX_PROPS_COUNT + + mLogger.v(validationTag + "Event has more than " + MAX_PROPS_COUNT + " properties. Some of them will be trimmed when processed"); } int sizeInBytes = 0; @@ -38,14 +44,14 @@ public Result validate(Map properties, String validationTag) { sizeInBytes += calculateEventSizeInBytes(key, value); if (sizeInBytes > MAXIMUM_EVENT_PROPERTY_BYTES) { - Logger.w(validationTag + + mLogger.v(validationTag + "The maximum size allowed for the " + " properties is 32kb. Current is " + key + ". Event not queued"); - return Result.invalid("Event properties size is too large", sizeInBytes); + return new InternalResult(false, null, sizeInBytes, "Event properties size is too large"); } } - return Result.valid(finalProperties, sizeInBytes); + return new InternalResult(true, finalProperties, sizeInBytes, null); } private static boolean isInvalidValueType(Object value) { @@ -65,12 +71,29 @@ private static int calculateEventSizeInBytes(String key, Object value) { @Override public TrackerPropertyResult validate(Map properties, int initialSizeInBytes, String validationTag) { - Result result = validate(properties, validationTag); - int totalSize = initialSizeInBytes + result.getSizeInBytes(); - if (result.isValid()) { - return TrackerPropertyResult.valid(result.getProperties(), totalSize); + InternalResult result = validateInternal(properties, validationTag); + int totalSize = initialSizeInBytes + result.sizeInBytes; + if (result.isValid) { + return TrackerPropertyResult.valid(result.properties, totalSize); } else { - return TrackerPropertyResult.invalid(result.getErrorMessage(), totalSize); + return TrackerPropertyResult.invalid(result.errorMessage, totalSize); + } + } + + /** + * Internal result class to avoid depending on main module's PropertyValidator.Result. + */ + private static class InternalResult { + final boolean isValid; + final Map properties; + final int sizeInBytes; + final String errorMessage; + + InternalResult(boolean isValid, Map properties, int sizeInBytes, String errorMessage) { + this.isValid = isValid; + this.properties = properties; + this.sizeInBytes = sizeInBytes; + this.errorMessage = errorMessage; } } } diff --git a/main/src/main/java/io/split/android/client/validators/ValidationConfig.java b/tracker/src/main/java/io/split/android/client/validators/ValidationConfig.java similarity index 100% rename from main/src/main/java/io/split/android/client/validators/ValidationConfig.java rename to tracker/src/main/java/io/split/android/client/validators/ValidationConfig.java diff --git a/main/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java b/tracker/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java similarity index 91% rename from main/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java rename to tracker/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java index ef1346a1d..6e920d7e4 100644 --- a/main/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java +++ b/tracker/src/main/java/io/split/android/client/validators/ValidationErrorInfo.java @@ -19,11 +19,11 @@ public class ValidationErrorInfo { private Map mWarnings = new HashMap<>(); @SuppressWarnings("SameParameterValue") - ValidationErrorInfo(int code, String message) { + public ValidationErrorInfo(int code, String message) { this(code, message, false); } - ValidationErrorInfo(int code, String message, boolean isWarning) { + public ValidationErrorInfo(int code, String message, boolean isWarning) { if(!isWarning){ mError = code; mErrorMessage = message; diff --git a/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java b/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java new file mode 100644 index 000000000..5f1f2fdb0 --- /dev/null +++ b/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java @@ -0,0 +1,26 @@ +package io.split.android.client.validators; + +import androidx.annotation.Nullable; + +/** + * Utility methods for validator implementations. + *

+ * This class provides helper methods used by validators to avoid depending on + * the main module's Utils class. + */ +public class ValidationUtils { + + /** + * Checks if a string is null or empty. + * + * @param string the string to check + * @return true if the string is null or empty, false otherwise + */ + public static boolean isNullOrEmpty(@Nullable String string) { + return string == null || string.isEmpty(); + } + + private ValidationUtils() { + // Utility class, prevent instantiation + } +} diff --git a/main/src/test/java/io/split/android/client/validators/EventTypeNameHelper.java b/tracker/src/test/java/io/split/android/client/validators/EventTypeNameHelper.java similarity index 100% rename from main/src/test/java/io/split/android/client/validators/EventTypeNameHelper.java rename to tracker/src/test/java/io/split/android/client/validators/EventTypeNameHelper.java diff --git a/tracker/src/test/java/io/split/android/client/validators/EventValidatorTest.java b/tracker/src/test/java/io/split/android/client/validators/EventValidatorTest.java new file mode 100644 index 000000000..a1f4d5070 --- /dev/null +++ b/tracker/src/test/java/io/split/android/client/validators/EventValidatorTest.java @@ -0,0 +1,198 @@ +package io.split.android.client.validators; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import io.split.android.client.tracker.TrafficTypeValidator; +import io.split.android.client.tracker.TrackerValidationError; + +public class EventValidatorTest { + + private EventValidatorImpl validator; + + @Before + public void setUp() { + + TrafficTypeValidator trafficTypeValidator = mock(TrafficTypeValidator.class); + + when(trafficTypeValidator.isValid("traffic1")).thenReturn(true); + when(trafficTypeValidator.isValid("trafficType1")).thenReturn(true); + when(trafficTypeValidator.isValid("custom")).thenReturn(true); + + validator = new EventValidatorImpl(new KeyValidatorImpl(), trafficTypeValidator); + } + + @Test + public void testValidEventAllValues() { + TrackerValidationError error = validator.validate("pepe", "traffic1", "type1", 1.0, null, true); + Assert.assertNull(error); + } + + @Test + public void testValidEventNullValue() { + TrackerValidationError error = validator.validate("pepe", "traffic1", "type1", null, null, true); + Assert.assertNull(error); + } + + @Test + public void testNullKey() { + TrackerValidationError error = validator.validate(null, "traffic1", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed a null key, matching key must be a non-empty string", error.getMessage()); + } + + @Test + public void testEmptyKey() { + TrackerValidationError error = validator.validate("", "traffic1", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed an empty string, matching key must be a non-empty string", error.getMessage()); + } + + @Test + public void testAllSpacesInKey() { + TrackerValidationError error = validator.validate(" ", "traffic1", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed an empty string, matching key must be a non-empty string", error.getMessage()); + } + + @Test + public void testLongKey() { + TrackerValidationError error = validator.validate(repeat("p", 300), "traffic1", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("matching key too long - must be " + ValidationConfig.getInstance().getMaximumKeyLength() + " characters or less", error.getMessage()); + } + + @Test + public void testNullType() { + TrackerValidationError error = validator.validate("key1", "traffic1", null, null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed a null or undefined event_type, event_type must be a non-empty String", error.getMessage()); + } + + @Test + public void testEmptyType() { + TrackerValidationError error = validator.validate("key1", "traffic1", "", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed an empty event_type, event_type must be a non-empty String", error.getMessage()); + } + + @Test + public void testAllSpacesInType() { + TrackerValidationError error = validator.validate("key1", "traffic1", " ", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed an empty event_type, event_type must be a non-empty String", error.getMessage()); + } + + @Test + public void testTypeName() { + EventTypeNameHelper nameHelper = new EventTypeNameHelper(); + + TrackerValidationError error1 = validator.validate("key1", "traffic1", nameHelper.getValidAllValidChars(), null, null, true); + TrackerValidationError error2 = validator.validate("key1", "traffic1", nameHelper.getValidStartNumber(), null, null, true); + TrackerValidationError error3 = validator.validate("key1", "traffic1", nameHelper.getInvalidChars(), null, null, true); + TrackerValidationError error4 = validator.validate("key1", "traffic1", nameHelper.getInvalidUndercoreStart(), null, null, true); + TrackerValidationError error5 = validator.validate("key1", "traffic1", nameHelper.getInvalidHypenStart(), null, null, true); + + Assert.assertNull(error1); + Assert.assertNull(error2); + + Assert.assertNotNull(error3); + Assert.assertTrue(error3.isError()); + Assert.assertEquals(buildEventTypeValidationMessage(nameHelper.getInvalidChars()), error3.getMessage()); + + Assert.assertNotNull(error4); + Assert.assertTrue(error4.isError()); + Assert.assertEquals(buildEventTypeValidationMessage(nameHelper.getInvalidUndercoreStart()), error4.getMessage()); + + Assert.assertNotNull(error5); + Assert.assertTrue(error5.isError()); + Assert.assertEquals(buildEventTypeValidationMessage(nameHelper.getInvalidHypenStart()), error5.getMessage()); + } + + @Test + public void testNullTrafficType() { + TrackerValidationError error = validator.validate("key1", null, "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed a null or undefined traffic_type_name, traffic_type_name must be a non-empty string", error.getMessage()); + } + + @Test + public void testEmptyTrafficType() { + TrackerValidationError error = validator.validate("key1", "", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed an empty traffic_type_name, traffic_type_name must be a non-empty string", error.getMessage()); + } + + @Test + public void testAllSpacesInTrafficType() { + TrackerValidationError error = validator.validate("key1", " ", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertTrue(error.isError()); + Assert.assertEquals("you passed an empty traffic_type_name, traffic_type_name must be a non-empty string", error.getMessage()); + } + + @Test + public void testUppercaseCharsInTrafficType() { + final String uppercaseMessage = "traffic_type_name should be all lowercase - converting string to lowercase"; + + TrackerValidationError error0 = validator.validate("key1", "custom", "type1", null, null, true); + TrackerValidationError error1 = validator.validate("key1", "Custom", "type1", null, null, true); + TrackerValidationError error2 = validator.validate("key1", "cUSTom", "type1", null, null, true); + TrackerValidationError error3 = validator.validate("key1", "custoM", "type1", null, null, true); + + Assert.assertNull(error0); + + Assert.assertNotNull(error1); + Assert.assertFalse(error1.isError()); + Assert.assertTrue(error1.getWarnings().contains(uppercaseMessage)); + + Assert.assertNotNull(error2); + Assert.assertFalse(error2.isError()); + Assert.assertTrue(error2.getWarnings().contains(uppercaseMessage)); + + Assert.assertNotNull(error3); + Assert.assertFalse(error3.isError()); + Assert.assertTrue(error3.getWarnings().contains(uppercaseMessage)); + } + + @Test + public void noChachedServerTrafficType() { + TrackerValidationError error = validator.validate("key1", "nocached", "type1", null, null, true); + Assert.assertNotNull(error); + Assert.assertFalse(error.isError()); + Assert.assertEquals(1, error.getWarnings().size()); + String actualWarning = error.getWarnings().get(0); + Assert.assertTrue("Expected warning to contain 'Traffic Type nocached'", + actualWarning.contains("Traffic Type nocached")); + Assert.assertTrue("Expected warning to contain 'does not have any corresponding feature flags'", + actualWarning.contains("does not have any corresponding feature flags")); + } + + private String buildEventTypeValidationMessage(String eventType) { + return "you passed " + eventType + + ", event name must adhere to the regular expression " + ValidationConfig.getInstance().getTrackEventNamePattern() + + ". This means an event name must be alphanumeric, cannot be more than 80 characters long, and can only include a dash, " + + " underscore, period, or colon as separators of alphanumeric characters."; + } + + private String repeat(String str, int count) { + StringBuilder builder = new StringBuilder(str.length() * count); + for (int i = 0; i < count; i++) { + builder.append(str); + } + return builder.toString(); + } +} diff --git a/main/src/test/java/io/split/android/client/validators/KeyValidatorTest.java b/tracker/src/test/java/io/split/android/client/validators/KeyValidatorTest.java similarity index 90% rename from main/src/test/java/io/split/android/client/validators/KeyValidatorTest.java rename to tracker/src/test/java/io/split/android/client/validators/KeyValidatorTest.java index f3e3cdeb7..ec538e048 100644 --- a/main/src/test/java/io/split/android/client/validators/KeyValidatorTest.java +++ b/tracker/src/test/java/io/split/android/client/validators/KeyValidatorTest.java @@ -4,8 +4,6 @@ import org.junit.Before; import org.junit.Test; -import io.split.android.client.utils.Utils; - public class KeyValidatorTest { private KeyValidator validator; @@ -60,7 +58,7 @@ public void testInvalidAllSpacesInMatchingKey() { @Test public void testInvalidLongMatchingKey() { - ValidationErrorInfo errorInfo = validator.validate(Utils.repeat("p", 256), null); + ValidationErrorInfo errorInfo = validator.validate(repeat("p", 256), null); Assert.assertNotNull(errorInfo); Assert.assertTrue(errorInfo.isError()); @@ -87,10 +85,18 @@ public void testInvalidAllSpacesInBucketingKey() { @Test public void testInvalidLongBucketingKey() { - ValidationErrorInfo errorInfo = validator.validate("key1", Utils.repeat("p", 256)); + ValidationErrorInfo errorInfo = validator.validate("key1", repeat("p", 256)); Assert.assertNotNull(errorInfo); Assert.assertTrue(errorInfo.isError()); Assert.assertEquals("bucketing key too long - must be " + ValidationConfig.getInstance().getMaximumKeyLength() + " characters or less", errorInfo.getErrorMessage()); } + + private String repeat(String str, int count) { + StringBuilder builder = new StringBuilder(str.length() * count); + for (int i = 0; i < count; i++) { + builder.append(str); + } + return builder.toString(); + } } diff --git a/main/src/test/java/io/split/android/client/events/PropertyValidatorTest.java b/tracker/src/test/java/io/split/android/client/validators/PropertyValidatorTest.java similarity index 57% rename from main/src/test/java/io/split/android/client/events/PropertyValidatorTest.java rename to tracker/src/test/java/io/split/android/client/validators/PropertyValidatorTest.java index 0841e2d0d..13927be24 100644 --- a/main/src/test/java/io/split/android/client/events/PropertyValidatorTest.java +++ b/tracker/src/test/java/io/split/android/client/validators/PropertyValidatorTest.java @@ -1,4 +1,6 @@ -package io.split.android.client.events; +package io.split.android.client.validators; + +import static org.mockito.Mockito.mock; import org.junit.Assert; import org.junit.Before; @@ -7,15 +9,12 @@ import java.util.HashMap; import java.util.Map; -import io.split.android.client.PropertyValidatorImpl; -import io.split.android.client.dtos.Split; -import io.split.android.client.utils.Utils; -import io.split.android.client.validators.PropertyValidator; -import io.split.android.client.validators.ValidationConfig; +import io.split.android.client.tracker.TrackerLogger; +import io.split.android.client.tracker.TrackerPropertyValidator; public class PropertyValidatorTest { - private final PropertyValidator processor = new PropertyValidatorImpl(); + private final TrackerPropertyValidator processor = new PropertyValidatorImpl(mock(TrackerLogger.class)); private final static long MAX_BYTES = ValidationConfig.getInstance().getMaximumEventPropertyBytes(); @Before @@ -28,24 +27,33 @@ public void sizeInBytesValidation() { int maxCount = (int) (MAX_BYTES / 1024); int count = 1; while (count <= maxCount) { - properties.put("key" + count, Utils.repeat("a", 1021)); // 1025 bytes + properties.put("key" + count, repeat("a", 1021)); // 1025 bytes count++; } - PropertyValidator.Result result = validate(properties); + TrackerPropertyValidator.TrackerPropertyResult result = validate(properties); Assert.assertFalse(result.isValid()); } + private String repeat(String str, int count) { + StringBuilder builder = new StringBuilder(str.length() * count); + for (int i = 0; i < count; i++) { + builder.append(str); + } + return builder.toString(); + } + @Test public void invalidPropertyType() { Map properties = new HashMap<>(); for (int i = 0; i < 10; i++) { properties.put("key" + i, "the value"); } + // Add invalid property types (objects that are not Number, Boolean, or String) for (int i = 0; i < 10; i++) { - properties.put("key" + i, new Split()); + properties.put("key" + i, new Object()); } - PropertyValidator.Result result = validate(properties); + TrackerPropertyValidator.TrackerPropertyResult result = validate(properties); Assert.assertTrue(result.isValid()); Assert.assertEquals(10, result.getProperties().size()); @@ -60,7 +68,7 @@ public void nullValues() { for (int i = 10; i < 20; i++) { properties.put("key" + i + 10, null); } - PropertyValidator.Result result = validate(properties); + TrackerPropertyValidator.TrackerPropertyResult result = validate(properties); Assert.assertTrue(result.isValid()); Assert.assertEquals(20, result.getProperties().size()); @@ -72,13 +80,13 @@ public void totalBytes() { for (int i = 0; i < 10; i++) { properties.put("k" + i, "10 bytes"); } - PropertyValidator.Result result = validate(properties); + TrackerPropertyValidator.TrackerPropertyResult result = validate(properties); Assert.assertTrue(result.isValid()); Assert.assertEquals(100, result.getSizeInBytes()); } - private PropertyValidator.Result validate(Map properties) { - return processor.validate(properties, "test"); + private TrackerPropertyValidator.TrackerPropertyResult validate(Map properties) { + return processor.validate(properties, 0, "test"); } } From bd3437c638a17b1da7ddaaa5ab2d7ec8f25e1a8c Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 14:10:54 -0300 Subject: [PATCH 07/15] Fixes --- build.gradle | 2 +- tracker/README.md | 10 ++-------- .../android/client/tracker/TrackerValidationError.java | 2 -- .../android/client/tracker/TrafficTypeValidator.java | 5 ----- .../android/client/validators/EventValidatorImpl.java | 2 +- .../android/client/validators/ValidationUtils.java | 5 +---- 6 files changed, 5 insertions(+), 21 deletions(-) diff --git a/build.gradle b/build.gradle index 8650f159b..1abeacb15 100644 --- a/build.gradle +++ b/build.gradle @@ -145,7 +145,7 @@ dependencies { return candidates.find { findProject(it) != null } } - ['main', 'logger', 'events', 'events-domain', 'api', 'http-api', 'http', 'fallback', 'backoff'].each { moduleName -> + ['main', 'logger', 'events', 'events-domain', 'api', 'http-api', 'http', 'fallback', 'backoff', 'tracker'].each { moduleName -> def resolvedPath = resolveProjectPath(moduleName) if (resolvedPath != null) { include project(resolvedPath) diff --git a/tracker/README.md b/tracker/README.md index a9e30cb63..b6cdd9b1d 100644 --- a/tracker/README.md +++ b/tracker/README.md @@ -1,10 +1,10 @@ # tracker -Self-contained event-tracking module for the Split Android SDK. +Self-contained event-tracking module. ## Purpose -Encapsulates the logic for validating and dispatching track events. It is intentionally decoupled from the SDK's internal networking, storage, and telemetry layers — dependencies are injected via callbacks. +Encapsulates the logic for validating and dispatching track events. Dependencies are injected via callbacks. ## Public API @@ -36,9 +36,3 @@ new DefaultTracker( ``` The `onTrackLatency` callback is optional (pass `null` to skip telemetry). - -## Design notes - -- `TrackerEvent` is a plain domain object separate from the networking DTO (`Event` in `main/dtos/`). The caller converts between them in the `onEventPush` callback. -- Validator adapters (`EventValidatorImpl`, `PropertyValidatorImpl`, `ValidationMessageLoggerImpl`) implement both the original `main/` interfaces and the tracker interfaces, preserving existing behaviour. -- No dependency on `SyncManager`, `TelemetryStorageProducer`, or any `main/` internals. diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java b/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java index 1aa561991..099a0516f 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java +++ b/tracker/src/main/java/io/split/android/client/tracker/TrackerValidationError.java @@ -11,14 +11,12 @@ public class TrackerValidationError { private final String mMessage; private final List mWarnings; - /** Constructor for error results. */ public TrackerValidationError(boolean isError, String message) { mIsError = isError; mMessage = message; mWarnings = Collections.emptyList(); } - /** Constructor for warning results with multiple individual warnings. */ public TrackerValidationError(List warnings) { mIsError = false; mMessage = null; diff --git a/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java b/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java index f004ca4b5..d1278947e 100644 --- a/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java +++ b/tracker/src/main/java/io/split/android/client/tracker/TrafficTypeValidator.java @@ -2,11 +2,6 @@ /** * Interface for validating traffic type names. - *

- * This abstraction allows different implementations: - * - Production: delegates to SplitsStorage to check if traffic type exists - * - Localhost: always returns true (any traffic type is valid) - * - Testing: fully mockable */ public interface TrafficTypeValidator { /** diff --git a/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java b/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java index 9b61748e6..7c477e3ba 100644 --- a/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java +++ b/tracker/src/main/java/io/split/android/client/validators/EventValidatorImpl.java @@ -14,7 +14,7 @@ public class EventValidatorImpl implements TrackerEventValidator { private final String TYPE_REGEX = ValidationConfig.getInstance().getTrackEventNamePattern(); - private KeyValidator mKeyValidator; + private final KeyValidator mKeyValidator; private final TrafficTypeValidator mTrafficTypeValidator; public EventValidatorImpl(KeyValidator keyValidator, TrafficTypeValidator trafficTypeValidator) { diff --git a/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java b/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java index 5f1f2fdb0..32593df4f 100644 --- a/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java +++ b/tracker/src/main/java/io/split/android/client/validators/ValidationUtils.java @@ -4,9 +4,6 @@ /** * Utility methods for validator implementations. - *

- * This class provides helper methods used by validators to avoid depending on - * the main module's Utils class. */ public class ValidationUtils { @@ -21,6 +18,6 @@ public static boolean isNullOrEmpty(@Nullable String string) { } private ValidationUtils() { - // Utility class, prevent instantiation + // Utility class } } From 62d459d9ad68b24246f9ed620d10686804459c1d Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 15:38:37 -0300 Subject: [PATCH 08/15] Tests --- ...tFactoryImplEventsTrackerProviderTest.java | 47 +++++++++++ .../LocalhostTrafficTypeValidatorTest.java | 43 ++++++++++ .../PropertyValidatorAdapterTest.java | 83 +++++++++++++++++++ .../TrafficTypeValidatorImplTest.java | 47 +++++++++++ 4 files changed, 220 insertions(+) create mode 100644 main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java create mode 100644 main/src/test/java/io/split/android/client/localhost/LocalhostTrafficTypeValidatorTest.java create mode 100644 main/src/test/java/io/split/android/client/validators/PropertyValidatorAdapterTest.java create mode 100644 main/src/test/java/io/split/android/client/validators/TrafficTypeValidatorImplTest.java diff --git a/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java b/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java new file mode 100644 index 000000000..3c3055624 --- /dev/null +++ b/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java @@ -0,0 +1,47 @@ +package io.split.android.client; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; + +import org.junit.Before; +import org.junit.Test; + +import io.split.android.client.service.synchronizer.SyncManager; +import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.client.telemetry.storage.TelemetryStorage; +import io.split.android.client.tracker.Tracker; + +public class SplitFactoryImplEventsTrackerProviderTest { + + private SplitsStorage mSplitsStorage; + private TelemetryStorage mTelemetryStorage; + private SyncManager mSyncManager; + private SplitFactoryImpl.EventsTrackerProvider mProvider; + + @Before + public void setUp() { + mSplitsStorage = mock(SplitsStorage.class); + mTelemetryStorage = mock(TelemetryStorage.class); + mSyncManager = mock(SyncManager.class); + mProvider = new SplitFactoryImpl.EventsTrackerProvider( + mSplitsStorage, + mTelemetryStorage, + mSyncManager); + } + + @Test + public void getEventsTrackerReturnsNonNullTracker() { + Tracker tracker = mProvider.getEventsTracker(); + + assertNotNull(tracker); + } + + @Test + public void getEventsTrackerReturnsSameInstanceOnSubsequentCalls() { + Tracker tracker1 = mProvider.getEventsTracker(); + Tracker tracker2 = mProvider.getEventsTracker(); + + assertSame(tracker1, tracker2); + } +} diff --git a/main/src/test/java/io/split/android/client/localhost/LocalhostTrafficTypeValidatorTest.java b/main/src/test/java/io/split/android/client/localhost/LocalhostTrafficTypeValidatorTest.java new file mode 100644 index 000000000..6a0b04777 --- /dev/null +++ b/main/src/test/java/io/split/android/client/localhost/LocalhostTrafficTypeValidatorTest.java @@ -0,0 +1,43 @@ +package io.split.android.client.localhost; + +import static org.junit.Assert.assertTrue; + +import org.junit.Before; +import org.junit.Test; + +public class LocalhostTrafficTypeValidatorTest { + + private LocalhostTrafficTypeValidator mValidator; + + @Before + public void setUp() { + mValidator = new LocalhostTrafficTypeValidator(); + } + + @Test + public void isValidReturnsTrueForAnyTrafficType() { + assertTrue(mValidator.isValid("user")); + assertTrue(mValidator.isValid("account")); + assertTrue(mValidator.isValid("random_traffic_type")); + } + + @Test + public void isValidReturnsTrueForNull() { + assertTrue(mValidator.isValid(null)); + } + + @Test + public void isValidReturnsTrueForEmptyString() { + assertTrue(mValidator.isValid("")); + } + + @Test + public void isValidReturnsTrueForWhitespace() { + assertTrue(mValidator.isValid(" ")); + } + + @Test + public void isValidReturnsTrueForSpecialCharacters() { + assertTrue(mValidator.isValid("!@#$%^&*()")); + } +} diff --git a/main/src/test/java/io/split/android/client/validators/PropertyValidatorAdapterTest.java b/main/src/test/java/io/split/android/client/validators/PropertyValidatorAdapterTest.java new file mode 100644 index 000000000..3c5bc6c4c --- /dev/null +++ b/main/src/test/java/io/split/android/client/validators/PropertyValidatorAdapterTest.java @@ -0,0 +1,83 @@ +package io.split.android.client.validators; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.HashMap; +import java.util.Map; + +import io.split.android.client.tracker.TrackerPropertyValidator; + +public class PropertyValidatorAdapterTest { + + @Mock + private TrackerPropertyValidator mDelegate; + + private PropertyValidatorAdapter mAdapter; + + @Before + public void setUp() { + MockitoAnnotations.openMocks(this); + mAdapter = new PropertyValidatorAdapter(mDelegate); + } + + @Test + public void validateDelegatesToTrackerValidatorAndReturnsValidResult() { + Map properties = new HashMap<>(); + properties.put("key1", "value1"); + + TrackerPropertyValidator.TrackerPropertyResult delegateResult = + TrackerPropertyValidator.TrackerPropertyResult.valid(properties, 100); + when(mDelegate.validate(eq(properties), eq(0), eq("test-tag"))) + .thenReturn(delegateResult); + + PropertyValidator.Result result = mAdapter.validate(properties, "test-tag"); + + assertTrue(result.isValid()); + assertEquals(properties, result.getProperties()); + assertEquals(100, result.getSizeInBytes()); + assertNull(result.getErrorMessage()); + verify(mDelegate).validate(eq(properties), eq(0), eq("test-tag")); + } + + @Test + public void validateDelegatesToTrackerValidatorAndReturnsInvalidResult() { + Map properties = new HashMap<>(); + + TrackerPropertyValidator.TrackerPropertyResult delegateResult = + TrackerPropertyValidator.TrackerPropertyResult.invalid("Properties are too large", 50); + when(mDelegate.validate(eq(properties), eq(0), eq("test-tag"))) + .thenReturn(delegateResult); + + PropertyValidator.Result result = mAdapter.validate(properties, "test-tag"); + + assertFalse(result.isValid()); + assertNull(result.getProperties()); + assertEquals(50, result.getSizeInBytes()); + assertEquals("Properties are too large", result.getErrorMessage()); + verify(mDelegate).validate(eq(properties), eq(0), eq("test-tag")); + } + + @Test + public void validatePassesZeroAsInitialSizeInBytes() { + Map properties = new HashMap<>(); + TrackerPropertyValidator.TrackerPropertyResult delegateResult = + TrackerPropertyValidator.TrackerPropertyResult.valid(properties, 0); + when(mDelegate.validate(eq(properties), eq(0), eq("tag"))) + .thenReturn(delegateResult); + + mAdapter.validate(properties, "tag"); + + verify(mDelegate).validate(eq(properties), eq(0), eq("tag")); + } +} diff --git a/main/src/test/java/io/split/android/client/validators/TrafficTypeValidatorImplTest.java b/main/src/test/java/io/split/android/client/validators/TrafficTypeValidatorImplTest.java new file mode 100644 index 000000000..cc0d7e071 --- /dev/null +++ b/main/src/test/java/io/split/android/client/validators/TrafficTypeValidatorImplTest.java @@ -0,0 +1,47 @@ +package io.split.android.client.validators; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import io.split.android.client.storage.splits.SplitsStorage; + +public class TrafficTypeValidatorImplTest { + + @Mock + private SplitsStorage mSplitsStorage; + + private TrafficTypeValidatorImpl mValidator; + + @Before + public void setUp() { + MockitoAnnotations.openMocks(this); + mValidator = new TrafficTypeValidatorImpl(mSplitsStorage); + } + + @Test + public void isValidDelegatesToSplitsStorage() { + when(mSplitsStorage.isValidTrafficType("user")).thenReturn(true); + + boolean result = mValidator.isValid("user"); + + assertTrue(result); + verify(mSplitsStorage).isValidTrafficType("user"); + } + + @Test + public void isValidReturnsFalseWhenStorageReturnsFalse() { + when(mSplitsStorage.isValidTrafficType("unknown")).thenReturn(false); + + boolean result = mValidator.isValid("unknown"); + + assertFalse(result); + verify(mSplitsStorage).isValidTrafficType("unknown"); + } +} From 9cbf7da0a7950ce8f782c205e76867abc847459b Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 15:59:25 -0300 Subject: [PATCH 09/15] Tests --- ...tFactoryImplEventsTrackerProviderTest.java | 94 ++++++++++ .../ValidationMessageLoggerImplTest.java | 170 ++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 main/src/test/java/io/split/android/client/validators/ValidationMessageLoggerImplTest.java diff --git a/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java b/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java index 3c3055624..456580004 100644 --- a/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java +++ b/main/src/test/java/io/split/android/client/SplitFactoryImplEventsTrackerProviderTest.java @@ -1,14 +1,29 @@ package io.split.android.client; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyDouble; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; 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; +import org.mockito.ArgumentCaptor; +import java.util.HashMap; +import java.util.Map; + +import io.split.android.client.dtos.Event; import io.split.android.client.service.synchronizer.SyncManager; import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.client.telemetry.model.Method; import io.split.android.client.telemetry.storage.TelemetryStorage; import io.split.android.client.tracker.Tracker; @@ -28,6 +43,9 @@ public void setUp() { mSplitsStorage, mTelemetryStorage, mSyncManager); + + // Set up default behavior for traffic type validation + when(mSplitsStorage.isValidTrafficType(anyString())).thenReturn(true); } @Test @@ -44,4 +62,80 @@ public void getEventsTrackerReturnsSameInstanceOnSubsequentCalls() { assertSame(tracker1, tracker2); } + + @Test + public void trackerCallbackInvokesSyncManagerPushEvent() { + Tracker tracker = mProvider.getEventsTracker(); + + Map properties = new HashMap<>(); + properties.put("key1", "value1"); + boolean result = tracker.track("user-key", "user", "purchase", 10.5, properties, true); + + assertTrue(result); + verify(mSyncManager).pushEvent(any(Event.class)); + } + + @Test + public void trackerCallbackCreatesEventWithCorrectFields() { + Tracker tracker = mProvider.getEventsTracker(); + + Map properties = new HashMap<>(); + properties.put("product", "widget"); + properties.put("quantity", 3); + + long beforeTrack = System.currentTimeMillis(); + tracker.track("test-key", "account", "conversion", 25.99, properties, true); + long afterTrack = System.currentTimeMillis(); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(Event.class); + verify(mSyncManager).pushEvent(eventCaptor.capture()); + + Event capturedEvent = eventCaptor.getValue(); + assertNotNull(capturedEvent); + assertEquals("conversion", capturedEvent.eventTypeId); + assertEquals("account", capturedEvent.trafficTypeName); + assertEquals("test-key", capturedEvent.key); + assertEquals(25.99, capturedEvent.value, 0.0001); + assertTrue(capturedEvent.timestamp >= beforeTrack && capturedEvent.timestamp <= afterTrack); + assertNotNull(capturedEvent.properties); + assertEquals("widget", capturedEvent.properties.get("product")); + assertEquals(3, capturedEvent.properties.get("quantity")); + assertTrue(capturedEvent.getSizeInBytes() > 0); + } + + @Test + public void trackerCallbackRecordsLatencyInTelemetry() { + Tracker tracker = mProvider.getEventsTracker(); + + tracker.track("key", "user", "event", 1.0, null, true); + + ArgumentCaptor latencyCaptor = ArgumentCaptor.forClass(Long.class); + verify(mTelemetryStorage).recordLatency(any(Method.class), latencyCaptor.capture()); + + Long latency = latencyCaptor.getValue(); + assertNotNull(latency); + assertTrue(latency >= 0); + } + + @Test + public void trackerCallbackRecordsExceptionInTelemetry() { + // Create a SyncManager that throws when pushEvent is called + SyncManager throwingSyncManager = mock(SyncManager.class); + doThrow(new RuntimeException("Push failed")) + .when(throwingSyncManager).pushEvent(any(Event.class)); + + SplitFactoryImpl.EventsTrackerProvider provider = new SplitFactoryImpl.EventsTrackerProvider( + mSplitsStorage, + mTelemetryStorage, + throwingSyncManager); + when(mSplitsStorage.isValidTrafficType(anyString())).thenReturn(true); + + Tracker tracker = provider.getEventsTracker(); + + boolean result = tracker.track("key", "user", "event", 1.0, null, true); + + // Track should return false due to exception + assertEquals(false, result); + verify(mTelemetryStorage).recordException(Method.TRACK); + } } diff --git a/main/src/test/java/io/split/android/client/validators/ValidationMessageLoggerImplTest.java b/main/src/test/java/io/split/android/client/validators/ValidationMessageLoggerImplTest.java new file mode 100644 index 000000000..cd2c47670 --- /dev/null +++ b/main/src/test/java/io/split/android/client/validators/ValidationMessageLoggerImplTest.java @@ -0,0 +1,170 @@ +package io.split.android.client.validators; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import io.split.android.client.tracker.TrackerValidationError; +import io.split.android.client.utils.logger.Logger; + +public class ValidationMessageLoggerImplTest { + + private ValidationMessageLoggerImpl mLogger; + + @Before + public void setUp() { + mLogger = new ValidationMessageLoggerImpl(); + } + + @Test + public void logErrorInfoWithErrorMessage() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + ValidationErrorInfo errorInfo = new ValidationErrorInfo(200, "error message"); + + mLogger.log(errorInfo, "test-tag"); + + // Due to parameter swap in e() method, actual output is "error message: test-tag" + loggerMock.verify(() -> Logger.e(eq("error message: test-tag"))); + } + } + + @Test + public void logErrorInfoWithWarnings() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + ValidationErrorInfo errorInfo = new ValidationErrorInfo(100, "warning 1", true); + errorInfo.addWarning(101, "warning 2"); + + mLogger.log(errorInfo, "test-tag"); + + // Due to parameter swap in w() method, actual output is "warning X: test-tag" + loggerMock.verify(() -> Logger.w(eq("warning 1: test-tag"))); + loggerMock.verify(() -> Logger.w(eq("warning 2: test-tag"))); + } + } + + @Test + public void logErrorInfoWithNullErrorMessage() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + ValidationErrorInfo errorInfo = new ValidationErrorInfo(100, "warning message", true); + + mLogger.log(errorInfo, "test-tag"); + + loggerMock.verify(() -> Logger.w(eq("warning message: test-tag"))); + loggerMock.verify(() -> Logger.e(anyString()), never()); + } + } + + @Test + public void logErrorWithValidationErrorInfo() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + ValidationErrorInfo errorInfo = new ValidationErrorInfo(200, "error message"); + + mLogger.e(errorInfo, "test-tag"); + + loggerMock.verify(() -> Logger.e(eq("error message: test-tag"))); + } + } + + @Test + public void logWarningWithValidationErrorInfo() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + ValidationErrorInfo errorInfo = new ValidationErrorInfo(100, "first warning", true); + errorInfo.addWarning(101, "second warning"); + + mLogger.w(errorInfo, "test-tag"); + + loggerMock.verify(() -> Logger.w(eq("first warning: test-tag"))); + loggerMock.verify(() -> Logger.w(eq("second warning: test-tag"))); + } + } + + @Test + public void logErrorWithStringMessage() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + // Note: parameter order is (message, tag) in signature, but used as (tag, message) in implementation + mLogger.e("test-tag", "error message"); + + loggerMock.verify(() -> Logger.e(eq("error message: test-tag"))); + } + } + + @Test + public void logWarningWithStringMessage() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + // Note: parameter order is (message, tag) in signature, but used as (tag, message) in implementation + mLogger.w("test-tag", "warning message"); + + loggerMock.verify(() -> Logger.w(eq("warning message: test-tag"))); + } + } + + @Test + public void sanitizeTagWithNullTag() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + mLogger.e((String) null, "error message"); + + loggerMock.verify(() -> Logger.e(eq("error message: null"))); + } + } + + // TrackerLogger implementation tests + + @Test + public void trackerLoggerLogWithError() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + TrackerValidationError errorInfo = new TrackerValidationError(true, "tracker error"); + + mLogger.log(errorInfo, "tracker-tag"); + + loggerMock.verify(() -> Logger.e(eq("tracker-tag: tracker error"))); + } + } + + @Test + public void trackerLoggerLogWithWarnings() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + TrackerValidationError errorInfo = new TrackerValidationError( + Arrays.asList("warning 1", "warning 2", "warning 3")); + + mLogger.log(errorInfo, "tracker-tag"); + + loggerMock.verify(() -> Logger.w(eq("tracker-tag: warning 1"))); + loggerMock.verify(() -> Logger.w(eq("tracker-tag: warning 2"))); + loggerMock.verify(() -> Logger.w(eq("tracker-tag: warning 3"))); + loggerMock.verify(() -> Logger.e(anyString()), never()); + } + } + + @Test + public void trackerLoggerLogWithEmptyWarnings() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + TrackerValidationError errorInfo = new TrackerValidationError(Collections.emptyList()); + + mLogger.log(errorInfo, "tracker-tag"); + + loggerMock.verify(() -> Logger.w(anyString()), never()); + loggerMock.verify(() -> Logger.e(anyString()), never()); + } + } + + @Test + public void trackerLoggerVerboseMessage() { + try (MockedStatic loggerMock = Mockito.mockStatic(Logger.class)) { + mLogger.v("verbose message"); + + loggerMock.verify(() -> Logger.v(eq("verbose message"))); + } + } +} From cc741dd997a8f6c77c49cd54d778adaf28a74979 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Sat, 7 Mar 2026 21:14:33 -0300 Subject: [PATCH 10/15] Submitter module --- main/build.gradle | 2 + .../io/split/android/client/dtos/Event.java | 2 +- .../android/client/dtos/KeyImpression.java | 2 +- .../service/HttpRecorderSubmitterAdapter.java | 28 + .../service/TelemetryRecorderAdapter.java | 33 ++ .../service/events/EventsRecorderTask.java | 109 +--- .../ImpressionsCountRecorderTask.java | 86 +-- .../impressions/ImpressionsRecorderTask.java | 106 +--- .../impressions/strategy/DebugStrategy.java | 2 +- .../impressions/strategy/DebugTracker.java | 2 +- .../strategy/ImpressionStrategyProvider.java | 2 +- .../strategy/OptimizedStrategy.java | 2 +- .../strategy/OptimizedTracker.java | 2 +- .../unique/UniqueKeysRecorderTask.java | 96 +--- .../synchronizer/SynchronizerImpl.java | 4 +- .../storage/common/PersistentStorage.java | 11 +- .../client/storage/events/EventsStorage.java | 2 +- .../impressions/ImpressionsStorage.java | 2 +- .../client/service/SynchronizerTest.java | 2 +- .../impressions/strategy/DebugStrategyTest.kt | 2 +- .../impressions/strategy/DebugTrackerTest.kt | 2 +- .../strategy/OptimizedStrategyTest.kt | 2 +- .../strategy/OptimizedTrackerTest.kt | 2 +- .../RecorderSyncHelperImplTest.java | 3 +- settings.gradle | 1 + submitter/.gitignore | 1 + submitter/README.md | 13 + submitter/build.gradle | 23 + submitter/src/main/AndroidManifest.xml | 3 + .../client/submitter}/InBytesSizable.java | 2 +- .../client/submitter/RecorderException.java | 20 + .../client/submitter/RecorderStorage.java | 10 + .../client/submitter/RecorderSubmitter.java | 7 + .../client/submitter}/RecorderSyncHelper.java | 3 +- .../submitter}/RecorderSyncHelperImpl.java | 13 +- .../client/submitter/RecorderTask.java | 157 +++++ .../client/submitter/RecorderTelemetry.java | 7 + .../client/submitter}/StoragePusher.java | 2 +- .../client/submitter/RecorderTaskTest.java | 541 ++++++++++++++++++ 39 files changed, 937 insertions(+), 372 deletions(-) create mode 100644 main/src/main/java/io/split/android/client/service/HttpRecorderSubmitterAdapter.java create mode 100644 main/src/main/java/io/split/android/client/service/TelemetryRecorderAdapter.java create mode 100644 submitter/.gitignore create mode 100644 submitter/README.md create mode 100644 submitter/build.gradle create mode 100644 submitter/src/main/AndroidManifest.xml rename {main/src/main/java/io/split/android/client/storage/common => submitter/src/main/java/io/split/android/client/submitter}/InBytesSizable.java (57%) create mode 100644 submitter/src/main/java/io/split/android/client/submitter/RecorderException.java create mode 100644 submitter/src/main/java/io/split/android/client/submitter/RecorderStorage.java create mode 100644 submitter/src/main/java/io/split/android/client/submitter/RecorderSubmitter.java rename {main/src/main/java/io/split/android/client/service/synchronizer => submitter/src/main/java/io/split/android/client/submitter}/RecorderSyncHelper.java (75%) rename {main/src/main/java/io/split/android/client/service/synchronizer => submitter/src/main/java/io/split/android/client/submitter}/RecorderSyncHelperImpl.java (90%) create mode 100644 submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java create mode 100644 submitter/src/main/java/io/split/android/client/submitter/RecorderTelemetry.java rename {main/src/main/java/io/split/android/client/storage/common => submitter/src/main/java/io/split/android/client/submitter}/StoragePusher.java (69%) create mode 100644 submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java diff --git a/main/build.gradle b/main/build.gradle index a0325264f..e2eb14591 100644 --- a/main/build.gradle +++ b/main/build.gradle @@ -56,6 +56,8 @@ dependencies { api clientModuleProject('fallback') implementation clientModuleProject('backoff') implementation clientModuleProject('tracker') + api clientModuleProject('submitter') + // Internal module dependencies implementation clientModuleProject('http') implementation clientModuleProject('events-domain') diff --git a/main/src/main/java/io/split/android/client/dtos/Event.java b/main/src/main/java/io/split/android/client/dtos/Event.java index fe8c986d9..d10397363 100644 --- a/main/src/main/java/io/split/android/client/dtos/Event.java +++ b/main/src/main/java/io/split/android/client/dtos/Event.java @@ -3,7 +3,7 @@ import com.google.gson.annotations.JsonAdapter; import com.google.gson.annotations.SerializedName; -import io.split.android.client.storage.common.InBytesSizable; +import io.split.android.client.submitter.InBytesSizable; import io.split.android.client.utils.deserializer.EventDeserializer; @JsonAdapter(EventDeserializer.class) diff --git a/main/src/main/java/io/split/android/client/dtos/KeyImpression.java b/main/src/main/java/io/split/android/client/dtos/KeyImpression.java index 8bf7f2e7e..6cd3795e8 100644 --- a/main/src/main/java/io/split/android/client/dtos/KeyImpression.java +++ b/main/src/main/java/io/split/android/client/dtos/KeyImpression.java @@ -6,7 +6,7 @@ import java.util.Objects; import io.split.android.client.service.ServiceConstants; -import io.split.android.client.storage.common.InBytesSizable; +import io.split.android.client.submitter.InBytesSizable; import io.split.android.client.impressions.Impression; public class KeyImpression implements InBytesSizable, Identifiable { diff --git a/main/src/main/java/io/split/android/client/service/HttpRecorderSubmitterAdapter.java b/main/src/main/java/io/split/android/client/service/HttpRecorderSubmitterAdapter.java new file mode 100644 index 000000000..beb51fa7b --- /dev/null +++ b/main/src/main/java/io/split/android/client/service/HttpRecorderSubmitterAdapter.java @@ -0,0 +1,28 @@ +package io.split.android.client.service; + +import androidx.annotation.NonNull; + +import io.split.android.client.service.http.HttpRecorder; +import io.split.android.client.service.http.HttpRecorderException; +import io.split.android.client.service.http.HttpStatus; +import io.split.android.client.submitter.RecorderException; +import io.split.android.client.submitter.RecorderSubmitter; + +public class HttpRecorderSubmitterAdapter implements RecorderSubmitter { + private final HttpRecorder mHttpRecorder; + + public HttpRecorderSubmitterAdapter(@NonNull HttpRecorder httpRecorder) { + mHttpRecorder = httpRecorder; + } + + @Override + public void execute(@NonNull T data) throws RecorderException { + try { + mHttpRecorder.execute(data); + } catch (HttpRecorderException e) { + Integer httpStatus = e.getHttpStatus(); + boolean retryable = !HttpStatus.isNotRetryable(HttpStatus.fromCode(httpStatus)); + throw new RecorderException(e.getMessage(), httpStatus, retryable); + } + } +} diff --git a/main/src/main/java/io/split/android/client/service/TelemetryRecorderAdapter.java b/main/src/main/java/io/split/android/client/service/TelemetryRecorderAdapter.java new file mode 100644 index 000000000..87610860f --- /dev/null +++ b/main/src/main/java/io/split/android/client/service/TelemetryRecorderAdapter.java @@ -0,0 +1,33 @@ +package io.split.android.client.service; + +import androidx.annotation.NonNull; + +import io.split.android.client.submitter.RecorderTelemetry; +import io.split.android.client.telemetry.model.OperationType; +import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; + +public class TelemetryRecorderAdapter implements RecorderTelemetry { + private final TelemetryRuntimeProducer mTelemetryProducer; + private final OperationType mOperationType; + + public TelemetryRecorderAdapter(@NonNull TelemetryRuntimeProducer telemetryProducer, + @NonNull OperationType operationType) { + mTelemetryProducer = telemetryProducer; + mOperationType = operationType; + } + + @Override + public void recordSuccess(long timestamp) { + mTelemetryProducer.recordSuccessfulSync(mOperationType, timestamp); + } + + @Override + public void recordError(Integer httpStatus) { + mTelemetryProducer.recordSyncError(mOperationType, httpStatus); + } + + @Override + public void recordLatency(long latencyMs) { + mTelemetryProducer.recordSyncLatency(mOperationType, latencyMs); + } +} diff --git a/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java b/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java index d380af4e7..c90857deb 100644 --- a/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java +++ b/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java @@ -1,116 +1,37 @@ package io.split.android.client.service.events; -import static io.split.android.client.utils.Utils.checkNotNull; -import static io.split.android.client.utils.Utils.partition; - import androidx.annotation.NonNull; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import io.split.android.client.dtos.Event; -import io.split.android.client.service.executor.SplitTask; -import io.split.android.client.service.executor.SplitTaskExecutionInfo; -import io.split.android.client.service.executor.SplitTaskExecutionStatus; +import io.split.android.client.service.HttpRecorderSubmitterAdapter; +import io.split.android.client.service.TelemetryRecorderAdapter; import io.split.android.client.service.executor.SplitTaskType; import io.split.android.client.service.http.HttpRecorder; -import io.split.android.client.service.http.HttpRecorderException; -import io.split.android.client.service.http.HttpStatus; import io.split.android.client.storage.events.PersistentEventsStorage; +import io.split.android.client.submitter.RecorderTask; import io.split.android.client.telemetry.model.OperationType; import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; -import io.split.android.client.utils.logger.Logger; -public class EventsRecorderTask implements SplitTask { - public final static int FAILING_CHUNK_SIZE = 20; - private final PersistentEventsStorage mPersistentEventsStorage; - private final HttpRecorder> mHttpRecorder; - private final EventsRecorderTaskConfig mConfig; - private final TelemetryRuntimeProducer mTelemetryRuntimeProducer; +public class EventsRecorderTask extends RecorderTask> { + + public static final int FAILING_CHUNK_SIZE = 20; public EventsRecorderTask(@NonNull HttpRecorder> httpRecorder, - @NonNull PersistentEventsStorage persistentEventsStorage, + @NonNull PersistentEventsStorage storage, @NonNull EventsRecorderTaskConfig config, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer) { - mHttpRecorder = checkNotNull(httpRecorder); - mPersistentEventsStorage = checkNotNull(persistentEventsStorage); - mConfig = checkNotNull(config); - mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); + super(storage, + new HttpRecorderSubmitterAdapter<>(httpRecorder), + config.getEventsPerPush(), + SplitTaskType.EVENTS_RECORDER, + new TelemetryRecorderAdapter(telemetryRuntimeProducer, OperationType.EVENTS), + FAILING_CHUNK_SIZE); } @Override - @NonNull - public SplitTaskExecutionInfo execute() { - SplitTaskExecutionStatus status = SplitTaskExecutionStatus.SUCCESS; - int nonSentRecords = 0; - long nonSentBytes = 0; - List events; - List failingEvents = new ArrayList<>(); - boolean doNotRetry = false; - do { - events = mPersistentEventsStorage.pop(mConfig.getEventsPerPush()); - if (events.size() > 0) { - long startTime = System.currentTimeMillis(); - long latency = 0; - try { - Logger.d("Posting %d Split events", events.size()); - mHttpRecorder.execute(events); - - long now = System.currentTimeMillis(); - latency = now - startTime; - mTelemetryRuntimeProducer.recordSuccessfulSync(OperationType.EVENTS, now); - - mPersistentEventsStorage.delete(events); - Logger.d("%d split events sent", events.size()); - } catch (HttpRecorderException e) { - status = SplitTaskExecutionStatus.ERROR; - nonSentRecords += mConfig.getEventsPerPush(); - nonSentBytes += sumEventBytes(events); - Logger.e("Event recorder task: Some events couldn't be sent" + - "Saving to send them in a new iteration: " + - e.getLocalizedMessage()); - failingEvents.addAll(events); - - mTelemetryRuntimeProducer.recordSyncError(OperationType.EVENTS, e.getHttpStatus()); - - if (HttpStatus.isNotRetryable(e.getHttpStatus())) { - doNotRetry = true; - break; - } - } finally { - mTelemetryRuntimeProducer.recordSyncLatency(OperationType.EVENTS, latency); - } - } - } while (events.size() == mConfig.getEventsPerPush()); - - // Update events by chunks to avoid sqlite errors - List> failingChunks = partition(failingEvents, FAILING_CHUNK_SIZE); - for (List chunk : failingChunks) { - mPersistentEventsStorage.setActive(chunk); - } - - if (status == SplitTaskExecutionStatus.ERROR) { - Map data = new HashMap<>(); - data.put(SplitTaskExecutionInfo.NON_SENT_RECORDS, nonSentRecords); - data.put(SplitTaskExecutionInfo.NON_SENT_BYTES, nonSentBytes); - if (doNotRetry) { - data.put(SplitTaskExecutionInfo.DO_NOT_RETRY, true); - } - - return SplitTaskExecutionInfo.error( - SplitTaskType.EVENTS_RECORDER, data); - } - return SplitTaskExecutionInfo.success(SplitTaskType.EVENTS_RECORDER); - } - - private long sumEventBytes(List events) { - long totalBytes = 0; - for (Event event : events) { - totalBytes += event.getSizeInBytes(); - } - return totalBytes; + protected long estimateItemSize(Event item) { + return item.getSizeInBytes(); } } diff --git a/main/src/main/java/io/split/android/client/service/impressions/ImpressionsCountRecorderTask.java b/main/src/main/java/io/split/android/client/service/impressions/ImpressionsCountRecorderTask.java index cd761d75b..976d73ef3 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/ImpressionsCountRecorderTask.java +++ b/main/src/main/java/io/split/android/client/service/impressions/ImpressionsCountRecorderTask.java @@ -1,96 +1,34 @@ package io.split.android.client.service.impressions; -import static io.split.android.client.utils.Utils.checkNotNull; - import androidx.annotation.NonNull; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; +import io.split.android.client.service.HttpRecorderSubmitterAdapter; import io.split.android.client.service.ServiceConstants; -import io.split.android.client.service.executor.SplitTask; -import io.split.android.client.service.executor.SplitTaskExecutionInfo; -import io.split.android.client.service.executor.SplitTaskExecutionStatus; +import io.split.android.client.service.TelemetryRecorderAdapter; import io.split.android.client.service.executor.SplitTaskType; import io.split.android.client.service.http.HttpRecorder; -import io.split.android.client.service.http.HttpRecorderException; -import io.split.android.client.service.http.HttpStatus; import io.split.android.client.storage.impressions.PersistentImpressionsCountStorage; +import io.split.android.client.submitter.RecorderTask; import io.split.android.client.telemetry.model.OperationType; import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; -import io.split.android.client.utils.logger.Logger; -public class ImpressionsCountRecorderTask implements SplitTask { - private final PersistentImpressionsCountStorage mPersistentStorage; - private final HttpRecorder mHttpRecorder; - private static int POP_COUNT = ServiceConstants.DEFAULT_IMPRESSION_COUNT_ROWS_POP; - private final TelemetryRuntimeProducer mTelemetryRuntimeProducer; +public class ImpressionsCountRecorderTask extends RecorderTask { public ImpressionsCountRecorderTask(@NonNull HttpRecorder httpRecorder, @NonNull PersistentImpressionsCountStorage persistentStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer) { - mHttpRecorder = checkNotNull(httpRecorder); - mPersistentStorage = checkNotNull(persistentStorage); - mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); + super(persistentStorage, + new HttpRecorderSubmitterAdapter<>(httpRecorder), + ServiceConstants.DEFAULT_IMPRESSION_COUNT_ROWS_POP, + SplitTaskType.IMPRESSIONS_COUNT_RECORDER, + new TelemetryRecorderAdapter(telemetryRuntimeProducer, OperationType.IMPRESSIONS_COUNT), + 0); } @Override - @NonNull - public SplitTaskExecutionInfo execute() { - SplitTaskExecutionStatus status = SplitTaskExecutionStatus.SUCCESS; - - List countList = new ArrayList<>(); - List failedSent = new ArrayList<>(); - boolean doNotRetry = false; - do { - countList = mPersistentStorage.pop(POP_COUNT); - if (countList.size() > 0) { - long startTime = System.currentTimeMillis(); - long latency = 0; - try { - Logger.d("Posting %d Split impressions count", countList.size()); - mHttpRecorder.execute(new ImpressionsCount(countList)); - - long now = System.currentTimeMillis(); - latency = now - startTime; - mTelemetryRuntimeProducer.recordSuccessfulSync(OperationType.IMPRESSIONS_COUNT, now); - - mPersistentStorage.delete(countList); - Logger.d("%d split impressions count sent", countList.size()); - } catch (HttpRecorderException e) { - status = SplitTaskExecutionStatus.ERROR; - Logger.e("Impressions count recorder task: Some counts couldn't be sent. " + - "Saving to send them in a new iteration\n" + - e.getLocalizedMessage()); - failedSent.addAll(countList); - - mTelemetryRuntimeProducer.recordSyncError(OperationType.IMPRESSIONS_COUNT, e.getHttpStatus()); - - if (HttpStatus.isNotRetryable(HttpStatus.fromCode(e.getHttpStatus()))) { - doNotRetry = true; - break; - } - } finally { - mTelemetryRuntimeProducer.recordSyncLatency(OperationType.IMPRESSIONS_COUNT, latency); - } - } - } while (countList.size() == POP_COUNT); - - if (failedSent.size() > 0) { - mPersistentStorage.setActive(failedSent); - } - - if (status == SplitTaskExecutionStatus.ERROR) { - Map data = new HashMap<>(); - if (doNotRetry) { - data.put(SplitTaskExecutionInfo.DO_NOT_RETRY, true); - } - - return SplitTaskExecutionInfo.error(SplitTaskType.IMPRESSIONS_COUNT_RECORDER, data); - } - - return SplitTaskExecutionInfo.success(SplitTaskType.IMPRESSIONS_COUNT_RECORDER); + protected ImpressionsCount transformForSubmission(List items) { + return new ImpressionsCount(items); } } diff --git a/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java b/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java index 7a4b122c9..c9e7aba55 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java +++ b/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java @@ -1,112 +1,38 @@ package io.split.android.client.service.impressions; -import static io.split.android.client.utils.Utils.checkNotNull; - import androidx.annotation.NonNull; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import io.split.android.client.dtos.KeyImpression; -import io.split.android.client.service.executor.SplitTask; -import io.split.android.client.service.executor.SplitTaskExecutionInfo; -import io.split.android.client.service.executor.SplitTaskExecutionStatus; +import io.split.android.client.service.HttpRecorderSubmitterAdapter; +import io.split.android.client.service.TelemetryRecorderAdapter; import io.split.android.client.service.executor.SplitTaskType; import io.split.android.client.service.http.HttpRecorder; -import io.split.android.client.service.http.HttpRecorderException; -import io.split.android.client.service.http.HttpStatus; import io.split.android.client.storage.impressions.PersistentImpressionsStorage; +import io.split.android.client.submitter.RecorderTask; import io.split.android.client.telemetry.model.OperationType; import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; -import io.split.android.client.utils.logger.Logger; -public class ImpressionsRecorderTask implements SplitTask { - public final static int FAILING_CHUNK_SIZE = 20; - private final PersistentImpressionsStorage mPersistenImpressionsStorage; - private final HttpRecorder> mHttpRecorder; - private final ImpressionsRecorderTaskConfig mConfig; - private final TelemetryRuntimeProducer mTelemetryRuntimeProducer; +public class ImpressionsRecorderTask extends RecorderTask> { + + private final long mEstimatedSizeInBytes; public ImpressionsRecorderTask(@NonNull HttpRecorder> httpRecorder, - @NonNull PersistentImpressionsStorage persistenEventsStorage, + @NonNull PersistentImpressionsStorage storage, @NonNull ImpressionsRecorderTaskConfig config, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer) { - mHttpRecorder = checkNotNull(httpRecorder); - mPersistenImpressionsStorage = checkNotNull(persistenEventsStorage); - mConfig = checkNotNull(config); - mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); + super(storage, + new HttpRecorderSubmitterAdapter<>(httpRecorder), + config.getImpressionsPerPush(), + SplitTaskType.IMPRESSIONS_RECORDER, + new TelemetryRecorderAdapter(telemetryRuntimeProducer, OperationType.IMPRESSIONS), + 0); + this.mEstimatedSizeInBytes = config.getEstimatedSizeInBytes(); } @Override - @NonNull - public SplitTaskExecutionInfo execute() { - SplitTaskExecutionStatus status = SplitTaskExecutionStatus.SUCCESS; - int nonSentRecords = 0; - long nonSentBytes = 0; - List impressions; - List failingImpressions = new ArrayList<>(); - boolean doNotRetry = false; - do { - impressions = mPersistenImpressionsStorage.pop(mConfig.getImpressionsPerPush()); - if (impressions.size() > 0) { - long startTime = System.currentTimeMillis(); - long latency = 0; - try { - Logger.d("Posting %d Split impressions", impressions.size()); - mHttpRecorder.execute(impressions); - - long now = System.currentTimeMillis(); - latency = now - startTime; - mTelemetryRuntimeProducer.recordSuccessfulSync(OperationType.IMPRESSIONS, now); - - mPersistenImpressionsStorage.delete(impressions); - Logger.d("%d split impressions sent", impressions.size()); - } catch (HttpRecorderException e) { - status = SplitTaskExecutionStatus.ERROR; - nonSentRecords += mConfig.getImpressionsPerPush(); - nonSentBytes += sumImpressionsBytes(impressions); - Logger.e("Impressions recorder task: Some impressions couldn't be sent. " + - "Saving to send them in a new iteration\n" + - e.getLocalizedMessage()); - failingImpressions.addAll(impressions); - - mTelemetryRuntimeProducer.recordSyncError(OperationType.IMPRESSIONS, e.getHttpStatus()); - - if (HttpStatus.isNotRetryable(HttpStatus.fromCode(e.getHttpStatus()))) { - doNotRetry = true; - break; - } - } finally { - mTelemetryRuntimeProducer.recordSyncLatency(OperationType.IMPRESSIONS, latency); - } - } - } while (impressions.size() == mConfig.getImpressionsPerPush()); - - if (failingImpressions.size() > 0) { - mPersistenImpressionsStorage.setActive(failingImpressions); - } - - if (status == SplitTaskExecutionStatus.ERROR) { - Map data = new HashMap<>(); - data.put(SplitTaskExecutionInfo.NON_SENT_RECORDS, nonSentRecords); - data.put(SplitTaskExecutionInfo.NON_SENT_BYTES, nonSentBytes); - if (doNotRetry) { - data.put(SplitTaskExecutionInfo.DO_NOT_RETRY, true); - } - - return SplitTaskExecutionInfo.error( - SplitTaskType.IMPRESSIONS_RECORDER, data); - } - return SplitTaskExecutionInfo.success(SplitTaskType.IMPRESSIONS_RECORDER); - } - - private long sumImpressionsBytes(List impressions) { - long totalBytes = 0; - for (KeyImpression impression : impressions) { - totalBytes += mConfig.getEstimatedSizeInBytes(); - } - return totalBytes; + protected long estimateItemSize(KeyImpression item) { + return mEstimatedSizeInBytes; } } diff --git a/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugStrategy.java b/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugStrategy.java index 79ea6dc1c..8fe7e3a99 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugStrategy.java +++ b/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugStrategy.java @@ -16,7 +16,7 @@ import io.split.android.client.service.executor.SplitTaskExecutor; import io.split.android.client.service.impressions.ImpressionsTaskFactory; import io.split.android.client.service.impressions.observer.ImpressionsObserver; -import io.split.android.client.service.synchronizer.RecorderSyncHelper; +import io.split.android.client.submitter.RecorderSyncHelper; import io.split.android.client.telemetry.model.ImpressionsDataType; import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; diff --git a/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java b/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java index 36d86ff7b..ad1d8219f 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java +++ b/main/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java @@ -15,7 +15,7 @@ import io.split.android.client.service.impressions.ImpressionsTaskFactory; import io.split.android.client.service.impressions.observer.ImpressionsObserver; import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTimer; -import io.split.android.client.service.synchronizer.RecorderSyncHelper; +import io.split.android.client.submitter.RecorderSyncHelper; class DebugTracker implements PeriodicTracker { diff --git a/main/src/main/java/io/split/android/client/service/impressions/strategy/ImpressionStrategyProvider.java b/main/src/main/java/io/split/android/client/service/impressions/strategy/ImpressionStrategyProvider.java index c331ee61f..c76b65dc1 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/strategy/ImpressionStrategyProvider.java +++ b/main/src/main/java/io/split/android/client/service/impressions/strategy/ImpressionStrategyProvider.java @@ -13,7 +13,7 @@ import io.split.android.client.service.impressions.observer.ImpressionsObserverImpl; import io.split.android.client.service.impressions.unique.UniqueKeysTracker; import io.split.android.client.service.impressions.unique.UniqueKeysTrackerImpl; -import io.split.android.client.service.synchronizer.RecorderSyncHelperImpl; +import io.split.android.client.submitter.RecorderSyncHelperImpl; import io.split.android.client.storage.common.SplitStorageContainer; import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; diff --git a/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedStrategy.java b/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedStrategy.java index 23f7c4b7c..5c3b85323 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedStrategy.java +++ b/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedStrategy.java @@ -18,7 +18,7 @@ import io.split.android.client.service.impressions.ImpressionsCounter; import io.split.android.client.service.impressions.ImpressionsTaskFactory; import io.split.android.client.service.impressions.observer.ImpressionsObserver; -import io.split.android.client.service.synchronizer.RecorderSyncHelper; +import io.split.android.client.submitter.RecorderSyncHelper; import io.split.android.client.telemetry.model.ImpressionsDataType; import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer; diff --git a/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java b/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java index 5f441dfaf..3ca2bc608 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java +++ b/main/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java @@ -15,7 +15,7 @@ import io.split.android.client.service.impressions.ImpressionsTaskFactory; import io.split.android.client.service.impressions.observer.ImpressionsObserver; import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTimer; -import io.split.android.client.service.synchronizer.RecorderSyncHelper; +import io.split.android.client.submitter.RecorderSyncHelper; class OptimizedTracker implements PeriodicTracker { diff --git a/main/src/main/java/io/split/android/client/service/impressions/unique/UniqueKeysRecorderTask.java b/main/src/main/java/io/split/android/client/service/impressions/unique/UniqueKeysRecorderTask.java index a258f16ce..cbafed13f 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/unique/UniqueKeysRecorderTask.java +++ b/main/src/main/java/io/split/android/client/service/impressions/unique/UniqueKeysRecorderTask.java @@ -1,7 +1,5 @@ package io.split.android.client.service.impressions.unique; -import static io.split.android.client.utils.Utils.checkNotNull; - import androidx.annotation.NonNull; import java.util.ArrayList; @@ -11,111 +9,49 @@ import java.util.Map; import java.util.Set; -import io.split.android.client.service.executor.SplitTask; -import io.split.android.client.service.executor.SplitTaskExecutionInfo; -import io.split.android.client.service.executor.SplitTaskExecutionStatus; +import io.split.android.client.service.HttpRecorderSubmitterAdapter; import io.split.android.client.service.executor.SplitTaskType; import io.split.android.client.service.http.HttpRecorder; -import io.split.android.client.service.http.HttpRecorderException; -import io.split.android.client.service.http.HttpStatus; import io.split.android.client.storage.impressions.PersistentImpressionsUniqueStorage; -import io.split.android.client.utils.logger.Logger; +import io.split.android.client.submitter.RecorderTask; -public class UniqueKeysRecorderTask implements SplitTask { +public class UniqueKeysRecorderTask extends RecorderTask { - private final HttpRecorder mHttpRecorder; - private final PersistentImpressionsUniqueStorage mStorage; - private final UniqueKeysRecorderTaskConfig mConfig; + private final long mEstimatedSizeInBytes; public UniqueKeysRecorderTask(@NonNull HttpRecorder uniqueImpressionsRecorder, @NonNull PersistentImpressionsUniqueStorage storage, @NonNull UniqueKeysRecorderTaskConfig config) { - mHttpRecorder = checkNotNull(uniqueImpressionsRecorder); - mStorage = checkNotNull(storage); - mConfig = checkNotNull(config); + super(storage, + new HttpRecorderSubmitterAdapter<>(uniqueImpressionsRecorder), + config.getElementsPerPush(), + SplitTaskType.UNIQUE_KEYS_RECORDER_TASK, + null, + 0); + this.mEstimatedSizeInBytes = config.getEstimatedSizeInBytes(); } - @NonNull @Override - public SplitTaskExecutionInfo execute() { - SplitTaskExecutionStatus status = SplitTaskExecutionStatus.SUCCESS; - int nonSentRecords = 0; - long nonSentBytes = 0; - List keys; - List failingKeys = new ArrayList<>(); - boolean doNotRetry = false; - do { - keys = mStorage.pop(mConfig.getElementsPerPush()); - if (keys.size() > 0) { - try { - Logger.d("Posting %d Split MTKs", keys.size()); - mHttpRecorder.execute(buildMTK(keys)); - - mStorage.delete(keys); - Logger.d("%d split MTKs sent", keys.size()); - } catch (HttpRecorderException e) { - status = SplitTaskExecutionStatus.ERROR; - nonSentRecords += mConfig.getElementsPerPush(); - nonSentBytes += sumImpressionsBytes(keys); - Logger.e("MTKs recorder task: Some keys couldn't be sent. " + - "Saving to send them in a new iteration\n" + - e.getLocalizedMessage()); - failingKeys.addAll(keys); - - if (HttpStatus.isNotRetryable(HttpStatus.fromCode(e.getHttpStatus()))) { - doNotRetry = true; - break; - } - } - } - } while (keys.size() == mConfig.getElementsPerPush()); - - if (failingKeys.size() > 0) { - mStorage.setActive(failingKeys); - } - - if (status == SplitTaskExecutionStatus.ERROR) { - Map data = new HashMap<>(); - data.put(SplitTaskExecutionInfo.NON_SENT_RECORDS, nonSentRecords); - data.put(SplitTaskExecutionInfo.NON_SENT_BYTES, nonSentBytes); - if (doNotRetry) { - data.put(SplitTaskExecutionInfo.DO_NOT_RETRY, true); - } - - return SplitTaskExecutionInfo.error( - SplitTaskType.UNIQUE_KEYS_RECORDER_TASK, data); - } - - return SplitTaskExecutionInfo.success(SplitTaskType.UNIQUE_KEYS_RECORDER_TASK); - } - - @NonNull - private static MTK buildMTK(List keys) { + protected MTK transformForSubmission(List items) { Map map = new HashMap<>(); - for (UniqueKey key : keys) { + for (UniqueKey key : items) { String userKey = key.getKey(); if (!map.containsKey(userKey)) { map.put(userKey, new UniqueKey(userKey, new HashSet<>())); } - UniqueKey uniqueKey = map.get(userKey); if (uniqueKey != null) { Set originalFeatures = uniqueKey.getFeatures(); Set newFeatures = key.getFeatures(); newFeatures.addAll(originalFeatures); - map.put(userKey, new UniqueKey(userKey, newFeatures)); } } - return new MTK(new ArrayList<>(map.values())); } - private long sumImpressionsBytes(List keys) { - long totalBytes = 0; - for (UniqueKey key : keys) { - totalBytes += mConfig.getEstimatedSizeInBytes(); - } - return totalBytes; + @Override + protected long estimateItemSize(UniqueKey item) { + return mEstimatedSizeInBytes; } } diff --git a/main/src/main/java/io/split/android/client/service/synchronizer/SynchronizerImpl.java b/main/src/main/java/io/split/android/client/service/synchronizer/SynchronizerImpl.java index abf55e7fe..51d6feaa4 100644 --- a/main/src/main/java/io/split/android/client/service/synchronizer/SynchronizerImpl.java +++ b/main/src/main/java/io/split/android/client/service/synchronizer/SynchronizerImpl.java @@ -30,7 +30,9 @@ import io.split.android.client.service.synchronizer.mysegments.MySegmentsSynchronizerRegistry; import io.split.android.client.service.synchronizer.mysegments.MySegmentsSynchronizerRegistryImpl; import io.split.android.client.shared.UserConsent; -import io.split.android.client.storage.common.StoragePusher; +import io.split.android.client.submitter.RecorderSyncHelper; +import io.split.android.client.submitter.RecorderSyncHelperImpl; +import io.split.android.client.submitter.StoragePusher; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.model.EventsDataRecordsEnum; import io.split.android.client.telemetry.model.streaming.SyncModeUpdateStreamingEvent; diff --git a/main/src/main/java/io/split/android/client/storage/common/PersistentStorage.java b/main/src/main/java/io/split/android/client/storage/common/PersistentStorage.java index c111303d1..21744d98f 100644 --- a/main/src/main/java/io/split/android/client/storage/common/PersistentStorage.java +++ b/main/src/main/java/io/split/android/client/storage/common/PersistentStorage.java @@ -4,16 +4,15 @@ import java.util.List; -public interface PersistentStorage extends StoragePusher { +import io.split.android.client.submitter.RecorderStorage; +import io.split.android.client.submitter.StoragePusher; + +public interface PersistentStorage extends StoragePusher, RecorderStorage { // Push method is defined in StoragePusher interface void pushMany(@NonNull List elements); - List pop(int count); - - void setActive(@NonNull List elements); - - void delete(@NonNull List elements); + // pop, delete, and setActive are inherited from RecorderStorage void deleteInvalid(long maxTimestamp); } diff --git a/main/src/main/java/io/split/android/client/storage/events/EventsStorage.java b/main/src/main/java/io/split/android/client/storage/events/EventsStorage.java index 1eb2d1ae7..5da50977a 100644 --- a/main/src/main/java/io/split/android/client/storage/events/EventsStorage.java +++ b/main/src/main/java/io/split/android/client/storage/events/EventsStorage.java @@ -11,7 +11,7 @@ import io.split.android.client.dtos.Event; import io.split.android.client.storage.common.Storage; -import io.split.android.client.storage.common.StoragePusher; +import io.split.android.client.submitter.StoragePusher; import io.split.android.client.utils.logger.Logger; public class EventsStorage implements Storage, StoragePusher { diff --git a/main/src/main/java/io/split/android/client/storage/impressions/ImpressionsStorage.java b/main/src/main/java/io/split/android/client/storage/impressions/ImpressionsStorage.java index 403fbe220..188e3d2db 100644 --- a/main/src/main/java/io/split/android/client/storage/impressions/ImpressionsStorage.java +++ b/main/src/main/java/io/split/android/client/storage/impressions/ImpressionsStorage.java @@ -12,7 +12,7 @@ import io.split.android.client.dtos.KeyImpression; import io.split.android.client.storage.common.PersistentStorage; import io.split.android.client.storage.common.Storage; -import io.split.android.client.storage.common.StoragePusher; +import io.split.android.client.submitter.StoragePusher; import io.split.android.client.utils.logger.Logger; public class ImpressionsStorage implements Storage, StoragePusher { diff --git a/main/src/test/java/io/split/android/client/service/SynchronizerTest.java b/main/src/test/java/io/split/android/client/service/SynchronizerTest.java index 6a5e4aa15..db14dcc2b 100644 --- a/main/src/test/java/io/split/android/client/service/SynchronizerTest.java +++ b/main/src/test/java/io/split/android/client/service/SynchronizerTest.java @@ -71,7 +71,7 @@ import io.split.android.client.service.splits.SplitsUpdateTask; import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTimer; import io.split.android.client.service.synchronizer.FeatureFlagsSynchronizer; -import io.split.android.client.service.synchronizer.RecorderSyncHelper; +import io.split.android.client.submitter.RecorderSyncHelper; import io.split.android.client.service.synchronizer.SynchronizerImpl; import io.split.android.client.service.synchronizer.WorkManagerWrapper; import io.split.android.client.service.synchronizer.attributes.AttributesSynchronizer; diff --git a/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt b/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt index 333a16b8d..c6c1d0bf9 100644 --- a/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt +++ b/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt @@ -8,7 +8,7 @@ import io.split.android.client.service.executor.SplitTaskType import io.split.android.client.service.impressions.ImpressionsRecorderTask import io.split.android.client.service.impressions.ImpressionsTaskFactory import io.split.android.client.service.impressions.observer.ImpressionsObserver -import io.split.android.client.service.synchronizer.RecorderSyncHelper +import io.split.android.client.submitter.RecorderSyncHelper import io.split.android.client.telemetry.model.ImpressionsDataType import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer import org.junit.Before diff --git a/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt b/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt index 6ddca42f7..66e17142c 100644 --- a/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt +++ b/main/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt @@ -9,7 +9,7 @@ import io.split.android.client.service.impressions.ImpressionsRecorderTask import io.split.android.client.service.impressions.ImpressionsTaskFactory import io.split.android.client.service.impressions.observer.ImpressionsObserver import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTimer -import io.split.android.client.service.synchronizer.RecorderSyncHelper +import io.split.android.client.submitter.RecorderSyncHelper import org.junit.Before import org.junit.Test import org.mockito.ArgumentCaptor diff --git a/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt b/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt index 198e4d43b..ddd5967c8 100644 --- a/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt +++ b/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt @@ -11,7 +11,7 @@ import io.split.android.client.service.impressions.ImpressionsCounter import io.split.android.client.service.impressions.ImpressionsRecorderTask import io.split.android.client.service.impressions.ImpressionsTaskFactory import io.split.android.client.service.impressions.observer.ImpressionsObserverImpl -import io.split.android.client.service.synchronizer.RecorderSyncHelper +import io.split.android.client.submitter.RecorderSyncHelper import io.split.android.client.telemetry.model.ImpressionsDataType import io.split.android.client.telemetry.storage.TelemetryRuntimeProducer import org.junit.Before diff --git a/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt b/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt index 71915b9ca..5bd1c4952 100644 --- a/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt +++ b/main/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt @@ -10,7 +10,7 @@ import io.split.android.client.service.executor.SplitTaskType import io.split.android.client.service.impressions.* import io.split.android.client.service.impressions.observer.ImpressionsObserver import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTimer -import io.split.android.client.service.synchronizer.RecorderSyncHelper +import io.split.android.client.submitter.RecorderSyncHelper import org.junit.Before import org.junit.Test import org.mockito.ArgumentCaptor diff --git a/main/src/test/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImplTest.java b/main/src/test/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImplTest.java index 1888bf831..dfdaf24a8 100644 --- a/main/src/test/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImplTest.java +++ b/main/src/test/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImplTest.java @@ -11,7 +11,8 @@ import io.split.android.client.service.executor.SplitTaskExecutionListener; import io.split.android.client.service.executor.SplitTaskExecutor; import io.split.android.client.service.executor.SplitTaskType; -import io.split.android.client.storage.common.StoragePusher; +import io.split.android.client.submitter.RecorderSyncHelperImpl; +import io.split.android.client.submitter.StoragePusher; public class RecorderSyncHelperImplTest { diff --git a/settings.gradle b/settings.gradle index 4b3af1a51..22c935f1b 100644 --- a/settings.gradle +++ b/settings.gradle @@ -10,3 +10,4 @@ include ':events' include ':events-domain' include ':backoff' include ':tracker' +include ':submitter' diff --git a/submitter/.gitignore b/submitter/.gitignore new file mode 100644 index 000000000..796b96d1c --- /dev/null +++ b/submitter/.gitignore @@ -0,0 +1 @@ +/build diff --git a/submitter/README.md b/submitter/README.md new file mode 100644 index 000000000..dcff89273 --- /dev/null +++ b/submitter/README.md @@ -0,0 +1,13 @@ +# submitter + +Generic batch recorder task abstraction for the Split Android SDK. + +## Purpose + +Encapsulates the logic for submitting batched data (such as impressions and events) to the Split platform. It provides a reusable abstraction for recorder tasks, decoupled from the SDK's internal storage and networking layers — dependencies are injected via callbacks. + +## Design notes + +- Depends on `events-domain` for shared domain types. +- Depends on `logger` for logging. +- No dependency on `main/` internals or networking implementation details. diff --git a/submitter/build.gradle b/submitter/build.gradle new file mode 100644 index 000000000..906878de7 --- /dev/null +++ b/submitter/build.gradle @@ -0,0 +1,23 @@ +plugins { + id 'com.android.library' +} + +apply from: "$projectDir/../gradle/common-android-library.gradle" + +android { + namespace 'io.split.android.client.submitter' + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + targetCompatibility JavaVersion.VERSION_1_8 + } +} + +dependencies { + api clientModuleProject('events-domain') + implementation clientModuleProject('logger') + implementation libs.annotation + + testImplementation libs.junit4 + testImplementation libs.mockitoCore +} diff --git a/submitter/src/main/AndroidManifest.xml b/submitter/src/main/AndroidManifest.xml new file mode 100644 index 000000000..9a40236b9 --- /dev/null +++ b/submitter/src/main/AndroidManifest.xml @@ -0,0 +1,3 @@ + + + diff --git a/main/src/main/java/io/split/android/client/storage/common/InBytesSizable.java b/submitter/src/main/java/io/split/android/client/submitter/InBytesSizable.java similarity index 57% rename from main/src/main/java/io/split/android/client/storage/common/InBytesSizable.java rename to submitter/src/main/java/io/split/android/client/submitter/InBytesSizable.java index 645224611..6cb1fbcd1 100644 --- a/main/src/main/java/io/split/android/client/storage/common/InBytesSizable.java +++ b/submitter/src/main/java/io/split/android/client/submitter/InBytesSizable.java @@ -1,4 +1,4 @@ -package io.split.android.client.storage.common; +package io.split.android.client.submitter; public interface InBytesSizable { long getSizeInBytes(); diff --git a/submitter/src/main/java/io/split/android/client/submitter/RecorderException.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderException.java new file mode 100644 index 000000000..31e49ffcb --- /dev/null +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderException.java @@ -0,0 +1,20 @@ +package io.split.android.client.submitter; + +public class RecorderException extends Exception { + private final Integer mHttpStatus; + private final boolean mRetryable; + + public RecorderException(String message, Integer httpStatus, boolean retryable) { + super(message); + this.mHttpStatus = httpStatus; + this.mRetryable = retryable; + } + + public Integer getHttpStatus() { + return mHttpStatus; + } + + public boolean isRetryable() { + return mRetryable; + } +} diff --git a/submitter/src/main/java/io/split/android/client/submitter/RecorderStorage.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderStorage.java new file mode 100644 index 000000000..350190c54 --- /dev/null +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderStorage.java @@ -0,0 +1,10 @@ +package io.split.android.client.submitter; + +import androidx.annotation.NonNull; +import java.util.List; + +public interface RecorderStorage { + List pop(int count); + void delete(@NonNull List items); + void setActive(@NonNull List items); +} diff --git a/submitter/src/main/java/io/split/android/client/submitter/RecorderSubmitter.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderSubmitter.java new file mode 100644 index 000000000..9ea278617 --- /dev/null +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderSubmitter.java @@ -0,0 +1,7 @@ +package io.split.android.client.submitter; + +import androidx.annotation.NonNull; + +public interface RecorderSubmitter { + void execute(@NonNull T data) throws RecorderException; +} diff --git a/main/src/main/java/io/split/android/client/service/synchronizer/RecorderSyncHelper.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderSyncHelper.java similarity index 75% rename from main/src/main/java/io/split/android/client/service/synchronizer/RecorderSyncHelper.java rename to submitter/src/main/java/io/split/android/client/submitter/RecorderSyncHelper.java index fc84c75e4..fd70cba57 100644 --- a/main/src/main/java/io/split/android/client/service/synchronizer/RecorderSyncHelper.java +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderSyncHelper.java @@ -1,7 +1,6 @@ -package io.split.android.client.service.synchronizer; +package io.split.android.client.submitter; import io.split.android.client.service.executor.SplitTaskExecutionListener; -import io.split.android.client.storage.common.InBytesSizable; public interface RecorderSyncHelper extends SplitTaskExecutionListener { boolean pushAndCheckIfFlushNeeded(T entity); diff --git a/main/src/main/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImpl.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderSyncHelperImpl.java similarity index 90% rename from main/src/main/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImpl.java rename to submitter/src/main/java/io/split/android/client/submitter/RecorderSyncHelperImpl.java index 4351930fb..09488a69e 100644 --- a/main/src/main/java/io/split/android/client/service/synchronizer/RecorderSyncHelperImpl.java +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderSyncHelperImpl.java @@ -1,11 +1,10 @@ -package io.split.android.client.service.synchronizer; - -import static io.split.android.client.utils.Utils.checkNotNull; +package io.split.android.client.submitter; import androidx.annotation.NonNull; import java.lang.ref.WeakReference; import java.util.HashSet; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; @@ -16,8 +15,6 @@ import io.split.android.client.service.executor.SplitTaskExecutionStatus; import io.split.android.client.service.executor.SplitTaskExecutor; import io.split.android.client.service.executor.SplitTaskType; -import io.split.android.client.storage.common.InBytesSizable; -import io.split.android.client.storage.common.StoragePusher; public class RecorderSyncHelperImpl implements RecorderSyncHelper { @@ -35,9 +32,9 @@ public RecorderSyncHelperImpl(SplitTaskType taskType, int maxQueueSize, long maxQueueSizeInBytes, SplitTaskExecutor splitTaskExecutor) { - mTaskType = checkNotNull(taskType); - mStorage = checkNotNull(storage); - mSplitTaskExecutor = checkNotNull(splitTaskExecutor); + mTaskType = Objects.requireNonNull(taskType); + mStorage = Objects.requireNonNull(storage); + mSplitTaskExecutor = Objects.requireNonNull(splitTaskExecutor); mPushedCount = new AtomicInteger(0); mTotalPushedSizeInBytes = new AtomicLong(0); mMaxQueueSize = maxQueueSize; diff --git a/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java new file mode 100644 index 000000000..435cc3299 --- /dev/null +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java @@ -0,0 +1,157 @@ +package io.split.android.client.submitter; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import io.split.android.client.service.executor.SplitTask; +import io.split.android.client.service.executor.SplitTaskExecutionInfo; +import io.split.android.client.service.executor.SplitTaskExecutionStatus; +import io.split.android.client.service.executor.SplitTaskType; +import io.split.android.client.utils.logger.Logger; + +/** + * Abstract base class for batch submission tasks. + *

+ * Encapsulates the common pop-submit-retry-setActive pattern used by + * impressions, events, and other batch recorder tasks. + * + * @param The storage item type (e.g., KeyImpression, Event) + * @param The submission payload type (e.g., List, ImpressionsCount) + */ +public abstract class RecorderTask implements SplitTask { + + private final RecorderStorage mStorage; + private final RecorderSubmitter mSubmitter; + private final int mBatchSize; + private final SplitTaskType mTaskType; + @Nullable + private final RecorderTelemetry mTelemetry; + private final int mFailingChunkSize; // 0 = no chunking + + protected RecorderTask(@NonNull RecorderStorage storage, + @NonNull RecorderSubmitter submitter, + int batchSize, + @NonNull SplitTaskType taskType, + @Nullable RecorderTelemetry telemetry, + int failingChunkSize) { + mStorage = storage; + mSubmitter = submitter; + mBatchSize = batchSize; + mTaskType = taskType; + mTelemetry = telemetry; + mFailingChunkSize = failingChunkSize; + } + + @NonNull + @Override + public final SplitTaskExecutionInfo execute() { + SplitTaskExecutionStatus status = SplitTaskExecutionStatus.SUCCESS; + int nonSentRecords = 0; + long nonSentBytes = 0; + List items; + List failingItems = new ArrayList<>(); + boolean doNotRetry = false; + + do { + items = mStorage.pop(mBatchSize); + if (!items.isEmpty()) { + long startTime = System.currentTimeMillis(); + try { + R payload = transformForSubmission(items); + mSubmitter.execute(payload); + + long now = System.currentTimeMillis(); + if (mTelemetry != null) { + mTelemetry.recordSuccess(now); + } + + mStorage.delete(items); + } catch (RecorderException e) { + status = SplitTaskExecutionStatus.ERROR; + nonSentRecords += items.size(); + nonSentBytes += sumBytes(items); + Logger.e("RecorderTask: " + items.size() + " items couldn't be submitted. " + + "Saving to retry in a new iteration: " + e.getLocalizedMessage()); + failingItems.addAll(items); + + if (mTelemetry != null) { + mTelemetry.recordError(e.getHttpStatus()); + } + + if (!e.isRetryable()) { + doNotRetry = true; + break; + } + } finally { + if (mTelemetry != null) { + mTelemetry.recordLatency(System.currentTimeMillis() - startTime); + } + } + } + } while (items.size() == mBatchSize); + + // Re-queue failed items for retry + if (!failingItems.isEmpty()) { + if (mFailingChunkSize > 0) { + // Chunk to avoid SQLite errors (used by EventsRecorderTask) + int size = failingItems.size(); + for (int i = 0; i < size; i += mFailingChunkSize) { + mStorage.setActive(failingItems.subList(i, Math.min(i + mFailingChunkSize, size))); + } + } else { + mStorage.setActive(failingItems); + } + } + + if (status == SplitTaskExecutionStatus.ERROR) { + Map data = new HashMap<>(); + data.put(SplitTaskExecutionInfo.NON_SENT_RECORDS, nonSentRecords); + data.put(SplitTaskExecutionInfo.NON_SENT_BYTES, nonSentBytes); + if (doNotRetry) { + data.put(SplitTaskExecutionInfo.DO_NOT_RETRY, true); + } + return SplitTaskExecutionInfo.error(mTaskType, data); + } + + return SplitTaskExecutionInfo.success(mTaskType); + } + + /** + * Transform storage items into the submission payload. + *

+ * Default implementation casts the list directly to R — works when T and R are the same + * type (e.g., {@code List} to {@code List}). + * Override this for tasks that need data transformation before submission + * (e.g., UniqueKeys deduplication, ImpressionsCount wrapping). + *

+ * WARNING: Subclasses where T and R are different types MUST override this + * method. The default unchecked cast will throw a {@link ClassCastException} at runtime if T + * and R differ. Failing to override in that case is a programming error. + */ + @SuppressWarnings("unchecked") + protected R transformForSubmission(List items) { + return (R) items; + } + + /** + * Estimate the byte size of one storage item for tracking non-sent bytes. + *

+ * Default returns 0. Override to enable byte tracking. + */ + protected long estimateItemSize(T item) { + return 0; + } + + private long sumBytes(List items) { + long total = 0; + for (T item : items) { + total += estimateItemSize(item); + } + return total; + } +} diff --git a/submitter/src/main/java/io/split/android/client/submitter/RecorderTelemetry.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderTelemetry.java new file mode 100644 index 000000000..7f8665ab2 --- /dev/null +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderTelemetry.java @@ -0,0 +1,7 @@ +package io.split.android.client.submitter; + +public interface RecorderTelemetry { + void recordSuccess(long timestamp); + void recordError(Integer httpStatus); + void recordLatency(long latencyMs); +} diff --git a/main/src/main/java/io/split/android/client/storage/common/StoragePusher.java b/submitter/src/main/java/io/split/android/client/submitter/StoragePusher.java similarity index 69% rename from main/src/main/java/io/split/android/client/storage/common/StoragePusher.java rename to submitter/src/main/java/io/split/android/client/submitter/StoragePusher.java index aa8e6cd6d..1ec49d849 100644 --- a/main/src/main/java/io/split/android/client/storage/common/StoragePusher.java +++ b/submitter/src/main/java/io/split/android/client/submitter/StoragePusher.java @@ -1,4 +1,4 @@ -package io.split.android.client.storage.common; +package io.split.android.client.submitter; import androidx.annotation.NonNull; diff --git a/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java b/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java new file mode 100644 index 000000000..5314fe5e3 --- /dev/null +++ b/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java @@ -0,0 +1,541 @@ +package io.split.android.client.submitter; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import androidx.annotation.NonNull; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.List; + +import io.split.android.client.service.executor.SplitTaskExecutionInfo; +import io.split.android.client.service.executor.SplitTaskExecutionStatus; +import io.split.android.client.service.executor.SplitTaskType; + +@SuppressWarnings("unchecked") +public class RecorderTaskTest { + + private static final int BATCH_SIZE = 10; + private static final SplitTaskType TASK_TYPE = SplitTaskType.IMPRESSIONS_RECORDER; + + private RecorderStorage mStorage; + private RecorderSubmitter> mSubmitter; + private RecorderTelemetry mTelemetry; + + @Before + public void setUp() { + mStorage = Mockito.mock(RecorderStorage.class); + mSubmitter = Mockito.mock(RecorderSubmitter.class); + mTelemetry = Mockito.mock(RecorderTelemetry.class); + } + + // region Successful submission + + @Test + public void successfulSingleBatchSubmission() throws RecorderException { + List batch = createItems(5); // less than BATCH_SIZE → loop terminates after one iteration + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + verify(mStorage, times(1)).pop(BATCH_SIZE); + verify(mSubmitter, times(1)).execute(batch); + verify(mStorage, times(1)).delete(batch); + verify(mStorage, never()).setActive(any()); + assertEquals(TASK_TYPE, result.getTaskType()); + assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); + assertNull(result.getIntegerValue(SplitTaskExecutionInfo.NON_SENT_RECORDS)); + assertNull(result.getLongValue(SplitTaskExecutionInfo.NON_SENT_BYTES)); + assertNull(result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY)); + } + + @Test + public void successfulMultiBatchSubmissionLoopsUntilSmallBatch() throws RecorderException { + List fullBatch = createItems(BATCH_SIZE); + List partialBatch = createItems(3); + + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(fullBatch) + .thenReturn(fullBatch) + .thenReturn(partialBatch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + // Three pops: full, full, partial (terminates) + verify(mStorage, times(3)).pop(BATCH_SIZE); + verify(mSubmitter, times(3)).execute(any()); + verify(mStorage, times(3)).delete(any()); + verify(mStorage, never()).setActive(any()); + assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); + } + + @Test + public void emptyFirstPopSkipsSubmissionAndSucceeds() throws RecorderException { + when(mStorage.pop(BATCH_SIZE)).thenReturn(new ArrayList<>()); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + verify(mStorage, times(1)).pop(BATCH_SIZE); + verify(mSubmitter, never()).execute(any()); + verify(mStorage, never()).delete(any()); + verify(mStorage, never()).setActive(any()); + assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); + } + + // endregion + + // region Error handling + + @Test + public void retryableErrorCollectsFailuresAndContinuesLoop() throws RecorderException { + List batch = createItems(BATCH_SIZE); + List partialBatch = createItems(3); + + // First pop returns a full batch (fails), second also returns full (succeeds), + // third returns partial (terminates) + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(batch) + .thenReturn(partialBatch); + + // Throw only on the first call to execute; subsequent calls succeed + doThrow(new RecorderException("retryable error", 500, true)) + .doNothing() + .doNothing() + .when(mSubmitter).execute(any()); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + // Three pops total: two full batches + one partial + verify(mStorage, times(3)).pop(BATCH_SIZE); + // First batch failed → not deleted; second and partial → deleted twice + verify(mStorage, times(2)).delete(any()); + // Failing items (one batch worth) are re-queued + verify(mStorage, times(1)).setActive(any()); + + assertEquals(SplitTaskExecutionStatus.ERROR, result.getStatus()); + assertEquals(Integer.valueOf(BATCH_SIZE), result.getIntegerValue(SplitTaskExecutionInfo.NON_SENT_RECORDS)); + } + + @Test + public void retryableErrorPopulatesNonSentRecordsCount() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + assertEquals(Integer.valueOf(BATCH_SIZE), result.getIntegerValue(SplitTaskExecutionInfo.NON_SENT_RECORDS)); + assertEquals(SplitTaskExecutionStatus.ERROR, result.getStatus()); + } + + @Test + public void nonRetryableErrorStopsLoopImmediately() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(batch); // Would be popped if loop continued + doThrow(new RecorderException("non-retryable", 9009, false)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + // Only one pop — loop broke immediately on non-retryable error + verify(mStorage, times(1)).pop(BATCH_SIZE); + verify(mStorage, never()).delete(any()); + verify(mStorage, times(1)).setActive(any()); + + assertEquals(SplitTaskExecutionStatus.ERROR, result.getStatus()); + assertTrue(Boolean.TRUE.equals(result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY))); + } + + @Test + public void retryableErrorDoesNotSetDoNotRetry() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + assertNull(result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY)); + } + + // endregion + + // region setActive + + @Test + public void setActiveIsCalledWithFailedItemsOnError() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mStorage, times(1)).setActive(batch); + } + + @Test + public void setActiveIsNotCalledOnSuccess() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mStorage, never()).setActive(any()); + } + + @Test + public void chunkedSetActiveWhenFailingChunkSizeIsPositive() throws RecorderException { + int failingChunkSize = 3; + // Create items whose count is a multiple of failingChunkSize for predictable verification + List batch = createItems(9); // 9 items / chunkSize 3 = 3 setActive calls + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, failingChunkSize); + task.execute(); + + // 9 items chunked into 3 → 3 setActive calls + verify(mStorage, times(3)).setActive(any()); + } + + @Test + public void chunkedSetActiveHandlesNonEvenDivision() throws RecorderException { + int failingChunkSize = 3; + // 10 items / chunkSize 3 = 4 calls (chunks of 3, 3, 3, 1) + List batch = createItems(10); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, failingChunkSize); + task.execute(); + + verify(mStorage, times(4)).setActive(any()); + } + + @Test + public void noChunkingWhenFailingChunkSizeIsZero() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + // No chunking → exactly one setActive call with all failing items + verify(mStorage, times(1)).setActive(batch); + } + + // endregion + + // region Byte tracking via estimateItemSize + + @Test + public void byteTrackingViaEstimateItemSizeOverride() throws RecorderException { + long itemSizeBytes = 50L; + List batch = createItems(BATCH_SIZE); // 10 items * 50 bytes = 500 bytes + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTaskWithItemSize(BATCH_SIZE, TASK_TYPE, mTelemetry, 0, itemSizeBytes); + SplitTaskExecutionInfo result = task.execute(); + + long expectedBytes = BATCH_SIZE * itemSizeBytes; + assertEquals(Long.valueOf(expectedBytes), result.getLongValue(SplitTaskExecutionInfo.NON_SENT_BYTES)); + } + + @Test + public void byteTrackingDefaultsToZeroWhenNotOverridden() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + // Default estimateItemSize returns 0 → nonSentBytes = 0 + assertEquals(Long.valueOf(0L), result.getLongValue(SplitTaskExecutionInfo.NON_SENT_BYTES)); + } + + // endregion + + // region transformForSubmission + + @Test + public void transformForSubmissionHookIsApplied() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + // Build a task with a custom transform that wraps items in a new list + RecorderTask> task = new SimpleRecorderTask( + mStorage, mSubmitter, BATCH_SIZE, TASK_TYPE, mTelemetry, 0) { + @Override + protected List transformForSubmission(List items) { + List transformed = new ArrayList<>(); + for (String item : items) { + transformed.add(item.toUpperCase()); + } + return transformed; + } + }; + task.execute(); + + // The submitter should receive the transformed list (all uppercase) + List expectedTransformed = new ArrayList<>(); + for (String item : batch) { + expectedTransformed.add(item.toUpperCase()); + } + verify(mSubmitter, times(1)).execute(expectedTransformed); + } + + // endregion + + // region Null telemetry + + @Test + public void nullTelemetryDoesNotThrowNpeOnSuccess() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, null, 0); + // Should not throw + SplitTaskExecutionInfo result = task.execute(); + + assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); + } + + @Test + public void nullTelemetryDoesNotThrowNpeOnError() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, null, 0); + // Should not throw + SplitTaskExecutionInfo result = task.execute(); + + assertEquals(SplitTaskExecutionStatus.ERROR, result.getStatus()); + } + + // endregion + + // region Telemetry interactions + + @Test + public void telemetryRecordSuccessCalledOnSuccessfulSubmission() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, times(1)).recordSuccess(anyLong()); + } + + @Test + public void telemetryRecordSuccessCalledOncePerBatch() throws RecorderException { + List fullBatch = createItems(BATCH_SIZE); + List partialBatch = createItems(3); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(fullBatch) + .thenReturn(partialBatch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, times(2)).recordSuccess(anyLong()); + } + + @Test + public void telemetryRecordLatencyCalledOnSuccess() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, atLeastOnce()).recordLatency(anyLong()); + } + + @Test + public void telemetryRecordLatencyCalledOnError() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, atLeastOnce()).recordLatency(anyLong()); + } + + @Test + public void telemetryRecordErrorCalledWithHttpStatusOnError() throws RecorderException { + int httpStatus = 500; + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", httpStatus, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, times(1)).recordError(httpStatus); + } + + @Test + public void telemetryRecordSuccessNotCalledOnError() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, never()).recordSuccess(anyLong()); + } + + @Test + public void telemetryRecordErrorNotCalledOnSuccess() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + task.execute(); + + verify(mTelemetry, never()).recordError(any(Integer.class)); + } + + // endregion + + // region Task type + + @Test + public void taskTypeIsPreservedInSuccessResult() throws RecorderException { + List batch = createItems(3); + when(mStorage.pop(BATCH_SIZE)).thenReturn(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + assertEquals(TASK_TYPE, result.getTaskType()); + } + + @Test + public void taskTypeIsPreservedInErrorResult() throws RecorderException { + List batch = createItems(BATCH_SIZE); + when(mStorage.pop(BATCH_SIZE)) + .thenReturn(batch) + .thenReturn(new ArrayList<>()); + doThrow(new RecorderException("error", 500, true)).when(mSubmitter).execute(batch); + + RecorderTask> task = buildTask(BATCH_SIZE, TASK_TYPE, mTelemetry, 0); + SplitTaskExecutionInfo result = task.execute(); + + assertEquals(TASK_TYPE, result.getTaskType()); + } + + // endregion + + // region Helpers + + private List createItems(int count) { + List items = new ArrayList<>(); + for (int i = 0; i < count; i++) { + items.add("item_" + i); + } + return items; + } + + /** + * Builds a standard {@link RecorderTask} with no custom overrides. + * Uses the default {@link RecorderTask#transformForSubmission} (identity cast) and + * {@link RecorderTask#estimateItemSize} (returns 0). + */ + private RecorderTask> buildTask(int batchSize, + SplitTaskType taskType, + RecorderTelemetry telemetry, + int failingChunkSize) { + return new SimpleRecorderTask(mStorage, mSubmitter, batchSize, taskType, telemetry, failingChunkSize); + } + + /** + * Builds a {@link RecorderTask} with a custom fixed item size returned from + * {@link RecorderTask#estimateItemSize}, to exercise byte tracking. + */ + private RecorderTask> buildTaskWithItemSize(int batchSize, + SplitTaskType taskType, + RecorderTelemetry telemetry, + int failingChunkSize, + long itemSizeBytes) { + return new SimpleRecorderTask(mStorage, mSubmitter, batchSize, taskType, telemetry, failingChunkSize) { + @Override + protected long estimateItemSize(String item) { + return itemSizeBytes; + } + }; + } + + /** + * Minimal concrete subclass of {@link RecorderTask} for testing. + * T = String, R = List<String> (identity transform via default unchecked cast). + */ + private static class SimpleRecorderTask extends RecorderTask> { + + SimpleRecorderTask(@NonNull RecorderStorage storage, + @NonNull RecorderSubmitter> submitter, + int batchSize, + @NonNull SplitTaskType taskType, + RecorderTelemetry telemetry, + int failingChunkSize) { + super(storage, submitter, batchSize, taskType, telemetry, failingChunkSize); + } + } + + // endregion +} From 480341f0e84b3a55ffab768d32d34c514006d368 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 16 Mar 2026 16:23:15 -0300 Subject: [PATCH 11/15] Cleanupo --- api/.gitignore | 2 ++ backoff/.gitignore | 2 ++ build.gradle | 2 +- events-domain/.gitignore | 2 ++ events/.gitignore | 4 +++- fallback/.gitignore | 2 ++ http-api/.gitignore | 2 ++ http/.gitignore | 2 ++ logger/.gitignore | 2 ++ main/.gitignore | 2 ++ .../service/events/EventsRecorderTask.java | 5 +++++ .../impressions/ImpressionsRecorderTask.java | 5 +++++ submitter/.gitignore | 2 ++ .../android/client/submitter/RecorderTask.java | 16 ++-------------- .../client/submitter/RecorderTaskTest.java | 7 ++++++- tracker/.gitignore | 2 ++ 16 files changed, 42 insertions(+), 17 deletions(-) diff --git a/api/.gitignore b/api/.gitignore index 796b96d1c..e4dbec6f2 100644 --- a/api/.gitignore +++ b/api/.gitignore @@ -1 +1,3 @@ /build +.classpath +.settings diff --git a/backoff/.gitignore b/backoff/.gitignore index 3a11ced48..6009265cd 100644 --- a/backoff/.gitignore +++ b/backoff/.gitignore @@ -2,3 +2,5 @@ .gradle *.iml .DS_Store +.classpath +.settings diff --git a/build.gradle b/build.gradle index 1abeacb15..0f558a3fe 100644 --- a/build.gradle +++ b/build.gradle @@ -145,7 +145,7 @@ dependencies { return candidates.find { findProject(it) != null } } - ['main', 'logger', 'events', 'events-domain', 'api', 'http-api', 'http', 'fallback', 'backoff', 'tracker'].each { moduleName -> + ['main', 'logger', 'events', 'events-domain', 'api', 'http-api', 'http', 'fallback', 'backoff', 'tracker', 'submitter'].each { moduleName -> def resolvedPath = resolveProjectPath(moduleName) if (resolvedPath != null) { include project(resolvedPath) diff --git a/events-domain/.gitignore b/events-domain/.gitignore index 796b96d1c..e4dbec6f2 100644 --- a/events-domain/.gitignore +++ b/events-domain/.gitignore @@ -1 +1,3 @@ /build +.classpath +.settings diff --git a/events/.gitignore b/events/.gitignore index 42afabfd2..0b60b6351 100644 --- a/events/.gitignore +++ b/events/.gitignore @@ -1 +1,3 @@ -/build \ No newline at end of file +/build +.classpath +.settings \ No newline at end of file diff --git a/fallback/.gitignore b/fallback/.gitignore index 3a11ced48..6009265cd 100644 --- a/fallback/.gitignore +++ b/fallback/.gitignore @@ -2,3 +2,5 @@ .gradle *.iml .DS_Store +.classpath +.settings diff --git a/http-api/.gitignore b/http-api/.gitignore index 796b96d1c..e4dbec6f2 100644 --- a/http-api/.gitignore +++ b/http-api/.gitignore @@ -1 +1,3 @@ /build +.classpath +.settings diff --git a/http/.gitignore b/http/.gitignore index 796b96d1c..e4dbec6f2 100644 --- a/http/.gitignore +++ b/http/.gitignore @@ -1 +1,3 @@ /build +.classpath +.settings diff --git a/logger/.gitignore b/logger/.gitignore index 3a11ced48..6009265cd 100644 --- a/logger/.gitignore +++ b/logger/.gitignore @@ -2,3 +2,5 @@ .gradle *.iml .DS_Store +.classpath +.settings diff --git a/main/.gitignore b/main/.gitignore index 3a11ced48..6009265cd 100644 --- a/main/.gitignore +++ b/main/.gitignore @@ -2,3 +2,5 @@ .gradle *.iml .DS_Store +.classpath +.settings diff --git a/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java b/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java index c90857deb..fa51ec1e0 100644 --- a/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java +++ b/main/src/main/java/io/split/android/client/service/events/EventsRecorderTask.java @@ -30,6 +30,11 @@ public EventsRecorderTask(@NonNull HttpRecorder> httpRecorder, FAILING_CHUNK_SIZE); } + @Override + protected List transformForSubmission(List items) { + return items; + } + @Override protected long estimateItemSize(Event item) { return item.getSizeInBytes(); diff --git a/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java b/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java index c9e7aba55..57f737b93 100644 --- a/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java +++ b/main/src/main/java/io/split/android/client/service/impressions/ImpressionsRecorderTask.java @@ -31,6 +31,11 @@ public ImpressionsRecorderTask(@NonNull HttpRecorder> httpRe this.mEstimatedSizeInBytes = config.getEstimatedSizeInBytes(); } + @Override + protected List transformForSubmission(List items) { + return items; + } + @Override protected long estimateItemSize(KeyImpression item) { return mEstimatedSizeInBytes; diff --git a/submitter/.gitignore b/submitter/.gitignore index 796b96d1c..e4dbec6f2 100644 --- a/submitter/.gitignore +++ b/submitter/.gitignore @@ -1 +1,3 @@ /build +.classpath +.settings diff --git a/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java b/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java index 435cc3299..5a822812e 100644 --- a/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java +++ b/submitter/src/main/java/io/split/android/client/submitter/RecorderTask.java @@ -122,21 +122,9 @@ public final SplitTaskExecutionInfo execute() { } /** - * Transform storage items into the submission payload. - *

- * Default implementation casts the list directly to R — works when T and R are the same - * type (e.g., {@code List} to {@code List}). - * Override this for tasks that need data transformation before submission - * (e.g., UniqueKeys deduplication, ImpressionsCount wrapping). - *

- * WARNING: Subclasses where T and R are different types MUST override this - * method. The default unchecked cast will throw a {@link ClassCastException} at runtime if T - * and R differ. Failing to override in that case is a programming error. + * Transform storage items into the submission payload before submitting. */ - @SuppressWarnings("unchecked") - protected R transformForSubmission(List items) { - return (R) items; - } + protected abstract R transformForSubmission(List items); /** * Estimate the byte size of one storage item for tracking non-sent bytes. diff --git a/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java b/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java index 5314fe5e3..1e0994e6e 100644 --- a/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java +++ b/submitter/src/test/java/io/split/android/client/submitter/RecorderTaskTest.java @@ -523,7 +523,7 @@ protected long estimateItemSize(String item) { /** * Minimal concrete subclass of {@link RecorderTask} for testing. - * T = String, R = List<String> (identity transform via default unchecked cast). + * T = String, R = List<String> (identity transform — same type). */ private static class SimpleRecorderTask extends RecorderTask> { @@ -535,6 +535,11 @@ private static class SimpleRecorderTask extends RecorderTask transformForSubmission(List items) { + return items; + } } // endregion diff --git a/tracker/.gitignore b/tracker/.gitignore index 796b96d1c..e4dbec6f2 100644 --- a/tracker/.gitignore +++ b/tracker/.gitignore @@ -1 +1,3 @@ /build +.classpath +.settings From 978ac03872fdbfd759cd650b73b0fce3e46a0c84 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 16 Mar 2026 16:42:44 -0300 Subject: [PATCH 12/15] Parallel UT runs --- gradle/common-android-library.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gradle/common-android-library.gradle b/gradle/common-android-library.gradle index 28bfe3eee..adf1d647d 100644 --- a/gradle/common-android-library.gradle +++ b/gradle/common-android-library.gradle @@ -14,6 +14,10 @@ tasks.withType(JavaCompile).configureEach { options.compilerArgs.add('-parameters') } +tasks.withType(Test).configureEach { + maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1 +} + def kotlinCompileClass = null try { kotlinCompileClass = Class.forName('org.jetbrains.kotlin.gradle.tasks.KotlinCompile') From 96bad9266fbfe057d9e8cb83112890f6747da4f8 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 16:25:23 -0300 Subject: [PATCH 13/15] Fix README --- submitter/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/submitter/README.md b/submitter/README.md index dcff89273..6e17599b8 100644 --- a/submitter/README.md +++ b/submitter/README.md @@ -1,10 +1,10 @@ # submitter -Generic batch recorder task abstraction for the Split Android SDK. +Generic batch recorder task abstraction. ## Purpose -Encapsulates the logic for submitting batched data (such as impressions and events) to the Split platform. It provides a reusable abstraction for recorder tasks, decoupled from the SDK's internal storage and networking layers — dependencies are injected via callbacks. +Encapsulates the logic for submitting batched data (such as impressions and events) to the backend. It provides a reusable abstraction for recorder tasks, decoupled from the SDK's internal storage and networking layers. Dependencies are injected via callbacks. ## Design notes From 867cae1232778909d354f98f0fc595b4290437ef Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 18:45:39 -0300 Subject: [PATCH 14/15] Update README --- submitter/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/submitter/README.md b/submitter/README.md index 6e17599b8..5735f8783 100644 --- a/submitter/README.md +++ b/submitter/README.md @@ -8,6 +8,5 @@ Encapsulates the logic for submitting batched data (such as impressions and even ## Design notes -- Depends on `events-domain` for shared domain types. +- For now depends on `events-domain` for the executor types. - Depends on `logger` for logging. -- No dependency on `main/` internals or networking implementation details. From 28881015be710a9b8cd86afb6287334086c0315c Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 17 Mar 2026 19:21:50 -0300 Subject: [PATCH 15/15] Add missing modules to coverage --- sonar-project.properties | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/sonar-project.properties b/sonar-project.properties index 85a95779d..185535a4c 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -3,8 +3,8 @@ sonar.projectKey=splitio_android-client sonar.projectName=android-client # Path to source directories (multi-module) -# Root project contains modules: api, events-domain, main, events, logger, http-api, http -sonar.sources=api/src/main/java,events-domain/src/main/java,main/src/main/java,events/src/main/java,logger/src/main/java,http-api/src/main/java,http/src/main/java +# Root project contains modules: api, events-domain, main, events, logger, http-api, http, fallback, backoff, tracker, submitter +sonar.sources=api/src/main/java,events-domain/src/main/java,main/src/main/java,events/src/main/java,logger/src/main/java,http-api/src/main/java,http/src/main/java,fallback/src/main/java,backoff/src/main/java,tracker/src/main/java,submitter/src/main/java # Path to compiled classes (multi-module) # Include binary paths for all modules: api, events-domain, main, events, logger, http-api, http @@ -15,7 +15,11 @@ sonar.java.binaries=\ events/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ logger/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ http-api/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ - http/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes + http/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ + fallback/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ + backoff/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ + tracker/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes,\ + submitter/build/intermediates/javac/debug/compileDebugJavaWithJavac/classes # Path to dependency/libraries jars (multi-module) sonar.java.libraries=\ @@ -46,11 +50,27 @@ sonar.java.libraries=\ http/build/intermediates/compile_library_classes_jar/debug/bundleLibCompileToJarDebug/classes.jar,\ http/build/intermediates/runtime_library_classes_jar/debug/bundleLibRuntimeToJarDebug/classes.jar,\ http/build/intermediates/compile_r_class_jar/debug/generateDebugRFile/R.jar,\ - http/build/intermediates/compile_and_runtime_r_class_jar/debugUnitTest/generateDebugUnitTestStubRFile/R.jar + http/build/intermediates/compile_and_runtime_r_class_jar/debugUnitTest/generateDebugUnitTestStubRFile/R.jar,\ + fallback/build/intermediates/compile_library_classes_jar/debug/bundleLibCompileToJarDebug/classes.jar,\ + fallback/build/intermediates/runtime_library_classes_jar/debug/bundleLibRuntimeToJarDebug/classes.jar,\ + fallback/build/intermediates/compile_r_class_jar/debug/generateDebugRFile/R.jar,\ + fallback/build/intermediates/compile_and_runtime_r_class_jar/debugUnitTest/generateDebugUnitTestStubRFile/R.jar,\ + backoff/build/intermediates/compile_library_classes_jar/debug/bundleLibCompileToJarDebug/classes.jar,\ + backoff/build/intermediates/runtime_library_classes_jar/debug/bundleLibRuntimeToJarDebug/classes.jar,\ + backoff/build/intermediates/compile_r_class_jar/debug/generateDebugRFile/R.jar,\ + backoff/build/intermediates/compile_and_runtime_r_class_jar/debugUnitTest/generateDebugUnitTestStubRFile/R.jar,\ + tracker/build/intermediates/compile_library_classes_jar/debug/bundleLibCompileToJarDebug/classes.jar,\ + tracker/build/intermediates/runtime_library_classes_jar/debug/bundleLibRuntimeToJarDebug/classes.jar,\ + tracker/build/intermediates/compile_r_class_jar/debug/generateDebugRFile/R.jar,\ + tracker/build/intermediates/compile_and_runtime_r_class_jar/debugUnitTest/generateDebugUnitTestStubRFile/R.jar,\ + submitter/build/intermediates/compile_library_classes_jar/debug/bundleLibCompileToJarDebug/classes.jar,\ + submitter/build/intermediates/runtime_library_classes_jar/debug/bundleLibRuntimeToJarDebug/classes.jar,\ + submitter/build/intermediates/compile_r_class_jar/debug/generateDebugRFile/R.jar,\ + submitter/build/intermediates/compile_and_runtime_r_class_jar/debugUnitTest/generateDebugUnitTestStubRFile/R.jar # Path to test directories (multi-module) # Only include test source folders that are guaranteed to exist in all environments -sonar.tests=api/src/test/java,events-domain/src/test/java,main/src/test/java,main/src/androidTest/java,main/src/sharedTest/java,events/src/test/java,logger/src/test/java,http-api/src/test/java,http/src/test/java +sonar.tests=api/src/test/java,events-domain/src/test/java,main/src/test/java,main/src/androidTest/java,main/src/sharedTest/java,events/src/test/java,logger/src/test/java,http-api/src/test/java,http/src/test/java,fallback/src/test/java,backoff/src/test/java,tracker/src/test/java,submitter/src/test/java # Encoding of the source code sonar.sourceEncoding=UTF-8