thomasmueller commented on code in PR #1254:
URL: https://github.com/apache/jackrabbit-oak/pull/1254#discussion_r1432444245


##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/spi/filter/PathFilter.java:
##########
@@ -164,26 +167,36 @@ public String toString() {
                 '}';
     }
 
-    private static Iterable<String> getStrings(NodeBuilder builder, String 
propertyName, 
-            Collection<String> defaultVal) {
+    /**
+     * Gets the value of a property of expected type Strings. However, if the 
property is of type String, interpret
+     * it as a single-element list instead of returning the default value.
+     *
+     * @return the value of the property if the property is set and is of type 
Strings or String, otherwise the default value
+     */
+    private static Iterable<String> getStringsLenient(NodeBuilder builder, 
String propertyName, Collection<String> defaultVal) {
         PropertyState property = builder.getProperty(propertyName);
         if (property != null && property.getType() == Type.STRINGS) {
             return property.getValue(Type.STRINGS);
+        } else if (property != null && property.getType() == Type.STRING) {
+            String value = property.getValue(Type.STRING);
+            LOG.warn("Property \"{}\"=\"{}\" has type String but it should be 
array of String. Proceeding by treating it as a " +

Review Comment:
   I would probably not log a warning. Instead, I would document that this is 
also supported.
   
   Reason: we already have many many entries that are defined as string (mostly 
"/dummy") instead of array.



##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/spi/filter/PathFilter.java:
##########
@@ -164,26 +167,36 @@ public String toString() {
                 '}';
     }
 
-    private static Iterable<String> getStrings(NodeBuilder builder, String 
propertyName, 
-            Collection<String> defaultVal) {
+    /**
+     * Gets the value of a property of expected type Strings. However, if the 
property is of type String, interpret
+     * it as a single-element list instead of returning the default value.
+     *
+     * @return the value of the property if the property is set and is of type 
Strings or String, otherwise the default value
+     */
+    private static Iterable<String> getStringsLenient(NodeBuilder builder, 
String propertyName, Collection<String> defaultVal) {
         PropertyState property = builder.getProperty(propertyName);
         if (property != null && property.getType() == Type.STRINGS) {
             return property.getValue(Type.STRINGS);
+        } else if (property != null && property.getType() == Type.STRING) {
+            String value = property.getValue(Type.STRING);
+            LOG.warn("Property \"{}\"=\"{}\" has type String but it should be 
array of String. Proceeding by treating it as a " +

Review Comment:
   I would make it public and add a unit test case for it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to