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/03/26 16:04:19 UTC

svn commit: r1461165 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: ItemImpl.java NodeImpl.java PropertyImpl.java delegate/ItemDelegate.java

Author: mduerig
Date: Tue Mar 26 15:04:19 2013
New Revision: 1461165

URL: http://svn.apache.org/r1461165
Log:
OAK-672: Avoid JCR APIs calling other JCR APIs
OAK-662: Reduce boilerplate code in JCR impl methods
 refactor node, property and item related methods to use common base class of SessionOperation, which takes care of establishing session preconditions.

Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
    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/PropertyImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ItemDelegate.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java?rev=1461165&r1=1461164&r2=1461165&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java Tue Mar 26 15:04:19 2013
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.oak.jcr;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.jcr.AccessDeniedException;
 import javax.jcr.InvalidItemStateException;
@@ -42,25 +43,63 @@ import org.slf4j.LoggerFactory;
  * TODO document
  */
 abstract class ItemImpl<T extends ItemDelegate> implements Item {
+    private static final Logger log = LoggerFactory.getLogger(ItemImpl.class);
 
     protected final SessionContext sessionContext;
     protected final T dlg;
     protected final SessionDelegate sessionDelegate;
 
-    /**
-     * logger instance
-     */
-    private static final Logger log = LoggerFactory.getLogger(ItemImpl.class);
-
     protected ItemImpl(T itemDelegate, SessionContext sessionContext) {
         this.sessionContext = sessionContext;
         this.dlg = itemDelegate;
         this.sessionDelegate = sessionContext.getSessionDelegate();
     }
 
-    protected <X> X perform(SessionOperation<X> operation)
-            throws RepositoryException {
-        return sessionDelegate.perform(operation);
+    protected abstract class ItemReadOperation<T> extends SessionOperation<T> {
+        @Override
+        protected void checkPreconditions() throws RepositoryException {
+            checkStatus();
+        }
+    }
+
+    protected abstract class ItemWriteOperation<T> extends SessionOperation<T> {
+        @Override
+        protected void checkPreconditions() throws RepositoryException {
+            checkStatus();
+            checkProtected();
+        }
+    }
+
+    /**
+     * Perform the passed {@link org.apache.jackrabbit.oak.jcr.ItemImpl.ItemReadOperation}.
+     * @param op  operation to perform
+     * @param <T>  return type of the operation
+     * @return  the result of {@code op.perform()}
+     * @throws RepositoryException as thrown by {@code op.perform()}.
+     */
+    @CheckForNull
+    protected <T> T perform(@Nonnull SessionOperation<T> op) throws RepositoryException {
+        return sessionDelegate.perform(op);
+    }
+
+    /**
+     * Perform the passed {@link org.apache.jackrabbit.oak.jcr.ItemImpl.ItemReadOperation} assuming it does not throw an
+     * {@code RepositoryException}. If it does, wrap it into and throw it as an
+     * {@code IllegalArgumentException}.
+     * @param op  operation to perform
+     * @param <T>  return type of the operation
+     * @return  the result of {@code op.perform()}
+     */
+    @CheckForNull
+    protected <T> T safePerform(@Nonnull SessionOperation<T> op) {
+        try {
+            return sessionDelegate.perform(op);
+        }
+        catch (RepositoryException e) {
+            String msg = "Unexpected exception thrown by operation " + op;
+            log.error(msg, e);
+            throw new IllegalArgumentException(msg, e);
+        }
     }
 
     //---------------------------------------------------------------< Item >---
@@ -71,7 +110,7 @@ abstract class ItemImpl<T extends ItemDe
     @Override
     @Nonnull
     public String getName() throws RepositoryException {
-        return perform(new SessionOperation<String>() {
+        return perform(new ItemReadOperation<String>() {
             @Override
             public String perform() throws RepositoryException {
                 String oakName = dlg.getName();
@@ -87,12 +126,7 @@ abstract class ItemImpl<T extends ItemDe
     @Override
     @Nonnull
     public String getPath() throws RepositoryException {
-        return perform(new SessionOperation<String>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<String>() {
             @Override
             public String perform() throws RepositoryException {
                 return toJcrPath(dlg.getPath());
@@ -108,12 +142,7 @@ abstract class ItemImpl<T extends ItemDe
 
     @Override
     public Item getAncestor(final int depth) throws RepositoryException {
-        return perform(new SessionOperation<Item>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Item>() {
             @Override
             protected Item perform() throws RepositoryException {
                 if (depth < 0) {
@@ -150,12 +179,7 @@ abstract class ItemImpl<T extends ItemDe
 
     @Override
     public int getDepth() throws RepositoryException {
-        return perform(new SessionOperation<Integer>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Integer>() {
             @Override
             public Integer perform() throws RepositoryException {
                 return PathUtils.getDepth(dlg.getPath());
@@ -225,22 +249,15 @@ abstract class ItemImpl<T extends ItemDe
     }
 
     //-----------------------------------------------------------< internal >---
+
     /**
      * Performs a sanity check on this item and the associated session.
      *
      * @throws RepositoryException if this item has been rendered invalid for some reason
      */
     void checkStatus() throws RepositoryException {
-        if (dlg.isStale()) {
-            throw new InvalidItemStateException("stale");
-        }
-
-        // check session status
-        if (!sessionDelegate.isAlive()) {
-            throw new RepositoryException("This session has been closed.");
-        }
-
-        // TODO: validate item state.
+        sessionDelegate.checkAlive();
+        dlg.checkNotStale();
     }
 
     protected abstract ItemDefinition getDefinition() throws RepositoryException;

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=1461165&r1=1461164&r2=1461165&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 Tue Mar 26 15:04:19 2013
@@ -16,6 +16,11 @@
  */
 package org.apache.jackrabbit.oak.jcr;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static javax.jcr.Property.JCR_LOCK_IS_DEEP;
+import static javax.jcr.Property.JCR_LOCK_OWNER;
+import static javax.jcr.PropertyType.UNDEFINED;
+
 import java.io.InputStream;
 import java.math.BigDecimal;
 import java.util.Arrays;
@@ -78,7 +83,6 @@ import org.apache.jackrabbit.oak.commons
 import org.apache.jackrabbit.oak.core.IdentifierManager;
 import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
 import org.apache.jackrabbit.oak.jcr.delegate.PropertyDelegate;
-import org.apache.jackrabbit.oak.jcr.delegate.SessionOperation;
 import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
 import org.apache.jackrabbit.oak.plugins.nodetype.DefinitionProvider;
 import org.apache.jackrabbit.oak.plugins.nodetype.EffectiveNodeType;
@@ -90,11 +94,6 @@ import org.apache.jackrabbit.value.Value
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.jcr.Property.JCR_LOCK_IS_DEEP;
-import static javax.jcr.Property.JCR_LOCK_OWNER;
-import static javax.jcr.PropertyType.UNDEFINED;
-
 /**
  * TODO document
  *
@@ -127,12 +126,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public Node getParent() throws RepositoryException {
-        return perform(new SessionOperation<NodeImpl<NodeDelegate>>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeImpl<NodeDelegate>>() {
             @Override
             public NodeImpl<NodeDelegate> perform() throws RepositoryException {
                 if (dlg.isRoot()) {
@@ -153,16 +147,12 @@ public class NodeImpl<T extends NodeDele
      */
     @Override
     public boolean isNew() {
-        try {
-            return perform(new SessionOperation<Boolean>() {
-                @Override
-                public Boolean perform() throws InvalidItemStateException {
-                    return !dlg.isStale() && dlg.getStatus() == Status.NEW;
-                }
-            });
-        } catch (RepositoryException e) {
-            return false;
-        }
+        return safePerform(new ItemReadOperation<Boolean>() {
+            @Override
+            public Boolean perform() {
+                return !dlg.isStale() && dlg.getStatus() == Status.NEW;
+            }
+        });
     }
 
     /**
@@ -170,16 +160,12 @@ public class NodeImpl<T extends NodeDele
      */
     @Override
     public boolean isModified() {
-        try {
-            return perform(new SessionOperation<Boolean>() {
-                @Override
-                public Boolean perform() throws InvalidItemStateException {
-                    return !dlg.isStale() && dlg.getStatus() == Status.MODIFIED;
-                }
-            });
-        } catch (RepositoryException e) {
-            return false;
-        }
+        return safePerform(new ItemReadOperation<Boolean>() {
+            @Override
+            public Boolean perform() {
+                return !dlg.isStale() && dlg.getStatus() == Status.MODIFIED;
+            }
+        });
     }
 
     /**
@@ -187,13 +173,7 @@ public class NodeImpl<T extends NodeDele
      */
     @Override
     public void remove() throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-                checkProtected();
-            }
-
+        perform(new ItemWriteOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
                 if (dlg.isRoot()) {
@@ -223,7 +203,6 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public Node addNode(String relPath) throws RepositoryException {
-        checkStatus();
         return addNode(relPath, null);
     }
 
@@ -234,10 +213,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public Node addNode(final String relPath, final String primaryNodeTypeName) throws RepositoryException {
-        checkStatus();
-        checkProtected();
-
-        return perform(new SessionOperation<Node>() {
+        return perform(new ItemWriteOperation<Node>() {
             @Override
             public Node perform() throws RepositoryException {
                 String oakPath = sessionContext.getOakPathKeepIndexOrThrowNotFound(relPath);
@@ -311,13 +287,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public void orderBefore(final String srcChildRelPath, final String destChildRelPath) throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-                checkProtected();
-            }
-
+        perform(new ItemWriteOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
                 getEffectiveNodeType().checkOrderableChildNodes();
@@ -387,14 +357,14 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         if (values != null) {
             int type = UNDEFINED;
-            for (int i = 0; i < values.length; i++) {
-                if (values[i] != null) {
+            for (Value value : values) {
+                if (value != null) {
                     if (type == UNDEFINED) {
-                        type = values[i].getType();
-                    } else if (values[i].getType() != type) {
+                        type = value.getType();
+                    } else if (value.getType() != type) {
                         throw new ValueFormatException(
                                 "All values of a multi-valued property"
-                                + " must be of the same type");
+                                        + " must be of the same type");
                     }
                 }
             }
@@ -551,12 +521,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public Node getNode(final String relPath) throws RepositoryException {
-        return perform(new SessionOperation<NodeImpl<?>>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeImpl<?>>() {
             @Override
             public NodeImpl<?> perform() throws RepositoryException {
                 String oakPath = getOakPathOrThrowNotFound(relPath);
@@ -574,12 +539,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public NodeIterator getNodes() throws RepositoryException {
-        return perform(new SessionOperation<NodeIterator>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeIterator>() {
             @Override
             public NodeIterator perform() throws RepositoryException {
                 Iterator<NodeDelegate> children = dlg.getChildren();
@@ -594,12 +554,7 @@ public class NodeImpl<T extends NodeDele
     public NodeIterator getNodes(final String namePattern)
             throws RepositoryException {
 
-        return perform(new SessionOperation<NodeIterator>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeIterator>() {
             @Override
             public NodeIterator perform() throws RepositoryException {
                 Iterator<NodeDelegate> children = Iterators.filter(dlg.getChildren(),
@@ -622,12 +577,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public NodeIterator getNodes(final String[] nameGlobs) throws RepositoryException {
-        return perform(new SessionOperation<NodeIterator>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeIterator>() {
             @Override
             public NodeIterator perform() throws RepositoryException {
                 Iterator<NodeDelegate> children = Iterators.filter(dlg.getChildren(),
@@ -650,12 +600,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public Property getProperty(final String relPath) throws RepositoryException {
-        return perform(new SessionOperation<PropertyImpl>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<PropertyImpl>() {
             @Override
             public PropertyImpl perform() throws RepositoryException {
                 String oakPath = getOakPathOrThrowNotFound(relPath);
@@ -672,12 +617,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public PropertyIterator getProperties() throws RepositoryException {
-        return perform(new SessionOperation<PropertyIterator>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<PropertyIterator>() {
             @Override
             public PropertyIterator perform() throws RepositoryException {
                 Iterator<PropertyDelegate> properties = dlg.getProperties();
@@ -690,12 +630,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public PropertyIterator getProperties(final String namePattern) throws RepositoryException {
-        return perform(new SessionOperation<PropertyIterator>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<PropertyIterator>() {
             @Override
             public PropertyIterator perform() throws RepositoryException {
                 Iterator<PropertyDelegate> properties = Iterators.filter(dlg.getProperties(),
@@ -718,12 +653,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public PropertyIterator getProperties(final String[] nameGlobs) throws RepositoryException {
-        return perform(new SessionOperation<PropertyIterator>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<PropertyIterator>() {
             @Override
             public PropertyIterator perform() throws RepositoryException {
                 Iterator<PropertyDelegate> propertyNames = Iterators.filter(dlg.getProperties(),
@@ -749,12 +679,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public Item getPrimaryItem() throws RepositoryException {
-        return perform(new SessionOperation<Item>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Item>() {
             @Override
             public Item perform() throws RepositoryException {
                 String name = getPrimaryNodeType().getPrimaryItemName();
@@ -778,12 +703,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public String getUUID() throws RepositoryException {
-        return perform(new SessionOperation<String>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<String>() {
             @Override
             public String perform() throws RepositoryException {
                 if (isNodeType(NodeType.MIX_REFERENCEABLE)) {
@@ -798,12 +718,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public String getIdentifier() throws RepositoryException {
-        return perform(new SessionOperation<String>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<String>() {
             @Override
             public String perform() throws RepositoryException {
                 return dlg.getIdentifier();
@@ -829,7 +744,6 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public PropertyIterator getReferences(final String name) throws RepositoryException {
-        checkStatus();
         return internalGetReferences(name, false);
     }
 
@@ -845,12 +759,11 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public PropertyIterator getWeakReferences(String name) throws RepositoryException {
-        checkStatus();
         return internalGetReferences(name, true);
     }
 
     private PropertyIterator internalGetReferences(final String name, final boolean weak) throws RepositoryException {
-        return perform(new SessionOperation<PropertyIterator>() {
+        return perform(new ItemReadOperation<PropertyIterator>() {
             @Override
             public PropertyIterator perform() throws InvalidItemStateException {
                 IdentifierManager idManager = sessionDelegate.getIdManager();
@@ -874,12 +787,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public boolean hasNode(final String relPath) throws RepositoryException {
-        return perform(new SessionOperation<Boolean>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
                 String oakPath = getOakPath(relPath);
@@ -890,12 +798,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public boolean hasProperty(final String relPath) throws RepositoryException {
-        return perform(new SessionOperation<Boolean>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
                 String oakPath = getOakPath(relPath);
@@ -906,12 +809,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public boolean hasNodes() throws RepositoryException {
-        return perform(new SessionOperation<Boolean>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
                 return dlg.getChildCount() != 0;
@@ -921,12 +819,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public boolean hasProperties() throws RepositoryException {
-        return perform(new SessionOperation<Boolean>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
                 return dlg.getPropertyCount() != 0;
@@ -940,12 +833,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public NodeType getPrimaryNodeType() throws RepositoryException {
-        return perform(new SessionOperation<NodeType>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeType>() {
             @Override
             public NodeType perform() throws RepositoryException {
                 String primaryNtName;
@@ -965,12 +853,7 @@ public class NodeImpl<T extends NodeDele
     @Override
     @Nonnull
     public NodeType[] getMixinNodeTypes() throws RepositoryException {
-        return perform(new SessionOperation<NodeType[]>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<NodeType[]>() {
             @Override
             public NodeType[] perform() throws RepositoryException {
                 // TODO: check if transient changes to mixin-types are reflected here
@@ -1004,25 +887,26 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public void setPrimaryType(final String nodeTypeName) throws RepositoryException {
-        checkStatus();
-        checkProtected();
-        if (!isCheckedOut()) {
-            throw new VersionException("Cannot set primary type. Node is " +
-                    "checked in.");
-        }
+        perform(new ItemWriteOperation<Void>() {
+            @Override
+            protected void checkPreconditions() throws RepositoryException {
+                super.checkPreconditions();
+                if (!isCheckedOut()) {
+                    throw new VersionException("Cannot set primary type. Node is checked in.");
+                }
+            }
 
-        internalSetPrimaryType(nodeTypeName);
+            @Override
+            protected Void perform() throws RepositoryException {
+                internalSetPrimaryType(nodeTypeName);
+                return null;
+            }
+        });
     }
 
     @Override
     public void addMixin(final String mixinName) throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-                checkProtected();
-            }
-
+        perform(new ItemWriteOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
                 // TODO: figure out the right place for this check
@@ -1061,13 +945,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public void removeMixin(final String mixinName) throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-                checkProtected();
-            }
-
+        perform(new ItemWriteOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
                 if (!isNodeType(mixinName)) {
@@ -1081,12 +959,7 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public boolean canAddMixin(final String mixinName) throws RepositoryException {
-        return perform(new SessionOperation<Boolean>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
                 // TODO: figure out the right place for this check
@@ -1410,9 +1283,9 @@ public class NodeImpl<T extends NodeDele
 
     @Override
     public void removeSharedSet() throws RepositoryException {
-        NodeIterator iter = getSharedSet();
-        while (iter.hasNext()) {
-            iter.nextNode().removeShare();
+        NodeIterator sharedSet = getSharedSet();
+        while (sharedSet.hasNext()) {
+            sharedSet.nextNode().removeShare();
         }
     }
 
@@ -1579,24 +1452,18 @@ public class NodeImpl<T extends NodeDele
     }
 
     private void internalSetPrimaryType(final String nodeTypeName) throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            public Void perform() throws RepositoryException {
-                // TODO: figure out the right place for this check
-                NodeType nt = getNodeTypeManager().getNodeType(nodeTypeName); // throws on not found
-                if (nt.isAbstract() || nt.isMixin()) {
-                    throw new ConstraintViolationException();
-                }
-                // TODO: END
+        // TODO: figure out the right place for this check
+        NodeType nt = getNodeTypeManager().getNodeType(nodeTypeName); // throws on not found
+        if (nt.isAbstract() || nt.isMixin()) {
+            throw new ConstraintViolationException();
+        }
+        // TODO: END
 
-                String jcrPrimaryType = getOakPath(Property.JCR_PRIMARY_TYPE);
-                Value value = getValueFactory().createValue(nodeTypeName, PropertyType.NAME);
+        String jcrPrimaryType = getOakPath(Property.JCR_PRIMARY_TYPE);
+        Value value = getValueFactory().createValue(nodeTypeName, PropertyType.NAME);
 
-                dlg.setProperty(PropertyStates.createProperty(jcrPrimaryType, value));
-                dlg.setOrderableChildren(nt.hasOrderableChildNodes());
-                return null;
-            }
-        });
+        dlg.setProperty(PropertyStates.createProperty(jcrPrimaryType, value));
+        dlg.setOrderableChildren(nt.hasOrderableChildNodes());
     }
 
     private Property internalSetProperty(
@@ -1604,14 +1471,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         final String oakName = getOakPath(checkNotNull(jcrName));
         checkNotNull(value);
-        return perform(new SessionOperation<Property>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                // TODO: Reduce boilerplate (OAK-662)
-                checkStatus();
-                checkProtected();
-            }
-
+        return perform(new ItemWriteOperation<Property>() {
             @Override
             public Property perform() throws RepositoryException {
                 PropertyState state =
@@ -1644,14 +1504,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         final String oakName = getOakPath(checkNotNull(jcrName));
         final Value[] nonNullValues = compact(checkNotNull(values));
-        return perform(new SessionOperation<Property>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                // TODO: Reduce boilerplate (OAK-662)
-                checkStatus();
-                checkProtected();
-            }
-
+        return perform(new ItemWriteOperation<Property>() {
             @Override
             public Property perform() throws RepositoryException {
                 PropertyState state = PropertyStates.createProperty(
@@ -1681,14 +1534,7 @@ public class NodeImpl<T extends NodeDele
     private Property internalRemoveProperty(final String jcrName)
             throws RepositoryException {
         final String oakName = getOakPath(checkNotNull(jcrName));
-        return perform(new SessionOperation<Property>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                // TODO: Reduce boilerplate (OAK-662)
-                checkStatus();
-                checkProtected();
-            }
-
+        return perform(new ItemWriteOperation<Property>() {
             @Override
             protected Property perform() throws RepositoryException {
                 // TODO: Avoid extra JCR method calls (OAK-672)
@@ -1715,8 +1561,8 @@ public class NodeImpl<T extends NodeDele
      */
     private static Value[] compact(Value[] values) {
         int n = 0;
-        for (int i = 0; i < values.length; i++) {
-            if (values[i] != null) {
+        for (Value value : values) {
+            if (value != null) {
                 n++;
             }
         }

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java?rev=1461165&r1=1461164&r2=1461165&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java Tue Mar 26 15:04:19 2013
@@ -16,6 +16,8 @@
  */
 package org.apache.jackrabbit.oak.jcr;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import java.io.InputStream;
 import java.math.BigDecimal;
 import java.util.ArrayList;
@@ -42,15 +44,12 @@ import com.google.common.collect.Iterabl
 import org.apache.jackrabbit.oak.api.Tree.Status;
 import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
 import org.apache.jackrabbit.oak.jcr.delegate.PropertyDelegate;
-import org.apache.jackrabbit.oak.jcr.delegate.SessionOperation;
 import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
 import org.apache.jackrabbit.oak.plugins.value.ValueFactoryImpl;
 import org.apache.jackrabbit.value.ValueHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 /**
  * TODO document
  */
@@ -83,7 +82,7 @@ public class PropertyImpl extends ItemIm
     @Override
     @Nonnull
     public Node getParent() throws RepositoryException {
-        return perform(new SessionOperation<NodeImpl<?>>() {
+        return perform(new ItemReadOperation<NodeImpl<?>>() {
             @Override
             public NodeImpl<?> perform() throws RepositoryException {
                 NodeDelegate parent = dlg.getParent();
@@ -101,16 +100,12 @@ public class PropertyImpl extends ItemIm
      */
     @Override
     public boolean isNew() {
-        try {
-            return perform(new SessionOperation<Boolean>() {
-                @Override
-                public Boolean perform() throws RepositoryException {
-                    return dlg.getStatus() == Status.NEW;
-                }
-            });
-        } catch (RepositoryException e) {
-            return false;
-        }
+        return safePerform(new ItemReadOperation<Boolean>() {
+            @Override
+            public Boolean perform() {
+                return dlg.getStatus() == Status.NEW;
+            }
+        });
     }
 
     /**
@@ -118,16 +113,12 @@ public class PropertyImpl extends ItemIm
      */
     @Override
     public boolean isModified() {
-        try {
-            return perform(new SessionOperation<Boolean>() {
-                @Override
-                public Boolean perform() throws RepositoryException {
-                    return dlg.getStatus() == Status.MODIFIED;
-                }
-            });
-        } catch (RepositoryException e) {
-            return false;
-        }
+        return safePerform(new ItemReadOperation<Boolean>() {
+            @Override
+            public Boolean perform() {
+                return dlg.getStatus() == Status.MODIFIED;
+            }
+        });
     }
 
     /**
@@ -135,13 +126,7 @@ public class PropertyImpl extends ItemIm
      */
     @Override
     public void remove() throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-                checkProtected();
-            }
-
+        perform(new ItemWriteOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
                 dlg.remove();
@@ -178,12 +163,7 @@ public class PropertyImpl extends ItemIm
      */
     @Override
     public void setValue(final Value[] values) throws RepositoryException {
-        perform(new SessionOperation<Void>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        perform(new ItemReadOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
                 // assert equal types for all values entries
@@ -358,12 +338,7 @@ public class PropertyImpl extends ItemIm
     @Override
     @Nonnull
     public Value getValue() throws RepositoryException {
-        return perform(new SessionOperation<Value>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Value>() {
             @Override
             public Value perform() throws RepositoryException {
                 return ValueFactoryImpl.createValue(dlg.getSingleState(), sessionContext);
@@ -374,12 +349,7 @@ public class PropertyImpl extends ItemIm
     @Override
     @Nonnull
     public Value[] getValues() throws RepositoryException {
-        return perform(new SessionOperation<List<Value>>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<List<Value>>() {
             @Override
             public List<Value> perform() throws RepositoryException {
                 return ValueFactoryImpl.createValues(dlg.getMultiState(), sessionContext);
@@ -463,7 +433,7 @@ public class PropertyImpl extends ItemIm
     @Override
     @Nonnull
     public Node getNode() throws RepositoryException {
-        return perform(new SessionOperation<Node>() {
+        return perform(new ItemReadOperation<Node>() {
             @Override
             public Node perform() throws RepositoryException {
                 Value value = getValue();
@@ -518,7 +488,7 @@ public class PropertyImpl extends ItemIm
     @Override
     @Nonnull
     public Property getProperty() throws RepositoryException {
-        return perform(new SessionOperation<Property>() {
+        return perform(new ItemReadOperation<Property>() {
             @Override
             public Property perform() throws RepositoryException {
                 Value value = getValue();
@@ -567,7 +537,7 @@ public class PropertyImpl extends ItemIm
      */
     @Override
     public int getType() throws RepositoryException {
-        return perform(new SessionOperation<Integer>() {
+        return perform(new ItemReadOperation<Integer>() {
             @Override
             public Integer perform() throws RepositoryException {
                 if (isMultiple()) {
@@ -587,12 +557,7 @@ public class PropertyImpl extends ItemIm
 
     @Override
     public boolean isMultiple() throws RepositoryException {
-        return perform(new SessionOperation<Boolean>() {
-            @Override
-            protected void checkPreconditions() throws RepositoryException {
-                checkStatus();
-            }
-
+        return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
                 return dlg.isArray();

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=1461165&r1=1461164&r2=1461165&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 Tue Mar 26 15:04:19 2013
@@ -17,6 +17,9 @@
 
 package org.apache.jackrabbit.oak.jcr.delegate;
 
+import static com.google.common.base.Objects.toStringHelper;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.jcr.InvalidItemStateException;
@@ -25,9 +28,6 @@ import org.apache.jackrabbit.oak.api.Tre
 import org.apache.jackrabbit.oak.api.TreeLocation;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 
-import static com.google.common.base.Objects.toStringHelper;
-import static com.google.common.base.Preconditions.checkNotNull;
-
 /**
  * Abstract base class for {@link NodeDelegate} and {@link PropertyDelegate}
  */
@@ -79,6 +79,12 @@ public abstract class ItemDelegate {
         return NodeDelegate.create(sessionDelegate, getLocation().getParent());
     }
 
+    public void checkNotStale() throws InvalidItemStateException {
+        if (isStale()) {
+            throw new InvalidItemStateException("stale");
+        }
+    }
+
     /**
      * Determine whether this item is stale
      * @return  {@code true} iff stale
@@ -88,16 +94,12 @@ public abstract class ItemDelegate {
     }
 
     /**
-     * Get the status of this item
-     * @return  {@link Status} of this item
+     * Get the status of this item.
+     * @return  {@link Status} of this item or {@code null} if not available.
      */
-    @Nonnull
-    public Status getStatus() throws InvalidItemStateException {
-        Status status = getLocation().getStatus();
-        if (status == null) {
-            throw new InvalidItemStateException();
-        }
-        return status;
+    @CheckForNull
+    public Status getStatus() {
+        return loadLocation().getStatus();
     }
 
     /**