Author: rombert
Date: Tue Jan 26 13:49:14 2016
New Revision: 1726793
URL: http://svn.apache.org/viewvc?rev=1726793&view=rev
Log:
SLING-4585 - Performance: Use JackrabbitSession#getItemOrNull to speed
up JcrResourceProvider#createResource
* Update Jackrabbit and Oak
* Fix calendar test issues which happen due to Jackrabbit/Oak upgrade
* Use getItemOrNull if session is a JackrabbitSession
Submitted-By: Joel Richard
Added:
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/AssertCalendar.java
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java
Modified:
sling/trunk/bundles/jcr/resource/pom.xml
sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrPropertyMapTest.java
Modified: sling/trunk/bundles/jcr/resource/pom.xml
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/pom.xml?rev=1726793&r1=1726792&r2=1726793&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/resource/pom.xml (original)
+++ sling/trunk/bundles/jcr/resource/pom.xml Tue Jan 26 13:49:14 2016
@@ -50,6 +50,8 @@
<properties>
<site.jira.version.id>12314286</site.jira.version.id>
<site.javadoc.exclude>**.internal.**</site.javadoc.exclude>
+ <oak.version>1.3.10</oak.version>
+ <jackrabbit.version>2.11.2</jackrabbit.version>
</properties>
<build>
@@ -143,19 +145,19 @@
<dependency>
<groupId>org.apache.jackrabbit</groupId>
<artifactId>jackrabbit-api</artifactId>
- <version>2.10.1</version>
+ <version>${jackrabbit.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.jackrabbit</groupId>
<artifactId>oak-core</artifactId>
- <version>1.3.0</version>
+ <version>${oak.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.jackrabbit</groupId>
<artifactId>jackrabbit-jcr-commons</artifactId>
- <version>2.10.1</version>
+ <version>${jackrabbit.version}</version>
<scope>provided</scope>
</dependency>
@@ -245,7 +247,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.commons.testing</artifactId>
- <version>2.0.6</version>
+ <version>2.0.24</version>
<scope>test</scope>
</dependency>
<dependency>
@@ -263,7 +265,7 @@
<dependency>
<groupId>org.apache.jackrabbit</groupId>
<artifactId>oak-jcr</artifactId>
- <version>1.3.0</version>
+ <version>${oak.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
Modified:
sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java?rev=1726793&r1=1726792&r2=1726793&view=diff
==============================================================================
---
sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java
(original)
+++
sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java
Tue Jan 26 13:49:14 2016
@@ -31,6 +31,7 @@ import javax.jcr.version.VersionManager;
import org.apache.commons.lang.StringUtils;
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.api.JackrabbitSession;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.jcr.resource.internal.HelperData;
@@ -84,15 +85,15 @@ public class JcrItemResourceFactory {
parentResourcePath = parent.getPath();
}
- Item item = null;
+ Item item;
if (parentNode != null && resourcePath.startsWith(parentResourcePath))
{
String subPath =
resourcePath.substring(parentResourcePath.length());
if (!subPath.isEmpty() && subPath.charAt(0) == '/') {
subPath = subPath.substring(1);
}
item = getSubitem(parentNode, subPath);
- } else if (itemExists(jcrPath)) {
- item = session.getItem(jcrPath);
+ } else {
+ item = getItemOrNull(jcrPath);
}
if (item != null && version != null) {
@@ -168,25 +169,25 @@ public class JcrItemResourceFactory {
private static boolean isVersionable(Item item) throws RepositoryException
{
return item.isNode() && ((Node)
item).isNodeType(JcrConstants.MIX_VERSIONABLE);
}
-
- /**
- * Checks whether the item exists and this content manager's session has
- * read access to the item. If the item does not exist, access control is
- * ignored by this method and <code>false</code> is returned.
- *
- * @param path The path to the item to check
- * @return <code>true</code> if the item exists and this content manager's
- * session has read access. If the item does not exist,
- * <code>false</code> is returned ignoring access control.
- */
- private boolean itemExists(final String path) {
- try {
- return session.itemExists(path);
- } catch (RepositoryException re) {
- log.debug("itemExists: Error checking for existence of {}: {}",
- path, re.toString());
- return false;
+ Item getItemOrNull(String path) throws RepositoryException {
+ Item item;
+ // Check first if the path is absolute. If it isn't, then we return
null because the previous itemExists method,
+ // which was replaced by this method, would have returned null as well
(instead of throwing an exception).
+ if (path.isEmpty() || path.charAt(0) != '/') {
+ item = null;
+ }
+ // Use fast getItemOrNull if session is a JackrabbitSession
+ else if (session instanceof JackrabbitSession) {
+ item = ((JackrabbitSession) session).getItemOrNull(path);
+ }
+ // Fallback to slower itemExists & getItem pattern
+ else if (session.itemExists(path)) {
+ item = session.getItem(path);
+ } else {
+ item = null;
}
+ return item;
}
+
}
Added:
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/AssertCalendar.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/AssertCalendar.java?rev=1726793&view=auto
==============================================================================
---
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/AssertCalendar.java
(added)
+++
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/AssertCalendar.java
Tue Jan 26 13:49:14 2016
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You 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 org.apache.sling.jcr.resource.internal;
+
+import org.junit.Assert;
+
+import java.util.Calendar;
+
+public final class AssertCalendar {
+
+ /**
+ * Asserts that two calendars are equal.
+ * <p>
+ * This differs from {@link Assert#assertEquals(Object, Object)} in the
way how the time zone is compared. While
+ * assertEquals expects the exact same {@link java.util.TimeZone} object,
this method is satisfied if the TimeZone
+ * objects describes the same time zone/have the same offset.
+ */
+ public static void assertEqualsCalendar(Calendar expected, Calendar
actual) {
+ if (expected == null) {
+ Assert.assertNull(actual);
+ } else {
+ Assert.assertNotNull(actual);
+ Assert.assertEquals(expected.getTime(), actual.getTime());
+ Assert.assertEquals(expected.getTimeZone().getRawOffset(),
actual.getTimeZone().getRawOffset());
+ }
+ }
+
+}
Modified:
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java?rev=1726793&r1=1726792&r2=1726793&view=diff
==============================================================================
---
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
(original)
+++
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
Tue Jan 26 13:49:14 2016
@@ -41,6 +41,8 @@ import org.apache.sling.api.resource.Val
import org.apache.sling.commons.testing.jcr.RepositoryTestBase;
import org.apache.sling.jcr.resource.JcrResourceUtil;
+import static
org.apache.sling.jcr.resource.internal.AssertCalendar.assertEqualsCalendar;
+
public class JcrModifiableValueMapTest extends RepositoryTestBase {
private String rootPath;
@@ -308,11 +310,11 @@ public class JcrModifiableValueMapTest e
// read with property map
final ValueMap vm = new JcrModifiableValueMap(testNode,
getHelperData());
assertEquals(dateValue1, vm.get(PROP1, Date.class));
- assertEquals(calendarValue1, vm.get(PROP1, Calendar.class));
+ assertEqualsCalendar(calendarValue1, vm.get(PROP1, Calendar.class));
assertEquals(dateValue2, vm.get(PROP2, Date.class));
- assertEquals(calendarValue2, vm.get(PROP2, Calendar.class));
+ assertEqualsCalendar(calendarValue2, vm.get(PROP2, Calendar.class));
assertEquals(dateValue3, vm.get(PROP3, Date.class));
- assertEquals(calendarValue3, vm.get(PROP3, Calendar.class));
+ assertEqualsCalendar(calendarValue3, vm.get(PROP3, Calendar.class));
// check types
assertTrue(vm.get(PROP1) instanceof Calendar);
@@ -320,8 +322,8 @@ public class JcrModifiableValueMapTest e
assertTrue(vm.get(PROP3) instanceof Calendar);
// read properties
- assertEquals(calendarValue1, testNode.getProperty(PROP1).getDate());
- assertEquals(calendarValue3, testNode.getProperty(PROP3).getDate());
+ assertEqualsCalendar(calendarValue1,
testNode.getProperty(PROP1).getDate());
+ assertEqualsCalendar(calendarValue3,
testNode.getProperty(PROP3).getDate());
}
Modified:
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrPropertyMapTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrPropertyMapTest.java?rev=1726793&r1=1726792&r2=1726793&view=diff
==============================================================================
---
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrPropertyMapTest.java
(original)
+++
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrPropertyMapTest.java
Tue Jan 26 13:49:14 2016
@@ -38,6 +38,8 @@ import org.apache.sling.api.resource.Val
import org.apache.sling.commons.testing.jcr.RepositoryTestBase;
import org.apache.sling.jcr.resource.JcrPropertyMap;
+import static
org.apache.sling.jcr.resource.internal.AssertCalendar.assertEqualsCalendar;
+
public class JcrPropertyMapTest extends RepositoryTestBase {
private static final String PROP_NAME = "prop_name";
@@ -251,6 +253,11 @@ public class JcrPropertyMapTest extends
assertEquals(value, map.get(PROP_NAME));
}
+ private void testValue(Node node, Calendar value) throws
RepositoryException {
+ ValueMap map = createProperty(node, value);
+ assertEqualsCalendar(value, map.get(PROP_NAME, Calendar.class));
+ }
+
private ValueMap createProperty(Node node, Object value)
throws RepositoryException {
if (node.hasProperty(PROP_NAME)) {
Added:
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java?rev=1726793&view=auto
==============================================================================
---
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java
(added)
+++
sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java
Tue Jan 26 13:49:14 2016
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.sling.jcr.resource.internal.helper.jcr;
+
+import org.apache.jackrabbit.commons.JcrUtils;
+import org.apache.sling.commons.testing.jcr.RepositoryTestBase;
+import org.apache.sling.jcr.resource.internal.HelperData;
+import org.apache.sling.jcr.resource.internal.PathMapperImpl;
+
+import javax.jcr.Item;
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+
+public class JcrItemResourceFactoryTest extends RepositoryTestBase {
+
+ public static final String EXISTING_NODE_PATH = "/existing";
+ public static final String NON_EXISTING_NODE_PATH = "/nonexisting";
+ public static final String NON_ABSOLUTE_PATH = "invalidpath";
+
+ private Node node;
+ private Session nonJackrabbitSession;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ final Session session = getSession();
+ node = JcrUtils.getOrCreateByPath(EXISTING_NODE_PATH,
"nt:unstructured", session);
+ session.save();
+
+ nonJackrabbitSession = (Session) Proxy.newProxyInstance(
+ getClass().getClassLoader(),
+ new Class<?>[]{Session.class},
+ new InvocationHandler() {
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
+ return method.invoke(session, args);
+ }
+ });
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ node.remove();
+ session.save();
+ super.tearDown();
+ }
+
+ // The following tests ensure that the behaviour between itemExists &
getItem and Jackrabbits getItemOrNull is
+ // the same
+
+ public void testGetItemOrNullExistingItem() throws RepositoryException {
+ compareGetItemOrNull(EXISTING_NODE_PATH, EXISTING_NODE_PATH);
+ }
+
+ public void testGetItemOrNullNonExistingItem() throws RepositoryException {
+ compareGetItemOrNull(NON_EXISTING_NODE_PATH, null);
+ }
+
+ public void testGetItemOrNullNonAbsolutePath() throws RepositoryException {
+ compareGetItemOrNull(NON_ABSOLUTE_PATH, null);
+ }
+
+ public void testGetItemOrNullEmptyPath() throws RepositoryException {
+ compareGetItemOrNull("", null);
+ }
+
+ private void compareGetItemOrNull(String path, String expectedPath) throws
RepositoryException {
+ HelperData helper = new HelperData(null, new PathMapperImpl());
+ Item item1 = new JcrItemResourceFactory(session,
helper).getItemOrNull(path);
+ Item item2 = new JcrItemResourceFactory(nonJackrabbitSession,
helper).getItemOrNull(path);
+ if (expectedPath == null) {
+ assertNull(item1);
+ assertNull(item2);
+ } else {
+ assertNotNull(item1);
+ assertEquals(expectedPath, item1.getPath());
+ assertNotNull(item2);
+ assertEquals(expectedPath, item2.getPath());
+ }
+ }
+
+}
