You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2010/03/11 20:10:30 UTC

svn commit: r921991 - in /jackrabbit/trunk: jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/ jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/ jackrabbit-jcr2dav/ jackrabbit-spi2dav/ jackrabbit-spi2dav/src/main/jav...

Author: angela
Date: Thu Mar 11 19:10:30 2010
New Revision: 921991

URL: http://svn.apache.org/viewvc?rev=921991&view=rev
Log:
JCR-2536: spi2davex: InvalidItemStateException not properly extracted from ambiguous response error
JCR-2565: spi2dav: Overwrite header T specified for MOVE and COPY causes failure if some API tests [+ adding test for copy with snsiblings allowed]

Modified:
    jackrabbit/trunk/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/JCRWebdavServerServlet.java
    jackrabbit/trunk/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/WorkspaceCopySameNameSibsTest.java
    jackrabbit/trunk/jackrabbit-jcr2dav/pom.xml
    jackrabbit/trunk/jackrabbit-spi2dav/pom.xml
    jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/ExceptionConverter.java
    jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java
    jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java
    jackrabbit/trunk/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/server/AbstractWebdavServlet.java

Modified: jackrabbit/trunk/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/JCRWebdavServerServlet.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/JCRWebdavServerServlet.java?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/JCRWebdavServerServlet.java (original)
+++ jackrabbit/trunk/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/JCRWebdavServerServlet.java Thu Mar 11 19:10:30 2010
@@ -23,6 +23,7 @@ import org.apache.jackrabbit.webdav.DavE
 import org.apache.jackrabbit.webdav.DavLocatorFactory;
 import org.apache.jackrabbit.webdav.DavResource;
 import org.apache.jackrabbit.webdav.DavResourceFactory;
+import org.apache.jackrabbit.webdav.DavServletResponse;
 import org.apache.jackrabbit.webdav.DavSessionProvider;
 import org.apache.jackrabbit.webdav.WebdavRequest;
 import org.apache.jackrabbit.webdav.jcr.observation.SubscriptionManagerImpl;
@@ -244,6 +245,59 @@ public abstract class JCRWebdavServerSer
     }
 
     /**
+     * Modified variant needed for JCR move and copy that isn't compliant to
+     * WebDAV. The latter requires both methods to fail if the destination already
+     * exists and Overwrite is set to F (false); in JCR however this depends on
+     * the node type characteristics of the parent (SNSiblings allowed or not).
+     *
+     * @param destResource destination resource to be validated.
+     * @param request
+     * @param checkHeader flag indicating if the destination header must be present.
+     * @return status code indicating whether the destination is valid.
+     */
+    @Override
+    protected int validateDestination(DavResource destResource, WebdavRequest request, boolean checkHeader)
+            throws DavException {
+
+        if (checkHeader) {
+            String destHeader = request.getHeader(HEADER_DESTINATION);
+            if (destHeader == null || "".equals(destHeader)) {
+                return DavServletResponse.SC_BAD_REQUEST;
+            }
+        }
+        if (destResource.getLocator().equals(request.getRequestLocator())) {
+            return DavServletResponse.SC_FORBIDDEN;
+        }
+
+        int status;
+        if (destResource.exists()) {
+            if (request.isOverwrite()) {
+                // matching if-header required for existing resources
+                if (!request.matchesIfHeader(destResource)) {
+                    return DavServletResponse.SC_PRECONDITION_FAILED;
+                } else {
+                    // overwrite existing resource
+                    destResource.getCollection().removeMember(destResource);
+                    status = DavServletResponse.SC_NO_CONTENT;
+                }
+            } else {
+              /* NO overwrite header:
+
+                 but, instead of return the 412 Precondition-Failed code required
+                 by the WebDAV specification(s) leave the validation to the
+                 JCR repository.
+               */
+                status = DavServletResponse.SC_CREATED;
+            }
+
+        } else {
+            // destination does not exist >> copy/move can be performed
+            status = DavServletResponse.SC_CREATED;
+        }
+        return status;
+    }
+
+    /**
      * Returns the configured path prefix
      *
      * @return resourcePathPrefix

Modified: jackrabbit/trunk/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/WorkspaceCopySameNameSibsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/WorkspaceCopySameNameSibsTest.java?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/WorkspaceCopySameNameSibsTest.java (original)
+++ jackrabbit/trunk/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/WorkspaceCopySameNameSibsTest.java Thu Mar 11 19:10:30 2010
@@ -83,8 +83,7 @@ public class WorkspaceCopySameNameSibsTe
         String dstAbsPath = snsfNode.getPath() + "/" + node1.getName();
         workspace.copy(node1.getPath(), dstAbsPath);
 
-        // try to copy again the node to same destAbsPath
-        // property already exist
+        // try to copy again the node to same destAbsPath where node already exists
         try {
             workspace.copy(node1.getPath(), dstAbsPath);
             fail("Node exists below '" + dstAbsPath + "'. Test should fail.");
@@ -92,4 +91,38 @@ public class WorkspaceCopySameNameSibsTe
             // successful
         }
     }
+
+    /**
+     * NO ItemExistsException is thrown if a node already exists at destAbsPath
+     * and the node allows same-name-siblings.
+     *
+     * @tck.config sameNameSibsTrueNodeType name of a node type that
+     * allows same name siblings.
+     * @tck.config nodeName3 name of a child node that allows children with
+     * same name.
+     */
+    public void testCopyNodesNodeExistsAtDestPath2() throws RepositoryException {
+        // create a parent node where allowSameNameSiblings are set to true
+        Node snsfNode = testRootNode.addNode(nodeName3, sameNameSibsTrueNodeType.getName());
+        testRootNode.save();
+
+        String dstAbsPath = snsfNode.getPath() + "/" + node1.getName();
+        workspace.copy(node1.getPath(), dstAbsPath);
+
+        // try to copy again the node to same destAbsPath where node already exists
+        // must succeed
+        workspace.copy(node1.getPath(), dstAbsPath);
+
+        // make sure the parent now has 2 children with the same name
+        NodeIterator it = snsfNode.getNodes(node1.getName());
+        long size = it.getSize();
+        if (it.getSize() == -1) {
+            size = 0;
+            while (it.hasNext()) {
+                it.nextNode();
+                size++;
+            }
+        }
+        assertEquals("After second copy 2 same-name-siblings must exist",2, size);
+    }
 }

