You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2013/04/03 13:08:49 UTC

svn commit: r1463916 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: ./ delegate/ version/

Author: mduerig
Date: Wed Apr  3 11:08:49 2013
New Revision: 1463916

URL: http://svn.apache.org/r1463916
Log:
OAK-672: Avoid JCR APIs calling other JCR APIs
 make ItemDelegate.getLocation package private

Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/VersionHistoryDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionImpl.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1463916&r1=1463915&r2=1463916&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java Wed Apr  3 11:08:49 2013
@@ -225,7 +225,7 @@ public class NodeImpl<T extends NodeDele
                     NodeDelegate grandParent = dlg.getChild(grandParentPath);
                     if (grandParent != null) {
                         String propName = PathUtils.getName(parentPath);
-                        if (grandParent.getProperty(propName) != null) {
+                        if (grandParent.getPropertyOrNull(propName) != null) {
                             throw new ConstraintViolationException("Can't add new node to property.");
                         }
                     }
@@ -574,7 +574,7 @@ public class NodeImpl<T extends NodeDele
             @Override
             public PropertyImpl perform() throws RepositoryException {
                 String oakPath = getOakPathOrThrowNotFound(relPath);
-                PropertyDelegate pd = dlg.getProperty(oakPath);
+                PropertyDelegate pd = dlg.getPropertyOrNull(oakPath);
                 if (pd == null) {
                     throw new PathNotFoundException(relPath + " not found on " + getPath());
                 } else {
@@ -772,7 +772,7 @@ public class NodeImpl<T extends NodeDele
             @Override
             public Boolean perform() throws RepositoryException {
                 String oakPath = getOakPathOrThrow(relPath);
-                return dlg.getProperty(oakPath) != null;
+                return dlg.getPropertyOrNull(oakPath) != null;
             }
         });
     }
@@ -885,7 +885,7 @@ public class NodeImpl<T extends NodeDele
                     return null;
                 }
 
-                PropertyDelegate mixins = dlg.getProperty(JcrConstants.JCR_MIXINTYPES);
+                PropertyDelegate mixins = dlg.getPropertyOrNull(JcrConstants.JCR_MIXINTYPES);
                 Value value = getValueFactory().createValue(mixinName, PropertyType.NAME);
 
                 boolean nodeModified = false;
@@ -1100,14 +1100,14 @@ public class NodeImpl<T extends NodeDele
                 String lockOwner = getOakPathOrThrow(JCR_LOCK_OWNER);
                 String lockIsDeep = getOakPathOrThrow(JCR_LOCK_IS_DEEP);
 
