#280 Improve efficiency of key calculation for cache lookups in the m…

…odel registry

Extracted efficient retrieval of resource type information to NodeUtil
Updated to the current mockito version and updated imports and unused stubbing with regard to the update.

Fixed minor typos and unused throw declarations where applicable

api/pom.xml

@@ -78,7 +78,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
         </dependency>
         <dependency>
             <groupId>org.assertj</groupId>

api/src/test/java/io/neba/api/tags/tags/DefineObjectsTagTest.java

@@ -28,18 +28,17 @@
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.junit.MockitoJUnitRunner;
 
-import javax.servlet.jsp.JspException;
 import javax.servlet.jsp.PageContext;
 import javax.servlet.jsp.tagext.TagSupport;
 import java.util.Map;
 
 import static org.apache.sling.api.scripting.SlingBindings.SLING;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;

core/pom.xml

@@ -135,7 +135,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
         </dependency>
         <dependency>
             <groupId>commons-fileupload</groupId>

core/src/main/java/io/neba/core/resourcemodels/registration/ModelRegistry.java

@@ -31,7 +31,6 @@
 import javax.jcr.Node;
 import javax.jcr.RepositoryException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
@@ -42,6 +41,8 @@
 
 import static io.neba.core.resourcemodels.registration.MappableTypeHierarchy.mappableTypeHierarchyOf;
 import static io.neba.core.util.BundleUtil.displayNameOf;
+import static io.neba.core.util.NodeUtil.geMixinTypes;
+import static io.neba.core.util.NodeUtil.getPrimaryType;
 import static java.util.Collections.unmodifiableCollection;
 
 /**
@@ -76,8 +77,8 @@ private static Key key(Resource resource, Object... furtherKeyElements) {
                 key = new Key(
                         resource.getResourceType(),
                         resource.getResourceSuperType(),
-                        node.getPrimaryNodeType().getName(),
-                        Arrays.toString(node.getMixinNodeTypes()),
+                        getPrimaryType(node),
+                        geMixinTypes(node),
                         furtherElementsKey);
             } catch (RepositoryException e) {
                 throw new RuntimeException("Unable to retrieve the primary type of " + resource + ".", e);
@@ -119,7 +120,7 @@ private static Collection<OsgiModelSource<?>> filter(Collection<OsgiModelSource<
     }
 
     /**
-     * @param sources  can be <code>null</code>.
+     * @param sources   can be <code>null</code>.
      * @param modelName can be <code>null</code>.
      * @return the original collection if sources or modelName are
      * <code>null</code>, or a collection representing the models
@@ -152,7 +153,7 @@ private static Collection<OsgiModelSource<?>> filter(Collection<OsgiModelSource<
      * Finds the most specific models for the given {@link Resource}. The model's model
      * name must match the provided model name.
      *
-     * @param resource must not be <code>null</code>.
+     * @param resource  must not be <code>null</code>.
      * @param modelName must not be <code>null</code>.
      * @return the resolved models, or <code>null</code> if no such models exist.
      */