Modified: jackrabbit/trunk/jackrabbit-jcr2dav/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2dav/pom.xml?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2dav/pom.xml (original)
+++ jackrabbit/trunk/jackrabbit-jcr2dav/pom.xml Thu Mar 11 19:10:30 2010
@@ -51,15 +51,8 @@
                   <name>known.issues</name>
                   <value>
                       <!-- ***** PLEASE update spi2dav/pom.xml as well ***** -->                      
-                      <!-- JCR-2536 : PathNotFoundEx instead of InvalidItemStateEx. -->
-                      org.apache.jackrabbit.test.api.NodeTest#testSaveInvalidStateException
-                      org.apache.jackrabbit.test.api.NodeTest#testRemoveInvalidItemStateException
-                      org.apache.jackrabbit.test.api.SessionTest#testSaveInvalidStateException
                       <!-- wrong exception: ConstraintViolationEx. instead SAXException/InvalidSerializedDataEx.-->
                       org.apache.jackrabbit.test.api.SerializationTest#testNodeTypeConstraintViolationWorkspace
-                      <!-- not yet debugged : unknown reason -->
-                      org.apache.jackrabbit.test.api.WorkspaceCopySameNameSibsTest#testCopyNodesNodeExistsAtDestPath
-                      org.apache.jackrabbit.test.api.WorkspaceMoveSameNameSibsTest#testMoveNodesNodeExistsAtDestPath
                       <!-- JCR-2538 : impersonation not implemented -->
                       org.apache.jackrabbit.test.api.ImpersonateTest
                       <!-- JCR-2099 : shareable nodes -->

