You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2020/03/24 16:24:45 UTC

[sling-org-apache-sling-repoinit-parser] 03/03: SLING-9171 - simplify the grammar and do more work in the PropertyLine class

This is an automated email from the ASF dual-hosted git repository.

bdelacretaz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-repoinit-parser.git

commit 52f656ea5c1de9061d49c55d35abecd4e6e464ed
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Mar 24 17:23:51 2020 +0100

    SLING-9171 - simplify the grammar and do more work in the PropertyLine class
---
 bnd.bnd                                            |  4 +
 pom.xml                                            |  9 +-
 .../repoinit/parser/operations/Operation.java      |  6 --
 .../repoinit/parser/operations/PropertyLine.java   | 72 ++++++++++++++--
 src/main/javacc/RepoInitGrammar.jjt                | 95 ++++++----------------
 .../repoinit/parser/test/ParsingErrorsTest.java    | 16 +++-
 .../repoinit/parser/test/PropertyLineTest.java     | 34 +++++++-
 src/test/resources/testcases/test-67-output.txt    | 24 +++---
 src/test/resources/testcases/test-67.txt           |  9 +-
 9 files changed, 163 insertions(+), 106 deletions(-)

diff --git a/bnd.bnd b/bnd.bnd
index 7738e89..dbff4b6 100644
--- a/bnd.bnd
+++ b/bnd.bnd
@@ -1,2 +1,6 @@
 -fixupmessages.bundleversion: "The bundle version change *";is:=ignore 
+
+-includeresource:\
+  @jackrabbit-jcr-commons-*.jar!/org/apache/jackrabbit/util/ISO8601.*
+
 Provide-Capability: org.apache.sling.repoinit.language;version:Version="8.0"
\ No newline at end of file
diff --git a/pom.xml b/pom.xml
index f96cea4..dd48638 100644
--- a/pom.xml
+++ b/pom.xml
@@ -95,7 +95,14 @@
             <groupId>org.osgi</groupId>
             <artifactId>org.osgi.service.component.annotations</artifactId>
         </dependency>
