From e08096e8f9638b0e69042a489552430215ca7170 Mon Sep 17 00:00:00 2001 From: Dipanshu Agrawal Date: Tue, 14 Jul 2026 07:42:06 -0700 Subject: [PATCH] Add overridable attachContent() to allow non-RIB roots in RibActivity Extracts the root-attach block from onCreate into a new `protected open attachContent(rootViewGroup, savedInstanceState)` method with the current behavior as its default. Subclasses can override it to host an alternative root (e.g. a Compose host) while still receiving RibActivity's lifecycle publishing and saved-state wrapping. Also drops the elvis-throw in onSaveInstanceState so a null router - now a valid state when attachContent is overridden to skip attach - no longer crashes the delegation. onDestroy and onBackPressed were already null-safe. Adds RibActivityTest cases exercising an override that skips attach: router stays null (interactor access throws), lifecycle CREATE still fires, onBackPressed falls through to super, and onSaveInstanceState no longer crashes. --- .../kotlin/com/uber/rib/core/RibActivity.kt | 21 +++-- .../com/uber/rib/core/RibActivityTest.kt | 81 +++++++++++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/libraries/rib-android/src/main/kotlin/com/uber/rib/core/RibActivity.kt b/libraries/rib-android/src/main/kotlin/com/uber/rib/core/RibActivity.kt index 98bdf87a1..a0b76d6a9 100644 --- a/libraries/rib-android/src/main/kotlin/com/uber/rib/core/RibActivity.kt +++ b/libraries/rib-android/src/main/kotlin/com/uber/rib/core/RibActivity.kt @@ -92,12 +92,20 @@ public abstract class RibActivity : _lifecycleFlow.tryEmit(createOnCreateEvent(savedInstanceState)) val wrappedBundle: Bundle? = if (savedInstanceState != null) Bundle(savedInstanceState) else null - router = createRouter(rootViewGroup) - router?.let { - it.dispatchAttach(wrappedBundle) - rootViewGroup.addView(it.view) - RibEvents.emitRouterEvent(RibEventType.ATTACHED, it, null) - } + attachContent(rootViewGroup, wrappedBundle) + } + + /** + * Attaches content to the activity after [RibActivity] has published its onCreate lifecycle + * event. The default implementation creates and attaches the root router; override to render an + * alternative root while still receiving [RibActivity]'s lifecycle publishing. + */ + protected open fun attachContent(rootViewGroup: ViewGroup, savedInstanceState: Bundle?) { + val newRouter = createRouter(rootViewGroup) + router = newRouter + newRouter.dispatchAttach(savedInstanceState) + rootViewGroup.addView(newRouter.view) + RibEvents.emitRouterEvent(RibEventType.ATTACHED, newRouter, null) } @CallSuper @@ -105,7 +113,6 @@ public abstract class RibActivity : super.onSaveInstanceState(outState) _callbacksFlow.tryEmit(createOnSaveInstanceStateEvent(outState)) router?.saveInstanceStateInternal(Bundle(outState)) - ?: throw NullPointerException("Router should not be null") } @CallSuper diff --git a/libraries/rib-android/src/test/kotlin/com/uber/rib/core/RibActivityTest.kt b/libraries/rib-android/src/test/kotlin/com/uber/rib/core/RibActivityTest.kt index 87f7ddc2f..a0941ca54 100644 --- a/libraries/rib-android/src/test/kotlin/com/uber/rib/core/RibActivityTest.kt +++ b/libraries/rib-android/src/test/kotlin/com/uber/rib/core/RibActivityTest.kt @@ -217,6 +217,53 @@ class RibActivityTest { create(ActivityLifecycleEvent.Type.CREATE) } + @Test + fun onCreate_whenAttachContentOverriddenToSkip_shouldLeaveRouterNull() { + val activity = Robolectric.buildActivity(RootlessActivity::class.java).create(null).get() + + assertThat(activity.attachContentInvocations).isEqualTo(1) + assertThat(activity.createRouterInvocations).isEqualTo(0) + // router is private; interactor throws IllegalStateException when router is null. + val error = runCatching { activity.interactor }.exceptionOrNull() + assertThat(error).isInstanceOf(IllegalStateException::class.java) + } + + @Test + fun onCreate_whenAttachContentOverriddenToSkip_shouldStillPublishLifecycle() { + val activityController = Robolectric.buildActivity(RootlessActivity::class.java) + val testSub = TestObserver() + activityController + .get() + .lifecycle() + .filter { it.type === ActivityLifecycleEvent.Type.CREATE } + .subscribe(testSub) + + activityController.create(null) + + testSub.assertValueCount(1) + assertThat(testSub.values()[0].type).isEqualTo(ActivityLifecycleEvent.Type.CREATE) + } + + @Test + fun onBackPressed_whenRouterIsNull_shouldFallThroughToSuper() { + val activity = Robolectric.buildActivity(RootlessActivity::class.java).create(null).get() + + activity.onBackPressed() + + // With router == null, the elvis-safe delegation returns null, which is != true, + // so the activity's fallback path runs (unhandled back + super.onBackPressed()). + assertThat(activity.unhandledBackPressedInvocations).isEqualTo(1) + assertThat(activity.isFinishing).isTrue() + } + + @Test + fun onSaveInstanceState_whenRouterIsNull_shouldNotCrash() { + val activityController = Robolectric.buildActivity(RootlessActivity::class.java).create(null) + + // Would previously throw NullPointerException("Router should not be null"). + activityController.saveInstanceState(android.os.Bundle()) + } + private class EmptyActivity : RibActivity() { override fun onCreate(savedInstanceState: android.os.Bundle?) { setTheme(R.style.Theme_AppCompat) @@ -240,6 +287,40 @@ class RibActivityTest { get() = interactor as TestInteractor } + /** + * Subclass that overrides [attachContent] to skip RIB attach entirely, exercising the + * non-RIB-root code path introduced by the refactor. + */ + private class RootlessActivity : RibActivity() { + var createRouterInvocations: Int = 0 + private set + + var attachContentInvocations: Int = 0 + private set + + var unhandledBackPressedInvocations: Int = 0 + private set + + override fun onCreate(savedInstanceState: android.os.Bundle?) { + setTheme(R.style.Theme_AppCompat) + super.onCreate(savedInstanceState) + } + + override fun attachContent(rootViewGroup: ViewGroup, savedInstanceState: Bundle?) { + attachContentInvocations++ + // Intentionally skip attach — the activity hosts nothing. + } + + override fun createRouter(parentViewGroup: ViewGroup): ViewRouter<*, *> { + createRouterInvocations++ + throw AssertionError("createRouter should not be called when attachContent is overridden") + } + + override fun onUnhandledBackPressed() { + unhandledBackPressedInvocations++ + } + } + private class EmptyRouter( view: FrameLayout, interactor: Interactor, *>,