Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove RxJava2 dependency from Workflow Android UI integration. #1150

Merged
merged 1 commit into from
May 11, 2020
Merged

Remove RxJava2 dependency from Workflow Android UI integration. #1150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions kotlin/buildSrc/src/main/java/Dependencies.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ object Dependencies {
const val gridlayout = "androidx.gridlayout:gridlayout:1.0.0"

object Lifecycle {
const val ktx = "androidx.lifecycle:lifecycle-runtime-ktx:2.2.0"
const val reactivestreams = "androidx.lifecycle:lifecycle-reactivestreams-ktx:2.2.0"
}

Expand Down
1 change: 1 addition & 0 deletions kotlin/samples/tictactoe/app/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ dependencies {
implementation(project(":workflow-tracing"))

implementation(Dependencies.AndroidX.constraint_layout)
implementation(Dependencies.AndroidX.Lifecycle.ktx)
implementation(Dependencies.okio)
implementation(Dependencies.rxandroid2)
implementation(Dependencies.Test.AndroidX.Espresso.idlingResource)
Expand Down
17 changes: 8 additions & 9 deletions kotlin/samples/tictactoe/app/src/main/java/com/squareup/sample/mainactivity/MainActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package com.squareup.sample.mainactivity

import android.os.Bundle
import androidx.appcompat.app.AppCompatActivity
import androidx.lifecycle.lifecycleScope
import androidx.test.espresso.IdlingResource
import com.squareup.sample.authworkflow.AuthViewFactories
import com.squareup.sample.container.SampleContainers
Expand All @@ -29,12 +30,11 @@ import com.squareup.workflow.ui.backstack.BackStackContainer
import com.squareup.workflow.ui.modal.AlertContainer
import com.squareup.workflow.ui.plus
import com.squareup.workflow.ui.setContentWorkflow
import io.reactivex.disposables.Disposables
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.launch
import timber.log.Timber

class MainActivity : AppCompatActivity() {
private var loggingSub = Disposables.disposed()

private lateinit var component: MainComponent

/** Exposed for use by espresso tests. */
Expand Down Expand Up @@ -67,16 +67,15 @@ class MainActivity : AppCompatActivity() {
onResult = { finish() }
)

loggingSub = workflowRunner.renderings.subscribe { Timber.d("rendering: %s", it) }
lifecycleScope.launch {
workflowRunner.renderings.collect {
Timber.d("rendering: %s", it)
}
}
}

override fun onRetainCustomNonConfigurationInstance(): Any = component

override fun onDestroy() {
loggingSub.dispose()
super.onDestroy()
}

private companion object {
val viewRegistry = SampleContainers +
AuthViewFactories +
Expand Down
8 changes: 4 additions & 4 deletions kotlin/workflow-ui/core-android/api/core-android.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,14 @@ public abstract class com/squareup/workflow/ui/WorkflowFragment : androidx/fragm
public final class com/squareup/workflow/ui/WorkflowLayout : android/widget/FrameLayout {
public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V
public synthetic fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun start (Lio/reactivex/Observable;Lcom/squareup/workflow/ui/ViewEnvironment;)V
public final fun start (Lio/reactivex/Observable;Lcom/squareup/workflow/ui/ViewRegistry;)V
public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow/ui/ViewEnvironment;)V
public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow/ui/ViewRegistry;)V
}

