Skip to content

Commit dd9769c

Browse files
enricocolasanteenricocolasante
committed
perf: Analyze rules before running to limit context [DHIS2-21245]
1 parent 1d0ea20 commit dd9769c

7 files changed

Lines changed: 243 additions & 1 deletion

File tree

‎api/rule-engine.api‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,25 @@ public final class org/hisp/dhis/rules/api/ItemValueType : java/lang/Enum {
6363
public static fun values ()[Lorg/hisp/dhis/rules/api/ItemValueType;
6464
}
6565

66+
public final class org/hisp/dhis/rules/api/RuleContextRequirements {
67+
public static final field Companion Lorg/hisp/dhis/rules/api/RuleContextRequirements$Companion;
68+
public fun <init> (Z)V
69+
public final fun component1 ()Z
70+
public final fun copy (Z)Lorg/hisp/dhis/rules/api/RuleContextRequirements;
71+
public static synthetic fun copy$default (Lorg/hisp/dhis/rules/api/RuleContextRequirements;ZILjava/lang/Object;)Lorg/hisp/dhis/rules/api/RuleContextRequirements;
72+
public fun equals (Ljava/lang/Object;)Z
73+
public final fun getNeedsAllEvents ()Z
74+
public fun hashCode ()I
75+
public fun toString ()Ljava/lang/String;
76+
}
77+
78+
public final class org/hisp/dhis/rules/api/RuleContextRequirements$Companion {
79+
public final fun getNONE ()Lorg/hisp/dhis/rules/api/RuleContextRequirements;
80+
}
81+
6682
public abstract interface class org/hisp/dhis/rules/api/RuleEngine {
6783
public static final field Companion Lorg/hisp/dhis/rules/api/RuleEngine$Companion;
84+
public abstract fun analyzeContextRequirements (Ljava/util/List;Ljava/util/List;)Lorg/hisp/dhis/rules/api/RuleContextRequirements;
6885
public abstract fun evaluate (Lorg/hisp/dhis/rules/models/RuleEnrollment;Ljava/util/List;Lorg/hisp/dhis/rules/api/RuleEngineContext;)Ljava/util/List;
6986
public abstract fun evaluate (Lorg/hisp/dhis/rules/models/RuleEvent;Lorg/hisp/dhis/rules/models/RuleEnrollment;Ljava/util/List;Lorg/hisp/dhis/rules/api/RuleEngineContext;)Ljava/util/List;
7087
public abstract fun evaluateAll (Lorg/hisp/dhis/rules/models/RuleEnrollment;Ljava/util/List;Lorg/hisp/dhis/rules/api/RuleEngineContext;)Ljava/util/List;

‎build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ repositories {
1010
maven { url = uri("https://central.sonatype.com/repository/maven-snapshots/") }
1111
}
1212

13-
version = "3.7.2-SNAPSHOT"
13+
version = "3.8.0-SNAPSHOT"
1414
group = "org.hisp.dhis.rules"
1515

1616
if (project.hasProperty("removeSnapshotSuffix")) {

‎src/commonMain/kotlin/org/hisp/dhis/rules/api/RuleContextRequirements.kt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package org.hisp.dhis.rules.api
2+
3+
data class RuleContextRequirements(
4+
val needsAllEvents: Boolean,
5+
) {
6+
companion object {
7+
val NONE = RuleContextRequirements(needsAllEvents = false)
8+
}
9+
}

‎src/commonMain/kotlin/org/hisp/dhis/rules/api/RuleEngine.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import org.hisp.dhis.rules.models.*
55
import kotlin.jvm.JvmStatic
66

77
interface RuleEngine {
8+
fun analyzeContextRequirements(rules: List<Rule>, variables: List<RuleVariable>): RuleContextRequirements
9+
810
fun validate(
911
expression: String,
1012
dataItemStore: Map<String, DataItem>,

‎src/commonMain/kotlin/org/hisp/dhis/rules/engine/DefaultRuleEngine.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import org.hisp.dhis.lib.expression.spi.IllegalExpressionException
66
import org.hisp.dhis.lib.expression.spi.ParseException
77
import org.hisp.dhis.lib.expression.spi.ValueType
88
import org.hisp.dhis.rules.api.DataItem
9+
import org.hisp.dhis.rules.api.RuleContextRequirements
910
import org.hisp.dhis.rules.api.RuleEngine
1011
import org.hisp.dhis.rules.api.RuleEngineContext
1112
import org.hisp.dhis.rules.models.*
@@ -74,6 +75,10 @@ internal class DefaultRuleEngine : RuleEngine {
7475
)
7576
}
7677

78+
override fun analyzeContextRequirements(rules: List<Rule>, variables: List<RuleVariable>): RuleContextRequirements {
79+
return RuleEngineAnalyzer.analyzeContextRequirements(rules, variables)
80+
}
81+
7782
override fun validate(
7883
expression: String,
7984
dataItemStore: Map<String, DataItem>,

‎src/commonMain/kotlin/org/hisp/dhis/rules/engine/RuleEngineAnalyzer.kt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package org.hisp.dhis.rules.engine
2+
3+
import org.hisp.dhis.rules.api.RuleContextRequirements
4+
import org.hisp.dhis.rules.models.Rule
5+
import org.hisp.dhis.rules.models.RuleVariable
6+
import org.hisp.dhis.rules.models.RuleVariableNewestEvent
7+
import org.hisp.dhis.rules.models.RuleVariableNewestStageEvent
8+
import org.hisp.dhis.rules.models.RuleVariablePreviousEvent
9+
10+
internal object RuleEngineAnalyzer {
11+
private val MULTI_EVENT_ENV_VARS = setOf("event_count")
12+
13+
fun analyzeContextRequirements(
14+
rules: List<Rule>,
15+
variables: List<RuleVariable>,
16+
): RuleContextRequirements {
17+
val envVars = mutableSetOf<String>()
18+
val referencedVarNames = mutableSetOf<String>()
19+
20+
for (rule in rules) {
21+
rule.conditionExpression.getOrNull()?.let {
22+
envVars += it.collectProgramVariablesNames()
23+
referencedVarNames += it.collectProgramRuleVariableNames()
24+
}
25+
for (action in rule.actions) {
26+
action.dataExpression.getOrNull()?.let {
27+
envVars += it.collectProgramVariablesNames()
28+
referencedVarNames += it.collectProgramRuleVariableNames()
29+
}
30+
}
31+
}
32+
33+
val byName = variables.associateBy { it.name }
34+
val needsAllEvents = envVars.any { it in MULTI_EVENT_ENV_VARS } ||
35+
referencedVarNames.any { name ->
36+
val v = byName[name]
37+
v is RuleVariableNewestEvent ||
38+
v is RuleVariableNewestStageEvent ||
39+
v is RuleVariablePreviousEvent
40+
}
41+
42+
return RuleContextRequirements(needsAllEvents)
43+
}
44+
}

‎src/commonTest/kotlin/org/hisp/dhis/rules/RuleEngineAnalyzerTest.kt‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
package org.hisp.dhis.rules
2+
3+
import org.hisp.dhis.rules.api.RuleContextRequirements
4+
import org.hisp.dhis.rules.api.RuleEngine
5+
import org.hisp.dhis.rules.engine.RuleEngineAnalyzer
6+
import org.hisp.dhis.rules.models.Option
7+
import org.hisp.dhis.rules.models.Rule
8+
import org.hisp.dhis.rules.models.RuleAction
9+
import org.hisp.dhis.rules.models.RuleValueType
10+
import org.hisp.dhis.rules.models.RuleVariableCurrentEvent
11+
import org.hisp.dhis.rules.models.RuleVariableNewestEvent
12+
import org.hisp.dhis.rules.models.RuleVariableNewestStageEvent
13+
import org.hisp.dhis.rules.models.RuleVariablePreviousEvent
14+
import kotlin.test.Test
15+
import kotlin.test.assertEquals
16+
import kotlin.test.assertFalse
17+
import kotlin.test.assertTrue
18+
19+
class RuleEngineAnalyzerTest {
20+
21+
private fun variable(
22+
name: String,
23+
field: String = "field_$name",
24+
) = RuleVariableCurrentEvent(
25+
name = name,
26+
useCodeForOptionSet = false,
27+
options = emptyList<Option>(),
28+
field = field,
29+
fieldType = RuleValueType.TEXT,
30+
)
31+
32+
private fun newestEvent(name: String) = RuleVariableNewestEvent(
33+
name = name,
34+
useCodeForOptionSet = false,
35+
options = emptyList<Option>(),
36+
field = "field_$name",
37+
fieldType = RuleValueType.TEXT,
38+
)
39+
40+
private fun newestStageEvent(name: String) = RuleVariableNewestStageEvent(
41+
name = name,
42+
useCodeForOptionSet = false,
43+
options = emptyList<Option>(),
44+
field = "field_$name",
45+
fieldType = RuleValueType.TEXT,
46+
programStage = "stage1",
47+
)
48+
49+
private fun previousEvent(name: String) = RuleVariablePreviousEvent(
50+
name = name,
51+
useCodeForOptionSet = false,
52+
options = emptyList<Option>(),
53+
field = "field_$name",
54+
fieldType = RuleValueType.TEXT,
55+
)
56+
57+
private fun rule(condition: String, vararg actionData: String?): Rule {
58+
val actions = actionData.map { RuleAction(it, "DISPLAYTEXT") }
59+
return Rule(condition, actions)
60+
}
61+
62+
// --- needsAllEvents = false cases ---
63+
64+
@Test
65+
fun emptyRulesAndVariablesReturnsFalse() {
66+
val result = RuleEngineAnalyzer.analyzeContextRequirements(emptyList(), emptyList())
67+
assertEquals(RuleContextRequirements.NONE, result)
68+
assertFalse(result.needsAllEvents)
69+
}
70+
71+
@Test
72+
fun unrelatedRulesAndVariablesReturnFalse() {
73+
val rules = listOf(rule("#{score} > 5", "#{score} + 1"))
74+
val variables = listOf(variable("score"))
75+
val result = RuleEngineAnalyzer.analyzeContextRequirements(rules, variables)
76+
assertFalse(result.needsAllEvents)
77+
}
78+
79+
@Test
80+
fun currentEventVariableDoesNotTriggerAllEvents() {
81+
val rules = listOf(rule("#{current_val} > 0"))
82+
val variables = listOf(variable("current_val"))
83+
assertFalse(RuleEngineAnalyzer.analyzeContextRequirements(rules, variables).needsAllEvents)
84+
}
85+
86+
// --- needsAllEvents = true via env-var ---
87+
88+
@Test
89+
fun eventCountEnvVarInConditionTriggersAllEvents() {
90+
val rules = listOf(rule("V{event_count} > 2"))
91+
assertTrue(RuleEngineAnalyzer.analyzeContextRequirements(rules, emptyList()).needsAllEvents)
92+
}
93+
94+
@Test
95+
fun eventCountEnvVarInActionDataTriggersAllEvents() {
96+
val rules = listOf(rule("true", "V{event_count}"))
97+
assertTrue(RuleEngineAnalyzer.analyzeContextRequirements(rules, emptyList()).needsAllEvents)
98+
}
99+
100+
@Test
101+
fun unrelatedEnvVarDoesNotTriggerAllEvents() {
102+
val rules = listOf(rule("V{enrollment_date} > '2020-01-01'"))
103+
assertFalse(RuleEngineAnalyzer.analyzeContextRequirements(rules, emptyList()).needsAllEvents)
104+
}
105+
106+
// --- needsAllEvents = true via variable type ---
107+
108+
@Test
109+
fun newestEventVariableTriggersAllEvents() {
110+
val rules = listOf(rule("#{weight} > 0"))
111+
val variables = listOf(newestEvent("weight"))
112+
assertTrue(RuleEngineAnalyzer.analyzeContextRequirements(rules, variables).needsAllEvents)
113+
}
114+
115+
@Test
116+
fun newestStageEventVariableTriggersAllEvents() {
117+
val rules = listOf(rule("#{bp} > 90"))
118+
val variables = listOf(newestStageEvent("bp"))
119+
assertTrue(RuleEngineAnalyzer.analyzeContextRequirements(rules, variables).needsAllEvents)
120+
}
121+
122+
@Test
123+
fun previousEventVariableTriggersAllEvents() {
124+
val rules = listOf(rule("#{prev_weight} < #{weight}"))
125+
val variables = listOf(previousEvent("prev_weight"), variable("weight"))
126+
assertTrue(RuleEngineAnalyzer.analyzeContextRequirements(rules, variables).needsAllEvents)
127+
}
128+
129+
@Test
130+
fun variableDefinedButNotReferencedDoesNotTrigger() {
131+
// The variable is a "needsAllEvents" type, but it isn't referenced in any rule
132+
val rules = listOf(rule("#{other} > 0"))
133+
val variables = listOf(newestEvent("unreferenced"), variable("other"))
134+
assertFalse(RuleEngineAnalyzer.analyzeContextRequirements(rules, variables).needsAllEvents)
135+
}
136+
137+
@Test
138+
fun newestEventVariableReferencedInActionDataTriggers() {
139+
val rules = listOf(rule("true", "#{latest_score}"))
140+
val variables = listOf(newestEvent("latest_score"))
141+
assertTrue(RuleEngineAnalyzer.analyzeContextRequirements(rules, variables).needsAllEvents)
142+
}
143+
144+
// --- NONE constant ---
145+
146+
@Test
147+
fun noneConstantHasNeedsAllEventsFalse() {
148+
assertFalse(RuleContextRequirements.NONE.needsAllEvents)
149+
}
150+
151+
// --- RuleEngine interface delegation ---
152+
153+
@Test
154+
fun ruleEngineAnalyzeContextRequirementsDelegatesCorrectly() {
155+
val rules = listOf(rule("V{event_count} > 2"))
156+
assertTrue(RuleEngine.getInstance().analyzeContextRequirements(rules, emptyList()).needsAllEvents)
157+
}
158+
159+
@Test
160+
fun ruleEngineAnalyzeContextRequirementsReturnsFalseForUnrelatedContext() {
161+
val rules = listOf(rule("#{score} > 5"))
162+
val variables = listOf(variable("score"))
163+
assertFalse(RuleEngine.getInstance().analyzeContextRequirements(rules, variables).needsAllEvents)
164+
}
165+
}

0 commit comments

Comments
 (0)