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

fix: Fix native trigger environment #135

Merged
merged 3 commits into from
May 16, 2024
Merged

fix: Fix native trigger environment #135

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2a0519
fix: Fix native trigger environment
enricocolasante May 14, 2024
fbfe246
Use default trigger environment
enricocolasante May 15, 2024
39c0846
Add manual overload
enricocolasante May 16, 2024
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
4 changes: 3 additions & 1 deletion src/commonMain/kotlin/org/hisp/dhis/rules/Environment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
package org.hisp.dhis.rules

expect fun getEnvironment(): String
import org.hisp.dhis.rules.models.TriggerEnvironment

expect fun getEnvironment(): TriggerEnvironment
9 changes: 6 additions & 3 deletions src/commonMain/kotlin/org/hisp/dhis/rules/api/RuleEngine.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
package org.hisp.dhis.rules.api

import org.hisp.dhis.rules.engine.DefaultRuleEngine
import org.hisp.dhis.rules.getEnvironment
import org.hisp.dhis.rules.models.*
import org.hisp.dhis.rules.models.TriggerEnvironment.SERVER
Copy link
Member

Choose a reason for hiding this comment

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

These two imports are not used and could be removed

import kotlin.jvm.JvmOverloads
Copy link
Member

Choose a reason for hiding this comment

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

Well, after a second thought, I do think we need to annotate the methods as overloaded if we want to make it optional in Java. In Kotlin, the last parameter would be optional, but in Java it will be mandatory unless the methods are mark as overloaded

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, @jvmoverloads is not allowed on interfaces and default parameters are not allowed on overriding methods.
So I am manually creating the overload methods.
Another option would be not to use an interface.
I am merging this so I can test the artifact but then we can discuss the best solution

import kotlin.jvm.JvmStatic

