Merged HEAD (5.2) to 5.2.N (5.2.1)

126450 jkaabimofrad: Merged FILE-FOLDER-API (5.2.0) to HEAD (5.2)
      122127 jvonka: Nodes (FileFolder) API - update/add -ve tests for move & copy operations
      - also fix-up error messages to show nodeId (rather than full nodeRef)
      RA-684, RA-806


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@126795 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/rest/api/Nodes.java

@@ -117,7 +117,7 @@ public interface Nodes
     Node createNode(String parentFolderNodeId, Node nodeInfo, Parameters parameters);
 
     /**
-     * Move node
+     * Move or Copy node
      *
      * @param sourceNodeId
      * @param parentFolderNodeId
@@ -125,18 +125,7 @@ public interface Nodes
      * @param parameters
      * @return
      */
-    Node moveNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters);
+    Node moveOrCopyNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters, boolean isCopy);
-
-    /**
-     * Copy node
-     *
-     * @param sourceNodeId
-     * @param parentFolderNodeId
-     * @param name
-     * @param parameters
-     * @return
-     */
-    Node copyNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters);
 
     /**
      * Update node meta-data.

source/java/org/alfresco/rest/api/impl/NodesImpl.java

@@ -487,7 +487,7 @@ public class NodesImpl implements Nodes
         }
         else
         {
-            throw new InvalidArgumentException("Node is not a file");
+            throw new InvalidArgumentException("Node is not a file: "+nodeRef.getId());
         }
     }
 
@@ -534,6 +534,11 @@ public class NodesImpl implements Nodes
     {
         NodeRef parentNodeRef;
 
+        if ((nodeId == null) || (nodeId.isEmpty()))
+        {
+            throw new InvalidArgumentException("Missing nodeId");
+        }
+
         if (nodeId.equals(PATH_ROOT))
         {
             parentNodeRef = repositoryHelper.getCompanyHome();
@@ -629,7 +634,7 @@ public class NodesImpl implements Nodes
         catch (FileNotFoundException fnfe)
         {
             // convert checked exception
-            throw new EntityNotFoundException(fnfe.getMessage()+" ["+parentNodeRef.getId()+","+path+"]");
+            throw new EntityNotFoundException(parentNodeRef.getId());
         }
 
         return fileInfo.getNodeRef();
@@ -1109,7 +1114,7 @@ public class NodesImpl implements Nodes
     {
         if (nodeInfo.getNodeRef() != null)
         {
-            throw new InvalidArgumentException("Unexpected id when trying to create a new node: "+nodeInfo.getNodeRef());
+            throw new InvalidArgumentException("Unexpected id when trying to create a new node: "+nodeInfo.getNodeRef().getId());
         }
 
         // check that requested parent node exists and it's type is a (sub-)type of folder
@@ -1388,26 +1393,29 @@ public class NodesImpl implements Nodes
         return getFolderOrDocument(nodeRef.getId(), parameters);
     }
 
-    public Node moveNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters)
+    public Node moveOrCopyNode(String sourceNodeId, String targetParentId, String name, Parameters parameters, boolean isCopy)
     {
-        final NodeRef parentNodeRef = validateOrLookupNode(parentFolderNodeId, null);
+        if ((sourceNodeId == null) || (sourceNodeId.isEmpty()))
+        {
+            throw new InvalidArgumentException("Missing sourceNodeId");
+        }
+
+        if ((targetParentId == null) || (targetParentId.isEmpty()))
+        {
+            throw new InvalidArgumentException("Missing targetParentId");
+        }
+
+        final NodeRef parentNodeRef = validateOrLookupNode(targetParentId, null);
         final NodeRef sourceNodeRef = validateOrLookupNode(sourceNodeId, null);
 
-        FileInfo fi = moveOrCopy(sourceNodeRef, parentNodeRef, name, false);
+        FileInfo fi = moveOrCopyImpl(sourceNodeRef, parentNodeRef, name, isCopy);
         return getFolderOrDocument(fi.getNodeRef().getId(), parameters);
     }
 
-    public Node copyNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters)
+    protected FileInfo moveOrCopyImpl(NodeRef nodeRef, NodeRef parentNodeRef, String name, boolean isCopy)
     {
-        final NodeRef parentNodeRef = validateOrLookupNode(parentFolderNodeId, null);
+        String targetParentId = parentNodeRef.getId();
-        final NodeRef sourceNodeRef = validateOrLookupNode(sourceNodeId, null);
 
-        FileInfo fi = moveOrCopy(sourceNodeRef, parentNodeRef, name, true);
-        return getFolderOrDocument(fi.getNodeRef().getId(), parameters);
-    }
-
-    protected FileInfo moveOrCopy(NodeRef nodeRef, NodeRef parentNodeRef, String name, boolean isCopy)
-    {
         try
         {
             if (isCopy)
@@ -1424,25 +1432,25 @@ public class NodesImpl implements Nodes
         }
         catch (InvalidNodeRefException inre)
         {
-            throw new EntityNotFoundException(inre.getMessage());
+            throw new EntityNotFoundException(targetParentId);
         }
         catch (FileNotFoundException fnfe)
         {
             // convert checked exception
-            throw new EntityNotFoundException(fnfe.getMessage());
+            throw new EntityNotFoundException(targetParentId);
         }
         catch (FileExistsException fee)
         {
             // duplicate - name clash
-            throw new ConstraintViolatedException(fee.getMessage());
+            throw new ConstraintViolatedException("Name already exists in target parent: "+name);
         }
         catch (FileFolderServiceImpl.InvalidTypeException ite)
         {
-            throw new InvalidArgumentException(ite.getMessage());
+            throw new InvalidArgumentException("Invalid type of target parent: "+targetParentId);
         }
         catch (CyclicChildRelationshipException ccre)
         {
-            throw new InvalidArgumentException(ccre.getMessage());
+            throw new InvalidArgumentException("Parent/child cycle detected: "+targetParentId);
         }
     }
 

source/java/org/alfresco/rest/api/model/NodeTarget.java

@@ -1,9 +1,26 @@
+/*
+ * Copyright (C) 2005-2015 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
 package org.alfresco.rest.api.model;
 
-import org.alfresco.service.cmr.repository.NodeRef;
-
 /**
  * A target object.
+ * 
  * @author Gethin James
  */
 public class NodeTarget extends Target

