From 270582a7544af68a4d3ad7d227ff01770cfba054 Mon Sep 17 00:00:00 2001 From: Laimonas Turauskas Date: Thu, 25 Jan 2024 14:23:58 -0500 Subject: [PATCH] Refactoring FormulaAndroid slightly and increasing code-coverage. --- .../instacart/formula/FormulaAndroidTest.kt | 37 +++++++++++++++ .../com/instacart/formula/FormulaAndroid.kt | 46 ++++++++----------- .../formula/android/FormulaFragment.kt | 3 +- .../android/internal/CompositeBinding.kt | 8 ++-- .../internal/FormulaFragmentDelegate.kt | 14 ++---- .../formula/compose/stopwatch/StopwatchApp.kt | 9 ++-- .../main/java/com/examples/todoapp/TodoApp.kt | 9 ++-- 7 files changed, 77 insertions(+), 49 deletions(-) create mode 100644 formula-android-tests/src/test/java/com/instacart/formula/FormulaAndroidTest.kt diff --git a/formula-android-tests/src/test/java/com/instacart/formula/FormulaAndroidTest.kt b/formula-android-tests/src/test/java/com/instacart/formula/FormulaAndroidTest.kt new file mode 100644 index 000000000..065670930 --- /dev/null +++ b/formula-android-tests/src/test/java/com/instacart/formula/FormulaAndroidTest.kt @@ -0,0 +1,37 @@ +package com.instacart.formula + +import android.app.Application +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class FormulaAndroidTest { + + @Test fun `crashes if initialized twice`() { + + try { + val result = runCatching { + val context = ApplicationProvider.getApplicationContext() + FormulaAndroid.init(context) {} + FormulaAndroid.init(context) {} + } + val error = result.exceptionOrNull()?.message + Truth.assertThat(error).isEqualTo("can only initialize the store once.") + } finally { + FormulaAndroid.reset() + } + } + + @Test fun `crashes if accessed before initialization`() { + val result = runCatching { + FormulaAndroid.onBackPressed(ActivityUpdateTest.TestActivity()) + } + val errorMessage = result.exceptionOrNull()?.message + Truth.assertThat(errorMessage).isEqualTo( + "Need to call FormulaAndroid.init() from your Application." + ) + } +} \ No newline at end of file diff --git a/formula-android/src/main/java/com/instacart/formula/FormulaAndroid.kt b/formula-android/src/main/java/com/instacart/formula/FormulaAndroid.kt index d7949053f..b135c9b6b 100644 --- a/formula-android/src/main/java/com/instacart/formula/FormulaAndroid.kt +++ b/formula-android/src/main/java/com/instacart/formula/FormulaAndroid.kt @@ -11,13 +11,13 @@ import com.instacart.formula.android.FragmentEnvironment import com.instacart.formula.android.internal.ActivityStoreFactory import com.instacart.formula.android.internal.AppManager import com.instacart.formula.android.FragmentKey -import com.instacart.formula.android.internal.FormulaFragmentDelegate import java.lang.IllegalStateException object FormulaAndroid { private var application: Application? = null private var appManager: AppManager? = null + private var fragmentEnvironment: FragmentEnvironment? = null /** * Initializes Formula Android integration. Should be called within [Application.onCreate]. @@ -40,31 +40,14 @@ object FormulaAndroid { this.application = application this.appManager = appManager - FormulaFragmentDelegate.appManager = appManager - FormulaFragmentDelegate.fragmentEnvironment = fragmentEnvironment - } - - /** - * Initializes Formula Android integration. Should be called within [Application.onCreate]. - * - * @param logger A logger for debug Formula Android events. - * @param onFragmentError A global handler for fragment errors. Override this to log the crashes. - */ - fun init( - application: Application, - logger: ((String) -> Unit)? = null, - onFragmentError: (FragmentKey, Throwable) -> Unit = { _, it -> throw it }, - activities: ActivityConfigurator.() -> Unit - ) { - val fragmentEnvironment = FragmentEnvironment(logger ?: {}, onFragmentError) - init(application, fragmentEnvironment, activities) + this.fragmentEnvironment = fragmentEnvironment } /** * Call this method in [FragmentActivity.onCreate] before calling [FragmentActivity.super.onCreate] */ fun onPreCreate(activity: FragmentActivity, savedInstance: Bundle?) { - managerOrThrow(activity).onPreCreate(activity, savedInstance) + appManagerOrThrow().onPreCreate(activity, savedInstance) } /** @@ -72,7 +55,7 @@ object FormulaAndroid { */ fun onActivityResult(activity: FragmentActivity, requestCode: Int, resultCode: Int, data: Intent?) { val result = ActivityResult(requestCode, resultCode, data) - managerOrThrow(activity).onActivityResult(activity, result) + appManagerOrThrow().onActivityResult(activity, result) } /** @@ -91,21 +74,30 @@ object FormulaAndroid { * ``` */ fun onBackPressed(activity: FragmentActivity): Boolean { - return managerOrThrow(activity).onBackPressed(activity) - } - - private fun managerOrThrow(activity: FragmentActivity): AppManager { - return appManager ?: throw IllegalStateException("call FormulaAndroid.init() from your Application: $activity") + return appManagerOrThrow().onBackPressed(activity) } /** * Used in testing to clear current store manager. */ @VisibleForTesting fun reset() { - val app = application ?: throw IllegalStateException("not initialized") + val app = ensureInitialized(application) app.unregisterActivityLifecycleCallbacks(appManager) application = null appManager = null } + + + internal fun appManagerOrThrow(): AppManager { + return ensureInitialized(appManager) + } + + internal fun fragmentEnvironment(): FragmentEnvironment { + return ensureInitialized(fragmentEnvironment) + } + + private fun ensureInitialized(variable: T?): T { + return checkNotNull(variable) { "Need to call FormulaAndroid.init() from your Application." } + } } diff --git a/formula-android/src/main/java/com/instacart/formula/android/FormulaFragment.kt b/formula-android/src/main/java/com/instacart/formula/android/FormulaFragment.kt index 82917b556..5d237b160 100644 --- a/formula-android/src/main/java/com/instacart/formula/android/FormulaFragment.kt +++ b/formula-android/src/main/java/com/instacart/formula/android/FormulaFragment.kt @@ -5,6 +5,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.fragment.app.Fragment +import com.instacart.formula.FormulaAndroid import com.instacart.formula.android.internal.FormulaFragmentDelegate import com.instacart.formula.android.internal.getFormulaFragmentId import java.lang.Exception @@ -33,7 +34,7 @@ class FormulaFragment : Fragment(), BaseFormulaFragment { } private val environment: FragmentEnvironment - get() = FormulaFragmentDelegate.fragmentEnvironment() + get() = FormulaAndroid.fragmentEnvironment() private val fragmentDelegate: FragmentEnvironment.FragmentDelegate get() = environment.fragmentDelegate diff --git a/formula-android/src/main/java/com/instacart/formula/android/internal/CompositeBinding.kt b/formula-android/src/main/java/com/instacart/formula/android/internal/CompositeBinding.kt index 8c44ac666..162614f70 100644 --- a/formula-android/src/main/java/com/instacart/formula/android/internal/CompositeBinding.kt +++ b/formula-android/src/main/java/com/instacart/formula/android/internal/CompositeBinding.kt @@ -35,10 +35,10 @@ internal class CompositeBinding( val component = state.component if (component != null) { val childInput = Input( - input.environment, - component.component, - input.activeFragments, - input.onInitializeFeature, + environment = input.environment, + component = component.component, + activeFragments = input.activeFragments, + onInitializeFeature = input.onInitializeFeature, ) bindings.forEachIndices { it.bind(context, childInput) diff --git a/formula-android/src/main/java/com/instacart/formula/android/internal/FormulaFragmentDelegate.kt b/formula-android/src/main/java/com/instacart/formula/android/internal/FormulaFragmentDelegate.kt index fd050e021..3f3f7aa4f 100644 --- a/formula-android/src/main/java/com/instacart/formula/android/internal/FormulaFragmentDelegate.kt +++ b/formula-android/src/main/java/com/instacart/formula/android/internal/FormulaFragmentDelegate.kt @@ -1,16 +1,13 @@ package com.instacart.formula.android.internal +import com.instacart.formula.FormulaAndroid +import com.instacart.formula.FormulaAndroid.fragmentEnvironment import com.instacart.formula.android.FormulaFragment -import com.instacart.formula.android.FragmentEnvironment -import com.instacart.formula.android.FragmentKey import com.instacart.formula.android.ViewFactory internal object FormulaFragmentDelegate { - var appManager: AppManager? = null - var fragmentEnvironment: FragmentEnvironment? = null - fun viewFactory(fragment: FormulaFragment): ViewFactory? { - val appManager = checkNotNull(appManager) { "FormulaAndroid.init() not called." } + val appManager = FormulaAndroid.appManagerOrThrow() val activity = fragment.activity ?: run { fragmentEnvironment().logger("FormulaFragment has no activity attached: ${fragment.getFragmentKey()}") @@ -30,9 +27,4 @@ internal object FormulaFragmentDelegate { } return viewFactory } - - - fun fragmentEnvironment(): FragmentEnvironment { - return checkNotNull(fragmentEnvironment) { "FormulaAndroid.init() not called." } - } } \ No newline at end of file diff --git a/samples/stopwatch-compose/src/main/java/com/instacart/formula/compose/stopwatch/StopwatchApp.kt b/samples/stopwatch-compose/src/main/java/com/instacart/formula/compose/stopwatch/StopwatchApp.kt index bd0fe9b32..67f744391 100644 --- a/samples/stopwatch-compose/src/main/java/com/instacart/formula/compose/stopwatch/StopwatchApp.kt +++ b/samples/stopwatch-compose/src/main/java/com/instacart/formula/compose/stopwatch/StopwatchApp.kt @@ -3,6 +3,7 @@ package com.instacart.formula.compose.stopwatch import android.app.Application import android.util.Log import com.instacart.formula.FormulaAndroid +import com.instacart.formula.android.FragmentEnvironment class StopwatchApp : Application() { @@ -11,9 +12,11 @@ class StopwatchApp : Application() { FormulaAndroid.init( application = this, - onFragmentError = { contract, error -> - Log.e("StopwatchApp", "fragment crashed", error) - }, + fragmentEnvironment = FragmentEnvironment( + onScreenError = { key, error -> + Log.e("StopwatchApp", "fragment crashed", error) + } + ), activities = { activity { store( diff --git a/samples/todoapp/src/main/java/com/examples/todoapp/TodoApp.kt b/samples/todoapp/src/main/java/com/examples/todoapp/TodoApp.kt index bb5a70a14..8429b16e3 100644 --- a/samples/todoapp/src/main/java/com/examples/todoapp/TodoApp.kt +++ b/samples/todoapp/src/main/java/com/examples/todoapp/TodoApp.kt @@ -4,6 +4,7 @@ import android.app.Application import android.util.Log import com.examples.todoapp.tasks.TaskListFeatureFactory import com.instacart.formula.FormulaAndroid +import com.instacart.formula.android.FragmentEnvironment class TodoApp : Application() { @@ -12,9 +13,11 @@ class TodoApp : Application() { FormulaAndroid.init( application = this, - onFragmentError = { contract, error -> - Log.e("TodoApp", "fragment crashed", error) - }, + fragmentEnvironment = FragmentEnvironment( + onScreenError = { _, error -> + Log.e("TodoApp", "fragment crashed", error) + } + ), activities = { activity { val component = TodoAppComponent(this)