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

Simplify ViewRegistry to act more like a simple map of rendering type to ViewFactory. #1148

Merged
merged 1 commit into from
May 11, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.squareup.workflow.ui.ViewFactory
import com.squareup.workflow.ui.ViewRegistry
import com.squareup.workflow.ui.backPressedHandler
import com.squareup.workflow.ui.bindShowRendering
import com.squareup.workflow.ui.buildView
import com.squareup.workflow.ui.getShowRendering

/**
Expand Down
7 changes: 5 additions & 2 deletions kotlin/workflow-ui/core-android/api/core-android.api
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ public final class com/squareup/workflow/ui/ViewFactory$DefaultImpls {

public abstract interface class com/squareup/workflow/ui/ViewRegistry {
public static final field Companion Lcom/squareup/workflow/ui/ViewRegistry$Companion;
public abstract fun buildView (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View;
public abstract fun getFactoryFor (Lkotlin/reflect/KClass;)Lcom/squareup/workflow/ui/ViewFactory;
public abstract fun getKeys ()Ljava/util/Set;
public abstract fun hasViewBeenBound (Landroid/view/View;)Z
}

public final class com/squareup/workflow/ui/ViewRegistry$Companion : com/squareup/workflow/ui/ViewEnvironmentKey {
Expand All @@ -97,14 +98,16 @@ public final class com/squareup/workflow/ui/ViewRegistry$Companion : com/squareu
}

public final class com/squareup/workflow/ui/ViewRegistry$DefaultImpls {
public static synthetic fun buildView$default (Lcom/squareup/workflow/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;ILjava/lang/Object;)Landroid/view/View;
public static fun hasViewBeenBound (Lcom/squareup/workflow/ui/ViewRegistry;Landroid/view/View;)Z
}

public final class com/squareup/workflow/ui/ViewRegistryKt {
public static final fun ViewRegistry ()Lcom/squareup/workflow/ui/ViewRegistry;
public static final fun ViewRegistry ([Lcom/squareup/workflow/ui/ViewFactory;)Lcom/squareup/workflow/ui/ViewRegistry;
public static final fun ViewRegistry ([Lcom/squareup/workflow/ui/ViewRegistry;)Lcom/squareup/workflow/ui/ViewRegistry;
public static final fun buildView (Lcom/squareup/workflow/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View;
public static final fun buildView (Lcom/squareup/workflow/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroid/view/ViewGroup;)Landroid/view/View;
public static synthetic fun buildView$default (Lcom/squareup/workflow/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;ILjava/lang/Object;)Landroid/view/View;
public static final fun plus (Lcom/squareup/workflow/ui/ViewRegistry;Lcom/squareup/workflow/ui/ViewFactory;)Lcom/squareup/workflow/ui/ViewRegistry;
public static final fun plus (Lcom/squareup/workflow/ui/ViewRegistry;Lcom/squareup/workflow/ui/ViewRegistry;)Lcom/squareup/workflow/ui/ViewRegistry;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
*/
package com.squareup.workflow.ui

import android.content.Context
import android.view.View
import android.view.ViewGroup
import kotlin.reflect.KClass

/**
Expand All @@ -35,34 +32,18 @@ internal class BindingViewRegistry private constructor(
check(keys.size == bindings.size) {
"${bindings.map { it.type }} must not have duplicate entries."
}
}
} as Map<KClass<*>, ViewFactory<*>>
)

override val keys: Set<KClass<*>> get() = bindings.keys

override fun <RenderingT : Any> buildView(
initialRendering: RenderingT,
initialViewEnvironment: ViewEnvironment,
contextForNewView: Context,
container: ViewGroup?
): View {
override fun <RenderingT : Any> getFactoryFor(
renderingType: KClass<out RenderingT>
): ViewFactory<RenderingT> {
@Suppress("UNCHECKED_CAST")
return (bindings[initialRendering::class] as? ViewFactory<RenderingT>)
?.buildView(
initialRendering,
initialViewEnvironment,
contextForNewView,
container
)
?.apply {
checkNotNull(getRendering<RenderingT>()) {
"View.bindShowRendering should have been called for $this, typically by the " +
"${ViewFactory::class.java.name} that created it."
}
}
?: throw IllegalArgumentException(
"A ${ViewFactory::class.java.name} should have been registered " +
"to display $initialRendering."
)
return requireNotNull(bindings[renderingType] as? ViewFactory<RenderingT>) {
"A ${ViewFactory::class.java.name} should have been registered " +
"to display a $renderingType."
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
*/
package com.squareup.workflow.ui

import android.content.Context
import android.view.View
import android.view.ViewGroup
import kotlin.reflect.KClass

