From 2e2a766edfb6abee66e549015554f73277c4fb6c Mon Sep 17 00:00:00 2001 From: Andrei Forascu Date: Thu, 17 Oct 2019 17:22:11 +0300 Subject: [PATCH] MNT-20714: /nodes/{nodeId}/content REST API fails for content created by a deleted user cherry-pick 3bf2a0ce73cf4b41a4fe4f0d57f5ab4e59b4aba7 from master --- .../org/alfresco/rest/api/impl/NodesImpl.java | 91 ++++++++----------- .../org/alfresco/rest/api/model/Node.java | 2 +- .../alfresco/rest/api/tests/NodeApiTest.java | 70 ++++++++++++++ 3 files changed, 108 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java b/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java index 3325102a8c..5987ddc91d 100644 --- a/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java +++ b/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java @@ -2449,26 +2449,21 @@ public class NodesImpl implements Nodes @Override public Node moveOrCopyNode(String sourceNodeId, String targetParentId, String name, Parameters parameters, boolean isCopy) { - FileInfo fi = retryingTransactionHelper.doInTransaction(() -> { - 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); - - return moveOrCopyImpl(sourceNodeRef, parentNodeRef, name, isCopy); - }, false, true); + if ((sourceNodeId == null) || (sourceNodeId.isEmpty())) + { + throw new InvalidArgumentException("Missing sourceNodeId"); + } - return retryingTransactionHelper.doInTransaction(() -> { - return getFolderOrDocument(fi.getNodeRef().getId(), parameters); - }, false, false); + if ((targetParentId == null) || (targetParentId.isEmpty())) + { + throw new InvalidArgumentException("Missing targetParentId"); + } + + final NodeRef parentNodeRef = validateOrLookupNode(targetParentId, null); + final NodeRef sourceNodeRef = validateOrLookupNode(sourceNodeId, null); + + FileInfo fi = moveOrCopyImpl(sourceNodeRef, parentNodeRef, name, isCopy); + return getFolderOrDocument(fi.getNodeRef().getId(), parameters); } public void updateCustomAspects(NodeRef nodeRef, List aspectNames, List excludedAspects) @@ -3271,28 +3266,22 @@ public class NodesImpl implements Nodes @Override public Node lock(String nodeId, LockInfo lockInfo, Parameters parameters) { - retryingTransactionHelper.doInTransaction(() -> { - NodeRef nodeRef = validateOrLookupNode(nodeId, null); + NodeRef nodeRef = validateOrLookupNode(nodeId, null); - if (isSpecialNode(nodeRef, getNodeType(nodeRef))) - { - throw new PermissionDeniedException("Current user doesn't have permission to lock node " + nodeId); - } + if (isSpecialNode(nodeRef, getNodeType(nodeRef))) + { + throw new PermissionDeniedException("Current user doesn't have permission to lock node " + nodeId); + } - if (!nodeMatches(nodeRef, Collections.singleton(ContentModel.TYPE_CONTENT), null, false)) - { - throw new InvalidArgumentException("Node of type cm:content or a subtype is expected: " + nodeId); - } + if (!nodeMatches(nodeRef, Collections.singleton(ContentModel.TYPE_CONTENT), null, false)) + { + throw new InvalidArgumentException("Node of type cm:content or a subtype is expected: " + nodeId); + } - LockInfo validatedLockInfo = validateLockInformation(lockInfo); - lockService.lock(nodeRef, validatedLockInfo.getMappedType(), validatedLockInfo.getTimeToExpire(), validatedLockInfo.getLifetime()); + LockInfo validatedLockInfo = validateLockInformation(lockInfo); + lockService.lock(nodeRef, validatedLockInfo.getMappedType(), validatedLockInfo.getTimeToExpire(), validatedLockInfo.getLifetime()); - return null; - }, false, true); - - return retryingTransactionHelper.doInTransaction(() -> { - return getFolderOrDocument(nodeId, parameters); - }, false, false); + return getFolderOrDocument(nodeId, parameters); } private LockInfo validateLockInformation(LockInfo lockInfo) @@ -3316,25 +3305,19 @@ public class NodesImpl implements Nodes @Override public Node unlock(String nodeId, Parameters parameters) { - retryingTransactionHelper.doInTransaction(() -> { - NodeRef nodeRef = validateOrLookupNode(nodeId, null); + NodeRef nodeRef = validateOrLookupNode(nodeId, null); - if (isSpecialNode(nodeRef, getNodeType(nodeRef))) - { - throw new PermissionDeniedException("Current user doesn't have permission to unlock node " + nodeId); - } - if (!lockService.isLocked(nodeRef)) - { - throw new IntegrityException("Can't unlock node " + nodeId + " because it isn't locked", null); - } + if (isSpecialNode(nodeRef, getNodeType(nodeRef))) + { + throw new PermissionDeniedException("Current user doesn't have permission to unlock node " + nodeId); + } + if (!lockService.isLocked(nodeRef)) + { + throw new IntegrityException("Can't unlock node " + nodeId + " because it isn't locked", null); + } - lockService.unlock(nodeRef); - return null; - }, false, true); - - return retryingTransactionHelper.doInTransaction(() -> { - return getFolderOrDocument(nodeId, parameters); - }, false, false); + lockService.unlock(nodeRef); + return getFolderOrDocument(nodeId, parameters); } /** diff --git a/src/main/java/org/alfresco/rest/api/model/Node.java b/src/main/java/org/alfresco/rest/api/model/Node.java index 2532c27755..055d70c951 100644 --- a/src/main/java/org/alfresco/rest/api/model/Node.java +++ b/src/main/java/org/alfresco/rest/api/model/Node.java @@ -163,7 +163,7 @@ public class Node implements Comparable PersonService.PersonInfo pInfo = null; try { - NodeRef pNodeRef = personService.getPerson(userName, false); + NodeRef pNodeRef = personService.getPersonOrNull(userName); if (pNodeRef != null) { pInfo = personService.getPerson(pNodeRef); diff --git a/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java b/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java index d79818d82e..f1b09045e6 100644 --- a/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java +++ b/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java @@ -4431,6 +4431,76 @@ public class NodeApiTest extends AbstractSingleNetworkSiteTest assertEquals(f1Id, documentResp.getParentId()); } + /** + * Tests update of content after owner of the document is deleted + *

PUT:

+ * {@literal :/alfresco/api/-default-/public/alfresco/versions/1/nodes//content} + */ + @Test + public void testUploadContentDeletedOwner() throws Exception + { + // Create person2delete + String personToDelete = createUser("usertodelete-" + RUNID, "userdelPassword", networkOne); + + // PersonToDelete creates a site and adds user1 as a site collab + setRequestContext(personToDelete); + String site1Title = "site-testUploadContentDeadUser_DocLib-" + RUNID; + String site1Id = createSite(site1Title, SiteVisibility.PUBLIC).getId(); + String site1DocLibNodeId = getSiteContainerNodeId(site1Id, "documentLibrary"); + + addSiteMember(site1Id, user1, SiteRole.SiteCollaborator); + + // PersonToDelete creates a file within DL + Document deadDoc = createTextFile(site1DocLibNodeId, "testdeaddoc.txt", "The quick brown fox jumps over the lazy dog 1."); + final String deadDocUrl = getNodeContentUrl(deadDoc.getId()); + + // PersonToDelete updates the file + String content = "Soft you a word or two before you go... I took by the throat the circumcised dog, And smote him, thus."; + String docName = "goodbye-world.txt"; + Map params_doc = new HashMap<>(); + params_doc.put(Nodes.PARAM_NAME, docName); + deadDoc = updateFileWithContent(deadDoc.getId(), content, params_doc, 200); + assertEquals("person2delete cannot update document", docName, deadDoc.getName()); + + // Download the file and confirm its contents on person2delete + HttpResponse response = getSingle(deadDocUrl, personToDelete, null, 200); + assertEquals("person2delete cannot view document", content, response.getResponse()); + + // Download the file and confirm its contents on user1 + response = getSingle(deadDocUrl, user1, null, 200); + assertEquals("user1 cannot view document", content, response.getResponse()); + + // PersonToDelete is deleted + transactionHelper.doInTransaction(() -> { + deleteUser(personToDelete, networkOne); + return null; + }); + + // User1 updates the file + setRequestContext(user1); + content = "This did I fear, but thought he had no weapon; For he was great of heart."; + updateFileWithContent(deadDoc.getId(), content, null, 200); + + // Download the file and confirm its contents (ensure rollback didn't happen) + response = getSingle(deadDocUrl, user1, null, 200); + assertEquals("user1 cannot update after owner is deleted", content, response.getResponse()); + } + + private Document updateFileWithContent(String docId, String content, Map params, int expectedStatus) throws Exception + { + ByteArrayInputStream inputStream = new ByteArrayInputStream(content.getBytes()); + File txtFile = TempFileProvider.createTempFile(inputStream, getClass().getSimpleName(), ".txt"); + + BinaryPayload payload = new BinaryPayload(txtFile); + + HttpResponse response = putBinary(getNodeContentUrl(docId), payload, null, params, expectedStatus); + if (expectedStatus != 200) + { + return null; + } + return RestApiUtil.parseRestApiEntry(response.getJsonResponse(), Document.class); + } + /** * Creates authority context *