jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Mueller <muel...@adobe.com>
Subject Re: svn commit: r1334046 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: NodeDelegate.java NodeImpl.java SessionDelegate.java
Date Mon, 07 May 2012 07:08:03 GMT
Hi,

> +        try {
> +   return dlg.getNodeStatus() == Status.MODIFIED;
> +        } catch (InvalidItemStateException ex) {
> +   return false;
> +        }

I would rather not use exception handling for flow control. One reason is
that throwing exceptions is very slow (the fillStackTrace() method is
slow).

Is there an alternative to the try ... catch, or is the exception only
thrown in very rare edge cases?

Regards,
Thomas





On 5/4/12 5:52 PM, "reschke@apache.org" <reschke@apache.org> wrote:

>Author: reschke
>Date: Fri May  4 15:52:35 2012
>New Revision: 1334046
>
>URL: http://svn.apache.org/viewvc?rev=1334046&view=rev
>Log:
>avoid NPEs, add minimal checks in move operations
>
>Modified:
>    
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeDelegate.java
>    
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeImpl.java
>    
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/S
>essionDelegate.java
>
>Modified: 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeDelegate.java
>URL: 
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/or
>g/apache/jackrabbit/oak/jcr/NodeDelegate.java?rev=1334046&r1=1334045&r2=13
>34046&view=diff
>==========================================================================
>====
>--- 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeDelegate.java (original)
>+++ 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeDelegate.java Fri May  4 15:52:35 2012
>@@ -65,8 +65,8 @@ public class NodeDelegate extends ItemDe
>         return getTree().getName();
>     }
> 
>-    Status getNodeStatus() {
>-        return getTree().getParent().getChildStatus(getName());
>+    Status getNodeStatus() throws InvalidItemStateException {
>+        return check(getTree().getParent()).getChildStatus(getName());
>     }
> 
>     NodeDelegate getNodeOrNull(String relOakPath) {
>
>Modified: 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeImpl.java
>URL: 
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/or
>g/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1334046&r1=1334045&r2=133404
>6&view=diff
>==========================================================================
>====
>--- 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeImpl.java (original)
>+++ 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/N
>odeImpl.java Fri May  4 15:52:35 2012
>@@ -31,6 +31,7 @@ import org.slf4j.Logger;
> import org.slf4j.LoggerFactory;
> 
> import javax.jcr.Binary;
>+import javax.jcr.InvalidItemStateException;
> import javax.jcr.Item;
> import javax.jcr.ItemNotFoundException;
> import javax.jcr.ItemVisitor;
>@@ -103,7 +104,11 @@ public class NodeImpl extends ItemImpl i
>      */
>     @Override
>     public boolean isNew() {
>-        return dlg.getNodeStatus() == Status.NEW;
>+        try {
>+            return dlg.getNodeStatus() == Status.NEW;
>+        } catch (InvalidItemStateException ex) {
>+            return false;
>+        }
>     }
> 
>     /**
>@@ -111,7 +116,11 @@ public class NodeImpl extends ItemImpl i
>      */
>     @Override
>     public boolean isModified() {
>-        return dlg.getNodeStatus() == Status.MODIFIED;
>+        try {
>+            return dlg.getNodeStatus() == Status.MODIFIED;
>+        } catch (InvalidItemStateException ex) {
>+            return false;
>+        }
>     }
> 
>     /**
>
>Modified: 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/S
>essionDelegate.java
>URL: 
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/or
>g/apache/jackrabbit/oak/jcr/SessionDelegate.java?rev=1334046&r1=1334045&r2
>=1334046&view=diff
>==========================================================================
>====
>--- 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/S
>essionDelegate.java (original)
>+++ 
>jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/S
>essionDelegate.java Fri May  4 15:52:35 2012
>@@ -16,6 +16,19 @@
>  */
> package org.apache.jackrabbit.oak.jcr;
> 
>+import java.io.IOException;
>+
>+import javax.jcr.ItemExistsException;
>+import javax.jcr.NamespaceRegistry;
>+import javax.jcr.PathNotFoundException;
>+import javax.jcr.Repository;
>+import javax.jcr.RepositoryException;
>+import javax.jcr.Session;
>+import javax.jcr.Workspace;
>+import javax.jcr.lock.LockManager;
>+import javax.jcr.nodetype.NodeTypeManager;
>+import javax.jcr.version.VersionManager;
>+
> import org.apache.jackrabbit.oak.api.AuthInfo;
> import org.apache.jackrabbit.oak.api.CommitFailedException;
> import org.apache.jackrabbit.oak.api.ContentSession;
>@@ -32,16 +45,6 @@ import org.apache.jackrabbit.oak.namepat
> import org.slf4j.Logger;
> import org.slf4j.LoggerFactory;
> 
>-import javax.jcr.NamespaceRegistry;
>-import javax.jcr.Repository;
>-import javax.jcr.RepositoryException;
>-import javax.jcr.Session;
>-import javax.jcr.Workspace;
>-import javax.jcr.lock.LockManager;
>-import javax.jcr.nodetype.NodeTypeManager;
>-import javax.jcr.version.VersionManager;
>-import java.io.IOException;
>-
> public class SessionDelegate {
>     static final Logger log =
>LoggerFactory.getLogger(SessionDelegate.class);
> 
>@@ -158,6 +161,28 @@ public class SessionDelegate {
> 
>         String srcPath = PathUtils.relativize("/", srcAbsPath);  //
>TODO: is this needed?
>         String destPath = PathUtils.relativize("/", destAbsPath);
>+        
>+        // TODO: do the checks below belong here?
>+
>+        // check destination
>+        Tree dest = getTree(destPath);
>+        if (dest != null) {
>+            throw new ItemExistsException(destPath);
>+        }
>+        
>+        // check parent of destination
>+        String destParentPath = PathUtils.getParentPath(destPath);
>+        Tree destParent = getTree(destParentPath);
>+        if (destParent == null) {
>+            throw new PathNotFoundException(destParentPath);
>+        }
>+        
>+        // check source exists
>+        Tree src = getTree(srcPath);
>+        if (src == null) {
>+            throw new PathNotFoundException(srcPath);
>+        }
>+        
>         try {
>             if (transientOp) {
>                 root.move(srcPath, destPath);
>
>


Mime
View raw message