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

Add a new global plugin API. #311

Merged
merged 3 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.instacart.formula.coroutines

import com.instacart.formula.FormulaPlugins
import com.instacart.formula.FormulaRuntime
import com.instacart.formula.IFormula
import com.instacart.formula.Inspector
Expand All @@ -26,13 +27,18 @@ object FlowRuntime {
return callbackFlow<Output> {
threadChecker.check("Need to subscribe on main thread.")

val mergedInspector = FormulaPlugins.inspector(
type = formula.type(),
local = inspector,
)

val runtimeFactory = {
FormulaRuntime(
threadChecker = threadChecker,
formula = formula,
onOutput = this::trySendBlocking,
onError = this::close,
inspector = inspector,
inspector = mergedInspector,
isValidationEnabled = isValidationEnabled,
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.instacart.formula.rxjava3

import com.instacart.formula.FormulaPlugins
import com.instacart.formula.FormulaRuntime
import com.instacart.formula.IFormula
import com.instacart.formula.Inspector
Expand All @@ -17,13 +18,17 @@ object RxJavaRuntime {
): Observable<Output> {
val threadChecker = ThreadChecker(formula)
return Observable.create<Output> { emitter ->
val mergedInspector = FormulaPlugins.inspector(
type = formula.type(),
local = inspector,
)
val runtimeFactory = {
FormulaRuntime(
threadChecker = threadChecker,
formula = formula,
onOutput = emitter::onNext,
onError = emitter::onError,
inspector = inspector,
inspector = mergedInspector,
isValidationEnabled = isValidationEnabled,
)
}
Expand Down
21 changes: 21 additions & 0 deletions formula/src/main/java/com/instacart/formula/FormulaPlugins.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.instacart.formula

import com.instacart.formula.internal.ListInspector
import kotlin.reflect.KClass

object FormulaPlugins {
private var plugin: Plugin? = null

fun setPlugin(plugin: Plugin?) {
this.plugin = plugin
}

fun inspector(type: KClass<*>, local: Inspector?): Inspector? {
val global = plugin?.inspector(type)
return when {
global == null -> local
local == null -> global
else -> ListInspector(listOf(global, local))
}
}
}
15 changes: 15 additions & 0 deletions formula/src/main/java/com/instacart/formula/Plugin.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.instacart.formula

import kotlin.reflect.KClass

interface Plugin {
/**
* A global callback to create [Inspector] for any formula. This will be called once when
* formula is initially started.
*
* @param type Formula type.
*/
fun inspector(type: KClass<*>): Inspector? {
return null
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.instacart.formula.internal

import com.instacart.formula.DeferredAction
import com.instacart.formula.Inspector
import kotlin.reflect.KClass

internal class ListInspector(
private val inspectors: List<Inspector>,
): Inspector {
override fun onFormulaStarted(formulaType: KClass<*>) {
for (inspector in inspectors) {
inspector.onFormulaStarted(formulaType)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: maybe introduce private fun forEveryInspector(block: Inspector.() -> Unit) to make this class less verbose and a little more kt idiomatic?

forEveryInspector { onFormulaStarted(formulaType) }

}

override fun onFormulaFinished(formulaType: KClass<*>) {
for (inspector in inspectors) {
inspector.onFormulaFinished(formulaType)
}
}

override fun onEvaluateStarted(formulaType: KClass<*>, state: Any?) {
for (inspector in inspectors) {
inspector.onEvaluateStarted(formulaType, state)
}
}

override fun onInputChanged(formulaType: KClass<*>, prevInput: Any?, newInput: Any?) {
for (inspector in inspectors) {
inspector.onInputChanged(formulaType, prevInput, newInput)
}
}

override fun onEvaluateFinished(formulaType: KClass<*>, output: Any?, evaluated: Boolean) {
for (inspector in inspectors) {
inspector.onEvaluateFinished(formulaType, output, evaluated)
}
}

override fun onActionStarted(formulaType: KClass<*>, action: DeferredAction<*>) {
for (inspector in inspectors) {
inspector.onActionStarted(formulaType, action)
}
}

override fun onActionFinished(formulaType: KClass<*>, action: DeferredAction<*>) {
for (inspector in inspectors) {
inspector.onActionFinished(formulaType, action)
}
}

override fun onStateChanged(formulaType: KClass<*>, old: Any?, new: Any?) {
for (inspector in inspectors) {
inspector.onStateChanged(formulaType, old, new)
}
}

override fun onRunStarted(evaluate: Boolean) {
for (inspector in inspectors) {
inspector.onRunStarted(evaluate)
}
}

override fun onRunFinished() {
for (inspector in inspectors) {
inspector.onRunFinished()
}
}
}
59 changes: 36 additions & 23 deletions formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.instacart.formula
import com.google.common.truth.Truth
import com.google.common.truth.Truth.assertThat
import com.instacart.formula.actions.EmptyAction
import com.instacart.formula.internal.ClearPluginsRule
import com.instacart.formula.internal.TestInspector
import com.instacart.formula.internal.Try
import com.instacart.formula.rxjava3.RxAction
Expand Down Expand Up @@ -79,6 +80,7 @@ import org.junit.rules.RuleChain
import org.junit.rules.TestName
import org.junit.runner.RunWith
import org.junit.runners.Parameterized
import kotlin.reflect.KClass

@RunWith(Parameterized::class)
class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {
Expand All @@ -92,7 +94,10 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {
}

@get:Rule
val rule = RuleChain.outerRule(TestName()).around(runtime.rule)
val rule = RuleChain
.outerRule(TestName())
.around(ClearPluginsRule())
.around(runtime.rule)

@Test fun `state change triggers an evaluation`() {
val formula = EventCallbackFormula()
Expand Down Expand Up @@ -1249,33 +1254,41 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {

@Test
fun `inspector events`() {
val globalInspector = TestInspector()
FormulaPlugins.setPlugin(object : Plugin {
override fun inspector(type: KClass<*>): Inspector {
return globalInspector
}
})
Comment on lines +1257 to +1262
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: should we have test cases for any possible (global, local) permutation, not only when both are not null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have some tests for local only, added a new test for global only.


val formula = StartStopFormula(runtime)
val inspector = TestInspector()
val subject = runtime.test(formula, Unit, inspector)
val localInspector = TestInspector()
val subject = runtime.test(formula, Unit, localInspector)
subject.output { startListening() }
subject.output { stopListening() }
subject.dispose()

assertThat(inspector.events).containsExactly(
"formula-run-started",
"formula-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-finished: com.instacart.formula.subjects.StartStopFormula",
"formula-run-finished",
"state-changed: com.instacart.formula.subjects.StartStopFormula",
"formula-run-started",
"evaluate-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-finished: com.instacart.formula.subjects.StartStopFormula",
"action-started: com.instacart.formula.subjects.StartStopFormula",
"formula-run-finished",
"state-changed: com.instacart.formula.subjects.StartStopFormula",
"formula-run-started",
"evaluate-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-finished: com.instacart.formula.subjects.StartStopFormula",
"action-finished: com.instacart.formula.subjects.StartStopFormula",
"formula-run-finished",
"formula-finished: com.instacart.formula.subjects.StartStopFormula"
).inOrder()
for (inspector in listOf(globalInspector, localInspector)) {
assertThat(inspector.events).containsExactly(
"formula-run-started",
"formula-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-finished: com.instacart.formula.subjects.StartStopFormula",
"formula-run-finished",
"state-changed: com.instacart.formula.subjects.StartStopFormula",
"formula-run-started",
"evaluate-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-finished: com.instacart.formula.subjects.StartStopFormula",
"action-started: com.instacart.formula.subjects.StartStopFormula",
"formula-run-finished",
"state-changed: com.instacart.formula.subjects.StartStopFormula",
"formula-run-started",
"evaluate-started: com.instacart.formula.subjects.StartStopFormula",
"evaluate-finished: com.instacart.formula.subjects.StartStopFormula",
"action-finished: com.instacart.formula.subjects.StartStopFormula",
"formula-run-finished",
"formula-finished: com.instacart.formula.subjects.StartStopFormula"
).inOrder()
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.instacart.formula.internal

import com.instacart.formula.FormulaPlugins
import org.junit.rules.TestRule
import org.junit.runner.Description
import org.junit.runners.model.Statement

class ClearPluginsRule : TestRule {
override fun apply(base: Statement, description: Description): Statement {
return object : Statement() {
override fun evaluate() {
FormulaPlugins.setPlugin(null)
try {
base.evaluate()
} finally {
FormulaPlugins.setPlugin(null)
}
}
}
}
}
Loading