Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions api/expression-parser.api
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public final class org/hisp/dhis/lib/expression/Expression {
}

public final class org/hisp/dhis/lib/expression/ExpressionMode : java/lang/Enum {
public static final field ANDROID_CUSTOM_INTENT_EXPRESSION Lorg/hisp/dhis/lib/expression/ExpressionMode;
public static final field INDICATOR_EXPRESSION Lorg/hisp/dhis/lib/expression/ExpressionMode;
public static final field PREDICTOR_GENERATOR_EXPRESSION Lorg/hisp/dhis/lib/expression/ExpressionMode;
public static final field PREDICTOR_SKIP_TEST Lorg/hisp/dhis/lib/expression/ExpressionMode;
Expand Down Expand Up @@ -361,6 +362,10 @@ public final class org/hisp/dhis/lib/expression/ast/Nodes$AggregationTypeNode :
public fun <init> (Lorg/hisp/dhis/lib/expression/ast/NodeType;Ljava/lang/String;)V
}

public final class org/hisp/dhis/lib/expression/ast/Nodes$AndroidCustomIntentNode : org/hisp/dhis/lib/expression/ast/Nodes$SimpleNode {
public fun <init> (Lorg/hisp/dhis/lib/expression/ast/NodeType;Ljava/lang/String;)V
}