public abstract interface class com/squareup/workflow/ui/WorkflowRunner {
public static final field Companion Lcom/squareup/workflow/ui/WorkflowRunner$Companion;
public abstract fun getRenderings ()Lio/reactivex/Observable;
public abstract fun getResult ()Lio/reactivex/Maybe;
public abstract fun awaitResult (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public abstract fun getRenderings ()Lkotlinx/coroutines/flow/Flow;
}

public final class com/squareup/workflow/ui/WorkflowRunner$Companion {
Expand Down
4 changes: 1 addition & 3 deletions kotlin/workflow-ui/core-android/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,13 @@ dependencies {

api(Dependencies.AndroidX.transition)
api(Dependencies.Kotlin.Stdlib.jdk6)
api(Dependencies.RxJava2.rxjava2)

implementation(Dependencies.AndroidX.activity)
implementation(Dependencies.AndroidX.fragment)
implementation(Dependencies.AndroidX.Lifecycle.reactivestreams)
implementation(Dependencies.AndroidX.Lifecycle.ktx)
implementation(Dependencies.AndroidX.savedstate)
implementation(Dependencies.Kotlin.Coroutines.android)
implementation(Dependencies.Kotlin.Coroutines.core)
implementation(Dependencies.Kotlin.Coroutines.rx2)

testImplementation(Dependencies.Test.junit)
testImplementation(Dependencies.Test.truth)
Expand Down
3 changes: 1 addition & 2 deletions kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/WorkflowFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ import android.view.ViewGroup
import androidx.fragment.app.Fragment
import com.squareup.workflow.Workflow
import com.squareup.workflow.ui.WorkflowRunner.Config
import io.reactivex.Flowable

/**
* A [Fragment] that can run a workflow. Subclasses implement [onCreateWorkflow]
* to configure themselves with a [Workflow], [ViewRegistry] and [inputs][Flowable].
* to configure themselves with a [Workflow], [ViewRegistry] and [inputs][Config.props].
zach-klippenstein marked this conversation as resolved.
Show resolved Hide resolved
*
* For a workflow with no inputs, or a static configuration, that's as simple as:
*
Expand Down
25 changes: 17 additions & 8 deletions kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/WorkflowLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ import android.view.View
import android.view.ViewGroup
import android.view.ViewGroup.LayoutParams.MATCH_PARENT
import android.widget.FrameLayout
import io.reactivex.Observable
import io.reactivex.disposables.SerialDisposable
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach

/**
* A view that can be driven by a [WorkflowRunner]. In most cases you'll use
Expand All @@ -49,7 +54,7 @@ class WorkflowLayout(
* making that view the only child of this one.
*/
fun start(
renderings: Observable<out Any>,
renderings: Flow<Any>,
registry: ViewRegistry
) {
start(renderings, ViewEnvironment(registry))
Expand All @@ -61,7 +66,7 @@ class WorkflowLayout(
* making that view the only child of this one.
*/
fun start(
renderings: Observable<out Any>,
renderings: Flow<Any>,
environment: ViewEnvironment
) {
val hintsWithDefaults = environment.withDefaultViewFactories()
Expand Down Expand Up @@ -133,18 +138,22 @@ class WorkflowLayout(
* Subscribes [update] to [source] only while this [View] is attached to a window.
*/
private fun <S : Any> View.takeWhileAttached(
source: Observable<S>,
source: Flow<S>,
update: (S) -> Unit
) {
val listener = object : OnAttachStateChangeListener {
var sub = SerialDisposable()
val scope = CoroutineScope(Dispatchers.Main.immediate)
var job: Job? = null

@OptIn(ExperimentalCoroutinesApi::class)
override fun onViewAttachedToWindow(v: View?) {
sub.replace(source.subscribe { screen -> update(screen) })
job = source.onEach { screen -> update(screen) }
.launchIn(scope)
}

override fun onViewDetachedFromWindow(v: View?) {
sub.dispose()
job?.cancel()
job = null
}
}

Expand Down
30 changes: 14 additions & 16 deletions kotlin/workflow-ui/core-android/src/main/java/com/squareup/workflow/ui/WorkflowRunner.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@ package com.squareup.workflow.ui

import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentActivity
import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.toLiveData
import androidx.lifecycle.lifecycleScope
import com.squareup.workflow.Workflow
import com.squareup.workflow.diagnostic.WorkflowDiagnosticListener
import com.squareup.workflow.ui.WorkflowRunner.Config
import io.reactivex.Maybe
import io.reactivex.Observable
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.Flow
Expand All @@ -39,21 +36,22 @@ import kotlinx.coroutines.flow.flowOf
* or subclass [WorkflowFragment] rather than instantiate a [WorkflowRunner] directly.
*/
interface WorkflowRunner<out OutputT : Any> {

/**
* A stream of the rendering values emitted by the running [Workflow].
*/
val renderings: Flow<Any>

/**
* Provides the first (and only) [OutputT] value emitted by the workflow, or
* nothing if it is canceled before emitting.
* Returns the first (and only) [OutputT] value emitted by the workflow. Throws the cancellation
* exception if the workflow was cancelled before emitting.
*
* The output of the root workflow is treated as a result code, handy for use
* as a sign that the host Activity or Fragment should be finished. Thus, once
* a value is emitted the workflow is ended its output value is reported through
* a value is emitted the workflow is ended and its output value is reported through
* this field.
*/
val result: Maybe<out OutputT>

/**
* A stream of the rendering values emitted by the running [Workflow].
*/
val renderings: Observable<out Any>
suspend fun awaitResult(): OutputT

/**
* @param diagnosticListener If non-null, will receive all diagnostic events from the workflow
Expand Down Expand Up @@ -161,9 +159,9 @@ fun <PropsT, OutputT : Any> FragmentActivity.setContentWorkflow(
start(runner.renderings, viewEnvironment)
}

runner.result.toFlowable()
.toLiveData()
.observe(this, Observer { result -> onResult(result) })
lifecycleScope.launchWhenStarted {
onResult(runner.awaitResult())
}

this.setContentView(layout)

Expand Down
34 changes: 20 additions & 14 deletions ...orkflow-ui/core-android/src/main/java/com/squareup/workflow/ui/WorkflowRunnerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,23 @@ import com.squareup.workflow.Snapshot
import com.squareup.workflow.WorkflowSession
import com.squareup.workflow.launchWorkflowIn
import com.squareup.workflow.ui.WorkflowRunner.Config
import io.reactivex.Maybe
import io.reactivex.Observable
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.rx2.asObservable
import kotlinx.coroutines.launch
import org.jetbrains.annotations.TestOnly
import java.util.concurrent.CancellationException

internal class WorkflowRunnerViewModel<OutputT : Any>(
private val scope: CoroutineScope,
session: WorkflowSession<OutputT, Any>
private val session: WorkflowSession<OutputT, Any>
) : ViewModel(), WorkflowRunner<OutputT>, SavedStateProvider {

internal class Factory<PropsT, OutputT : Any>(
Expand All @@ -64,28 +66,32 @@ internal class WorkflowRunnerViewModel<OutputT : Any>(
}
}

@Suppress("EXPERIMENTAL_API_USAGE")
override val result: Maybe<out OutputT> = session.outputs.asObservable()
.firstElement()
.doAfterTerminate {
scope.cancel(CancellationException("WorkflowRunnerViewModel delivered result"))
}
.cache()
private val result = scope.async {
session.outputs.first()
}

override suspend fun awaitResult(): OutputT = result.await()

init {
@Suppress("EXPERIMENTAL_API_USAGE")
session.renderingsAndSnapshots
.map { it.snapshot }
.onEach { lastSnapshot = it }
.launchIn(scope)

// Cancel the entire workflow runtime after the first output is emitted.
// Use the Unconfined dispatcher to ensure the cancellation happens as immediately as possible.
scope.launch(Dispatchers.Unconfined) {
result.join()
scope.cancel(CancellationException("WorkflowRunnerViewModel delivered result"))
}
Comment on lines +69 to +87
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjrjr FYI I just reworked this a bit. The result deferred is now created using async, which is much more straightforward and means we don't have to manually propagate results and exceptions. The cancel-on-output logic is now responsible only for that, which I think makes it easier to read.

}

private var lastSnapshot: Snapshot = Snapshot.EMPTY

@OptIn(ExperimentalCoroutinesApi::class)
override val renderings: Observable<out Any> = session.renderingsAndSnapshots
override val renderings: Flow<Any> = session.renderingsAndSnapshots
.map { it.rendering }
.asObservable()

override fun onCleared() {
scope.cancel(CancellationException("WorkflowRunnerViewModel cleared."))
Expand Down