source/java/org/alfresco/rest/api/nodes/NodesEntityResource.java

@@ -134,14 +134,14 @@ public class NodesEntityResource implements
     @WebApiDescription(title = "Copy Node", description="Copy one or more nodes (files or folders) to a new target folder, with option to rename.")
     public Node copyById(String nodeId, NodeTarget target, Parameters parameters)
     {
-       return nodes.copyNode(nodeId, target.getTargetParentId(), target.getName(), parameters);
+       return nodes.moveOrCopyNode(nodeId, target.getTargetParentId(), target.getName(), parameters, true);
     }
 
     @Operation("move")
     @WebApiDescription(title = "Move Node", description="Moves one or more nodes (files or folders) to a new target folder, with option to rename.")
     public Node moveById(String nodeId, NodeTarget target, Parameters parameters)
     {
-        return nodes.moveNode(nodeId, target.getTargetParentId(), target.getName(), parameters);
+        return nodes.moveOrCopyNode(nodeId, target.getTargetParentId(), target.getName(), parameters, false);
     }
 
 }

source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java

@@ -1107,10 +1107,10 @@ public class NodeApiTest extends AbstractBaseApiTest
 
         // move file (without rename)
 
-        NodeTarget moveTgt = new NodeTarget();
+        NodeTarget tgt = new NodeTarget();
-        moveTgt.setTargetParentId(f2Id);
+        tgt.setTargetParentId(f2Id);
 
-        HttpResponse response = post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 201);
+        HttpResponse response = post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 201);
         Document documentResp = RestApiUtil.parseRestApiEntry(response.getJsonResponse(), Document.class);
 
         assertEquals(d1Name, documentResp.getName());
@@ -1120,11 +1120,11 @@ public class NodeApiTest extends AbstractBaseApiTest
 
         String d1NewName = d1Name+" updated !!";
 
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setName(d1NewName);
+        tgt.setName(d1NewName);
-        moveTgt.setTargetParentId(f1Id);
+        tgt.setTargetParentId(f1Id);
 
-        response = post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 201);
+        response = post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 201);
         documentResp = RestApiUtil.parseRestApiEntry(response.getJsonResponse(), Document.class);
 
         assertEquals(d1NewName, documentResp.getName());
