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.

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

Cache identifier #904

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ android.useAndroidX=true
systemProp.org.gradle.internal.publish.checksums.insecure=true

GROUP=com.squareup.workflow1
VERSION_NAME=1.8.0-beta13-SNAPSHOT
VERSION_NAME=1.8.0-beta13-cache2-SNAPSHOT

POM_DESCRIPTION=Square Workflow

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package com.squareup.sample.compose.hellocomposeworkflow

import androidx.compose.runtime.Composable
import com.squareup.workflow1.IdCacheable
import com.squareup.workflow1.RenderContext
import com.squareup.workflow1.Sink
import com.squareup.workflow1.StatefulWorkflow
import com.squareup.workflow1.Workflow
import com.squareup.workflow1.WorkflowIdentifier
import com.squareup.workflow1.computeIdentifier
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.compose.ComposeScreen
import kotlin.LazyThreadSafetyMode.NONE

/**
* A stateless [Workflow] that [renders][RenderingContent] itself as a [Composable] function.
Expand All @@ -29,7 +33,7 @@ import com.squareup.workflow1.ui.compose.ComposeScreen
*/
@WorkflowUiExperimentalApi
abstract class ComposeWorkflow<in PropsT, out OutputT : Any> :
Workflow<PropsT, OutputT, ComposeScreen> {
Workflow<PropsT, OutputT, ComposeScreen>, IdCacheable {

/**
* Renders [props] by emitting Compose UI. This function will be called to update the UI whenever
Expand All @@ -48,6 +52,12 @@ abstract class ComposeWorkflow<in PropsT, out OutputT : Any> :

override fun asStatefulWorkflow(): StatefulWorkflow<PropsT, *, OutputT, ComposeScreen> =
ComposeWorkflowImpl(this)

override val cachedIdentifier: WorkflowIdentifier by lazy(
mode = NONE
) {
computeIdentifier()
}
}

/**
Expand Down
11 changes: 9 additions & 2 deletions workflow-core/api/workflow-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ public final class com/squareup/workflow1/BaseRenderContext$DefaultImpls {
public static synthetic fun renderChild$default (Lcom/squareup/workflow1/BaseRenderContext;Lcom/squareup/workflow1/Workflow;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object;
}

public abstract interface class com/squareup/workflow1/IdCacheable {
public abstract fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier;
}

public abstract interface class com/squareup/workflow1/ImpostorWorkflow {
public abstract fun describeRealIdentifier ()Ljava/lang/String;
public abstract fun getRealIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier;
Expand Down Expand Up @@ -115,9 +119,10 @@ public final class com/squareup/workflow1/Snapshots {
public static final fun writeUtf8WithLength (Lokio/BufferedSink;Ljava/lang/String;)Lokio/BufferedSink;
}

public abstract class com/squareup/workflow1/StatefulWorkflow : com/squareup/workflow1/Workflow {
public abstract class com/squareup/workflow1/StatefulWorkflow : com/squareup/workflow1/IdCacheable, com/squareup/workflow1/Workflow {
public fun <init> ()V
public final fun asStatefulWorkflow ()Lcom/squareup/workflow1/StatefulWorkflow;
public fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier;
public abstract fun initialState (Ljava/lang/Object;Lcom/squareup/workflow1/Snapshot;)Ljava/lang/Object;
public fun onPropsChanged (Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
public abstract fun render (Ljava/lang/Object;Ljava/lang/Object;Lcom/squareup/workflow1/StatefulWorkflow$RenderContext;)Ljava/lang/Object;
Expand All @@ -141,9 +146,10 @@ public final class com/squareup/workflow1/StatefulWorkflow$RenderContext : com/s
public fun runningSideEffect (Ljava/lang/String;Lkotlin/jvm/functions/Function2;)V
}

public abstract class com/squareup/workflow1/StatelessWorkflow : com/squareup/workflow1/Workflow {
public abstract class com/squareup/workflow1/StatelessWorkflow : com/squareup/workflow1/IdCacheable, com/squareup/workflow1/Workflow {
public fun <init> ()V
public final fun asStatefulWorkflow ()Lcom/squareup/workflow1/StatefulWorkflow;
public fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier;
public abstract fun render (Ljava/lang/Object;Lcom/squareup/workflow1/StatelessWorkflow$RenderContext;)Ljava/lang/Object;
}

Expand Down Expand Up @@ -283,6 +289,7 @@ public final class com/squareup/workflow1/Workflows {
public static synthetic fun action$default (Lcom/squareup/workflow1/StatelessWorkflow;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/WorkflowAction;
public static synthetic fun action$default (Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/WorkflowAction;
public static final fun applyTo (Lcom/squareup/workflow1/WorkflowAction;Ljava/lang/Object;Ljava/lang/Object;)Lkotlin/Pair;
public static final fun computeIdentifier (Lcom/squareup/workflow1/Workflow;)Lcom/squareup/workflow1/WorkflowIdentifier;
public static final fun contraMap (Lcom/squareup/workflow1/Sink;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/Sink;
public static final fun getIdentifier (Lcom/squareup/workflow1/Workflow;)Lcom/squareup/workflow1/WorkflowIdentifier;
public static final fun mapRendering (Lcom/squareup/workflow1/Workflow;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/Workflow;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.squareup.workflow1

/**
* If your Workflow caches its [WorkflowIdentifier] (to avoid frequent lookups) then implement
* this interface. Note that [StatefulWorkflow] and [StatelessWorkflow] already implement this,
* so you only need to do so if you do not extend one of those classes.
*
* If your Workflow is an [ImpostorWorkflow] use the lazy delegate pattern that [StatefulWorkflow]
* and [StatelessWorkflow] do in order to initialize everything in the proper order.
*/
public interface IdCacheable {

public val cachedIdentifier: WorkflowIdentifier
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import kotlin.jvm.JvmName
public interface ImpostorWorkflow {
/**
* The [WorkflowIdentifier] of another workflow to be combined with the identifier of this
* workflow, as obtained by [Workflow.identifier].
* workflow, as obtained by [Workflow.computeIdentifier].
*
* For workflows that implement operators, this should be the identifier of the upstream
* [Workflow] that this workflow wraps.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package com.squareup.workflow1

import com.squareup.workflow1.StatefulWorkflow.RenderContext
import com.squareup.workflow1.WorkflowAction.Companion.toString
import kotlin.LazyThreadSafetyMode.NONE
import kotlin.jvm.JvmMultifileClass
import kotlin.jvm.JvmName

Expand Down Expand Up @@ -69,7 +70,7 @@ public abstract class StatefulWorkflow<
StateT,
out OutputT,
out RenderingT
> : Workflow<PropsT, OutputT, RenderingT> {
> : Workflow<PropsT, OutputT, RenderingT>, IdCacheable {

public inner class RenderContext internal constructor(
baseContext: BaseRenderContext<PropsT, StateT, OutputT>
Expand Down Expand Up @@ -129,6 +130,18 @@ public abstract class StatefulWorkflow<
context: RenderContext
): RenderingT

/**
* Use a lazy delegate so that any [ImpostorWorkflow.realIdentifier] will have been computed
* before this is initialized and cached.
*
* We use [LazyThreadSafetyMode.NONE] because access to these identifiers is thread-confined.
*/
override val cachedIdentifier: WorkflowIdentifier by lazy(
mode = NONE
) {
computeIdentifier()
}

/**
* Called whenever the state changes to generate a new [Snapshot] of the state.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.squareup.workflow1

import kotlin.LazyThreadSafetyMode.NONE
import kotlin.jvm.JvmMultifileClass
import kotlin.jvm.JvmName

Expand All @@ -24,7 +25,7 @@ import kotlin.jvm.JvmName
* @see StatefulWorkflow
*/
public abstract class StatelessWorkflow<in PropsT, out OutputT, out RenderingT> :
Workflow<PropsT, OutputT, RenderingT> {
Workflow<PropsT, OutputT, RenderingT>, IdCacheable {

@Suppress("UNCHECKED_CAST")
public inner class RenderContext internal constructor(
Expand Down Expand Up @@ -57,6 +58,18 @@ public abstract class StatelessWorkflow<in PropsT, out OutputT, out RenderingT>
context: RenderContext
): RenderingT

/**
* Use a lazy delegate so that any [ImpostorWorkflow.realIdentifier] will have been computed
* before this is initialized and cached.
*
* We use [LazyThreadSafetyMode.NONE] because access to these identifiers is thread-confined.
*/
override val cachedIdentifier: WorkflowIdentifier by lazy(
mode = NONE
) {
computeIdentifier()
}

/**
* Satisfies the [Workflow] interface by wrapping `this` in a [StatefulWorkflow] with `Unit`
* state.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ Workflow<PropsT, OutputT, FromRenderingT>.mapRendering(
transform: (FromRenderingT) -> ToRenderingT
): Workflow<PropsT, OutputT, ToRenderingT> =
object : StatelessWorkflow<PropsT, OutputT, ToRenderingT>(), ImpostorWorkflow {
override val realIdentifier: WorkflowIdentifier get() = [email protected]
override val realIdentifier: WorkflowIdentifier = [email protected]

override fun render(
renderProps: PropsT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,32 @@ public class WorkflowIdentifier internal constructor(

/**
* The [WorkflowIdentifier] that identifies this [Workflow].
*
* This can carry some cost if the Workflow class does not implement [IdCacheable]. Note that
* [StatelessWorkflow] and [StatefulWorkflow] do implement the caching.
*/
public val Workflow<*, *, *>.identifier: WorkflowIdentifier
public inline val Workflow<*, *, *>.identifier: WorkflowIdentifier
get() {
val maybeImpostor = this as? ImpostorWorkflow
return WorkflowIdentifier(
type = Snapshottable(this::class),
proxiedIdentifier = maybeImpostor?.realIdentifier,
description = maybeImpostor?.let { it::describeRealIdentifier }
)
return when (this) {
is IdCacheable -> cachedIdentifier
else -> computeIdentifier()
}
}

/**
* Compute the [WorkflowIdentifier] for this Workflow. Any [IdCacheable] Workflow should call this
* and then store the value in the cachedIdentifier property so as to prevent
* the extra work needed to create the [WorkflowIdentifier] and look up the class name each time.
*/
public fun Workflow<*, *, *>.computeIdentifier(): WorkflowIdentifier {
val maybeImpostor = this as? ImpostorWorkflow
return WorkflowIdentifier(
type = Snapshottable(this::class),
proxiedIdentifier = maybeImpostor?.realIdentifier,
description = maybeImpostor?.let { it::describeRealIdentifier }
)
}

/**
* Creates a [WorkflowIdentifier] that is not capable of being snapshotted and will cause any
* [ImpostorWorkflow] workflow identified by it to also not be snapshotted.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ internal class WorkflowInterceptorTest {

private val Workflow<*, *, *>.session: WorkflowSession
get() = object : WorkflowSession {
override val identifier: WorkflowIdentifier = [email protected]
override val identifier: WorkflowIdentifier
get() = [email protected]
override val renderKey: String = ""
override val sessionId: Long = 0
override val parent: WorkflowSession? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.squareup.workflow1.WorkflowIdentifier
import com.squareup.workflow1.WorkflowOutput
import com.squareup.workflow1.action
import com.squareup.workflow1.asWorker
import com.squareup.workflow1.computeIdentifier
import com.squareup.workflow1.contraMap
import com.squareup.workflow1.identifier
import com.squareup.workflow1.renderChild
Expand Down Expand Up @@ -1473,32 +1474,32 @@ internal class RealRenderTesterTest {

@Test fun `realTypeMatchesExpectation() matches mockito mock of expected interface`() {
val expected = TestWorkflowInterface::class.workflowIdentifier
val actual = mock<TestWorkflowInterface>().identifier
val actual = mock<TestWorkflowInterface>().computeIdentifier()
assertTrue(actual.realTypeMatchesExpectation(expected))
}

@Test fun `realTypeMatchesExpectation() matches mockito mock of expected abstract class`() {
val expected = ExpectedWorkflowClass::class.workflowIdentifier
val actual = mock<ExpectedWorkflowClass>().identifier
val actual = mock<ExpectedWorkflowClass>().computeIdentifier()
assertTrue(actual.realTypeMatchesExpectation(expected))
}

@Test fun `realTypeMatchesExpectation() doesn't match mockito mock of unexpected interface`() {
val expected = TestWorkflowInterface::class.workflowIdentifier
val actual = mock<Workflow<Unit, Nothing, Unit>>().identifier
val actual = mock<Workflow<Unit, Nothing, Unit>>().computeIdentifier()
assertFalse(actual.realTypeMatchesExpectation(expected))
}

@Test
fun `realTypeMatchesExpectation() doesn't match mockito mock of unexpected abstract class`() {
val expected = ExpectedWorkflowClass::class.workflowIdentifier
val actual = mock<UnexpectedWorkflowClass>().identifier
val actual = mock<UnexpectedWorkflowClass>().computeIdentifier()
assertFalse(actual.realTypeMatchesExpectation(expected))
}

@Test fun `realTypeMatchesExpectation() handles mockk mocks`() {
val expected = TestWorkflowInterface::class.workflowIdentifier
val actual = mockk<TestWorkflowInterface>().identifier
val actual = mockk<TestWorkflowInterface>().computeIdentifier()
assertTrue(actual.realTypeMatchesExpectation(expected))
}

Expand Down