@@ -194,7 +195,7 @@ Collection<LookupResult> lookupMostSpecificModels(Resource resource, String mode
      * @param resource must not be <code>null</code>.
      * @return the model sources, or <code>null</code> if no models exist for the resource.
      */
-    public Collection<LookupResult> lookupMostSpecificModels(Resource resource) {
+    Collection<LookupResult> lookupMostSpecificModels(Resource resource) {
         if (resource == null) {
             throw new IllegalArgumentException("Method argument resource must not be null.");
         }
@@ -420,7 +421,7 @@ private Collection<LookupResult> resolveModelSources(Resource resource, Class<?>
      * {@link Resource} who's {@link OsgiModelSource#getModelName() model name}
      * matches the given model name.
      *
-     * @param resource must not be <code>null</code>.
+     * @param resource  must not be <code>null</code>.
      * @param modelName can be <code>null</code>.
      * @return never <code>null</code> but rather an empty collection.
      */

core/src/main/java/io/neba/core/util/NodeUtil.java

@@ -0,0 +1,74 @@
+/*
+  Copyright 2013 the original author or authors.
+
+  Licensed under the Apache License, Version 2.0 the "License";
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+package io.neba.core.util;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+
+/**
+ * Utilities for working with {@link javax.jcr.Node JCR nodes}.
+ *
+ * @author Olaf Otto
+ */
+public class NodeUtil {
+    private static final String JCR_PRIMARY_TYPE = "jcr:primaryType";
+    private static final String JCR_MIXIN_TYPES = "jcr:mixinTypes";
+
+    /**
+     * @param node must not ne <code>null</code>.
+     * @return the primary type name of the given node, never <code>null</code>.
+     * @throws RepositoryException if accessing the node fails due to an unrecoverable repository error.
+     */
+    public static @Nonnull
+    String getPrimaryType(@Nonnull Node node) throws RepositoryException {
+        return node.hasProperty(JCR_PRIMARY_TYPE) ?
+                node.getProperty(JCR_PRIMARY_TYPE).getString() :
+                node.getPrimaryNodeType().getName();
+    }
+
+    /**
+     * @param node must not ne <code>null</code>.
+     * @return a String with the comma separated mixin type names assigned to the given node, or <code>null</code> if the node has no mixin types.
+     * @throws RepositoryException if accessing the node fails due to an unrecoverable repository error.
+     */
+    public static @CheckForNull
+    String geMixinTypes(@Nonnull Node node) throws RepositoryException {
+        Object[] mixinTypes = node.hasProperty(JCR_MIXIN_TYPES) ?
+                node.getProperty(JCR_MIXIN_TYPES).getValues() :
+                node.getMixinNodeTypes();
+
+        if (mixinTypes == null || mixinTypes.length == 0) {
+            return null;
+        }
+
+        StringBuilder commaSeparatedMixinTypeNames = new StringBuilder(64);
+
+        for (int i = 0; i < mixinTypes.length; ++i) {
+            commaSeparatedMixinTypeNames.append(mixinTypes[i].toString());
+            if (i < mixinTypes.length - 1) {
+                commaSeparatedMixinTypeNames.append(',');
+            }
+        }
+
+        return commaSeparatedMixinTypeNames.toString();
+    }
+
+    private NodeUtil() {
+    }
+}

core/src/main/java/io/neba/core/util/ResourceTypeHierarchyIterator.java

@@ -16,15 +16,16 @@
 
 package io.neba.core.util;
 
-import java.util.Iterator;
-import java.util.NoSuchElementException;
-import javax.jcr.Node;
-import javax.jcr.RepositoryException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
 
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import static io.neba.api.Constants.SYNTHETIC_RESOURCETYPE_ROOT;
+import static io.neba.core.util.NodeUtil.getPrimaryType;
 import static org.apache.sling.api.resource.ResourceUtil.isSyntheticResource;
 
 /**
@@ -37,9 +38,9 @@
  * falls back to the {@link javax.jcr.Node#getPrimaryNodeType() primary node type}
  * if no "sling:resourceType" property is set; however the node type hierarchy
  * is covered by the {@link NodeTypeHierarchyIterator} and is not used by this iterator.
- * 
- * @see ResourceResolver#getParentResourceType(String)
+ *
  * @author Olaf Otto
+ * @see ResourceResolver#getParentResourceType(String)
  */
 public class ResourceTypeHierarchyIterator implements Iterator<String>, Iterable<String> {
     /**
@@ -80,7 +81,7 @@ public static ResourceTypeHierarchyIterator typeHierarchyOf(final Resource resou
                 // However, the resourceType provided by resource#getResourceType could be the node type since
                 // Resource#getResourceType falls back to the node type of no sling:resourceType is specified.
                 try {
-                    String nodeType = node.getPrimaryNodeType().getName();
+                    String nodeType = getPrimaryType(node);
                     if (!nodeType.equals(resourceType)) {
                         this.currentResourceType = resourceType;
                     }
@@ -122,7 +123,7 @@ private boolean resolveNext() {
      * This iterator provides a virtual resource type root
      * common to all synthetic resources to enable mapping to all
      * resources including synthetic ones.
-     * 
+     *
      * @see io.neba.api.Constants#SYNTHETIC_RESOURCETYPE_ROOT
      */
     private boolean isProvideSyntheticResourceRoot() {

core/src/test/java/io/neba/core/logviewer/LogFilesTest.java

@@ -25,16 +25,16 @@
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.junit.MockitoJUnitRunner;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.service.cm.Configuration;
 import org.osgi.service.cm.ConfigurationAdmin;
 
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;

core/src/test/java/io/neba/core/logviewer/LogfileViewerConsolePluginTest.java

@@ -16,6 +16,24 @@
 
 package io.neba.core.logviewer;
 
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.util.DecoratedObjectFactory;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+
+import javax.servlet.Servlet;
+import javax.servlet.ServletConfig;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.ServletOutputStream;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -29,34 +47,15 @@
 import java.util.Collection;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
-import javax.servlet.Servlet;
-import javax.servlet.ServletConfig;
-import javax.servlet.ServletContext;
-import javax.servlet.ServletException;
-import javax.servlet.ServletOutputStream;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.server.handler.ContextHandler;
-import org.eclipse.jetty.util.DecoratedObjectFactory;
-import org.eclipse.jetty.util.thread.ThreadPool;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
-import org.mockito.stubbing.Answer;
-
 
 import static io.neba.core.util.ReflectionUtil.findField;
 import static io.neba.core.util.ZipFileUtil.toZipFileEntryName;
 import static org.apache.commons.io.FileUtils.listFiles;
 import static org.apache.commons.io.IOUtils.toByteArray;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Matchers.isA;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
@@ -110,27 +109,12 @@ public void setUp() throws Exception {
         this.testLogfileDirectory = new File(testLogfileUrl.getFile());
         this.availableLogFiles = listFiles(this.testLogfileDirectory, null, true);
 
-        when(this.context.getContextHandler())
-                .thenReturn(contextHandler);
-
-        when(this.contextHandler.getServer())
-                .thenReturn(this.server);
-
-        when(this.server.getThreadPool())
-                .thenReturn(mock(ThreadPool.class));
-
         when(this.request.getServletPath())
                 .thenReturn("/system/console");
 
         when(this.request.getServerName())
                 .thenReturn("servername");
 
-        when(this.request.getMethod())
-                .thenReturn("GET");
-
-        when(this.request.getProtocol())
-                .thenReturn("HTTP");
-
         when(this.response.getWriter())
                 .thenReturn(writer);
 
@@ -344,7 +328,6 @@ private void doGet() throws ServletException, IOException {
 
     private void withRequestPath(String requestPath) {
         when(this.request.getRequestURI()).thenReturn(requestPath);
-        when(this.request.getPathInfo()).thenReturn(requestPath);
     }
 
     private void assertHtmlResponseContains(String expected) {

core/src/test/java/io/neba/core/logviewer/TailTest.java

@@ -24,15 +24,15 @@
 import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.junit.MockitoJUnitRunner;
 
 
 import static java.io.File.createTempFile;
 import static java.nio.file.Files.move;
 import static java.util.concurrent.Executors.newSingleThreadExecutor;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;

core/src/test/java/io/neba/core/logviewer/TailTests.java

@@ -27,14 +27,14 @@
 import org.junit.Before;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.junit.MockitoJUnitRunner;
 import org.mockito.stubbing.Answer;
 
 
 import static java.lang.Thread.sleep;
 import static java.lang.Thread.yield;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.isA;
 import static org.mockito.Mockito.never;