Allow duplicate leaf names in Cps Path leaf condition

Presently, a query using the same leaf name twice will fail:
 //books[@price > 10 and @price < 20]
It is caused by storing data leaves in a Map.
This is fixed by storing data leaves in a List<DataLeaf>.

Issue-ID: CPS-1779
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: Ie6990ea5e622cf37e986b720a0a07fb69ce4f03b

5 files changed, 64 insertions, 158 deletions

diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBooleanOperatorType.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBooleanOperatorType.java
deleted file mode 100644
index b2f1dddb19..0000000000
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBooleanOperatorType.java
+++ /dev/null
@@ -1,51 +0,0 @@
-/*

- *  ============LICENSE_START=======================================================

- *  Copyright (C) 2023 TechMahindra Ltd

- *  ================================================================================

- *  Licensed under the Apache License, Version 2.0 (the "License");

- *  you may not use this file except in compliance with the License.

- *  You may obtain a copy of the License at

- *

- *        http://www.apache.org/licenses/LICENSE-2.0

- *

- *  Unless required by applicable law or agreed to in writing, software

- *  distributed under the License is distributed on an "AS IS" BASIS,

- *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

- *  See the License for the specific language governing permissions and

- *  limitations under the License.

- *

- *  SPDX-License-Identifier: Apache-2.0

- *  ============LICENSE_END=========================================================

- */

-

-package org.onap.cps.cpspath.parser;

-

-public enum CpsPathBooleanOperatorType {

-    AND("and"),

-    OR("or");

-

-    private final String operatorValue;

-

-    CpsPathBooleanOperatorType(final String operatorValue) {

-        this.operatorValue = operatorValue;

-    }

-

-    public String getValues() {

-        return this.operatorValue;

-    }

-

-    /**

-     * Finds the value of the given enumeration.

-     *

-     * @param operatorValue value of the enum

-     * @return a booleanOperatorType

-     */

-    public static CpsPathBooleanOperatorType fromString(final String operatorValue) {

-        for (final CpsPathBooleanOperatorType booleanOperatorType : CpsPathBooleanOperatorType.values()) {

-            if (booleanOperatorType.operatorValue.equalsIgnoreCase(operatorValue)) {

-                return booleanOperatorType;

-            }

-        }

-        return null;

-    }

-}

diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java
index 5c47127375..99135962f8 100644
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java
+++ b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2023 Nordix Foundation
  *  Modifications Copyright (C) 2023 TechMahindra Ltd
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,9 +24,7 @@ package org.onap.cps.cpspath.parser;
 import static org.onap.cps.cpspath.parser.CpsPathPrefixType.DESCENDANT;
 
 import java.util.ArrayList;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import org.onap.cps.cpspath.parser.antlr4.CpsPathBaseListener;
 import org.onap.cps.cpspath.parser.antlr4.CpsPathParser;
 import org.onap.cps.cpspath.parser.antlr4.CpsPathParser.AncestorAxisContext;
