From 7b328dfa6ce9394facf5f49d92417af729b562e4 Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Wed, 16 Nov 2016 15:26:52 +0200 Subject: [PATCH 1/9] RM-4373 - made transfer and hold containers editable --- .../capability/rm-capabilities-fileplan-context.xml | 11 +++++++++++ .../capability/rm-capabilities-freeze-context.xml | 3 ++- .../capability/rm-capabilities-group-context.xml | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml index e8eeae8602..f5811bddb5 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml @@ -94,4 +94,15 @@ + + + + + + TRANSFER_CONTAINER + TRANSFER + + + \ No newline at end of file diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-freeze-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-freeze-context.xml index 5a1cceb841..3de1c78472 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-freeze-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-freeze-context.xml @@ -4,7 +4,7 @@ - + @@ -30,6 +30,7 @@ + HOLD_CONTAINER HOLD diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-group-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-group-context.xml index 91a50ef89d..87e4ea0edc 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-group-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-group-context.xml @@ -44,6 +44,7 @@ + @@ -62,6 +63,7 @@ + From c782ce305bb9852bf4eac1d0bb40920b6a50f3fd Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Wed, 16 Nov 2016 16:40:53 +0200 Subject: [PATCH 2/9] RM-4373 - added filling condition for editing transfer container and transfer nodes --- .../capability/rm-capabilities-fileplan-context.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml index f5811bddb5..1d5e045a92 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-fileplan-context.xml @@ -104,5 +104,10 @@ TRANSFER + + + + + \ No newline at end of file From 891f67782a109696203e013297d2152d75ee2e18 Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Wed, 16 Nov 2016 17:40:48 +0200 Subject: [PATCH 3/9] RM-4373 - make transfers non editable because the rule inheritance is disabled on transfer container --- .../messages/action-service.properties | 1 + .../rma/aspect/FilePlanComponentAspect.java | 4 +-- .../model/rma/type/TransferType.java | 36 ++++++++++++++++--- .../transfer/TransferServiceImpl.java | 2 ++ 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/action-service.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/action-service.properties index fe9a6d959e..3d36f0ec7c 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/action-service.properties +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/action-service.properties @@ -44,4 +44,5 @@ rm.action.multiple.children.type-error-message=Operation failed. Children of typ rm.action.create.transfer.container.child-error-message=Operation failed. Creation is not allowed in Transfer Container. rm.action.create.transfer.child-error-message=Operation failed. Creation is not allowed in Transfer Folders. rm.action.create.record.folder.child-error-message=Only records can be created in record folders but it was {0} +rm.action.transfer-non-editable=The metadata of transfer nodes is not editable. diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FilePlanComponentAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FilePlanComponentAspect.java index 4def09023c..7b9cae0f0c 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FilePlanComponentAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FilePlanComponentAspect.java @@ -129,7 +129,7 @@ public class FilePlanComponentAspect extends BaseBehaviourBean ) public void onUpdateProperties(final NodeRef nodeRef, final Map before, final Map after) { - AuthenticationUtil.runAs(new RunAsWork() + AuthenticationUtil.runAsSystem(new RunAsWork() { @Override public Void doWork() @@ -141,7 +141,7 @@ public class FilePlanComponentAspect extends BaseBehaviourBean return null; } - }, AuthenticationUtil.getAdminUserName()); + }); } /** diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/TransferType.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/TransferType.java index aa8be553e4..770f84ed91 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/TransferType.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/TransferType.java @@ -27,6 +27,9 @@ package org.alfresco.module.org_alfresco_module_rm.model.rma.type; +import java.io.Serializable; +import java.util.Map; + import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.node.integrity.IntegrityException; @@ -34,6 +37,8 @@ import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; import org.alfresco.service.cmr.repository.ChildAssociationRef; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.namespace.QName; import org.springframework.extensions.surf.util.I18NUtil; /** @@ -43,10 +48,15 @@ import org.springframework.extensions.surf.util.I18NUtil; * @since 2.6 */ @BehaviourBean(defaultType = "rma:transfer") -public class TransferType extends BaseBehaviourBean implements NodeServicePolicies.OnCreateChildAssociationPolicy +public class TransferType extends BaseBehaviourBean + implements NodeServicePolicies.OnCreateChildAssociationPolicy, + NodeServicePolicies.OnUpdatePropertiesPolicy { private final static String MSG_ERROR_ADD_CHILD_TO_TRANSFER = "rm.action.create.transfer.child-error-message"; - private static final String BEHAVIOUR_NAME = "onCreateChildAssocsForTransferType"; + private final static String MSG_ERROR_EDIT_TRANSFER_PROPERTIES = "rm.action.transfer-non-editable"; + + private static final String CREATE_CHILD_BEHAVIOUR_NAME = "onCreateChildAssocsForTransferType"; + private static final String EDIT_PROPERTIES_BEHAVIOUR_NAME = "onEditTransferProperties"; /** * Disable the behaviours for this transaction @@ -54,7 +64,8 @@ public class TransferType extends BaseBehaviourBean implements NodeServicePolici */ public void disable() { - getBehaviour(BEHAVIOUR_NAME).disable(); + getBehaviour(CREATE_CHILD_BEHAVIOUR_NAME).disable(); + getBehaviour(EDIT_PROPERTIES_BEHAVIOUR_NAME).disable(); } /** @@ -63,7 +74,8 @@ public class TransferType extends BaseBehaviourBean implements NodeServicePolici */ public void enable() { - getBehaviour(BEHAVIOUR_NAME).enable(); + getBehaviour(CREATE_CHILD_BEHAVIOUR_NAME).enable(); + getBehaviour(EDIT_PROPERTIES_BEHAVIOUR_NAME).enable(); } /** @@ -73,10 +85,24 @@ public class TransferType extends BaseBehaviourBean implements NodeServicePolici @Behaviour ( kind = BehaviourKind.ASSOCIATION, - name = BEHAVIOUR_NAME + name = CREATE_CHILD_BEHAVIOUR_NAME ) public void onCreateChildAssociation(ChildAssociationRef childAssocRef, boolean isNewNode) { throw new IntegrityException(I18NUtil.getMessage(MSG_ERROR_ADD_CHILD_TO_TRANSFER), null); } + + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + name = EDIT_PROPERTIES_BEHAVIOUR_NAME + ) + public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { + if(!authenticationUtil.isRunAsUserTheSystemUser()) + { + throw new IntegrityException(I18NUtil.getMessage(MSG_ERROR_EDIT_TRANSFER_PROPERTIES), null); + } + } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/transfer/TransferServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/transfer/TransferServiceImpl.java index be3775462f..b075a43904 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/transfer/TransferServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/transfer/TransferServiceImpl.java @@ -182,6 +182,7 @@ public class TransferServiceImpl extends ServiceBaseImpl NodeRef transferContainer = filePlanService.getTransferContainer(root); transferContainerType.disable(); + transferType.disable(); try { transferNodeRef = nodeService.createNode(transferContainer, @@ -194,6 +195,7 @@ public class TransferServiceImpl extends ServiceBaseImpl finally { transferContainerType.enable(); + transferType.enable(); } // Bind the hold node reference to the transaction AlfrescoTransactionSupport.bindResource(KEY_TRANSFER_NODEREF, transferNodeRef); From 45cbc77ef3ba05c27df6e77140c14f0f508c0299 Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Thu, 17 Nov 2016 11:07:14 +0200 Subject: [PATCH 4/9] Revert "Revert "Merge branch 'feature/RM-4357_AllowableOperations' into 'master'"" This reverts commit 54717418e1d3accb23eca355e4990cdf0d63f9b9. --- .../rm-public-rest-context.xml | 1 + .../rm/rest/api/impl/RMNodesImpl.java | 76 +++++++++++---- .../rm/rest/api/impl/RMNodesImplUnitTest.java | 97 +++++++++++++------ 3 files changed, 122 insertions(+), 52 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-rest-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-rest-context.xml index 2f8e766d91..2f92b31328 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-rest-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-rest-context.xml @@ -27,6 +27,7 @@ + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java index b127e2fbe1..20c0c2a9bd 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java @@ -37,6 +37,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.RecordsManagementServiceRegistry; +import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService; import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule; import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; @@ -64,6 +65,8 @@ import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; import org.alfresco.util.ParameterCheck; +import net.sf.acegisecurity.vote.AccessDecisionVoter; + /** * Centralizes access to the repository. * @@ -84,6 +87,7 @@ public class RMNodesImpl extends NodesImpl implements RMNodes private Repository repositoryHelper; private DictionaryService dictionaryService; private DispositionService dispositionService; + private CapabilityService capabilityService; /** * TODO to remove this after isSpecialNode is made protected in core implementation(REPO-1459) @@ -113,6 +117,11 @@ public class RMNodesImpl extends NodesImpl implements RMNodes this.filePlanService = filePlanService; } + public void setCapabilityService(CapabilityService capabilityService) + { + this.capabilityService = capabilityService; + } + @Override public Node getFolderOrDocument(final NodeRef nodeRef, NodeRef parentNodeRef, QName nodeTypeQName, List includeParam, Map mapUserInfo) { @@ -123,26 +132,6 @@ public class RMNodesImpl extends NodesImpl implements RMNodes nodeTypeQName = nodeService.getType(nodeRef); } - //TODO to remove this part of code after isSpecialNode will be made protected on core, will not need this anymore since the right allowed operations will be returned from core(REPO-1459). - if (includeParam.contains(PARAM_INCLUDE_ALLOWABLEOPERATIONS) && originalNode.getAllowableOperations() != null) - { - List allowableOperations = originalNode.getAllowableOperations(); - List modifiedAllowableOperations = new ArrayList<>(); - modifiedAllowableOperations.addAll(allowableOperations); - - for (String op : allowableOperations) - { - if (op.equals(OP_DELETE) && (isSpecialNode(nodeRef, nodeTypeQName))) - { - // special case: do not return "delete" (as an allowable op) for specific system nodes - modifiedAllowableOperations.remove(op); - } - } - - originalNode.setAllowableOperations((modifiedAllowableOperations.size() > 0 )? modifiedAllowableOperations : null); - } - - RMNodeType type = getType(nodeTypeQName, nodeRef); FileplanComponentNode node = null; if (mapUserInfo == null) @@ -193,9 +182,56 @@ public class RMNodesImpl extends NodesImpl implements RMNodes } } + if (includeParam.contains(PARAM_INCLUDE_ALLOWABLEOPERATIONS)) + { + node.setAllowableOperations(getAllowableOperations(nodeRef, type)); + } + return node; } + /** + * Helper method that generates allowable operation for the provided node + * @param nodeRef the node to get the allowable operations for + * @param type the type of the provided nodeRef + * @return a sublist of [{@link Nodes.OP_DELETE}, {@link Nodes.OP_CREATE}, {@link Nodes.OP_UPDATE}] representing the allowable operations for the provided node + */ + private List getAllowableOperations(NodeRef nodeRef, RMNodeType type) + { + List allowableOperations = new ArrayList<>(); + + NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); + boolean isFilePlan = nodeRef.equals(filePlan); + boolean isTransferContainer = nodeRef.equals(filePlanService.getTransferContainer(filePlan)); + boolean isUnfiledContainer = nodeRef.equals(filePlanService.getUnfiledContainer(filePlan)); + boolean isHoldsContainer = nodeRef.equals(filePlanService.getHoldContainer(filePlan)) ; + boolean isSpecialContainer = isFilePlan || isTransferContainer || isUnfiledContainer || isHoldsContainer; + + // DELETE + if(!isSpecialContainer && + capabilityService.getCapability("Delete").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED) + { + allowableOperations.add(OP_DELETE); + } + + // CREATE + if(type != RMNodeType.FILE && + !isFilePlan && + !isTransferContainer && + capabilityService.getCapability("FillingPermissionOnly").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED) + { + allowableOperations.add(OP_CREATE); + } + + // UPDATE + if (capabilityService.getCapability("Update").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED) + { + allowableOperations.add(OP_UPDATE); + } + + return allowableOperations; + } + @Override public NodeRef validateNode(String nodeId) { diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java index 3ae63a99c2..abbb8e9aa7 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java @@ -30,7 +30,6 @@ package org.alfresco.rm.rest.api.impl; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; @@ -45,6 +44,8 @@ import java.util.ArrayList; import java.util.List; import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.capability.Capability; +import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService; import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.module.org_alfresco_module_rm.test.util.AlfMock; @@ -74,6 +75,8 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import net.sf.acegisecurity.vote.AccessDecisionVoter; + /** * Unit Test class for RMNodesImpl. * @@ -103,10 +106,17 @@ public class RMNodesImplUnitTest extends BaseUnitTest @Mock private ServiceRegistry mockedServiceRegistry; + + @Mock + private CapabilityService mockedCapabilityService; @InjectMocks private RMNodesImpl rmNodesImpl; + private Capability deleteCapability; + private Capability createCapability; + private Capability updateCapability; + @Before public void before() { @@ -117,6 +127,12 @@ public class RMNodesImplUnitTest extends BaseUnitTest when(mockedNamespaceService.getPrefixes(any(String.class))).thenReturn(prefixes); when(mockedNamespaceService.getNamespaceURI(any(String.class))).thenReturn(RM_URI); + deleteCapability = mock(Capability.class); + when(mockedCapabilityService.getCapability("Delete")).thenReturn(deleteCapability); + createCapability = mock(Capability.class); + when(mockedCapabilityService.getCapability("FillingPermissionOnly")).thenReturn(createCapability); + updateCapability = mock(Capability.class); + when(mockedCapabilityService.getCapability("Update")).thenReturn(updateCapability); } @Test @@ -160,19 +176,13 @@ public class RMNodesImplUnitTest extends BaseUnitTest setPermissions(nodeRef, AccessStatus.ALLOWED); + when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(mockedFilePlanService.getFilePlanBySiteId(RM_SITE_ID)).thenReturn(nodeRef); Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null); - assertNotNull(folderOrDocument); - assertTrue(FileplanComponentNode.class.isInstance(folderOrDocument)); - - FileplanComponentNode resultNode = (FileplanComponentNode) folderOrDocument; - assertEquals(false, resultNode.getIsRecordFolder()); - assertEquals(false, resultNode.getIsFile()); - assertEquals(false, resultNode.getIsCategory()); - List allowableOperations = resultNode.getAllowableOperations(); - assertTrue("Create operation should be available for FilePlan.", allowableOperations.contains(RMNodes.OP_CREATE)); - assertTrue("Update operation should be available for FilePlan.", allowableOperations.contains(RMNodes.OP_UPDATE)); - assertFalse("Delete operation should note be available for FilePlan.", allowableOperations.contains(RMNodes.OP_DELETE)); + checksAllowedOperations(folderOrDocument, false, true, false); } @Test @@ -203,15 +213,15 @@ public class RMNodesImplUnitTest extends BaseUnitTest assertEquals(false, resultNode.getIsFile()); assertEquals(false, resultNode.getIsCategory()); List allowableOperations = resultNode.getAllowableOperations(); - assertNull(allowableOperations); + assertEquals(0, allowableOperations.size()); } @Test public void testGetTransferContainerAllowableOperations() throws Exception { NodeRef nodeRef = AlfMock.generateNodeRef(mockedNodeService); - when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_RECORD_CATEGORY); - when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_RECORD_CATEGORY, ContentModel.TYPE_FOLDER)).thenReturn(true); + when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_TRANSFER_CONTAINER); + when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_TRANSFER_CONTAINER, ContentModel.TYPE_FOLDER)).thenReturn(true); setupCompanyHomeAndPrimaryParent(nodeRef); @@ -224,16 +234,22 @@ public class RMNodesImplUnitTest extends BaseUnitTest when(mockedFilePlanService.getFilePlanBySiteId(RM_SITE_ID)).thenReturn(filePlanNodeRef); when(mockedFilePlanService.getTransferContainer(filePlanNodeRef)).thenReturn(nodeRef); + when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + + when(mockedFilePlanService.isFilePlanComponent(nodeRef)).thenReturn(true); + Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null); - checkSpecialContainersAllowedOperations(folderOrDocument); + checksAllowedOperations(folderOrDocument, false, true, false); } @Test public void testGetHoldContainerAllowableOperations() throws Exception { NodeRef nodeRef = AlfMock.generateNodeRef(mockedNodeService); - when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_RECORD_CATEGORY); - when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_RECORD_CATEGORY, ContentModel.TYPE_FOLDER)).thenReturn(true); + when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_HOLD_CONTAINER); + when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_HOLD_CONTAINER, ContentModel.TYPE_FOLDER)).thenReturn(true); setupCompanyHomeAndPrimaryParent(nodeRef); @@ -250,16 +266,22 @@ public class RMNodesImplUnitTest extends BaseUnitTest when(mockedFilePlanService.getHoldContainer(filePlanNodeRef)).thenReturn(nodeRef); + when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + + when(mockedFilePlanService.isFilePlanComponent(nodeRef)).thenReturn(true); + Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null); - checkSpecialContainersAllowedOperations(folderOrDocument); + checksAllowedOperations(folderOrDocument, true, true, false); } @Test public void testGetUnfiledContainerAllowableOperations() throws Exception { NodeRef nodeRef = AlfMock.generateNodeRef(mockedNodeService); - when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_RECORD_CATEGORY); - when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_RECORD_CATEGORY, ContentModel.TYPE_FOLDER)).thenReturn(true); + when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_UNFILED_RECORD_CONTAINER); + when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_UNFILED_RECORD_CONTAINER, ContentModel.TYPE_FOLDER)).thenReturn(true); setupCompanyHomeAndPrimaryParent(nodeRef); @@ -279,8 +301,14 @@ public class RMNodesImplUnitTest extends BaseUnitTest when(mockedFilePlanService.getUnfiledContainer(filePlanNodeRef)).thenReturn(nodeRef); + when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED); + + when(mockedFilePlanService.isFilePlanComponent(nodeRef)).thenReturn(true); + Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null); - checkSpecialContainersAllowedOperations(folderOrDocument); + checksAllowedOperations(folderOrDocument, true, true, false); } @Test @@ -741,18 +769,23 @@ public class RMNodesImplUnitTest extends BaseUnitTest when(mockedPermissionService.hasPermission(nodeRef, PermissionService.ADD_CHILDREN)).thenReturn(permissionToSet); } - private void checkSpecialContainersAllowedOperations(Node containerNode) + private void checksAllowedOperations(Node containerNode, boolean allowCreate, boolean allowUpdate, boolean allowDelete) { assertNotNull(containerNode); - assertTrue(RecordCategoryNode.class.isInstance(containerNode)); - - RecordCategoryNode resultNode = (RecordCategoryNode) containerNode; - assertEquals(false, resultNode.getIsRecordFolder()); - assertEquals(false, resultNode.getIsFile()); - assertEquals(true, resultNode.getIsCategory()); + assertTrue(FileplanComponentNode.class.isInstance(containerNode)); + FileplanComponentNode resultNode = (FileplanComponentNode) containerNode; List allowableOperations = resultNode.getAllowableOperations(); - assertTrue("Create operation should be available for provided container.", allowableOperations.contains(RMNodes.OP_CREATE)); - assertTrue("Update operation should be available for provided container.", allowableOperations.contains(RMNodes.OP_UPDATE)); - assertFalse("Delete operation should note be available for provided container.", allowableOperations.contains(RMNodes.OP_DELETE)); + + assertEquals("Create operation should " + (allowCreate?"":"not ") + "be available for provided container.", + allowCreate, + allowableOperations.contains(RMNodes.OP_CREATE)); + + assertEquals("Update operation should " + (allowCreate?"":"not ") + "be available for provided container.", + allowUpdate, + allowableOperations.contains(RMNodes.OP_UPDATE)); + + assertEquals("Delete operation should " + (allowCreate?"":"not ") + "be available for provided container.", + allowDelete, + allowableOperations.contains(RMNodes.OP_DELETE)); } } From a4dafd56b0804f5d5efccb357aea1dc947293418 Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Thu, 17 Nov 2016 11:07:21 +0200 Subject: [PATCH 5/9] Revert "Revert "Merge branch 'feature/RM-4357_AllowableOperations' into 'master'"" This reverts commit ba9807118c3d8fa3d2d53b7330b357aa497568dc. --- .../source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java index 20c0c2a9bd..b5bd4267f3 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java @@ -216,7 +216,6 @@ public class RMNodesImpl extends NodesImpl implements RMNodes // CREATE if(type != RMNodeType.FILE && - !isFilePlan && !isTransferContainer && capabilityService.getCapability("FillingPermissionOnly").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED) { From d10fcea5fa52b9c613b975af2534af602d990a0f Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Thu, 17 Nov 2016 11:38:17 +0200 Subject: [PATCH 6/9] RM-4357 - delete unused methods after merge --- .../rm/rest/api/impl/RMNodesImpl.java | 81 ------------------- 1 file changed, 81 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java index 9e61d8c8cc..4ee4692c0e 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java @@ -332,85 +332,4 @@ public class RMNodesImpl extends NodesImpl implements RMNodes return new Pair<>(searchTypeQNames, ignoreAspectQNames); } - - /** - * TODO only override core method when will be made protected in core(REPO-1459). - * - * @param nodeRef - * @param type - * @return - */ - private boolean isSpecialNode(NodeRef nodeRef, QName type) - { - // Check for File Plan, Transfers Container, Holds Container, Unfiled Records Container - NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); - if(filePlan != null) - { - if(filePlan.equals(nodeRef)) - { - return true; - } - else if(filePlanService.getTransferContainer(filePlan).equals(nodeRef)) - { - return true; - } - else if(filePlanService.getHoldContainer(filePlan).equals(nodeRef)) - { - return true; - } - else if(filePlanService.getUnfiledContainer(filePlan).equals(nodeRef)) - { - return true; - } - } - //TODO just run super after method after it will be made protected on core(REPO-1459) - return isCoreSpecialNode(nodeRef, type); - } - - /** - * Copied from core implementation, because it is protected and we can't extend it with our special nodes. - * - * TODO to remove when isSpecialNode method will be made protected in core (REPO-1459). - * - * @param nodeRef - * @param type - * @return - */ - private boolean isCoreSpecialNode(NodeRef nodeRef, QName type) - { - // Check for Company Home, Sites and Data Dictionary (note: must be tenant-aware) - if (nodeRef.equals(repositoryHelper.getCompanyHome())) - { - return true; - } - else if (type.equals(SiteModel.TYPE_SITES) || type.equals(SiteModel.TYPE_SITE)) - { - // note: alternatively, we could inject SiteServiceInternal and use getSitesRoot (or indirectly via node locator) - return true; - } - else - { - String tenantDomain = TenantUtil.getCurrentDomain(); - NodeRef ddNodeRef = ddCache.get(tenantDomain); - if (ddNodeRef == null) - { - List ddAssocs = nodeService.getChildAssocs( - repositoryHelper.getCompanyHome(), - ContentModel.ASSOC_CONTAINS, - QName.createQName(NamespaceService.APP_MODEL_1_0_URI, "dictionary")); - if (ddAssocs.size() == 1) - { - ddNodeRef = ddAssocs.get(0).getChildRef(); - ddCache.put(tenantDomain, ddNodeRef); - } - } - - if (nodeRef.equals(ddNodeRef)) - { - return true; - } - } - - return false; - } } From c8f6bfe33373de71ce1eba5bdeac2b84e6599568 Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Thu, 17 Nov 2016 13:01:25 +0200 Subject: [PATCH 7/9] RM-4357 - updated test for fileplan create operation check --- .../java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java index abda9f8a2b..700e420b25 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java @@ -177,7 +177,7 @@ public class RMNodesImplUnitTest extends BaseUnitTest when(mockedFilePlanService.getFilePlanBySiteId(RM_SITE_ID)).thenReturn(nodeRef); Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null); - checksAllowedOperations(folderOrDocument, false, true, false); + checksAllowedOperations(folderOrDocument, true, true, false); } @Test From d42dc63664424ea12ed60ed7606897856e4b32bc Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Thu, 17 Nov 2016 13:50:09 +0200 Subject: [PATCH 8/9] RM-4357 - fixed allowable operations automation test --- .../rest/rm/fileplancomponents/FilePlanTests.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/fileplancomponents/FilePlanTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/fileplancomponents/FilePlanTests.java index ebc396e80f..425963fced 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/fileplancomponents/FilePlanTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/fileplancomponents/FilePlanTests.java @@ -17,6 +17,7 @@ import static org.alfresco.rest.rm.base.AllowableOperations.CREATE; import static org.alfresco.rest.rm.base.AllowableOperations.DELETE; import static org.alfresco.rest.rm.base.AllowableOperations.UPDATE; import static org.alfresco.rest.rm.model.fileplancomponents.FilePlanComponentAlias.FILE_PLAN_ALIAS; +import static org.alfresco.rest.rm.model.fileplancomponents.FilePlanComponentAlias.TRANSFERS_ALIAS; import static org.alfresco.rest.rm.model.fileplancomponents.FilePlanComponentFields.NAME; import static org.alfresco.rest.rm.model.fileplancomponents.FilePlanComponentFields.NODE_TYPE; import static org.alfresco.rest.rm.model.fileplancomponents.FilePlanComponentFields.PROPERTIES; @@ -147,8 +148,16 @@ public class FilePlanTests extends BaseRestTest FilePlanComponent filePlanComponent = filePlanComponentAPI.withParams("include=allowableOperations").getFilePlanComponent(specialContainerAlias); // Check the list of allowableOperations returned - assertTrue(filePlanComponent.getAllowableOperations().containsAll(asList(UPDATE, CREATE)), + if(specialContainerAlias.equals(TRANSFERS_ALIAS.toString())) + { + assertTrue(filePlanComponent.getAllowableOperations().containsAll(asList(UPDATE)), + "Wrong list of the allowable operations is return" + filePlanComponent.getAllowableOperations().toString()); + } + else + { + assertTrue(filePlanComponent.getAllowableOperations().containsAll(asList(UPDATE, CREATE)), "Wrong list of the allowable operations is return" + filePlanComponent.getAllowableOperations().toString()); + } // Check the list of allowableOperations doesn't contains DELETE operation assertFalse(filePlanComponent.getAllowableOperations().contains(DELETE), From 28026941c3786e781414ae10c0a2fb79a86843b5 Mon Sep 17 00:00:00 2001 From: Ana Bozianu Date: Thu, 17 Nov 2016 14:50:17 +0200 Subject: [PATCH 9/9] RM-4357 - return null when allowableOperations is empty --- .../rm/rest/api/impl/RMNodesImpl.java | 21 +++---------------- .../rm/rest/api/impl/RMNodesImplUnitTest.java | 4 ++-- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java index 4ee4692c0e..1ef80777c3 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/rm/rest/api/impl/RMNodesImpl.java @@ -33,7 +33,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.RecordsManagementServiceRegistry; @@ -42,9 +41,6 @@ import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedul import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; -import org.alfresco.repo.model.Repository; -import org.alfresco.repo.site.SiteModel; -import org.alfresco.repo.tenant.TenantUtil; import org.alfresco.rest.api.impl.NodesImpl; import org.alfresco.rest.api.model.Node; import org.alfresco.rest.api.model.UserInfo; @@ -55,10 +51,8 @@ import org.alfresco.rm.rest.api.model.RecordCategoryNode; import org.alfresco.rm.rest.api.model.RecordFolderNode; import org.alfresco.rm.rest.api.model.RecordNode; import org.alfresco.service.cmr.dictionary.DictionaryService; -import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; -import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; import org.alfresco.util.ParameterCheck; @@ -82,15 +76,10 @@ public class RMNodesImpl extends NodesImpl implements RMNodes private FilePlanService filePlanService; private NodeService nodeService; private RecordsManagementServiceRegistry serviceRegistry; - private Repository repositoryHelper; private DictionaryService dictionaryService; private DispositionService dispositionService; private CapabilityService capabilityService; - /** - * TODO to remove this after isSpecialNode is made protected in core implementation(REPO-1459) - */ - private ConcurrentHashMap ddCache = new ConcurrentHashMap<>(); public void init() { super.init(); @@ -104,12 +93,6 @@ public class RMNodesImpl extends NodesImpl implements RMNodes this.serviceRegistry = serviceRegistry; } - public void setRepositoryHelper(Repository repositoryHelper) - { - super.setRepositoryHelper(repositoryHelper); - this.repositoryHelper = repositoryHelper; - } - public void setFilePlanService(FilePlanService filePlanService) { this.filePlanService = filePlanService; @@ -182,7 +165,9 @@ public class RMNodesImpl extends NodesImpl implements RMNodes if (includeParam.contains(PARAM_INCLUDE_ALLOWABLEOPERATIONS)) { - node.setAllowableOperations(getAllowableOperations(nodeRef, type)); + // If the user does not have any of the mapped permissions then "allowableOperations" is not returned (rather than an empty array) + List allowableOperations = getAllowableOperations(nodeRef, type); + node.setAllowableOperations((allowableOperations.size() > 0 )? allowableOperations : null); } return node; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java index 700e420b25..f3fbf7ec29 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/rm/rest/api/impl/RMNodesImplUnitTest.java @@ -28,8 +28,8 @@ package org.alfresco.rm.rest.api.impl; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; @@ -208,7 +208,7 @@ public class RMNodesImplUnitTest extends BaseUnitTest assertEquals(false, resultNode.getIsFile()); assertEquals(false, resultNode.getIsCategory()); List allowableOperations = resultNode.getAllowableOperations(); - assertEquals(0, allowableOperations.size()); + assertNull(allowableOperations); } @Test