Merge pull request #724 from square/ray/catchup-1.7.0

Merge main into ray/ui-update

Author: rjrjr

.buildscript/binary-validation.gradle

@@ -13,4 +13,6 @@ apiValidation {
   // Only leaf project name is valid configuration, and every project must be individually ignored.
   // See https://github.com/Kotlin/binary-compatibility-validator/issues/3
   ignoredProjects += project('samples').subprojects.collect { it.name }
+  ignoredProjects += project('benchmarks').subprojects.collect { it.name }
 }
+

RELEASING.md

@@ -6,8 +6,9 @@
 1. Make sure you're on the `main` branch (or fix branch, e.g. `v0.1-fixes`).
 
 1. Confirm that the kotlin build is green before committing any changes
+   (Note we exclude benchmarks, but you can check those too!)
    ```bash
-   ./gradlew build connectedCheck
+   ./gradlew build && ./gradlew connectedCheck -x :benchmarks:dungeon-benchmark:connectedCheck
    ```
 
 1. Update your tags.

benchmarks/README.md

@@ -1,16 +1,17 @@
-# benchmark
+# benchmarks
 
 This module contains benchmarks. Used to measure and help improve Workflow performance. These tests
 should be run on physical devices.
 
 ## dungeon-benchmark
 
-Currently this is the only benchmark included here. It is for the /samples/dungeon/app. This
-includes a macrobenchmark [test](benchmarks/dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineProfiles.kt)
-that exercises this sample app (with UiAutomator) to collect a [baseline profile](https://developer.android.com/studio/profile/baselineprofiles).
-After running this the `profile.txt` file can be taken off of the device and used directly in the
-/src/main directory of the application. This will include the profile into the APK for guided
-optimization at install time.
+This is for the /samples/dungeon/app. This includes a macrobenchmark
+[test](dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineProfiles.kt)
+that exercises this sample app (with UiAutomator) to collect
+a [baseline profile](https://developer.android.com/studio/profile/baselineprofiles). After running
+this the `profile.txt` file can be taken off of the device and used directly in the /src/main
+directory of the application. This will include the profile into the APK for guided optimization at
+install time.
 
 Better yet, the profile can be split up and added into each Workflow module's src directory so that
 it will be included with all APKs built using Workflow (including 3rd party). To do this a java
@@ -28,8 +29,23 @@ This will create an output file separated by module and then also by package as
 profile for each module can be added into its /src/main directory as `baseline-prof.txt`. Then on a
 release build this will be included with the resulting APK/binary.
 
-The other [test](benchmarks/dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineBenchmark.kt)
+The other [test](dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineBenchmark.kt)
 is used to verify the results of including the baseline profiles on the startup time. This runs the
 same scenario with and without forcing the use of the profiles. To force the use of profiles, the
 `libs.androidx.profileinstaller` dependency is included into the app under profile (dungeon in this
 case) for side-loading the profiles.
+
+## performance-poetry
+
+Module of code for performance testing related to poetry applications.
+
+### complex-poetry
+
+This application is a modification of the samples/containers/app-poetry app which uses also the
+common components in samples/containers/common and samples/containers/poetry. It modifies this
+application to pass the Workflow
+a [SimulatedPerfConfig](performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/poetry/SimulatedPerfConfig.kt).
+
+In this case we specify that the app should be more 'complex' which adds delays into each of the
+selections that are run by Worker's which then trigger a loading state that is handled by the
+[MaybeLoadingGatekeeperWorkflow](performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/poetry/MaybeLoadingGatekeeperWorkflow.kt).

benchmarks/performancepoetry/build.gradle.kts renamed to benchmarks/performance-poetry/complex-poetry/build.gradle.kts

@@ -8,7 +8,7 @@ apply(from = rootProject.file(".buildscript/android-ui-tests.gradle"))
 
 android {
   defaultConfig {
-    applicationId = "com.squareup.benchmarks.performance.poetry"
+    applicationId = "com.squareup.benchmarks.performance.complex.poetry"
   }
 }
 

benchmarks/performancepoetry/src/main/AndroidManifest.xml renamed to benchmarks/performance-poetry/complex-poetry/src/main/AndroidManifest.xml

@@ -1,15 +1,15 @@
 <?xml version="1.0" encoding="utf-8"?>
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:tools="http://schemas.android.com/tools"
-    package="com.squareup.benchmarks.performance.poetry">
+    package="com.squareup.benchmarks.performance.complex.poetry">
 
   <application
       android:allowBackup="false"
       android:label="@string/app_name"
       android:theme="@style/AppTheme"
       tools:ignore="AllowBackup,GoogleAppIndexingWarning,MissingApplicationIcon">
     <activity
-        android:name=".PerformancePoetryActivity"
+        android:name="com.squareup.benchmarks.performance.complex.poetry.PerformancePoetryActivity"
         android:exported="true">
       <intent-filter>
         <action android:name="android.intent.action.MAIN" />

benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/MaybeLoadingGatekeeperWorkflow.kt

@@ -0,0 +1,47 @@
+package com.squareup.benchmarks.performance.complex.poetry
+
+import com.squareup.benchmarks.performance.complex.poetry.views.LoaderSpinner
+import com.squareup.benchmarks.performance.complex.poetry.views.MayBeLoadingScreen
+import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
+import com.squareup.workflow1.Snapshot
+import com.squareup.workflow1.StatefulWorkflow
+import com.squareup.workflow1.Workflow
+import com.squareup.workflow1.action
+import com.squareup.workflow1.asWorker
+import com.squareup.workflow1.runningWorker
+import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
+import kotlinx.coroutines.flow.Flow
+
+typealias IsLoading = Boolean
+
+@OptIn(WorkflowUiExperimentalApi::class)
+class MaybeLoadingGatekeeperWorkflow<T : Any>(
+  private val childWithLoading: Workflow<T, Any, OverviewDetailScreen>,
+  private val childProps: T,
+  private val isLoading: Flow<Boolean>
+) : StatefulWorkflow<Unit, IsLoading, Unit, MayBeLoadingScreen>() {
+  override fun initialState(
+    props: Unit,
+    snapshot: Snapshot?
+  ): IsLoading = false
+
+  override fun render(
+    renderProps: Unit,
+    renderState: IsLoading,
+    context: RenderContext
+  ): MayBeLoadingScreen {
+    context.runningWorker(isLoading.asWorker()) {
+      action {
+        state = it
+      }
+    }
+    return MayBeLoadingScreen(
+      baseScreen = context.renderChild(childWithLoading, childProps) {
+        action { setOutput(Unit) }
+      },
+      loaders = if (renderState) listOf(LoaderSpinner) else emptyList()
+    )
+  }
+
+  override fun snapshotState(state: IsLoading): Snapshot? = null
+}

benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoemWorkflow.kt

@@ -0,0 +1,241 @@
+package com.squareup.benchmarks.performance.complex.poetry
+
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.ClearSelection
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.HandleStanzaListOutput
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.SelectNext
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.SelectPrevious
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State.ComplexCall
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State.Initializing
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State.Selected
+import com.squareup.benchmarks.performance.complex.poetry.views.BlankScreen
+import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
+import com.squareup.sample.poetry.PoemWorkflow
+import com.squareup.sample.poetry.PoemWorkflow.ClosePoem
+import com.squareup.sample.poetry.StanzaListWorkflow
+import com.squareup.sample.poetry.StanzaListWorkflow.NO_SELECTED_STANZA
+import com.squareup.sample.poetry.StanzaScreen
+import com.squareup.sample.poetry.StanzaWorkflow
+import com.squareup.sample.poetry.StanzaWorkflow.Output.CloseStanzas
+import com.squareup.sample.poetry.StanzaWorkflow.Output.ShowNextStanza
+import com.squareup.sample.poetry.StanzaWorkflow.Output.ShowPreviousStanza
+import com.squareup.sample.poetry.StanzaWorkflow.Props
+import com.squareup.sample.poetry.model.Poem
+import com.squareup.workflow1.Snapshot
+import com.squareup.workflow1.StatefulWorkflow
+import com.squareup.workflow1.Worker
+import com.squareup.workflow1.WorkflowAction
+import com.squareup.workflow1.WorkflowAction.Companion.noAction
+import com.squareup.workflow1.action
+import com.squareup.workflow1.runningWorker
+import com.squareup.workflow1.ui.Screen
+import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
+import com.squareup.workflow1.ui.container.BackStackScreen
+import com.squareup.workflow1.ui.container.toBackStackScreen
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.MutableStateFlow
+
+/**
+ * Version of [PoemWorkflow] that takes in a [SimulatedPerfConfig] to control the performance
+ * behavior of the Workflow.
+ *
+ * @param [simulatedPerfConfig] specifies whether to make the Workflow more 'complex' by
+ * introducing some asynchronous delays. See [SimulatedPerfConfig] for more details.
+ *
+ * @param [isLoading] will be set to true while this workflow is 'loading'. This is so that another
+ * component, such as a [MaybeLoadingGatekeeperWorkflow] can overlay the screen with a visual
+ * loading state. N.B. that whether or not this is loading could be included in the
+ * RenderingT if the interface [PoemWorkflow] had been left more flexible.
+ *
+ * ** Also note that raw mutable state sharing like this will almost always be a smell. It would
+ * be better to inject an interface of a 'Loading' service that could trigger this and likely
+ * break ties/conflicts with a token in the start/stop requests. We leave that complexity out
+ * here. **
+ */
+class PerformancePoemWorkflow(
+  private val simulatedPerfConfig: SimulatedPerfConfig = SimulatedPerfConfig.NO_SIMULATED_PERF,
+  private val isLoading: MutableStateFlow<Boolean>
+) : PoemWorkflow, StatefulWorkflow<Poem, State, ClosePoem, OverviewDetailScreen>() {
+
+  sealed class State {
+    // N.B. This state is a smell. We include it to be able to mimic smells
+    // we encounter in real life. Best practice would be to fold it
+    // into [Selected(NO_SELECTED_STANZA)] at the very least.
+    object Initializing : State()
+    data class ComplexCall(
+      val payload: Int
+    ) : State()
+
+    data class Selected(val stanzaIndex: Int) : State()
+  }
+
+  override fun initialState(
+    props: Poem,
+    snapshot: Snapshot?
+  ): State {
+    return if (simulatedPerfConfig.useInitializingState) Initializing else Selected(
+      NO_SELECTED_STANZA
+    )
+  }
+
+  @OptIn(WorkflowUiExperimentalApi::class)
+  override fun render(
+    renderProps: Poem,
+    renderState: State,
+    context: RenderContext
+  ): OverviewDetailScreen {
+    return when (renderState) {
+      Initializing -> {
+        // Again, then entire `Initializing` state is a smell, which is most obvious from the
+        // use of `Worker.from { Unit }`. A Worker doing no work and only shuttling the state
+        // along is usually the sign you have an extraneous state that can be collapsed!
+        // Don't try this at home.
+        context.runningWorker(Worker.from { Unit }, "initializing") {
+        isLoading.value = true
+        action {
+          isLoading.value = false
+          state = Selected(NO_SELECTED_STANZA)
+        }
+      }
+        OverviewDetailScreen(overviewRendering = BackStackScreen(BlankScreen))
+      }
+      else -> {
+        val (stanzaIndex, currentStateIsLoading) = when (renderState) {
+          is ComplexCall -> Pair(renderState.payload, true)
+          is Selected -> Pair(renderState.stanzaIndex, false)
+          Initializing -> throw IllegalStateException("No longer initializing.")
+        }
+
+        if (currentStateIsLoading) {
+          context.runningWorker(
+            Worker.from {
+              isLoading.value = true
+              delay(simulatedPerfConfig.complexityDelay)
+              // No Output for Worker is necessary because the selected index
+              // is already in the state.
+            }
+          ) {
+            action {
+              isLoading.value = false
+              (state as? ComplexCall)?.let { currentState ->
+                state = Selected(currentState.payload)
+              }
+            }
+          }
+        }
+
+        val previousStanzas: List<StanzaScreen> =
+          if (stanzaIndex == NO_SELECTED_STANZA) emptyList()
+          else renderProps.stanzas.subList(0, stanzaIndex)
+            .mapIndexed { index, _ ->
+              context.renderChild(StanzaWorkflow, Props(renderProps, index), "$index") {
+                noAction()
+              }
+            }
+
+        val visibleStanza =
+          if (stanzaIndex == NO_SELECTED_STANZA) {
+            null
+          } else {
+            context.renderChild(
+              StanzaWorkflow, Props(renderProps, stanzaIndex), "$stanzaIndex"
+            ) {
+              when (it) {
+                CloseStanzas -> ClearSelection(simulatedPerfConfig)
+                ShowPreviousStanza -> SelectPrevious(simulatedPerfConfig)
+                ShowNextStanza -> SelectNext(simulatedPerfConfig)
+              }
+            }
+          }
+
+        val stackedStanzas = visibleStanza?.let {
+          (previousStanzas + visibleStanza).toBackStackScreen<Screen>()
+        }
+
+        val stanzaListOverview =
+          context.renderChild(
+            StanzaListWorkflow,
+            renderProps
+          ) { selected ->
+            HandleStanzaListOutput(simulatedPerfConfig, selected)
+          }
+            .copy(selection = stanzaIndex)
+
+        stackedStanzas
+          ?.let {
+            OverviewDetailScreen(
+              overviewRendering = BackStackScreen(stanzaListOverview),
+              detailRendering = it
+            )
+          } ?: OverviewDetailScreen(
+          overviewRendering = BackStackScreen(stanzaListOverview),
+          selectDefault = {
+            context.actionSink.send(HandleStanzaListOutput(simulatedPerfConfig, 0))
+          }
+        )
+      }
+    }
+  }
+
+  override fun snapshotState(state: State): Snapshot? = null
+
+  internal sealed class Action : WorkflowAction<Poem, State, ClosePoem>() {
+    abstract val simulatedPerfConfig: SimulatedPerfConfig
+
+    class ClearSelection(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
+    class SelectPrevious(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
+    class SelectNext(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
+    class HandleStanzaListOutput(
+      override val simulatedPerfConfig: SimulatedPerfConfig,
+      val selection: Int
+    ) : Action()
+
+    class ExitPoem(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
+
+    override fun Updater.apply() {
+      val currentIndex: Int = when (val solidState = state) {
+        is ComplexCall -> solidState.payload
+        Initializing -> NO_SELECTED_STANZA
+        is Selected -> solidState.stanzaIndex
+      }
+      when (this@Action) {
+        is ClearSelection ->
+          state =
+            if (simulatedPerfConfig.isComplex) {
+              ComplexCall(NO_SELECTED_STANZA)
+            } else {
+              Selected(
+                NO_SELECTED_STANZA
+              )
+            }
+        is SelectPrevious ->
+          state =
+            if (simulatedPerfConfig.isComplex) {
+              ComplexCall(currentIndex - 1)
+            } else {
+              Selected(
+                currentIndex - 1
+              )
+            }
+        is SelectNext ->
+          state =
+            if (simulatedPerfConfig.isComplex) {
+              ComplexCall(currentIndex + 1)
+            } else {
+              Selected(
+                currentIndex + 1
+              )
+            }
+        is HandleStanzaListOutput -> {
+          if (selection == NO_SELECTED_STANZA) setOutput(ClosePoem)
+          state = if (simulatedPerfConfig.isComplex) {
+            ComplexCall(selection)
+          } else {
+            Selected(selection)
+          }
+        }
+        is ExitPoem -> setOutput(ClosePoem)
+      }
+    }
+  }
+}