diff --git a/CHANGELOG.md b/CHANGELOG.md index f244d4cc1..771a9fca4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ - Fixed an Android crash when marking an in-app message as read after that message was no longer in the native queue. - `setReadForMessage` logs and returns when the message id is missing, matching the iOS bridge. The Android bridge was passing that missing message into `IterableInAppManager.setRead`, which threw a `NullPointerException` and killed the app process. +- Fixed the Android module failing to compile against its pinned native Android SDK 3.10.1 in 3.2.0. The bridge now implements `onEmbeddedMessagingSyncSucceeded` and `onEmbeddedMessagingSyncFailed` from `IterableEmbeddedUpdateHandler` as no-ops, matching the native defaults. +- Fixed an Android crash in `Iterable.inAppManager.showMessage` (`IllegalStateException: Method addObserver must be called on the main thread`), present since 3.2.0. + - The bridge now displays the message on the main thread, matching `setAutoDisplayPaused` and the iOS SDK. It was calling the native in-app displayer on the React Native native-modules thread, which crashes when the current activity is not a `FragmentActivity` (for example a Jetpack Compose `ComponentActivity`). + - The promise now rejects when the message id is no longer in the native queue instead of crashing with a `NullPointerException`. + - Dismissing a message by tapping outside it now resolves the promise with `null` instead of crashing on `url.toString()`. ## 3.2.0 diff --git a/android/build.gradle b/android/build.gradle index 623d9e5fc..663f80532 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -107,5 +107,10 @@ dependencies { implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" api "com.iterable:iterableapi:3.10.1" // api project(":iterableapi") // links to local android SDK repo rather than by release + + testImplementation "junit:junit:4.13.2" + testImplementation "org.mockito:mockito-core:4.8.0" + testImplementation "org.mockito:mockito-inline:4.8.0" + testImplementation "org.robolectric:robolectric:4.14.1" } diff --git a/android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java b/android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java index 3e266b224..c3396295d 100644 --- a/android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java +++ b/android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java @@ -286,15 +286,26 @@ public void updateSubscriptions(@Nullable ReadableArray emailListIds, @Nullable ); } - public void showMessage(String messageId, boolean consume, final Promise promise) { - if (messageId == null || messageId == "") { + public void showMessage(String messageId, final boolean consume, final Promise promise) { + if (messageId == null || messageId.isEmpty()) { promise.reject("", "messageId is null or empty"); return; } - IterableApi.getInstance().getInAppManager().showMessage(RNIterableInternal.getMessageById(messageId), consume, new IterableHelper.IterableUrlCallback() { + final IterableInAppMessage message = RNIterableInternal.getMessageById(messageId); + if (message == null) { + promise.reject("", "Could not find message with id: " + messageId); + return; + } + // In-app display attaches lifecycle observers and fragments, which AndroidX requires on the main thread. + UiThreadUtil.runOnUiThread(new Runnable() { @Override - public void execute(@Nullable Uri url) { - promise.resolve(url.toString()); + public void run() { + IterableApi.getInstance().getInAppManager().showMessage(message, consume, new IterableHelper.IterableUrlCallback() { + @Override + public void execute(@Nullable Uri url) { + promise.resolve(url != null ? url.toString() : null); + } + }); } }); } @@ -729,6 +740,16 @@ public void onEmbeddedMessagingDisabled() { IterableLogger.d(TAG, "onEmbeddedMessagingDisabled"); sendEvent(EventName.handleEmbeddedMessagingDisabledCalled.name(), null); } + + @Override + public void onEmbeddedMessagingSyncSucceeded() { + IterableLogger.d(TAG, "onEmbeddedMessagingSyncSucceeded"); + } + + @Override + public void onEmbeddedMessagingSyncFailed(@Nullable String reason) { + IterableLogger.d(TAG, "onEmbeddedMessagingSyncFailed: " + reason); + } // --------------------------------------------------------------------------------------- // endregion diff --git a/android/src/test/java/com/iterable/iterableapi/RNIterableAPIModuleImplShowMessageTest.java b/android/src/test/java/com/iterable/iterableapi/RNIterableAPIModuleImplShowMessageTest.java new file mode 100644 index 000000000..5f0fd1708 --- /dev/null +++ b/android/src/test/java/com/iterable/iterableapi/RNIterableAPIModuleImplShowMessageTest.java @@ -0,0 +1,126 @@ +package com.iterable.iterableapi; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.robolectric.Shadows.shadowOf; + +import android.net.Uri; +import android.os.Looper; + +import com.facebook.react.bridge.Promise; +import com.facebook.react.bridge.ReactApplicationContext; +import com.iterable.reactnative.RNIterableAPIModuleImpl; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import java.util.concurrent.atomic.AtomicReference; + +@RunWith(RobolectricTestRunner.class) +@Config(sdk = 34) +public class RNIterableAPIModuleImplShowMessageTest { + + private static final String MESSAGE_ID = "message-id"; + + private IterableApi originalApi; + private IterableInAppManager inAppManager; + private IterableInAppMessage message; + private Promise promise; + private RNIterableAPIModuleImpl module; + + @Before + public void setUp() { + originalApi = IterableApi.sharedInstance; + IterableApi api = mock(IterableApi.class); + inAppManager = mock(IterableInAppManager.class); + message = mock(IterableInAppMessage.class); + promise = mock(Promise.class); + when(api.getInAppManager()).thenReturn(inAppManager); + when(inAppManager.getMessageById(MESSAGE_ID)).thenReturn(message); + IterableApi.sharedInstance = api; + module = new RNIterableAPIModuleImpl(mock(ReactApplicationContext.class)); + } + + @After + public void tearDown() { + IterableApi.sharedInstance = originalApi; + } + + @Test + public void showMessageFromBridgeThreadDisplaysOnMainThread() throws InterruptedException { + final AtomicReference displayThread = new AtomicReference<>(); + doAnswer(invocation -> { + displayThread.set(Thread.currentThread()); + return null; + }).when(inAppManager).showMessage(eq(message), eq(true), any(IterableHelper.IterableUrlCallback.class)); + + Thread bridgeThread = new Thread(() -> module.showMessage(MESSAGE_ID, true, promise)); + bridgeThread.start(); + bridgeThread.join(); + + assertNull(displayThread.get()); + + shadowOf(Looper.getMainLooper()).idle(); + + assertSame(Looper.getMainLooper().getThread(), displayThread.get()); + } + + @Test + public void showMessageRejectsWhenMessageIsMissing() { + module.showMessage("missing", true, promise); + shadowOf(Looper.getMainLooper()).idle(); + + verify(promise).reject("", "Could not find message with id: missing"); + verify(inAppManager, never()).showMessage(any(IterableInAppMessage.class), anyBoolean(), any(IterableHelper.IterableUrlCallback.class)); + } + + @Test + public void showMessageRejectsWhenMessageIdIsEmpty() { + module.showMessage(new String(""), true, promise); + shadowOf(Looper.getMainLooper()).idle(); + + verify(promise).reject("", "messageId is null or empty"); + verify(inAppManager, never()).showMessage(any(IterableInAppMessage.class), anyBoolean(), any(IterableHelper.IterableUrlCallback.class)); + } + + @Test + public void showMessageResolvesClickedUrl() { + stubClickWith(Uri.parse("https://iterable.com")); + + module.showMessage(MESSAGE_ID, false, promise); + shadowOf(Looper.getMainLooper()).idle(); + + verify(promise).resolve("https://iterable.com"); + } + + @Test + public void showMessageResolvesNullWhenClosedWithoutUrl() { + stubClickWith(null); + + module.showMessage(MESSAGE_ID, false, promise); + shadowOf(Looper.getMainLooper()).idle(); + + verify(promise).resolve(isNull()); + } + + private void stubClickWith(final Uri url) { + doAnswer(invocation -> { + IterableHelper.IterableUrlCallback callback = invocation.getArgument(2); + callback.execute(url); + return null; + }).when(inAppManager).showMessage(eq(message), eq(false), any(IterableHelper.IterableUrlCallback.class)); + } +} diff --git a/package.json b/package.json index 3ffca9aa5..a4067b684 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "!android/gradlew", "!android/gradlew.bat", "!android/local.properties", + "!android/src/test", "!**/*.test.{js,ts,tsx}", "!**/__tests__", "!**/__fixtures__",