interface RuleEngine {
fun validate(expression: String, dataItemStore: Map<String, DataItem>): RuleValidationResult
fun validateDataFieldExpression(expression: String, dataItemStore: Map<String, DataItem>): RuleValidationResult
fun evaluateAll(enrollmentTarget: RuleEnrollment?, eventsTarget: List<RuleEvent>, executionContext: RuleEngineContext): List<RuleEffects>
fun evaluate(target: RuleEnrollment, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext): List<RuleEffect>
fun evaluate(target: RuleEvent, ruleEnrollment: RuleEnrollment?, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext): List<RuleEffect>
fun evaluateAll(enrollmentTarget: RuleEnrollment?, eventsTarget: List<RuleEvent>, executionContext: RuleEngineContext, triggerEnvironment: TriggerEnvironment = getEnvironment()): List<RuleEffects>
fun evaluate(target: RuleEnrollment, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext, triggerEnvironment: TriggerEnvironment = getEnvironment()): List<RuleEffect>
fun evaluate(target: RuleEvent, ruleEnrollment: RuleEnrollment?, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext, triggerEnvironment: TriggerEnvironment = getEnvironment()): List<RuleEffect>

companion object {
@JvmStatic
Expand Down
13 changes: 7 additions & 6 deletions src/commonMain/kotlin/org/hisp/dhis/rules/engine/DefaultRuleEngine.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import org.hisp.dhis.rules.api.RuleEngine
import org.hisp.dhis.rules.api.RuleEngineContext
import org.hisp.dhis.rules.getEnvironment
import org.hisp.dhis.rules.models.*
import kotlin.jvm.JvmOverloads

internal class DefaultRuleEngine: RuleEngine {
override fun evaluate(target: RuleEvent, ruleEnrollment: RuleEnrollment?, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext): List<RuleEffect> {
override fun evaluate(target: RuleEvent, ruleEnrollment: RuleEnrollment?, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext, triggerEnvironment: TriggerEnvironment): List<RuleEffect> {
val valueMap = RuleVariableValueMapBuilder.target(target)
.ruleVariables(executionContext.ruleVariables)
.ruleEnrollment(ruleEnrollment)
.triggerEnvironment(TriggerEnvironment.valueOf(getEnvironment()))
.triggerEnvironment(triggerEnvironment)
.ruleEvents(ruleEvents)
.constantValueMap(executionContext.constantsValues)
.build()
Expand All @@ -26,10 +27,10 @@ internal class DefaultRuleEngine: RuleEngine {
)
}

override fun evaluate(target: RuleEnrollment, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext): List<RuleEffect> {
override fun evaluate(target: RuleEnrollment, ruleEvents: List<RuleEvent>, executionContext: RuleEngineContext, triggerEnvironment: TriggerEnvironment): List<RuleEffect> {
val valueMap = RuleVariableValueMapBuilder.target(target)
.ruleVariables(executionContext.ruleVariables)
.triggerEnvironment(TriggerEnvironment.valueOf(getEnvironment()))
.triggerEnvironment(triggerEnvironment)
.ruleEvents(ruleEvents)
.constantValueMap(executionContext.constantsValues)
.build()
Expand All @@ -39,11 +40,11 @@ internal class DefaultRuleEngine: RuleEngine {
)
}

override fun evaluateAll(enrollmentTarget: RuleEnrollment?, eventsTarget: List<RuleEvent>, executionContext: RuleEngineContext): List<RuleEffects> {
override fun evaluateAll(enrollmentTarget: RuleEnrollment?, eventsTarget: List<RuleEvent>, executionContext: RuleEngineContext, triggerEnvironment: TriggerEnvironment): List<RuleEffects> {
val valueMap = RuleVariableValueMapBuilder.target()
.ruleVariables(executionContext.ruleVariables)
.ruleEnrollment(enrollmentTarget)
.triggerEnvironment(TriggerEnvironment.valueOf(getEnvironment()))
.triggerEnvironment(triggerEnvironment)
.ruleEvents(eventsTarget)
.constantValueMap(executionContext.constantsValues)
.multipleBuild()
Expand Down
1 change: 1 addition & 0 deletions src/commonMain/kotlin/org/hisp/dhis/rules/models/TriggerEnvironment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import kotlin.js.JsExport
@JsExport
enum class TriggerEnvironment(val clientName: String) {
ANDROIDCLIENT("AndroidClient"),
NATIVE("Native"),
SERVER("Server"),
WEBCLIENT("WebClient")

Expand Down
2 changes: 1 addition & 1 deletion src/commonTest/kotlin/org/hisp/dhis/rules/ProgramRuleVariableTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class ProgramRuleVariableTest {
fun testEnvironmentProgramVariableIsAssigned() {
val rule = getRule("V{environment}")
val ruleEffects = callEnrollmentRuleEngine(rule)
assertProgramRuleVariableAssignment(ruleEffects, rule, TriggerEnvironment.valueOf(getEnvironment()).clientName)
assertProgramRuleVariableAssignment(ruleEffects, rule, getEnvironment().clientName)
}

@Test
Expand Down
6 changes: 4 additions & 2 deletions src/jsMain/kotlin/org/hisp/dhis/rules/Environment.js.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.hisp.dhis.rules

actual fun getEnvironment(): String {
return "WEBCLIENT"
import org.hisp.dhis.rules.models.TriggerEnvironment

actual fun getEnvironment(): TriggerEnvironment {
return TriggerEnvironment.WEBCLIENT
}
6 changes: 4 additions & 2 deletions src/jvmMain/kotlin/org/hisp/dhis/rules/Environment.jvm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.hisp.dhis.rules

actual fun getEnvironment(): String {
return "SERVER"
import org.hisp.dhis.rules.models.TriggerEnvironment

actual fun getEnvironment(): TriggerEnvironment {
return TriggerEnvironment.SERVER
}
6 changes: 4 additions & 2 deletions src/nativeMain/kotlin/org/hisp/dhis/rules/Environment.native.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.hisp.dhis.rules

actual fun getEnvironment(): String {
return "ANDROIDCLIENT"
import org.hisp.dhis.rules.models.TriggerEnvironment

actual fun getEnvironment(): TriggerEnvironment {
return TriggerEnvironment.NATIVE
}