From 307b92ee30760da0634f09b7922c7a001cd23487 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Wed, 6 Feb 2019 17:08:51 +0200 Subject: [PATCH 01/14] tests to cover cases when deleting records or classified files when the files have copies --- .../community/records/DeleteRecordTests.java | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java index e9d7c60493..6a64805db5 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java @@ -35,8 +35,8 @@ import static org.alfresco.rest.rm.community.model.user.UserPermissions.PERMISSI import static org.alfresco.rest.rm.community.model.user.UserRoles.ROLE_RM_POWER_USER; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.IMAGE_FILE; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createElectronicRecordModel; -import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createNonElectronicRecordModel; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createElectronicUnfiledContainerChildModel; +import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createNonElectronicRecordModel; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createNonElectronicUnfiledContainerChildModel; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.getFile; import static org.alfresco.utility.constants.UserRole.SiteCollaborator; @@ -46,6 +46,8 @@ import static org.springframework.http.HttpStatus.NOT_FOUND; import static org.springframework.http.HttpStatus.NO_CONTENT; import static org.springframework.http.HttpStatus.OK; +import org.alfresco.dataprep.CMISUtil; +import org.alfresco.rest.core.JsonBodyGenerator; import org.alfresco.rest.core.RestResponse; import org.alfresco.rest.model.RestNodeBodyMoveCopyModel; import org.alfresco.rest.model.RestNodeModel; @@ -59,6 +61,7 @@ import org.alfresco.rest.rm.community.requests.gscore.api.RecordFolderAPI; import org.alfresco.rest.rm.community.requests.gscore.api.RecordsAPI; import org.alfresco.test.AlfrescoTest; import org.alfresco.utility.data.RandomData; +import org.alfresco.utility.model.FileModel; import org.alfresco.utility.model.RepoTestModel; import org.alfresco.utility.model.SiteModel; import org.alfresco.utility.model.UserModel; @@ -285,6 +288,43 @@ public class DeleteRecordTests extends BaseRMRestTest getNodeContent(recordId); } + + /** + *
+     * Given a file that has  copy
+     * And the original file is declared as record
+     * When I delete the original
+     * Then it is still possible to view the content of the copy
+     * 
+ */ + @Test (description = "Deleting record doesn't delete the copied filed") + @AlfrescoTest (jira = "MNT-20145") + public void deleteOriginOfRecord() throws Exception + { + Step.STEP("Create a file."); + testSite = dataSite.usingAdmin().createPublicRandomSite(); + FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN); + + Step.STEP("Create a file copy."); + String postBody = JsonBodyGenerator.keyValueJson("targetParentId", testSite.getGuid()); + RestNodeModel copyOfTestFile = getRestAPIFactory().getNodeAPI(testFile).copyNode(postBody); + + Step.STEP("Declare original file as record"); + getRestAPIFactory().getFilesAPI().declareAsRecord(testFile.getNodeRefWithoutVersion()); + assertStatusCode(CREATED); + + Step.STEP("Delete the record."); + getRestAPIFactory().getRecordsAPI().deleteRecord(testFile.getNodeRefWithoutVersion()); + assertStatusCode(NO_CONTENT); + + Step.STEP("Check that it's possible to load the copy content."); + getNodeContent(copyOfTestFile.getId()); + assertStatusCode(OK); + + Step.STEP("Clean up."); + dataSite.deleteSite(testSite); + } + /** * Utility method to delete a record and verify successful deletion * @@ -299,7 +339,7 @@ public class DeleteRecordTests extends BaseRMRestTest assertStatusCode(NO_CONTENT); // Try to get deleted record - recordsAPI.deleteRecord(recordId); + recordsAPI.getRecord(recordId); assertStatusCode(NOT_FOUND); } From 59c6909d2dbea73b4a79d061670fc08248628d92 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Thu, 7 Feb 2019 09:36:18 +0200 Subject: [PATCH 02/14] java docs updates --- .../alfresco/rest/rm/community/records/DeleteRecordTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java index 6a64805db5..79e30ea0d4 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java @@ -297,7 +297,7 @@ public class DeleteRecordTests extends BaseRMRestTest * Then it is still possible to view the content of the copy * */ - @Test (description = "Deleting record doesn't delete the copied filed") + @Test (description = "Deleting record doesn't delete the content for the copies") @AlfrescoTest (jira = "MNT-20145") public void deleteOriginOfRecord() throws Exception { @@ -305,7 +305,7 @@ public class DeleteRecordTests extends BaseRMRestTest testSite = dataSite.usingAdmin().createPublicRandomSite(); FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN); - Step.STEP("Create a file copy."); + Step.STEP("Create a copy for the file created."); String postBody = JsonBodyGenerator.keyValueJson("targetParentId", testSite.getGuid()); RestNodeModel copyOfTestFile = getRestAPIFactory().getNodeAPI(testFile).copyNode(postBody); From 87b56030bd6b5e7caa6e43bfa4835cb4eaf4f26f Mon Sep 17 00:00:00 2001 From: cagache Date: Thu, 7 Feb 2019 17:54:46 +0200 Subject: [PATCH 03/14] MNT-20145 Don't delete the content url if the file has copies or it is a copy --- .../content/ContentDestructionComponent.java | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java index fb77f3efb6..5bdfd1dc33 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java @@ -33,6 +33,7 @@ import java.util.Set; import org.alfresco.model.ContentModel; import org.alfresco.model.RenditionModel; +import org.alfresco.module.org_alfresco_module_rm.version.RecordableVersionModel; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.service.cmr.dictionary.DictionaryService; @@ -214,16 +215,22 @@ public class ContentDestructionComponent // get content data ContentData dataContent = (ContentData)entry.getValue(); - // if enabled cleanse content - if (isCleansingEnabled()) - { - // register for cleanse then immediate destruction - getEagerContentStoreCleaner().registerOrphanedContentUrlForCleansing(dataContent.getContentUrl()); - } - else - { - // register for immediate destruction - getEagerContentStoreCleaner().registerOrphanedContentUrl(dataContent.getContentUrl(), true); + // destroy the nodes content properties only if it doesn't have copies or it is a copy + // destroy the nodes content properties only if the versionedNodeRef is deleted + if (getNodeService().getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() && + getNodeService().getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() && + getNodeService().getProperty(nodeRef, RecordableVersionModel.PROP_VERSIONED_NODEREF) == null) + { // if enabled cleanse content + if (isCleansingEnabled()) + { + // register for cleanse then immediate destruction + getEagerContentStoreCleaner().registerOrphanedContentUrlForCleansing(dataContent.getContentUrl()); + } + else + { + // register for immediate destruction + getEagerContentStoreCleaner().registerOrphanedContentUrl(dataContent.getContentUrl(), true); + } } // clear the property From da2115435063d8afcb238d3fb1e10b406e58605c Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Tue, 12 Feb 2019 10:49:55 +0200 Subject: [PATCH 04/14] add tests to check the content bin is available for: - the copy when the original file is destroyed - file when having a record version that gets deleted --- .../service/DispositionScheduleService.java | 19 ++- .../community/records/DeleteRecordTests.java | 139 ++++++++++++++++-- 2 files changed, 143 insertions(+), 15 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java b/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java index 866c550dff..f3cecae507 100644 --- a/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java +++ b/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java @@ -30,7 +30,6 @@ package org.alfresco.rest.v0.service; import java.util.HashMap; import org.alfresco.rest.core.v0.BaseAPI; -import org.alfresco.rest.rm.community.model.recordcategory.RecordCategory; import org.alfresco.rest.v0.RecordCategoriesAPI; import org.alfresco.utility.data.DataUser; import org.springframework.beans.factory.annotation.Autowired; @@ -85,6 +84,24 @@ public class DispositionScheduleService extends BaseAPI dataUser.getAdminUser().getPassword(), categoryName, cutOffStep); } + /** + * Helper method for adding a destroy with ghosting after period + * + * @param categoryName the category in whose schedule the step will be added + * @param period + * @return + */ + public void addDestroyWithGhostingAfterPeriodStep(String categoryName, String period) + { + HashMap destroyStep = new HashMap<>(); + destroyStep.put(RETENTION_SCHEDULE.NAME, "destroy"); + destroyStep.put(RETENTION_SCHEDULE.RETENTION_PERIOD, "immediately|"); + destroyStep.put(RETENTION_SCHEDULE.DESCRIPTION, "Destroy immediately"); + destroyStep.put(RETENTION_SCHEDULE.RETENTION_GHOST, "on"); + recordCategoriesAPI.addDispositionScheduleSteps(dataUser.getAdminUser().getUsername(), + dataUser.getAdminUser().getPassword(), categoryName, destroyStep); + } + /** * Helper method for adding a cut off after an event occurs step * diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java index 79e30ea0d4..1267f371a2 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java @@ -40,6 +40,8 @@ import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.create import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createNonElectronicUnfiledContainerChildModel; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.getFile; import static org.alfresco.utility.constants.UserRole.SiteCollaborator; +import static org.alfresco.utility.data.RandomData.getRandomName; +import static org.alfresco.utility.report.log.Step.STEP; import static org.springframework.http.HttpStatus.CREATED; import static org.springframework.http.HttpStatus.FORBIDDEN; import static org.springframework.http.HttpStatus.NOT_FOUND; @@ -49,23 +51,29 @@ import static org.springframework.http.HttpStatus.OK; import org.alfresco.dataprep.CMISUtil; import org.alfresco.rest.core.JsonBodyGenerator; import org.alfresco.rest.core.RestResponse; +import org.alfresco.rest.core.v0.BaseAPI.RM_ACTIONS; import org.alfresco.rest.model.RestNodeBodyMoveCopyModel; import org.alfresco.rest.model.RestNodeModel; import org.alfresco.rest.requests.Node; import org.alfresco.rest.rm.community.base.BaseRMRestTest; import org.alfresco.rest.rm.community.model.record.Record; +import org.alfresco.rest.rm.community.model.record.RecordBodyFile; +import org.alfresco.rest.rm.community.model.recordcategory.RecordCategory; import org.alfresco.rest.rm.community.model.recordcategory.RecordCategoryChild; import org.alfresco.rest.rm.community.model.unfiledcontainer.UnfiledContainerChild; import org.alfresco.rest.rm.community.requests.gscore.api.RecordCategoryAPI; import org.alfresco.rest.rm.community.requests.gscore.api.RecordFolderAPI; import org.alfresco.rest.rm.community.requests.gscore.api.RecordsAPI; +import org.alfresco.rest.v0.RMRolesAndActionsAPI; +import org.alfresco.rest.v0.service.DispositionScheduleService; import org.alfresco.test.AlfrescoTest; import org.alfresco.utility.data.RandomData; import org.alfresco.utility.model.FileModel; +import org.alfresco.utility.model.FolderModel; import org.alfresco.utility.model.RepoTestModel; import org.alfresco.utility.model.SiteModel; import org.alfresco.utility.model.UserModel; -import org.alfresco.utility.report.log.Step; +import org.springframework.beans.factory.annotation.Autowired; import org.testng.annotations.Test; /** @@ -76,6 +84,13 @@ import org.testng.annotations.Test; */ public class DeleteRecordTests extends BaseRMRestTest { + @Autowired + private DispositionScheduleService dispositionScheduleService; + @Autowired + private RMRolesAndActionsAPI rmRolesAndActionsAPI; + @Autowired + private org.alfresco.rest.v0.RecordsAPI recordsAPI; + /** *
      * Given an electronic record
