diff --git a/source/java/org/alfresco/rest/api/Nodes.java b/source/java/org/alfresco/rest/api/Nodes.java
index 10ca0acb53..fdf66fc345 100644
--- a/source/java/org/alfresco/rest/api/Nodes.java
+++ b/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);
-
- /**
- * Copy node
- *
- * @param sourceNodeId
- * @param parentFolderNodeId
- * @param name
- * @param parameters
- * @return
- */
- Node copyNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters);
+ Node moveOrCopyNode(String sourceNodeId, String parentFolderNodeId, String name, Parameters parameters, boolean isCopy);
/**
* Update node meta-data.
diff --git a/source/java/org/alfresco/rest/api/impl/NodesImpl.java b/source/java/org/alfresco/rest/api/impl/NodesImpl.java
index 88b7fd5041..d6266e78a1 100644
--- a/source/java/org/alfresco/rest/api/impl/NodesImpl.java
+++ b/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);
- final NodeRef sourceNodeRef = validateOrLookupNode(sourceNodeId, null);
+ String targetParentId = parentNodeRef.getId();
- 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);
}
}
diff --git a/source/java/org/alfresco/rest/api/model/NodeTarget.java b/source/java/org/alfresco/rest/api/model/NodeTarget.java
index afe2c2cca9..a00dc4df94 100644
--- a/source/java/org/alfresco/rest/api/model/NodeTarget.java
+++ b/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 .
+ */
package org.alfresco.rest.api.model;
-import org.alfresco.service.cmr.repository.NodeRef;
-
/**
* A target object.
+ *
* @author Gethin James
*/
public class NodeTarget extends Target
diff --git a/source/java/org/alfresco/rest/api/nodes/NodesEntityResource.java b/source/java/org/alfresco/rest/api/nodes/NodesEntityResource.java
index 58ea4479ca..45860fd190 100644
--- a/source/java/org/alfresco/rest/api/nodes/NodesEntityResource.java
+++ b/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);
}
}
diff --git a/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java b/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java
index 6638da75a9..fd126d5374 100644
--- a/source/test-java/org/alfresco/rest/api/tests/NodeApiTest.java
+++ b/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();
- moveTgt.setTargetParentId(f2Id);
+ NodeTarget tgt = new NodeTarget();
+ 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();
- moveTgt.setName(d1NewName);
- moveTgt.setTargetParentId(f1Id);
+ tgt = new NodeTarget();
+ tgt.setName(d1NewName);
+ 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();
- moveTgt.setName(d2Name);
- moveTgt.setTargetParentId(f2Id);
- post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 409);
+ tgt = new NodeTarget();
+ tgt.setName(d2Name);
+ tgt.setTargetParentId(f2Id);
+ post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 409);
// unknown source nodeId
- moveTgt = new NodeTarget();
- moveTgt.setTargetParentId(f2Id);
- post("nodes/"+UUID.randomUUID().toString()+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 404);
+ tgt = new NodeTarget();
+ tgt.setTargetParentId(f2Id);
+ post("nodes/"+UUID.randomUUID().toString()+"/move", user1, toJsonAsStringNonNull(tgt), null, 404);
// unknown target nodeId
- moveTgt = new NodeTarget();
- moveTgt.setTargetParentId(UUID.randomUUID().toString());
- post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 404);
+ tgt = new NodeTarget();
+ tgt.setTargetParentId(UUID.randomUUID().toString());
+ post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 404);
// target is not a folder
- moveTgt = new NodeTarget();
- moveTgt.setTargetParentId(d2Id);
- post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 400);
+ tgt = new NodeTarget();
+ tgt.setTargetParentId(d2Id);
+ 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();
- moveTgt.setTargetParentId(f3Id);
- post("nodes/"+f2Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 400);
+ tgt = new NodeTarget();
+ tgt.setTargetParentId(f3Id);
+ post("nodes/"+f2Id+"/move", user1, toJsonAsStringNonNull(tgt), null, 400);
// no (write/create) permissions to move to target
- moveTgt = new NodeTarget();
- moveTgt.setTargetParentId(rootNodeId);
- post("nodes/"+d1Id+"/move", user1, toJsonAsStringNonNull(moveTgt), null, 403);
+ tgt = new NodeTarget();
+ tgt.setTargetParentId(rootNodeId);
+ 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();
- moveTgt.setTargetParentId(my2NodeId);
- post("nodes/"+f1Id+"/move", user2, toJsonAsStringNonNull(moveTgt), null, 403);
+ tgt = new NodeTarget();
+ tgt.setTargetParentId(my2NodeId);
+ post("nodes/"+f1Id+"/move", user2, toJsonAsStringNonNull(tgt), null, 403);
}
-
/**
* Tests copy (file or folder)
* POST:
@@ -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.