From c27b7b60a9d165931a87f63faf8ca9cc023b7632 Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Tue, 9 Aug 2016 12:01:16 +1000 Subject: [PATCH] RM-3074: Rename set/remove methods on extended security interface * create deprecated service interface to tidy things up * create set method and deprecate exisiting * crate remove method and deprecate exisiting * remove deprecation warnings in code --- .../model/rma/aspect/RecordAspect.java | 2 +- .../record/InplaceRecordServiceImpl.java | 2 +- .../record/RecordServiceImpl.java | 4 +- .../security/ExtendedSecurityService.java | 89 +++---------------- .../security/ExtendedSecurityServiceImpl.java | 63 +++++++++---- .../version/RecordableVersionServiceImpl.java | 2 +- .../ExtendedSecurityServiceImplTest.java | 8 +- .../legacy/service/RecordServiceImplTest.java | 2 +- .../ExtendedSecurityServiceImplUnitTest.java | 12 +-- 9 files changed, 73 insertions(+), 111 deletions(-) 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 08a2d66172..95d7e939fc 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 @@ -136,7 +136,7 @@ public class RecordAspect extends AbstractDisposableItem Set writers = extendedSecurityService.getExtendedWriters(parent); if (readers != null && readers.size() != 0) { - extendedSecurityService.addExtendedSecurity(thumbnail, readers, writers); + extendedSecurityService.set(thumbnail, readers, writers); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/InplaceRecordServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/InplaceRecordServiceImpl.java index 929eddcb04..964da2d9a6 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/InplaceRecordServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/InplaceRecordServiceImpl.java @@ -117,7 +117,7 @@ public class InplaceRecordServiceImpl extends ServiceBaseImpl implements Inplace // remove the extended security from the node // this prevents the users from continuing to see the record in searchs and other linked locations - extendedSecurityService.removeAllExtendedSecurity(nodeRef); + extendedSecurityService.remove(nodeRef); return null; } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java index 3ca29bf958..1756b09a90 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java @@ -940,7 +940,7 @@ public class RecordServiceImpl extends BaseBehaviourBean nodeService.addChild(parentAssoc.getParentRef(), nodeRef, parentAssoc.getTypeQName(), parentAssoc.getQName()); // set the extended security - extendedSecurityService.addExtendedSecurity(nodeRef, readersAndWriters.getFirst(), readersAndWriters.getSecond()); + extendedSecurityService.set(nodeRef, readersAndWriters); } finally { @@ -1042,7 +1042,7 @@ public class RecordServiceImpl extends BaseBehaviourBean } // set extended security on record - extendedSecurityService.addExtendedSecurity(record, readersAndWriters.getFirst(), readersAndWriters.getSecond()); + extendedSecurityService.set(record, readersAndWriters); return record; } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityService.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityService.java index 4960749eee..5b3b76b3ba 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityService.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityService.java @@ -31,6 +31,7 @@ import java.util.Set; import org.alfresco.api.AlfrescoPublicApi; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.util.Pair; /** * Extended security service. @@ -39,8 +40,9 @@ import org.alfresco.service.cmr.repository.NodeRef; * @since 2.1 */ @AlfrescoPublicApi -public interface ExtendedSecurityService +public interface ExtendedSecurityService extends DeprecatedExtendedSecurityService { + /** IPR group prefix */ static final String IPR_GROUP_PREFIX = "IPR"; /** @@ -66,90 +68,25 @@ public interface ExtendedSecurityService * @return {@link Set}<{@link String}> set of extended writers */ Set getExtendedWriters(NodeRef nodeRef); - - /** - * Add extended security for the specified authorities to a node. - * - * As of, 2.5 this method no longer applies the extended security to parents. - * - * @param nodeRef node reference - * @param readers set of authorities to add extended read permissions - * @param writers set of authorities to add extended write permissions - * - */ - // TODO rename to setExtendedSecurity to reflect that this doesn't update the extended security any more - void addExtendedSecurity(NodeRef nodeRef, Set readers, Set writers); - - /** - * Remove all extended readers and writers from the given node reference. - * - * @param nodeRef node reference - */ - // TODO rename to removeExtendedSecurity - void removeAllExtendedSecurity(NodeRef nodeRef); /** - * Add extended security for the specified authorities to a node. - *

- * If specified, the read and write extended permissions are applied to all parents up to the file plan as - * extended read. This ensures parental read, but not parental write. - * - * @param nodeRef node reference - * @param readers set of authorities to add extended read permissions - * @param writers set of authorities to add extended write permissions - * @param applyToParents true if extended security applied to parents (read only) false otherwise. * - * @deprecated as of 2.5, because extended security is no longer applied to parents. Note that calling this method will - * only apply the extended security to the node and the applyToParents parameter value will be ignored. - * - * @see #addExtendedSecurity(NodeRef, Set, Set) + * @param nodeRef + * @param readersAndWriters */ - @Deprecated void addExtendedSecurity(NodeRef nodeRef, Set readers, Set writers, boolean applyToParents); + void set(NodeRef nodeRef, Pair, Set> readersAndWriters); /** - * Remove the extended security for the specified authorities from a node. - * - * @param nodeRef node reference - * @param readers set of authorities to remove as extended readers - * @param writers set of authorities to remove as extended writers * - * @deprecated as of 2.5, because partial removal of readers and writers from node or parents is no longer supported. - * Note that calling this method will now remove all extended security from the node and never applied to parents. - * - * @see #removeAllExtendedSecurity(NodeRef) + * @param nodeRef + * @param readers + * @param writers */ - @Deprecated void removeExtendedSecurity(NodeRef nodeRef, Set readers, Set writers); - + void set(NodeRef nodeRef, Set readers, Set writers); + /** - * Remove the extended security for the specified authorities from a node. - *

- * If specified, extended security will also be removed from the parent hierarchy.(read only). Note that - * extended security is records as a reference count, so security will only be utterly removed from the parent - * hierarchy if all references to the authority are removed. - * - * @param nodeRef node reference - * @param readers set of authorities to remove as extended readers - * @param writers set of authorities to remove as extedned writers - * @param applyToParents true if removal of extended security is applied to parent hierarchy (read only), false - * otherwise * - * @deprecated as of 2.5, because partial removal of readers and writers from node or parents is no longer supported. - * Note that calling this method will now remove all extended security from the node and never applied to parents. - * - * @see #removeAllExtendedSecurity(NodeRef) + * @param nodeRef */ - @Deprecated void removeExtendedSecurity(NodeRef nodeRef, Set readers, Set writers, boolean applyToParents); - - /** - * Remove all extended readers and writers from the given node reference. - * - * @param nodeRef node reference - * @param applyToParents if true then apply removal to parent hierarchy (read only) false otherwise. - * - * @deprecated as of 2.5, because partial removal of readers and writers from node or parents is no longer supported. - * Note that calling this method will now remove all extended security from the node and never applied to parents. - * - * @see #removeAllExtendedSecurity(NodeRef) - */ - @Deprecated void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents); + void remove(NodeRef nodeRef); } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java index 169c25b03b..46e8d559a5 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java @@ -215,12 +215,21 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl return result; } - + /** - * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set) + * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#set(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.util.Pair) */ @Override - public void addExtendedSecurity(NodeRef nodeRef, Set readers, Set writers) + public void set(NodeRef nodeRef, Pair, Set> readersAndWriters) + { + set(nodeRef, readersAndWriters.getFirst(), readersAndWriters.getSecond()); + } + + /** + * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#set(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set) + */ + @Override + public void set(NodeRef nodeRef, Set readers, Set writers) { ParameterCheck.mandatory("nodeRef", nodeRef); @@ -228,7 +237,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl // TODO need to clear existing groups and add new ones // add extended security impl - addExtendedSecurityImpl(nodeRef, readers, writers); + setImpl(nodeRef, readers, writers); } /** @@ -238,7 +247,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl * @param readers readers set * @param writers writers set */ - private void addExtendedSecurityImpl(final NodeRef nodeRef, Set readers, Set writers) + private void setImpl(final NodeRef nodeRef, Set readers, Set writers) { ParameterCheck.mandatory("nodeRef", nodeRef); @@ -540,14 +549,14 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl } /** - * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef) + * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#remove(org.alfresco.service.cmr.repository.NodeRef) */ @Override - public void removeAllExtendedSecurity(NodeRef nodeRef) + public void remove(NodeRef nodeRef) { if (hasExtendedSecurity(nodeRef)) { - removeExtendedSecurityImpl(nodeRef); + removeImpl(nodeRef); // remove the readers from any renditions of the content if (isRecord(nodeRef)) @@ -556,7 +565,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl for (ChildAssociationRef assoc : assocs) { NodeRef child = assoc.getChildRef(); - removeExtendedSecurityImpl(child); + removeImpl(child); } } } @@ -567,7 +576,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl * * @param nodeRef node reference */ - private void removeExtendedSecurityImpl(NodeRef nodeRef) + private void removeImpl(NodeRef nodeRef) { ParameterCheck.mandatory("nodeRef", nodeRef); @@ -580,37 +589,53 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl // TODO delete the groups if they are no longer in use (easier said than done perhaps!) } - } + } /** - * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean) + * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set) + */ + @Override @Deprecated public void addExtendedSecurity(NodeRef nodeRef, Set readers, Set writers) + { + set(nodeRef, readers, writers); + } + + /** + * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean) */ @Override @Deprecated public void addExtendedSecurity(NodeRef nodeRef, Set readers, Set writers, boolean applyToParents) { - addExtendedSecurity(nodeRef, readers, writers); + set(nodeRef, readers, writers); } /** - * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set) + * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef) + */ + @Override @Deprecated public void removeAllExtendedSecurity(NodeRef nodeRef) + { + remove(nodeRef); + } + + /** + * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set) */ @Override @Deprecated public void removeExtendedSecurity(NodeRef nodeRef, Set readers, Set writers) { - removeAllExtendedSecurity(nodeRef); + remove(nodeRef); } /** - * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean) + * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean) */ @Override @Deprecated public void removeExtendedSecurity(NodeRef nodeRef, Set readers, Setwriters, boolean applyToParents) { - removeAllExtendedSecurity(nodeRef); + remove(nodeRef); } /** - * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, boolean) + * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, boolean) */ @Override @Deprecated public void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents) { - removeAllExtendedSecurity(nodeRef); + remove(nodeRef); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java index 7f96526ce4..0b714bc9da 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java @@ -735,7 +735,7 @@ public class RecordableVersionServiceImpl extends Version2ServiceImpl linkToPreviousVersionRecord(nodeRef, record); // set the extended security - extendedSecurityService.addExtendedSecurity(record, readersAndWriters.getFirst(), readersAndWriters.getSecond()); + extendedSecurityService.set(record, readersAndWriters); return record; } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java index e89963e833..f874e0d671 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java @@ -106,18 +106,18 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase extendedReaders.add(monkey); extendedReaders.add(elephant); - extendedSecurityService.addExtendedSecurity(record, extendedReaders, null); + extendedSecurityService.set(record, extendedReaders, null); checkExtendedReaders(record, extendedReaders); Set extendedReadersToo = new HashSet(2); extendedReadersToo.add(monkey); extendedReadersToo.add(snake); - extendedSecurityService.addExtendedSecurity(recordToo, extendedReadersToo, null); + extendedSecurityService.set(recordToo, extendedReadersToo, null); checkExtendedReaders(recordToo, extendedReadersToo); // test remove - extendedSecurityService.removeAllExtendedSecurity(recordToo); + extendedSecurityService.remove(recordToo); assertFalse(extendedSecurityService.hasExtendedSecurity(recordToo)); assertTrue(extendedSecurityService.getExtendedReaders(recordToo).isEmpty()); @@ -151,7 +151,7 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase assertTrue(extendedSecurityService.getExtendedReaders(record).isEmpty()); - extendedSecurityService.addExtendedSecurity(record, extendedReaders, null); + extendedSecurityService.set(record, extendedReaders, null); checkExtendedReaders(record, extendedReaders); assertFalse(extendedSecurityService.hasExtendedSecurity(moveRecordCategory)); diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordServiceImplTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordServiceImplTest.java index bf2366335e..fcff2a59da 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordServiceImplTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordServiceImplTest.java @@ -212,7 +212,7 @@ public class RecordServiceImplTest extends BaseRMTestCase { Set writers = new HashSet(1); writers.add(dmCollaborator); - extendedSecurityService.addExtendedSecurity(recordOne, null, writers); + extendedSecurityService.set(recordOne, null, writers); assertTrue(extendedSecurityService.getExtendedReaders(recordOne).isEmpty()); assertFalse(extendedSecurityService.getExtendedWriters(recordOne).isEmpty()); diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java index 4d782c1fb8..fc7bd8a7fa 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java @@ -402,7 +402,7 @@ public class ExtendedSecurityServiceImplUnitTest .thenReturn(mockedReadPagingResults); // add extended security - extendedSecurityService.addExtendedSecurity(nodeRef, READERS, WRITERS); + extendedSecurityService.set(nodeRef, READERS, WRITERS); // verify read group created correctly verify(mockedAuthorityService).createAuthority(AuthorityType.GROUP, readGroup, readGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); @@ -477,7 +477,7 @@ public class ExtendedSecurityServiceImplUnitTest .collect(Collectors.toSet())); // add extended security - extendedSecurityService.addExtendedSecurity(nodeRef, READERS, WRITERS); + extendedSecurityService.set(nodeRef, READERS, WRITERS); // verify read group is not recreated verify(mockedAuthorityService, never()).createAuthority(AuthorityType.GROUP, readGroup, readGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); @@ -553,7 +553,7 @@ public class ExtendedSecurityServiceImplUnitTest .collect(Collectors.toSet())); // add extended security - extendedSecurityService.addExtendedSecurity(nodeRef, READERS, WRITERS); + extendedSecurityService.set(nodeRef, READERS, WRITERS); // new group names readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, 1); @@ -641,7 +641,7 @@ public class ExtendedSecurityServiceImplUnitTest .collect(Collectors.toSet())); // add extended security - extendedSecurityService.addExtendedSecurity(nodeRef, READERS, WRITERS); + extendedSecurityService.set(nodeRef, READERS, WRITERS); // verify read group is not recreated verify(mockedAuthorityService, never()).createAuthority(AuthorityType.GROUP, readGroup, readGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); @@ -702,7 +702,7 @@ public class ExtendedSecurityServiceImplUnitTest .thenReturn(permissions); // remove extended security - extendedSecurityService.removeAllExtendedSecurity(nodeRef); + extendedSecurityService.remove(nodeRef); // verify that the groups permissions have been removed verify(mockedPermissionService).clearPermission(nodeRef, readGroup); @@ -720,7 +720,7 @@ public class ExtendedSecurityServiceImplUnitTest .thenReturn(HAS_NO_EXTENDED_SECURITY); // remove extended security - extendedSecurityService.removeAllExtendedSecurity(nodeRef); + extendedSecurityService.remove(nodeRef); // verify that the groups permissions have been removed verify(mockedPermissionService, never()).clearPermission(eq(nodeRef), anyString());