@@ -266,24 +281,24 @@ public class DeleteRecordTests extends BaseRMRestTest
     @AlfrescoTest(jira="MNT-18806")
     public void deleteCopyOfRecord()
     {
-        Step.STEP("Create two record categories and folders.");
+        STEP("Create two record categories and folders.");
         RecordCategoryChild recordFolderA = createCategoryFolderInFilePlan();
         RecordCategoryChild recordFolderB = createCategoryFolderInFilePlan();
 
-        Step.STEP("Create a record in folder A and copy it into folder B.");
+        STEP("Create a record in folder A and copy it into folder B.");
         String recordId = getRestAPIFactory().getRecordFolderAPI()
                     .createRecord(createElectronicRecordModel(), recordFolderA.getId(), getFile(IMAGE_FILE)).getId();
         String copyId = copyRecord(recordId, recordFolderB.getId()).getId();
         assertStatusCode(CREATED);
 
-        Step.STEP("Check that it's possible to load the original content.");
+        STEP("Check that it's possible to load the original content.");
         getNodeContent(recordId);
         assertStatusCode(OK);
 
-        Step.STEP("Delete the copy.");
+        STEP("Delete the copy.");
         deleteAndVerify(copyId);
 
-        Step.STEP("Check that the original record node and content still exist.");
+        STEP("Check that the original record node and content still exist.");
         checkNodeExists(recordId);
         getNodeContent(recordId);
     }
@@ -301,30 +316,126 @@ public class DeleteRecordTests extends BaseRMRestTest
     @AlfrescoTest (jira = "MNT-20145")
     public void deleteOriginOfRecord() throws Exception
     {
-        Step.STEP("Create a file.");
+        STEP("Create a file.");
         testSite = dataSite.usingAdmin().createPublicRandomSite();
         FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN);
 
-        Step.STEP("Create a copy for the file created.");
+        STEP("Create a copy for the file created.");
         String postBody = JsonBodyGenerator.keyValueJson("targetParentId", testSite.getGuid());
         RestNodeModel copyOfTestFile = getRestAPIFactory().getNodeAPI(testFile).copyNode(postBody);
 
-        Step.STEP("Declare original file as record");
+        STEP("Declare original file as record");
         getRestAPIFactory().getFilesAPI().declareAsRecord(testFile.getNodeRefWithoutVersion());
         assertStatusCode(CREATED);
 