-                if (dlg.getProperty(lockOwner) != null) {
+                if (dlg.getPropertyOrNull(lockOwner) != null) {
                     return true;
                 }
 
                 NodeDelegate parent = dlg.getParent();
                 while (parent != null) {
-                    if (parent.getProperty(lockOwner) != null) {
-                        PropertyDelegate isDeep = parent.getProperty(lockIsDeep);
+                    if (parent.getPropertyOrNull(lockOwner) != null) {
+                        PropertyDelegate isDeep = parent.getPropertyOrNull(lockIsDeep);
                         if (isDeep != null && !isDeep.isArray()) {
                             if (isDeep.getBoolean()) {
                                 return true;
@@ -1132,7 +1132,7 @@ public class NodeImpl<T extends NodeDele
             @Override
             protected Boolean perform() throws RepositoryException {
                 String lockOwner = getOakPathOrThrow(JCR_LOCK_OWNER);
-                return dlg.getProperty(lockOwner) != null;
+                return dlg.getPropertyOrNull(lockOwner) != null;
             }
         });
     }
@@ -1337,7 +1337,7 @@ public class NodeImpl<T extends NodeDele
     private void autoCreateItems() throws RepositoryException {
         EffectiveNodeType effective = getEffectiveNodeType();
         for (PropertyDefinition pd : effective.getAutoCreatePropertyDefinitions()) {
-            if (dlg.getProperty(pd.getName()) == null) {
+            if (dlg.getPropertyOrNull(pd.getName()) == null) {
                 if (pd.isMultiple()) {
                     dlg.setProperty(PropertyStates.createProperty(pd.getName(), getAutoCreatedValues(pd)));
                 } else {
@@ -1492,11 +1492,8 @@ public class NodeImpl<T extends NodeDele
                     property.remove();
                     return property;
                 } else {
-                    // Return a property instance which throws on access. See
-                    // OAK-395
-                    return new PropertyImpl(new PropertyDelegate(
-                            sessionDelegate, dlg.getLocation()
-                                    .getChild(oakName)), sessionContext);
+                    // Return a property instance which throws on access. See OAK-395
+                    return new PropertyImpl(dlg.getProperty(oakName), sessionContext);
                 }
             }
         });

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java?rev=1463916&r1=1463915&r2=1463916&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java Wed Apr  3 11:08:49 2013
@@ -94,22 +94,24 @@ public abstract class ItemDelegate {
         return location.getStatus();
     }
 
+    @Override
+    public String toString() {
+        return toStringHelper(this).add("location", location).toString();
+    }
+
+    //------------------------------------------------------------< internal >---
+
     /**
      * The underlying {@link org.apache.jackrabbit.oak.api.TreeLocation} of this item.
      * @return  tree location of the underlying item
      * @throws InvalidItemStateException if the location points to a stale item
      */
-    @Nonnull // FIXME this should be package private. OAK-672
-    public TreeLocation getLocation() throws InvalidItemStateException {
+    @Nonnull
+    TreeLocation getLocation() throws InvalidItemStateException {
         if (!location.exists()) {
             throw new InvalidItemStateException("Item is stale");
         }
         return location;
     }
 
-    @Override
-    public String toString() {
-        return toStringHelper(this).add("location", location).toString();
-    }
-
 }

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java?rev=1463916&r1=1463915&r2=1463916&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java Wed Apr  3 11:08:49 2013
@@ -96,15 +96,28 @@ public class NodeDelegate extends ItemDe
      *         no such property exists
      */
     @CheckForNull
-    public PropertyDelegate getProperty(String relPath) throws RepositoryException {
+    public PropertyDelegate getPropertyOrNull(String relPath) throws RepositoryException {
         TreeLocation propertyLocation = getChildLocation(relPath);
-        PropertyState propertyState = propertyLocation.getProperty();
-        return propertyState == null
+        return propertyLocation.getProperty() == null
                 ? null
                 : new PropertyDelegate(sessionDelegate, propertyLocation);
     }
 
     /**
+     * Get a property. In contrast to {@link #getPropertyOrNull(String)} this
+     * method never returns {@code null}. In the case where no property exists
+     * at the given path, the returned property delegate throws an
+     * {@code InvalidItemStateException} on access. See See OAK-395.
+     *
+     * @param relPath oak path
+     * @return property at the path given by {@code relPath}.
+     */
+    @Nonnull
+    public PropertyDelegate getProperty(String relPath) throws RepositoryException {
+        return new PropertyDelegate(sessionDelegate, getChildLocation(relPath));
+    }
+
+    /**
      * Get the properties of the node
      *
      * @return properties of the node

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java?rev=1463916&r1=1463915&r2=1463916&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java Wed Apr  3 11:08:49 2013
@@ -32,7 +32,7 @@ import org.apache.jackrabbit.oak.api.Typ
  */
 public class PropertyDelegate extends ItemDelegate {
 
-    public PropertyDelegate(SessionDelegate sessionDelegate, TreeLocation location) {
+    PropertyDelegate(SessionDelegate sessionDelegate, TreeLocation location) {
         super(sessionDelegate, location);
     }
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/VersionHistoryDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/VersionHistoryDelegate.java?rev=1463916&r1=1463915&r2=1463916&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/VersionHistoryDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/VersionHistoryDelegate.java Wed Apr  3 11:08:49 2013
@@ -16,6 +16,8 @@
  */
 package org.apache.jackrabbit.oak.jcr.delegate;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Iterator;
@@ -37,8 +39,6 @@ import org.apache.jackrabbit.oak.api.Typ
 import org.apache.jackrabbit.oak.plugins.value.Conversions;
 import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 /**
  * {@code VersionHistoryDelegate}...
  */
@@ -126,7 +126,7 @@ public class VersionHistoryDelegate exte
             NodeDelegate n = it.next();
             String primaryType = n.getProperty(JcrConstants.JCR_PRIMARYTYPE).getString();
             if (primaryType.equals(VersionConstants.NT_VERSION)) {
-                PropertyDelegate created = n.getProperty(JcrConstants.JCR_CREATED);
+                PropertyDelegate created = n.getPropertyOrNull(JcrConstants.JCR_CREATED);
                 if (created != null) {
                     Calendar cal = Conversions.convert(created.getDate()).toCalendar();
                     versions.put(cal, n.getName());

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionImpl.java?rev=1463916&r1=1463915&r2=1463916&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionImpl.java Wed Apr  3 11:08:49 2013
@@ -16,6 +16,8 @@
  */
 package org.apache.jackrabbit.oak.jcr.version;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.List;
@@ -40,8 +42,6 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
 import org.apache.jackrabbit.oak.util.TODO;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 class VersionImpl extends NodeImpl<VersionDelegate> implements Version {
 
     public VersionImpl(VersionDelegate dlg, SessionContext sessionContext) {
@@ -114,7 +114,7 @@ class VersionImpl extends NodeImpl<Versi
     @Nonnull
     private PropertyDelegate getPropertyOrThrow(@Nonnull String name)
             throws RepositoryException {
-        PropertyDelegate p = dlg.getProperty(checkNotNull(name));
+        PropertyDelegate p = dlg.getPropertyOrNull(checkNotNull(name));
         if (p == null) {
             throw new RepositoryException("Inconsistent version storage. " +
                     "Version does not have a " + name + " property.");