fixes whitespace char handling in constraint name in dataquality trees (

#2986)
finos · Jul 30, 2024 · 9185ff3 · 9185ff3
1 parent c9f892e
commit 9185ff3
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 5 deletions.
diff --git a/legend-engine-xts-dataquality/legend-engine-xt-dataquality-api/pom.xml b/legend-engine-xts-dataquality/legend-engine-xt-dataquality-api/pom.xml
@@ -273,4 +273,4 @@
         </dependency>
 
     </dependencies>
-</project>
+</project>
diff --git a/...lr4/org/finos/legend/engine/language/pure/grammar/from/antlr4/DataQualityParserGrammar.g4 b/...lr4/org/finos/legend/engine/language/pure/grammar/from/antlr4/DataQualityParserGrammar.g4
@@ -96,5 +96,5 @@ constraintList:                     LESS_THAN
                                       dqConstraintName (COMMA dqConstraintName)*
                                     GREATER_THAN
 ;
-dqConstraintName:                   STRING | identifier
+dqConstraintName:                   identifier
 ;
diff --git a/...java/org/finos/legend/engine/language/dataquality/grammar/from/DataQualityTreeWalker.java b/...java/org/finos/legend/engine/language/dataquality/grammar/from/DataQualityTreeWalker.java
@@ -124,12 +124,17 @@ private DataQualityRootGraphFetchTree visitRootGraphDefinition(DataQualityParser
             result.constraints = new ArrayList<>();
             for (DataQualityParserGrammar.DqConstraintNameContext dqConstraintNameContext : validationDefinitionContext.constraintList().dqConstraintName())
             {
-                result.constraints.add(dqConstraintNameContext.getText());
+                result.constraints.add(visitConstraintName(dqConstraintNameContext));
             }
         }
         return result;
     }
 
+    private String visitConstraintName(DataQualityParserGrammar.DqConstraintNameContext dqConstraintNameContext)
+    {
+        return PureGrammarParserUtility.fromIdentifier(dqConstraintNameContext.identifier());
+    }
+
     private PropertyGraphFetchTree visitGraphPathContext(DataQualityParserGrammar.GraphPathContext graphPathContext)
     {
         List<GraphFetchTree> subTrees = new ArrayList<>();

diff --git a/...os/legend/engine/language/dataquality/grammar/to/DataQualityGrammarComposerExtension.java b/...os/legend/engine/language/dataquality/grammar/to/DataQualityGrammarComposerExtension.java
@@ -23,6 +23,7 @@
 import org.finos.legend.engine.language.pure.grammar.to.DEPRECATED_PureGrammarComposerCore;
 import org.finos.legend.engine.language.pure.grammar.to.HelperValueSpecificationGrammarComposer;
 import org.finos.legend.engine.language.pure.grammar.to.PureGrammarComposerContext;
+import org.finos.legend.engine.language.pure.grammar.to.PureGrammarComposerUtility;
 import org.finos.legend.engine.language.pure.grammar.to.extension.PureGrammarComposerExtension;
 import org.finos.legend.engine.protocol.dataquality.metamodel.DataQuality;
 import org.finos.legend.engine.protocol.dataquality.metamodel.DataQualityPropertyGraphFetchTree;
@@ -124,7 +125,12 @@ private static String printConstraints(List<String> constraints)
         {
             return "";
         }
-        return "<" + String.join(", ", constraints) + ">";
+        return "<" + constraints.stream().map(DataQualityGrammarComposerExtension::renderConstraintName).collect(Collectors.joining(", ")) + ">";
+    }
+
+    private static String renderConstraintName(String constraintName)
+    {
+        return PureGrammarComposerUtility.convertIdentifier(constraintName);
     }
 
     public static String printFullPath(String fullPath)

diff --git a/...ava/org/finos/legend/engine/language/dataquality/grammar/from/TestDataQualityParsing.java b/...ava/org/finos/legend/engine/language/dataquality/grammar/from/TestDataQualityParsing.java
@@ -118,8 +118,25 @@ public void testParserForValidGrammar()
                 "    ]$;\n" +
                 "    filter: p:Person[1] | p.age >= 18;\n" +
                 "}");
+        // whitespace chars in constraint names
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "    validationTree: $[\n" +
+                "     Person<ageMustBePositive, 'nameMust \\r\\nNotBeBlank'>{\n" +
+                "       lastName,\n" +
+                "       name,\n" +
+                "       addresses{\n" +
+                "         addressId\n" +
+                "       }\n" +
+                "     }\n" +
+                "    ]$;\n" +
+                "    filter: p:Person[1] | p.age >= 18;\n" +
+                "}");
+
     }
 
 
 
-}
+}
diff --git a/...ava/org/finos/legend/engine/language/dataquality/grammar/to/TestDataQualityRoundtrip.java b/...ava/org/finos/legend/engine/language/dataquality/grammar/to/TestDataQualityRoundtrip.java
@@ -100,5 +100,23 @@ public void testDataQuality_withOnlyStructuralConstraints()
                 "}\n");
     }
 
+    @Test
+    public void testParserForValidGrammar_whiteSpaceCharsInConstraintNames()
+    {
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "   context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "   validationTree: $[\n" +
+                "      Person<ageMustBePositive, 'nameMust  \\r\\nNotBeBlank'>{\n" +
+                "        lastName,\n" +
+                "        name,\n" +
+                "        addresses<streetMustNotBeBlank>{\n" +
+                "          addressId\n" +
+                "        }\n" +
+                "      }\n" +
+                "    ]$;\n" +
+                "}\n");
+    }
 
 }