From 1033bbc151187629a4b61454e0a2370747366b5d Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Mon, 21 Sep 2026 09:42:09 +0100 Subject: [PATCH 1/7] [SDK-707] Replace request AsyncTask with SDK executor --- CHANGELOG.md | 2 +- .../com/iterable/iterableapi/IterableApi.java | 6 + .../iterableapi/IterableApiClient.java | 22 +- .../iterableapi/IterableExecutors.java | 49 +++-- .../IterableRequestDispatcher.java | 117 ++++++++++ .../iterableapi/IterableRequestTask.java | 133 +++++++----- .../iterableapi/OfflineRequestProcessor.java | 50 ++++- .../iterableapi/OnlineRequestProcessor.java | 27 ++- .../com/iterable/iterableapi/BaseTest.java | 8 +- ...mbeddedSessionManagerThreadSafetyTest.java | 2 +- .../iterableapi/IterableApiAuthJWTTests.java | 2 +- .../IterableApiAuthSecurityTests.java | 5 +- .../iterableapi/IterableApiAuthTests.java | 2 +- .../IterableApiCriteriaFetchTests.java | 2 +- .../IterableApiCustomEventTests.java | 2 +- .../IterableApiIntegrationTest.java | 2 +- .../IterableApiMergeUserEmailTests.java | 2 +- .../IterableApiRequestExecutionTest.kt | 200 ++++++++++++++++++ .../iterableapi/IterableApiRequestTest.java | 128 +++++------ .../iterable/iterableapi/IterableApiTest.java | 2 +- .../IterableAsyncInitializationTest.java | 9 +- .../IterableEmbeddedManagerTest.java | 4 +- .../IterableInAppDialogNotificationTest.java | 4 +- .../IterableInAppManagerSyncTest.java | 2 +- .../iterableapi/IterableInAppManagerTest.java | 14 +- .../iterableapi/IterableInboxTest.java | 4 +- .../IterablePushActionReceiverTest.java | 2 +- .../IterableRequestDispatcherTest.kt | 82 +++++++ .../iterableapi/IterableRequestTaskTest.kt | 167 +++++++++++++++ .../iterableapi/IterableTaskRunnerTest.java | 2 +- .../iterableapi/IterableTestUtils.java | 37 +++- .../OfflineRequestProcessorTest.java | 74 ------- .../OfflineRequestProcessorTest.kt | 132 ++++++++++++ .../iterableapi/TaskSchedulerTest.java | 55 ----- .../iterable/iterableapi/TaskSchedulerTest.kt | 131 ++++++++++++ 35 files changed, 1174 insertions(+), 308 deletions(-) create mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestExecutionTest.kt create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt delete mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 22dc10702..418d2f615 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Fixed -- Push token registration and disable operations, including their API requests and retries, now run on a dedicated SDK-owned serial executor, while Iterable deep-link redirects use a separate SDK-owned serial executor. This removes their dependency on Android's process-wide `AsyncTask` queue, preserves ordering within each operation type, prevents slow redirects from delaying push work, and keeps client callbacks and attribution updates on the main thread. +- Push token registration, disable operations, Iterable deep-link redirects, and API requests now use dedicated SDK-owned executors instead of Android's process-wide `AsyncTask` queues. Push, deep-link, and offline operations preserve ordering in isolated serial lanes, while ordinary online API requests retain concurrent execution. Slow work in one lane no longer delays unrelated SDK operations, and client callbacks and attribution updates continue on the main thread. ## [3.11.0] ### Added diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index a89a360cc..049d8593a 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -992,6 +992,12 @@ static void initializeForPush(@Nullable Context context) { this.embeddedManager = embeddedManager; this.pushRegistration = Objects.requireNonNull(pushRegistration); } + void setRequestDispatcher(IterableRequestDispatcher requestDispatcher) { + apiClient = new IterableApiClient( + new IterableApiAuthProvider(), + IterableRequestDispatchers.same(Objects.requireNonNull(requestDispatcher)) + ); + } //endregion diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java index fb331b12c..3fdeed5af 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java @@ -21,7 +21,8 @@ class IterableApiClient { private static final String TAG = "IterableApiClient"; private final @NonNull AuthProvider authProvider; - private final IterablePushRegistrationRequestProcessor pushRegistrationRequestProcessor; + private final @NonNull IterableRequestDispatchers requestDispatchers; + private final OnlineRequestProcessor pushRegistrationRequestProcessor; // A newer push action invalidates retries from earlier registration or disable requests. private final AtomicLong pushRegistrationRequestGeneration = new AtomicLong(); private RequestProcessor requestProcessor; @@ -45,14 +46,22 @@ interface AuthProvider { } IterableApiClient(@NonNull AuthProvider authProvider) { + this(authProvider, IterableRequestDispatchers.sdk()); + } + + IterableApiClient( + @NonNull AuthProvider authProvider, + @NonNull IterableRequestDispatchers requestDispatchers + ) { this.authProvider = authProvider; + this.requestDispatchers = requestDispatchers; pushRegistrationRequestProcessor = - new IterablePushRegistrationRequestProcessor(); + new OnlineRequestProcessor(requestDispatchers.push()); } private RequestProcessor getRequestProcessor() { if (requestProcessor == null) { - requestProcessor = new OnlineRequestProcessor(); + requestProcessor = new OnlineRequestProcessor(requestDispatchers.online()); } return requestProcessor; } @@ -70,8 +79,11 @@ void setOfflineProcessingEnabled(boolean offlineMode) { } this.requestProcessor = offlineMode - ? new OfflineRequestProcessor(authProvider.getContext()) - : new OnlineRequestProcessor(); + ? new OfflineRequestProcessor( + authProvider.getContext(), + requestDispatchers.offline() + ) + : new OnlineRequestProcessor(requestDispatchers.online()); } void getRemoteConfiguration(IterableHelper.IterableActionHandler actionHandler) { diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java index 632ed7591..d3f412402 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java @@ -5,21 +5,25 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; final class IterableExecutors { - private static final Executor PUSH_EXECUTOR = Executors.newSingleThreadExecutor(runnable -> { - Thread thread = new Thread(runnable, "IterablePushExecutor"); - thread.setDaemon(true); - thread.setPriority(Thread.NORM_PRIORITY); - return thread; - }); + private static final int REQUEST_THREAD_COUNT = + Math.max(2, Math.min(Runtime.getRuntime().availableProcessors(), 4)); + private static final AtomicInteger REQUEST_THREAD_ID = new AtomicInteger(); + private static final Executor PUSH_EXECUTOR = + newSingleThreadExecutor("IterablePushExecutor"); private static final Executor DEEP_LINK_EXECUTOR = - Executors.newSingleThreadExecutor(runnable -> { - Thread thread = new Thread(runnable, "IterableDeepLinkExecutor"); - thread.setDaemon(true); - thread.setPriority(Thread.NORM_PRIORITY); - return thread; - }); + newSingleThreadExecutor("IterableDeepLinkExecutor"); + private static final Executor OFFLINE_EXECUTOR = + newSingleThreadExecutor("IterableOfflineExecutor"); + private static final Executor REQUEST_EXECUTOR = Executors.newFixedThreadPool( + REQUEST_THREAD_COUNT, + runnable -> newThread( + runnable, + "IterableRequestExecutor-" + REQUEST_THREAD_ID.incrementAndGet() + ) + ); private static final Executor MAIN_EXECUTOR = runnable -> new Handler(Looper.getMainLooper()).post(runnable); @@ -34,7 +38,28 @@ static Executor deepLink() { return DEEP_LINK_EXECUTOR; } + static Executor request() { + return REQUEST_EXECUTOR; + } + + static Executor offline() { + return OFFLINE_EXECUTOR; + } + static Executor main() { return MAIN_EXECUTOR; } + + private static Executor newSingleThreadExecutor(String threadName) { + return Executors.newSingleThreadExecutor( + runnable -> newThread(runnable, threadName) + ); + } + + private static Thread newThread(Runnable runnable, String threadName) { + Thread thread = new Thread(runnable, threadName); + thread.setDaemon(true); + thread.setPriority(Thread.NORM_PRIORITY); + return thread; + } } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java new file mode 100644 index 000000000..46326344b --- /dev/null +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java @@ -0,0 +1,117 @@ +package com.iterable.iterableapi; + +import android.os.Handler; +import android.os.Looper; + +import java.util.concurrent.Executor; + +final class IterableRequestDispatcher { + interface RetryScheduler { + void schedule(Runnable runnable, long delayMs); + } + + private static final RetryScheduler SDK_RETRY_SCHEDULER = + (runnable, delayMs) -> + new Handler(Looper.getMainLooper()).postDelayed(runnable, delayMs); + private static final IterableRequestDispatcher ONLINE_DISPATCHER = + sdkDispatcher(IterableExecutors.request()); + private static final IterableRequestDispatcher PUSH_DISPATCHER = + sdkDispatcher(IterableExecutors.push()); + private static final IterableRequestDispatcher OFFLINE_DISPATCHER = + sdkDispatcher(IterableExecutors.offline()); + + private final Executor requestExecutor; + private final Executor callbackExecutor; + private final RetryScheduler retryScheduler; + + static IterableRequestDispatcher online() { + return ONLINE_DISPATCHER; + } + + static IterableRequestDispatcher push() { + return PUSH_DISPATCHER; + } + + static IterableRequestDispatcher offline() { + return OFFLINE_DISPATCHER; + } + + IterableRequestDispatcher( + Executor requestExecutor, + Executor callbackExecutor, + RetryScheduler retryScheduler + ) { + this.requestExecutor = requestExecutor; + this.callbackExecutor = callbackExecutor; + this.retryScheduler = retryScheduler; + } + + void execute(IterableApiRequest request) { + requestExecutor.execute(new IterableRequestTask(request, 0, this)); + } + + void executeRetry(IterableApiRequest request, int retryCount) { + if (request.canRetry()) { + requestExecutor.execute(new IterableRequestTask(request, retryCount, this, true)); + } + } + + void deliverResult(Runnable runnable) { + callbackExecutor.execute(runnable); + } + + void retry(IterableApiRequest request, int retryCount, long delayMs) { + retryScheduler.schedule(() -> executeRetry(request, retryCount), delayMs); + } + + private static IterableRequestDispatcher sdkDispatcher(Executor requestExecutor) { + return new IterableRequestDispatcher( + requestExecutor, + IterableExecutors.main(), + SDK_RETRY_SCHEDULER + ); + } +} + +final class IterableRequestDispatchers { + private static final IterableRequestDispatchers SDK_DISPATCHERS = + new IterableRequestDispatchers( + IterableRequestDispatcher.online(), + IterableRequestDispatcher.push(), + IterableRequestDispatcher.offline() + ); + + private final IterableRequestDispatcher online; + private final IterableRequestDispatcher push; + private final IterableRequestDispatcher offline; + + static IterableRequestDispatchers sdk() { + return SDK_DISPATCHERS; + } + + static IterableRequestDispatchers same(IterableRequestDispatcher dispatcher) { + return new IterableRequestDispatchers(dispatcher, dispatcher, dispatcher); + } + + IterableRequestDispatchers( + IterableRequestDispatcher online, + IterableRequestDispatcher push, + IterableRequestDispatcher offline + ) { + this.online = online; + this.push = push; + this.offline = offline; + } + + IterableRequestDispatcher online() { + return online; + } + + IterableRequestDispatcher push() { + return push; + } + + IterableRequestDispatcher offline() { + return offline; + } +} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java index db2822496..b345220e0 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java @@ -4,9 +4,6 @@ import static com.iterable.iterableapi.IterableConstants.ENDPOINT_GET_REMOTE_CONFIGURATION; import android.net.Uri; -import android.os.AsyncTask; -import android.os.Handler; -import android.os.Looper; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -29,10 +26,10 @@ import java.util.Objects; /** - * Async task to handle sending data to the Iterable server + * Runnable task to handle sending data to the Iterable server * Created by David Truong dt@iterable.com */ -class IterableRequestTask extends AsyncTask { +class IterableRequestTask implements Runnable { static final String TAG = "IterableRequest"; static String overrideUrl; @@ -45,47 +42,75 @@ class IterableRequestTask extends AsyncTask 0) { - iterableApiRequest = params[0]; + IterableRequestTask( + IterableApiRequest iterableApiRequest, + int retryCount, + IterableRequestDispatcher requestDispatcher + ) { + this(iterableApiRequest, retryCount, requestDispatcher, false); + } + + IterableRequestTask( + IterableApiRequest iterableApiRequest, + int retryCount, + IterableRequestDispatcher requestDispatcher, + boolean retry + ) { + this.iterableApiRequest = iterableApiRequest; + this.retryCount = retryCount; + this.requestDispatcher = requestDispatcher; + this.retry = retry; + } + + @Override + public void run() { + if (retry && !iterableApiRequest.canRetry()) { + return; } - return executeApiRequest(iterableApiRequest); + + IterableApiResponse response = executeApiRequest(iterableApiRequest, requestDispatcher); + requestDispatcher.deliverResult(() -> { + if (response != null && (!retry || iterableApiRequest.canRetry())) { + handleResponse(response); + } + }); } - private static void retryRequestWithNewAuthToken(String newAuthToken, IterableApiRequest iterableApiRequest) { + static void retryRequestWithNewAuthToken( + String newAuthToken, + IterableApiRequest iterableApiRequest, + IterableRequestDispatcher requestDispatcher + ) { IterableApiRequest request = new IterableApiRequest( iterableApiRequest.apiKey, iterableApiRequest.resourcePath, iterableApiRequest.json, iterableApiRequest.requestType, newAuthToken, - iterableApiRequest.legacyCallback); - IterableRequestTask requestTask = new IterableRequestTask(); - requestTask.execute(request); + iterableApiRequest.legacyCallback + ); + request.copyRetryStateFrom(iterableApiRequest); + requestDispatcher.executeRetry(request, 0); } @WorkerThread static IterableApiResponse executeApiRequest(IterableApiRequest iterableApiRequest) { - return executeApiRequest( - iterableApiRequest, - IterableRequestTask::retryRequestWithNewAuthToken - ); + return executeApiRequest(iterableApiRequest, IterableRequestDispatcher.online()); } + /** + * Sends the given request to Iterable using a HttpURLConnection. + * Reference - http://developer.android.com/reference/java/net/HttpURLConnection.html + */ @WorkerThread - static IterableApiResponse executeApiRequest( + private static IterableApiResponse executeApiRequest( IterableApiRequest iterableApiRequest, - IterableRequestAuthRetryHandler authRetryHandler + IterableRequestDispatcher requestDispatcher ) { IterableApiResponse apiResponse = null; String requestResult = null; @@ -215,7 +240,7 @@ static IterableApiResponse executeApiRequest( apiResponse = IterableApiResponse.failure(responseCode, requestResult, jsonResponse, "JWT Authorization header error"); IterableApi.getInstance().getAuthManager().handleAuthFailure(iterableApiRequest.authToken, getMappedErrorCodeForMessage(jsonResponse)); - handleJwtAuthRetry(iterableApiRequest, authRetryHandler); + handleJwtAuthRetry(iterableApiRequest, requestDispatcher); } else { apiResponse = IterableApiResponse.failure(responseCode, requestResult, jsonResponse, "Invalid API Key"); } @@ -277,7 +302,7 @@ static IterableApiResponse executeApiRequest( */ private static void handleJwtAuthRetry( IterableApiRequest iterableApiRequest, - IterableRequestAuthRetryHandler authRetryHandler + IterableRequestDispatcher requestDispatcher ) { boolean autoRetry = IterableApi.getInstance().isAutoRetryOnJwtFailure(); if (autoRetry && iterableApiRequest.getProcessorType() == IterableApiRequest.ProcessorType.OFFLINE) { @@ -290,7 +315,7 @@ private static void handleJwtAuthRetry( null ); } else { - requestNewAuthTokenAndRetry(iterableApiRequest, authRetryHandler); + requestNewAuthTokenAndRetry(iterableApiRequest, requestDispatcher); } } @@ -377,11 +402,7 @@ private static boolean isSensitive(String key) { return (key.equals(IterableConstants.HEADER_API_KEY)) || key.equals(IterableConstants.HEADER_SDK_AUTHORIZATION); } - private static final Handler handler = new Handler(Looper.getMainLooper()); - - @Override - protected void onPostExecute(IterableApiResponse response) { - + void handleResponse(IterableApiResponse response) { if (shouldRetry(response)) { retryRequestWithDelay(); return; @@ -394,7 +415,6 @@ protected void onPostExecute(IterableApiResponse response) { if (iterableApiRequest.legacyCallback != null) { iterableApiRequest.legacyCallback.execute(response.responseBody); } - super.onPostExecute(response); } private boolean shouldRetry(IterableApiResponse response) { @@ -402,17 +422,8 @@ private boolean shouldRetry(IterableApiResponse response) { } private void retryRequestWithDelay() { - final IterableRequestTask requestTask = new IterableRequestTask(); - requestTask.setRetryCount(retryCount + 1); - long delay = (retryCount > 2) ? RETRY_DELAY_MS * retryCount : 0; - - handler.postDelayed(new Runnable() { - @Override - public void run() { - requestTask.execute(iterableApiRequest); - } - }, delay); + requestDispatcher.retry(iterableApiRequest, retryCount + 1, delay); } private void handleSuccessResponse(IterableApiResponse response) { @@ -441,7 +452,7 @@ private void handleErrorResponse(IterableApiResponse response) { private static void requestNewAuthTokenAndRetry( IterableApiRequest iterableApiRequest, - IterableRequestAuthRetryHandler authRetryHandler + IterableRequestDispatcher requestDispatcher ) { IterableApi.getInstance().getAuthManager().setIsLastAuthTokenValid(false); long retryInterval = IterableApi.getInstance().getAuthManager().getNextRetryInterval(); @@ -451,9 +462,10 @@ private static void requestNewAuthTokenAndRetry( data -> { try { String newAuthToken = data.getString("newAuthToken"); - authRetryHandler.retryWithNewAuthToken( + retryRequestWithNewAuthToken( newAuthToken, - iterableApiRequest + iterableApiRequest, + requestDispatcher ); } catch (JSONException e) { e.printStackTrace(); @@ -461,14 +473,6 @@ private static void requestNewAuthTokenAndRetry( } ); } - - protected void setRetryCount(int count) { - retryCount = count; - } -} - -interface IterableRequestAuthRetryHandler { - void retryWithNewAuthToken(String newAuthToken, IterableApiRequest request); } /** @@ -486,6 +490,7 @@ class IterableApiRequest { final JSONObject json; final String requestType; final String authToken; + private @Nullable IterableRequestRetryState retryState; private ProcessorType processorType = ProcessorType.ONLINE; IterableHelper.IterableActionHandler legacyCallback; @@ -517,6 +522,18 @@ void setProcessorType(ProcessorType processorType) { this.processorType = processorType; } + void setRetryState(@Nullable IterableRequestRetryState retryState) { + this.retryState = retryState; + } + + void copyRetryStateFrom(@NonNull IterableApiRequest request) { + setRetryState(request.retryState); + } + + boolean canRetry() { + return retryState == null || retryState.canRetry(); + } + IterableApiRequest(String apiKey, String baseUrl, String resourcePath, JSONObject json, String requestType, String authToken, IterableHelper.SuccessHandler onSuccess, IterableHelper.FailureHandler onFailure) { this.apiKey = apiKey; this.baseUrl = baseUrl; diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java index ea5de7e45..5d97c0c97 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java @@ -5,7 +5,6 @@ import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; import org.json.JSONException; import org.json.JSONObject; @@ -20,6 +19,7 @@ class OfflineRequestProcessor implements RequestProcessor { private IterableTaskRunner taskRunner; private IterableTaskStorage taskStorage; private HealthMonitor healthMonitor; + private final IterableRequestDispatcher requestDispatcher; private static final Set offlineApiSet = new HashSet<>(Arrays.asList( IterableConstants.ENDPOINT_TRACK, @@ -38,6 +38,14 @@ class OfflineRequestProcessor implements RequestProcessor { )); OfflineRequestProcessor(Context context) { + this(context, IterableRequestDispatcher.offline()); + } + + OfflineRequestProcessor( + Context context, + IterableRequestDispatcher requestDispatcher + ) { + this.requestDispatcher = requestDispatcher; IterableNetworkConnectivityManager networkConnectivityManager = IterableNetworkConnectivityManager.sharedInstance(context); taskStorage = IterableTaskStorage.sharedInstance(context); healthMonitor = new HealthMonitor(taskStorage); @@ -47,7 +55,7 @@ class OfflineRequestProcessor implements RequestProcessor { networkConnectivityManager, healthMonitor, classification); - taskScheduler = new TaskScheduler(taskStorage, taskRunner); + taskScheduler = new TaskScheduler(taskStorage, taskRunner, requestDispatcher); // Register task runner as auth token ready listener for JWT auto-retry support try { @@ -70,24 +78,40 @@ void dispose() { } } - @VisibleForTesting OfflineRequestProcessor(TaskScheduler scheduler, IterableTaskRunner iterableTaskRunner, IterableTaskStorage storage, HealthMonitor mockHealthMonitor) { + this( + scheduler, + iterableTaskRunner, + storage, + mockHealthMonitor, + IterableRequestDispatcher.offline() + ); + } + + OfflineRequestProcessor( + TaskScheduler scheduler, + IterableTaskRunner iterableTaskRunner, + IterableTaskStorage storage, + HealthMonitor mockHealthMonitor, + IterableRequestDispatcher requestDispatcher + ) { taskRunner = iterableTaskRunner; taskScheduler = scheduler; taskStorage = storage; healthMonitor = mockHealthMonitor; + this.requestDispatcher = requestDispatcher; } @Override public void processGetRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.IterableActionHandler onCallback) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, json, IterableApiRequest.GET, authToken, onCallback); - new IterableRequestTask().execute(request); + requestDispatcher.execute(request); } @Override public void processGetRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.SuccessHandler onSuccess, @Nullable IterableHelper.FailureHandler onFailure) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, json, IterableApiRequest.GET, authToken, onSuccess, onFailure); - new IterableRequestTask().execute(request); + requestDispatcher.execute(request); } @Override @@ -97,7 +121,7 @@ public void processPostRequest(@Nullable String apiKey, @NonNull String resource request.setProcessorType(IterableApiRequest.ProcessorType.OFFLINE); taskScheduler.scheduleTask(request, onSuccess, onFailure); } else { - new IterableRequestTask().execute(request); + requestDispatcher.execute(request); } } @@ -116,10 +140,20 @@ class TaskScheduler implements IterableTaskRunner.TaskCompletedListener { static HashMap failureCallbackMap = new HashMap<>(); private final IterableTaskStorage taskStorage; private final IterableTaskRunner taskRunner; + private final IterableRequestDispatcher requestDispatcher; TaskScheduler(IterableTaskStorage taskStorage, IterableTaskRunner taskRunner) { + this(taskStorage, taskRunner, IterableRequestDispatcher.offline()); + } + + TaskScheduler( + IterableTaskStorage taskStorage, + IterableTaskRunner taskRunner, + IterableRequestDispatcher requestDispatcher + ) { this.taskStorage = taskStorage; this.taskRunner = taskRunner; + this.requestDispatcher = requestDispatcher; taskRunner.addTaskCompletedListener(this); } @@ -129,13 +163,13 @@ void scheduleTask(IterableApiRequest request, @Nullable IterableHelper.SuccessHa serializedRequest = request.toJSONObject(); } catch (JSONException e) { IterableLogger.e("RequestProcessor", "Failed serializing the request for offline execution. Attempting to request the request now..."); - new IterableRequestTask().execute(request); + requestDispatcher.execute(request); return; } String taskId = taskStorage.createTask(request.resourcePath, IterableTaskType.API, serializedRequest.toString()); if (taskId == null) { - new IterableRequestTask().execute(request); + requestDispatcher.execute(request); return; } successCallbackMap.put(taskId, onSuccess); diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java index 013f7f0ad..4b82ded93 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java @@ -1,7 +1,6 @@ package com.iterable.iterableapi; import android.content.Context; -import android.os.AsyncTask; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -14,23 +13,41 @@ class OnlineRequestProcessor implements RequestProcessor { private static final String TAG = "OnlineRequestProcessor"; + private final IterableRequestDispatcher requestDispatcher; + + OnlineRequestProcessor() { + this(IterableRequestDispatcher.online()); + } + + OnlineRequestProcessor(IterableRequestDispatcher requestDispatcher) { + this.requestDispatcher = requestDispatcher; + } @Override public void processGetRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.IterableActionHandler onCallback) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, addCreatedAtToJson(json), IterableApiRequest.GET, authToken, onCallback); - new IterableRequestTask().executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, request); + executeRequest(request, null); } @Override public void processGetRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.SuccessHandler onSuccess, @Nullable IterableHelper.FailureHandler onFailure) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, addCreatedAtToJson(json), IterableApiRequest.GET, authToken, onSuccess, onFailure); - new IterableRequestTask().executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, request); + executeRequest(request, null); } @Override public void processPostRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.SuccessHandler onSuccess, @Nullable IterableHelper.FailureHandler onFailure) { + processPostRequest(apiKey, resourcePath, json, authToken, onSuccess, onFailure, null); + } + + void processPostRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.SuccessHandler onSuccess, @Nullable IterableHelper.FailureHandler onFailure, @Nullable IterableRequestRetryState retryState) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, addCreatedAtToJson(json), IterableApiRequest.POST, authToken, onSuccess, onFailure); - new IterableRequestTask().executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, request); + executeRequest(request, retryState); + } + + private void executeRequest(@NonNull IterableApiRequest request, @Nullable IterableRequestRetryState retryState) { + request.setRetryState(retryState); + requestDispatcher.execute(request); } @Override @@ -47,7 +64,7 @@ JSONObject addCreatedAtToJson(JSONObject jsonObject) { createdAt = new Date().getTime() / 1000; } jsonObject.put(IterableConstants.KEY_CREATED_AT, createdAt); - } catch (JSONException e) { + } catch (JSONException | NumberFormatException e) { IterableLogger.e(TAG, "Could not add createdAt timestamp to json object"); } return jsonObject; diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java index 9f66e61f6..4c6b22122 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java @@ -7,6 +7,7 @@ import com.iterable.iterableapi.unit.TestRunner; import org.junit.After; +import org.junit.Before; import org.junit.Rule; import org.junit.rules.TestWatcher; import org.junit.runner.Description; @@ -23,12 +24,17 @@ public abstract class BaseTest { @Rule public AsyncTaskRule asyncTaskRule = new AsyncTaskRule(); + @Before + public void baseTestSetUp() { + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); + } + @After public void baseTestTearDown() { IterableActivityMonitor.getInstance().unregisterLifecycleCallbacks(getContext()); IterableActivityMonitor.instance = new IterableActivityMonitor(); IterablePushNotificationUtil.clearPendingAction(); - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); } protected IterableUtilImpl getIterableUtilSpy() { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/EmbeddedSessionManagerThreadSafetyTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/EmbeddedSessionManagerThreadSafetyTest.java index 7560c5e6e..1e9656609 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/EmbeddedSessionManagerThreadSafetyTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/EmbeddedSessionManagerThreadSafetyTest.java @@ -24,7 +24,7 @@ public class EmbeddedSessionManagerThreadSafetyTest extends BaseTest { @Before public void setUp() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); sessionManager = new EmbeddedSessionManager(); } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthJWTTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthJWTTests.java index c5520bf5b..f86a74b47 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthJWTTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthJWTTests.java @@ -154,7 +154,7 @@ public void tearDown() throws IOException { } private void reInitIterableApi() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); authHandler = mock(IterableAuthHandler.class); IterableTestUtils.createIterableApiNew(builder -> builder.setAuthHandler(authHandler), null); IterableConfig iterableConfig = new IterableConfig.Builder().setEnableUnknownUserActivation(true).setAuthHandler(authHandler).build(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java index 2fa168b59..666f2f461 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthSecurityTests.java @@ -65,7 +65,7 @@ public void tearDown() throws IOException { } private void initIterableWithAuth() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); authHandler = mock(IterableAuthHandler.class); IterableConfig iterableConfig = new IterableConfig.Builder() .setAuthHandler(authHandler) @@ -75,7 +75,7 @@ private void initIterableWithAuth() { } private void initIterableWithoutAuth() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); IterableConfig iterableConfig = new IterableConfig.Builder() .setAutoPushRegistration(false) .build(); @@ -354,4 +354,3 @@ public void testStaleKeychainCredentials_NoToken_SkipsSensitiveOps() throws Exce verify(mockEmbeddedManager, never()).syncMessages(); } } - diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java index 7ba5961c6..d2533d644 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiAuthTests.java @@ -66,7 +66,7 @@ public void tearDown() throws IOException { } private void reInitIterableApi() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); authHandler = mock(IterableAuthHandler.class); } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCriteriaFetchTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCriteriaFetchTests.java index 155117168..20b1623ae 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCriteriaFetchTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCriteriaFetchTests.java @@ -47,7 +47,7 @@ public void setUp() { } private void reInitIterableApi() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); } @After diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCustomEventTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCustomEventTests.java index e1c4c6639..8a6713d85 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCustomEventTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiCustomEventTests.java @@ -101,7 +101,7 @@ public void setUp() { private void reInitIterableApi() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); } @After diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java index e6576b06f..2e5c12a9c 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java @@ -41,7 +41,7 @@ public void setUp() { server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); IterableInAppManager inAppManagerMock = mock(IterableInAppManager.class); - IterableApi.sharedInstance = new IterableApi(inAppManagerMock); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManagerMock); originalPushRegistrationUtil = IterablePushRegistrationTask.Util.instance; pushRegistrationUtilMock = mock(IterablePushRegistrationTask.Util.UtilImpl.class); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiMergeUserEmailTests.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiMergeUserEmailTests.java index fba70269f..53b08aa03 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiMergeUserEmailTests.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiMergeUserEmailTests.java @@ -144,7 +144,7 @@ public void setUp() { } private void reInitIterableApi() { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); } @After diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestExecutionTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestExecutionTest.kt new file mode 100644 index 000000000..c30c89554 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestExecutionTest.kt @@ -0,0 +1,200 @@ +package com.iterable.iterableapi + +import android.os.Looper +import okhttp3.mockwebserver.Dispatcher +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.fail +import org.junit.Before +import org.junit.Test +import org.robolectric.Shadows.shadowOf +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.CopyOnWriteArrayList + +class IterableApiRequestExecutionTest : BaseTest() { + private lateinit var server: MockWebServer + private val recordedRequests = CopyOnWriteArrayList() + private val updateEmailResponses = ConcurrentLinkedQueue() + + @Before + fun setUp() { + getContext() + .getSharedPreferences( + IterableConstants.SHARED_PREFS_SAVED_CONFIGURATION, + 0 + ) + .edit() + .clear() + .commit() + + server = MockWebServer() + server.dispatcher = object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse { + recordedRequests.add(request) + if (request.isFor(IterableConstants.ENDPOINT_UPDATE_EMAIL)) { + return updateEmailResponses.poll() ?: successfulResponse() + } + return successfulResponse() + } + } + + IterableApi.overrideURLEndpointPath(server.url("/").toString()) + IterableApi.initialize( + getContext(), + "api-key", + IterableConfig.Builder() + .setAutoPushRegistration(false) + .setEnableEmbeddedMessaging(true) + .build() + ) + IterableApi.getInstance().setEmail("current@example.com") + runMainCallbacks() + recordedRequests.clear() + } + + @After + fun tearDown() { + server.shutdown() + } + + @Test + fun `updating email calls the client success callback on main`() { + updateEmailResponses.add(successfulResponse()) + var callbackLooper: Looper? = null + var failureCalled = false + + IterableApi.getInstance().updateEmail( + "new@example.com", + IterableHelper.SuccessHandler { + callbackLooper = Looper.myLooper() + }, + IterableHelper.FailureHandler { _, _ -> + failureCalled = true + } + ) + runMainCallbacks() + + assertEquals("new@example.com", IterableApi.getInstance().email) + assertSame(Looper.getMainLooper(), callbackLooper) + assertFalse(failureCalled) + assertEquals( + "current@example.com", + updateEmailRequests().single().bodyAsJson().getString("currentEmail") + ) + } + + @Test + fun `updating email calls the client failure callback on main`() { + updateEmailResponses.add( + MockResponse() + .setResponseCode(400) + .setBody("""{"msg":"Invalid email"}""") + ) + var callbackLooper: Looper? = null + var failureReason: String? = null + var responseData: JSONObject? = null + + IterableApi.getInstance().updateEmail( + "invalid", + IterableHelper.SuccessHandler { + fail("Success callback should not be called") + }, + IterableHelper.FailureHandler { reason, data -> + callbackLooper = Looper.myLooper() + failureReason = reason + responseData = data + } + ) + runMainCallbacks() + + assertEquals("current@example.com", IterableApi.getInstance().email) + assertSame(Looper.getMainLooper(), callbackLooper) + assertEquals("Invalid email", failureReason) + assertEquals(400, responseData?.getInt(IterableConstants.HTTP_STATUS_CODE)) + assertEquals(1, updateEmailRequests().size) + } + + @Test + fun `getting embedded messages calls the legacy client callback on main`() { + var callbackLooper: Looper? = null + var callbackData: String? = null + + IterableApi.getInstance().getEmbeddedMessages( + arrayOf(123L), + IterableHelper.IterableActionHandler { data -> + callbackLooper = Looper.myLooper() + callbackData = data + } + ) + runMainCallbacks() + + assertSame(Looper.getMainLooper(), callbackLooper) + assertEquals("{}", callbackData) + assertEquals( + 1, + recordedRequests.count { + it.isFor(IterableConstants.ENDPOINT_GET_EMBEDDED_MESSAGES) + } + ) + } + + @Test + fun `server error is retried before calling success exactly once`() { + updateEmailResponses.add( + MockResponse() + .setResponseCode(500) + .setBody("""{"msg":"Server unavailable"}""") + ) + updateEmailResponses.add(successfulResponse()) + var successCount = 0 + var failureCount = 0 + var callbackLooper: Looper? = null + + IterableApi.getInstance().updateEmail( + "new@example.com", + IterableHelper.SuccessHandler { + callbackLooper = Looper.myLooper() + successCount++ + }, + IterableHelper.FailureHandler { _, _ -> + failureCount++ + } + ) + runMainCallbacks() + + assertEquals(2, updateEmailRequests().size) + assertEquals(1, successCount) + assertEquals(0, failureCount) + assertSame(Looper.getMainLooper(), callbackLooper) + } + + private fun runMainCallbacks() { + shadowOf(Looper.getMainLooper()).idle() + } + + private fun successfulResponse(): MockResponse { + return MockResponse() + .setResponseCode(200) + .setBody("{}") + } + + private fun updateEmailRequests(): List { + return recordedRequests.filter { + it.isFor(IterableConstants.ENDPOINT_UPDATE_EMAIL) + } + } + + private fun RecordedRequest.isFor(endpoint: String): Boolean { + return path?.substringBefore("?") == "/$endpoint" + } + + private fun RecordedRequest.bodyAsJson(): JSONObject { + return JSONObject(body.clone().readUtf8()) + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestTest.java index 166f1bcbb..7e062f123 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiRequestTest.java @@ -13,22 +13,25 @@ import org.json.JSONObject; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; +import org.skyscreamer.jsonassert.JSONAssert; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.robolectric.Shadows.shadowOf; +import android.os.Looper; import junit.framework.Assert; import java.util.ArrayList; -import java.util.Date; import java.util.List; import java.util.concurrent.TimeUnit; +import okhttp3.mockwebserver.Dispatcher; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.QueueDispatcher; import okhttp3.mockwebserver.RecordedRequest; @RunWith(TestRunner.class) @@ -37,10 +40,25 @@ public class IterableApiRequestTest { private MockWebServer server; @Before - public void setUp() { - createIterableApi(); + public void setUp() throws Exception { server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); + server.setDispatcher(new Dispatcher() { + @NonNull + @Override + public MockResponse dispatch(@NonNull RecordedRequest request) { + return new MockResponse().setResponseCode(200).setBody("{}"); + } + }); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); + createIterableApi(); + + shadowOf(Looper.getMainLooper()).idle(); + int bootstrapRequestCount = server.getRequestCount(); + for (int i = 0; i < bootstrapRequestCount; i++) { + assertNotNull(server.takeRequest(5, TimeUnit.SECONDS)); + } + server.setDispatcher(new QueueDispatcher()); } @After @@ -99,20 +117,18 @@ public void testUpdateCart() throws Exception { assertNotNull(request); Assert.assertEquals("/" + IterableConstants.ENDPOINT_UPDATE_CART, request.getPath()); - String expectedRequest = new StringBuilder( - new StringBuffer("{\"user\":{\"email\":\"test_email\"},") - .append("\"items\":[{\"id\":\"sku123\",\"name\":\"Item\",\"price\":50,\"quantity\":2}],") - .append("\"createdAt\":").append(new Date().getTime() / 1000) - .append("}")).toString(); - - String requestBody = request.getBody().readUtf8(); - Assert.assertEquals(expectedRequest, requestBody); + JSONObject requestJson = requestBodyWithoutCreatedAt(request); + JSONAssert.assertEquals( + "{\"user\":{\"email\":\"test_email\"}," + + "\"items\":[{\"id\":\"sku123\",\"name\":\"Item\"," + + "\"price\":50,\"quantity\":2}]}", + requestJson, + true + ); } @Test public void testTrackPurchase() throws Exception { - String expectedRequest = new StringBuilder(new StringBuffer("{\"user\":{\"email\":\"test_email\"},\"items\":[{\"id\":\"sku123\",\"name\":\"Item\",\"price\":50,\"quantity\":2}],\"total\":100").append(",\"createdAt\":").append(new Date().getTime() / 1000).append("}")).toString(); - CommerceItem item1 = new CommerceItem("sku123", "Item", 50.0, 2); List items = new ArrayList(); items.add(item1); @@ -121,13 +137,18 @@ public void testTrackPurchase() throws Exception { RecordedRequest request = server.takeRequest(5, TimeUnit.SECONDS); Assert.assertEquals("/" + IterableConstants.ENDPOINT_TRACK_PURCHASE, request.getPath()); - Assert.assertEquals(expectedRequest, request.getBody().readUtf8()); + JSONObject requestJson = requestBodyWithoutCreatedAt(request); + JSONAssert.assertEquals( + "{\"user\":{\"email\":\"test_email\"}," + + "\"items\":[{\"id\":\"sku123\",\"name\":\"Item\"," + + "\"price\":50,\"quantity\":2}],\"total\":100}", + requestJson, + true + ); } @Test public void testTrackPurchaseWithDataFields() throws Exception { - String expectedRequest = new StringBuilder(new StringBuffer("{\"user\":{\"email\":\"test_email\"},\"items\":[{\"id\":\"sku123\",\"name\":\"Item\",\"price\":50,\"quantity\":2}],\"total\":100,\"dataFields\":{\"field\":\"testValue\"}").append(",\"createdAt\":").append(new Date().getTime() / 1000).append("}")).toString(); - CommerceItem item1 = new CommerceItem("sku123", "Item", 50.0, 2); List items = new ArrayList(); items.add(item1); @@ -139,7 +160,15 @@ public void testTrackPurchaseWithDataFields() throws Exception { RecordedRequest request = server.takeRequest(5, TimeUnit.SECONDS); assertNotNull(request); Assert.assertEquals("/" + IterableConstants.ENDPOINT_TRACK_PURCHASE, request.getPath()); - Assert.assertEquals(expectedRequest, request.getBody().readUtf8()); + JSONObject requestJson = requestBodyWithoutCreatedAt(request); + JSONAssert.assertEquals( + "{\"user\":{\"email\":\"test_email\"}," + + "\"items\":[{\"id\":\"sku123\",\"name\":\"Item\"," + + "\"price\":50,\"quantity\":2}],\"total\":100," + + "\"dataFields\":{\"field\":\"testValue\"}}", + requestJson, + true + ); } @Test @@ -188,19 +217,24 @@ public void testTrackPurchaseWithOptionalParameters() throws Exception { IterableApi.sharedInstance.trackPurchase(42, items); - long createdAt = new Date().getTime() / 1000; RecordedRequest request = server.takeRequest(5, TimeUnit.SECONDS); assert request != null; Assert.assertEquals("/" + IterableConstants.ENDPOINT_TRACK_PURCHASE, request.getPath()); - String expectedRequest = new StringBuilder( - new StringBuffer("{\"user\":{\"email\":\"test_email\"},") - .append("\"items\":[{\"id\":\"273\",\"name\":\"Bow and Arrow\",\"price\":42,\"quantity\":1,\"sku\":\"DIAMOND-IS-UNBREAKABLE\",\"description\":\"When a living creature is pierced by one of the Arrows, it will catalyze and awaken the individual’s dormant Stand.\",\"url\":\"placeholderUrl\",\"imageUrl\":\"placeholderImageUrl\",\"dataFields\":{\"color\":\"yellow\",\"count\":8},\"categories\":[\"bow\",\"arrow\"]}],") - .append("\"total\":42,").append("\"createdAt\":").append(createdAt) - .append("}")).toString(); - - String requestBody = request.getBody().readUtf8(); - Assert.assertEquals(expectedRequest, requestBody); + JSONObject requestJson = requestBodyWithoutCreatedAt(request); + JSONAssert.assertEquals( + "{\"user\":{\"email\":\"test_email\"}," + + "\"items\":[{\"id\":\"273\",\"name\":\"Bow and Arrow\"," + + "\"price\":42,\"quantity\":1," + + "\"sku\":\"DIAMOND-IS-UNBREAKABLE\"," + + "\"description\":\"When a living creature is pierced by one of the " + + "Arrows, it will catalyze and awaken the individual’s dormant Stand.\"," + + "\"url\":\"placeholderUrl\",\"imageUrl\":\"placeholderImageUrl\"," + + "\"dataFields\":{\"color\":\"yellow\",\"count\":8}," + + "\"categories\":[\"bow\",\"arrow\"]}],\"total\":42}", + requestJson, + true + ); } @Test @@ -225,38 +259,6 @@ public void testPostRequestHeaders() throws Exception { Assert.assertEquals("fake_key", request.getHeader(IterableConstants.HEADER_API_KEY)); } - @Ignore("Blocked: IterableAuthManager.executor is not injectable - auth token requests run on uncontrollable background thread") - @Test - public void testUpdateEmailRequest() throws Exception { - server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); - - // Plain request check - IterableApi.sharedInstance.updateEmail("test@example.com"); - - RecordedRequest request1 = server.takeRequest(5, TimeUnit.SECONDS); - assertNotNull(request1); - Assert.assertEquals("/" + IterableConstants.ENDPOINT_UPDATE_EMAIL, request1.getPath()); - Assert.assertEquals("{\"currentEmail\":\"test_email\",\"newEmail\":\"test@example.com\"}", request1.getBody().readUtf8()); - Thread.sleep(100); // We need the callback to run to verify the internal email field change - - server.enqueue(new MockResponse().setResponseCode(400).setBody("{}")); - - // Check that we handle failures properly - IterableApi.sharedInstance.updateEmail("invalid_mail!!123"); - - RecordedRequest request2 = server.takeRequest(5, TimeUnit.SECONDS); - assertNotNull(request2); - Assert.assertEquals("{\"currentEmail\":\"test@example.com\",\"newEmail\":\"invalid_mail!!123\"}", request2.getBody().readUtf8()); - Thread.sleep(100); // We need the callback to run to verify the internal email field change - - // Check that we still pass a valid (old) email after trying to update to an invalid one - IterableApi.sharedInstance.updateEmail("another@email.com"); - - RecordedRequest request3 = server.takeRequest(5, TimeUnit.SECONDS); - assertNotNull(request3); - Assert.assertEquals("{\"currentEmail\":\"test@example.com\",\"newEmail\":\"another@email.com\"}", request3.getBody().readUtf8()); - } - @Test public void testTrackConsentWithUserId() throws Exception { server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); @@ -346,4 +348,12 @@ public void testTrackConsentRequestHeaders() throws Exception { Assert.assertEquals("fake_key", request.getHeader(IterableConstants.HEADER_API_KEY)); assertNotNull("sentAt header should be present", request.getHeader(IterableConstants.KEY_SENT_AT)); } + + private JSONObject requestBodyWithoutCreatedAt(RecordedRequest request) throws Exception { + JSONObject requestJson = new JSONObject(request.getBody().readUtf8()); + assertTrue(requestJson.has(IterableConstants.KEY_CREATED_AT)); + assertTrue(requestJson.getLong(IterableConstants.KEY_CREATED_AT) > 0); + requestJson.remove(IterableConstants.KEY_CREATED_AT); + return requestJson; + } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java index e2d0aa7ee..bab548907 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java @@ -83,7 +83,7 @@ private void reInitIterableApi() { IterablePushRegistration pushRegistration = new IterablePushRegistration(pushRegistrationExecutor); - IterableApi.sharedInstance = new IterableApi( + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests( inAppManagerMock, embeddedManagerMock, pushRegistration); originalApiClient = IterableApi.sharedInstance.apiClient; diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAsyncInitializationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAsyncInitializationTest.java index c0c0a2b42..ab1c28238 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableAsyncInitializationTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableAsyncInitializationTest.java @@ -15,7 +15,6 @@ import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLooper; -import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -158,7 +157,7 @@ public void onSDKInitialized() { @Test public void testOperationQueuing_DuringInitialization() throws InterruptedException { CountDownLatch initLatch = new CountDownLatch(1); - List executedOperations = new ArrayList<>(); + CountDownLatch operationsProcessed = new CountDownLatch(1); // Start background initialization IterableApi.initializeInBackground(context, TEST_API_KEY, new IterableInitializationCallback() { @@ -174,12 +173,18 @@ public void onSDKInitialized() { IterableApi.getInstance().setEmail(TEST_EMAIL); IterableApi.getInstance().track("testEvent"); IterableApi.getInstance().setUserId(TEST_USER_ID); + IterableBackgroundInitializer.queueOrExecute( + operationsProcessed::countDown, + "test operation queue sentinel" + ); // Verify operations are queued assertTrue("Operations should be queued", IterableBackgroundInitializer.getQueuedOperationCount() > 0); // Wait for initialization to complete assertTrue("Initialization should complete", waitForAsyncInitialization(initLatch, 5)); + assertTrue("Queued operations should complete", + operationsProcessed.await(5, TimeUnit.SECONDS)); // Process queue ShadowLooper.runUiThreadTasksIncludingDelayedTasks(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableEmbeddedManagerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableEmbeddedManagerTest.java index 111c222aa..aebf5d7aa 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableEmbeddedManagerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableEmbeddedManagerTest.java @@ -34,7 +34,7 @@ public void setUp() throws IOException { server.setDispatcher(dispatcher); IterableApi.overrideURLEndpointPath(server.url("").toString()); - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { @@ -327,4 +327,4 @@ public void testOnEmbeddedMessagingSyncFailed() throws Exception { verify(mockHandler, never()).onEmbeddedMessagingSyncSucceeded(); } -} \ No newline at end of file +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppDialogNotificationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppDialogNotificationTest.java index 2d3bd20e3..f316598ec 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppDialogNotificationTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppDialogNotificationTest.java @@ -363,7 +363,7 @@ public void backPress_shouldTrackClickCloseAndRemove() { public void hostRealDestroy_shouldCallProcessMessageRemoval_beforeDismiss() { IterableInAppManager mockManager = Mockito.mock(IterableInAppManager.class); IterableApi originalApi = IterableApi.sharedInstance; - IterableApi.sharedInstance = new IterableApi(mockManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(mockManager); IterableInAppMessage message = mockMessage("destroy-msg"); Mockito.when(message.isMarkedForDeletion()).thenReturn(true); @@ -397,7 +397,7 @@ public void hostRealDestroy_shouldCallProcessMessageRemoval_beforeDismiss() { public void hostConfigChangeDestroy_shouldNotConsumeMessage_butStillDismiss() { IterableInAppManager mockManager = Mockito.mock(IterableInAppManager.class); IterableApi originalApi = IterableApi.sharedInstance; - IterableApi.sharedInstance = new IterableApi(mockManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(mockManager); IterableInAppMessage message = mockMessage("rotation-msg"); Mockito.when(message.isMarkedForDeletion()).thenReturn(true); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerSyncTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerSyncTest.java index 6d03f73cf..352f4cfaf 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerSyncTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerSyncTest.java @@ -75,7 +75,7 @@ public void testSyncOnLaunch() throws Exception { @Test public void testSyncOnLogin() throws Exception { IterableInAppManager inAppManagerMock = mock(IterableInAppManager.class); - IterableApi.sharedInstance = new IterableApi(inAppManagerMock); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManagerMock); IterableApi.initialize(getApplicationContext(), "apiKey"); // Reset after initialize since it may also trigger syncInApp via background init reset(inAppManagerMock); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java index 8733087cf..e631d0b4a 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java @@ -71,7 +71,7 @@ public void setUp() throws Exception { customActionHandler = mock(IterableCustomActionHandler.class); urlHandler = mock(IterableUrlHandler.class); IterableApi.overrideURLEndpointPath(server.url("").toString()); - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { @@ -278,7 +278,7 @@ public void testHandleActionLink() throws Exception { IterableInAppDisplayer inAppDisplayerMock = mock(IterableInAppDisplayer.class); IterableInAppManager inAppManager = spy(new IterableInAppManager(IterableApi.sharedInstance, new IterableDefaultInAppHandler(), 30.0, new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), inAppDisplayerMock)); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { @@ -345,7 +345,7 @@ public void testHandleCustomActionDelete() throws Exception { IterableInAppDisplayer inAppDisplayerMock = mock(IterableInAppDisplayer.class); IterableInAppManager inAppManager = spy(new IterableInAppManager(IterableApi.sharedInstance, new IterableSkipInAppHandler(), 30.0, new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), inAppDisplayerMock)); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { @@ -406,7 +406,7 @@ private IterableInAppManager createManagerWithHandler(IterableInAppDisplayer dis IterableActivityMonitor.getInstance().unregisterLifecycleCallbacks(getContext()); IterableActivityMonitor.instance = new IterableActivityMonitor(); IterableInAppManager inAppManager = spy(new IterableInAppManager(IterableApi.sharedInstance, handler, 30.0, new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), displayer)); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { @@ -582,7 +582,7 @@ public void testJsonOnlyMessageDisplay() throws Exception { new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), mockDisplayer)); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(payload.toString())); @@ -717,7 +717,7 @@ public void testJsonOnlyInAppMessageDelegateCallbacks() throws Exception { new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), mock(IterableInAppDisplayer.class))); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); // Flush constructor sync callback so messages are loaded shadowOf(getMainLooper()).idle(); @@ -871,7 +871,7 @@ public void testJsonOnlyInAppMessageProcessingAndDisplay() throws Exception { new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), mockDisplayer)); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); // First sync to get messages inAppManager.syncInApp(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInboxTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInboxTest.java index 7b2b8de57..72f130590 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInboxTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInboxTest.java @@ -52,7 +52,7 @@ public void setUp() throws Exception { customActionHandler = mock(IterableCustomActionHandler.class); urlHandler = mock(IterableUrlHandler.class); IterableApi.overrideURLEndpointPath(server.url("").toString()); - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { @@ -214,7 +214,7 @@ public void testShowInboxMessageImmediate() throws Exception { IterableInAppDisplayer inAppDisplayerMock = mock(IterableInAppDisplayer.class); when(inAppDisplayerMock.showMessage(any(IterableInAppMessage.class), eq(IterableInAppLocation.IN_APP), any(IterableHelper.IterableUrlCallback.class))).thenReturn(true); IterableInAppManager inAppManager = spy(new IterableInAppManager(IterableApi.sharedInstance, new IterableDefaultInAppHandler(), 30.0, new IterableInAppMemoryStorage(), IterableActivityMonitor.getInstance(), inAppDisplayerMock)); - IterableApi.sharedInstance = new IterableApi(inAppManager); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); IterableTestUtils.createIterableApiNew(new IterableTestUtils.ConfigBuilderExtender() { @Override public IterableConfig.Builder run(IterableConfig.Builder builder) { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterablePushActionReceiverTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterablePushActionReceiverTest.java index b45841354..d6431392e 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterablePushActionReceiverTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterablePushActionReceiverTest.java @@ -39,7 +39,7 @@ public class IterablePushActionReceiverTest extends BaseTest { @Before public void setUp() throws Exception { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); IterableTestUtils.createIterableApi(); server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt new file mode 100644 index 000000000..9745f8f1e --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt @@ -0,0 +1,82 @@ +package com.iterable.iterableapi + +import com.iterable.iterableapi.unit.TestRunner +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import java.util.concurrent.Executor + +@RunWith(TestRunner::class) +class IterableRequestDispatcherTest { + private val requestExecutor = RecordingExecutor() + private val callbackExecutor = RecordingExecutor() + private val retryScheduler = RecordingRetryScheduler() + private val dispatcher = IterableRequestDispatcher( + requestExecutor, + callbackExecutor, + retryScheduler + ) + + @Test + fun `executing a request submits runnable work to the request executor`() { + dispatcher.execute(request()) + + assertEquals(1, requestExecutor.tasks.size) + assertTrue(requestExecutor.tasks.single() is IterableRequestTask) + } + + @Test + fun `delivering a result submits the callback to the callback executor`() { + val callback = Runnable {} + + dispatcher.deliverResult(callback) + + assertSame(callback, callbackExecutor.tasks.single()) + } + + @Test + fun `retry waits for the scheduler before submitting request work`() { + dispatcher.retry(request(), 4, 6_000) + + assertTrue(requestExecutor.tasks.isEmpty()) + assertEquals(6_000L, retryScheduler.delayMs) + + retryScheduler.task?.run() + + assertEquals(1, requestExecutor.tasks.size) + assertTrue(requestExecutor.tasks.single() is IterableRequestTask) + } + + private fun request(): IterableApiRequest { + return IterableApiRequest( + "api-key", + "api/test", + JSONObject(), + IterableApiRequest.POST, + null, + null, + null + ) + } + + private class RecordingExecutor : Executor { + val tasks = mutableListOf() + + override fun execute(command: Runnable) { + tasks.add(command) + } + } + + private class RecordingRetryScheduler : IterableRequestDispatcher.RetryScheduler { + var task: Runnable? = null + var delayMs: Long? = null + + override fun schedule(runnable: Runnable, delayMs: Long) { + task = runnable + this.delayMs = delayMs + } + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt new file mode 100644 index 000000000..57ac1fce7 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt @@ -0,0 +1,167 @@ +package com.iterable.iterableapi + +import com.iterable.iterableapi.unit.TestRunner +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyLong +import org.mockito.Mockito.mock +import org.mockito.Mockito.never +import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoInteractions + +@RunWith(TestRunner::class) +class IterableRequestTaskTest { + private lateinit var requestDispatcher: IterableRequestDispatcher + + @Before + fun setUp() { + requestDispatcher = mock(IterableRequestDispatcher::class.java) + } + + @Test + fun `server error schedules a retry without calling a terminal callback`() { + val success = mock(IterableHelper.SuccessHandler::class.java) + val failure = mock(IterableHelper.FailureHandler::class.java) + val request = request(success, failure) + + IterableRequestTask(request, 0, requestDispatcher).handleResponse( + serverError() + ) + + verify(requestDispatcher).retry(request, 1, 0) + verifyNoInteractions(success, failure) + } + + @Test + fun `later server error retries use the existing backoff`() { + val request = request() + + IterableRequestTask(request, 3, requestDispatcher).handleResponse( + serverError() + ) + + verify(requestDispatcher).retry( + request, + 4, + IterableRequestTask.RETRY_DELAY_MS * 3 + ) + } + + @Test + fun `server error after retry limit calls failure exactly once`() { + val failure = mock(IterableHelper.FailureHandler::class.java) + val request = request(onFailure = failure) + val responseData = JSONObject() + + IterableRequestTask( + request, + IterableRequestTask.MAX_RETRY_COUNT + 1, + requestDispatcher + ).handleResponse( + IterableApiResponse.failure( + 500, + """{"msg":"Server unavailable"}""", + responseData, + "Server unavailable" + ) + ) + + verify(requestDispatcher, never()).retry( + any(IterableApiRequest::class.java), + anyInt(), + anyLong() + ) + verify(failure).onFailure("Server unavailable", responseData) + assertEquals( + 500, + responseData.getInt(IterableConstants.HTTP_STATUS_CODE) + ) + } + + @Test + fun `auth retry does not attach modern callbacks to the retried request`() { + val success = mock(IterableHelper.SuccessHandler::class.java) + val failure = mock(IterableHelper.FailureHandler::class.java) + val request = request(success, failure).apply { + setProcessorType(IterableApiRequest.ProcessorType.OFFLINE) + } + + IterableRequestTask.retryRequestWithNewAuthToken( + "fresh-token", + request, + requestDispatcher + ) + + val retriedRequest = captureDispatchedRequest() + assertEquals("fresh-token", retriedRequest.authToken) + assertEquals(request.resourcePath, retriedRequest.resourcePath) + assertSame(request.json, retriedRequest.json) + assertNull(retriedRequest.successCallback) + assertNull(retriedRequest.failureCallback) + assertEquals( + IterableApiRequest.ProcessorType.ONLINE, + retriedRequest.processorType + ) + } + + @Test + fun `auth retry keeps the legacy callback and replaces token`() { + val callback = mock(IterableHelper.IterableActionHandler::class.java) + val request = IterableApiRequest( + "api-key", + "api/test", + JSONObject(), + IterableApiRequest.GET, + "expired-token", + callback + ) + + IterableRequestTask.retryRequestWithNewAuthToken( + "fresh-token", + request, + requestDispatcher + ) + + val retriedRequest = captureDispatchedRequest() + assertEquals("fresh-token", retriedRequest.authToken) + assertSame(callback, retriedRequest.legacyCallback) + } + + private fun captureDispatchedRequest(): IterableApiRequest { + val requestCaptor = ArgumentCaptor.forClass(IterableApiRequest::class.java) + verify(requestDispatcher).execute(requestCaptor.capture()) + return requestCaptor.value + } + + private fun request( + onSuccess: IterableHelper.SuccessHandler? = null, + onFailure: IterableHelper.FailureHandler? = null + ): IterableApiRequest { + return IterableApiRequest( + "api-key", + "api/test", + JSONObject(), + IterableApiRequest.POST, + "expired-token", + onSuccess, + onFailure + ) + } + + private fun serverError(): IterableApiResponse { + return IterableApiResponse.failure( + 500, + """{"msg":"Server unavailable"}""", + JSONObject(), + "Server unavailable" + ) + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java index 5559ba72a..ff17da9bc 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java @@ -182,7 +182,7 @@ private String createJwt401ResponseBody() throws Exception { } private IterableAuthHandler initApiWithAutoRetry(boolean autoRetryEnabled) { - IterableApi.sharedInstance = new IterableApi(); + IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); final IterableAuthHandler mockAuthHandler = mock(IterableAuthHandler.class); doReturn(null).when(mockAuthHandler).onAuthTokenRequested(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java index 4810aaad7..9401bfa9c 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java @@ -1,5 +1,7 @@ package com.iterable.iterableapi; +import android.os.Handler; +import android.os.Looper; import android.os.Bundle; import androidx.test.core.app.ApplicationProvider; @@ -23,6 +25,13 @@ public class IterableTestUtils { public static final String apiKey = "fake_key"; public static final String userEmail = "test_email"; + private static final IterableRequestDispatcher INLINE_REQUEST_DISPATCHER = + new IterableRequestDispatcher( + Runnable::run, + IterableExecutors.main(), + (runnable, delayMs) -> + new Handler(Looper.getMainLooper()).postDelayed(runnable, delayMs) + ); public interface ConfigBuilderExtender { IterableConfig.Builder run(IterableConfig.Builder builder); @@ -53,8 +62,34 @@ public static void createIterableApiNew(ConfigBuilderExtender extender, String e IterableApi.getInstance().setEmail(email); } + /** + * Creates an API whose request work runs inline while callbacks and retries + * still use the main looper, matching the production delivery contract. + */ + static IterableApi newApiWithInlineRequests() { + IterableApi api = new IterableApi(); + api.setRequestDispatcher(INLINE_REQUEST_DISPATCHER); + return api; + } + + static IterableApi newApiWithInlineRequests(IterableInAppManager inAppManager) { + IterableApi api = new IterableApi(inAppManager); + api.setRequestDispatcher(INLINE_REQUEST_DISPATCHER); + return api; + } + + static IterableApi newApiWithInlineRequests( + IterableInAppManager inAppManager, + IterableEmbeddedManager embeddedManager, + IterablePushRegistration pushRegistration + ) { + IterableApi api = new IterableApi(inAppManager, embeddedManager, pushRegistration); + api.setRequestDispatcher(INLINE_REQUEST_DISPATCHER); + return api; + } + public static void resetIterableApi() { - IterableApi.sharedInstance = new IterableApi(mock(IterableInAppManager.class)); + IterableApi.sharedInstance = newApiWithInlineRequests(mock(IterableInAppManager.class)); // Use the new dedicated method for resetting background initialization state IterableBackgroundInitializer.resetBackgroundInitializationState(); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.java deleted file mode 100644 index a8175a886..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.java +++ /dev/null @@ -1,74 +0,0 @@ -package com.iterable.iterableapi; - -import com.iterable.iterableapi.unit.TestRunner; - -import org.json.JSONObject; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -@RunWith(TestRunner.class) -public class OfflineRequestProcessorTest extends BaseTest { - private OfflineRequestProcessor offlineRequestProcessor; - private IterableTaskRunner mockTaskRunner; - private TaskScheduler mockTaskScheduler; - private IterableTaskStorage mockTaskStorage; - private HealthMonitor mockHealthMonitor; - - @Before - public void setUp() { - mockTaskRunner = mock(IterableTaskRunner.class); - mockTaskScheduler = mock(TaskScheduler.class); - mockTaskStorage = mock(IterableTaskStorage.class); - mockHealthMonitor = mock(HealthMonitor.class); - offlineRequestProcessor = new OfflineRequestProcessor(mockTaskScheduler, mockTaskRunner, mockTaskStorage, mockHealthMonitor); - } - - @Test - public void testOfflineRequestIsStored() { - IterableApiRequest request = new IterableApiRequest("apiKey", IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, new JSONObject(), "POST", null, null, null); - when(mockHealthMonitor.canSchedule()).thenReturn(true); - offlineRequestProcessor.processPostRequest(request.apiKey, request.resourcePath, request.json, request.authToken, request.successCallback, request.failureCallback); - verify(mockTaskScheduler).scheduleTask(any(IterableApiRequest.class), isNull(), isNull()); - } - - @Test - public void testNonOfflineRequestIsNotStored() { - IterableApiRequest request = new IterableApiRequest("apiKey", IterableConstants.ENDPOINT_UPDATE_EMAIL, new JSONObject(), "POST", null, null, null); - offlineRequestProcessor.processPostRequest(request.apiKey, request.resourcePath, request.json, request.authToken, request.successCallback, request.failureCallback); - verifyNoInteractions(mockTaskScheduler); - } - - @Test - public void testOnlineRequestWhenDBError() { - IterableApiRequest request = new IterableApiRequest("apiKey", IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, new JSONObject(), "POST", null, null, null); - when(mockHealthMonitor.canSchedule()).thenReturn(false); - offlineRequestProcessor.processPostRequest(request.apiKey, request.resourcePath, request.json, request.authToken, request.successCallback, request.failureCallback); - verifyNoInteractions(mockTaskScheduler); - } - - @Test - public void testAllOfflineApisUseTaskScheduler() { - String[] offlineApis = new String[]{ - IterableConstants.ENDPOINT_TRACK, - IterableConstants.ENDPOINT_TRACK_PUSH_OPEN, - IterableConstants.ENDPOINT_TRACK_PURCHASE, - IterableConstants.ENDPOINT_TRACK_INAPP_OPEN, - IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, - IterableConstants.ENDPOINT_TRACK_INAPP_CLOSE, - IterableConstants.ENDPOINT_TRACK_INBOX_SESSION, - IterableConstants.ENDPOINT_TRACK_INAPP_DELIVERY, - IterableConstants.ENDPOINT_INAPP_CONSUME}; - for (String uri : offlineApis) { - assertTrue(offlineRequestProcessor.isRequestOfflineCompatible(uri)); - } - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt new file mode 100644 index 000000000..95652534b --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt @@ -0,0 +1,132 @@ +package com.iterable.iterableapi + +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatchers.isNull +import org.mockito.Mockito.mock +import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoInteractions +import org.mockito.Mockito.`when` + +class OfflineRequestProcessorTest : BaseTest() { + private lateinit var requestProcessor: OfflineRequestProcessor + private lateinit var taskScheduler: TaskScheduler + private lateinit var healthMonitor: HealthMonitor + private lateinit var requestDispatcher: IterableRequestDispatcher + + @Before + fun setUp() { + taskScheduler = mock(TaskScheduler::class.java) + healthMonitor = mock(HealthMonitor::class.java) + requestDispatcher = mock(IterableRequestDispatcher::class.java) + requestProcessor = OfflineRequestProcessor( + taskScheduler, + mock(IterableTaskRunner::class.java), + mock(IterableTaskStorage::class.java), + healthMonitor, + requestDispatcher + ) + } + + @Test + fun `offline compatible request is stored when storage is healthy`() { + `when`(healthMonitor.canSchedule()).thenReturn(true) + + requestProcessor.processPostRequest( + "api-key", + IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, + JSONObject(), + null, + null, + null + ) + + val requestCaptor = ArgumentCaptor.forClass(IterableApiRequest::class.java) + verify(taskScheduler).scheduleTask( + requestCaptor.capture(), + isNull(), + isNull() + ) + assertEquals( + IterableApiRequest.ProcessorType.OFFLINE, + requestCaptor.value.processorType + ) + verifyNoInteractions(requestDispatcher) + } + + @Test + fun `request that cannot be stored is dispatched immediately`() { + `when`(healthMonitor.canSchedule()).thenReturn(false) + val success = mock(IterableHelper.SuccessHandler::class.java) + val failure = mock(IterableHelper.FailureHandler::class.java) + + requestProcessor.processPostRequest( + "api-key", + IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, + JSONObject(), + "auth-token", + success, + failure + ) + + val request = captureDispatchedRequest() + assertEquals( + IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, + request.resourcePath + ) + assertSame(success, request.successCallback) + assertSame(failure, request.failureCallback) + verifyNoInteractions(taskScheduler) + } + + @Test + fun `request not supported offline is dispatched immediately`() { + requestProcessor.processPostRequest( + "api-key", + IterableConstants.ENDPOINT_UPDATE_EMAIL, + JSONObject(), + null, + null, + null + ) + + assertEquals( + IterableConstants.ENDPOINT_UPDATE_EMAIL, + captureDispatchedRequest().resourcePath + ) + verifyNoInteractions(taskScheduler) + } + + @Test + fun `all supported offline endpoints use task storage`() { + val supportedEndpoints = listOf( + IterableConstants.ENDPOINT_TRACK, + IterableConstants.ENDPOINT_TRACK_PUSH_OPEN, + IterableConstants.ENDPOINT_TRACK_PURCHASE, + IterableConstants.ENDPOINT_TRACK_INAPP_OPEN, + IterableConstants.ENDPOINT_TRACK_INAPP_CLICK, + IterableConstants.ENDPOINT_TRACK_INAPP_CLOSE, + IterableConstants.ENDPOINT_TRACK_INBOX_SESSION, + IterableConstants.ENDPOINT_TRACK_INAPP_DELIVERY, + IterableConstants.ENDPOINT_INAPP_CONSUME, + IterableConstants.ENDPOINT_UPDATE_CART, + IterableConstants.ENDPOINT_TRACK_EMBEDDED_RECEIVED, + IterableConstants.ENDPOINT_TRACK_EMBEDDED_CLICK, + IterableConstants.ENDPOINT_TRACK_EMBEDDED_SESSION + ) + + supportedEndpoints.forEach { + assertEquals(true, requestProcessor.isRequestOfflineCompatible(it)) + } + } + + private fun captureDispatchedRequest(): IterableApiRequest { + val requestCaptor = ArgumentCaptor.forClass(IterableApiRequest::class.java) + verify(requestDispatcher).execute(requestCaptor.capture()) + return requestCaptor.value + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.java deleted file mode 100644 index c79956909..000000000 --- a/iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.java +++ /dev/null @@ -1,55 +0,0 @@ -package com.iterable.iterableapi; - -import com.iterable.iterableapi.unit.TestRunner; - -import org.json.JSONObject; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -@RunWith(TestRunner.class) -public class TaskSchedulerTest { - private IterableTaskStorage mockTaskStorage; - private IterableTaskRunner mockTaskRunner; - private TaskScheduler taskScheduler; - - @Before - public void setUp() throws Exception { - mockTaskStorage = mock(IterableTaskStorage.class); - mockTaskRunner = mock(IterableTaskRunner.class); - taskScheduler = new TaskScheduler(mockTaskStorage, mockTaskRunner); - } - - @Test - public void testScheduleTaskCreatesTaskInStorage() throws Exception { - IterableApiRequest request = new IterableApiRequest("apiKey", "api/test", new JSONObject(), "POST", null, null, null); - taskScheduler.scheduleTask(request, null, null); - verify(mockTaskStorage).createTask(eq("api/test"), eq(IterableTaskType.API), eq(request.toJSONObject().toString())); - } - - @Test - public void testSuccessCallbackIsCalledOnCompletion() throws Exception { - IterableHelper.SuccessHandler successHandler = mock(IterableHelper.SuccessHandler.class); - IterableApiRequest request = new IterableApiRequest("apiKey", "api/test", new JSONObject(), "POST", null, null, null); - when(mockTaskStorage.createTask(any(String.class), any(IterableTaskType.class), any(String.class))).thenReturn("testTaskId"); - taskScheduler.scheduleTask(request, successHandler, null); - taskScheduler.onTaskCompleted("testTaskId", IterableTaskRunner.TaskResult.SUCCESS, IterableApiResponse.success(200, "", new JSONObject())); - verify(successHandler).onSuccess(any(JSONObject.class)); - } - - @Test - public void testFailureCallbackIsCalledOnCompletion() throws Exception { - IterableHelper.FailureHandler failureHandler = mock(IterableHelper.FailureHandler.class); - IterableApiRequest request = new IterableApiRequest("apiKey", "api/test", new JSONObject(), "POST", null, null, null); - when(mockTaskStorage.createTask(any(String.class), any(IterableTaskType.class), any(String.class))).thenReturn("testTaskId"); - taskScheduler.scheduleTask(request, null, failureHandler); - taskScheduler.onTaskCompleted("testTaskId", IterableTaskRunner.TaskResult.FAILURE, IterableApiResponse.failure(400, "", new JSONObject(), "TestError")); - verify(failureHandler).onFailure(eq("TestError"), any(JSONObject.class)); - } -} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.kt new file mode 100644 index 000000000..7688f0bd0 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/TaskSchedulerTest.kt @@ -0,0 +1,131 @@ +package com.iterable.iterableapi + +import com.iterable.iterableapi.unit.TestRunner +import org.json.JSONObject +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.eq +import org.mockito.Mockito.mock +import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` + +@RunWith(TestRunner::class) +class TaskSchedulerTest { + private lateinit var taskStorage: IterableTaskStorage + private lateinit var taskRunner: IterableTaskRunner + private lateinit var requestDispatcher: IterableRequestDispatcher + private lateinit var taskScheduler: TaskScheduler + + @Before + fun setUp() { + TaskScheduler.successCallbackMap.clear() + TaskScheduler.failureCallbackMap.clear() + taskStorage = mock(IterableTaskStorage::class.java) + taskRunner = mock(IterableTaskRunner::class.java) + requestDispatcher = mock(IterableRequestDispatcher::class.java) + taskScheduler = TaskScheduler( + taskStorage, + taskRunner, + requestDispatcher + ) + } + + @Test + fun `scheduling a request stores its serialized data`() { + val request = request() + + taskScheduler.scheduleTask(request, null, null) + + verify(taskStorage).createTask( + "api/test", + IterableTaskType.API, + request.toJSONObject().toString() + ) + } + + @Test + fun `storage failure dispatches the original request immediately`() { + val success = mock(IterableHelper.SuccessHandler::class.java) + val failure = mock(IterableHelper.FailureHandler::class.java) + val request = request(success, failure) + `when`( + taskStorage.createTask( + any(String::class.java), + any(IterableTaskType::class.java), + any(String::class.java) + ) + ).thenReturn(null) + + taskScheduler.scheduleTask(request, success, failure) + + verify(requestDispatcher).execute(request) + } + + @Test + fun `successful stored request calls its client success callback`() { + val success = mock(IterableHelper.SuccessHandler::class.java) + val request = request(success, null) + `when`( + taskStorage.createTask( + any(String::class.java), + any(IterableTaskType::class.java), + any(String::class.java) + ) + ).thenReturn("task-id") + taskScheduler.scheduleTask(request, success, null) + + val responseData = JSONObject() + taskScheduler.onTaskCompleted( + "task-id", + IterableTaskRunner.TaskResult.SUCCESS, + IterableApiResponse.success(200, "{}", responseData) + ) + + verify(success).onSuccess(responseData) + } + + @Test + fun `failed stored request calls its client failure callback`() { + val failure = mock(IterableHelper.FailureHandler::class.java) + val request = request(null, failure) + `when`( + taskStorage.createTask( + any(String::class.java), + any(IterableTaskType::class.java), + any(String::class.java) + ) + ).thenReturn("task-id") + taskScheduler.scheduleTask(request, null, failure) + + val responseData = JSONObject() + taskScheduler.onTaskCompleted( + "task-id", + IterableTaskRunner.TaskResult.FAILURE, + IterableApiResponse.failure( + 400, + """{"msg":"Bad request"}""", + responseData, + "Bad request" + ) + ) + + verify(failure).onFailure(eq("Bad request"), eq(responseData)) + } + + private fun request( + success: IterableHelper.SuccessHandler? = null, + failure: IterableHelper.FailureHandler? = null + ): IterableApiRequest { + return IterableApiRequest( + "api-key", + "api/test", + JSONObject(), + IterableApiRequest.POST, + null, + success, + failure + ) + } +} From 6353d6dbdc99838afc51dee40785bd934c2d1e69 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Wed, 23 Sep 2026 13:11:39 +0100 Subject: [PATCH 2/7] [SDK-707] Address request executor review feedback --- .../iterableapi/IterableApiResponseTest.java | 32 ++-- .../com/iterable/iterableapi/IterableApi.java | 3 - .../IterableRequestDispatcher.java | 10 ++ .../iterableapi/IterableRequestTask.java | 7 +- .../iterableapi/IterableTaskRunner.java | 143 +++++++++--------- .../iterableapi/OfflineRequestProcessor.java | 21 +-- .../iterableapi/OnlineRequestProcessor.java | 4 - .../com/iterable/iterableapi/BaseTest.java | 15 -- .../IterableApiGetAndTrackDeepLinkTest.kt | 2 - .../IterableApiIntegrationTest.java | 4 +- .../iterable/iterableapi/IterableApiTest.java | 3 +- .../iterableapi/IterableInAppManagerTest.java | 58 +++---- .../iterableapi/IterableRequestTaskTest.kt | 3 +- .../iterableapi/IterableTaskRunnerTest.java | 138 ++++++++++++++++- .../iterableapi/IterableTestUtils.java | 1 + 15 files changed, 275 insertions(+), 169 deletions(-) diff --git a/iterableapi/src/androidTest/java/com/iterable/iterableapi/IterableApiResponseTest.java b/iterableapi/src/androidTest/java/com/iterable/iterableapi/IterableApiResponseTest.java index afab7676b..c835cf41e 100644 --- a/iterableapi/src/androidTest/java/com/iterable/iterableapi/IterableApiResponseTest.java +++ b/iterableapi/src/androidTest/java/com/iterable/iterableapi/IterableApiResponseTest.java @@ -84,7 +84,7 @@ public void onSuccess(@NonNull JSONObject data) { signal.countDown(); } }, null); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onSuccess is called", signal.await(1, TimeUnit.SECONDS)); @@ -103,7 +103,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(1, TimeUnit.SECONDS)); @@ -122,7 +122,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(1, TimeUnit.SECONDS)); @@ -141,7 +141,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(5, TimeUnit.SECONDS)); @@ -162,7 +162,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(1, TimeUnit.SECONDS)); @@ -181,7 +181,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(1, TimeUnit.SECONDS)); @@ -200,7 +200,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(1, TimeUnit.SECONDS)); @@ -222,7 +222,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { "}"); stubAnyRequestReturningStatusCode(200, responseData); - new IterableRequestTask().execute(new IterableApiRequest("fake_key", "", new JSONObject(), IterableApiRequest.POST, null, new IterableHelper.SuccessHandler() { + dispatchRequest(new IterableApiRequest("fake_key", "", new JSONObject(), IterableApiRequest.POST, null, new IterableHelper.SuccessHandler() { @Override public void onSuccess(@NonNull JSONObject successData) { try { @@ -246,7 +246,7 @@ public void onSuccess(@NonNull JSONObject successData) { } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(5, TimeUnit.SECONDS); // Await for the background tasks to complete @@ -260,8 +260,7 @@ public void testMaxRetriesOnMultipleInvalidJwtPayloads() throws Exception { } IterableApiRequest request = new IterableApiRequest("fake_key", "", new JSONObject(), IterableApiRequest.POST, null, null, null); - IterableRequestTask task = new IterableRequestTask(); - task.execute(request); + dispatchRequest(request); RecordedRequest request1 = server.takeRequest(5, TimeUnit.SECONDS); RecordedRequest request2 = server.takeRequest(5, TimeUnit.SECONDS); @@ -279,8 +278,7 @@ public void testResponseCode500() throws Exception { } IterableApiRequest request = new IterableApiRequest("fake_key", "", new JSONObject(), IterableApiRequest.POST, null, null, null); - IterableRequestTask task = new IterableRequestTask(); - task.execute(request); + dispatchRequest(request); RecordedRequest request1 = server.takeRequest(1, TimeUnit.SECONDS); RecordedRequest request2 = server.takeRequest(5, TimeUnit.SECONDS); @@ -300,7 +298,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(1, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(5, TimeUnit.SECONDS)); @@ -319,9 +317,13 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) { signal.countDown(); } }); - new IterableRequestTask().execute(request); + dispatchRequest(request); server.takeRequest(1, TimeUnit.SECONDS); assertTrue("onFailure is called", signal.await(1, TimeUnit.SECONDS)); } + + private void dispatchRequest(IterableApiRequest request) { + IterableRequestDispatcher.online().execute(request); + } } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index 049d8593a..458c49b04 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -937,7 +937,6 @@ public static boolean isSDKInitialized() { return initializationRun && backgroundInitComplete && sdkConfigured; } - /** * Register a callback to be notified when SDK initialization completes. * If the SDK is already initialized, the callback is invoked immediately. @@ -998,7 +997,6 @@ void setRequestDispatcher(IterableRequestDispatcher requestDispatcher) { IterableRequestDispatchers.same(Objects.requireNonNull(requestDispatcher)) ); } - //endregion //region SDK public functions @@ -1573,7 +1571,6 @@ public void trackPurchase(double total, @NonNull List items, @Null queueOrExecute(() -> trackPurchase(total, items, dataFields, null), "trackPurchase(" + total + ", " + items.size() + " items, dataFields)"); } - /** * Tracks a purchase. * @param total total purchase amount diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java index 46326344b..c92eda58d 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java @@ -10,6 +10,10 @@ interface RetryScheduler { void schedule(Runnable runnable, long delayMs); } + interface ResponseHandler { + void onResponse(IterableApiResponse response); + } + private static final RetryScheduler SDK_RETRY_SCHEDULER = (runnable, delayMs) -> new Handler(Looper.getMainLooper()).postDelayed(runnable, delayMs); @@ -50,6 +54,12 @@ void execute(IterableApiRequest request) { requestExecutor.execute(new IterableRequestTask(request, 0, this)); } + void executeForResponse(IterableApiRequest request, ResponseHandler responseHandler) { + requestExecutor.execute(() -> responseHandler.onResponse( + IterableRequestTask.executeApiRequest(request, this) + )); + } + void executeRetry(IterableApiRequest request, int retryCount) { if (request.canRetry()) { requestExecutor.execute(new IterableRequestTask(request, retryCount, this, true)); diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java index b345220e0..c49590b55 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java @@ -98,17 +98,12 @@ static void retryRequestWithNewAuthToken( requestDispatcher.executeRetry(request, 0); } - @WorkerThread - static IterableApiResponse executeApiRequest(IterableApiRequest iterableApiRequest) { - return executeApiRequest(iterableApiRequest, IterableRequestDispatcher.online()); - } - /** * Sends the given request to Iterable using a HttpURLConnection. * Reference - http://developer.android.com/reference/java/net/HttpURLConnection.html */ @WorkerThread - private static IterableApiResponse executeApiRequest( + static IterableApiResponse executeApiRequest( IterableApiRequest iterableApiRequest, IterableRequestDispatcher requestDispatcher ) { diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java index 90b2d2dc2..e9deafb35 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java @@ -21,6 +21,7 @@ class IterableTaskRunner implements IterableTaskStorage.TaskCreatedListener, Han private IterableNetworkConnectivityManager networkConnectivityManager; private HealthMonitor healthMonitor; private ApiEndpointClassification classification; + private final IterableRequestDispatcher requestDispatcher; private static final int RETRY_INTERVAL_SECONDS = 60; @@ -28,6 +29,7 @@ class IterableTaskRunner implements IterableTaskStorage.TaskCreatedListener, Han private final HandlerThread networkThread = new HandlerThread("NetworkThread"); Handler handler; + private boolean requestInFlight; enum TaskResult { SUCCESS, FAILURE, RETRY @@ -47,12 +49,14 @@ interface TaskCompletedListener { IterableActivityMonitor activityMonitor, IterableNetworkConnectivityManager networkConnectivityManager, HealthMonitor healthMonitor, - ApiEndpointClassification classification) { + ApiEndpointClassification classification, + IterableRequestDispatcher requestDispatcher) { this.taskStorage = taskStorage; this.activityMonitor = activityMonitor; this.networkConnectivityManager = networkConnectivityManager; this.healthMonitor = healthMonitor; this.classification = classification; + this.requestDispatcher = requestDispatcher; networkThread.start(); handler = new Handler(networkThread.getLooper(), this); taskStorage.addTaskCreatedListener(this); @@ -60,14 +64,6 @@ interface TaskCompletedListener { activityMonitor.addCallback(this); } - // Preserved for backward compatibility with existing tests - IterableTaskRunner(IterableTaskStorage taskStorage, - IterableActivityMonitor activityMonitor, - IterableNetworkConnectivityManager networkConnectivityManager, - HealthMonitor healthMonitor) { - this(taskStorage, activityMonitor, networkConnectivityManager, healthMonitor, new ApiEndpointClassification()); - } - void addTaskCompletedListener(TaskCompletedListener listener) { taskCompletedListeners.add(listener); } @@ -129,6 +125,10 @@ public boolean handleMessage(@NonNull Message msg) { @WorkerThread private void processTasks() { + if (requestInFlight) { + return; + } + if (!activityMonitor.isInForeground()) { IterableLogger.d(TAG, "App not in foreground, skipping processing tasks"); return; @@ -140,22 +140,13 @@ private void processTasks() { boolean autoRetry = IterableApi.getInstance().isAutoRetryOnJwtFailure(); - while (networkConnectivityManager.isConnected()) { - IterableTask task = getNextActionableTask(autoRetry); - - if (task == null) { - return; - } + if (!networkConnectivityManager.isConnected()) { + return; + } - boolean proceed = processTask(task, autoRetry); - if (!proceed) { - // Only schedule timed retry for non-auth failures. - // Auth failures will resume via onAuthTokenReady() callback. - if (!autoRetry || !isPausedForAuth) { - scheduleRetry(); - } - return; - } + IterableTask task = getNextActionableTask(autoRetry); + if (task != null) { + processTask(task, autoRetry); } } @@ -173,53 +164,69 @@ void setIsPausedForAuth(boolean paused) { } @WorkerThread - private boolean processTask(@NonNull IterableTask task, boolean autoRetry) { - if (task.taskType == IterableTaskType.API) { - IterableApiResponse response = null; - TaskResult result = TaskResult.FAILURE; - try { - // Use the current live auth token instead of the stale one stored in the DB. - // The token in the DB was captured at queue time and may have since expired. - String currentAuthToken = IterableApi.getInstance().getAuthToken(); - IterableApiRequest request = IterableApiRequest.fromJSON(getTaskDataWithDate(task), currentAuthToken, null, null); - request.setProcessorType(IterableApiRequest.ProcessorType.OFFLINE); - response = IterableRequestTask.executeApiRequest(request); - } catch (Exception e) { - IterableLogger.e(TAG, "Error while processing request task", e); - healthMonitor.onDBError(); - } + private void processTask(@NonNull IterableTask task, boolean autoRetry) { + if (task.taskType != IterableTaskType.API) { + return; + } - if (response != null) { - if (response.success) { - result = TaskResult.SUCCESS; - } else { - // If autoRetry is enabled and response is a 401 JWT error, - // retain the task and pause processing until a valid JWT is obtained. - if (autoRetry && isJwtFailure(response)) { - IterableLogger.d(TAG, "JWT auth failure on task " + task.id + ". Retaining task and pausing processing."); - IterableApi.getInstance().getAuthManager().setAuthTokenInvalid(); - isPausedForAuth = true; - callTaskCompletedListeners(task.id, TaskResult.RETRY, response); - return false; - } - - if (isPermanentFailure(response)) { - result = TaskResult.FAILURE; - } else { - result = TaskResult.RETRY; - } - } - } - callTaskCompletedListeners(task.id, result, response); - if (result == TaskResult.RETRY) { - // Keep the task, stop further processing - return false; - } else { - taskStorage.deleteTask(task.id); - return true; + try { + // Use the current live auth token instead of the stale one stored in the DB. + // The token in the DB was captured at queue time and may have since expired. + String currentAuthToken = IterableApi.getInstance().getAuthToken(); + IterableApiRequest request = IterableApiRequest.fromJSON( + getTaskDataWithDate(task), + currentAuthToken, + null, + null + ); + request.setProcessorType(IterableApiRequest.ProcessorType.OFFLINE); + requestInFlight = true; + requestDispatcher.executeForResponse( + request, + response -> handler.post(() -> + handleTaskResponse(task, autoRetry, response)) + ); + } catch (Exception e) { + IterableLogger.e(TAG, "Error while processing request task", e); + healthMonitor.onDBError(); + handleTaskResponse(task, autoRetry, null); + } + } + + @WorkerThread + private void handleTaskResponse( + @NonNull IterableTask task, + boolean autoRetry, + IterableApiResponse response + ) { + requestInFlight = false; + TaskResult result = TaskResult.FAILURE; + + if (response != null) { + if (response.success) { + result = TaskResult.SUCCESS; + } else if (autoRetry && isJwtFailure(response)) { + IterableLogger.d( + TAG, + "JWT auth failure on task " + task.id + + ". Retaining task and pausing processing." + ); + IterableApi.getInstance().getAuthManager().setAuthTokenInvalid(); + isPausedForAuth = true; + callTaskCompletedListeners(task.id, TaskResult.RETRY, response); + return; + } else if (!isPermanentFailure(response)) { + result = TaskResult.RETRY; } } - return false; + + callTaskCompletedListeners(task.id, result, response); + if (result == TaskResult.RETRY) { + scheduleRetry(); + } else { + taskStorage.deleteTask(task.id); + runNow(); + } } JSONObject getTaskDataWithDate(IterableTask task) { diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java index 5d97c0c97..7c896a9a0 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java @@ -37,10 +37,6 @@ class OfflineRequestProcessor implements RequestProcessor { IterableConstants.ENDPOINT_TRACK_EMBEDDED_SESSION )); - OfflineRequestProcessor(Context context) { - this(context, IterableRequestDispatcher.offline()); - } - OfflineRequestProcessor( Context context, IterableRequestDispatcher requestDispatcher @@ -54,7 +50,8 @@ class OfflineRequestProcessor implements RequestProcessor { IterableActivityMonitor.getInstance(), networkConnectivityManager, healthMonitor, - classification); + classification, + requestDispatcher); taskScheduler = new TaskScheduler(taskStorage, taskRunner, requestDispatcher); // Register task runner as auth token ready listener for JWT auto-retry support @@ -78,16 +75,6 @@ void dispose() { } } - OfflineRequestProcessor(TaskScheduler scheduler, IterableTaskRunner iterableTaskRunner, IterableTaskStorage storage, HealthMonitor mockHealthMonitor) { - this( - scheduler, - iterableTaskRunner, - storage, - mockHealthMonitor, - IterableRequestDispatcher.offline() - ); - } - OfflineRequestProcessor( TaskScheduler scheduler, IterableTaskRunner iterableTaskRunner, @@ -142,10 +129,6 @@ class TaskScheduler implements IterableTaskRunner.TaskCompletedListener { private final IterableTaskRunner taskRunner; private final IterableRequestDispatcher requestDispatcher; - TaskScheduler(IterableTaskStorage taskStorage, IterableTaskRunner taskRunner) { - this(taskStorage, taskRunner, IterableRequestDispatcher.offline()); - } - TaskScheduler( IterableTaskStorage taskStorage, IterableTaskRunner taskRunner, diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java index 4b82ded93..16bccb425 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/OnlineRequestProcessor.java @@ -15,10 +15,6 @@ class OnlineRequestProcessor implements RequestProcessor { private static final String TAG = "OnlineRequestProcessor"; private final IterableRequestDispatcher requestDispatcher; - OnlineRequestProcessor() { - this(IterableRequestDispatcher.online()); - } - OnlineRequestProcessor(IterableRequestDispatcher requestDispatcher) { this.requestDispatcher = requestDispatcher; } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java index 4c6b22122..cd52c6fc3 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java @@ -9,11 +9,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; -import org.junit.rules.TestWatcher; -import org.junit.runner.Description; import org.junit.runner.RunWith; -import org.robolectric.android.util.concurrent.InlineExecutorService; -import org.robolectric.shadows.ShadowPausedAsyncTask; @RunWith(TestRunner.class) public abstract class BaseTest { @@ -21,9 +17,6 @@ public abstract class BaseTest { @Rule public IterableUtilRule utilsRule = new IterableUtilRule(); - @Rule - public AsyncTaskRule asyncTaskRule = new AsyncTaskRule(); - @Before public void baseTestSetUp() { IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(); @@ -44,12 +37,4 @@ protected IterableUtilImpl getIterableUtilSpy() { protected Context getContext() { return ApplicationProvider.getApplicationContext(); } - - private static class AsyncTaskRule extends TestWatcher { - @Override - protected void starting(Description description) { - ShadowPausedAsyncTask.overrideExecutor(new InlineExecutorService()); - } - } - } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiGetAndTrackDeepLinkTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiGetAndTrackDeepLinkTest.kt index a3c5deb6f..72228e8d5 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiGetAndTrackDeepLinkTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiGetAndTrackDeepLinkTest.kt @@ -18,7 +18,6 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.robolectric.Shadows.shadowOf -import org.robolectric.shadows.ShadowPausedAsyncTask import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit @@ -399,7 +398,6 @@ class IterableApiGetAndTrackDeepLinkTest : BaseTest() { @Suppress("DEPRECATION") private fun blockHostAsyncTaskQueue() { - ShadowPausedAsyncTask.reset() val hostTaskStarted = CountDownLatch(1) val releaseTask = CountDownLatch(1) val taskFinished = CountDownLatch(1) diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java index 2e5c12a9c..cf20cc215 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiIntegrationTest.java @@ -8,7 +8,6 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.robolectric.shadows.ShadowPausedAsyncTask; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; @@ -41,12 +40,11 @@ public void setUp() { server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); IterableInAppManager inAppManagerMock = mock(IterableInAppManager.class); - IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManagerMock); + IterableApi.sharedInstance = new IterableApi(inAppManagerMock); originalPushRegistrationUtil = IterablePushRegistrationTask.Util.instance; pushRegistrationUtilMock = mock(IterablePushRegistrationTask.Util.UtilImpl.class); IterablePushRegistrationTask.Util.instance = pushRegistrationUtilMock; - ShadowPausedAsyncTask.reset(); // Enable real threading in AsyncTask so we keep the execution sequence similar to the real one. } @After diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java index bab548907..3c8258a57 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiTest.java @@ -273,7 +273,8 @@ public void testSetEmailWithoutAutomaticPushRegistration() throws Exception { @Test public void testSetUserIdWithAutomaticPushRegistration() throws Exception { IterableApi.initialize(getContext(), "fake_key", new IterableConfig.Builder().setPushIntegrationName("pushIntegration").setAutoPushRegistration(true).build()); - // Reset after initialize since it may trigger push registration via background init + // Flush any pending looper callbacks from initialize, then reset mock + shadowOf(getMainLooper()).idle(); Mockito.reset(pushRegistrationExecutor); // Check that setUserId calls registerForPush diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java index e631d0b4a..005fe7f2c 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java @@ -47,6 +47,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; import static org.mockito.ArgumentMatchers.argThat; import static org.junit.Assert.assertNotNull; @@ -708,31 +709,32 @@ public void testJsonOnlyInAppMessageDelegateCallbacks() throws Exception { dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(payload.toString())); - // Create InAppManager with mock handler final IterableInAppHandler inAppHandler = mock(IterableInAppHandler.class); + when(inAppHandler.onNewInApp(any())) + .thenReturn(IterableInAppHandler.InAppResponse.SHOW); + IterableActivityMonitor activityMonitor = mock(IterableActivityMonitor.class); + when(activityMonitor.isInForeground()).thenReturn(false); IterableInAppManager inAppManager = spy(new IterableInAppManager( IterableApi.sharedInstance, inAppHandler, - 30.0, + 0.0, new IterableInAppMemoryStorage(), - IterableActivityMonitor.getInstance(), + activityMonitor, mock(IterableInAppDisplayer.class))); IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); - // Flush constructor sync callback so messages are loaded - shadowOf(getMainLooper()).idle(); - - // Process messages by bringing app to foreground - ActivityController activityController = Robolectric.buildActivity(Activity.class).create().start().resume(); + // Complete the constructor sync while the app is in the background. shadowOf(getMainLooper()).idle(); + assertEquals(2, inAppManager.getMessages().size()); - // Verify immediate trigger message was processed + // The production processing pass handles only the immediate message. + when(activityMonitor.isInForeground()).thenReturn(true); + inAppManager.scheduleProcessing(); ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(IterableInAppMessage.class); verify(inAppHandler).onNewInApp(messageCaptor.capture()); assertEquals("immediate", messageCaptor.getValue().getCustomPayload().getString("key")); assertEquals("message1", messageCaptor.getValue().getMessageId()); - // Verify never trigger message was not processed verify(inAppHandler, never()).onNewInApp(argThat(msg -> msg.getMessageId().equals("message2"))); } @@ -860,39 +862,38 @@ public void testJsonOnlyInAppMessageProcessingAndDisplay() throws Exception { dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(payload.toString())); - // Create InAppManager with mock handler and displayer IterableInAppDisplayer mockDisplayer = mock(IterableInAppDisplayer.class); final IterableInAppHandler inAppHandler = mock(IterableInAppHandler.class); + when(inAppHandler.onNewInApp(any())) + .thenReturn(IterableInAppHandler.InAppResponse.SHOW); + IterableActivityMonitor activityMonitor = mock(IterableActivityMonitor.class); + when(activityMonitor.isInForeground()).thenReturn(false); IterableInAppManager inAppManager = spy(new IterableInAppManager( IterableApi.sharedInstance, inAppHandler, - 30.0, + 0.0, new IterableInAppMemoryStorage(), - IterableActivityMonitor.getInstance(), + activityMonitor, mockDisplayer)); IterableApi.sharedInstance = IterableTestUtils.newApiWithInlineRequests(inAppManager); - // First sync to get messages - inAppManager.syncInApp(); + // Complete the constructor sync before processing the loaded message. shadowOf(getMainLooper()).idle(); + assertEquals(1, inAppManager.getMessages().size()); - // Process messages by bringing app to foreground - Robolectric.buildActivity(Activity.class).create().start().resume(); - shadowOf(getMainLooper()).idle(); + when(activityMonitor.isInForeground()).thenReturn(true); + inAppManager.scheduleProcessing(); - // Verify handler was called with correct message ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(IterableInAppMessage.class); verify(inAppHandler).onNewInApp(messageCaptor.capture()); assertEquals("value", messageCaptor.getValue().getCustomPayload().getString("key")); - // Verify displayer was never called verify(mockDisplayer, never()).showMessage( any(IterableInAppMessage.class), any(IterableInAppLocation.class), any(IterableHelper.IterableUrlCallback.class)); - // Verify message was consumed (not in queue) assertEquals(0, inAppManager.getMessages().size()); } @@ -936,21 +937,24 @@ public void testJsonOnlyMessageConsume() throws Exception { dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(payload.toString())); - // Create InAppManager with spied IterableApi IterableApi spyApi = spy(IterableApi.sharedInstance); + IterableActivityMonitor activityMonitor = mock(IterableActivityMonitor.class); + when(activityMonitor.isInForeground()).thenReturn(false); IterableInAppManager inAppManager = new IterableInAppManager( spyApi, new IterableDefaultInAppHandler(), - 30.0, + 0.0, new IterableInAppMemoryStorage(), - IterableActivityMonitor.getInstance(), + activityMonitor, mock(IterableInAppDisplayer.class)); - // Process messages by bringing app to foreground - Robolectric.buildActivity(Activity.class).create().start().resume(); + // Complete the constructor sync before processing the loaded message. shadowOf(getMainLooper()).idle(); + assertEquals(1, inAppManager.getMessages().size()); + + when(activityMonitor.isInForeground()).thenReturn(true); + inAppManager.scheduleProcessing(); - // Verify inAppConsume was called with the correct parameters verify(spyApi).inAppConsume( argThat(message -> message.getMessageId().equals("message1")), eq(null), diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt index 57ac1fce7..20afdd097 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt @@ -12,6 +12,7 @@ import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyLong +import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.verify @@ -137,7 +138,7 @@ class IterableRequestTaskTest { private fun captureDispatchedRequest(): IterableApiRequest { val requestCaptor = ArgumentCaptor.forClass(IterableApiRequest::class.java) - verify(requestDispatcher).execute(requestCaptor.capture()) + verify(requestDispatcher).executeRetry(requestCaptor.capture(), eq(0)) return requestCaptor.value } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java index ff17da9bc..769659a5e 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java @@ -12,6 +12,9 @@ import org.junit.Test; import org.junit.runner.RunWith; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import okhttp3.mockwebserver.MockResponse; @@ -43,6 +46,7 @@ public class IterableTaskRunnerTest extends BaseTest { private IterableActivityMonitor mockActivityMonitor; private HealthMonitor mockHealthMonitor; private IterableNetworkConnectivityManager mockNetworkConnectivityManager; + private IterableRequestDispatcher requestDispatcher; private MockWebServer server; @Before @@ -51,7 +55,19 @@ public void setUp() throws Exception { mockActivityMonitor = mock(IterableActivityMonitor.class); mockNetworkConnectivityManager = mock(IterableNetworkConnectivityManager.class); mockHealthMonitor = mock(HealthMonitor.class); - taskRunner = new IterableTaskRunner(mockTaskStorage, mockActivityMonitor, mockNetworkConnectivityManager, mockHealthMonitor); + requestDispatcher = new IterableRequestDispatcher( + Runnable::run, + Runnable::run, + (runnable, delayMs) -> runnable.run() + ); + taskRunner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + new ApiEndpointClassification(), + requestDispatcher + ); server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); } @@ -172,6 +188,71 @@ public void testIfNetworkCheckedBeforeProcessingTask() throws Exception { verify(mockNetworkConnectivityManager, times(2)).isConnected(); } + @Test + public void testStoredAndImmediateRequestsUseTheSameQueueInSubmissionOrder() + throws Exception { + RecordingExecutor requestExecutor = new RecordingExecutor(); + IterableRequestDispatcher sharedDispatcher = new IterableRequestDispatcher( + requestExecutor, + runnable -> { }, + (runnable, delayMs) -> requestExecutor.execute(runnable) + ); + IterableTaskRunner runner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + new ApiEndpointClassification(), + sharedDispatcher + ); + IterableApiRequest storedRequest = new IterableApiRequest( + "apiKey", + "api/stored", + new JSONObject(), + IterableApiRequest.POST, + null, + null, + null + ); + IterableTask storedTask = new IterableTask( + "storedTask", + IterableTaskType.API, + storedRequest.toJSONObject().toString() + ); + when(mockTaskStorage.getNextScheduledTask()).thenReturn(storedTask).thenReturn(null); + when(mockActivityMonitor.isInForeground()).thenReturn(true); + when(mockNetworkConnectivityManager.isConnected()).thenReturn(true); + when(mockHealthMonitor.canProcess()).thenReturn(true); + server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); + server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); + + sharedDispatcher.execute(new IterableApiRequest( + "apiKey", + "api/immediate", + new JSONObject(), + IterableApiRequest.POST, + null, + null, + null + )); + runner.onTaskCreated(null); + runHandlerTasks(runner); + + assertEquals(2, requestExecutor.pendingTaskCount()); + requestExecutor.runAll(); + + RecordedRequest immediateRequest = server.takeRequest(1, TimeUnit.SECONDS); + assertNotNull(immediateRequest); + assertEquals("/api/immediate", immediateRequest.getPath()); + + RecordedRequest persistedRequest = server.takeRequest(1, TimeUnit.SECONDS); + assertNotNull(persistedRequest); + assertEquals("/api/stored", persistedRequest.getPath()); + + runHandlerTasks(runner); + verify(mockTaskStorage).deleteTask(storedTask.id); + } + // region Auto-Retry on JWT Failure Tests private String createJwt401ResponseBody() throws Exception { @@ -510,7 +591,14 @@ public void testAuthTokenReadyListener_NotifiedOnStateTransitionFromInvalid() { @Test public void testUnauthenticatedTaskExecutesDuringAuthPause() throws Exception { ApiEndpointClassification classification = new ApiEndpointClassification(); - IterableTaskRunner runner = new IterableTaskRunner(mockTaskStorage, mockActivityMonitor, mockNetworkConnectivityManager, mockHealthMonitor, classification); + IterableTaskRunner runner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + classification, + requestDispatcher + ); IterableApiRequest request = new IterableApiRequest("apiKey", IterableConstants.ENDPOINT_DISABLE_DEVICE, new JSONObject(), "POST", null, null, null); IterableTask unauthTask = new IterableTask(IterableConstants.ENDPOINT_DISABLE_DEVICE, IterableTaskType.API, request.toJSONObject().toString()); @@ -535,7 +623,14 @@ public void testUnauthenticatedTaskExecutesDuringAuthPause() throws Exception { @Test public void testAuthRequiredTaskStaysBlockedDuringAuthPause() throws Exception { ApiEndpointClassification classification = new ApiEndpointClassification(); - IterableTaskRunner runner = new IterableTaskRunner(mockTaskStorage, mockActivityMonitor, mockNetworkConnectivityManager, mockHealthMonitor, classification); + IterableTaskRunner runner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + classification, + requestDispatcher + ); when(mockTaskStorage.getNextScheduledTaskNotRequiringJwt(classification)).thenReturn(null); when(mockActivityMonitor.isInForeground()).thenReturn(true); @@ -554,7 +649,14 @@ public void testAuthRequiredTaskStaysBlockedDuringAuthPause() throws Exception { @Test public void testQueueIntegrityAfterAuthPausedProcessing() throws Exception { ApiEndpointClassification classification = new ApiEndpointClassification(); - IterableTaskRunner runner = new IterableTaskRunner(mockTaskStorage, mockActivityMonitor, mockNetworkConnectivityManager, mockHealthMonitor, classification); + IterableTaskRunner runner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + classification, + requestDispatcher + ); IterableApiRequest trackRequestA = new IterableApiRequest("apiKey", IterableConstants.ENDPOINT_TRACK, new JSONObject("{\"eventName\":\"A\"}"), "POST", null, null, null); IterableTask trackTaskA = new IterableTask(IterableConstants.ENDPOINT_TRACK, IterableTaskType.API, trackRequestA.toJSONObject().toString()); @@ -595,7 +697,14 @@ public void testQueueIntegrityAfterAuthPausedProcessing() throws Exception { @Test public void testAuthRequiredTasksResumeAfterAuthReady() throws Exception { ApiEndpointClassification classification = new ApiEndpointClassification(); - IterableTaskRunner runner = new IterableTaskRunner(mockTaskStorage, mockActivityMonitor, mockNetworkConnectivityManager, mockHealthMonitor, classification); + IterableTaskRunner runner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + classification, + requestDispatcher + ); IterableApiRequest trackRequest = new IterableApiRequest("apiKey", IterableConstants.ENDPOINT_TRACK, new JSONObject(), "POST", null, null, null); IterableTask trackTask = new IterableTask(IterableConstants.ENDPOINT_TRACK, IterableTaskType.API, trackRequest.toJSONObject().toString()); @@ -631,4 +740,23 @@ public void testAuthRequiredTasksResumeAfterAuthReady() throws Exception { private void runHandlerTasks(IterableTaskRunner taskRunner) throws InterruptedException { shadowOf(taskRunner.handler.getLooper()).idle(); } + + private static class RecordingExecutor implements Executor { + private final List tasks = new ArrayList<>(); + + @Override + public void execute(Runnable command) { + tasks.add(command); + } + + int pendingTaskCount() { + return tasks.size(); + } + + void runAll() { + while (!tasks.isEmpty()) { + tasks.remove(0).run(); + } + } + } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java index 9401bfa9c..07e17b74f 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java @@ -65,6 +65,7 @@ public static void createIterableApiNew(ConfigBuilderExtender extender, String e /** * Creates an API whose request work runs inline while callbacks and retries * still use the main looper, matching the production delivery contract. + * Tests asserting callback-driven follow-up work must idle the main looper. */ static IterableApi newApiWithInlineRequests() { IterableApi api = new IterableApi(); From 9d8a71a1521cbec7fff3ee655cb2be4f24667103 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Wed, 23 Sep 2026 13:51:57 +0100 Subject: [PATCH 3/7] [SDK-707] Cover concurrent request execution --- .../com/iterable/iterableapi/IterableApi.java | 6 +- .../IterableApiClientRequestRoutingTest.kt | 72 +++++++ ...IterableRequestExecutionConcurrencyTest.kt | 180 ++++++++++++++++++ 3 files changed, 254 insertions(+), 4 deletions(-) create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java index 458c49b04..1f6cb7e6b 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java @@ -992,10 +992,8 @@ static void initializeForPush(@Nullable Context context) { this.pushRegistration = Objects.requireNonNull(pushRegistration); } void setRequestDispatcher(IterableRequestDispatcher requestDispatcher) { - apiClient = new IterableApiClient( - new IterableApiAuthProvider(), - IterableRequestDispatchers.same(Objects.requireNonNull(requestDispatcher)) - ); + apiClient = new IterableApiClient(new IterableApiAuthProvider(), + IterableRequestDispatchers.same(Objects.requireNonNull(requestDispatcher))); } //endregion diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt new file mode 100644 index 000000000..7f49825a1 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt @@ -0,0 +1,72 @@ +package com.iterable.iterableapi + +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito.mock +import java.util.concurrent.Executor + +class IterableApiClientRequestRoutingTest { + private lateinit var onlineExecutor: RecordingExecutor + private lateinit var pushExecutor: RecordingExecutor + private lateinit var offlineExecutor: RecordingExecutor + private lateinit var client: IterableApiClient + + @Before + fun setUp() { + onlineExecutor = RecordingExecutor() + pushExecutor = RecordingExecutor() + offlineExecutor = RecordingExecutor() + client = IterableApiClient( + mock(IterableApiClient.AuthProvider::class.java), + IterableRequestDispatchers( + dispatcher(onlineExecutor), + dispatcher(pushExecutor), + dispatcher(offlineExecutor) + ) + ) + } + + @Test + fun `ordinary API request uses online lane`() { + client.sendPostRequest("api/ordinary", JSONObject()) + + assertEquals(1, onlineExecutor.tasks.size) + assertTrue(pushExecutor.tasks.isEmpty()) + assertTrue(offlineExecutor.tasks.isEmpty()) + } + + @Test + fun `push token request uses push lane`() { + client.disableToken( + "user@example.com", + null, + null, + "device-token", + null, + null + ) + + assertEquals(1, pushExecutor.tasks.size) + assertTrue(onlineExecutor.tasks.isEmpty()) + assertTrue(offlineExecutor.tasks.isEmpty()) + } + + private fun dispatcher(executor: Executor): IterableRequestDispatcher { + return IterableRequestDispatcher( + executor, + Runnable::run, + { runnable, _ -> runnable.run() } + ) + } + + private class RecordingExecutor : Executor { + val tasks = mutableListOf() + + override fun execute(command: Runnable) { + tasks.add(command) + } + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt new file mode 100644 index 000000000..9736b20e6 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt @@ -0,0 +1,180 @@ +package com.iterable.iterableapi + +import com.iterable.iterableapi.unit.TestRunner +import okhttp3.mockwebserver.Dispatcher +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +@RunWith(TestRunner::class) +class IterableRequestExecutionConcurrencyTest { + private lateinit var server: MockWebServer + + @Before + fun setUp() { + IterableApi.sharedInstance = IterableApi() + server = MockWebServer() + IterableApi.overrideURLEndpointPath(server.url("/").toString()) + } + + @After + fun tearDown() { + IterableRequestTask.overrideUrl = null + server.shutdown() + IterableApi.sharedInstance = IterableApi() + } + + @Test + fun `online requests can execute concurrently`() { + val slowRequestStarted = CountDownLatch(1) + val releaseFirstResponse = CountDownLatch(1) + val fastRequestStarted = CountDownLatch(1) + + server.dispatcher = blockingFirstRequestDispatcher( + slowRequestStarted, + releaseFirstResponse, + fastRequestStarted + ) + val slowRequestFinished = execute( + IterableRequestDispatcher.online(), + "api/first" + ) + + try { + assertCompleted(slowRequestStarted, "Slow online request did not reach the server") + + val fastRequestFinished = execute( + IterableRequestDispatcher.online(), + "api/second" + ) + + assertCompleted( + fastRequestStarted, + "Second online request waited for the first response", + ) + assertCompleted( + fastRequestFinished, + "Second online request did not finish while the first response was blocked", + ) + } finally { + releaseFirstResponse.countDown() + } + + assertCompleted( + slowRequestFinished, + "First online request did not finish after its response was released" + ) + } + + @Test + fun `slow online request does not block push request`() { + val onlineRequestStarted = CountDownLatch(1) + val releaseOnlineResponse = CountDownLatch(1) + val pushRequestStarted = CountDownLatch(1) + + server.dispatcher = blockingFirstRequestDispatcher( + onlineRequestStarted, + releaseOnlineResponse, + pushRequestStarted + ) + val onlineRequestFinished = execute( + IterableRequestDispatcher.online(), + "api/first" + ) + + try { + assertCompleted(onlineRequestStarted, "Online request did not reach the server") + + val pushRequestFinished = execute( + IterableRequestDispatcher.push(), + "api/second" + ) + + assertCompleted( + pushRequestStarted, + "Push request waited for the online response", + ) + assertCompleted( + pushRequestFinished, + "Push request did not finish while the online response was blocked", + ) + } finally { + releaseOnlineResponse.countDown() + } + + assertCompleted( + onlineRequestFinished, + "Online request did not finish after its response was released" + ) + } + + private fun blockingFirstRequestDispatcher( + firstRequestStarted: CountDownLatch, + releaseFirstResponse: CountDownLatch, + secondRequestStarted: CountDownLatch + ): Dispatcher { + return object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse { + return when (request.path?.substringBefore("?")) { + "/api/first" -> { + firstRequestStarted.countDown() + releaseFirstResponse.await() + successfulResponse() + } + + "/api/second" -> { + secondRequestStarted.countDown() + successfulResponse() + } + + else -> MockResponse().setResponseCode(404) + } + } + } + } + + private fun execute( + dispatcher: IterableRequestDispatcher, + resourcePath: String + ): CountDownLatch { + return CountDownLatch(1).also { requestFinished -> + dispatcher.executeForResponse(request(resourcePath)) { + requestFinished.countDown() + } + } + } + + private fun assertCompleted(latch: CountDownLatch, message: String) { + assertTrue(message, latch.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) + } + + private fun request(resourcePath: String): IterableApiRequest { + return IterableApiRequest( + "api-key", + resourcePath, + JSONObject(), + IterableApiRequest.GET, + null, + null, + null + ) + } + + private fun successfulResponse(): MockResponse { + return MockResponse() + .setResponseCode(200) + .setBody("{}") + } + + companion object { + private const val TIMEOUT_SECONDS = 5L + } +} From 2066495451518cda16d2c82b86f777bd8f918943 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Wed, 23 Sep 2026 14:06:44 +0100 Subject: [PATCH 4/7] [SDK-707] Stabilize callback-driven test setup --- .../IterableFirebaseMessagingServiceTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java index 478ac0c1f..b6425ed56 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableFirebaseMessagingServiceTest.java @@ -25,6 +25,7 @@ import okhttp3.mockwebserver.MockWebServer; +import static android.os.Looper.getMainLooper; import static com.iterable.iterableapi.IterableTestUtils.bundleToMap; import static junit.framework.Assert.assertEquals; import static junit.framework.TestCase.assertFalse; @@ -33,9 +34,11 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.robolectric.Shadows.shadowOf; public class IterableFirebaseMessagingServiceTest extends BaseTest { @@ -51,6 +54,7 @@ public class IterableFirebaseMessagingServiceTest extends BaseTest { public void setUp() throws Exception { IterableTestUtils.resetIterableApi(); IterableTestUtils.createIterableApiNew(); + shadowOf(getMainLooper()).idle(); server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); @@ -106,7 +110,7 @@ public void testOnMessageReceived() throws Exception { @Test public void testSilentPushInAppUpdated() throws Exception { IterableInAppManager inAppManagerSpy = spy(IterableApi.getInstance().getInAppManager()); - when(apiMock.getInAppManager()).thenReturn(inAppManagerSpy); + doReturn(inAppManagerSpy).when(apiMock).getInAppManager(); doNothing().when(inAppManagerSpy).syncInApp(); RemoteMessage.Builder builder = new RemoteMessage.Builder("1234@gcm.googleapis.com"); @@ -118,7 +122,7 @@ public void testSilentPushInAppUpdated() throws Exception { @Test public void testSilentPushInAppRemoved() throws Exception { IterableInAppManager inAppManagerSpy = spy(IterableApi.getInstance().getInAppManager()); - when(apiMock.getInAppManager()).thenReturn(inAppManagerSpy); + doReturn(inAppManagerSpy).when(apiMock).getInAppManager(); doNothing().when(inAppManagerSpy).syncInApp(); doNothing().when(inAppManagerSpy).removeMessage(any(String.class)); @@ -147,7 +151,7 @@ public void testIsGhostPushWithNotificationMessage() throws Exception { @Test public void testUpdateMessagesIsCalled() throws Exception { IterableEmbeddedManager embeddedManagerSpy = spy(IterableApi.getInstance().getEmbeddedManager()); - when(apiMock.getEmbeddedManager()).thenReturn(embeddedManagerSpy); + doReturn(embeddedManagerSpy).when(apiMock).getEmbeddedManager(); RemoteMessage.Builder builder = new RemoteMessage.Builder("1234@gcm.googleapis.com"); builder.setData(IterableTestUtils.getMapFromJsonResource("push_payload_embedded_update.json")); From 83dca2a5c4d2f098e5276c2e3f628094c98b992e Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Fri, 2 Oct 2026 11:43:28 +0100 Subject: [PATCH 5/7] [SDK-707] Consolidate push request dispatch --- ...rablePushRegistrationRequestProcessor.java | 249 ------------------ .../iterableapi/IterableRequestTask.java | 4 + ...t.java => OnlineRequestProcessorTest.java} | 16 +- 3 files changed, 12 insertions(+), 257 deletions(-) delete mode 100644 iterableapi/src/main/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessor.java rename iterableapi/src/test/java/com/iterable/iterableapi/{IterablePushRegistrationRequestProcessorTest.java => OnlineRequestProcessorTest.java} (69%) diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessor.java deleted file mode 100644 index 89b3906cc..000000000 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessor.java +++ /dev/null @@ -1,249 +0,0 @@ -package com.iterable.iterableapi; - -import static com.iterable.iterableapi.IterableConstants.ENDPOINT_DISABLE_DEVICE; - -import android.os.Handler; -import android.os.Looper; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; - -import org.json.JSONException; -import org.json.JSONObject; - -import java.util.Date; -import java.util.Objects; -import java.util.concurrent.Executor; - -final class IterablePushRegistrationRequestProcessor { - private static final String TAG = "IterablePushRegistrationRequestProcessor"; - - interface RetryScheduler { - void schedule(Runnable runnable, long delayMs); - } - - private static final RetryScheduler SDK_RETRY_SCHEDULER = - (runnable, delayMs) -> - new Handler(Looper.getMainLooper()).postDelayed(runnable, delayMs); - - private final Executor requestExecutor; - private final Executor callbackExecutor; - private final RetryScheduler retryScheduler; - - IterablePushRegistrationRequestProcessor() { - this( - IterableExecutors.push(), - IterableExecutors.main(), - SDK_RETRY_SCHEDULER - ); - } - - IterablePushRegistrationRequestProcessor( - Executor requestExecutor, - Executor callbackExecutor, - RetryScheduler retryScheduler - ) { - this.requestExecutor = requestExecutor; - this.callbackExecutor = callbackExecutor; - this.retryScheduler = retryScheduler; - } - - void processPostRequest( - @Nullable String apiKey, - @NonNull String resourcePath, - @NonNull JSONObject json, - @Nullable String authToken, - @Nullable IterableHelper.SuccessHandler onSuccess, - @Nullable IterableHelper.FailureHandler onFailure, - @NonNull PushRegistrationRetryState retryState - ) { - IterableApiRequest request = new IterableApiRequest( - apiKey, - resourcePath, - addCreatedAtToJson(json), - IterableApiRequest.POST, - authToken, - onSuccess, - onFailure - ); - execute(request, retryState, 0, false); - } - - void scheduleRetry( - IterableApiRequest request, - @NonNull PushRegistrationRetryState retryState, - int retryCount, - long delayMs - ) { - retryScheduler.schedule( - () -> execute(request, retryState, retryCount, true), - delayMs - ); - } - - void retryWithNewAuthToken( - String newAuthToken, - IterableApiRequest request, - @NonNull PushRegistrationRetryState retryState - ) { - IterableApiRequest retryRequest = new IterableApiRequest( - request.apiKey, - request.resourcePath, - request.json, - request.requestType, - newAuthToken, - request.legacyCallback - ); - execute(retryRequest, retryState, 0, true); - } - - void deliverResult(Runnable runnable) { - callbackExecutor.execute(runnable); - } - - private void execute( - IterableApiRequest request, - @NonNull PushRegistrationRetryState retryState, - int retryCount, - boolean retry - ) { - requestExecutor.execute(new IterablePushRegistrationRequestTask( - request, - retryState, - retryCount, - retry, - this - )); - } - - private JSONObject addCreatedAtToJson(JSONObject json) { - try { - long createdAt; - if (json.has(IterableConstants.KEY_CREATED_AT)) { - createdAt = Long.parseLong( - json.getString(IterableConstants.KEY_CREATED_AT) - ); - } else { - createdAt = new Date().getTime() / 1000; - } - json.put(IterableConstants.KEY_CREATED_AT, createdAt); - } catch (JSONException | NumberFormatException e) { - IterableLogger.e( - TAG, - "Could not add createdAt timestamp to json object" - ); - } - return json; - } -} - -final class IterablePushRegistrationRequestTask implements Runnable { - private final IterableApiRequest request; - private final PushRegistrationRetryState retryState; - private final int retryCount; - private final boolean retry; - private final IterablePushRegistrationRequestProcessor requestProcessor; - - IterablePushRegistrationRequestTask( - IterableApiRequest request, - PushRegistrationRetryState retryState, - int retryCount, - boolean retry, - IterablePushRegistrationRequestProcessor requestProcessor - ) { - this.request = request; - this.retryState = retryState; - this.retryCount = retryCount; - this.retry = retry; - this.requestProcessor = requestProcessor; - } - - @Override - public void run() { - if (retry && !retryState.canRetry()) { - return; - } - - IterableApiResponse response = IterableRequestTask.executeApiRequest( - request, - (newAuthToken, originalRequest) -> - requestProcessor.retryWithNewAuthToken( - newAuthToken, - originalRequest, - retryState - ) - ); - requestProcessor.deliverResult(() -> handleResponse(response)); - } - - void handleResponse(IterableApiResponse response) { - if (response == null || (retry && !retryState.canRetry())) { - return; - } - - if (shouldRetry(response)) { - int nextRetryCount = retryCount + 1; - long delayMs = retryCount > 2 - ? IterableRequestTask.RETRY_DELAY_MS * retryCount - : 0; - requestProcessor.scheduleRetry( - request, - retryState, - nextRetryCount, - delayMs - ); - return; - } - - if (response.success) { - handleSuccess(response); - } else { - handleFailure(response); - } - - if (request.legacyCallback != null) { - request.legacyCallback.execute(response.responseBody); - } - } - - private boolean shouldRetry(IterableApiResponse response) { - return retryState.canRetry() - && !response.success - && response.responseCode >= 500 - && retryCount <= IterableRequestTask.MAX_RETRY_COUNT; - } - - private void handleSuccess(IterableApiResponse response) { - if (!Objects.equals(request.resourcePath, ENDPOINT_DISABLE_DEVICE)) { - IterableApi.getInstance().getAuthManager().resetFailedAuth(); - IterableApi.getInstance().getAuthManager().pauseAuthRetries(false); - IterableApi.getInstance().getAuthManager().setIsLastAuthTokenValid(true); - } - - if (request.successCallback != null) { - request.successCallback.onSuccess(response.responseJson); - } - } - - private void handleFailure(IterableApiResponse response) { - if (request.failureCallback == null) { - return; - } - - JSONObject responseJson = response.responseJson; - if (responseJson != null) { - try { - responseJson.put( - IterableConstants.HTTP_STATUS_CODE, - response.responseCode - ); - } catch (JSONException ignored) { - } - } - request.failureCallback.onFailure(response.errorMessage, responseJson); - } -} - -interface PushRegistrationRetryState { - boolean canRetry(); -} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java index c49590b55..f49757606 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java @@ -603,6 +603,10 @@ static IterableApiRequest fromJSON(JSONObject jsonData, @Nullable String authTok } } +interface IterableRequestRetryState { + boolean canRetry(); +} + class IterableApiResponse { final boolean success; final int responseCode; diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessorTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/OnlineRequestProcessorTest.java similarity index 69% rename from iterableapi/src/test/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessorTest.java rename to iterableapi/src/test/java/com/iterable/iterableapi/OnlineRequestProcessorTest.java index 7a83f6374..b765eb83e 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterablePushRegistrationRequestProcessorTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/OnlineRequestProcessorTest.java @@ -8,18 +8,18 @@ import java.util.concurrent.atomic.AtomicReference; -public class IterablePushRegistrationRequestProcessorTest extends BaseTest { +public class OnlineRequestProcessorTest extends BaseTest { @Test public void testMalformedCreatedAtDoesNotPreventPushRequestSubmission() throws JSONException { AtomicReference submittedRequest = new AtomicReference<>(); - IterablePushRegistrationRequestProcessor processor = - new IterablePushRegistrationRequestProcessor( - submittedRequest::set, - Runnable::run, - (runnable, delayMs) -> { - } - ); + IterableRequestDispatcher dispatcher = new IterableRequestDispatcher( + submittedRequest::set, + Runnable::run, + (runnable, delayMs) -> { + } + ); + OnlineRequestProcessor processor = new OnlineRequestProcessor(dispatcher); JSONObject requestJson = new JSONObject() .put(IterableConstants.KEY_CREATED_AT, "not-a-timestamp"); From b6d0d841013920d87721e72503fbe25c88475977 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Tue, 6 Oct 2026 09:54:15 +0100 Subject: [PATCH 6/7] [SDK-707] Preserve concurrent request behavior --- .../iterableapi/IterableApiClient.java | 1 + .../iterableapi/IterableExecutors.java | 45 ++++++--- .../IterableRequestDispatcher.java | 41 ++++++++- .../iterableapi/IterableTaskRunner.java | 54 ++++++++++- .../iterableapi/IterableTaskStorage.java | 58 +++++++++++- .../iterableapi/OfflineRequestProcessor.java | 26 ++++-- .../iterableapi/IterableExecutorsTest.java | 75 +++++++++++++++ .../IterableRequestDispatcherTest.kt | 63 ++++++++++++- .../iterableapi/IterableRequestTaskTest.kt | 16 ++++ .../iterableapi/IterableTaskRunnerTest.java | 91 ++----------------- .../iterableapi/IterableTaskStorageTest.java | 69 ++++++++++++++ .../OfflineRequestProcessorTest.kt | 10 +- 12 files changed, 426 insertions(+), 123 deletions(-) create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java create mode 100644 iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java index 3fdeed5af..81bdb5917 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java @@ -81,6 +81,7 @@ void setOfflineProcessingEnabled(boolean offlineMode) { this.requestProcessor = offlineMode ? new OfflineRequestProcessor( authProvider.getContext(), + requestDispatchers.online(), requestDispatchers.offline() ) : new OnlineRequestProcessor(requestDispatchers.online()); diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java index d3f412402..dbea1890c 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java @@ -2,14 +2,23 @@ import android.os.Handler; import android.os.Looper; +import android.os.Process; +import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; final class IterableExecutors { - private static final int REQUEST_THREAD_COUNT = - Math.max(2, Math.min(Runtime.getRuntime().availableProcessors(), 4)); + // HttpURLConnection is blocking I/O, so ordinary API work uses a bounded + // multi-thread pool rather than the serial executors used for ordered work. + static final int REQUEST_THREAD_COUNT = 8; + // Matches the historical AsyncTask queue bound without inheriting its + // platform-version-dependent thread-pool behavior. + static final int REQUEST_QUEUE_CAPACITY = 128; + private static final long REQUEST_THREAD_KEEP_ALIVE_SECONDS = 30; private static final AtomicInteger REQUEST_THREAD_ID = new AtomicInteger(); private static final Executor PUSH_EXECUTOR = newSingleThreadExecutor("IterablePushExecutor"); @@ -17,13 +26,8 @@ final class IterableExecutors { newSingleThreadExecutor("IterableDeepLinkExecutor"); private static final Executor OFFLINE_EXECUTOR = newSingleThreadExecutor("IterableOfflineExecutor"); - private static final Executor REQUEST_EXECUTOR = Executors.newFixedThreadPool( - REQUEST_THREAD_COUNT, - runnable -> newThread( - runnable, - "IterableRequestExecutor-" + REQUEST_THREAD_ID.incrementAndGet() - ) - ); + private static final Executor REQUEST_EXECUTOR = + newRequestExecutor(REQUEST_THREAD_COUNT, REQUEST_QUEUE_CAPACITY); private static final Executor MAIN_EXECUTOR = runnable -> new Handler(Looper.getMainLooper()).post(runnable); @@ -56,10 +60,29 @@ private static Executor newSingleThreadExecutor(String threadName) { ); } + static ThreadPoolExecutor newRequestExecutor(int threadCount, int queueCapacity) { + ThreadPoolExecutor executor = new ThreadPoolExecutor( + threadCount, + threadCount, + REQUEST_THREAD_KEEP_ALIVE_SECONDS, + TimeUnit.SECONDS, + new ArrayBlockingQueue<>(queueCapacity), + runnable -> newThread( + runnable, + "IterableRequestExecutor-" + REQUEST_THREAD_ID.incrementAndGet() + ), + new ThreadPoolExecutor.AbortPolicy() + ); + executor.allowCoreThreadTimeOut(true); + return executor; + } + private static Thread newThread(Runnable runnable, String threadName) { - Thread thread = new Thread(runnable, threadName); + Thread thread = new Thread(() -> { + Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); + runnable.run(); + }, threadName); thread.setDaemon(true); - thread.setPriority(Thread.NORM_PRIORITY); return thread; } } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java index c92eda58d..befef60d8 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java @@ -4,8 +4,13 @@ import android.os.Looper; import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionException; final class IterableRequestDispatcher { + private static final String TAG = "RequestDispatcher"; + private static final String QUEUE_FULL_ERROR = + "Iterable request queue is full"; + interface RetryScheduler { void schedule(Runnable runnable, long delayMs); } @@ -51,18 +56,40 @@ static IterableRequestDispatcher offline() { } void execute(IterableApiRequest request) { - requestExecutor.execute(new IterableRequestTask(request, 0, this)); + IterableRequestTask requestTask = new IterableRequestTask(request, 0, this); + try { + requestExecutor.execute(requestTask); + } catch (RejectedExecutionException e) { + IterableLogger.e(TAG, QUEUE_FULL_ERROR, e); + deliverResult(() -> requestTask.handleResponse(queueFullResponse())); + } } void executeForResponse(IterableApiRequest request, ResponseHandler responseHandler) { - requestExecutor.execute(() -> responseHandler.onResponse( - IterableRequestTask.executeApiRequest(request, this) - )); + try { + requestExecutor.execute(() -> responseHandler.onResponse( + IterableRequestTask.executeApiRequest(request, this) + )); + } catch (RejectedExecutionException e) { + IterableLogger.e(TAG, QUEUE_FULL_ERROR, e); + deliverResult(() -> responseHandler.onResponse(queueFullResponse())); + } } void executeRetry(IterableApiRequest request, int retryCount) { if (request.canRetry()) { - requestExecutor.execute(new IterableRequestTask(request, retryCount, this, true)); + IterableRequestTask requestTask = + new IterableRequestTask(request, retryCount, this, true); + try { + requestExecutor.execute(requestTask); + } catch (RejectedExecutionException e) { + IterableLogger.e(TAG, QUEUE_FULL_ERROR, e); + deliverResult(() -> { + if (request.canRetry()) { + requestTask.handleResponse(queueFullResponse()); + } + }); + } } } @@ -81,6 +108,10 @@ private static IterableRequestDispatcher sdkDispatcher(Executor requestExecutor) SDK_RETRY_SCHEDULER ); } + + private static IterableApiResponse queueFullResponse() { + return IterableApiResponse.failure(0, null, null, QUEUE_FULL_ERROR); + } } final class IterableRequestDispatchers { diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java index e9deafb35..cfc178912 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java @@ -29,7 +29,8 @@ class IterableTaskRunner implements IterableTaskStorage.TaskCreatedListener, Han private final HandlerThread networkThread = new HandlerThread("NetworkThread"); Handler handler; - private boolean requestInFlight; + private volatile boolean requestInFlight; + private volatile boolean disposed; enum TaskResult { SUCCESS, FAILURE, RETRY @@ -72,6 +73,22 @@ void removeTaskCompletedListener(TaskCompletedListener listener) { taskCompletedListeners.remove(listener); } + void dispose() { + if (disposed) { + return; + } + disposed = true; + taskStorage.removeTaskCreatedListener(this); + networkConnectivityManager.removeNetworkListener(this); + activityMonitor.removeCallback(this); + handler.post(() -> { + handler.removeCallbacksAndMessages(null); + if (!requestInFlight) { + finishDispose(); + } + }); + } + @Override public void onTaskCreated(IterableTask iterableTask) { runNow(); @@ -104,11 +121,17 @@ public void onAuthTokenReady() { } private synchronized void runNow() { + if (disposed) { + return; + } handler.removeMessages(OPERATION_PROCESS_TASKS); handler.sendEmptyMessage(OPERATION_PROCESS_TASKS); } private void scheduleRetry() { + if (disposed) { + return; + } handler.removeCallbacksAndMessages(OPERATION_PROCESS_TASKS); handler.sendEmptyMessageDelayed(OPERATION_PROCESS_TASKS, RETRY_INTERVAL_SECONDS * 1000); } @@ -125,7 +148,7 @@ public boolean handleMessage(@NonNull Message msg) { @WorkerThread private void processTasks() { - if (requestInFlight) { + if (disposed || requestInFlight) { return; } @@ -165,7 +188,12 @@ void setIsPausedForAuth(boolean paused) { @WorkerThread private void processTask(@NonNull IterableTask task, boolean autoRetry) { - if (task.taskType != IterableTaskType.API) { + if (disposed || task.taskType != IterableTaskType.API) { + return; + } + + if (!taskStorage.markTaskProcessingIfAvailable(task.id)) { + runNow(); return; } @@ -213,7 +241,9 @@ private void handleTaskResponse( ); IterableApi.getInstance().getAuthManager().setAuthTokenInvalid(); isPausedForAuth = true; + taskStorage.updateIsProcessing(task.id, false); callTaskCompletedListeners(task.id, TaskResult.RETRY, response); + finishDisposeIfNeeded(); return; } else if (!isPermanentFailure(response)) { result = TaskResult.RETRY; @@ -222,11 +252,13 @@ private void handleTaskResponse( callTaskCompletedListeners(task.id, result, response); if (result == TaskResult.RETRY) { + taskStorage.updateIsProcessing(task.id, false); scheduleRetry(); } else { taskStorage.deleteTask(task.id); runNow(); } + finishDisposeIfNeeded(); } JSONObject getTaskDataWithDate(IterableTask task) { @@ -269,7 +301,7 @@ private boolean isJwtFailure(IterableApiResponse response) { @WorkerThread private void callTaskCompletedListeners(final String taskId, final TaskResult result, final IterableApiResponse response) { - for (final TaskCompletedListener listener : taskCompletedListeners) { + for (final TaskCompletedListener listener : new ArrayList<>(taskCompletedListeners)) { new Handler(Looper.getMainLooper()).post(new Runnable() { @Override public void run() { @@ -278,4 +310,18 @@ public void run() { }); } } + + @WorkerThread + private void finishDisposeIfNeeded() { + if (disposed && !requestInFlight) { + finishDispose(); + } + } + + @WorkerThread + private void finishDispose() { + handler.removeCallbacksAndMessages(null); + taskCompletedListeners.clear(); + networkThread.quitSafely(); + } } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java index aa8f50211..42df5b569 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java @@ -78,6 +78,7 @@ private IterableTaskStorage(Context context) { databaseManager = new IterableDatabaseManager(context); } database = databaseManager.getWritableDatabase(); + resetProcessingState(); } catch (SQLException e) { IterableLogger.e(TAG, "Database cannot be opened for writing"); } @@ -95,6 +96,10 @@ void addTaskCreatedListener(TaskCreatedListener listener) { } void removeDatabaseStatusListener(TaskCreatedListener listener) { + removeTaskCreatedListener(listener); + } + + void removeTaskCreatedListener(TaskCreatedListener listener) { taskCreatedListeners.remove(listener); } @@ -278,7 +283,12 @@ IterableTask getNextScheduledTask() { if (!isDatabaseReady()) { return null; } - Cursor cursor = database.rawQuery("select * from OfflineTask order by scheduled limit 1", null); + Cursor cursor = database.rawQuery( + "select * from OfflineTask" + + " where processing is null or processing = 0" + + " order by scheduled, rowid limit 1", + null + ); IterableTask task = null; if (cursor.moveToFirst()) { task = createTaskFromCursor(cursor); @@ -300,7 +310,12 @@ IterableTask getNextScheduledTaskNotRequiringJwt(ApiEndpointClassification class if (!isDatabaseReady()) { return null; } - Cursor cursor = database.rawQuery("select * from OfflineTask order by scheduled", null); + Cursor cursor = database.rawQuery( + "select * from OfflineTask" + + " where processing is null or processing = 0" + + " order by scheduled, rowid", + null + ); IterableTask task = null; if (cursor.moveToFirst()) { do { @@ -410,6 +425,22 @@ boolean updateIsProcessing(String id, Boolean state) { return updateTaskWithContentValues(id, contentValues); } + /** + * Atomically claims a task so two runners cannot dispatch the same stored request. + */ + boolean markTaskProcessingIfAvailable(String id) { + if (!isDatabaseReady()) return false; + ContentValues contentValues = new ContentValues(); + contentValues.put(PROCESSING, true); + int updatedRows = database.update( + ITERABLE_TASK_TABLE_NAME, + contentValues, + TASK_ID + "=? AND (" + PROCESSING + " IS NULL OR " + PROCESSING + "=0)", + new String[]{id} + ); + return updatedRows == 1; + } + /** * Updates the failed state of task in OfflineTask table * @@ -485,7 +516,26 @@ boolean updateData(String id, String data) { } private boolean updateTaskWithContentValues(String id, ContentValues contentValues) { - return (0 > database.update(ITERABLE_TASK_TABLE_NAME, contentValues, TASK_ID + "=?", new String[]{id})); + return database.update( + ITERABLE_TASK_TABLE_NAME, + contentValues, + TASK_ID + "=?", + new String[]{id} + ) > 0; + } + + private void resetProcessingState() { + if (database == null || !database.isOpen()) { + return; + } + ContentValues contentValues = new ContentValues(); + contentValues.put(PROCESSING, false); + database.update( + ITERABLE_TASK_TABLE_NAME, + contentValues, + PROCESSING + "=1", + null + ); } private boolean isDatabaseReady() { @@ -527,4 +577,4 @@ public void run() { } }); } -} \ No newline at end of file +} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java index 7c896a9a0..17d54f5d4 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java @@ -19,7 +19,7 @@ class OfflineRequestProcessor implements RequestProcessor { private IterableTaskRunner taskRunner; private IterableTaskStorage taskStorage; private HealthMonitor healthMonitor; - private final IterableRequestDispatcher requestDispatcher; + private final IterableRequestDispatcher immediateRequestDispatcher; private static final Set offlineApiSet = new HashSet<>(Arrays.asList( IterableConstants.ENDPOINT_TRACK, @@ -39,9 +39,10 @@ class OfflineRequestProcessor implements RequestProcessor { OfflineRequestProcessor( Context context, - IterableRequestDispatcher requestDispatcher + IterableRequestDispatcher immediateRequestDispatcher, + IterableRequestDispatcher offlineRequestDispatcher ) { - this.requestDispatcher = requestDispatcher; + this.immediateRequestDispatcher = immediateRequestDispatcher; IterableNetworkConnectivityManager networkConnectivityManager = IterableNetworkConnectivityManager.sharedInstance(context); taskStorage = IterableTaskStorage.sharedInstance(context); healthMonitor = new HealthMonitor(taskStorage); @@ -51,8 +52,12 @@ class OfflineRequestProcessor implements RequestProcessor { networkConnectivityManager, healthMonitor, classification, - requestDispatcher); - taskScheduler = new TaskScheduler(taskStorage, taskRunner, requestDispatcher); + offlineRequestDispatcher); + taskScheduler = new TaskScheduler( + taskStorage, + taskRunner, + immediateRequestDispatcher + ); // Register task runner as auth token ready listener for JWT auto-retry support try { @@ -73,6 +78,7 @@ void dispose() { } catch (Exception e) { IterableLogger.w("OfflineRequestProcessor", "Failed to unregister auth token listener on dispose."); } + taskRunner.dispose(); } OfflineRequestProcessor( @@ -80,25 +86,25 @@ void dispose() { IterableTaskRunner iterableTaskRunner, IterableTaskStorage storage, HealthMonitor mockHealthMonitor, - IterableRequestDispatcher requestDispatcher + IterableRequestDispatcher immediateRequestDispatcher ) { taskRunner = iterableTaskRunner; taskScheduler = scheduler; taskStorage = storage; healthMonitor = mockHealthMonitor; - this.requestDispatcher = requestDispatcher; + this.immediateRequestDispatcher = immediateRequestDispatcher; } @Override public void processGetRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.IterableActionHandler onCallback) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, json, IterableApiRequest.GET, authToken, onCallback); - requestDispatcher.execute(request); + immediateRequestDispatcher.execute(request); } @Override public void processGetRequest(@Nullable String apiKey, @NonNull String resourcePath, @NonNull JSONObject json, String authToken, @Nullable IterableHelper.SuccessHandler onSuccess, @Nullable IterableHelper.FailureHandler onFailure) { IterableApiRequest request = new IterableApiRequest(apiKey, resourcePath, json, IterableApiRequest.GET, authToken, onSuccess, onFailure); - requestDispatcher.execute(request); + immediateRequestDispatcher.execute(request); } @Override @@ -108,7 +114,7 @@ public void processPostRequest(@Nullable String apiKey, @NonNull String resource request.setProcessorType(IterableApiRequest.ProcessorType.OFFLINE); taskScheduler.scheduleTask(request, onSuccess, onFailure); } else { - requestDispatcher.execute(request); + immediateRequestDispatcher.execute(request); } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java new file mode 100644 index 000000000..353376ecf --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java @@ -0,0 +1,75 @@ +package com.iterable.iterableapi; + +import android.os.Process; + +import com.iterable.iterableapi.unit.TestRunner; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +@RunWith(TestRunner.class) +public class IterableExecutorsTest { + @Test + public void requestExecutorIsBoundedAndNeverRunsRejectedWorkOnCaller() + throws Exception { + ThreadPoolExecutor executor = IterableExecutors.newRequestExecutor(2, 1); + CountDownLatch workersStarted = new CountDownLatch(2); + CountDownLatch releaseWorkers = new CountDownLatch(1); + AtomicInteger callerRuns = new AtomicInteger(); + + try { + Runnable blockingWork = () -> { + workersStarted.countDown(); + try { + releaseWorkers.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }; + executor.execute(blockingWork); + executor.execute(blockingWork); + assertTrue(workersStarted.await(5, TimeUnit.SECONDS)); + + executor.execute(() -> { }); + try { + executor.execute(callerRuns::incrementAndGet); + fail("Expected the bounded executor to reject excess work"); + } catch (RejectedExecutionException expected) { + assertEquals(0, callerRuns.get()); + } + } finally { + releaseWorkers.countDown(); + executor.shutdownNow(); + } + } + + @Test + public void requestWorkersUseAndroidBackgroundPriority() throws Exception { + ThreadPoolExecutor executor = IterableExecutors.newRequestExecutor(1, 1); + CountDownLatch completed = new CountDownLatch(1); + AtomicInteger priority = new AtomicInteger(Integer.MIN_VALUE); + + try { + executor.execute(() -> { + priority.set(Process.getThreadPriority(Process.myTid())); + completed.countDown(); + }); + + assertTrue(completed.await(5, TimeUnit.SECONDS)); + assertEquals(Process.THREAD_PRIORITY_BACKGROUND, priority.get()); + assertTrue(executor.getThreadFactory().newThread(() -> { }).isDaemon()); + } finally { + executor.shutdownNow(); + } + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt index 9745f8f1e..3cfa746aa 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt @@ -8,6 +8,7 @@ import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import java.util.concurrent.Executor +import java.util.concurrent.RejectedExecutionException @RunWith(TestRunner::class) class IterableRequestDispatcherTest { @@ -50,7 +51,57 @@ class IterableRequestDispatcherTest { assertTrue(requestExecutor.tasks.single() is IterableRequestTask) } - private fun request(): IterableApiRequest { + @Test + fun `stale retry is not submitted`() { + val staleRequest = request().apply { + setRetryState { false } + } + + dispatcher.executeRetry(staleRequest, 0) + + assertTrue(requestExecutor.tasks.isEmpty()) + } + + @Test + fun `a saturated executor fails asynchronously without running network work on caller`() { + val failure = RecordingFailureHandler() + val rejectingDispatcher = IterableRequestDispatcher( + Executor { throw RejectedExecutionException("full") }, + callbackExecutor, + retryScheduler + ) + + rejectingDispatcher.execute(request(failure)) + + assertEquals(1, callbackExecutor.tasks.size) + assertEquals(null, failure.message) + + callbackExecutor.tasks.single().run() + + assertEquals("Iterable request queue is full", failure.message) + } + + @Test + fun `a saturated response request receives a transient failure`() { + var response: IterableApiResponse? = null + val rejectingDispatcher = IterableRequestDispatcher( + Executor { throw RejectedExecutionException("full") }, + callbackExecutor, + retryScheduler + ) + + rejectingDispatcher.executeForResponse(request()) { + response = it + } + callbackExecutor.tasks.single().run() + + assertEquals(0, response?.responseCode) + assertEquals(false, response?.success) + } + + private fun request( + failure: IterableHelper.FailureHandler? = null + ): IterableApiRequest { return IterableApiRequest( "api-key", "api/test", @@ -58,7 +109,7 @@ class IterableRequestDispatcherTest { IterableApiRequest.POST, null, null, - null + failure ) } @@ -79,4 +130,12 @@ class IterableRequestDispatcherTest { this.delayMs = delayMs } } + + private class RecordingFailureHandler : IterableHelper.FailureHandler { + var message: String? = null + + override fun onFailure(reason: String, data: JSONObject?) { + message = reason + } + } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt index 20afdd097..bf1a1d5e0 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt @@ -3,6 +3,7 @@ package com.iterable.iterableapi import com.iterable.iterableapi.unit.TestRunner import org.json.JSONObject import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertSame import org.junit.Before @@ -136,6 +137,21 @@ class IterableRequestTaskTest { assertSame(callback, retriedRequest.legacyCallback) } + @Test + fun `auth retry preserves a stale request guard`() { + val request = request().apply { + setRetryState { false } + } + + IterableRequestTask.retryRequestWithNewAuthToken( + "fresh-token", + request, + requestDispatcher + ) + + assertFalse(captureDispatchedRequest().canRetry()) + } + private fun captureDispatchedRequest(): IterableApiRequest { val requestCaptor = ArgumentCaptor.forClass(IterableApiRequest::class.java) verify(requestDispatcher).executeRetry(requestCaptor.capture(), eq(0)) diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java index 769659a5e..7987cc2b9 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java @@ -12,9 +12,6 @@ import org.junit.Test; import org.junit.runner.RunWith; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import okhttp3.mockwebserver.MockResponse; @@ -28,6 +25,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doReturn; @@ -60,6 +58,7 @@ public void setUp() throws Exception { Runnable::run, (runnable, delayMs) -> runnable.run() ); + when(mockTaskStorage.markTaskProcessingIfAvailable(anyString())).thenReturn(true); taskRunner = new IterableTaskRunner( mockTaskStorage, mockActivityMonitor, @@ -74,6 +73,7 @@ public void setUp() throws Exception { @After public void tearDown() throws Exception { + taskRunner.dispose(); server.shutdown(); IterableTestUtils.resetIterableApi(); } @@ -189,68 +189,13 @@ public void testIfNetworkCheckedBeforeProcessingTask() throws Exception { } @Test - public void testStoredAndImmediateRequestsUseTheSameQueueInSubmissionOrder() - throws Exception { - RecordingExecutor requestExecutor = new RecordingExecutor(); - IterableRequestDispatcher sharedDispatcher = new IterableRequestDispatcher( - requestExecutor, - runnable -> { }, - (runnable, delayMs) -> requestExecutor.execute(runnable) - ); - IterableTaskRunner runner = new IterableTaskRunner( - mockTaskStorage, - mockActivityMonitor, - mockNetworkConnectivityManager, - mockHealthMonitor, - new ApiEndpointClassification(), - sharedDispatcher - ); - IterableApiRequest storedRequest = new IterableApiRequest( - "apiKey", - "api/stored", - new JSONObject(), - IterableApiRequest.POST, - null, - null, - null - ); - IterableTask storedTask = new IterableTask( - "storedTask", - IterableTaskType.API, - storedRequest.toJSONObject().toString() - ); - when(mockTaskStorage.getNextScheduledTask()).thenReturn(storedTask).thenReturn(null); - when(mockActivityMonitor.isInForeground()).thenReturn(true); - when(mockNetworkConnectivityManager.isConnected()).thenReturn(true); - when(mockHealthMonitor.canProcess()).thenReturn(true); - server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); - server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); - - sharedDispatcher.execute(new IterableApiRequest( - "apiKey", - "api/immediate", - new JSONObject(), - IterableApiRequest.POST, - null, - null, - null - )); - runner.onTaskCreated(null); - runHandlerTasks(runner); - - assertEquals(2, requestExecutor.pendingTaskCount()); - requestExecutor.runAll(); - - RecordedRequest immediateRequest = server.takeRequest(1, TimeUnit.SECONDS); - assertNotNull(immediateRequest); - assertEquals("/api/immediate", immediateRequest.getPath()); - - RecordedRequest persistedRequest = server.takeRequest(1, TimeUnit.SECONDS); - assertNotNull(persistedRequest); - assertEquals("/api/stored", persistedRequest.getPath()); + public void testDisposeUnregistersEventListeners() throws Exception { + taskRunner.dispose(); + runHandlerTasks(taskRunner); - runHandlerTasks(runner); - verify(mockTaskStorage).deleteTask(storedTask.id); + verify(mockTaskStorage).removeTaskCreatedListener(taskRunner); + verify(mockNetworkConnectivityManager).removeNetworkListener(taskRunner); + verify(mockActivityMonitor).removeCallback(taskRunner); } // region Auto-Retry on JWT Failure Tests @@ -741,22 +686,4 @@ private void runHandlerTasks(IterableTaskRunner taskRunner) throws InterruptedEx shadowOf(taskRunner.handler.getLooper()).idle(); } - private static class RecordingExecutor implements Executor { - private final List tasks = new ArrayList<>(); - - @Override - public void execute(Runnable command) { - tasks.add(command); - } - - int pendingTaskCount() { - return tasks.size(); - } - - void runAll() { - while (!tasks.isEmpty()) { - tasks.remove(0).run(); - } - } - } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java new file mode 100644 index 000000000..995faf44a --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java @@ -0,0 +1,69 @@ +package com.iterable.iterableapi; + +import android.content.ContentValues; +import android.database.sqlite.SQLiteDatabase; + +import com.iterable.iterableapi.unit.TestRunner; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.util.ReflectionHelpers; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@RunWith(TestRunner.class) +public class IterableTaskStorageTest extends BaseTest { + private IterableTaskStorage storage; + + @Before + public void setUp() { + ReflectionHelpers.setStaticField( + IterableTaskStorage.class, + "sharedInstance", + null + ); + storage = IterableTaskStorage.sharedInstance(getContext()); + storage.deleteAllTasks(); + } + + @After + public void tearDown() { + storage.deleteAllTasks(); + ReflectionHelpers.setStaticField( + IterableTaskStorage.class, + "sharedInstance", + null + ); + } + + @Test + public void equalScheduledTimesUseInsertionOrderAndClaimedTasksAreSkipped() { + String firstId = storage.createTask("first", IterableTaskType.API, "{}"); + String secondId = storage.createTask("second", IterableTaskType.API, "{}"); + assertNotNull(firstId); + assertNotNull(secondId); + + SQLiteDatabase database = ReflectionHelpers.getField(storage, "database"); + ContentValues scheduled = new ContentValues(); + scheduled.put(IterableTaskStorage.SCHEDULED_AT, 1000L); + database.update( + IterableTaskStorage.ITERABLE_TASK_TABLE_NAME, + scheduled, + null, + null + ); + + assertEquals(firstId, storage.getNextScheduledTask().id); + assertTrue(storage.markTaskProcessingIfAvailable(firstId)); + assertFalse(storage.markTaskProcessingIfAvailable(firstId)); + assertEquals(secondId, storage.getNextScheduledTask().id); + + assertTrue(storage.updateIsProcessing(firstId, false)); + assertEquals(firstId, storage.getNextScheduledTask().id); + } +} diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt index 95652534b..7ab7004fa 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/OfflineRequestProcessorTest.kt @@ -16,19 +16,19 @@ class OfflineRequestProcessorTest : BaseTest() { private lateinit var requestProcessor: OfflineRequestProcessor private lateinit var taskScheduler: TaskScheduler private lateinit var healthMonitor: HealthMonitor - private lateinit var requestDispatcher: IterableRequestDispatcher + private lateinit var immediateRequestDispatcher: IterableRequestDispatcher @Before fun setUp() { taskScheduler = mock(TaskScheduler::class.java) healthMonitor = mock(HealthMonitor::class.java) - requestDispatcher = mock(IterableRequestDispatcher::class.java) + immediateRequestDispatcher = mock(IterableRequestDispatcher::class.java) requestProcessor = OfflineRequestProcessor( taskScheduler, mock(IterableTaskRunner::class.java), mock(IterableTaskStorage::class.java), healthMonitor, - requestDispatcher + immediateRequestDispatcher ) } @@ -55,7 +55,7 @@ class OfflineRequestProcessorTest : BaseTest() { IterableApiRequest.ProcessorType.OFFLINE, requestCaptor.value.processorType ) - verifyNoInteractions(requestDispatcher) + verifyNoInteractions(immediateRequestDispatcher) } @Test @@ -126,7 +126,7 @@ class OfflineRequestProcessorTest : BaseTest() { private fun captureDispatchedRequest(): IterableApiRequest { val requestCaptor = ArgumentCaptor.forClass(IterableApiRequest::class.java) - verify(requestDispatcher).execute(requestCaptor.capture()) + verify(immediateRequestDispatcher).execute(requestCaptor.capture()) return requestCaptor.value } } From a34b9e37466239c82cd7f2d6ac8cfed6793e30b1 Mon Sep 17 00:00:00 2001 From: Franco Zalamena Date: Wed, 7 Oct 2026 00:59:28 +0100 Subject: [PATCH 7/7] [SDK-707] Preserve legacy request execution behavior --- CHANGELOG.md | 2 +- .../iterableapi/IterableApiClient.java | 70 ++++-- .../iterableapi/IterableExecutors.java | 61 +++++- .../IterableRequestDispatcher.java | 87 ++++++-- .../iterableapi/IterableTaskRunner.java | 4 + .../iterableapi/IterableTaskStorage.java | 8 + .../iterableapi/OfflineRequestProcessor.java | 4 +- .../IterableApiClientRequestRoutingTest.kt | 91 +++++++- .../iterableapi/IterableExecutorsTest.java | 44 ++-- .../IterableRequestDispatcherTest.kt | 17 +- ...IterableRequestExecutionConcurrencyTest.kt | 200 ++++++++++++++++++ .../iterableapi/IterableTaskRunnerTest.java | 31 +++ .../iterableapi/IterableTaskStorageTest.java | 3 + 13 files changed, 547 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 418d2f615..c799eb848 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Fixed -- Push token registration, disable operations, Iterable deep-link redirects, and API requests now use dedicated SDK-owned executors instead of Android's process-wide `AsyncTask` queues. Push, deep-link, and offline operations preserve ordering in isolated serial lanes, while ordinary online API requests retain concurrent execution. Slow work in one lane no longer delays unrelated SDK operations, and client callbacks and attribution updates continue on the main thread. +- Push token registration, disable operations, Iterable deep-link redirects, and API requests now use dedicated SDK-owned executors instead of Android's process-wide `AsyncTask` queues. Push and deep-link work retain isolated serial lanes. Ordinary online API requests remain concurrent, while their automatic retries and offline immediate requests retain serial ordering; stored offline requests retain their own FIFO flow. Slow work in one flow no longer delays unrelated SDK operations, and client callbacks and attribution updates continue on the main thread. ## [3.11.0] ### Added diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java index 81bdb5917..8a6cd41a8 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java @@ -26,6 +26,7 @@ class IterableApiClient { // A newer push action invalidates retries from earlier registration or disable requests. private final AtomicLong pushRegistrationRequestGeneration = new AtomicLong(); private RequestProcessor requestProcessor; + private OfflineRequestProcessor offlineRequestProcessor; interface AuthProvider { @Nullable @@ -59,32 +60,50 @@ interface AuthProvider { new OnlineRequestProcessor(requestDispatchers.push()); } - private RequestProcessor getRequestProcessor() { + private synchronized RequestProcessor getRequestProcessor() { if (requestProcessor == null) { requestProcessor = new OnlineRequestProcessor(requestDispatchers.online()); } return requestProcessor; } - void setOfflineProcessingEnabled(boolean offlineMode) { - if (offlineMode && this.requestProcessor instanceof OfflineRequestProcessor) { - return; - } - if (!offlineMode && this.requestProcessor instanceof OnlineRequestProcessor) { + synchronized void setOfflineProcessingEnabled(boolean offlineMode) { + if (!offlineMode + && offlineRequestProcessor == null + && !hasPendingOfflineRequests()) { + if (!(requestProcessor instanceof OnlineRequestProcessor)) { + requestProcessor = new OnlineRequestProcessor(requestDispatchers.online()); + } return; } - if (this.requestProcessor instanceof OfflineRequestProcessor) { - ((OfflineRequestProcessor) this.requestProcessor).dispose(); + // Once needed, keep one offline processor alive while new requests use the + // online processor. Its runner must finish requests queued before offline + // processing was disabled or restored from a previous process. + OfflineRequestProcessor persistentOfflineRequestProcessor = + getOfflineRequestProcessor(); + if (offlineMode) { + requestProcessor = persistentOfflineRequestProcessor; + } else if (!(requestProcessor instanceof OnlineRequestProcessor)) { + requestProcessor = new OnlineRequestProcessor(requestDispatchers.online()); } + } - this.requestProcessor = offlineMode - ? new OfflineRequestProcessor( + private OfflineRequestProcessor getOfflineRequestProcessor() { + if (offlineRequestProcessor == null) { + offlineRequestProcessor = new OfflineRequestProcessor( authProvider.getContext(), - requestDispatchers.online(), - requestDispatchers.offline() - ) - : new OnlineRequestProcessor(requestDispatchers.online()); + requestDispatchers.offlineImmediate(), + requestDispatchers.offlineStored() + ); + } + return offlineRequestProcessor; + } + + private boolean hasPendingOfflineRequests() { + return IterableTaskStorage + .sharedInstance(authProvider.getContext()) + .hasPendingTasks(); } void getRemoteConfiguration(IterableHelper.IterableActionHandler actionHandler) { @@ -832,11 +851,30 @@ void sendGetRequest(@NonNull String resourcePath, @NonNull JSONObject json, @Non getRequestProcessor().processGetRequest(authProvider.getApiKey(), resourcePath, json, authProvider.getAuthToken(), onSuccess, onFailure); } - void onLogout() { - getRequestProcessor().onLogout(authProvider.getContext()); + synchronized void onLogout() { + Context context = authProvider.getContext(); + RequestProcessor activeRequestProcessor = getRequestProcessor(); + if (offlineRequestProcessor != null) { + offlineRequestProcessor.onLogout(context); + } else { + IterableTaskStorage.sharedInstance(context).deleteAllTasks(); + } + if (activeRequestProcessor != offlineRequestProcessor) { + activeRequestProcessor.onLogout(context); + } authProvider.resetAuth(); } + synchronized void dispose() { + if (offlineRequestProcessor != null) { + if (requestProcessor == offlineRequestProcessor) { + requestProcessor = null; + } + offlineRequestProcessor.dispose(); + offlineRequestProcessor = null; + } + } + void mergeUser(String sourceEmail, String sourceUserId, String destinationEmail, String destinationUserId, @Nullable IterableHelper.SuccessHandler successHandler, @Nullable IterableHelper.FailureHandler failureHandler) { JSONObject requestJson = new JSONObject(); try { diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java index dbea1890c..b53aa0b70 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java @@ -7,6 +7,7 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -15,19 +16,32 @@ final class IterableExecutors { // HttpURLConnection is blocking I/O, so ordinary API work uses a bounded // multi-thread pool rather than the serial executors used for ordered work. static final int REQUEST_THREAD_COUNT = 8; - // Matches the historical AsyncTask queue bound without inheriting its + // Bounds the primary pool without inheriting AsyncTask's // platform-version-dependent thread-pool behavior. static final int REQUEST_QUEUE_CAPACITY = 128; private static final long REQUEST_THREAD_KEEP_ALIVE_SECONDS = 30; + private static final int REQUEST_OVERFLOW_THREAD_COUNT = 5; + private static final long REQUEST_OVERFLOW_KEEP_ALIVE_SECONDS = 3; private static final AtomicInteger REQUEST_THREAD_ID = new AtomicInteger(); + private static final AtomicInteger REQUEST_OVERFLOW_THREAD_ID = new AtomicInteger(); private static final Executor PUSH_EXECUTOR = newSingleThreadExecutor("IterablePushExecutor"); private static final Executor DEEP_LINK_EXECUTOR = newSingleThreadExecutor("IterableDeepLinkExecutor"); - private static final Executor OFFLINE_EXECUTOR = - newSingleThreadExecutor("IterableOfflineExecutor"); + private static final Executor SERIAL_REQUEST_EXECUTOR = + newSingleThreadExecutor("IterableSerialRequestExecutor"); + private static final Executor OFFLINE_STORED_EXECUTOR = + newSingleThreadExecutor("IterableOfflineStoredExecutor"); + // AsyncTask's concurrent executor used a backup queue instead of dropping + // work when its primary pool was saturated. Preserve that delivery behavior. + private static final Executor REQUEST_OVERFLOW_EXECUTOR = + newRequestOverflowExecutor(REQUEST_OVERFLOW_THREAD_COUNT); private static final Executor REQUEST_EXECUTOR = - newRequestExecutor(REQUEST_THREAD_COUNT, REQUEST_QUEUE_CAPACITY); + newRequestExecutor( + REQUEST_THREAD_COUNT, + REQUEST_QUEUE_CAPACITY, + REQUEST_OVERFLOW_EXECUTOR + ); private static final Executor MAIN_EXECUTOR = runnable -> new Handler(Looper.getMainLooper()).post(runnable); @@ -46,8 +60,16 @@ static Executor request() { return REQUEST_EXECUTOR; } - static Executor offline() { - return OFFLINE_EXECUTOR; + static Executor offlineImmediate() { + return SERIAL_REQUEST_EXECUTOR; + } + + static Executor requestRetry() { + return SERIAL_REQUEST_EXECUTOR; + } + + static Executor offlineStored() { + return OFFLINE_STORED_EXECUTOR; } static Executor main() { @@ -61,6 +83,14 @@ private static Executor newSingleThreadExecutor(String threadName) { } static ThreadPoolExecutor newRequestExecutor(int threadCount, int queueCapacity) { + return newRequestExecutor(threadCount, queueCapacity, REQUEST_OVERFLOW_EXECUTOR); + } + + static ThreadPoolExecutor newRequestExecutor( + int threadCount, + int queueCapacity, + Executor overflowExecutor + ) { ThreadPoolExecutor executor = new ThreadPoolExecutor( threadCount, threadCount, @@ -71,7 +101,24 @@ static ThreadPoolExecutor newRequestExecutor(int threadCount, int queueCapacity) runnable, "IterableRequestExecutor-" + REQUEST_THREAD_ID.incrementAndGet() ), - new ThreadPoolExecutor.AbortPolicy() + (runnable, ignored) -> overflowExecutor.execute(runnable) + ); + executor.allowCoreThreadTimeOut(true); + return executor; + } + + static ThreadPoolExecutor newRequestOverflowExecutor(int threadCount) { + ThreadPoolExecutor executor = new ThreadPoolExecutor( + threadCount, + threadCount, + REQUEST_OVERFLOW_KEEP_ALIVE_SECONDS, + TimeUnit.SECONDS, + new LinkedBlockingQueue<>(), + runnable -> newThread( + runnable, + "IterableRequestOverflowExecutor-" + + REQUEST_OVERFLOW_THREAD_ID.incrementAndGet() + ) ); executor.allowCoreThreadTimeOut(true); return executor; diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java index befef60d8..9743a99a5 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java @@ -8,8 +8,8 @@ final class IterableRequestDispatcher { private static final String TAG = "RequestDispatcher"; - private static final String QUEUE_FULL_ERROR = - "Iterable request queue is full"; + private static final String REQUEST_REJECTED_ERROR = + "Iterable request executor rejected work"; interface RetryScheduler { void schedule(Runnable runnable, long delayMs); @@ -23,13 +23,19 @@ interface ResponseHandler { (runnable, delayMs) -> new Handler(Looper.getMainLooper()).postDelayed(runnable, delayMs); private static final IterableRequestDispatcher ONLINE_DISPATCHER = - sdkDispatcher(IterableExecutors.request()); + sdkDispatcher( + IterableExecutors.request(), + IterableExecutors.requestRetry() + ); private static final IterableRequestDispatcher PUSH_DISPATCHER = sdkDispatcher(IterableExecutors.push()); - private static final IterableRequestDispatcher OFFLINE_DISPATCHER = - sdkDispatcher(IterableExecutors.offline()); + private static final IterableRequestDispatcher OFFLINE_IMMEDIATE_DISPATCHER = + sdkDispatcher(IterableExecutors.offlineImmediate()); + private static final IterableRequestDispatcher OFFLINE_STORED_DISPATCHER = + sdkDispatcher(IterableExecutors.offlineStored()); private final Executor requestExecutor; + private final Executor retryExecutor; private final Executor callbackExecutor; private final RetryScheduler retryScheduler; @@ -41,16 +47,30 @@ static IterableRequestDispatcher push() { return PUSH_DISPATCHER; } - static IterableRequestDispatcher offline() { - return OFFLINE_DISPATCHER; + static IterableRequestDispatcher offlineImmediate() { + return OFFLINE_IMMEDIATE_DISPATCHER; + } + + static IterableRequestDispatcher offlineStored() { + return OFFLINE_STORED_DISPATCHER; } IterableRequestDispatcher( Executor requestExecutor, Executor callbackExecutor, RetryScheduler retryScheduler + ) { + this(requestExecutor, requestExecutor, callbackExecutor, retryScheduler); + } + + IterableRequestDispatcher( + Executor requestExecutor, + Executor retryExecutor, + Executor callbackExecutor, + RetryScheduler retryScheduler ) { this.requestExecutor = requestExecutor; + this.retryExecutor = retryExecutor; this.callbackExecutor = callbackExecutor; this.retryScheduler = retryScheduler; } @@ -60,8 +80,8 @@ void execute(IterableApiRequest request) { try { requestExecutor.execute(requestTask); } catch (RejectedExecutionException e) { - IterableLogger.e(TAG, QUEUE_FULL_ERROR, e); - deliverResult(() -> requestTask.handleResponse(queueFullResponse())); + IterableLogger.e(TAG, REQUEST_REJECTED_ERROR, e); + deliverResult(() -> requestTask.handleResponse(rejectedRequestResponse())); } } @@ -71,8 +91,8 @@ void executeForResponse(IterableApiRequest request, ResponseHandler responseHand IterableRequestTask.executeApiRequest(request, this) )); } catch (RejectedExecutionException e) { - IterableLogger.e(TAG, QUEUE_FULL_ERROR, e); - deliverResult(() -> responseHandler.onResponse(queueFullResponse())); + IterableLogger.e(TAG, REQUEST_REJECTED_ERROR, e); + deliverResult(() -> responseHandler.onResponse(rejectedRequestResponse())); } } @@ -81,12 +101,12 @@ void executeRetry(IterableApiRequest request, int retryCount) { IterableRequestTask requestTask = new IterableRequestTask(request, retryCount, this, true); try { - requestExecutor.execute(requestTask); + retryExecutor.execute(requestTask); } catch (RejectedExecutionException e) { - IterableLogger.e(TAG, QUEUE_FULL_ERROR, e); + IterableLogger.e(TAG, REQUEST_REJECTED_ERROR, e); deliverResult(() -> { if (request.canRetry()) { - requestTask.handleResponse(queueFullResponse()); + requestTask.handleResponse(rejectedRequestResponse()); } }); } @@ -102,15 +122,23 @@ void retry(IterableApiRequest request, int retryCount, long delayMs) { } private static IterableRequestDispatcher sdkDispatcher(Executor requestExecutor) { + return sdkDispatcher(requestExecutor, requestExecutor); + } + + private static IterableRequestDispatcher sdkDispatcher( + Executor requestExecutor, + Executor retryExecutor + ) { return new IterableRequestDispatcher( requestExecutor, + retryExecutor, IterableExecutors.main(), SDK_RETRY_SCHEDULER ); } - private static IterableApiResponse queueFullResponse() { - return IterableApiResponse.failure(0, null, null, QUEUE_FULL_ERROR); + private static IterableApiResponse rejectedRequestResponse() { + return IterableApiResponse.failure(0, null, null, REQUEST_REJECTED_ERROR); } } @@ -119,29 +147,38 @@ final class IterableRequestDispatchers { new IterableRequestDispatchers( IterableRequestDispatcher.online(), IterableRequestDispatcher.push(), - IterableRequestDispatcher.offline() + IterableRequestDispatcher.offlineImmediate(), + IterableRequestDispatcher.offlineStored() ); private final IterableRequestDispatcher online; private final IterableRequestDispatcher push; - private final IterableRequestDispatcher offline; + private final IterableRequestDispatcher offlineImmediate; + private final IterableRequestDispatcher offlineStored; static IterableRequestDispatchers sdk() { return SDK_DISPATCHERS; } static IterableRequestDispatchers same(IterableRequestDispatcher dispatcher) { - return new IterableRequestDispatchers(dispatcher, dispatcher, dispatcher); + return new IterableRequestDispatchers( + dispatcher, + dispatcher, + dispatcher, + dispatcher + ); } IterableRequestDispatchers( IterableRequestDispatcher online, IterableRequestDispatcher push, - IterableRequestDispatcher offline + IterableRequestDispatcher offlineImmediate, + IterableRequestDispatcher offlineStored ) { this.online = online; this.push = push; - this.offline = offline; + this.offlineImmediate = offlineImmediate; + this.offlineStored = offlineStored; } IterableRequestDispatcher online() { @@ -152,7 +189,11 @@ IterableRequestDispatcher push() { return push; } - IterableRequestDispatcher offline() { - return offline; + IterableRequestDispatcher offlineImmediate() { + return offlineImmediate; + } + + IterableRequestDispatcher offlineStored() { + return offlineStored; } } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java index cfc178912..0b0e208ef 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java @@ -69,6 +69,10 @@ void addTaskCompletedListener(TaskCompletedListener listener) { taskCompletedListeners.add(listener); } + void start() { + runNow(); + } + void removeTaskCompletedListener(TaskCompletedListener listener) { taskCompletedListeners.remove(listener); } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java index 42df5b569..ad8c6662b 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java @@ -91,6 +91,14 @@ static IterableTaskStorage sharedInstance(Context context) { return sharedInstance; } + boolean hasPendingTasks() { + return database != null + && DatabaseUtils.queryNumEntries( + database, + ITERABLE_TASK_TABLE_NAME + ) > 0; + } + void addTaskCreatedListener(TaskCreatedListener listener) { taskCreatedListeners.add(listener); } diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java index 17d54f5d4..629cc2aa0 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java @@ -66,11 +66,11 @@ class OfflineRequestProcessor implements RequestProcessor { IterableLogger.w("OfflineRequestProcessor", "Failed to register auth token listener. " + "Auto-retry on JWT failure will not work until AuthManager is available."); } + taskRunner.start(); } /** - * Unregisters the auth token listener to prevent stale listener accumulation - * when the processor is replaced (e.g., when offline mode is toggled). + * Releases the persisted-task runner when the owning API client is disposed. */ void dispose() { try { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt index 7f49825a1..de9ef3673 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt @@ -1,41 +1,73 @@ package com.iterable.iterableapi import org.json.JSONObject +import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.mockito.Mockito.mock +import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` +import org.robolectric.util.ReflectionHelpers import java.util.concurrent.Executor -class IterableApiClientRequestRoutingTest { +class IterableApiClientRequestRoutingTest : BaseTest() { private lateinit var onlineExecutor: RecordingExecutor private lateinit var pushExecutor: RecordingExecutor - private lateinit var offlineExecutor: RecordingExecutor + private lateinit var offlineImmediateExecutor: RecordingExecutor + private lateinit var offlineStoredExecutor: RecordingExecutor + private lateinit var authProvider: IterableApiClient.AuthProvider + private lateinit var taskStorage: IterableTaskStorage private lateinit var client: IterableApiClient @Before fun setUp() { onlineExecutor = RecordingExecutor() pushExecutor = RecordingExecutor() - offlineExecutor = RecordingExecutor() + offlineImmediateExecutor = RecordingExecutor() + offlineStoredExecutor = RecordingExecutor() + authProvider = mock(IterableApiClient.AuthProvider::class.java) + `when`(authProvider.context).thenReturn(getContext()) + ReflectionHelpers.setStaticField( + IterableTaskStorage::class.java, + "sharedInstance", + null + ) + taskStorage = IterableTaskStorage.sharedInstance(getContext()) + taskStorage.deleteAllTasks() client = IterableApiClient( - mock(IterableApiClient.AuthProvider::class.java), + authProvider, IterableRequestDispatchers( dispatcher(onlineExecutor), dispatcher(pushExecutor), - dispatcher(offlineExecutor) + dispatcher(offlineImmediateExecutor), + dispatcher(offlineStoredExecutor) ) ) } + @After + fun tearDown() { + client.dispose() + taskStorage.deleteAllTasks() + ReflectionHelpers.setStaticField( + IterableTaskStorage::class.java, + "sharedInstance", + null + ) + } + @Test fun `ordinary API request uses online lane`() { client.sendPostRequest("api/ordinary", JSONObject()) assertEquals(1, onlineExecutor.tasks.size) assertTrue(pushExecutor.tasks.isEmpty()) - assertTrue(offlineExecutor.tasks.isEmpty()) + assertTrue(offlineImmediateExecutor.tasks.isEmpty()) + assertTrue(offlineStoredExecutor.tasks.isEmpty()) } @Test @@ -51,7 +83,52 @@ class IterableApiClientRequestRoutingTest { assertEquals(1, pushExecutor.tasks.size) assertTrue(onlineExecutor.tasks.isEmpty()) - assertTrue(offlineExecutor.tasks.isEmpty()) + assertTrue(offlineImmediateExecutor.tasks.isEmpty()) + assertTrue(offlineStoredExecutor.tasks.isEmpty()) + } + + @Test + fun `offline immediate request uses its own lane`() { + client.setOfflineProcessingEnabled(true) + + client.sendPostRequest(IterableConstants.ENDPOINT_UPDATE_EMAIL, JSONObject()) + + assertEquals(1, offlineImmediateExecutor.tasks.size) + assertTrue(onlineExecutor.tasks.isEmpty()) + assertTrue(pushExecutor.tasks.isEmpty()) + assertTrue(offlineStoredExecutor.tasks.isEmpty()) + } + + @Test + fun `logout clears persisted requests before offline processor is created`() { + val taskId = taskStorage.createTask( + "api/stored-before-restart", + IterableTaskType.API, + "{}" + ) + assertNotNull(taskId) + + client.onLogout() + + assertNull(taskStorage.getNextScheduledTask()) + verify(authProvider).resetAuth() + } + + @Test + fun `logout clears persisted requests after switching back online`() { + client.setOfflineProcessingEnabled(true) + val taskId = taskStorage.createTask( + "api/stored-before-disable", + IterableTaskType.API, + "{}" + ) + assertNotNull(taskId) + client.setOfflineProcessingEnabled(false) + + client.onLogout() + + assertNull(taskStorage.getNextScheduledTask()) + verify(authProvider).resetAuth() } private fun dispatcher(executor: Executor): IterableRequestDispatcher { diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java index 353376ecf..14c087977 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java @@ -8,24 +8,32 @@ import org.junit.runner.RunWith; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; @RunWith(TestRunner.class) public class IterableExecutorsTest { @Test - public void requestExecutorIsBoundedAndNeverRunsRejectedWorkOnCaller() + public void requestExecutorSpillsExcessWorkToBackgroundOverflowWithoutDroppingIt() throws Exception { - ThreadPoolExecutor executor = IterableExecutors.newRequestExecutor(2, 1); + ThreadPoolExecutor overflowExecutor = + IterableExecutors.newRequestOverflowExecutor(1); + ThreadPoolExecutor executor = + IterableExecutors.newRequestExecutor(2, 1, overflowExecutor); CountDownLatch workersStarted = new CountDownLatch(2); CountDownLatch releaseWorkers = new CountDownLatch(1); - AtomicInteger callerRuns = new AtomicInteger(); + CountDownLatch queuedWorkCompleted = new CountDownLatch(1); + CountDownLatch overflowWorkCompleted = new CountDownLatch(1); + AtomicLong overflowThreadId = new AtomicLong(); + AtomicInteger overflowPriority = new AtomicInteger(Integer.MIN_VALUE); + AtomicInteger overflowRuns = new AtomicInteger(); + long callerThreadId = Thread.currentThread().getId(); try { Runnable blockingWork = () -> { @@ -40,16 +48,28 @@ public void requestExecutorIsBoundedAndNeverRunsRejectedWorkOnCaller() executor.execute(blockingWork); assertTrue(workersStarted.await(5, TimeUnit.SECONDS)); - executor.execute(() -> { }); - try { - executor.execute(callerRuns::incrementAndGet); - fail("Expected the bounded executor to reject excess work"); - } catch (RejectedExecutionException expected) { - assertEquals(0, callerRuns.get()); - } + executor.execute(queuedWorkCompleted::countDown); + executor.execute(() -> { + overflowRuns.incrementAndGet(); + overflowThreadId.set(Thread.currentThread().getId()); + overflowPriority.set( + Process.getThreadPriority(Process.myTid()) + ); + overflowWorkCompleted.countDown(); + }); + + assertTrue(overflowWorkCompleted.await(5, TimeUnit.SECONDS)); + assertEquals(1, overflowRuns.get()); + assertNotEquals(callerThreadId, overflowThreadId.get()); + assertEquals( + Process.THREAD_PRIORITY_BACKGROUND, + overflowPriority.get() + ); } finally { releaseWorkers.countDown(); + assertTrue(queuedWorkCompleted.await(5, TimeUnit.SECONDS)); executor.shutdownNow(); + overflowExecutor.shutdownNow(); } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt index 3cfa746aa..fdc6cda39 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt @@ -13,10 +13,12 @@ import java.util.concurrent.RejectedExecutionException @RunWith(TestRunner::class) class IterableRequestDispatcherTest { private val requestExecutor = RecordingExecutor() + private val retryExecutor = RecordingExecutor() private val callbackExecutor = RecordingExecutor() private val retryScheduler = RecordingRetryScheduler() private val dispatcher = IterableRequestDispatcher( requestExecutor, + retryExecutor, callbackExecutor, retryScheduler ) @@ -42,13 +44,14 @@ class IterableRequestDispatcherTest { fun `retry waits for the scheduler before submitting request work`() { dispatcher.retry(request(), 4, 6_000) - assertTrue(requestExecutor.tasks.isEmpty()) + assertTrue(retryExecutor.tasks.isEmpty()) assertEquals(6_000L, retryScheduler.delayMs) retryScheduler.task?.run() - assertEquals(1, requestExecutor.tasks.size) - assertTrue(requestExecutor.tasks.single() is IterableRequestTask) + assertTrue(requestExecutor.tasks.isEmpty()) + assertEquals(1, retryExecutor.tasks.size) + assertTrue(retryExecutor.tasks.single() is IterableRequestTask) } @Test @@ -59,11 +62,11 @@ class IterableRequestDispatcherTest { dispatcher.executeRetry(staleRequest, 0) - assertTrue(requestExecutor.tasks.isEmpty()) + assertTrue(retryExecutor.tasks.isEmpty()) } @Test - fun `a saturated executor fails asynchronously without running network work on caller`() { + fun `a rejecting executor fails asynchronously without running network work on caller`() { val failure = RecordingFailureHandler() val rejectingDispatcher = IterableRequestDispatcher( Executor { throw RejectedExecutionException("full") }, @@ -78,11 +81,11 @@ class IterableRequestDispatcherTest { callbackExecutor.tasks.single().run() - assertEquals("Iterable request queue is full", failure.message) + assertEquals("Iterable request executor rejected work", failure.message) } @Test - fun `a saturated response request receives a transient failure`() { + fun `a rejected response request receives a transient failure`() { var response: IterableApiResponse? = null val rejectingDispatcher = IterableRequestDispatcher( Executor { throw RejectedExecutionException("full") }, diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt index 9736b20e6..c31a6b7df 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt @@ -7,12 +7,14 @@ import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.RecordedRequest import org.json.JSONObject import org.junit.After +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger @RunWith(TestRunner::class) class IterableRequestExecutionConcurrencyTest { @@ -116,6 +118,184 @@ class IterableRequestExecutionConcurrencyTest { ) } + @Test + fun `offline immediate requests execute serially`() { + val firstRequestStarted = CountDownLatch(1) + val releaseFirstResponse = CountDownLatch(1) + val secondRequestStarted = CountDownLatch(1) + + server.dispatcher = blockingFirstRequestDispatcher( + firstRequestStarted, + releaseFirstResponse, + secondRequestStarted + ) + val firstRequestFinished = execute( + IterableRequestDispatcher.offlineImmediate(), + "api/first" + ) + + try { + assertCompleted( + firstRequestStarted, + "First offline immediate request did not reach the server" + ) + + val secondRequestFinished = execute( + IterableRequestDispatcher.offlineImmediate(), + "api/second" + ) + + assertStillPending( + secondRequestStarted, + "Second offline immediate request overtook the blocked first request" + ) + releaseFirstResponse.countDown() + assertCompleted( + secondRequestFinished, + "Second offline immediate request did not finish after the first response" + ) + } finally { + releaseFirstResponse.countDown() + } + + assertCompleted( + firstRequestFinished, + "First offline immediate request did not finish after its response was released" + ) + } + + @Test + fun `stored offline request does not wait for the immediate offline lane`() { + val immediateRequestStarted = CountDownLatch(1) + val releaseImmediateResponse = CountDownLatch(1) + val storedRequestStarted = CountDownLatch(1) + + server.dispatcher = blockingFirstRequestDispatcher( + immediateRequestStarted, + releaseImmediateResponse, + storedRequestStarted + ) + val immediateRequestFinished = execute( + IterableRequestDispatcher.offlineImmediate(), + "api/first" + ) + + try { + assertCompleted( + immediateRequestStarted, + "Offline immediate request did not reach the server" + ) + + val storedRequestFinished = execute( + IterableRequestDispatcher.offlineStored(), + "api/second" + ) + + assertCompleted( + storedRequestStarted, + "Stored offline request waited for the immediate offline lane" + ) + assertCompleted( + storedRequestFinished, + "Stored offline request did not finish while the immediate lane was blocked" + ) + } finally { + releaseImmediateResponse.countDown() + } + + assertCompleted( + immediateRequestFinished, + "Offline immediate request did not finish after its response was released" + ) + } + + @Test + fun `online server-error retries execute serially`() { + val firstRetryStarted = CountDownLatch(1) + val releaseFirstRetry = CountDownLatch(1) + val secondInitialResponse = CountDownLatch(1) + val secondRetryScheduled = CountDownLatch(1) + val secondRetryStarted = CountDownLatch(1) + val firstRequestFinished = CountDownLatch(1) + val secondRequestFinished = CountDownLatch(1) + val firstRequestCount = AtomicInteger() + val secondRequestCount = AtomicInteger() + val retryCount = AtomicInteger() + + server.dispatcher = object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse { + return when (request.path?.substringBefore("?")) { + "/api/first" -> { + if (firstRequestCount.incrementAndGet() == 1) { + MockResponse().setResponseCode(500) + } else { + firstRetryStarted.countDown() + releaseFirstRetry.await() + successfulResponse() + } + } + + "/api/second" -> { + if (secondRequestCount.incrementAndGet() == 1) { + secondInitialResponse.countDown() + MockResponse().setResponseCode(500) + } else { + secondRetryStarted.countDown() + successfulResponse() + } + } + + else -> MockResponse().setResponseCode(404) + } + } + } + val dispatcher = IterableRequestDispatcher( + IterableExecutors.request(), + IterableExecutors.requestRetry(), + Runnable::run, + { runnable, _ -> + if (retryCount.incrementAndGet() == 2) { + secondRetryScheduled.countDown() + } + runnable.run() + } + ) + + dispatcher.execute(retryingRequest("api/first", firstRequestFinished)) + assertCompleted(firstRetryStarted, "First online retry did not reach the server") + + try { + dispatcher.execute(retryingRequest("api/second", secondRequestFinished)) + assertCompleted( + secondInitialResponse, + "Second initial online request did not reach the server" + ) + assertCompleted( + secondRetryScheduled, + "Second online retry was not submitted" + ) + assertStillPending( + secondRetryStarted, + "Second online retry overtook the blocked first retry" + ) + } finally { + releaseFirstRetry.countDown() + } + + assertCompleted( + secondRetryStarted, + "Second online retry did not run after the first retry completed" + ) + assertCompleted( + firstRequestFinished, + "First online request did not finish after its retry response" + ) + assertCompleted( + secondRequestFinished, + "Second online request did not finish after its retry response" + ) + } + private fun blockingFirstRequestDispatcher( firstRequestStarted: CountDownLatch, releaseFirstResponse: CountDownLatch, @@ -156,6 +336,10 @@ class IterableRequestExecutionConcurrencyTest { assertTrue(message, latch.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) } + private fun assertStillPending(latch: CountDownLatch, message: String) { + assertFalse(message, latch.await(SERIAL_ASSERTION_MILLIS, TimeUnit.MILLISECONDS)) + } + private fun request(resourcePath: String): IterableApiRequest { return IterableApiRequest( "api-key", @@ -168,6 +352,21 @@ class IterableRequestExecutionConcurrencyTest { ) } + private fun retryingRequest( + resourcePath: String, + requestFinished: CountDownLatch + ): IterableApiRequest { + return IterableApiRequest( + "api-key", + resourcePath, + JSONObject(), + IterableApiRequest.GET, + null, + IterableHelper.SuccessHandler { requestFinished.countDown() }, + null + ) + } + private fun successfulResponse(): MockResponse { return MockResponse() .setResponseCode(200) @@ -176,5 +375,6 @@ class IterableRequestExecutionConcurrencyTest { companion object { private const val TIMEOUT_SECONDS = 5L + private const val SERIAL_ASSERTION_MILLIS = 500L } } diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java index 7987cc2b9..b2be5de5e 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java @@ -98,6 +98,37 @@ public void testRunOnTaskCreatedMakesApiRequest() throws Exception { verify(mockTaskStorage).deleteTask(any(String.class)); } + @Test + public void testStartProcessesTaskPersistedBeforeRunnerWasCreated() throws Exception { + IterableApiRequest request = new IterableApiRequest( + "apiKey", + "api/test", + new JSONObject(), + "POST", + null, + null, + null + ); + IterableTask task = new IterableTask( + "testTask", + IterableTaskType.API, + request.toJSONObject().toString() + ); + when(mockTaskStorage.getNextScheduledTask()).thenReturn(task).thenReturn(null); + when(mockActivityMonitor.isInForeground()).thenReturn(true); + when(mockNetworkConnectivityManager.isConnected()).thenReturn(true); + when(mockHealthMonitor.canProcess()).thenReturn(true); + server.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); + + taskRunner.start(); + runHandlerTasks(taskRunner); + + RecordedRequest recordedRequest = server.takeRequest(1, TimeUnit.SECONDS); + assertNotNull(recordedRequest); + assertEquals("/api/test", recordedRequest.getPath()); + verify(mockTaskStorage).deleteTask(task.id); + } + @Test public void testRunOnTaskCreatedCallsCompletionListener() throws Exception { IterableApiRequest request = new IterableApiRequest("apiKey", "api/test", new JSONObject(), "POST", null, null, null); diff --git a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java index 995faf44a..fadec60ea 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java @@ -43,10 +43,13 @@ public void tearDown() { @Test public void equalScheduledTimesUseInsertionOrderAndClaimedTasksAreSkipped() { + assertFalse(storage.hasPendingTasks()); + String firstId = storage.createTask("first", IterableTaskType.API, "{}"); String secondId = storage.createTask("second", IterableTaskType.API, "{}"); assertNotNull(firstId); assertNotNull(secondId); + assertTrue(storage.hasPendingTasks()); SQLiteDatabase database = ReflectionHelpers.getField(storage, "database"); ContentValues scheduled = new ContentValues();