-      <!-- testing -->
+        <dependency>
+          <groupId>org.apache.jackrabbit</groupId>
+          <artifactId>jackrabbit-jcr-commons</artifactId>
+          <version>2.19.6</version>
+          <scope>compile</scope>
+        </dependency>
+
+        <!-- testing -->
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
diff --git a/src/main/java/org/apache/sling/repoinit/parser/operations/Operation.java b/src/main/java/org/apache/sling/repoinit/parser/operations/Operation.java
index 88d65a3..b2afd9b 100644
--- a/src/main/java/org/apache/sling/repoinit/parser/operations/Operation.java
+++ b/src/main/java/org/apache/sling/repoinit/parser/operations/Operation.java
@@ -32,12 +32,6 @@ public abstract class Operation {
         if(s == null) {
             return null;
         }
-        if(s.startsWith(DQUOTE)) {
-            s = s.substring(1);
-        }
-        if(s.endsWith(DQUOTE)) {
-            s = s.substring(0, s.length() - 1);
-        }
         s = s.trim();
         if(s.length() == 0) {
             return null;
diff --git a/src/main/java/org/apache/sling/repoinit/parser/operations/PropertyLine.java b/src/main/java/org/apache/sling/repoinit/parser/operations/PropertyLine.java
index fff4e7f..337cfd1 100644
--- a/src/main/java/org/apache/sling/repoinit/parser/operations/PropertyLine.java
+++ b/src/main/java/org/apache/sling/repoinit/parser/operations/PropertyLine.java
@@ -18,15 +18,21 @@
 package org.apache.sling.repoinit.parser.operations;
 
 import java.util.ArrayList;
+import java.util.Calendar;
 import java.util.Collections;
 import java.util.List;
 
+import javax.sound.sampled.Port;
+
+import org.apache.jackrabbit.util.ISO8601;
+import org.apache.sling.repoinit.parser.impl.ParseException;
+
 /** A single "set property" line */
 public class PropertyLine {
 
     private final String name;
     private final PropertyType propertyType;
-    private final List<String> values;
+    private final List<Object> values;
     private boolean isDefault = false;
 
     /** Valid types for these properties */
@@ -44,22 +50,59 @@ public class PropertyLine {
      *  @param typeString property type, as a String
      *  @param values  values of the property
      */
-    public PropertyLine(String name, String typeString, List<String> values, boolean isDefault) {
+    public PropertyLine(String name, String typeString, List<String> values, boolean isDefault) throws ParseException {
         this.name = name;
-        this.propertyType = typeString == null ? PropertyType.String : PropertyType.valueOf(typeString);
-        this.values = values == null ? new ArrayList<>() : values;
+        this.propertyType = typeString == null ? PropertyType.String : parseType(typeString);
+        this.values = parseList(this.propertyType, values);
         this.isDefault = isDefault;
 
     }
 
-    /** @return the name of the property to set */
+    private static PropertyType parseType(String type) throws ParseException {
+        try {
+            return PropertyType.valueOf(type);
+        } catch(IllegalArgumentException e) {
+            throw new ParseException("Invalid property type:" + type);
+        }
+    }
+
+    private static List<Object> parseList(PropertyType type, List<String> values) throws ParseException {
+        if(values == null) {
+            return new ArrayList<>();
+        }
+        final List<Object> result = new ArrayList<>();
+        for(String val : values) {
+            result.add(parseValue(type, val));
+        }
+        return result;
+    }
+
+    private static Object parseValue(PropertyType type, String value) throws ParseException {
+        if(value == null) throw new ParseException("Null value for type " + type);
+        if(type == PropertyType.String) return value;
+        if(type == PropertyType.Long) return Long.valueOf(value);
+        if(type == PropertyType.Double) return Double.valueOf(value);
+        if(type == PropertyType.Date) return parseDate(value);
+        if(type == PropertyType.Boolean) return Boolean.valueOf(value);
+        throw new ParseException("Invalid type " + type);
+    }
+
+    private static Calendar parseDate(String dateStr) throws ParseException {
+        final Calendar result = ISO8601.parse(dateStr);
+        if(result == null) {
+            throw new ParseException("Invalid ISO8601 date: " + dateStr);
+        }
+        return result;
+    }
+
+        /** @return the name of the property to set */
     public String getPropertyName() {return name;};
 
     /** @return the type of the property to set */
     public PropertyType getPropertyType() {return propertyType;};
 
     /** @return the list ot values of the property to set */
-    public List<String> getPropertyValues() {
+    public List<Object> getPropertyValues() {
         return Collections.unmodifiableList(values);
     }
 
@@ -79,12 +122,23 @@ public class PropertyLine {
         }
 
         sb.append(name);
-        sb.append("=");
         sb.append("{");
         sb.append(propertyType.toString());
-        sb.append("}");
+        sb.append("}=[");
+
+        String sep = "";
+        for(Object value : values) {
+            sb.append(sep);
+            sep = ", ";
+            sb.append("{").append(value.getClass().getSimpleName()).append("}");
+            if(value instanceof Calendar) {
+                sb.append(ISO8601.format((Calendar)value));
+            } else {
+                sb.append(value.toString());
+            }
+        }
 
-        sb.append(values);
+        sb.append("]");
         return sb.toString();
     }
 }
\ No newline at end of file
diff --git a/src/main/javacc/RepoInitGrammar.jjt b/src/main/javacc/RepoInitGrammar.jjt
index d759689..5cb520f 100644
--- a/src/main/javacc/RepoInitGrammar.jjt
+++ b/src/main/javacc/RepoInitGrammar.jjt
@@ -95,11 +95,6 @@ TOKEN:
 |   < RESTRICTION: "restriction" >
 |   < ACL_OPTIONS: "ACLOptions" >
 |   < PROPERTIES: "properties" >
-|   < PROP_TYPE_STRING: "String" >
-|   < PROP_TYPE_LONG: "Long" >
-|   < PROP_TYPE_DOUBLE: "Double" >
-|   < PROP_TYPE_DATE: "Date" >
-|   < PROP_TYPE_BOOLEAN: "Boolean" >
 |   < SETDEF: "default" >
 
 /* The order of these fuzzy statements is important (first match wins?) */ 
@@ -107,7 +102,6 @@ TOKEN:
 |   < PATH_STRING: "/" (["a"-"z"] | ["A"-"Z"] | ["0"-"9"] | ["-"] | ["_"] | ["."] | ["@"] | [":"] | ["+"] | ["/"]) * >
 |   < PATH_REPOSITORY: ":repository" >
 |   < STRING: (["a"-"z"] | ["A"-"Z"] | ["0"-"9"] | ["-"] | ["_"] | ["."] | ["/"] | [":"] | ["*"]) + >
-|   < DATE_STRING: ("+" | "-")? ["0"-"9"]["0"-"9"]["0"-"9"]["0"-"9"] ["-"] ((["0"]["1"-"9"]) | (["1"]["0"-"2"])) ["-"] (["0"]["1"-"9"] | (["1"]|["2"])["0"-"9"] | ["3"](["0"] | ["1"])) ["T"] (["0"]["0"-"9"] | ["1"]["0"-"9"] | ["2"]["0"-"3"]) [":"] (["0"-"5"]["0"-"9"]) [":"] (["0"-"5"]["0"-"9"]) ["."] ["0"-"9"]["0"-"9"]["0"-"9"] (["Z"]| ((["+"]|["-"])(["0"]["0"-"9"] | ["1"]["0"-"9"] | ["2"]["0"-"3"]) [":"] (["0"-"5"]["0"-"9"]))) >
 |   < EOL: "\n" >
 }
 
@@ -663,40 +657,34 @@ void deleteUserStatement(List<Operation> result) :
     }
 }
 
-String escapeQuotedString(String s) :
-{}
+Token quotedString() :
 {
+    Token t = null;
+}
+{
+    ( t = <QUOTED> )
     {
-        return s
+        // Remove start/end quotes + escaping backslash for double quotes
+        final String unescaped = t.image.trim()
+            .replaceAll("^\"|\"$", "")
             .replaceAll("\\\\\"", "\"")
             .replaceAll("\\\\\\\\", "\\\\")
         ;
-     }
-}
-
-String quotedString() :
-{
-    Token str = null;
-}
-{
-    ( str = <QUOTED> )
-    {
-        // Remove escaping backslash for double quotes
-        return escapeQuotedString(str.image);
+        return new Token(t.kind, unescaped);
     }
 }
 
 void disableServiceUserStatement(List<Operation> result) :
 {
     Token user = null;
-    String msg = null;
+    Token msg = null;
 }
 {
     <DISABLE> <SERVICE> <USER> 
     ( user = <STRING> )
     ( <COLON> msg = quotedString() )
     {
-        result.add(new DisableServiceUser(user.image, msg));
+        result.add(new DisableServiceUser(user.image, msg.image));
     }
 }
 
@@ -734,78 +722,41 @@ void removeFromGroupStatement(List<Operation> result) :
     }
 }
 