public final class org/hisp/dhis/lib/expression/ast/Nodes$ArgumentNode : org/hisp/dhis/lib/expression/ast/Nodes$ComplexNode {
public fun <init> (Lorg/hisp/dhis/lib/expression/ast/NodeType;Ljava/lang/String;)V
public fun getValueType ()Lorg/hisp/dhis/lib/expression/spi/ValueType;
Expand Down Expand Up @@ -1304,6 +1309,17 @@ public final class org/hisp/dhis/lib/expression/spi/AggregationType : java/lang/
public static fun values ()[Lorg/hisp/dhis/lib/expression/spi/AggregationType;
}

public final class org/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable : java/lang/Enum {
public static final field orgunit_code Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
public static final field orgunit_id Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
public static final field orgunit_path Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
public static final field user_id Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
public static final field user_username Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
public static fun getEntries ()Lkotlin/enums/EnumEntries;
public static fun valueOf (Ljava/lang/String;)Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
public static fun values ()[Lorg/hisp/dhis/lib/expression/spi/AndroidCustomIntentVariable;
}

public final class org/hisp/dhis/lib/expression/spi/DataItem {
public fun <init> (Lorg/hisp/dhis/lib/expression/spi/DataItemType;Lorg/hisp/dhis/lib/expression/spi/ID;Ljava/util/List;Ljava/util/List;Lorg/hisp/dhis/lib/expression/spi/QueryModifiers;)V
public fun <init> (Lorg/hisp/dhis/lib/expression/spi/DataItemType;Lorg/hisp/dhis/lib/expression/spi/ID;Lorg/hisp/dhis/lib/expression/spi/QueryModifiers;)V
Expand All @@ -1327,6 +1343,7 @@ public final class org/hisp/dhis/lib/expression/spi/DataItem {
}

public final class org/hisp/dhis/lib/expression/spi/DataItemType : java/lang/Enum {
public static final field ANDROID_CUSTOM_INTENT Lorg/hisp/dhis/lib/expression/spi/DataItemType;
Copy link
Member Author

Choose a reason for hiding this comment

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

There are many things in the public API that must be internal, such as this enum. I didn't want to make it internal just to be consistent with the rest of the modes. @jbee should we create a task to clean this up? I can push it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, new task. I think I first should also do the PR with the unified context map. Then we can do the cleanup.

public static final field ATTRIBUTE Lorg/hisp/dhis/lib/expression/spi/DataItemType;
public static final field CONSTANT Lorg/hisp/dhis/lib/expression/spi/DataItemType;
public static final field Companion Lorg/hisp/dhis/lib/expression/spi/DataItemType$Companion;
Expand Down Expand Up @@ -1504,6 +1521,7 @@ public final class org/hisp/dhis/lib/expression/spi/ID {
}

public final class org/hisp/dhis/lib/expression/spi/IDType : java/lang/Enum {
public static final field AndroidCustomIntent Lorg/hisp/dhis/lib/expression/spi/IDType;
public static final field AttributeOptionComboUID Lorg/hisp/dhis/lib/expression/spi/IDType;
public static final field AttributeUID Lorg/hisp/dhis/lib/expression/spi/IDType;
public static final field CategoryOptionComboUID Lorg/hisp/dhis/lib/expression/spi/IDType;
Expand Down Expand Up @@ -1744,6 +1762,7 @@ public final class org/hisp/dhis/lib/expression/syntax/Expr$Companion {

public final class org/hisp/dhis/lib/expression/syntax/ExpressionGrammar {
public static final field INSTANCE Lorg/hisp/dhis/lib/expression/syntax/ExpressionGrammar;
public final fun getAndroidCustomIntentMode ()Ljava/util/List;
public final fun getIndicatorExpressionMode ()Ljava/util/List;
public final fun getPredictorExpressionMode ()Ljava/util/List;
public final fun getPredictorSkipTestMode ()Ljava/util/List;
Expand Down
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ repositories {
mavenCentral()
}

version = "1.2.0-SNAPSHOT"
version = "1.2.1-SNAPSHOT"
group = "org.hisp.dhis.lib.expression"

if (project.hasProperty("removeSnapshotSuffix")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@ enum class ExpressionMode(
ValueType.BOOLEAN,
ValueType.STRING,
ValueType.NUMBER,
ValueType.DATE);
ValueType.DATE),

// android custom intent request parameters
ANDROID_CUSTOM_INTENT_EXPRESSION(
ExpressionGrammar.AndroidCustomIntentMode,
ValueType.BOOLEAN,
ValueType.STRING,
ValueType.NUMBER);

internal val resultTypes: Set<ValueType> = setOf(*resultTypes)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,10 @@ object Nodes {
SimpleNode<ProgramVariable>(type, rawValue, ProgramVariable::valueOf, rethrowAs(
ProgramVariable::class.simpleName, ProgramVariable.entries, ProgramVariable::name))

class AndroidCustomIntentNode(type: NodeType, rawValue: String) :
SimpleNode<AndroidCustomIntentVariable>(type, rawValue, AndroidCustomIntentVariable::valueOf, rethrowAs(
AndroidCustomIntentVariable::class.simpleName, AndroidCustomIntentVariable.entries, AndroidCustomIntentVariable::name))

class NamedValueNode(type: NodeType, rawValue: String) :
SimpleNode<NamedValue>(type, rawValue, NamedValue::valueOf, rethrowAs(
NamedValue::class.simpleName, NamedValue.entries, NamedValue::name))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.hisp.dhis.lib.expression.spi

import kotlin.js.JsExport

@JsExport
enum class AndroidCustomIntentVariable {
Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to create an enum just to be able to validate the expression.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Using an enum sure makes VAR{...} just usable by android (unless someone has the exact use case of one or more of these variables). It is still fine but it is somewhat of an odd mix to pair a generic name with a very specific usability. Anyhow, the expression syntax is already odd so I think we just go with what you like better. If we ever want to make something that composes better we would have to invent a 2.0 version anyhow.

FYI: When I make this into the context key-value the key would be a wrapper on this enum or the enum itself if it can implement the interface. For the value we should talk about if you rather want a wrapper on Any or a wrapper having 1 field per possible return type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. This variable name can be changed later if we find that is messing things up. The only client will probably be the Android app, unless for a while.

Anyway, wouldn't be possible to create another node that uses VAR{} and links to another enum of variables? I got the feeling the other day after our conversation that it was possible to override the those symbols and link them to a different node. Just for curiosity.

orgunit_code,
orgunit_id,
orgunit_path,
user_id,
user_username
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ enum class DataItemType(internal val symbol: String, private val parameterTypes:
INDICATOR("N", IDType.IndicatorUID),
ORG_UNIT_GROUP("OUG", IDType.OrganisationUnitGroupUID),
REPORTING_RATE("R", IDType.DataSetUID, IDType.ReportingRateType),
PROGRAM_VARIABLE("V", IDType.ProgramVariableName);
PROGRAM_VARIABLE("V", IDType.ProgramVariableName),
ANDROID_CUSTOM_INTENT("VAR", IDType.AndroidCustomIntent);

constructor(symbol: String, vararg parameterTypes: IDType) : this(
symbol, listOf<List<IDType>>(listOf<IDType>(*parameterTypes)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import kotlin.js.JsExport

@JsExport
enum class IDType {
AndroidCustomIntent,
AttributeUID,
AttributeOptionComboUID,
CategoryOptionUID,
Expand All @@ -24,6 +25,6 @@ enum class IDType {
ReportingRateType;

fun isUID(): Boolean {
return this != ProgramVariableName && this != ReportingRateType;
return this != ProgramVariableName && this != ReportingRateType && this != AndroidCustomIntent
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.hisp.dhis.lib.expression.ast.NamedFunction
import org.hisp.dhis.lib.expression.ast.Node
import org.hisp.dhis.lib.expression.ast.NodeType
import org.hisp.dhis.lib.expression.ast.Nodes.AggregationTypeNode
import org.hisp.dhis.lib.expression.ast.Nodes.AndroidCustomIntentNode
import org.hisp.dhis.lib.expression.ast.Nodes.ProgramVariableNode
import org.hisp.dhis.lib.expression.ast.Nodes.ReportingRateTypeNode
import org.hisp.dhis.lib.expression.spi.DataItemType
Expand Down Expand Up @@ -150,6 +151,7 @@ object ExpressionGrammar {
private val OUG_BRACE = item(DataItemType.ORG_UNIT_GROUP, UID)
private val N_BRACE = item(DataItemType.INDICATOR, UID)
private val V_BRACE = variable(DataItemType.PROGRAM_VARIABLE, IDENTIFIER.by(Node.Factory.new(::ProgramVariableNode)))
private val ACI_BRACE = variable(DataItemType.ANDROID_CUSTOM_INTENT, IDENTIFIER.by(Node.Factory.new(::AndroidCustomIntentNode)))
private val CommonDataItems = listOf(HASH_BRACE, A_BRACE, C_BRACE, D_BRACE, I_BRACE, R_BRACE, OUG_BRACE)

/*
Expand Down Expand Up @@ -197,6 +199,15 @@ object ExpressionGrammar {
RuleEngineD2Functions,
listOf(HASH_BRACE, A_BRACE, C_BRACE, V_BRACE))

val AndroidCustomIntentMode = concat(
CommonBooleanFunctions,
CommonConstants,
CommonNumberFunctions,
CommonSameFunctions,
CommonD2Functions,
RuleEngineD2Functions,
listOf(ACI_BRACE))

/*
Block expressions
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package org.hisp.dhis.lib.expression

import org.hisp.dhis.lib.expression.spi.ExpressionData
import org.hisp.dhis.lib.expression.spi.ParseException
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

/**
* Tests that the Android Custom Intent mode
*/
internal class AndroidCustomIntentExpressionTest {

@Test
fun testLiteralValues() {
assertEquals("faJk7SeA5e", evaluate("'faJk7SeA5e'"))
assertEquals(5.0, evaluate("5"))
assertEquals(5.5, evaluate("5.5"))
assertEquals(true, evaluate("true"))
}

@Test
fun testSecondLevelOrgunit() {
val data = ExpressionData().copy(
programVariableValues = mapOf(
"orgunit_path" to "/ImspTQPwCqd/O6uvpzGd5pu/YuQRtpLP10I/g8upMTyEZGZ"
)
)

assertEquals("O6uvpzGd5pu", evaluate("d2:split(VAR{orgunit_path}, '/', 2)", data))
}

@Test
fun testConcatenateUserOrgunit() {
val data = ExpressionData().copy(
programVariableValues = mapOf(
"user_username" to "admin",
"orgunit_code" to "OUC32"
)
)

assertEquals(
"admin_OUC32",
evaluate("d2:concatenate(VAR{user_username}, '_', VAR{orgunit_code})", data)
)
}

@Test
fun testValidateExpression() {
validate("VAR{orgunit_code}", valid = true)
validate("VAR{invalid_var}", valid = false)
}

companion object {
private fun evaluate(expression: String, data: ExpressionData = ExpressionData()): Any? {
return Expression(expression, ExpressionMode.ANDROID_CUSTOM_INTENT_EXPRESSION).evaluate(data)
}

private fun validate(expression: String, valid: Boolean) {
try {
Expression(expression, ExpressionMode.ANDROID_CUSTOM_INTENT_EXPRESSION).validate(mapOf())
assertTrue(valid)
} catch (e: ParseException) {
assertFalse(valid)
}
}
}
}