You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/09/02 14:55:56 UTC

[GitHub] [sling-org-apache-sling-feature-cpconverter] anchela commented on a change in pull request #111: SLING-10783 : XmlConfigurationEntryHandler - sonar findings

anchela commented on a change in pull request #111:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/111#discussion_r701164905



##########
File path: src/main/java/org/apache/sling/feature/cpconverter/handlers/XmlConfigurationEntryHandler.java
##########
@@ -69,73 +69,71 @@ protected void onJcrRootElement(String uri, String localName, String qName, Attr
                 // ignore jcr: and similar properties
                 if (attributeQName.indexOf(':') == -1) {
                     String attributeValue = attributes.getValue(i);
-
-                    if (attributeValue != null && !attributeValue.isEmpty()) {
+                    if (!isEmptyOrNull(attributeValue)) {
                         DocViewProperty property = DocViewProperty.parse(attributeQName, attributeValue);
-                        Object value = property.values;
-                        List<String> strValues = Arrays.asList(property.values);
-                        switch (property.type) {
-                            case PropertyType.DATE:
-                                // Date was never properly supported as osgi configs don't support dates so converting to millis 
-                                // Scenario should just be theoretical
-                                value = strValues.stream().map(s -> {
-                                    Long res = null;
-                                    if (s != null) {
-                                        Calendar cal = ISO8601.parse(s);
-                                        if (cal != null) {
-                                            res = cal.getTimeInMillis();
-                                        }
-                                    }
-                                    return res;
-                                }).toArray();
-                                break;
-                            case PropertyType.DOUBLE:
-                                value = strValues.stream().map(s -> {
-                                    Double res = null;
-                                    if (!s.isEmpty()) {
-                                        res = Double.parseDouble(s);
-                                    }
-                                    return res;
-                                }).toArray();
-                                break;
-                            case PropertyType.LONG:
-                                value = strValues.stream().map(s -> {
-                                    Long res = null;
-                                    if (!s.isEmpty()) {
-                                        res = Long.parseLong(s);
-                                    }
-                                    return res;
-                                }).toArray();
-                                break;
-                            case PropertyType.BOOLEAN:
-                                value = strValues.stream().map(s -> {
-                                    Boolean res = null;
-                                    if (s != null) {
-                                        res = Boolean.valueOf(s);
-                                    }
-                                    return res;
-                                }).toArray();
-                                break;
+                        Object[] values = getValues(property);
+                        if (values.length == 0) {
+                            // ignore empty values (either property.values were empty or value mapping resulted in null 
+                            // results that got filtered)
+                            continue;
                         }
                         if (!property.isMulti) {
-                            // first element to be used in case of singlevalue
-                            value = ((Object[])value)[0];
-                        }
-                         
-                        
-                        if (property.values.length > 0) {
-                            configuration.put(attributeQName, value);
+                            // first element to be used in case of single-value property
+                            configuration.put(attributeQName, values[0]);
+                        } else {
+                            configuration.put(attributeQName, values);
                         }
                     }
                 }
             }
         }
+        
+        private static boolean isEmptyOrNull(@Nullable String s) {
+            return s == null || s.isEmpty();
+        }
+        
+        @NotNull 
+        private static Object[] getValues(@NotNull DocViewProperty property) {
+            Object[] values;
+            switch (property.type) {
+                case PropertyType.DATE:
+                    // Date was never properly supported as osgi configs don't support dates so converting to millis 
+                    // Scenario should just be theoretical
+                    values = mapValues(property.values, s -> {
+                        Calendar cal = ISO8601.parse(s);
+                        return (cal != null) ? cal.getTimeInMillis() : null;
+                    });
+                    break;
+                case PropertyType.DOUBLE:
+                    values = mapValues(property.values, Double::parseDouble);
+                    break;
+                case PropertyType.LONG:
+                    values = mapValues(property.values, Long::parseLong);
+                    break;
+                case PropertyType.BOOLEAN:
+                    values = mapValues(property.values, Boolean::valueOf);
+                    break;
+                default:
+                    values = property.values;
+            }
+            return values;
+        }
+        
+        @NotNull 
+        private static Object[] mapValues(@NotNull String[] strValues, Function<String, Object> function) {
+            return Arrays.stream(strValues).map(s -> {

Review comment:
       yes, indeed. thanks.... please take a look if you are ok with the change.




-- 
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: dev-unsubscribe@sling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org