-        Step.STEP("Delete the record.");
-        getRestAPIFactory().getRecordsAPI().deleteRecord(testFile.getNodeRefWithoutVersion());
-        assertStatusCode(NO_CONTENT);
+        STEP("Delete the record.");
+        deleteAndVerify(testFile.getNodeRefWithoutVersion());
 
-        Step.STEP("Check that it's possible to load the copy content.");
+        STEP("Check that it's possible to load the copy content.");
         getNodeContent(copyOfTestFile.getId());
         assertStatusCode(OK);
 
-        Step.STEP("Clean up.");
+        STEP("Clean up.");
         dataSite.deleteSite(testSite);
     }
 
+    /**
+     * 
+     * Given a file that has  copy
+     * And the original file is declared as record
+     * And the file becomes part of a disposition schedule with a destroy step
+     * When the record is destroyed
+     * Then it is still possible to view the content of the copy
+     * 
+ */ + @Test (description = "Destroying record doesn't delete the content for the associated copy") + @AlfrescoTest (jira = "MNT-20145") + public void destroyOfRecord() throws Exception + { + STEP("Create a file."); + testSite = dataSite.usingAdmin().createPublicRandomSite(); + FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN); + FolderModel folderModel =dataContent.usingSite(testSite).createFolder(); + + STEP("Create a copy for the file created."); + RestNodeModel copy = copyRecord(testFile.getNodeRefWithoutVersion(), folderModel.getNodeRefWithoutVersion()); + assertStatusCode(CREATED); + + STEP("Declare the file as record."); + getRestAPIFactory().getFilesAPI().declareAsRecord(testFile.getNodeRefWithoutVersion()); + assertStatusCode(CREATED); + + STEP("Create a record category with a disposition schedule."); + RecordCategory recordCategory = createRootCategory(getRandomName("Category with disposition")); + dispositionScheduleService.createCategoryRetentionSchedule(recordCategory.getName(), true); + + STEP("Add retention schedule cut off and destroy step with immediate period."); + dispositionScheduleService.addCutOffAfterPeriodStep(recordCategory.getName(), "immediately"); + dispositionScheduleService.addDestroyWithGhostingAfterPeriodStep(recordCategory.getName(), "immediately"); + + STEP("Create a record folder and file the record"); + RecordCategoryChild recFolder = createFolder(recordCategory.getId(), getRandomName("recFolder")); + RecordBodyFile recordBodyFile = RecordBodyFile.builder().targetParentId(recFolder.getId()).build(); + Record recordFiled = getRestAPIFactory().getRecordsAPI().fileRecord(recordBodyFile, testFile.getNodeRefWithoutVersion()); + getRestAPIFactory().getRecordsAPI().completeRecord(recordFiled.getId()); + assertStatusCode(CREATED); + + STEP("Execute the disposition schedule steps ."); + rmRolesAndActionsAPI.executeAction(getAdminUser().getUsername(), getAdminUser().getUsername(), recordFiled.getName(), + RM_ACTIONS.CUT_OFF); + rmRolesAndActionsAPI.executeAction(getAdminUser().getUsername(), getAdminUser().getUsername(), recordFiled.getName(), + RM_ACTIONS.DESTROY); + + STEP("Check that it's possible to load the copy content."); + getNodeContent(copy.getId()); + assertStatusCode(OK); + + STEP("Clean up."); + dataSite.deleteSite(testSite); + + } + + /** + *
+     * Given a file that is declared version as record
+     * When the record is deleted
+     * Then it is still possible to view the content of the file
+     * 
+ */ + @Test (description = "Destroying record doesn't delete the content for the associated copy") + @AlfrescoTest (jira = "MNT-20145") + public void deleteVersionDeclaredAsRecord() throws Exception + { + STEP("Create a file."); + testSite = dataSite.usingAdmin().createPublicRandomSite(); + FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN); + FolderModel folderModel = dataContent.usingSite(testSite).createFolder(); + + STEP("Declare the file as record."); + // declare documents as records + recordsAPI.declareDocumentVersionAsRecord(getAdminUser().getUsername(), getAdminUser().getPassword(), testSite.getId(), + testFile.getName()); + UnfiledContainerChild unfiledContainerChild = getRestAPIFactory().getUnfiledContainersAPI() + .getUnfiledContainerChildren(UNFILED_RECORDS_CONTAINER_ALIAS) + .getEntries().stream() + .filter(child -> child.getEntry().getName() + .startsWith(testFile.getName().substring(0, testFile.getName().indexOf(".")))) + .findFirst() + .get().getEntry(); + + STEP("Delete the record."); + deleteAndVerify(unfiledContainerChild.getId()); + + STEP("Check that it's possible to load the file declared version as record."); + getNodeContent(testFile.getNodeRefWithoutVersion()); + assertStatusCode(OK); + + STEP("Clean up."); + dataSite.deleteSite(testSite); + + } + + /** * Utility method to delete a record and verify successful deletion * From d3a8dca23003bff91ae650daa0674cb88c65f2a3 Mon Sep 17 00:00:00 2001 From: cagache Date: Tue, 12 Feb 2019 12:25:10 +0200 Subject: [PATCH 05/14] code review changes --- .../service/DispositionScheduleService.java | 17 ++++---- .../community/records/DeleteRecordTests.java | 41 +++++++++---------- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java b/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java index f3cecae507..6de77ec194 100644 --- a/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java +++ b/rm-automation/rm-automation-community-rest-api/src/main/java/org/alfresco/rest/v0/service/DispositionScheduleService.java @@ -55,16 +55,15 @@ public class DispositionScheduleService extends BaseAPI * * @param categoryName the category in whose schedule the step will be added * @param period - * @return */ public void addRetainAfterPeriodStep(String categoryName, String period) { - HashMap cutOffStep = new HashMap<>(); - cutOffStep.put(RETENTION_SCHEDULE.NAME, "retain"); - cutOffStep.put(RETENTION_SCHEDULE.RETENTION_PERIOD, period); - cutOffStep.put(RETENTION_SCHEDULE.DESCRIPTION, "Retain after a period step"); + HashMap retainStep = new HashMap<>(); + retainStep.put(RETENTION_SCHEDULE.NAME, "retain"); + retainStep.put(RETENTION_SCHEDULE.RETENTION_PERIOD, period); + retainStep.put(RETENTION_SCHEDULE.DESCRIPTION, "Retain after a period step"); recordCategoriesAPI.addDispositionScheduleSteps(dataUser.getAdminUser().getUsername(), - dataUser.getAdminUser().getPassword(), categoryName, cutOffStep); + dataUser.getAdminUser().getPassword(), categoryName, retainStep); } /** @@ -72,7 +71,6 @@ public class DispositionScheduleService extends BaseAPI * * @param categoryName the category in whose schedule the step will be added * @param period - * @return */ public void addCutOffAfterPeriodStep(String categoryName, String period) { @@ -89,14 +87,13 @@ public class DispositionScheduleService extends BaseAPI * * @param categoryName the category in whose schedule the step will be added * @param period - * @return */ public void addDestroyWithGhostingAfterPeriodStep(String categoryName, String period) { HashMap destroyStep = new HashMap<>(); destroyStep.put(RETENTION_SCHEDULE.NAME, "destroy"); - destroyStep.put(RETENTION_SCHEDULE.RETENTION_PERIOD, "immediately|"); - destroyStep.put(RETENTION_SCHEDULE.DESCRIPTION, "Destroy immediately"); + destroyStep.put(RETENTION_SCHEDULE.RETENTION_PERIOD, period); + destroyStep.put(RETENTION_SCHEDULE.DESCRIPTION, "Destroy after a period step"); destroyStep.put(RETENTION_SCHEDULE.RETENTION_GHOST, "on"); recordCategoriesAPI.addDispositionScheduleSteps(dataUser.getAdminUser().getUsername(), dataUser.getAdminUser().getPassword(), categoryName, destroyStep); diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java index 1267f371a2..a091b54d01 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/records/DeleteRecordTests.java @@ -288,7 +288,7 @@ public class DeleteRecordTests extends BaseRMRestTest STEP("Create a record in folder A and copy it into folder B."); String recordId = getRestAPIFactory().getRecordFolderAPI() .createRecord(createElectronicRecordModel(), recordFolderA.getId(), getFile(IMAGE_FILE)).getId(); - String copyId = copyRecord(recordId, recordFolderB.getId()).getId(); + String copyId = copyNode(recordId, recordFolderB.getId()).getId(); assertStatusCode(CREATED); STEP("Check that it's possible to load the original content."); @@ -306,7 +306,7 @@ public class DeleteRecordTests extends BaseRMRestTest /** *
-     * Given a file that has  copy
+     * Given a file that has a copy
      * And the original file is declared as record
      * When I delete the original
      * Then it is still possible to view the content of the copy
