Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion workflow-core/api/workflow-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public final class com/squareup/workflow/WorkflowActionKt {
public static final fun action (Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/WorkflowAction;
public static final fun action (Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/WorkflowAction;
public static synthetic fun action$default (Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow/WorkflowAction;
public static final fun applyTo (Lcom/squareup/workflow/WorkflowAction;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Lkotlin/Pair;
public static final fun applyTo (Lcom/squareup/workflow/WorkflowAction;Ljava/lang/Object;)Lkotlin/Pair;
}

public final class com/squareup/workflow/WorkflowIdentifier {
Expand All @@ -245,3 +245,11 @@ public final class com/squareup/workflow/WorkflowKt {
public static final fun mapRendering (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/Workflow;
}

public final class com/squareup/workflow/WorkflowOutput {
public fun <init> (Ljava/lang/Object;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getValue ()Ljava/lang/Object;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,16 @@ interface WorkflowAction<StateT, out OutputT> {
* @param nextState the state that the workflow should move to. Default is the current state.
*/
class Updater<S, in O>(var nextState: S) {
private var output: @UnsafeVariance O? = null
private var isOutputSet = false
internal var output: WorkflowOutput<@UnsafeVariance O>? = null
private set

/**
* Sets the value the workflow will emit as output when this action is applied.
* If this method is not called, there will be no output.
*/
fun setOutput(output: O) {
this.output = output
isOutputSet = true
this.output = WorkflowOutput(output)
Copy link
Collaborator

Choose a reason for hiding this comment

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

OMG so much better.

}

@Suppress("UNCHECKED_CAST")
internal fun <T : Any> mapOutput(mapper: (@UnsafeVariance O) -> T): T? =
if (isOutputSet) mapper(output as O) else null
}

/**
Expand Down Expand Up @@ -211,19 +206,25 @@ inline fun <StateT, OutputT> action(
override fun toString(): String = "WorkflowAction(${name()})@${hashCode()}"
}

/**
* Applies this [WorkflowAction] to [state]. If the action sets an output, the output will be
* transformed by [mapOutput], and then both the new state and the transformed output will be
* returned.
*
* If the action sets the output multiple times, only the last one will be used.
*/
fun <StateT, OutputT, T : Any> WorkflowAction<StateT, OutputT>.applyTo(
state: StateT,
mapOutput: (OutputT) -> T
): Pair<StateT, T?> {
/** Applies this [WorkflowAction] to [state]. */
@ExperimentalWorkflowApi
fun <StateT, OutputT> WorkflowAction<StateT, OutputT>.applyTo(
state: StateT
): Pair<StateT, WorkflowOutput<OutputT>?> {
val updater = Updater<StateT, OutputT>(state)
updater.apply()
val output = updater.mapOutput(mapOutput)
return Pair(updater.nextState, output)
return Pair(updater.nextState, updater.output)
}

/** Wrapper around a potentially-nullable [OutputT] value. */
class WorkflowOutput<out OutputT>(val value: OutputT) {
override fun toString(): String = "WorkflowOutput($value)"

override fun equals(other: Any?): Boolean = when {
this === other -> true
other !is WorkflowOutput<*> -> false
else -> value == other.value
}

override fun hashCode(): Int = value.hashCode()
}
23 changes: 8 additions & 15 deletions workflow-core/src/test/java/com/squareup/workflow/SinkTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import kotlinx.coroutines.test.runBlockingTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
import kotlin.test.fail
Expand Down Expand Up @@ -52,11 +51,9 @@ class SinkTest {
assertEquals(1, sink.actions.size)
sink.actions.removeFirst()
.let { action ->
val (newState, output) = action.applyTo("state") {
assertEquals("output: 1", it)
}
val (newState, output) = action.applyTo("state")
assertEquals("state 1", newState)
assertNotNull(output)
assertEquals("output: 1", output?.value)
}
assertTrue(sink.actions.isEmpty())

Expand All @@ -65,11 +62,9 @@ class SinkTest {
assertEquals(1, sink.actions.size)
sink.actions.removeFirst()
.let { action ->
val (newState, output) = action.applyTo("state") {
assertEquals("output: 2", it)
}
val (newState, output) = action.applyTo("state")
assertEquals("state 2", newState)
assertNotNull(output)
assertEquals("output: 2", output?.value)
}

collector.cancel()
Expand All @@ -89,12 +84,10 @@ class SinkTest {
advanceUntilIdle()

val enqueuedAction = sink.actions.removeFirst()
val (newState, output) = enqueuedAction.applyTo("state") {
assertEquals("output", it)
}
val (newState, output) = enqueuedAction.applyTo("state")
assertEquals(1, applications)
assertEquals("state applied", newState)
assertNotNull(output)
assertEquals("output", output?.value)
}
}

Expand All @@ -114,7 +107,7 @@ class SinkTest {

val enqueuedAction = sink.actions.removeFirst()
pauseDispatcher()
enqueuedAction.applyTo("state") {}
enqueuedAction.applyTo("state")

assertFalse(resumed)
resumeDispatcher()
Expand All @@ -138,7 +131,7 @@ class SinkTest {
val enqueuedAction = sink.actions.removeFirst()
sendJob.cancel()
advanceUntilIdle()
val (newState, output) = enqueuedAction.applyTo("ignored") {}
val (newState, output) = enqueuedAction.applyTo("ignored")

assertFalse(applied)
assertEquals("ignored", newState)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull

@OptIn(ExperimentalWorkflowApi::class)
class WorkflowActionTest {

@Test fun `applyTo works when no output is set`() {
Expand All @@ -29,7 +30,7 @@ class WorkflowActionTest {
nextState = "nextState: $nextState"
}
}
val (nextState, output) = action.applyTo("state", ::OutputHolder)
val (nextState, output) = action.applyTo("state")
assertEquals("nextState: state", nextState)
assertNull(output)
}
Expand All @@ -41,7 +42,7 @@ class WorkflowActionTest {
setOutput(null)
}
}
val (nextState, output) = action.applyTo("state", ::OutputHolder)
val (nextState, output) = action.applyTo("state")
assertEquals("nextState: state", nextState)
assertNotNull(output)
assertNull(output.value)
Expand All @@ -54,40 +55,9 @@ class WorkflowActionTest {
setOutput("output")
}
}
val (nextState, output) = action.applyTo("state", ::OutputHolder)
val (nextState, output) = action.applyTo("state")
assertEquals("nextState: state", nextState)
assertNotNull(output)
assertEquals("output", output.value)
}

@Test fun `applyTo doens't invoke mapOutput when output is not set`() {
val action = object : WorkflowAction<String, String?> {
override fun Updater<String, String?>.apply() {
nextState = "nextState: $nextState"
}
}
var outputCalls = 0
val (nextState, output) = action.applyTo("state") { outputCalls++ }
assertEquals("nextState: state", nextState)
assertNull(output)
assertEquals(0, outputCalls)
}

@Test fun `applyTo only invokes mapOutput once when output is set multiple times`() {
val action = object : WorkflowAction<String, String?> {
override fun Updater<String, String?>.apply() {
setOutput("first output")
nextState = "nextState: $nextState"
setOutput(null)
setOutput("third output")
}
}
val outputs = mutableListOf<String?>()
val (nextState, output) = action.applyTo("state") { outputs += it }
assertEquals("nextState: state", nextState)
assertNotNull(output)
assertEquals(listOf<String?>("third output"), outputs)
}

private data class OutputHolder<O>(val value: O)
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
// After receiving an output, the next render pass must be done before emitting that output,
// so that the workflow states appear consistent to observers of the outputs and renderings.
renderingsAndSnapshots.value = runner.nextRendering()
output.withValue { onOutput(it) }
output?.let { onOutput(it.value) }
}
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.squareup.workflow.Workflow
import com.squareup.workflow.WorkflowAction
import com.squareup.workflow.WorkflowInterceptor
import com.squareup.workflow.WorkflowInterceptor.WorkflowSession
import com.squareup.workflow.WorkflowOutput
import kotlinx.coroutines.selects.SelectBuilder
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext
Expand Down Expand Up @@ -98,7 +99,7 @@ import kotlin.coroutines.EmptyCoroutineContext
internal class SubtreeManager<StateT, OutputT>(
snapshotCache: Map<WorkflowNodeId, TreeSnapshot>,
private val contextForChildren: CoroutineContext,
private val emitActionToParent: (WorkflowAction<StateT, OutputT>) -> MaybeOutput<Any?>,
private val emitActionToParent: (WorkflowAction<StateT, OutputT>) -> WorkflowOutput<Any?>?,
private val workflowSession: WorkflowSession? = null,
private val interceptor: WorkflowInterceptor = NoopWorkflowInterceptor,
private val idCounter: IdCounter? = null,
Expand Down Expand Up @@ -158,7 +159,7 @@ internal class SubtreeManager<StateT, OutputT>(
* Uses [selector] to invoke [WorkflowNode.tick] for every running child workflow this instance
* is managing.
*/
fun <T> tickChildren(selector: SelectBuilder<MaybeOutput<T>>) {
fun <T> tickChildren(selector: SelectBuilder<WorkflowOutput<T>?>) {
children.forEachActive { child ->
child.workflowNode.tick(selector)
}
Expand All @@ -182,7 +183,7 @@ internal class SubtreeManager<StateT, OutputT>(
val id = child.id(key)
lateinit var node: WorkflowChildNode<ChildPropsT, ChildOutputT, StateT, OutputT>

fun acceptChildOutput(output: ChildOutputT): MaybeOutput<Any?> {
fun acceptChildOutput(output: ChildOutputT): WorkflowOutput<Any?>? {
val action = node.acceptChildOutput(output)
return emitActionToParent(action)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.squareup.workflow.WorkflowAction
import com.squareup.workflow.WorkflowIdentifier
import com.squareup.workflow.WorkflowInterceptor
import com.squareup.workflow.WorkflowInterceptor.WorkflowSession
import com.squareup.workflow.WorkflowOutput
import com.squareup.workflow.applyTo
import com.squareup.workflow.intercept
import com.squareup.workflow.internal.RealRenderContext.SideEffectRunner
Expand Down Expand Up @@ -60,7 +61,7 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
initialProps: PropsT,
snapshot: TreeSnapshot,
baseContext: CoroutineContext,
private val emitOutputToParent: (OutputT) -> MaybeOutput<Any?> = { MaybeOutput.of(it) },
private val emitOutputToParent: (OutputT) -> WorkflowOutput<Any?>? = { WorkflowOutput(it) },
override val parent: WorkflowSession? = null,
private val interceptor: WorkflowInterceptor = NoopWorkflowInterceptor,
idCounter: IdCounter? = null,
Expand Down Expand Up @@ -183,7 +184,7 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
*
* It is an error to call this method after calling [cancel].
*/
fun <T> tick(selector: SelectBuilder<MaybeOutput<T>>) {
fun <T> tick(selector: SelectBuilder<WorkflowOutput<T>?>) {
// Listen for any child workflow updates.
subtreeManager.tickChildren(selector)

Expand All @@ -198,7 +199,7 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
// Set the tombstone flag so we don't continue to listen to the subscription.
child.tombstone = true
// Nothing to do on close other than update the session, so don't emit any output.
return@onReceive MaybeOutput.none()
return@onReceive null
} else {
val update = child.acceptUpdate(valueOrDone.value)
@Suppress("UNCHECKED_CAST")
Expand Down Expand Up @@ -277,11 +278,11 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
* Applies [action] to this workflow's [state] and
* [emits an output to its parent][emitOutputToParent] if necessary.
*/
private fun <T> applyAction(action: WorkflowAction<StateT, OutputT>): MaybeOutput<T> {
val (newState, tickResult) = action.applyTo(state, emitOutputToParent)
private fun <T> applyAction(action: WorkflowAction<StateT, OutputT>): WorkflowOutput<T>? {
val (newState, tickResult) = action.applyTo(state)
state = newState
@Suppress("UNCHECKED_CAST")
return (tickResult ?: MaybeOutput.none()) as MaybeOutput<T>
return tickResult?.let { emitOutputToParent(it.value) } as WorkflowOutput<T>?
}

private fun <T> createWorkerNode(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.squareup.workflow.RenderingAndSnapshot
import com.squareup.workflow.TreeSnapshot
import com.squareup.workflow.Workflow
import com.squareup.workflow.WorkflowInterceptor
import com.squareup.workflow.WorkflowOutput
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -83,7 +84,7 @@ internal class WorkflowRunner<PropsT, OutputT, RenderingT>(

// Tick _might_ return an output, but if it returns null, it means the state or a child
// probably changed, so we should re-render/snapshot and emit again.
suspend fun nextOutput(): MaybeOutput<OutputT> = select {
suspend fun nextOutput(): WorkflowOutput<OutputT>? = select {
// Stop trying to read from the inputs channel after it's closed.
if (!propsChannel.isClosedForReceive) {
// TODO(https://github.com/square/workflow/issues/512) Replace with receiveOrClosed.
Expand All @@ -95,7 +96,7 @@ internal class WorkflowRunner<PropsT, OutputT, RenderingT>(
}
}
// Return null to tell the caller to do another render pass, but not emit an output.
return@onReceiveOrNull MaybeOutput.none()
return@onReceiveOrNull null
}
}

Expand Down
Loading