-void addValueToList(List<String> list, Token valueToken) :
-{}
-{
-    {
-        if(QUOTED == valueToken.kind) {
-            list.add(escapeQuotedString(valueToken.image));
-        } else {
-            list.add(valueToken.image);
-        }
-    }
-}
-
-List<String> valuesList() :
+Token propertyValue() :
 {
     Token t = null;
-    List<String> values = new ArrayList<String>();
 }
 {
-    (t = <STRING> | t = <QUOTED> | t = <PATH_STRING> | t = <NAMESPACED_ITEM>)  { addValueToList(values, t);}
-    ( <COMMA> (t = <STRING> | t = <QUOTED> | t = <PATH_STRING> | t = <NAMESPACED_ITEM>) { addValueToList(values, t); } )*
-    { return values; }
+    (t = <STRING> | t = quotedString() | t = <PATH_STRING> | t = <NAMESPACED_ITEM> ) { return t; }
 }
 
-List<String> dateValuesList() :
+List<String> propertyValuesList() :
 {
     Token t = null;
     List<String> values = new ArrayList<String>();
 }
 {
-    (t = <DATE_STRING>)  { addValueToList(values, t);}
-    ( <COMMA> (t = <DATE_STRING>) { addValueToList(values, t); } )*
+    ( t = propertyValue() ) { values.add(t.image); }
+    ( <COMMA> ( t = propertyValue() ) { values.add(t.image); } )*
     { return values; }
 }
 
