fix: contains and containsItems with explicit pattern argument and re…

…quires validation [DHIS2-16211]
dhis2 · Mar 4, 2024 · 9992e91 · 9992e91
1 parent 5981235
commit 9992e91
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 36 deletions.
diff --git a/src/commonMain/kotlin/org/hisp/dhis/lib/expression/ast/NamedFunction.kt b/src/commonMain/kotlin/org/hisp/dhis/lib/expression/ast/NamedFunction.kt
@@ -9,8 +9,8 @@ enum class NamedFunction(
     vararg parameterTypes: ValueType
 ) : Typed {
     // Base Functions
-    contains("contains", ValueType.BOOLEAN, true, ValueType.STRING),
-    containsItems("containsItems", ValueType.BOOLEAN, true, ValueType.STRING),
+    contains("contains", ValueType.BOOLEAN, true, ValueType.STRING, ValueType.STRING, ValueType.STRING),
+    containsItems("containsItems", ValueType.BOOLEAN, true, ValueType.STRING, ValueType.STRING, ValueType.STRING),
     firstNonNull("firstNonNull", ValueType.SAME, true, ValueType.SAME),
     greatest("greatest", ValueType.NUMBER, true, ValueType.NUMBER),
     ifThenElse("if", ValueType.SAME, ValueType.BOOLEAN, ValueType.SAME, ValueType.SAME),

diff --git a/src/commonMain/kotlin/org/hisp/dhis/lib/expression/eval/Calculator.kt b/src/commonMain/kotlin/org/hisp/dhis/lib/expression/eval/Calculator.kt
@@ -56,8 +56,8 @@ internal class Calculator(
             null // return value of only data item in the expression from map
         }
         else when (fnInfo) {
-            NamedFunction.contains -> functions.contains(evalToStrings(fn.children()))
-            NamedFunction.containsItems -> functions.containsItems(evalToStrings(fn.children()))
+            NamedFunction.contains -> functions.contains(evalToString(fn.child(0)), evalToStrings(fn.children()).drop(1))
+            NamedFunction.containsItems -> functions.containsItems(evalToString(fn.child(0)), evalToStrings(fn.children()).drop(1))
             NamedFunction.firstNonNull -> functions.firstNonNull(evalToMixed(fn.children()))
             NamedFunction.greatest -> functions.greatest(evalToNumbers(fn.children()))
             NamedFunction.ifThenElse -> functions.ifThenElse(

diff --git a/src/commonMain/kotlin/org/hisp/dhis/lib/expression/spi/ExpressionFunctions.kt b/src/commonMain/kotlin/org/hisp/dhis/lib/expression/spi/ExpressionFunctions.kt
@@ -24,23 +24,26 @@ fun interface ExpressionFunctions {
     fun unsupported(name: String): Any?
 
     /**
-     * Returns true if first arg contains all others as substrings.
+     * @param pattern the string checked to contain the all the other substrings
+     * @param allOf the substrings that are checked to be contained
+     * @return true, the pattern parameter contains all the values of the #allOf parameter
      */
-    fun contains(values: List<String?>): Boolean {
-        val targetValue = values[0] ?: ""
-        val containedValues = values.drop(1)
-
-        return containedValues.all { it != null && targetValue.contains(it) }
+    fun contains(pattern: String?, allOf: List<String?>): Boolean {
+        require(pattern != null) { "pattern parameter of contains must not be null" }
+        require(allOf.isNotEmpty()) { "allOf parameter of contains contain at least one value" }
+        return allOf.all { it != null && pattern.contains(it) }
     }
 
     /**
-     * Returns true if first arg split by commas contains all others as exact matches.
+     * @param pattern the string checked to contain the all the other substrings
+     * @param allOf the substrings that are checked to be contained
+     * @return true if pattern split by comma contains all others as exact matches.
      */
-    fun containsItems(values: List<String?>): Boolean {
-        val targetValues = (values[0] ?: "").split(",")
-        val containedValues = values.drop(1)
-
-        return containedValues.all { targetValues.contains(it) }
+    fun containsItems(pattern: String?, allOf: List<String?>): Boolean {
+        require(pattern != null) { "pattern parameter of contains must not be null" }
+        require(allOf.isNotEmpty()) { "allOf parameter of contains contain at least one value" }
+        val patterns = pattern.split(",")
+        return allOf.all { patterns.contains(it) }
     }
 
     /**

diff --git a/src/commonMain/kotlin/org/hisp/dhis/lib/expression/syntax/ExpressionGrammar.kt b/src/commonMain/kotlin/org/hisp/dhis/lib/expression/syntax/ExpressionGrammar.kt
@@ -46,8 +46,8 @@ object ExpressionGrammar {
     private val SUB_EXPRESSION = fn(NamedFunction.subExpression, expr)
 
     private val CommonFunctions = listOf( // (alphabetical)
-        fn(NamedFunction.contains, expr, expr, expr.plus()),
-        fn(NamedFunction.containsItems, expr, expr, expr.plus()),
+        fn(NamedFunction.contains, expr, expr.plus()),
+        fn(NamedFunction.containsItems, expr, expr.plus()),
         fn(NamedFunction.firstNonNull, expr.plus()),
         fn(NamedFunction.greatest, expr.plus()),
         fn(NamedFunction.ifThenElse, expr, expr, expr),

diff --git a/src/commonTest/kotlin/org/hisp/dhis/lib/expression/function/ContainsItemsTest.kt b/src/commonTest/kotlin/org/hisp/dhis/lib/expression/function/ContainsItemsTest.kt
@@ -2,11 +2,7 @@ package org.hisp.dhis.lib.expression.function
 
 import org.hisp.dhis.lib.expression.Expression
 import org.hisp.dhis.lib.expression.Expression.Mode
-
-import kotlin.test.Test
-import kotlin.test.assertFailsWith
-import kotlin.test.assertFalse
-import kotlin.test.assertTrue
+import kotlin.test.*
 
 /**
  * Tests the `contains` function.
@@ -18,21 +14,24 @@ internal class ContainsItemsTest {
     fun testContainsItems() {
         assertTrue(evaluate("containsItems('MOLD_ALLERGY,LATEX_ALLERGY', 'MOLD_ALLERGY')"))
         assertTrue(evaluate("containsItems('MOLD_ALLERGY,LATEX_ALLERGY', 'LATEX_ALLERGY', 'MOLD_ALLERGY')"))
+        assertTrue(evaluate("containsItems('abcdef', 'abcdef')"))
+
         assertFalse(evaluate("containsItems('MOLD_ALLERGY,LATEX_ALLERGY', 'ALLERGY')"))
         assertFalse(evaluate("containsItems('MOLD_ALLERGY,LATEX_ALLERGY', 'RGY,LAT')"))
-        assertTrue(evaluate("containsItems('abcdef', 'abcdef')"))
         assertFalse(evaluate("containsItems('abcdef', 'bcd')"))
         assertFalse(evaluate("containsItems('abcdef', 'xyz')"))
+        assertFalse(evaluate("containsItems('abcdef', null)"))
     }
 
     @Test
-    fun testContainsItemsNoArgs() {
+    fun testContainsItems_NoArgs() {
         assertFailsWith(IndexOutOfBoundsException::class) { evaluate("containsItems()") }
     }
 
     @Test
-    fun testContainsItemsOneArg() {
-        assertTrue(evaluate("containsItems('abcdef')"))
+    fun testContainsItems_OneArg() {
+        val ex = assertFailsWith(IllegalArgumentException::class) { evaluate("containsItems('abcdef')") }
+        assertEquals("allOf parameter of contains contain at least one value", ex.message)
     }
 
     private fun evaluate(expression: String): Boolean {

diff --git a/src/commonTest/kotlin/org/hisp/dhis/lib/expression/function/ContainsTest.kt b/src/commonTest/kotlin/org/hisp/dhis/lib/expression/function/ContainsTest.kt
@@ -2,11 +2,7 @@ package org.hisp.dhis.lib.expression.function
 
 import org.hisp.dhis.lib.expression.Expression
 import org.hisp.dhis.lib.expression.Expression.Mode
-
-import kotlin.test.Test
-import kotlin.test.assertFailsWith
-import kotlin.test.assertFalse
-import kotlin.test.assertTrue
+import kotlin.test.*
 
 /**
  * Tests the `contains` function.
@@ -22,18 +18,20 @@ internal class ContainsTest {
         assertTrue(evaluate("contains('MOLD_ALLERGY,LATEX_ALLERGY', 'RGY,LAT')"))
         assertTrue(evaluate("contains('abcdef', 'abcdef')"))
         assertTrue(evaluate("contains('abcdef', 'bcd')"))
+
         assertFalse(evaluate("contains('abcdef', 'xyz')"))
-        assertTrue(evaluate("contains('abcdef')"))
+        assertFalse(evaluate("contains('abcdef', null)"))
     }
 
     @Test
-    fun testContainsNoArgs() {
+    fun testContains_NoArgs() {
         assertFailsWith(IndexOutOfBoundsException::class) { evaluate("contains()") }
     }
 
     @Test
-    fun testContainsOneArg() {
-        assertTrue(evaluate("contains('abcdef')"))
+    fun testContains_OneArg() {
+        val ex = assertFailsWith(IllegalArgumentException::class) { evaluate("contains('abcdef')") }
+        assertEquals("allOf parameter of contains contain at least one value", ex.message)
     }
 
     private fun evaluate(expression: String): Boolean {