diff --git a/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java b/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java index d17019865..36d86ff7b 100644 --- a/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java +++ b/src/main/java/io/split/android/client/service/impressions/strategy/DebugTracker.java @@ -4,8 +4,13 @@ import androidx.annotation.NonNull; +import java.util.concurrent.atomic.AtomicBoolean; + import io.split.android.client.dtos.KeyImpression; import io.split.android.client.service.ServiceConstants; +import io.split.android.client.service.executor.SplitTaskExecutionInfo; +import io.split.android.client.service.executor.SplitTaskExecutionListener; +import io.split.android.client.service.executor.SplitTaskExecutionStatus; 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; @@ -19,7 +24,21 @@ class DebugTracker implements PeriodicTracker { private final ImpressionsTaskFactory mImpressionsTaskFactory; private final RetryBackoffCounterTimer mRetryTimer; private final int mImpressionsRefreshRate; + private final AtomicBoolean mIsSynchronizing = new AtomicBoolean(true); private final ImpressionsObserver mImpressionsObserver; + /** @noinspection FieldCanBeLocal*/ + private final SplitTaskExecutionListener mTaskExecutionListener = new SplitTaskExecutionListener() { + @Override + public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { + // this listener intercepts impressions recording task + if (taskInfo.getStatus() == SplitTaskExecutionStatus.ERROR) { + if (Boolean.TRUE.equals(taskInfo.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY))) { + mIsSynchronizing.compareAndSet(true, false); + stopPeriodicRecording(); + } + } + } + }; private String mImpressionsRecorderTaskId; DebugTracker(@NonNull ImpressionsObserver impressionsObserver, @@ -30,6 +49,7 @@ class DebugTracker implements PeriodicTracker { int impressionsRefreshRate) { mImpressionsObserver = checkNotNull(impressionsObserver); mImpressionsSyncHelper = checkNotNull(impressionsSyncHelper); + mImpressionsSyncHelper.addListener(mTaskExecutionListener); mTaskExecutor = checkNotNull(taskExecutor); mImpressionsTaskFactory = checkNotNull(taskFactory); mRetryTimer = retryTimer; @@ -50,7 +70,9 @@ private void flushImpressions() { @Override public void startPeriodicRecording() { - scheduleImpressionsRecorderTask(); + if (mIsSynchronizing.get()) { + scheduleImpressionsRecorderTask(); + } } private void scheduleImpressionsRecorderTask() { diff --git a/src/main/java/io/split/android/client/service/impressions/strategy/NoneTracker.java b/src/main/java/io/split/android/client/service/impressions/strategy/NoneTracker.java index 566272c02..de9b5e06c 100644 --- a/src/main/java/io/split/android/client/service/impressions/strategy/NoneTracker.java +++ b/src/main/java/io/split/android/client/service/impressions/strategy/NoneTracker.java @@ -7,13 +7,10 @@ import java.util.concurrent.atomic.AtomicBoolean; 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.SplitTaskExecutionListener; -import io.split.android.client.service.executor.SplitTaskExecutionStatus; import io.split.android.client.service.executor.SplitTaskExecutor; import io.split.android.client.service.executor.SplitTaskSerialWrapper; -import io.split.android.client.service.executor.SplitTaskType; import io.split.android.client.service.impressions.ImpressionsCounter; import io.split.android.client.service.impressions.ImpressionsTaskFactory; import io.split.android.client.service.impressions.unique.UniqueKeysTracker; diff --git a/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java b/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java index 0ba6f4c25..5f441dfaf 100644 --- a/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java +++ b/src/main/java/io/split/android/client/service/impressions/strategy/OptimizedTracker.java @@ -8,6 +8,9 @@ import io.split.android.client.dtos.KeyImpression; import io.split.android.client.service.ServiceConstants; +import io.split.android.client.service.executor.SplitTaskExecutionInfo; +import io.split.android.client.service.executor.SplitTaskExecutionListener; +import io.split.android.client.service.executor.SplitTaskExecutionStatus; 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; @@ -25,6 +28,22 @@ class OptimizedTracker implements PeriodicTracker { private final int mImpressionsRefreshRate; private String mImpressionsRecorderTaskId; private final AtomicBoolean mTrackingIsEnabled; + private final AtomicBoolean mIsSynchronizing = new AtomicBoolean(true); + /** + * @noinspection FieldCanBeLocal + */ + private final SplitTaskExecutionListener mTaskExecutionListener = new SplitTaskExecutionListener() { + @Override + public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { + // this listener intercepts impressions recording task + if (taskInfo.getStatus() == SplitTaskExecutionStatus.ERROR) { + if (Boolean.TRUE.equals(taskInfo.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY))) { + mIsSynchronizing.compareAndSet(true, false); + stopPeriodicRecording(); + } + } + } + }; OptimizedTracker(@NonNull ImpressionsObserver impressionsObserver, @NonNull RecorderSyncHelper impressionsSyncHelper, @@ -36,6 +55,7 @@ class OptimizedTracker implements PeriodicTracker { boolean isTrackingEnabled) { mImpressionsObserver = checkNotNull(impressionsObserver); mImpressionsSyncHelper = checkNotNull(impressionsSyncHelper); + mImpressionsSyncHelper.addListener(mTaskExecutionListener); mTaskExecutor = checkNotNull(taskExecutor); mImpressionsTaskFactory = checkNotNull(taskFactory); @@ -51,7 +71,9 @@ public void flush() { @Override public void startPeriodicRecording() { - scheduleImpressionsRecorderTask(); + if (mIsSynchronizing.get()) { + scheduleImpressionsRecorderTask(); + } } @Override diff --git a/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt b/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt index 5e395ba9f..25c9919d9 100644 --- a/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt +++ b/src/test/java/io/split/android/client/service/impressions/strategy/DebugStrategyTest.kt @@ -1,7 +1,6 @@ package io.split.android.client.service.impressions.strategy import io.split.android.client.dtos.KeyImpression -import io.split.android.client.impressions.Impression import io.split.android.client.service.executor.SplitTaskExecutionInfo import io.split.android.client.service.executor.SplitTaskExecutionListener import io.split.android.client.service.executor.SplitTaskExecutor @@ -17,7 +16,14 @@ import org.junit.Test import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any import org.mockito.Mock -import org.mockito.Mockito.* +import org.mockito.Mockito.argThat +import org.mockito.Mockito.eq +import org.mockito.Mockito.mock +import org.mockito.Mockito.never +import org.mockito.Mockito.spy +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` import org.mockito.MockitoAnnotations import org.mockito.invocation.InvocationOnMock @@ -101,47 +107,6 @@ class DebugStrategyTest { spy(impression).withPreviousTime(20421) } -// @Test -// fun `call stop periodic tracking when sync listener returns do not retry`() { -// val listenerCaptor = ArgumentCaptor.forClass(SplitTaskExecutionListener::class.java) -// -// `when`(impressionsSyncHelper.addListener(listenerCaptor.capture())).thenAnswer { it } -// `when`(impressionsSyncHelper.taskExecuted(argThat { -// it.taskType == SplitTaskType.IMPRESSIONS_RECORDER -// })).thenAnswer { -// listenerCaptor.value.taskExecuted( -// SplitTaskExecutionInfo.error( -// SplitTaskType.IMPRESSIONS_RECORDER, -// mapOf(SplitTaskExecutionInfo.DO_NOT_RETRY to true) -// ) -// ) -// it -// } -// -// strategy = DebugStrategy( -// impressionsObserver, -// impressionsSyncHelper, -// taskExecutor, -// taskFactory, -// telemetryRuntimeProducer, -// ) -// -// strategy.startPeriodicRecording() -// // simulate sync helper trigger -// impressionsSyncHelper.taskExecuted( -// SplitTaskExecutionInfo.error( -// SplitTaskType.IMPRESSIONS_RECORDER, -// mapOf(SplitTaskExecutionInfo.DO_NOT_RETRY to true) -// ) -// ) -// -// // start periodic recording again to verify it is not working anymore -// strategy.startPeriodicRecording() -// -// verify(tracker, times(1)).startPeriodicRecording() -// verify(tracker).stopPeriodicRecording() -// } - @Test fun `do not submit recording task when push fails with do not retry`() { val listenerCaptor = ArgumentCaptor.forClass(SplitTaskExecutionListener::class.java) diff --git a/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt b/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt index 426a670eb..6ddca42f7 100644 --- a/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt +++ b/src/test/java/io/split/android/client/service/impressions/strategy/DebugTrackerTest.kt @@ -1,7 +1,10 @@ package io.split.android.client.service.impressions.strategy import io.split.android.client.dtos.KeyImpression +import io.split.android.client.service.executor.SplitTaskExecutionInfo +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.service.impressions.ImpressionsRecorderTask import io.split.android.client.service.impressions.ImpressionsTaskFactory import io.split.android.client.service.impressions.observer.ImpressionsObserver @@ -9,6 +12,7 @@ import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTi import io.split.android.client.service.synchronizer.RecorderSyncHelper import org.junit.Before import org.junit.Test +import org.mockito.ArgumentCaptor import org.mockito.Mock import org.mockito.Mockito.* import org.mockito.MockitoAnnotations @@ -100,4 +104,50 @@ class DebugTrackerTest { verify(impressionsObserver).persist() } + + @Test + fun `call stop periodic tracking when sync listener returns do not retry`() { + val listenerCaptor = ArgumentCaptor.forClass(SplitTaskExecutionListener::class.java) + + val impressionsRecorderTask = mock(ImpressionsRecorderTask::class.java) + `when`(taskFactory.createImpressionsRecorderTask()).thenReturn(impressionsRecorderTask) + `when`(taskExecutor.schedule(eq(impressionsRecorderTask), eq(0L), eq(30L), any())).thenReturn("250") + `when`(syncHelper.addListener(listenerCaptor.capture())).thenAnswer { it } + `when`(syncHelper.taskExecuted(argThat { + it.taskType == SplitTaskType.IMPRESSIONS_RECORDER + })).thenAnswer { + listenerCaptor.value.taskExecuted( + SplitTaskExecutionInfo.error( + SplitTaskType.IMPRESSIONS_RECORDER, + mapOf(SplitTaskExecutionInfo.DO_NOT_RETRY to true) + ) + ) + it + } + + tracker = DebugTracker( + impressionsObserver, + syncHelper, + taskExecutor, + taskFactory, + retryBackoffCounterTimer, + 30 + ) + + tracker.startPeriodicRecording() + val spy = spy(tracker) + // simulate sync helper trigger + syncHelper.taskExecuted( + SplitTaskExecutionInfo.error( + SplitTaskType.IMPRESSIONS_RECORDER, + mapOf(SplitTaskExecutionInfo.DO_NOT_RETRY to true) + ) + ) + + // start periodic recording again to verify it is not working anymore + spy.startPeriodicRecording() + + verify(taskExecutor).stopTask("250") + verify(impressionsObserver).persist() + } } diff --git a/src/test/java/io/split/android/client/service/impressions/strategy/NoneStrategyTest.kt b/src/test/java/io/split/android/client/service/impressions/strategy/NoneStrategyTest.kt index 96c9fd5fa..daaeb89aa 100644 --- a/src/test/java/io/split/android/client/service/impressions/strategy/NoneStrategyTest.kt +++ b/src/test/java/io/split/android/client/service/impressions/strategy/NoneStrategyTest.kt @@ -7,7 +7,6 @@ import io.split.android.client.service.impressions.ImpressionsCounter import io.split.android.client.service.impressions.ImpressionsTaskFactory import io.split.android.client.service.impressions.unique.SaveUniqueImpressionsTask import io.split.android.client.service.impressions.unique.UniqueKeysTracker -import io.split.android.client.service.sseclient.sseclient.RetryBackoffCounterTimer import org.junit.Before import org.junit.Test import org.mockito.ArgumentMatchers.any @@ -31,15 +30,6 @@ class NoneStrategyTest { @Mock private lateinit var uniqueKeysTracker: UniqueKeysTracker - @Mock - private lateinit var countRetryTimer: RetryBackoffCounterTimer - - @Mock - private lateinit var uniqueKeysRetryTimer: RetryBackoffCounterTimer - - @Mock - private lateinit var tracker: PeriodicTracker - private lateinit var strategy: NoneStrategy @Before diff --git a/src/test/java/io/split/android/client/service/impressions/strategy/NoneTrackerTest.kt b/src/test/java/io/split/android/client/service/impressions/strategy/NoneTrackerTest.kt index 00028caa0..8d1ebe1f9 100644 --- a/src/test/java/io/split/android/client/service/impressions/strategy/NoneTrackerTest.kt +++ b/src/test/java/io/split/android/client/service/impressions/strategy/NoneTrackerTest.kt @@ -186,7 +186,6 @@ class NoneTrackerTest { verify(taskExecutor, never()).submit(eq(countTask), any()) } - @Test fun `stop periodic recording does not save unique keys when tracking is disabled`() { val countTask = mock(SaveUniqueImpressionsTask::class.java) diff --git a/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt b/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt index 3d0472667..7e05fae92 100644 --- a/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt +++ b/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedStrategyTest.kt @@ -150,49 +150,6 @@ class OptimizedStrategyTest { ) } -// @Test -// fun `call stop periodic tracking when sync listener returns do not retry`() { -// val listenerCaptor = ArgumentCaptor.forClass(SplitTaskExecutionListener::class.java) -// -// `when`(impressionsSyncHelper.addListener(listenerCaptor.capture())).thenAnswer { it } -// `when`(impressionsSyncHelper.taskExecuted(argThat { -// it.taskType == SplitTaskType.IMPRESSIONS_RECORDER -// })).thenAnswer { -// listenerCaptor.value.taskExecuted( -// SplitTaskExecutionInfo.error( -// SplitTaskType.IMPRESSIONS_RECORDER, -// mapOf(DO_NOT_RETRY to true) -// ) -// ) -// it -// } -// -// strategy = OptimizedStrategy( -// impressionsObserver, -// impressionsCounter, -// impressionsSyncHelper, -// taskExecutor, -// taskFactory, -// telemetryRuntimeProducer, -// dedupeTimeInterval -// ) -// -// strategy.startPeriodicRecording() -// // simulate sync helper trigger -// impressionsSyncHelper.taskExecuted( -// SplitTaskExecutionInfo.error( -// SplitTaskType.IMPRESSIONS_RECORDER, -// mapOf(DO_NOT_RETRY to true) -// ) -// ) -// -// // start periodic recording again to verify it is not working anymore -// strategy.startPeriodicRecording() -// -// verify(tracker, times(1)).startPeriodicRecording() -// verify(tracker).stopPeriodicRecording() -// } - @Test fun `do not submit recording task when push fails with do not retry`() { val listenerCaptor = ArgumentCaptor.forClass(SplitTaskExecutionListener::class.java) diff --git a/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt b/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt index 4e543f8aa..71915b9ca 100644 --- a/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt +++ b/src/test/java/io/split/android/client/service/impressions/strategy/OptimizedTrackerTest.kt @@ -1,15 +1,19 @@ package io.split.android.client.service.impressions.strategy import io.split.android.client.dtos.KeyImpression +import io.split.android.client.service.executor.SplitTaskExecutionInfo +import io.split.android.client.service.executor.SplitTaskExecutionInfo.DO_NOT_RETRY import io.split.android.client.service.executor.SplitTaskExecutionListener import io.split.android.client.service.executor.SplitTaskExecutor import io.split.android.client.service.executor.SplitTaskSerialWrapper +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 org.junit.Before import org.junit.Test +import org.mockito.ArgumentCaptor import org.mockito.Mock import org.mockito.Mockito.* import org.mockito.MockitoAnnotations @@ -47,6 +51,16 @@ class OptimizedTrackerTest { ) } + @Test + fun `start periodic recording schedules impression recorder task`() { + val task = mock(ImpressionsRecorderTask::class.java) + `when`(taskFactory.createImpressionsRecorderTask()).thenReturn(task) + + tracker.startPeriodicRecording() + + verify(taskExecutor).schedule(task, 0L, 30L, syncHelper) + } + @Test fun `flush flushes impressions`() { val task = mock(ImpressionsRecorderTask::class.java) @@ -83,27 +97,56 @@ class OptimizedTrackerTest { } @Test - fun `stop periodic recording does not save impression count when tracking is disabled`() { - val countTask = mock(SaveImpressionsCountTask::class.java) - `when`(taskFactory.createSaveImpressionsCountTask(any())).thenReturn(countTask) - `when`( - taskExecutor.schedule( - any(SaveImpressionsCountTask::class.java), - eq(0L), - eq(40L), - eq(null) - ) - ).thenReturn("id_1") - tracker.enableTracking(false) + fun `stopPeriodicRecording calls persist on ImpressionsObserver`() { tracker.stopPeriodicRecording() - verify(taskExecutor, never()).submit(eq(countTask), any()) + verify(impressionsObserver).persist() } @Test - fun `stopPeriodicRecording calls persist on ImpressionsObserver`() { - tracker.stopPeriodicRecording() + fun `call stop periodic tracking when sync listener returns do not retry`() { + val listenerCaptor = ArgumentCaptor.forClass(SplitTaskExecutionListener::class.java) + + val impressionsRecorderTask = mock(ImpressionsRecorderTask::class.java) + `when`(taskFactory.createImpressionsRecorderTask()).thenReturn(impressionsRecorderTask) + `when`(taskExecutor.schedule(eq(impressionsRecorderTask), eq(0L), eq(30L), any())).thenReturn("250") + `when`(syncHelper.addListener(listenerCaptor.capture())).thenAnswer { it } + `when`(syncHelper.taskExecuted(argThat { + it.taskType == SplitTaskType.IMPRESSIONS_RECORDER + })).thenAnswer { + listenerCaptor.value.taskExecuted( + SplitTaskExecutionInfo.error( + SplitTaskType.IMPRESSIONS_RECORDER, + mapOf(DO_NOT_RETRY to true) + ) + ) + it + } + tracker = OptimizedTracker( + impressionsObserver, + syncHelper, + taskExecutor, + taskFactory, + impressionTimer, + 30, + true + ) + + val spy = spy(tracker) + tracker.startPeriodicRecording() + // simulate sync helper trigger + syncHelper.taskExecuted( + SplitTaskExecutionInfo.error( + SplitTaskType.IMPRESSIONS_RECORDER, + mapOf(DO_NOT_RETRY to true) + ) + ) + + // start periodic recording again to verify it is not working anymore + spy.startPeriodicRecording() + + verify(taskExecutor).stopTask("250") verify(impressionsObserver).persist() } }