-String propertyDataType() :
-{
-   Token t;
-}
-{
-  ( t = <PROP_TYPE_STRING> | t = <PROP_TYPE_LONG> | t = <PROP_TYPE_DOUBLE> | t = <PROP_TYPE_BOOLEAN> )
-  {
-      return t.image;
-  }
-
-}
-
-String dateDataType() :
-{
-   Token t;
-}
-{
-  (t = <PROP_TYPE_DATE>)
-  {
-      return t.image;
-  }
-
-}
-
 void propertyLine(List<PropertyLine> lines) :
 {
     Token name = null;
-    String type = null;
+    Token type = null;
     List<String> values;
     Token t = null;
     boolean isDefault = false;
 }
 {
-    (t = <SET> | t = <SETDEF> {isDefault = true;}) ( name = <STRING> | name = <NAMESPACED_ITEM>)  ((( <LCURLY> ( type = propertyDataType() ) <RCURLY> )? <TO>
-    values = valuesList()) | (( <LCURLY> ( type = dateDataType() ) <RCURLY> ) <TO>  values = dateValuesList()))
+    (t = <SET> | t = <SETDEF> {isDefault = true;} )
+    ( name = <STRING> | name = <NAMESPACED_ITEM>)
+    ( <LCURLY> ( type = <STRING> ) <RCURLY> )?
+    <TO> ( values = propertyValuesList() )
     <EOL>
     {
-        lines.add(new PropertyLine(name.image,  type == null ? "String" : type, values, isDefault));
+        lines.add(new PropertyLine(name.image, type == null ? null : type.image, values, isDefault));
     }
 }
 
@@ -822,4 +773,4 @@ void setPropertiesStatement(List<Operation> result) :
     {
         result.add(new SetProperties(paths, lines));
     }
-}
+}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/repoinit/parser/test/ParsingErrorsTest.java b/src/test/java/org/apache/sling/repoinit/parser/test/ParsingErrorsTest.java
index ad65548..554a73f 100644
--- a/src/test/java/org/apache/sling/repoinit/parser/test/ParsingErrorsTest.java
+++ b/src/test/java/org/apache/sling/repoinit/parser/test/ParsingErrorsTest.java
@@ -21,7 +21,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.io.StringReader;
+import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -116,6 +118,7 @@ public class ParsingErrorsTest {
             add(new Object[] { "set properties on /pathA/b  \n set lowercasetype{string} to abc \n end", ParseException.class });
             add(new Object[] { "set properties on /pathA/b  \n set {String} to missingPropertyName \n end", ParseException.class });
             add(new Object[] { "set properties on /pathA/b  \n set somepProp{String} withoutTo \n end", ParseException.class });
+            add(new Object[] { "set properties on /noPropsFails  \n end", ParseException.class });
         }};
         return result;
     }
@@ -125,6 +128,15 @@ public class ParsingErrorsTest {
         this.expected = expected;
     }
 