@@ -320,9 +320,8 @@ public class DeleteRecordTests extends BaseRMRestTest
         testSite = dataSite.usingAdmin().createPublicRandomSite();
         FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN);
 
-        STEP("Create a copy for the file created.");
-        String postBody = JsonBodyGenerator.keyValueJson("targetParentId", testSite.getGuid());
-        RestNodeModel copyOfTestFile = getRestAPIFactory().getNodeAPI(testFile).copyNode(postBody);
+        STEP("Create a copy of the file.");
+        RestNodeModel copyOfTestFile = copyNode(testFile.getNodeRefWithoutVersion(), testSite.getGuid());
 
         STEP("Declare original file as record");
         getRestAPIFactory().getFilesAPI().declareAsRecord(testFile.getNodeRefWithoutVersion());
@@ -341,9 +340,9 @@ public class DeleteRecordTests extends BaseRMRestTest
 
     /**
      * 
-     * Given a file that has  copy
+     * Given a file that has a copy
      * And the original file is declared as record
-     * And the file becomes part of a disposition schedule with a destroy step
+     * And the record becomes part of a disposition schedule with a destroy step
      * When the record is destroyed
      * Then it is still possible to view the content of the copy
      * 
@@ -355,10 +354,10 @@ public class DeleteRecordTests extends BaseRMRestTest STEP("Create a file."); testSite = dataSite.usingAdmin().createPublicRandomSite(); FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN); - FolderModel folderModel =dataContent.usingSite(testSite).createFolder(); + FolderModel folderModel = dataContent.usingSite(testSite).createFolder(); - STEP("Create a copy for the file created."); - RestNodeModel copy = copyRecord(testFile.getNodeRefWithoutVersion(), folderModel.getNodeRefWithoutVersion()); + STEP("Create a copy of the file."); + RestNodeModel copy = copyNode(testFile.getNodeRefWithoutVersion(), folderModel.getNodeRefWithoutVersion()); assertStatusCode(CREATED); STEP("Declare the file as record."); @@ -366,7 +365,7 @@ public class DeleteRecordTests extends BaseRMRestTest assertStatusCode(CREATED); STEP("Create a record category with a disposition schedule."); - RecordCategory recordCategory = createRootCategory(getRandomName("Category with disposition")); + RecordCategory recordCategory = createRootCategory(getRandomName("Category with disposition")); dispositionScheduleService.createCategoryRetentionSchedule(recordCategory.getName(), true); STEP("Add retention schedule cut off and destroy step with immediate period."); @@ -380,7 +379,7 @@ public class DeleteRecordTests extends BaseRMRestTest getRestAPIFactory().getRecordsAPI().completeRecord(recordFiled.getId()); assertStatusCode(CREATED); - STEP("Execute the disposition schedule steps ."); + STEP("Execute the disposition schedule steps."); rmRolesAndActionsAPI.executeAction(getAdminUser().getUsername(), getAdminUser().getUsername(), recordFiled.getName(), RM_ACTIONS.CUT_OFF); rmRolesAndActionsAPI.executeAction(getAdminUser().getUsername(), getAdminUser().getUsername(), recordFiled.getName(), @@ -397,22 +396,20 @@ public class DeleteRecordTests extends BaseRMRestTest /** *
-     * Given a file that is declared version as record
+     * Given a file that has version declared as record
      * When the record is deleted
      * Then it is still possible to view the content of the file
      * 
*/ - @Test (description = "Destroying record doesn't delete the content for the associated copy") + @Test (description = "Deleting record made from version doesn't delete the content for the file") @AlfrescoTest (jira = "MNT-20145") public void deleteVersionDeclaredAsRecord() throws Exception { STEP("Create a file."); testSite = dataSite.usingAdmin().createPublicRandomSite(); FileModel testFile = dataContent.usingSite(testSite).createContent(CMISUtil.DocumentType.TEXT_PLAIN); - FolderModel folderModel = dataContent.usingSite(testSite).createFolder(); - STEP("Declare the file as record."); - // declare documents as records + STEP("Declare file version as record."); recordsAPI.declareDocumentVersionAsRecord(getAdminUser().getUsername(), getAdminUser().getPassword(), testSite.getId(), testFile.getName()); UnfiledContainerChild unfiledContainerChild = getRestAPIFactory().getUnfiledContainersAPI() @@ -455,15 +452,15 @@ public class DeleteRecordTests extends BaseRMRestTest } /** - * Copy a record to a record folder. + * Copy a node to a folder. * - * @param recordId The id of the record to copy. - * @param destinationFolder The id of the record folder to copy it to. + * @param nodeId The id of the node to copy. + * @param destinationFolder The id of the folder to copy it to. * @return The model returned by the copy API. */ - private RestNodeModel copyRecord(String recordId, String destinationFolder) + private RestNodeModel copyNode(String nodeId, String destinationFolder) { - Node node = getNode(recordId); + Node node = getNode(nodeId); RestNodeBodyMoveCopyModel copyBody = new RestNodeBodyMoveCopyModel(); copyBody.setTargetParentId(destinationFolder); try From d94b0fb7fdb86e26af5555d2a6140823b4fad145 Mon Sep 17 00:00:00 2001 From: cagache Date: Wed, 13 Feb 2019 16:00:05 +0200 Subject: [PATCH 06/14] added IT for delete classified content with copies --- .../content/ContentDestructionComponent.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java index 5bdfd1dc33..07226abc96 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java @@ -215,12 +215,11 @@ public class ContentDestructionComponent // get content data ContentData dataContent = (ContentData)entry.getValue(); - // destroy the nodes content properties only if it doesn't have copies or it is a copy - // destroy the nodes content properties only if the versionedNodeRef is deleted + // destroy the node's content properties only if it doesn't have copies or it is a copy if (getNodeService().getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() && - getNodeService().getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() && - getNodeService().getProperty(nodeRef, RecordableVersionModel.PROP_VERSIONED_NODEREF) == null) - { // if enabled cleanse content + getNodeService().getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty()) + { + // if enabled cleanse content if (isCleansingEnabled()) { // register for cleanse then immediate destruction From 1cae71c02ada11d59b0c6107bfb45a9fe33f5182 Mon Sep 17 00:00:00 2001 From: cagache Date: Thu, 14 Feb 2019 12:55:10 +0200 Subject: [PATCH 07/14] added behaviour to duplicate the bin before declaring a version record --- .../rm-version-context.xml | 1 + .../model/rma/aspect/VersionRecordAspect.java | 43 +++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml index 91c2f622ea..94ddc06345 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml @@ -16,6 +16,7 @@ + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java index 9437c6c6ef..a22ce023c0 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java @@ -34,12 +34,17 @@ import org.alfresco.module.org_alfresco_module_rm.relationship.Relationship; import org.alfresco.module.org_alfresco_module_rm.relationship.RelationshipService; import org.alfresco.module.org_alfresco_module_rm.version.RecordableVersionService; import org.alfresco.repo.node.NodeServicePolicies; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; +import org.alfresco.service.cmr.model.FileFolderService; +import org.alfresco.service.cmr.repository.ContentReader; +import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.version.Version; +import org.alfresco.service.namespace.QName; /** * rmv:versionRecord behaviour bean @@ -52,14 +57,18 @@ import org.alfresco.service.cmr.version.Version; defaultType = "rmv:versionRecord" ) public class VersionRecordAspect extends BaseBehaviourBean - implements NodeServicePolicies.BeforeDeleteNodePolicy + implements NodeServicePolicies.BeforeAddAspectPolicy, + NodeServicePolicies.BeforeDeleteNodePolicy { /** recordable version service */ private RecordableVersionService recordableVersionService; /** relationship service */ private RelationshipService relationshipService; - + + /** File folder service */ + private FileFolderService fileFolderService; + /** * @param recordableVersionService recordable version service */ @@ -75,7 +84,16 @@ public class VersionRecordAspect extends BaseBehaviourBean { this.relationshipService = relationshipService; } - + + /** + * + * @param fileFolderService file folder service + */ + public void setFileFolderService(FileFolderService fileFolderService) + { + this.fileFolderService = fileFolderService; + } + /** * If the record is a version record then delete the associated version entry * @@ -129,4 +147,23 @@ public class VersionRecordAspect extends BaseBehaviourBean }); } } + + /** + * Behaviour to duplicate the bin before declaring a version record + * + * @see org.alfresco.repo.node.NodeServicePolicies.BeforeAddAspectPolicy#beforeAddAspect(org.alfresco.service.cmr.repository.NodeRef, + * org.alfresco.service.namespace.QName) + */ + @Override + @Behaviour(kind = BehaviourKind.CLASS, notificationFrequency = NotificationFrequency.FIRST_EVENT) + public void beforeAddAspect(NodeRef nodeRef, QName qName) + { + //create a new content URL for the version record + ContentReader reader = fileFolderService.getReader(nodeRef); + if (reader != null) + { + ContentWriter writer = fileFolderService.getWriter(nodeRef); + writer.putContent(reader); + } + } } From 9a72cc8e1183e2747e7c25e9ee14ae5d0bc032a4 Mon Sep 17 00:00:00 2001 From: cagache Date: Thu, 14 Feb 2019 16:22:57 +0200 Subject: [PATCH 08/14] fixed failing tests --- .../version/RecordableVersionsBaseTest.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java index c75997e0e1..6fda8324da 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java @@ -238,7 +238,16 @@ public abstract class RecordableVersionsBaseTest extends BaseRMTestCase implemen if (frozenProperties.containsKey(beforePropertyName)) { Serializable frozenValue = frozenProperties.get(beforePropertyName); - assertEquals("Frozen property " + beforePropertyName.getLocalName() + " value is incorrect.", entry.getValue(), frozenValue); + if(beforePropertyName.equals(ContentModel.PROP_CONTENT)) + { + assertTrue("Frozen property " + beforePropertyName.getLocalName() + " value is incorrect.", + entry.getValue() != frozenValue); + } + else + { + assertEquals("Frozen property " + beforePropertyName.getLocalName() + " value is incorrect.", + entry.getValue(), frozenValue); + } cloneFrozenProperties.remove(beforePropertyName); } else if (!PROP_FILE_PLAN.equals(beforePropertyName) && From 6ba8c3628137036dcb67844341b52fff6f57db44 Mon Sep 17 00:00:00 2001 From: cagache Date: Fri, 15 Feb 2019 16:41:18 +0200 Subject: [PATCH 09/14] MNT-20145 Duplicate the content url when declaring node as record, version as record, classifying or securing if the node has copies or it is a copy --- .../rm-model-context.xml | 1 - .../rm-service-context.xml | 1 + .../rm-version-context.xml | 1 - .../content/ContentDestructionComponent.java | 23 ++++------ .../model/rma/aspect/RecordAspect.java | 44 +++++++++---------- .../model/rma/aspect/VersionRecordAspect.java | 22 +--------- .../util/ServiceBaseImpl.java | 29 +++++++++++- 7 files changed, 61 insertions(+), 60 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml index 4cb71d28e0..ddd303c982 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml @@ -143,7 +143,6 @@ - diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index 01ac11f766..8ae5ad48fa 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -45,6 +45,7 @@ + diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml index 94ddc06345..91c2f622ea 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-version-context.xml @@ -16,7 +16,6 @@ - diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java index 07226abc96..6c714ae61c 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java @@ -215,21 +215,16 @@ public class ContentDestructionComponent // get content data ContentData dataContent = (ContentData)entry.getValue(); - // destroy the node's content properties only if it doesn't have copies or it is a copy - if (getNodeService().getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() && - getNodeService().getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty()) + // if enabled cleanse content + if (isCleansingEnabled()) { - // if enabled cleanse content - if (isCleansingEnabled()) - { - // register for cleanse then immediate destruction - getEagerContentStoreCleaner().registerOrphanedContentUrlForCleansing(dataContent.getContentUrl()); - } - else - { - // register for immediate destruction - getEagerContentStoreCleaner().registerOrphanedContentUrl(dataContent.getContentUrl(), true); - } + // register for cleanse then immediate destruction + getEagerContentStoreCleaner().registerOrphanedContentUrlForCleansing(dataContent.getContentUrl()); + } + else + { + // register for immediate destruction + getEagerContentStoreCleaner().registerOrphanedContentUrl(dataContent.getContentUrl(), true); } // clear the property diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java index b59181191b..fbc4725afe 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java @@ -51,13 +51,9 @@ import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; -import org.alfresco.service.cmr.model.FileFolderService; import org.alfresco.service.cmr.quickshare.QuickShareService; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.ContentData; -import org.alfresco.service.cmr.repository.ContentReader; -import org.alfresco.service.cmr.repository.ContentService; -import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.ScriptService; import org.alfresco.service.namespace.QName; @@ -98,9 +94,6 @@ public class RecordAspect extends AbstractDisposableItem /** quickShare service */ private QuickShareService quickShareService; - /** File folder service */ - private FileFolderService fileFolderService; - /** I18N */ private static final String MSG_CANNOT_UPDATE_RECORD_CONTENT = "rm.service.update-record-content"; @@ -137,15 +130,6 @@ public class RecordAspect extends AbstractDisposableItem this.quickShareService = quickShareService; } - /** - * - * @param fileFolderService file folder service - */ - public void setFileFolderService(FileFolderService fileFolderService) - { - this.fileFolderService = fileFolderService; - } - /** * Behaviour to ensure renditions have the appropriate extended security. * @@ -376,12 +360,7 @@ public class RecordAspect extends AbstractDisposableItem extendedSecurityService.remove(targetNodeRef); //create a new content URL for the copy - ContentReader reader = fileFolderService.getReader(targetNodeRef); - if (reader != null) - { - ContentWriter writer = fileFolderService.getWriter(targetNodeRef); - writer.putContent(reader); - } + createNewContentURL(targetNodeRef); } } @@ -402,6 +381,7 @@ public class RecordAspect extends AbstractDisposableItem /** * Behaviour to remove the shared link before declare a record + * and to create new bin if the node is a copy or has copies * * @see org.alfresco.repo.node.NodeServicePolicies.BeforeAddAspectPolicy#beforeAddAspect(org.alfresco.service.cmr.repository.NodeRef, * org.alfresco.service.namespace.QName) @@ -421,6 +401,26 @@ public class RecordAspect extends AbstractDisposableItem quickShareService.unshareContent(sharedId); } + // if the node has a copy or is a copy of an existing node + if (!nodeService.getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() || + !nodeService.getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty()) + { + //disabling versioning and auditing + behaviourFilter.disableBehaviour(ContentModel.ASPECT_AUDITABLE); + behaviourFilter.disableBehaviour(ContentModel.ASPECT_VERSIONABLE); + try + { + //create a new content URL for the copy/original node + createNewContentURL(nodeRef); + } + finally + { + //enable versioning and auditing + behaviourFilter.enableBehaviour(ContentModel.ASPECT_AUDITABLE); + behaviourFilter.enableBehaviour(ContentModel.ASPECT_VERSIONABLE); + } + } + return null; } }, AuthenticationUtil.getSystemUserName()); diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java index a22ce023c0..f4712a8995 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java @@ -39,9 +39,6 @@ import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; -import org.alfresco.service.cmr.model.FileFolderService; -import org.alfresco.service.cmr.repository.ContentReader; -import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.version.Version; import org.alfresco.service.namespace.QName; @@ -66,9 +63,6 @@ public class VersionRecordAspect extends BaseBehaviourBean /** relationship service */ private RelationshipService relationshipService; - /** File folder service */ - private FileFolderService fileFolderService; - /** * @param recordableVersionService recordable version service */ @@ -85,15 +79,6 @@ public class VersionRecordAspect extends BaseBehaviourBean this.relationshipService = relationshipService; } - /** - * - * @param fileFolderService file folder service - */ - public void setFileFolderService(FileFolderService fileFolderService) - { - this.fileFolderService = fileFolderService; - } - /** * If the record is a version record then delete the associated version entry * @@ -159,11 +144,6 @@ public class VersionRecordAspect extends BaseBehaviourBean public void beforeAddAspect(NodeRef nodeRef, QName qName) { //create a new content URL for the version record - ContentReader reader = fileFolderService.getReader(nodeRef); - if (reader != null) - { - ContentWriter writer = fileFolderService.getWriter(nodeRef); - writer.putContent(reader); - } + createNewContentURL(nodeRef); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java index 8f45c69441..5eccd498e5 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java @@ -36,8 +36,11 @@ import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.hold.HoldService; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.service.cmr.dictionary.DictionaryService; +import org.alfresco.service.cmr.model.FileFolderService; import org.alfresco.service.cmr.rendition.RenditionService; import org.alfresco.service.cmr.repository.ChildAssociationRef; +import org.alfresco.service.cmr.repository.ContentReader; +import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; @@ -73,7 +76,10 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte protected AuthenticationUtil authenticationUtil; /** transactional resource helper */ - protected TransactionalResourceHelper transactionalResourceHelper; + protected TransactionalResourceHelper transactionalResourceHelper; + + /** File folder service */ + protected FileFolderService fileFolderService; /** * @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext) @@ -124,6 +130,16 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte this.transactionalResourceHelper = transactionalResourceHelper; } + /** + * Set the file folder service + * + * @param fileFolderService file folder service + */ + public void setFileFolderService(FileFolderService fileFolderService) + { + this.fileFolderService = fileFolderService; + } + /** * Helper to get internal node service. *

@@ -537,4 +553,15 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte result.add(nodeService.getType(nodeRef)); return result; } + + protected void createNewContentURL(NodeRef nodeRef) + { + //create a new content URL for the copy + ContentReader reader = fileFolderService.getReader(nodeRef); + if (reader != null) + { + ContentWriter writer = fileFolderService.getWriter(nodeRef); + writer.putContent(reader); + } + } } From b09d7bf8ea4bb396736e5740ef084b39af7482c9 Mon Sep 17 00:00:00 2001 From: cagache Date: Fri, 15 Feb 2019 17:11:04 +0200 Subject: [PATCH 10/14] added javadoc and removed unused imports --- .../content/ContentDestructionComponent.java | 1 - .../org_alfresco_module_rm/util/ServiceBaseImpl.java | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java index 6c714ae61c..fb77f3efb6 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java @@ -33,7 +33,6 @@ import java.util.Set; import org.alfresco.model.ContentModel; import org.alfresco.model.RenditionModel; -import org.alfresco.module.org_alfresco_module_rm.version.RecordableVersionModel; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.service.cmr.dictionary.DictionaryService; diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java index 5eccd498e5..bccb9b0c2b 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java @@ -74,7 +74,7 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte /** authentication helper */ protected AuthenticationUtil authenticationUtil; - + /** transactional resource helper */ protected TransactionalResourceHelper transactionalResourceHelper; @@ -554,9 +554,14 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte return result; } + /** + * Helper to create a new content URL for the node + * + * @param nodeRef the node + */ protected void createNewContentURL(NodeRef nodeRef) { - //create a new content URL for the copy + //create a new content URL for the node ContentReader reader = fileFolderService.getReader(nodeRef); if (reader != null) { From 6a5c26717046ed9a76bf67f01e39870d3dac3866 Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 18 Feb 2019 10:57:16 +0200 Subject: [PATCH 11/14] addressed code review comments --- .../rm-service-context.xml | 2 +- .../model/rma/aspect/RecordAspect.java | 8 +--- .../util/ServiceBaseImpl.java | 39 ++++++++++++------- .../version/RecordableVersionsBaseTest.java | 2 +- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index 8ae5ad48fa..de5519decd 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -45,7 +45,7 @@ - + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java index fbc4725afe..441af631bb 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java @@ -335,11 +335,7 @@ public class RecordAspect extends AbstractDisposableItem /** * On copy complete behaviour for record aspect. * - * @param classRef - * @param sourceNodeRef - * @param targetNodeRef - * @param copyToNewNode - * @param copyMap + * @see org.alfresco.repo.copy.CopyServicePolicies.OnCopyCompletePolicy#onCopyComplete(QName, NodeRef, NodeRef, boolean, Map) */ @Override @Behaviour @@ -405,7 +401,7 @@ public class RecordAspect extends AbstractDisposableItem if (!nodeService.getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() || !nodeService.getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty()) { - //disabling versioning and auditing + //disable versioning and auditing behaviourFilter.disableBehaviour(ContentModel.ASPECT_AUDITABLE); behaviourFilter.disableBehaviour(ContentModel.ASPECT_VERSIONABLE); try diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java index bccb9b0c2b..77b7ab0d79 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java @@ -28,9 +28,11 @@ package org.alfresco.module.org_alfresco_module_rm.util; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.WeakHashMap; +import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanComponentKind; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.hold.HoldService; @@ -40,6 +42,7 @@ import org.alfresco.service.cmr.model.FileFolderService; import org.alfresco.service.cmr.rendition.RenditionService; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.ContentReader; +import org.alfresco.service.cmr.repository.ContentService; import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; @@ -78,8 +81,8 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte /** transactional resource helper */ protected TransactionalResourceHelper transactionalResourceHelper; - /** File folder service */ - protected FileFolderService fileFolderService; + /** Content service */ + protected ContentService contentService; /** * @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext) @@ -131,13 +134,13 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte } /** - * Set the file folder service + * Set the content service * - * @param fileFolderService file folder service + * @param contentService content service */ - public void setFileFolderService(FileFolderService fileFolderService) + public void setContentService(ContentService contentService) { - this.fileFolderService = fileFolderService; + this.contentService = contentService; } /** @@ -554,6 +557,22 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte return result; } + /** + * Helper to update the given content property for the node + * + * @param nodeRef the node + * @param contentProperty the property to be updated + */ + protected void updateContentProperty(NodeRef nodeRef, QName contentProperty) + { + ContentReader reader = contentService.getReader(nodeRef, contentProperty); + if (reader != null) + { + ContentWriter writer = contentService.getWriter(nodeRef, contentProperty, true); + writer.putContent(reader); + } + } + /** * Helper to create a new content URL for the node * @@ -561,12 +580,6 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte */ protected void createNewContentURL(NodeRef nodeRef) { - //create a new content URL for the node - ContentReader reader = fileFolderService.getReader(nodeRef); - if (reader != null) - { - ContentWriter writer = fileFolderService.getWriter(nodeRef); - writer.putContent(reader); - } + updateContentProperty(nodeRef, ContentModel.PROP_CONTENT); } } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java index 6fda8324da..2727c0b730 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java @@ -240,7 +240,7 @@ public abstract class RecordableVersionsBaseTest extends BaseRMTestCase implemen Serializable frozenValue = frozenProperties.get(beforePropertyName); if(beforePropertyName.equals(ContentModel.PROP_CONTENT)) { - assertTrue("Frozen property " + beforePropertyName.getLocalName() + " value is incorrect.", + assertTrue("Content property value should be different.", entry.getValue() != frozenValue); } else From dfb473eef698370c44d28175ab16613a0a6e1e0e Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 18 Feb 2019 15:16:48 +0200 Subject: [PATCH 12/14] added unit tests --- .../util/ServiceBaseImpl.java | 2 - .../rma/aspect/RecordAspectUnitTest.java | 139 ++++++++++++++++++ 2 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java index 77b7ab0d79..8fc5094b05 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java @@ -28,7 +28,6 @@ package org.alfresco.module.org_alfresco_module_rm.util; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.WeakHashMap; @@ -38,7 +37,6 @@ import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.hold.HoldService; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.service.cmr.dictionary.DictionaryService; -import org.alfresco.service.cmr.model.FileFolderService; import org.alfresco.service.cmr.rendition.RenditionService; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.ContentReader; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java new file mode 100644 index 0000000000..78c6263dbf --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java @@ -0,0 +1,139 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2019 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * 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 . + * #L% + */ +package org.alfresco.module.org_alfresco_module_rm.model.rma.aspect; + +import static java.util.Arrays.asList; + +import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASPECT_RECORD; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import org.alfresco.model.ContentModel; +import org.alfresco.repo.policy.BehaviourFilter; +import org.alfresco.service.cmr.repository.AssociationRef; +import org.alfresco.service.cmr.repository.ContentReader; +import org.alfresco.service.cmr.repository.ContentService; +import org.alfresco.service.cmr.repository.ContentWriter; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +/** + * Unit tests for the {@link RecordAspect}. + * + * @author Claudia Agache + */ +public class RecordAspectUnitTest +{ + private static final NodeRef NODE_REF = new NodeRef("node://Ref/"); + private static final NodeRef COPY_REF = new NodeRef("node://Copy/"); + private static final AssociationRef SOURCE_ASSOC_REF = new AssociationRef(COPY_REF, ContentModel.ASSOC_ORIGINAL, + NODE_REF); + private static final AssociationRef TARGET_ASSOC_REF = new AssociationRef(NODE_REF, ContentModel.ASSOC_ORIGINAL, + COPY_REF); + + @InjectMocks + private RecordAspect recordAspect; + @Mock + private NodeService mockNodeService; + @Mock + private BehaviourFilter mockBehaviorFilter; + @Mock + private ContentService mockContentService; + @Mock + private ContentReader mockContentReader; + @Mock + private ContentWriter mockContentWriter; + + @Before + public void setUp() + { + initMocks(this); + } + + /** Check that the bin is duplicated before adding the aspect if the file has a copy. */ + @Test + public void testDuplicateBinForFileWithCopy() + { + when(mockNodeService.getSourceAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(SOURCE_ASSOC_REF)); + when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader); + when(mockContentService.getWriter(NODE_REF, ContentModel.PROP_CONTENT, true)).thenReturn(mockContentWriter); + + recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD); + + verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + verify(mockContentService, times(1)).getReader(NODE_REF, ContentModel.PROP_CONTENT); + verify(mockContentService, times(1)).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true); + verify(mockContentWriter, times(1)).putContent(mockContentReader); + verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + } + + /** Check that the bin is duplicated before adding the aspect if the file is a copy. */ + @Test + public void testDuplicateBinForCopy() + { + when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(TARGET_ASSOC_REF)); + when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader); + when(mockContentService.getWriter(NODE_REF, ContentModel.PROP_CONTENT, true)).thenReturn(mockContentWriter); + + recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD); + + verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + verify(mockContentService, times(1)).getReader(NODE_REF, ContentModel.PROP_CONTENT); + verify(mockContentService, times(1)).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true); + verify(mockContentWriter, times(1)).putContent(mockContentReader); + verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + } + + /** Check that no content bin is created if the file does not have content. */ + @Test + public void testFileWithNoContent() + { + when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(TARGET_ASSOC_REF)); + when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(null); + + recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD); + + verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + verify(mockContentService, times(1)).getReader(NODE_REF, ContentModel.PROP_CONTENT); + verify(mockContentService, never()).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true); + verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + } +} From adbed03427f57b7c1e2ccf1738d825572ae3afb3 Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 18 Feb 2019 17:08:05 +0200 Subject: [PATCH 13/14] addressed code review comments --- .../rma/aspect/RecordAspectUnitTest.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java index 78c6263dbf..b11aa53026 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java @@ -37,6 +37,7 @@ import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.service.cmr.repository.AssociationRef; import org.alfresco.service.cmr.repository.ContentReader; @@ -75,6 +76,8 @@ public class RecordAspectUnitTest private ContentReader mockContentReader; @Mock private ContentWriter mockContentWriter; + @Mock + private ExtendedSecurityService mockExtendedSecurityService; @Before public void setUp() @@ -84,7 +87,7 @@ public class RecordAspectUnitTest /** Check that the bin is duplicated before adding the aspect if the file has a copy. */ @Test - public void testDuplicateBinForFileWithCopy() + public void testDuplicateBinBeforeAddingAspectForFileWithCopy() { when(mockNodeService.getSourceAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(SOURCE_ASSOC_REF)); when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader); @@ -103,7 +106,7 @@ public class RecordAspectUnitTest /** Check that the bin is duplicated before adding the aspect if the file is a copy. */ @Test - public void testDuplicateBinForCopy() + public void testDuplicateBinBeforeAddingAspectForCopy() { when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(TARGET_ASSOC_REF)); when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader); @@ -136,4 +139,21 @@ public class RecordAspectUnitTest verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); } + + /** Check that the bin is duplicated when copying a record. */ + @Test + public void testDuplicateBinWhenCopyingRecord() + { + when(mockNodeService.exists(COPY_REF)).thenReturn(true); + when(mockNodeService.hasAspect(COPY_REF, ASPECT_RECORD)).thenReturn(true); + when(mockContentService.getReader(COPY_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader); + when(mockContentService.getWriter(COPY_REF, ContentModel.PROP_CONTENT, true)).thenReturn(mockContentWriter); + + recordAspect.onCopyComplete(null, NODE_REF, COPY_REF, true, null); + + verify(mockExtendedSecurityService, times(1)).remove(COPY_REF); + verify(mockContentService, times(1)).getReader(COPY_REF, ContentModel.PROP_CONTENT); + verify(mockContentService, times(1)).getWriter(COPY_REF, ContentModel.PROP_CONTENT, true); + verify(mockContentWriter, times(1)).putContent(mockContentReader); + } } From 33570a80da9d9e80fec76207e5921a5d5ce1777a Mon Sep 17 00:00:00 2001 From: cagache Date: Tue, 19 Feb 2019 09:37:36 +0200 Subject: [PATCH 14/14] addressed code review comments --- .../rma/aspect/RecordAspectUnitTest.java | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java index b11aa53026..730da904f3 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java @@ -27,6 +27,7 @@ package org.alfresco.module.org_alfresco_module_rm.model.rma.aspect; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASPECT_RECORD; import static org.mockito.Matchers.eq; @@ -95,13 +96,7 @@ public class RecordAspectUnitTest recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD); - verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); - verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); - verify(mockContentService, times(1)).getReader(NODE_REF, ContentModel.PROP_CONTENT); - verify(mockContentService, times(1)).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true); - verify(mockContentWriter, times(1)).putContent(mockContentReader); - verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); - verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + verifyBeforeAddAspectMethodsInvocations(1); } /** Check that the bin is duplicated before adding the aspect if the file is a copy. */ @@ -114,18 +109,12 @@ public class RecordAspectUnitTest recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD); - verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); - verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); - verify(mockContentService, times(1)).getReader(NODE_REF, ContentModel.PROP_CONTENT); - verify(mockContentService, times(1)).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true); - verify(mockContentWriter, times(1)).putContent(mockContentReader); - verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); - verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + verifyBeforeAddAspectMethodsInvocations(1); } /** Check that no content bin is created if the file does not have content. */ @Test - public void testFileWithNoContent() + public void testBeforeAddAspectOnFileWithNoContent() { when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(TARGET_ASSOC_REF)); when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(null); @@ -140,6 +129,18 @@ public class RecordAspectUnitTest verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); } + /** Check that the bin is not duplicated before adding the aspect if the node has no copies. */ + @Test + public void testNotDuplicateBinForFileWithNoCopies() + { + when(mockNodeService.getSourceAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(emptyList()); + when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(emptyList()); + + recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD); + + verifyBeforeAddAspectMethodsInvocations(0); + } + /** Check that the bin is duplicated when copying a record. */ @Test public void testDuplicateBinWhenCopyingRecord() @@ -156,4 +157,20 @@ public class RecordAspectUnitTest verify(mockContentService, times(1)).getWriter(COPY_REF, ContentModel.PROP_CONTENT, true); verify(mockContentWriter, times(1)).putContent(mockContentReader); } + + /** + * Helper to verify beforeAddAspect methods invocations + * + * @param wantedNumberOfInvocations wanted number of invocations for each method + */ + private void verifyBeforeAddAspectMethodsInvocations(int wantedNumberOfInvocations) + { + verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + verify(mockContentService, times(wantedNumberOfInvocations)).getReader(NODE_REF, ContentModel.PROP_CONTENT); + verify(mockContentService, times(wantedNumberOfInvocations)).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true); + verify(mockContentWriter, times(wantedNumberOfInvocations)).putContent(mockContentReader); + verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE)); + verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE)); + } }