SLING-11311 : Improve org-apache-sling-jcr-resource
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrResourceUtil.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrResourceUtil.java
index 2907a06..5c4e61f 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrResourceUtil.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrResourceUtil.java
@@ -104,36 +104,40 @@
     public static @NotNull Object toJavaObject(@NotNull Property property) throws RepositoryException {
         // multi-value property: return an array of values
         if (property.isMultiple()) {
-            Value[] values = property.getValues();
-            final Object firstValue = values.length > 0 ? toJavaObject(values[0]) : null;
-            final Object[] result;
-            if (firstValue instanceof Boolean) {
-                result = new Boolean[values.length];
-            } else if (firstValue instanceof Calendar) {
-                result = new Calendar[values.length];
-            } else if (firstValue instanceof Double) {
-                result = new Double[values.length];
-            } else if (firstValue instanceof Long) {
-                result = new Long[values.length];
-            } else if (firstValue instanceof BigDecimal) {
-                result = new BigDecimal[values.length];
-            } else if (firstValue instanceof InputStream) {
-                result = new Object[values.length];
-            } else {
-                result = new String[values.length];
-            }
-            for (int i = 0; i < values.length; i++) {
-                Value value = values[i];
-                if (value != null) {
-                    result[i] = toJavaObject(value);
-                }
-            }
-            return result;
+            return toJavaObjectArray(property);
         }
 
         // single value property
         return toJavaObject(property.getValue());
     }
+    
+    private static Object[] toJavaObjectArray(@NotNull Property property) throws RepositoryException {
+        Value[] values = property.getValues();
+        final Object firstValue = values.length > 0 ? toJavaObject(values[0]) : null;
+        final Object[] result;
+        if (firstValue instanceof Boolean) {
+            result = new Boolean[values.length];
+        } else if (firstValue instanceof Calendar) {
+            result = new Calendar[values.length];
+        } else if (firstValue instanceof Double) {
+            result = new Double[values.length];
+        } else if (firstValue instanceof Long) {
+            result = new Long[values.length];
+        } else if (firstValue instanceof BigDecimal) {
+            result = new BigDecimal[values.length];
+        } else if (firstValue instanceof InputStream) {
+            result = new Object[values.length];
+        } else {
+            result = new String[values.length];
+        }
+        for (int i = 0; i < values.length; i++) {
+            Value value = values[i];
+            if (value != null) {
+                result[i] = toJavaObject(value);
+            }
+        }
+        return result;
+    }
 
     /**
      * Creates a {@link javax.jcr.Value JCR Value} for the given object with
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/NumberConverter.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/NumberConverter.java
index ce40596..d72ad1b 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/NumberConverter.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/NumberConverter.java
@@ -87,14 +87,14 @@
     }
 
     @Override
-    public ZonedDateTime toZonedDateTime() {
+    public @NotNull ZonedDateTime toZonedDateTime() {
         return new CalendarConverter(toCalendar()).toZonedDateTime();
     }
 
     /**
      * @see org.apache.sling.jcr.resource.internal.helper.Converter#toCalendar()
      */