@@ -43,21 +41,21 @@ public class CpsPathBuilder extends CpsPathBaseListener {
 
     private static final String CLOSE_BRACKET = "]";
 
-    final CpsPathQuery cpsPathQuery = new CpsPathQuery();
+    private final CpsPathQuery cpsPathQuery = new CpsPathQuery();
 
-    final Map<String, Object> leavesData = new LinkedHashMap<>();
+    private final List<CpsPathQuery.DataLeaf> leavesData = new ArrayList<>();
 
-    final StringBuilder normalizedXpathBuilder = new StringBuilder();
+    private final StringBuilder normalizedXpathBuilder = new StringBuilder();
 
-    final StringBuilder normalizedAncestorPathBuilder = new StringBuilder();
+    private final StringBuilder normalizedAncestorPathBuilder = new StringBuilder();
 
-    boolean processingAncestorAxis = false;
+    private boolean processingAncestorAxis = false;
 
-    private List<String> containerNames = new ArrayList<>();
+    private final List<String> containerNames = new ArrayList<>();
 
-    final List<String> booleanOperators = new ArrayList<>();
+    private final List<String> booleanOperators = new ArrayList<>();
 
-    final List<String> comparativeOperators = new ArrayList<>();
+    private final List<String> comparativeOperators = new ArrayList<>();
 
     @Override
     public void exitInvalidPostFix(final CpsPathParser.InvalidPostFixContext ctx) {
@@ -99,16 +97,12 @@ public class CpsPathBuilder extends CpsPathBaseListener {
 
     @Override
     public void exitBooleanOperators(final CpsPathParser.BooleanOperatorsContext ctx) {
-        final CpsPathBooleanOperatorType cpsPathBooleanOperatorType = CpsPathBooleanOperatorType.fromString(
-                ctx.getText());
-        booleanOperators.add(cpsPathBooleanOperatorType.getValues());
+        booleanOperators.add(ctx.getText());
     }
 
     @Override
     public void exitComparativeOperators(final CpsPathParser.ComparativeOperatorsContext ctx) {
-        final CpsPathComparativeOperator cpsPathComparativeOperator = CpsPathComparativeOperator.fromString(
-                ctx.getText());
-        comparativeOperators.add(cpsPathComparativeOperator.getLabel());
+        comparativeOperators.add(ctx.getText());
     }
 
     @Override
@@ -122,6 +116,8 @@ public class CpsPathBuilder extends CpsPathBaseListener {
     public void enterMultipleLeafConditions(final MultipleLeafConditionsContext ctx)  {
         normalizedXpathBuilder.append(OPEN_BRACKET);
         leavesData.clear();
+        booleanOperators.clear();
+        comparativeOperators.clear();
     }
 
     @Override
@@ -193,7 +189,7 @@ public class CpsPathBuilder extends CpsPathBaseListener {
     }
 
     private void leafContext(final CpsPathParser.LeafNameContext ctx, final Object comparisonValue) {
-        leavesData.put(ctx.getText(), comparisonValue);
+        leavesData.add(new CpsPathQuery.DataLeaf(ctx.getText(), comparisonValue));
         appendCondition(normalizedXpathBuilder, ctx.getText(), comparisonValue);
         if (processingAncestorAxis) {
             appendCondition(normalizedAncestorPathBuilder, ctx.getText(), comparisonValue);
diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathComparativeOperator.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathComparativeOperator.java
deleted file mode 100644
index c7ffd0d7ec..0000000000
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathComparativeOperator.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*

- *  ============LICENSE_START=======================================================

- *  Copyright (C) 2023 Tech Mahindra Ltd

- *  ================================================================================

- *  Licensed under the Apache License, Version 2.0 (the "License");

- *  you may not use this file except in compliance with the License.

- *  You may obtain a copy of the License at

- *

- *        http://www.apache.org/licenses/LICENSE-2.0

- *

- *  Unless required by applicable law or agreed to in writing, software

- *  distributed under the License is distributed on an "AS IS" BASIS,

- *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

- *  See the License for the specific language governing permissions and

- *  limitations under the License.

- *

- *  SPDX-License-Identifier: Apache-2.0

- *  ============LICENSE_END=========================================================

- */

-

-package org.onap.cps.cpspath.parser;

-

-import java.util.HashMap;

-import java.util.Map;

-

-public enum CpsPathComparativeOperator {

-    EQ("="),

-    GT(">"),

-    LT("<"),

-    GE(">="),

-    LE("<=");

-

-    private final String label;

-

-    CpsPathComparativeOperator(final String label) {

-        this.label = label;

-    }

-

-    public final String getLabel() {

-        return this.label;

-    }

-

-    private static final Map<String, CpsPathComparativeOperator> cpsPathComparativeOperatorPerLabel = new HashMap<>();

-

-    static {

-        for (final CpsPathComparativeOperator cpsPathComparativeOperator : CpsPathComparativeOperator.values()) {

-            cpsPathComparativeOperatorPerLabel.put(cpsPathComparativeOperator.label, cpsPathComparativeOperator);

-        }

-    }

-

-    /**

-     * Finds the value of the given enumeration.

-     *

-     * @param label value of the enum

-     * @return a comparativeOperatorType

-     */

-    public static CpsPathComparativeOperator fromString(final String label) {

-        if (!cpsPathComparativeOperatorPerLabel.containsKey(label)) {

-            throw new PathParsingException("Incomplete leaf condition (no operator)");

-        }

-        return cpsPathComparativeOperatorPerLabel.get(label);

-    }

-}

-

diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java
index 3c3cbccf7e..f98df05a28 100644
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java
+++ b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java
@@ -24,8 +24,9 @@ package org.onap.cps.cpspath.parser;
 import static org.onap.cps.cpspath.parser.CpsPathPrefixType.ABSOLUTE;
 
 import java.util.List;
-import java.util.Map;
 import lombok.AccessLevel;
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.Setter;
 
@@ -39,7 +40,7 @@ public class CpsPathQuery {
     private List<String> containerNames;
     private CpsPathPrefixType cpsPathPrefixType = ABSOLUTE;
     private String descendantName;
-    private Map<String, Object> leavesData;
+    private List<DataLeaf> leavesData;
     private String ancestorSchemaNodeIdentifier = "";
     private String textFunctionConditionLeafName;
     private String textFunctionConditionValue;
@@ -103,4 +104,11 @@ public class CpsPathQuery {
         return cpsPathPrefixType == ABSOLUTE && hasLeafConditions();
     }
 
+    @Getter
+    @EqualsAndHashCode
+    @AllArgsConstructor
+    public static class DataLeaf {
+        private final String name;
+        private final Object value;
+    }
 }
diff --git a/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy b/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy
index 9ab5491b5d..78963033da 100644
--- a/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy
+++ b/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2023 Nordix Foundation
  *  Modifications Copyright (C) 2023 TechMahindra Ltd
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -32,12 +32,12 @@ class CpsPathQuerySpec extends Specification {
         when: 'the given cps path is parsed'
             def result = CpsPathQuery.createFrom(cpsPath)
         then: 'the query has the right xpath type'
-            result.cpsPathPrefixType == ABSOLUTE
+            assert result.cpsPathPrefixType == ABSOLUTE
         and: 'the right query parameters are set'
-            result.xpathPrefix == expectedXpathPrefix
-            result.hasLeafConditions()
-            result.leavesData.containsKey(expectedLeafName)
-            result.leavesData.get(expectedLeafName) == expectedLeafValue
+            assert result.xpathPrefix == expectedXpathPrefix
+            assert result.hasLeafConditions()
+            assert result.leavesData[0].name == expectedLeafName
+            assert result.leavesData[0].value == expectedLeafValue
         where: 'the following data is used'
             scenario               | cpsPath                                                    || expectedXpathPrefix                             | expectedLeafName       | expectedLeafValue
             'leaf of type String'  | '/parent/child[@common-leaf-name="common-leaf-value"]'     || '/parent/child'                                 | 'common-leaf-name'     | 'common-leaf-value'
@@ -103,31 +103,24 @@ class CpsPathQuerySpec extends Specification {
             'descendant anywhere'  | '//xpath'  || '//xpath'
     }
 
-    def 'Parse cps path that ends with a yang list containing #scenario.'() {
+    def 'Parse cps path that ends with a yang list containing multiple leaf conditions.'() {
         when: 'the given cps path is parsed'
             def result = CpsPathQuery.createFrom(cpsPath)
-        then: 'the query has the right xpath type'
-            result.cpsPathPrefixType == DESCENDANT
-        and: 'the right parameters are set'
-            result.descendantName == "child"
+        then: 'the expected number of leaves are returned'
             result.leavesData.size() == expectedNumberOfLeaves
         and: 'the given operator(s) returns in the correct order'
             result.booleanOperators == expectedOperators
         and: 'the given comparativeOperator(s) returns in the correct order'
             result.comparativeOperators == expectedComparativeOperator
         where: 'the following data is used'
-            scenario                                                      | cpsPath                                                                                   || expectedNumberOfLeaves || expectedOperators || expectedComparativeOperator
-            'one attribute'                                               | '//child[@common-leaf-name-int=5]'                                                        || 1                      || []                || ['=']
-            'more than one attribute has AND operator'                    | '//child[@int-leaf=5 and @leaf-name="leaf value"]'                                        || 2                      || ['and']           || ['=', '=']
-            'more than one attribute has OR operator'                     | '//child[@int-leaf=5 or @leaf-name="leaf value"]'                                         || 2                      || ['or']            || ['=', '=']
-            'more than one attribute has combinations AND operators'      | '//child[@int-leaf=5 and @common-leaf-name="leaf value" and @leaf-name="leaf value1" ]'   || 3                      || ['and', 'and']    || ['=', '=', '=']
-            'more than one attribute has combinations OR operators'       | '//child[@int-leaf=5 or @common-leaf-name="leaf value" or @leaf-name="leaf value1" ]'     || 3                      || ['or', 'or']      || ['=', '=', '=']
-            'more than one attribute has combinations AND/OR combination' | '//child[@int-leaf=5 and @common-leaf-name="leaf value" or @leaf-name="leaf value1" ]'    || 3                      || ['and', 'or']     || ['=', '=', '=']
-            'more than one attribute has combinations OR/AND combination' | '//child[@int-leaf=5 or @common-leaf-name="leaf value" and @leaf-name="leaf value1" ]'    || 3                      || ['or', 'and']     || ['=', '=', '=']
-            'more than one attribute has AND/> operators'                 | '//child[@int-leaf>15 and @leaf-name="leaf value"]'                                       || 2                      || ['and']           || ['>', '=']
-            'more than one attribute has OR/< operators'                  | '//child[@int-leaf<5 or @leaf-name="leaf value"]'                                         || 2                      || ['or']            || ['<', '=']
-            'more than one attribute has combinations AND/>= operators'   | '//child[@int-leaf>=18 and @common-leaf-name="leaf value" and @leaf-name="leaf value1" ]' || 3                      || ['and', 'and']    || ['>=', '=', '=']
-            'more than one attribute has combinations OR/<= operators'    | '//child[@int-leaf<=25 or @common-leaf-name="leaf value" or @leaf-name="leaf value1" ]'   || 3                      || ['or', 'or']      || ['<=', '=', '=']
+            cpsPath                                                                                   || expectedNumberOfLeaves || expectedOperators || expectedComparativeOperator
+            '/parent[@code=1]/child[@common-leaf-name-int=5]'                                         || 1                      || []                || ['=']
+            '//child[@int-leaf>15 and @leaf-name="leaf value"]'                                       || 2                      || ['and']           || ['>', '=']
+            '//child[@int-leaf<5 or @leaf-name="leaf value"]'                                         || 2                      || ['or']            || ['<', '=']
+            '//child[@int-leaf=5 and @common-leaf-name="leaf value" or @leaf-name="leaf value1" ]'    || 3                      || ['and', 'or']     || ['=', '=', '=']
+            '//child[@int-leaf=5 or @common-leaf-name="leaf value" and @leaf-name="leaf value1" ]'    || 3                      || ['or', 'and']     || ['=', '=', '=']
+            '//child[@int-leaf>=18 and @common-leaf-name="leaf value" and @leaf-name="leaf value1" ]' || 3                      || ['and', 'and']    || ['>=', '=', '=']
+            '//child[@int-leaf<=25 or @common-leaf-name="leaf value" or @leaf-name="leaf value1" ]'   || 3                      || ['or', 'or']      || ['<=', '=', '=']
     }
 
     def 'Parse #scenario cps path with text function condition'() {
@@ -220,4 +213,28 @@ class CpsPathQuerySpec extends Specification {
             'container with list-parent' | '//parent[@id=1]/child'               || "parent[@id='1']/child"  | false
             'container with list-parent' | '//parent[@id=1]/child[@name="test"]' || "parent[@id='1']/child"  | true
     }
+
+    def 'Parse cps path with multiple conditions on same leaf.'() {
+        when: 'the given cps path is parsed using multiple conditions on same leaf'
+            def result = CpsPathQuery.createFrom('/test[@same-name="value1" or @same-name="value2"]')
+        then: 'two leaves are present with correct values'
+            assert result.leavesData.size() == 2
+            assert result.leavesData[0].name == "same-name"
+            assert result.leavesData[0].value == "value1"
+            assert result.leavesData[1].name == "same-name"
+            assert result.leavesData[1].value == "value2"
+    }
+
+    def 'Ordering of data leaves is preserved.'() {
+        when: 'the given cps path is parsed'
+            def result = CpsPathQuery.createFrom(cpsPath)
+        then: 'the order of the data leaves is preserved'
+            assert result.leavesData[0].name == expectedFirstLeafName
+            assert result.leavesData[1].name == expectedSecondLeafName
+        where: 'the following data is used'
+            cpsPath                                      || expectedFirstLeafName | expectedSecondLeafName
+            '/test[@name1="value1" and @name2="value2"]' || 'name1'               | 'name2'
+            '/test[@name2="value2" and @name1="value1"]' || 'name2'               | 'name1'
+    }
+
 }