diff --git a/CHANGELOG.md b/CHANGELOG.md index 22dc10702..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 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 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/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 a89a360cc..1f6cb7e6b 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. @@ -992,7 +991,10 @@ 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 //region SDK public functions @@ -1567,7 +1569,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/IterableApiClient.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java index fb331b12c..8a6cd41a8 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableApiClient.java @@ -21,10 +21,12 @@ 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; + private OfflineRequestProcessor offlineRequestProcessor; interface AuthProvider { @Nullable @@ -45,33 +47,63 @@ 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() { + private synchronized RequestProcessor getRequestProcessor() { if (requestProcessor == null) { - requestProcessor = new OnlineRequestProcessor(); + requestProcessor = new OnlineRequestProcessor(requestDispatchers.online()); } return requestProcessor; } - void setOfflineProcessingEnabled(boolean offlineMode) { - if (offlineMode && this.requestProcessor instanceof OfflineRequestProcessor) { + synchronized void setOfflineProcessingEnabled(boolean offlineMode) { + if (!offlineMode + && offlineRequestProcessor == null + && !hasPendingOfflineRequests()) { + if (!(requestProcessor instanceof OnlineRequestProcessor)) { + requestProcessor = new OnlineRequestProcessor(requestDispatchers.online()); + } return; } - if (!offlineMode && this.requestProcessor instanceof OnlineRequestProcessor) { - return; + + // 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()); } + } - if (this.requestProcessor instanceof OfflineRequestProcessor) { - ((OfflineRequestProcessor) this.requestProcessor).dispose(); + private OfflineRequestProcessor getOfflineRequestProcessor() { + if (offlineRequestProcessor == null) { + offlineRequestProcessor = new OfflineRequestProcessor( + authProvider.getContext(), + requestDispatchers.offlineImmediate(), + requestDispatchers.offlineStored() + ); } + return offlineRequestProcessor; + } - this.requestProcessor = offlineMode - ? new OfflineRequestProcessor(authProvider.getContext()) - : new OnlineRequestProcessor(); + private boolean hasPendingOfflineRequests() { + return IterableTaskStorage + .sharedInstance(authProvider.getContext()) + .hasPendingTasks(); } void getRemoteConfiguration(IterableHelper.IterableActionHandler actionHandler) { @@ -819,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 632ed7591..b53aa0b70 100644 --- a/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableExecutors.java @@ -2,24 +2,46 @@ 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.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +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; - }); + // 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; + // 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 = - 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 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, + REQUEST_OVERFLOW_EXECUTOR + ); private static final Executor MAIN_EXECUTOR = runnable -> new Handler(Looper.getMainLooper()).post(runnable); @@ -34,7 +56,80 @@ static Executor deepLink() { return DEEP_LINK_EXECUTOR; } + static Executor request() { + return REQUEST_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() { return MAIN_EXECUTOR; } + + private static Executor newSingleThreadExecutor(String threadName) { + return Executors.newSingleThreadExecutor( + runnable -> newThread(runnable, 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, + REQUEST_THREAD_KEEP_ALIVE_SECONDS, + TimeUnit.SECONDS, + new ArrayBlockingQueue<>(queueCapacity), + runnable -> newThread( + runnable, + "IterableRequestExecutor-" + REQUEST_THREAD_ID.incrementAndGet() + ), + (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; + } + + private static Thread newThread(Runnable runnable, String threadName) { + Thread thread = new Thread(() -> { + Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); + runnable.run(); + }, threadName); + thread.setDaemon(true); + return thread; + } } 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/IterableRequestDispatcher.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java new file mode 100644 index 000000000..9743a99a5 --- /dev/null +++ b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestDispatcher.java @@ -0,0 +1,199 @@ +package com.iterable.iterableapi; + +import android.os.Handler; +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 REQUEST_REJECTED_ERROR = + "Iterable request executor rejected work"; + + 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); + private static final IterableRequestDispatcher ONLINE_DISPATCHER = + sdkDispatcher( + IterableExecutors.request(), + IterableExecutors.requestRetry() + ); + private static final IterableRequestDispatcher PUSH_DISPATCHER = + sdkDispatcher(IterableExecutors.push()); + 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; + + static IterableRequestDispatcher online() { + return ONLINE_DISPATCHER; + } + + static IterableRequestDispatcher push() { + return PUSH_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; + } + + void execute(IterableApiRequest request) { + IterableRequestTask requestTask = new IterableRequestTask(request, 0, this); + try { + requestExecutor.execute(requestTask); + } catch (RejectedExecutionException e) { + IterableLogger.e(TAG, REQUEST_REJECTED_ERROR, e); + deliverResult(() -> requestTask.handleResponse(rejectedRequestResponse())); + } + } + + void executeForResponse(IterableApiRequest request, ResponseHandler responseHandler) { + try { + requestExecutor.execute(() -> responseHandler.onResponse( + IterableRequestTask.executeApiRequest(request, this) + )); + } catch (RejectedExecutionException e) { + IterableLogger.e(TAG, REQUEST_REJECTED_ERROR, e); + deliverResult(() -> responseHandler.onResponse(rejectedRequestResponse())); + } + } + + void executeRetry(IterableApiRequest request, int retryCount) { + if (request.canRetry()) { + IterableRequestTask requestTask = + new IterableRequestTask(request, retryCount, this, true); + try { + retryExecutor.execute(requestTask); + } catch (RejectedExecutionException e) { + IterableLogger.e(TAG, REQUEST_REJECTED_ERROR, e); + deliverResult(() -> { + if (request.canRetry()) { + requestTask.handleResponse(rejectedRequestResponse()); + } + }); + } + } + } + + 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 sdkDispatcher(requestExecutor, requestExecutor); + } + + private static IterableRequestDispatcher sdkDispatcher( + Executor requestExecutor, + Executor retryExecutor + ) { + return new IterableRequestDispatcher( + requestExecutor, + retryExecutor, + IterableExecutors.main(), + SDK_RETRY_SCHEDULER + ); + } + + private static IterableApiResponse rejectedRequestResponse() { + return IterableApiResponse.failure(0, null, null, REQUEST_REJECTED_ERROR); + } +} + +final class IterableRequestDispatchers { + private static final IterableRequestDispatchers SDK_DISPATCHERS = + new IterableRequestDispatchers( + IterableRequestDispatcher.online(), + IterableRequestDispatcher.push(), + IterableRequestDispatcher.offlineImmediate(), + IterableRequestDispatcher.offlineStored() + ); + + private final IterableRequestDispatcher online; + private final IterableRequestDispatcher push; + 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, + dispatcher + ); + } + + IterableRequestDispatchers( + IterableRequestDispatcher online, + IterableRequestDispatcher push, + IterableRequestDispatcher offlineImmediate, + IterableRequestDispatcher offlineStored + ) { + this.online = online; + this.push = push; + this.offlineImmediate = offlineImmediate; + this.offlineStored = offlineStored; + } + + IterableRequestDispatcher online() { + return online; + } + + IterableRequestDispatcher push() { + return push; + } + + IterableRequestDispatcher offlineImmediate() { + return offlineImmediate; + } + + IterableRequestDispatcher offlineStored() { + return offlineStored; + } +} diff --git a/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java index db2822496..f49757606 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,70 @@ 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); - } - - @WorkerThread - static IterableApiResponse executeApiRequest(IterableApiRequest iterableApiRequest) { - return executeApiRequest( - iterableApiRequest, - IterableRequestTask::retryRequestWithNewAuthToken + iterableApiRequest.legacyCallback ); + request.copyRetryStateFrom(iterableApiRequest); + requestDispatcher.executeRetry(request, 0); } + /** + * Sends the given request to Iterable using a HttpURLConnection. + * Reference - http://developer.android.com/reference/java/net/HttpURLConnection.html + */ @WorkerThread static IterableApiResponse executeApiRequest( IterableApiRequest iterableApiRequest, - IterableRequestAuthRetryHandler authRetryHandler + IterableRequestDispatcher requestDispatcher ) { IterableApiResponse apiResponse = null; String requestResult = null; @@ -215,7 +235,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 +297,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 +310,7 @@ private static void handleJwtAuthRetry( null ); } else { - requestNewAuthTokenAndRetry(iterableApiRequest, authRetryHandler); + requestNewAuthTokenAndRetry(iterableApiRequest, requestDispatcher); } } @@ -377,11 +397,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 +410,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 +417,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 +447,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 +457,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 +468,6 @@ private static void requestNewAuthTokenAndRetry( } ); } - - protected void setRetryCount(int count) { - retryCount = count; - } -} - -interface IterableRequestAuthRetryHandler { - void retryWithNewAuthToken(String newAuthToken, IterableApiRequest request); } /** @@ -486,6 +485,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 +517,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; @@ -591,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/main/java/com/iterable/iterableapi/IterableTaskRunner.java b/iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java index 90b2d2dc2..0b0e208ef 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,8 @@ class IterableTaskRunner implements IterableTaskStorage.TaskCreatedListener, Han private final HandlerThread networkThread = new HandlerThread("NetworkThread"); Handler handler; + private volatile boolean requestInFlight; + private volatile boolean disposed; enum TaskResult { SUCCESS, FAILURE, RETRY @@ -47,12 +50,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,22 +65,34 @@ 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); } + void start() { + runNow(); + } + 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(); @@ -108,11 +125,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); } @@ -129,6 +152,10 @@ public boolean handleMessage(@NonNull Message msg) { @WorkerThread private void processTasks() { + if (disposed || requestInFlight) { + return; + } + if (!activityMonitor.isInForeground()) { IterableLogger.d(TAG, "App not in foreground, skipping processing tasks"); return; @@ -140,22 +167,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 +191,78 @@ 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 (disposed || 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; + if (!taskStorage.markTaskProcessingIfAvailable(task.id)) { + runNow(); + return; + } + + 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; + taskStorage.updateIsProcessing(task.id, false); + callTaskCompletedListeners(task.id, TaskResult.RETRY, response); + finishDisposeIfNeeded(); + return; + } else if (!isPermanentFailure(response)) { + result = TaskResult.RETRY; } } - return false; + + 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) { @@ -262,7 +305,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() { @@ -271,4 +314,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..ad8c6662b 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"); } @@ -90,11 +91,23 @@ 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); } void removeDatabaseStatusListener(TaskCreatedListener listener) { + removeTaskCreatedListener(listener); + } + + void removeTaskCreatedListener(TaskCreatedListener listener) { taskCreatedListeners.remove(listener); } @@ -278,7 +291,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 +318,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 +433,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 +524,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 +585,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 ea5de7e45..629cc2aa0 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 immediateRequestDispatcher; private static final Set offlineApiSet = new HashSet<>(Arrays.asList( IterableConstants.ENDPOINT_TRACK, @@ -37,7 +37,12 @@ class OfflineRequestProcessor implements RequestProcessor { IterableConstants.ENDPOINT_TRACK_EMBEDDED_SESSION )); - OfflineRequestProcessor(Context context) { + OfflineRequestProcessor( + Context context, + IterableRequestDispatcher immediateRequestDispatcher, + IterableRequestDispatcher offlineRequestDispatcher + ) { + this.immediateRequestDispatcher = immediateRequestDispatcher; IterableNetworkConnectivityManager networkConnectivityManager = IterableNetworkConnectivityManager.sharedInstance(context); taskStorage = IterableTaskStorage.sharedInstance(context); healthMonitor = new HealthMonitor(taskStorage); @@ -46,8 +51,13 @@ class OfflineRequestProcessor implements RequestProcessor { IterableActivityMonitor.getInstance(), networkConnectivityManager, healthMonitor, - classification); - taskScheduler = new TaskScheduler(taskStorage, taskRunner); + classification, + offlineRequestDispatcher); + taskScheduler = new TaskScheduler( + taskStorage, + taskRunner, + immediateRequestDispatcher + ); // Register task runner as auth token ready listener for JWT auto-retry support try { @@ -56,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 { @@ -68,26 +78,33 @@ void dispose() { } catch (Exception e) { IterableLogger.w("OfflineRequestProcessor", "Failed to unregister auth token listener on dispose."); } + taskRunner.dispose(); } - @VisibleForTesting - OfflineRequestProcessor(TaskScheduler scheduler, IterableTaskRunner iterableTaskRunner, IterableTaskStorage storage, HealthMonitor mockHealthMonitor) { + OfflineRequestProcessor( + TaskScheduler scheduler, + IterableTaskRunner iterableTaskRunner, + IterableTaskStorage storage, + HealthMonitor mockHealthMonitor, + IterableRequestDispatcher immediateRequestDispatcher + ) { taskRunner = iterableTaskRunner; taskScheduler = scheduler; taskStorage = storage; healthMonitor = mockHealthMonitor; + 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); - new IterableRequestTask().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); - new IterableRequestTask().execute(request); + immediateRequestDispatcher.execute(request); } @Override @@ -97,7 +114,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); + immediateRequestDispatcher.execute(request); } } @@ -116,10 +133,16 @@ 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) { + TaskScheduler( + IterableTaskStorage taskStorage, + IterableTaskRunner taskRunner, + IterableRequestDispatcher requestDispatcher + ) { this.taskStorage = taskStorage; this.taskRunner = taskRunner; + this.requestDispatcher = requestDispatcher; taskRunner.addTaskCompletedListener(this); } @@ -129,13 +152,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..16bccb425 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,37 @@ class OnlineRequestProcessor implements RequestProcessor { private static final String TAG = "OnlineRequestProcessor"; + private final IterableRequestDispatcher requestDispatcher; + + 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 +60,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..cd52c6fc3 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/BaseTest.java @@ -7,12 +7,9 @@ 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; import org.junit.runner.RunWith; -import org.robolectric.android.util.concurrent.InlineExecutorService; -import org.robolectric.shadows.ShadowPausedAsyncTask; @RunWith(TestRunner.class) public abstract class BaseTest { @@ -20,15 +17,17 @@ public abstract class BaseTest { @Rule public IterableUtilRule utilsRule = new IterableUtilRule(); - @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() { @@ -38,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/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/IterableApiClientRequestRoutingTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt new file mode 100644 index 000000000..de9ef3673 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableApiClientRequestRoutingTest.kt @@ -0,0 +1,149 @@ +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 : BaseTest() { + private lateinit var onlineExecutor: RecordingExecutor + private lateinit var pushExecutor: 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() + 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( + authProvider, + IterableRequestDispatchers( + dispatcher(onlineExecutor), + dispatcher(pushExecutor), + 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(offlineImmediateExecutor.tasks.isEmpty()) + assertTrue(offlineStoredExecutor.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(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 { + 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/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/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 e6576b06f..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; @@ -46,7 +45,6 @@ public void setUp() { 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/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..3c8258a57 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; @@ -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/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/IterableExecutorsTest.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java new file mode 100644 index 000000000..14c087977 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableExecutorsTest.java @@ -0,0 +1,95 @@ +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.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; + +@RunWith(TestRunner.class) +public class IterableExecutorsTest { + @Test + public void requestExecutorSpillsExcessWorkToBackgroundOverflowWithoutDroppingIt() + throws Exception { + ThreadPoolExecutor overflowExecutor = + IterableExecutors.newRequestOverflowExecutor(1); + ThreadPoolExecutor executor = + IterableExecutors.newRequestExecutor(2, 1, overflowExecutor); + CountDownLatch workersStarted = new CountDownLatch(2); + CountDownLatch releaseWorkers = new CountDownLatch(1); + 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 = () -> { + 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(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(); + } + } + + @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/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")); 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..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; @@ -71,7 +72,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 +279,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 +346,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 +407,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 +583,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())); @@ -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 = new IterableApi(inAppManager); + 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 = new IterableApi(inAppManager); + 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/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..fdc6cda39 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestDispatcherTest.kt @@ -0,0 +1,144 @@ +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 +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 + ) + + @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(retryExecutor.tasks.isEmpty()) + assertEquals(6_000L, retryScheduler.delayMs) + + retryScheduler.task?.run() + + assertTrue(requestExecutor.tasks.isEmpty()) + assertEquals(1, retryExecutor.tasks.size) + assertTrue(retryExecutor.tasks.single() is IterableRequestTask) + } + + @Test + fun `stale retry is not submitted`() { + val staleRequest = request().apply { + setRetryState { false } + } + + dispatcher.executeRetry(staleRequest, 0) + + assertTrue(retryExecutor.tasks.isEmpty()) + } + + @Test + fun `a rejecting 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 executor rejected work", failure.message) + } + + @Test + fun `a rejected 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", + JSONObject(), + IterableApiRequest.POST, + null, + null, + failure + ) + } + + 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 + } + } + + 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/IterableRequestExecutionConcurrencyTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt new file mode 100644 index 000000000..c31a6b7df --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestExecutionConcurrencyTest.kt @@ -0,0 +1,380 @@ +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.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 { + 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" + ) + } + + @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, + 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 assertStillPending(latch: CountDownLatch, message: String) { + assertFalse(message, latch.await(SERIAL_ASSERTION_MILLIS, TimeUnit.MILLISECONDS)) + } + + private fun request(resourcePath: String): IterableApiRequest { + return IterableApiRequest( + "api-key", + resourcePath, + JSONObject(), + IterableApiRequest.GET, + null, + null, + null + ) + } + + 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) + .setBody("{}") + } + + 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/IterableRequestTaskTest.kt b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt new file mode 100644 index 000000000..bf1a1d5e0 --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableRequestTaskTest.kt @@ -0,0 +1,184 @@ +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 +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.ArgumentMatchers.eq +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) + } + + @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)) + 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..b2be5de5e 100644 --- a/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskRunnerTest.java @@ -25,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; @@ -43,6 +44,7 @@ public class IterableTaskRunnerTest extends BaseTest { private IterableActivityMonitor mockActivityMonitor; private HealthMonitor mockHealthMonitor; private IterableNetworkConnectivityManager mockNetworkConnectivityManager; + private IterableRequestDispatcher requestDispatcher; private MockWebServer server; @Before @@ -51,13 +53,27 @@ 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() + ); + when(mockTaskStorage.markTaskProcessingIfAvailable(anyString())).thenReturn(true); + taskRunner = new IterableTaskRunner( + mockTaskStorage, + mockActivityMonitor, + mockNetworkConnectivityManager, + mockHealthMonitor, + new ApiEndpointClassification(), + requestDispatcher + ); server = new MockWebServer(); IterableApi.overrideURLEndpointPath(server.url("").toString()); } @After public void tearDown() throws Exception { + taskRunner.dispose(); server.shutdown(); IterableTestUtils.resetIterableApi(); } @@ -82,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); @@ -172,6 +219,16 @@ public void testIfNetworkCheckedBeforeProcessingTask() throws Exception { verify(mockNetworkConnectivityManager, times(2)).isConnected(); } + @Test + public void testDisposeUnregistersEventListeners() throws Exception { + taskRunner.dispose(); + runHandlerTasks(taskRunner); + + verify(mockTaskStorage).removeTaskCreatedListener(taskRunner); + verify(mockNetworkConnectivityManager).removeNetworkListener(taskRunner); + verify(mockActivityMonitor).removeCallback(taskRunner); + } + // region Auto-Retry on JWT Failure Tests private String createJwt401ResponseBody() throws Exception { @@ -182,7 +239,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(); @@ -510,7 +567,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 +599,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 +625,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 +673,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 +716,5 @@ public void testAuthRequiredTasksResumeAfterAuthReady() throws Exception { private void runHandlerTasks(IterableTaskRunner taskRunner) throws InterruptedException { shadowOf(taskRunner.handler.getLooper()).idle(); } + } 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..fadec60ea --- /dev/null +++ b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTaskStorageTest.java @@ -0,0 +1,72 @@ +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() { + 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(); + 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/IterableTestUtils.java b/iterableapi/src/test/java/com/iterable/iterableapi/IterableTestUtils.java index 4810aaad7..07e17b74f 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,35 @@ 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. + * Tests asserting callback-driven follow-up work must idle the main looper. + */ + 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..7ab7004fa --- /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 immediateRequestDispatcher: IterableRequestDispatcher + + @Before + fun setUp() { + taskScheduler = mock(TaskScheduler::class.java) + healthMonitor = mock(HealthMonitor::class.java) + immediateRequestDispatcher = mock(IterableRequestDispatcher::class.java) + requestProcessor = OfflineRequestProcessor( + taskScheduler, + mock(IterableTaskRunner::class.java), + mock(IterableTaskStorage::class.java), + healthMonitor, + immediateRequestDispatcher + ) + } + + @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(immediateRequestDispatcher) + } + + @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(immediateRequestDispatcher).execute(requestCaptor.capture()) + return requestCaptor.value + } +} 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"); 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 + ) + } +}