/**
Expand All @@ -42,18 +39,15 @@ internal class CompositeViewRegistry private constructor(

override val keys: Set<KClass<*>> get() = registriesByKey.keys

override fun <RenderingT : Any> buildView(
initialRendering: RenderingT,
initialViewEnvironment: ViewEnvironment,
contextForNewView: Context,
container: ViewGroup?
): View {
val registry = registriesByKey[initialRendering::class]
?: throw IllegalArgumentException(
"A ${ViewFactory::class.java.name} should have been registered " +
"to display $initialRendering."
)
return registry.buildView(initialRendering, initialViewEnvironment, contextForNewView, container)
override fun <RenderingT : Any> getFactoryFor(
renderingType: KClass<out RenderingT>
): ViewFactory<RenderingT> = getRegistryFor(renderingType).getFactoryFor(renderingType)

private fun getRegistryFor(renderingType: KClass<out Any>): ViewRegistry {
return requireNotNull(registriesByKey[renderingType]) {
"A ${ViewFactory::class.java.name} should have been registered " +
"to display a $renderingType."
}
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import kotlin.reflect.KClass
*
* Sets of bindings are gathered in [ViewRegistry] instances.
*/
interface ViewFactory<RenderingT : Any> {
val type: KClass<RenderingT>
interface ViewFactory<in RenderingT : Any> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that's a good sign.

val type: KClass<in RenderingT>

/**
* Returns a View ready to display [initialRendering] (and any succeeding values)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,23 @@ interface ViewRegistry {
val keys: Set<KClass<*>>

/**
* It is usually more convenient to use [WorkflowViewStub] than to call this method directly.
* This method is not for general use, use [WorkflowViewStub] instead.
*
* Creates a [View] to display [initialRendering], which can be updated via calls
* to [View.showRendering].
* Returns the [ViewFactory] that was registered for the given [renderingType].
*
* @throws IllegalArgumentException if no binding can be find for type [RenderingT]
* @throws IllegalArgumentException if no factory can be found for type [RenderingT]
*/
fun <RenderingT : Any> getFactoryFor(
renderingType: KClass<out RenderingT>
): ViewFactory<RenderingT>

/**
* This method is not for general use, it's called by [buildView] to validate views returned by
* [ViewFactory]s.
*
* @throws IllegalStateException if the matching [ViewFactory] fails to call
* [View.bindShowRendering] when constructing the view
* Returns true iff [view] has been bound to a [ShowRenderingTag] by calling [bindShowRendering].
*/
fun <RenderingT : Any> buildView(
initialRendering: RenderingT,
initialViewEnvironment: ViewEnvironment,
contextForNewView: Context,
container: ViewGroup? = null
): View
fun hasViewBeenBound(view: View): Boolean = view.getRendering<Any>() != null

companion object : ViewEnvironmentKey<ViewRegistry>(ViewRegistry::class) {
override val default: ViewRegistry
Expand All @@ -104,6 +105,38 @@ fun ViewRegistry(vararg registries: ViewRegistry): ViewRegistry = CompositeViewR
*/
fun ViewRegistry(): ViewRegistry = BindingViewRegistry()

/**
* It is usually more convenient to use [WorkflowViewStub] than to call this method directly.
*
* Creates a [View] to display [initialRendering], which can be updated via calls
* to [View.showRendering].
*
* @throws IllegalArgumentException if no factory can be find for type [RenderingT]
*
* @throws IllegalStateException if [ViewRegistry.hasViewBeenBound] returns false (i.e. if the
* matching [ViewFactory] fails to call [View.bindShowRendering] when constructing the view)
*/
fun <RenderingT : Any> ViewRegistry.buildView(
initialRendering: RenderingT,
initialViewEnvironment: ViewEnvironment,
contextForNewView: Context,
container: ViewGroup? = null
): View {
return getFactoryFor(initialRendering::class)
.buildView(
initialRendering,
initialViewEnvironment,
contextForNewView,
container
)
.apply {
check(hasViewBeenBound(this)) {
"View.bindShowRendering should have been called for $this, typically by the " +
"${ViewFactory::class.java.name} that created it."
}
}
}

/**
* It is usually more convenient to use [WorkflowViewStub] than to call this method directly.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,36 +23,44 @@ import kotlin.test.assertTrue
class BindingViewRegistryTest {

@Test fun `keys from bindings`() {
val binding1 = TestBinding(FooRendering::class)
val binding2 = TestBinding(BarRendering::class)
val registry = BindingViewRegistry(binding1, binding2)
val factory1 = TestViewFactory(FooRendering::class)
val factory2 = TestViewFactory(BarRendering::class)
val registry = BindingViewRegistry(factory1, factory2)

assertThat(registry.keys).containsExactly(binding1.type, binding2.type)
assertThat(registry.keys).containsExactly(factory1.type, factory2.type)
}

@Test fun `throws on duplicates`() {
val binding1 = TestBinding(FooRendering::class)
val binding2 = TestBinding(FooRendering::class)
@Test fun `constructor throws on duplicates`() {
val factory1 = TestViewFactory(FooRendering::class)
val factory2 = TestViewFactory(FooRendering::class)

val error = assertFailsWith<IllegalStateException> {
BindingViewRegistry(binding1, binding2)
BindingViewRegistry(factory1, factory2)
}
assertThat(error).hasMessageThat()
.endsWith("must not have duplicate entries.")
assertThat(error).hasMessageThat()
.contains(FooRendering::class.java.name)
}

@Test fun `throws on missing binding`() {
val fooBinding = TestBinding(FooRendering::class)
val registry = BindingViewRegistry(fooBinding)
@Test fun `getFactoryFor works`() {
val fooFactory = TestViewFactory(FooRendering::class)
val registry = BindingViewRegistry(fooFactory)

val factory = registry.getFactoryFor(FooRendering::class)
assertThat(factory).isSameInstanceAs(fooFactory)
}

@Test fun `getFactoryFor throws on missing binding`() {
val fooFactory = TestViewFactory(FooRendering::class)
val registry = BindingViewRegistry(fooFactory)

val error = assertFailsWith<IllegalArgumentException> {
registry.buildView(BarRendering)
registry.getFactoryFor(BarRendering::class)
}
assertThat(error).hasMessageThat()
.isEqualTo(
"A ${ViewFactory::class.java.name} should have been registered to display $BarRendering."
"A ${ViewFactory::class.java.name} should have been registered to display a ${BarRendering::class}."
)
}

Expand Down