@@ -1132,26 +1132,31 @@ public class NodeApiTest extends AbstractBaseApiTest
 
         // -ve tests
 
+        // missing target
+        tgt = new NodeTarget();
+        tgt.setName("new name");
+        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 400);
+
         // name already exists
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setName(d2Name);
+        tgt.setName(d2Name);
-        moveTgt.setTargetParentId(f2Id);
+        tgt.setTargetParentId(f2Id);
-        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 409);
+        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 409);
 
         // unknown source nodeId
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setTargetParentId(f2Id);
+        tgt.setTargetParentId(f2Id);
-        post("nodes/"+UUID.randomUUID().toString()+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 404);
+        post("nodes/"+UUID.randomUUID().toString()+"/move", user1, toJsonAsStringNonNull(tgt), null, 404);
 
         // unknown target nodeId
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setTargetParentId(UUID.randomUUID().toString());
+        tgt.setTargetParentId(UUID.randomUUID().toString());
-        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 404);
+        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 404);
 
         // target is not a folder
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setTargetParentId(d2Id);
+        tgt.setTargetParentId(d2Id);
-        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 400);
+        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 400);
 
         String rootNodeId = getRootNodeId(user1);
 
@@ -1160,26 +1165,25 @@ public class NodeApiTest extends AbstractBaseApiTest
         String f3Id = folderResp.getId();
 
         // can't create cycle (move into own subtree)
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setTargetParentId(f3Id);
+        tgt.setTargetParentId(f3Id);
-        post("nodes/"+f2Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 400);
+        post("nodes/"+f2Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 400);
 
         // no (write/create) permissions to move to target
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setTargetParentId(rootNodeId);
+        tgt.setTargetParentId(rootNodeId);
-        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 403);
+        post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 403);
 
         AuthenticationUtil.setFullyAuthenticatedUser(user2);
 
         String my2NodeId = getMyNodeId(user2);
 
         // no (write/delete) permissions to move source
-        moveTgt = new NodeTarget();
+        tgt = new NodeTarget();
-        moveTgt.setTargetParentId(my2NodeId);
+        tgt.setTargetParentId(my2NodeId);
-        post("nodes/"+f1Id+"/move", user2, toJsonAsStringNonNull(moveTgt), null, 403);
+        post("nodes/"+f1Id+"/move", user2, toJsonAsStringNonNull(tgt), null, 403);
     }
 
-
     /**
      * Tests copy (file or folder)
      * <p>POST:</p>
@@ -1231,6 +1235,41 @@ public class NodeApiTest extends AbstractBaseApiTest
 
         assertEquals(newD2Name, documentResp.getName());
         assertEquals(target, documentResp.getParentId());
+
+        // -ve tests
+
+        // missing target
+        NodeTarget tgt = new NodeTarget();
+        tgt.setName("new name");
+        post("nodes/"+d1Id+"/copy", user1, toJsonAsStringNonNull(tgt), null, 400);
+
+        // name already exists
+        tgt = new NodeTarget();
+        tgt.setName(newD2Name);
+        tgt.setTargetParentId(target);
+        post("nodes/"+d1Id+"/copy", user1, toJsonAsStringNonNull(tgt), null, 409);
+
+        // unknown source nodeId
+        tgt = new NodeTarget();
+        tgt.setTargetParentId(target);
+        post("nodes/"+UUID.randomUUID().toString()+"/copy", user1, toJsonAsStringNonNull(tgt), null, 404);
+
+        // unknown target nodeId
+        tgt = new NodeTarget();
+        tgt.setTargetParentId(UUID.randomUUID().toString());
+        post("nodes/"+d1Id+"/copy", user1, toJsonAsStringNonNull(tgt), null, 404);
+
+        // target is not a folder
+        tgt = new NodeTarget();
+        tgt.setTargetParentId(d2Id);
+        post("nodes/"+d1Id+"/copy", user1, toJsonAsStringNonNull(tgt), null, 400);
+
+        String rootNodeId = getRootNodeId(user1);
+
+        // no (write/create) permissions to copy to target
+        tgt = new NodeTarget();
+        tgt.setTargetParentId(rootNodeId);
+        post("nodes/"+d1Id+"/copy", user1, toJsonAsStringNonNull(tgt), null, 403);
     }
     /**
      * Tests create folder.