Modified: jackrabbit/trunk/jackrabbit-spi2dav/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/pom.xml?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/pom.xml (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/pom.xml Thu Mar 11 19:10:30 2010
@@ -49,15 +49,8 @@
               <name>known.issues</name>
               <value>
                   <!-- ***** PLEASE update jcr2dav/pom.xml as well ********* -->
-                  <!-- JCR-2536 : PathNotFoundEx instead of InvalidItemStateEx. -->
-                  org.apache.jackrabbit.test.api.NodeTest#testSaveInvalidStateException
-                  org.apache.jackrabbit.test.api.NodeTest#testRemoveInvalidItemStateException
-                  org.apache.jackrabbit.test.api.SessionTest#testSaveInvalidStateException
                   <!-- wrong exception: ConstraintViolationEx. instead SAXException/InvalidSerializedDataEx.-->
                   org.apache.jackrabbit.test.api.SerializationTest#testNodeTypeConstraintViolationWorkspace
-                  <!-- not yet debugged : unknown reason -->
-                  org.apache.jackrabbit.test.api.WorkspaceCopySameNameSibsTest#testCopyNodesNodeExistsAtDestPath
-                  org.apache.jackrabbit.test.api.WorkspaceMoveSameNameSibsTest#testMoveNodesNodeExistsAtDestPath
                   <!-- JCR-2538 : impersonation not implemented -->
                   org.apache.jackrabbit.test.api.ImpersonateTest
                   <!-- JCR-2099 : shareable nodes -->

Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/ExceptionConverter.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/ExceptionConverter.java?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/ExceptionConverter.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/ExceptionConverter.java Thu Mar 11 19:10:30 2010
@@ -16,22 +16,21 @@
  */
 package org.apache.jackrabbit.spi2dav;
 
+import org.apache.jackrabbit.webdav.DavConstants;
 import org.apache.jackrabbit.webdav.DavException;
+import org.apache.jackrabbit.webdav.DavMethods;
 import org.apache.jackrabbit.webdav.DavServletResponse;
-import org.apache.jackrabbit.webdav.DavConstants;
 import org.apache.jackrabbit.webdav.client.methods.DavMethod;
-import org.apache.jackrabbit.webdav.client.methods.DeleteMethod;
-import org.apache.jackrabbit.webdav.client.methods.MkColMethod;
 import org.apache.jackrabbit.webdav.xml.DomUtil;
-import org.apache.commons.httpclient.methods.PutMethod;
 import org.w3c.dom.Element;
 
-import javax.jcr.RepositoryException;
-import javax.jcr.ItemNotFoundException;
 import javax.jcr.InvalidItemStateException;
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.PathNotFoundException;
+import javax.jcr.RepositoryException;
 import javax.jcr.UnsupportedRepositoryOperationException;
-import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.lock.LockException;
+import javax.jcr.nodetype.ConstraintViolationException;
 import java.lang.reflect.Constructor;
 
 /**
@@ -47,6 +46,12 @@ public class ExceptionConverter {
     }
 
     public static RepositoryException generate(DavException davExc, DavMethod method) {
+        String name = (method == null) ? "_undefined_" : method.getName();
+        int code = DavMethods.getMethodCode(name);
+        return generate(davExc, code, name);
+    }
+
+    public static RepositoryException generate(DavException davExc, int methodCode, String name) {
         String msg = davExc.getMessage();
         if (davExc.hasErrorCondition()) {
             try {
@@ -62,7 +67,10 @@ public class ExceptionConverter {
                             Constructor<?> excConstr = cl.getConstructor(String.class);
                             if (excConstr != null) {
                                 Object o = excConstr.newInstance(msg);
-                                if (o instanceof RepositoryException) {
+                                if (o instanceof PathNotFoundException && methodCode == DavMethods.DAV_POST) {
+                                    // see JCR-2536
+                                    return new InvalidItemStateException(msg);
+                                } else if (o instanceof RepositoryException) {
                                     return (RepositoryException) o;
                                 } else if (o instanceof Exception) {
                                     return new RepositoryException(msg, (Exception)o);
@@ -80,14 +88,16 @@ public class ExceptionConverter {
         switch (davExc.getErrorCode()) {
             // TODO: mapping DAV_error to jcr-exception is ambiguous. to be improved
             case DavServletResponse.SC_NOT_FOUND :
-                if (method != null && (method instanceof DeleteMethod ||
-                                       method instanceof MkColMethod ||
-                                       method instanceof PutMethod)) {
-                    // target item has probably while transient changes have
-                    // been made.
-                    return new InvalidItemStateException(msg, davExc);
-                } else {
-                    return new ItemNotFoundException(msg, davExc);
+                switch (methodCode) {
+                    case DavMethods.DAV_DELETE:
+                    case DavMethods.DAV_MKCOL:
+                    case DavMethods.DAV_PUT:
+                    case DavMethods.DAV_POST:
+                        // target item has probably while transient changes have
+                        // been made.
+                        return new InvalidItemStateException(msg, davExc);
+                    default:
+                        return new ItemNotFoundException(msg, davExc);
                 }
             case DavServletResponse.SC_LOCKED :
                 return new LockException(msg, davExc);
@@ -98,10 +108,10 @@ public class ExceptionConverter {
             case DavServletResponse.SC_PRECONDITION_FAILED :
                 return new LockException(msg, davExc);
             case DavServletResponse.SC_NOT_IMPLEMENTED:
-                if (method != null) {
+                if (methodCode > 0 && name != null) {
                     return new UnsupportedRepositoryOperationException(
                             "Missing implementation: Method "
-                            + method + " could not be executed", davExc);
+                            + name + " could not be executed", davExc);
                 } else {
                     return new UnsupportedRepositoryOperationException(
                             "Missing implementation", davExc);

Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java Thu Mar 11 19:10:30 2010
@@ -482,7 +482,7 @@ public class RepositoryServiceImpl imple
         } catch (IOException e) {
             throw new RepositoryException(e);
         } catch (DavException e) {
-            throw ExceptionConverter.generate(e);
+            throw ExceptionConverter.generate(e, method);
         } finally {
             if (method != null) {
                 method.releaseConnection();
@@ -1297,7 +1297,7 @@ public class RepositoryServiceImpl imple
     public void move(SessionInfo sessionInfo, NodeId srcNodeId, NodeId destParentNodeId, Name destName) throws RepositoryException {
         String uri = getItemUri(srcNodeId, sessionInfo);
         String destUri = getItemUri(destParentNodeId, destName, sessionInfo);
-        MoveMethod method = new MoveMethod(uri, destUri, true);
+        MoveMethod method = new MoveMethod(uri, destUri, false);
         execute(method, sessionInfo);
         // need to clear the cache as the move may have affected nodes with uuid.
         clearItemUriCache(sessionInfo);
@@ -1309,7 +1309,7 @@ public class RepositoryServiceImpl imple
     public void copy(SessionInfo sessionInfo, String srcWorkspaceName, NodeId srcNodeId, NodeId destParentNodeId, Name destName) throws RepositoryException {
         String uri = uriResolver.getItemUri(srcNodeId, srcWorkspaceName, sessionInfo);
         String destUri = getItemUri(destParentNodeId, destName, sessionInfo);
-        CopyMethod method = new CopyMethod(uri, destUri, true, false);
+        CopyMethod method = new CopyMethod(uri, destUri, false, false);
         execute(method, sessionInfo);
     }
 
@@ -2659,7 +2659,7 @@ public class RepositoryServiceImpl imple
             checkConsumed();
             String uri = getItemUri(srcNodeId, sessionInfo);
             String destUri = getItemUri(destParentNodeId, destName, sessionInfo);
-            MoveMethod method = new MoveMethod(uri, destUri, true);
+            MoveMethod method = new MoveMethod(uri, destUri, false);
 
             methods.add(method);
             clear = true;

Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java Thu Mar 11 19:10:30 2010
@@ -350,7 +350,7 @@ public class RepositoryServiceImpl exten
         } catch (IOException e) {
             throw new RepositoryException(e);
         } catch (DavException e) {
-            throw ExceptionConverter.generate(e);
+            throw ExceptionConverter.generate(e, method);
         } finally {
             if (method != null) {
                 method.releaseConnection();
@@ -389,7 +389,7 @@ public class RepositoryServiceImpl exten
         } catch (IOException e) {
             throw new RepositoryException(e);
         } catch (DavException e) {
-            throw ExceptionConverter.generate(e);
+            throw ExceptionConverter.generate(e, method);
         } finally {
             if (method != null) {
                 method.releaseConnection();
@@ -472,7 +472,7 @@ public class RepositoryServiceImpl exten
             }  catch (IOException e) {
                 throw new RepositoryException(e);
             } catch (DavException e) {
-                throw ExceptionConverter.generate(e);
+                throw ExceptionConverter.generate(e, method);
             } finally {
                 method.releaseConnection();
             }

Modified: jackrabbit/trunk/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/server/AbstractWebdavServlet.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/server/AbstractWebdavServlet.java?rev=921991&r1=921990&r2=921991&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/server/AbstractWebdavServlet.java (original)
+++ jackrabbit/trunk/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/server/AbstractWebdavServlet.java Thu Mar 11 19:10:30 2010
@@ -709,9 +709,10 @@ abstract public class AbstractWebdavServ
      *
      * @param destResource destination resource to be validated.
      * @param request
+     * @param checkHeader flag indicating if the destination header must be present.
      * @return status code indicating whether the destination is valid.
      */
-    private int validateDestination(DavResource destResource, WebdavRequest request, boolean checkHeader)
+    protected int validateDestination(DavResource destResource, WebdavRequest request, boolean checkHeader)
             throws DavException {
 
         if (checkHeader) {