+    private String getInfo(String msg, Throwable unexpected) {
+        final StringWriter sw = new StringWriter();
+        final PrintWriter pw = new PrintWriter(sw);
+        pw.println("For input '" + input + "', unexpected stack trace=");
+        unexpected.printStackTrace(pw);
+        pw.flush();
+        return sw.toString();
+    }
+
     @Test
     public void checkResult() throws ParseException, IOException {
         final StringReader r = new StringReader(input);
@@ -133,9 +145,9 @@ public class ParsingErrorsTest {
             new RepoInitParserImpl(r).parse();
             noException = true;
         } catch(Exception e) {
-            assertEquals("for input " + input, expected, e.getClass());
+            assertEquals(getInfo(input, e), expected, e.getClass());
         } catch(Error err) {
-            assertEquals("for input " + input, expected, err.getClass());
+            assertEquals(getInfo(input, err), expected, err.getClass());
         } finally {
             r.close();
         }
diff --git a/src/test/java/org/apache/sling/repoinit/parser/test/PropertyLineTest.java b/src/test/java/org/apache/sling/repoinit/parser/test/PropertyLineTest.java
index 3f401c7..7e166d0 100644
--- a/src/test/java/org/apache/sling/repoinit/parser/test/PropertyLineTest.java
+++ b/src/test/java/org/apache/sling/repoinit/parser/test/PropertyLineTest.java
@@ -18,25 +18,51 @@ package org.apache.sling.repoinit.parser.test;
 
 import static org.junit.Assert.assertEquals;
 
+import java.util.Arrays;
+import java.util.Date;
+
+import org.apache.jackrabbit.util.ISO8601;
 import org.apache.sling.repoinit.parser.operations.PropertyLine;
+import org.apache.sling.repoinit.parser.impl.ParseException;
 import org.junit.Test;
 
 public class PropertyLineTest {
 
     @Test
-    public void testDefaultPropertyType() {
+    public void testDefaultPropertyType() throws ParseException {
         final PropertyLine p = new PropertyLine("someName", null, null, false);
         assertEquals(PropertyLine.PropertyType.String, p.getPropertyType());
     }
 
     @Test
-    public void testValidPropertyType() {
+    public void testValidPropertyType() throws ParseException {
         final PropertyLine p = new PropertyLine("someName", "Boolean", null, false);
         assertEquals(PropertyLine.PropertyType.Boolean, p.getPropertyType());
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testInvalidPropertyType() {
+    @Test(expected = ParseException.class)
+    public void testInvalidPropertyType() throws ParseException {
         new PropertyLine("someName", "invalidTypeName", null, false);
     }
+
+    @Test
+    public void testValidDateFormat() throws ParseException {
+        final Date now = new Date();
+        final String [] value = { ISO8601.format(now) };
+        final PropertyLine p = new PropertyLine("someName", "Date", Arrays.asList(value), false);
+        assertEquals(ISO8601.parse(value[0]), p.getPropertyValues().get(0));
+    }
+
+    @Test(expected=ParseException.class)
+    public void testInvalidDateFormat() throws ParseException {
+        final String [] notAnIsoDate = { "really not a date" };
+        new PropertyLine("someName", "Date", Arrays.asList(notAnIsoDate), false);
+    }
+
+    @Test
+    public void testInvalidDateFormatAsString() throws ParseException {
+        final String [] notAnIsoDate = { "2020-03-24" };
+        final PropertyLine p = new PropertyLine("someName", "String", Arrays.asList(notAnIsoDate), false);
+        assertEquals(notAnIsoDate[0], p.getPropertyValues().get(0));
+    }
 }
diff --git a/src/test/resources/testcases/test-67-output.txt b/src/test/resources/testcases/test-67-output.txt
index c669625..72fcbf3 100644
--- a/src/test/resources/testcases/test-67-output.txt
+++ b/src/test/resources/testcases/test-67-output.txt
@@ -1,12 +1,14 @@
 SetProperties on /pathA /path/B
-  PropertyLine sling:ResourceType={String}[/x/y/z]
-  PropertyLine cq:allowedTemplates={String}[/d/e/f/*, m/n/*]
-  PropertyLine default someInteger={Long}[42]
-  PropertyLine someFlag={Boolean}[true]
-  PropertyLine default someDate={Date}[2020-03-19T11:39:33.437+05:30]
-  PropertyLine customSingleValueStringProp={String}[test]
-  PropertyLine customSingleValueQuotedStringProp={String}["hello, you!"]
-  PropertyLine customMultiValueStringProp={String}[test1, test2]
-
-
-
+  PropertyLine sling:ResourceType{String}=[{String}/x/y/z]
+  PropertyLine cq:allowedTemplates{String}=[{String}/d/e/f/*, {String}m/n/*]
+  PropertyLine default someInteger{Long}=[{Long}42]
+  PropertyLine someFlag{Boolean}=[{Boolean}true]
+  PropertyLine default someDate{Date}=[{GregorianCalendar}2020-03-19T11:39:33.437+05:30]
+  PropertyLine customSingleValueStringProp{String}=[{String}test]
+  PropertyLine customSingleValueQuotedStringProp{String}=[{String}hello, you!]
+  PropertyLine customMultiValueStringProp{String}=[{String}test1, {String}test2]
+  PropertyLine default threeValues{String}=[{String}test1, {String}test2, {String}test3]
+  PropertyLine quotedA{String}=[{String}Here's a "double quoted string" with suffix]
+  PropertyLine quotedMix{String}=[{String}quoted, {String}non-quoted, {String}the last " one]
+SetProperties on /single/path
+  PropertyLine someString{String}=[{String}some string]
\ No newline at end of file
diff --git a/src/test/resources/testcases/test-67.txt b/src/test/resources/testcases/test-67.txt
index 91b52e9..2eafc25 100644
--- a/src/test/resources/testcases/test-67.txt
+++ b/src/test/resources/testcases/test-67.txt
@@ -4,8 +4,15 @@ set properties on /pathA, /path/B
   set cq:allowedTemplates to /d/e/f/*, m/n/*
   default someInteger{Long} to 42
   set someFlag{Boolean} to true
-  default someDate{Date} to 2020-03-19T11:39:33.437+05:30
+  default someDate{Date} to "2020-03-19T11:39:33.437+05:30"
   set customSingleValueStringProp to test
   set customSingleValueQuotedStringProp to "hello, you!"
   set customMultiValueStringProp to test1, test2
+  default threeValues to test1, test2, test3
+  set quotedA to "Here's a \"double quoted string\" with suffix"
+  set quotedMix to "quoted", non-quoted, "the last \" one"
+end
+
+set properties on /single/path
+  set someString to "some string"
 end
\ No newline at end of file