-    public Calendar toCalendar() {
+    public @NotNull Calendar toCalendar() {
         final Calendar c = Calendar.getInstance();
         c.setTimeInMillis(this.toLong());
         return c;
@@ -103,21 +103,21 @@
     /**
      * @see org.apache.sling.jcr.resource.internal.helper.Converter#toDate()
      */
-    public Date toDate() {
+    public @NotNull Date toDate() {
         return new Date(this.toLong());
     }
 
     /**
      * @see org.apache.sling.jcr.resource.internal.helper.Converter#toBoolean()
      */
-    public Boolean toBoolean() {
+    public @NotNull Boolean toBoolean() {
         return false;
     }
 
     /**
      * @see org.apache.sling.jcr.resource.internal.helper.Converter#toBigDecimal()
      */
-    public BigDecimal toBigDecimal() {
+    public @NotNull BigDecimal toBigDecimal() {
         if ( this.value instanceof BigDecimal ) {
             return (BigDecimal)this.value;
         }
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java
index bef8ac5..39d7eb6 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BasicQueryLanguageProvider.java
@@ -49,7 +49,7 @@
 
 public class BasicQueryLanguageProvider implements QueryLanguageProvider<JcrProviderState> {
 
-    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+    private static final Logger logger = LoggerFactory.getLogger(BasicQueryLanguageProvider.class);
 
     @SuppressWarnings("deprecation")
     private static final String DEFAULT_QUERY_LANGUAGE = Query.XPATH;
@@ -102,85 +102,91 @@
 
         try {
             final QueryResult result = JcrResourceUtil.query(getSession(ctx), query, queryLanguage);
-            final String[] colNames = result.getColumnNames();
-            final RowIterator rows = result.getRows();
-
-            return new Iterator<ValueMap>() {
-
-                private ValueMap next;
-
-                {
-                    next = seek();
-                }
-
-                @Override
-                public boolean hasNext() {
-                    return next != null;
-                }
-
-                @Override
-                public ValueMap next() {
-                    if ( next == null ) {
-                        throw new NoSuchElementException();
-                    }
-                    final ValueMap result = next;
-                    next = seek();
-                    return result;
-                }
-
-                private ValueMap seek() {
-                    ValueMap result = null;
-                    while ( result == null && rows.hasNext() ) {
-                        try {
-                            final Row jcrRow = rows.nextRow();
-                            final String resourcePath = jcrRow.getPath();
-                            if ( resourcePath != null && providerContext.getExcludedPaths().matches(resourcePath) == null) {
-                                final Map<String, Object> row = new HashMap<>();
-
-                                boolean didPath = false;
-                                boolean didScore = false;
-                                final Value[] values = jcrRow.getValues();
-                                for (int i = 0; i < values.length; i++) {
-                                    Value v = values[i];
-                                    if (v != null) {
-                                        String colName = colNames[i];
-                                        row.put(colName,
-                                            JcrResourceUtil.toJavaObject(values[i]));
-                                        if (colName.equals(QUERY_COLUMN_PATH)) {
-                                            didPath = true;
-                                            row.put(colName, JcrResourceUtil.toJavaObject(values[i]).toString());
-                                        }
-                                        if (colName.equals(QUERY_COLUMN_SCORE)) {
-                                            didScore = true;
-                                        }
-                                    }
-                                }
-                                if (!didPath) {
-                                    row.put(QUERY_COLUMN_PATH, jcrRow.getPath());
-                                }
-                                if (!didScore) {
-                                    row.put(QUERY_COLUMN_SCORE, jcrRow.getScore());
-                                }
-                                result = new ValueMapDecorator(row);
-                            }
-                        } catch (final RepositoryException re) {
-                            logger.error(
-                                "queryResources$next: Problem accessing row values",
-                                re);
-                        }
-                    }
-                    return result;
-                }
-
-                @Override
-                public void remove() {
-                    throw new UnsupportedOperationException("remove");
-                }
-            };
+            return new ValueMapIterator(result.getColumnNames(), result.getRows());
         } catch (final javax.jcr.query.InvalidQueryException iqe) {
             throw new QuerySyntaxException(iqe.getMessage(), query, language, iqe);
         } catch (final RepositoryException re) {
             throw new SlingException(re.getMessage(), re);
         }
     }
+    
+    private class ValueMapIterator implements Iterator<ValueMap> {
+
+        private final String[] colNames;
+        private final RowIterator rows;
+        
+        private ValueMap next;
+        
+        private ValueMapIterator(@NotNull String[] colNames, @NotNull RowIterator rows) {
+            this.colNames = colNames;
+            this.rows = rows;
+
+            next = seek();
+        }
+
+        @Override
+        public boolean hasNext() {
+            return next != null;
+        }
+
+        @Override
+        public ValueMap next() {
+            if ( next == null ) {
+                throw new NoSuchElementException();
+            }
+            final ValueMap result = next;
+            next = seek();
+            return result;
+        }
+
+        private ValueMap seek() {
+            ValueMap result = null;
+            while (result == null && rows.hasNext()) {
+                try {
+                    final Row jcrRow = rows.nextRow();
+                    final String resourcePath = jcrRow.getPath();
+                    if (resourcePath != null && providerContext.getExcludedPaths().matches(resourcePath) == null) {
+                        result = new ValueMapDecorator(getRow(jcrRow));
+                    }
+                } catch (final RepositoryException re) {
+                    logger.error("queryResources$next: Problem accessing row values", re);
+                }
+            }
+            return result;
+        }
+        
+        private Map<String, Object> getRow(@NotNull Row jcrRow) throws RepositoryException {
+            final Map<String, Object> row = new HashMap<>();
+
+            boolean didPath = false;
+            boolean didScore = false;
+            final Value[] values = jcrRow.getValues();
+            for (int i = 0; i < values.length; i++) {
+                Value v = values[i];
+                if (v != null) {
+                    String colName = colNames[i];
+                    row.put(colName, JcrResourceUtil.toJavaObject(values[i]));
+                    if (colName.equals(QUERY_COLUMN_PATH)) {
+                        didPath = true;
+                        row.put(colName, JcrResourceUtil.toJavaObject(values[i]).toString());
+                    }
+                    if (colName.equals(QUERY_COLUMN_SCORE)) {
+                        didScore = true;
+                    }
+                }
+            }
+            if (!didPath) {
+                row.put(QUERY_COLUMN_PATH, jcrRow.getPath());
+            }
+            if (!didScore) {
+                row.put(QUERY_COLUMN_SCORE, jcrRow.getScore());
+            }
+            return row;
+        }
+
+        @Override
+        public void remove() {
+            throw new UnsupportedOperationException("remove");
+        }
+    }
 }
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java
index 841f509..e92556b 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java
@@ -131,9 +131,8 @@
 
     @Activate
     protected void activate(final ComponentContext context) {
-        SlingRepository repository = context.locateService(REPOSITORY_REFERENCE_NAME,
-                this.repositoryReference);
-        if (repository == null) {
+        SlingRepository repo = context.locateService(REPOSITORY_REFERENCE_NAME, this.repositoryReference);
+        if (repo == null) {
             // concurrent unregistration of SlingRepository service
             // don't care, this component is going to be deactivated
             // so we just stop working
@@ -141,10 +140,9 @@
             return;
         }
 
-        this.repository = repository;
+        this.repository = repo;
 
-        this.stateFactory = new JcrProviderStateFactory(repositoryReference, repository,
-                classLoaderManagerReference, uriProviderReference);
+        this.stateFactory = new JcrProviderStateFactory(repositoryReference, repo, classLoaderManagerReference, uriProviderReference);
     }
 
     @Deactivate
@@ -410,82 +408,37 @@
         // check for node type
         final Object nodeObj = (properties != null ? properties.get(JcrConstants.JCR_PRIMARYTYPE) : null);
         // check for sling:resourcetype
-        final String nodeType;
-        if (nodeObj != null) {
-            nodeType = nodeObj.toString();
-        } else {
-            final Object rtObj = (properties != null ? properties.get(JcrResourceConstants.SLING_RESOURCE_TYPE_PROPERTY) : null);
-            boolean isNodeType = false;
-            if (rtObj != null) {
-                final String resourceType = rtObj.toString();
-                if (resourceType.indexOf(':') != -1 && resourceType.indexOf('/') == -1) {
-                    try {
-                        getSession(ctx).getWorkspace().getNodeTypeManager().getNodeType(resourceType);
-                        isNodeType = true;
-                    } catch (final RepositoryException ignore) {
-                        // we expect this, if this isn't a valid node type, therefore ignoring
-                    }
-                }
-            }
-            if (isNodeType) {
-                nodeType = rtObj.toString();
-            } else {
-                nodeType = null;
-            }
-        }
-        final String jcrPath = path;
-        if (jcrPath == null) {
+        final String nodeType = getNodeTypeName(nodeObj, properties, ctx);
+        
+        if (path == null) {
             throw new PersistenceException("Unable to create node at " + path, null, path, null);
         }
-        Node node = null;
+        Node node;
         try {
-            final int lastPos = jcrPath.lastIndexOf('/');
+            final int lastPos = path.lastIndexOf('/');
             final Node parent;
             if (lastPos == 0) {
                 parent = getSession(ctx).getRootNode();
             } else {
-                parent = (Node) getSession(ctx).getItem(jcrPath.substring(0, lastPos));
+                parent = (Node) getSession(ctx).getItem(path.substring(0, lastPos));
             }
-            final String name = jcrPath.substring(lastPos + 1);
+            final String name = path.substring(lastPos + 1);
             if (nodeType != null) {
                 node = parent.addNode(name, nodeType);
             } else {
                 node = parent.addNode(name);
             }
-
-            if (properties != null) {
-                // create modifiable map
-                final JcrModifiableValueMap jcrMap = new JcrModifiableValueMap(node, getHelperData(ctx));
-                // check mixin types first
-                final Object value = properties.get(JcrConstants.JCR_MIXINTYPES);
-                if (value != null) {
-                    jcrMap.put(JcrConstants.JCR_MIXINTYPES, value);
-                }
-                for (final Map.Entry<String, Object> entry : properties.entrySet()) {
-                    if (!IGNORED_PROPERTIES.contains(entry.getKey())) {
-                        try {
-                            jcrMap.put(entry.getKey(), entry.getValue());
-                        } catch (final IllegalArgumentException iae) {
-                            try {
-                                node.remove();
-                            } catch (final RepositoryException re) {
-                                // we ignore this
-                            }
-                            throw new PersistenceException(iae.getMessage(), iae, path, entry.getKey());
-                        }
-                    }
-                }
-            }
+            setProperties(node, path, properties, ctx);
 
             return new JcrNodeResource(ctx.getResourceResolver(), path, null, node, getHelperData(ctx));
         } catch (final RepositoryException e) {
-            throw new PersistenceException("Unable to create node at " + jcrPath, e, path, null);
+            throw new PersistenceException("Unable to create node at " + path, e, path, null);
         }
     }
-
+    
     @Override
     public boolean orderBefore(@NotNull ResolveContext<JcrProviderState> ctx, @NotNull Resource parent, @NotNull String name,
-            @Nullable String followingSiblingName) throws PersistenceException {
+                               @Nullable String followingSiblingName) throws PersistenceException {
         Node node = parent.adaptTo(Node.class);
         if (node == null) {
             throw new PersistenceException("The resource " + parent.getPath() + " cannot be adapted to Node. It is probably not provided by the JcrResourceProvider");
@@ -527,12 +480,8 @@
         // try to adapt to Item
         Item item = resource.adaptTo(Item.class);
         try {
-            if ( item == null ) {
+            if (item == null) {
                 final String jcrPath = resource.getPath();
-                if (jcrPath == null) {
-                    logger.debug("delete: {} maps to an empty JCR path", resource.getPath());
-                    throw new PersistenceException("Unable to delete resource", null, resource.getPath(), null);
-                }
                 item = getSession(ctx).getItem(jcrPath);
             }
             item.remove();
@@ -650,4 +599,64 @@
         return !name.equals(JcrResourceConstants.AUTHENTICATION_INFO_CREDENTIALS)
             && !name.contains("password");
     }
+
+    @Nullable
+    private static String getNodeTypeName(@Nullable Object nodeObj, @Nullable Map<String, Object> properties,
+                                          @NotNull ResolveContext<JcrProviderState> ctx) {
+        final String nodeType;
+        if (nodeObj != null) {
+            nodeType = nodeObj.toString();
+        } else {
+            final Object rtObj = (properties != null ? properties.get(JcrResourceConstants.SLING_RESOURCE_TYPE_PROPERTY) : null);
+            if (isNodeType(rtObj, ctx)) {
+                nodeType = rtObj.toString();
+            } else {
+                nodeType = null;
+            }
+        }
+        return nodeType;
+    }
+    
+    private static boolean isNodeType(@Nullable Object rtObj, @NotNull ResolveContext<JcrProviderState> ctx) {
+        boolean isNodeType = false;
+        if (rtObj != null) {
+            final String resourceType = rtObj.toString();
+            if (resourceType.indexOf(':') != -1 && resourceType.indexOf('/') == -1) {
+                try {
+                    getSession(ctx).getWorkspace().getNodeTypeManager().getNodeType(resourceType);
+                    isNodeType = true;
+                } catch (final RepositoryException ignore) {
+                    // we expect this, if this isn't a valid node type, therefore ignoring
+                }
+            }
+        }
+        return isNodeType;
+    }
+    
+    private static void setProperties(@NotNull Node node, @NotNull String path, @Nullable Map<String, Object> properties, @NotNull ResolveContext<JcrProviderState> ctx) throws PersistenceException {
+        if (properties == null) {
+            return;
+        }
+        // create modifiable map
+        final JcrModifiableValueMap jcrMap = new JcrModifiableValueMap(node, getHelperData(ctx));
+        // check mixin types first
+        final Object value = properties.get(JcrConstants.JCR_MIXINTYPES);
+        if (value != null) {
+            jcrMap.put(JcrConstants.JCR_MIXINTYPES, value);
+        }
+        for (final Map.Entry<String, Object> entry : properties.entrySet()) {
+            if (!IGNORED_PROPERTIES.contains(entry.getKey())) {
+                try {
+                    jcrMap.put(entry.getKey(), entry.getValue());
+                } catch (final IllegalArgumentException iae) {
+                    try {
+                        node.remove();
+                    } catch (final RepositoryException re) {
+                        // we ignore this
+                    }
+                    throw new PersistenceException(iae.getMessage(), iae, path, entry.getKey());
+                }
+            }
+        }
+    }
 }