From 90e1522a56fc6bf659d145c78fd43baec4b6c573 Mon Sep 17 00:00:00 2001 From: Eva Vasques Date: Thu, 11 Sep 2025 12:05:48 +0100 Subject: [PATCH] ACS-10042 - AGS Concurrent IPR Group Creation (#3566) --- .../security/ExtendedSecurityServiceImpl.java | 5 +- .../ExtendedSecurityServiceImplTest.java | 246 +++++++++++++++++- 2 files changed, 248 insertions(+), 3 deletions(-) diff --git a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java index 8eda4f182d..79d7ae8f88 100644 --- a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java +++ b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java @@ -36,6 +36,7 @@ import java.util.Set; import org.springframework.context.ApplicationListener; import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.dao.ConcurrencyFailureException; import org.springframework.extensions.webscripts.ui.common.StringUtils; import org.alfresco.model.ContentModel; @@ -649,8 +650,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl } catch (DuplicateChildNodeNameException ex) { - // the group was concurrently created - group = authorityService.getName(AuthorityType.GROUP, groupShortName); + // Rethrow as ConcurrencyFailureException so that is can be retried and linked to the group created by the concurrent transaction + throw new ConcurrencyFailureException("IPR group creation failed due to concurrent duplicate group name creation: " + groupShortName); } return group; diff --git a/amps/ags/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java b/amps/ags/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java index 86b4678874..ded252a135 100644 --- a/amps/ags/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java +++ b/amps/ags/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java @@ -29,14 +29,23 @@ package org.alfresco.module.org_alfresco_module_rm.test.legacy.service; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.CompletableFuture; + +import org.junit.Assert; +import org.springframework.dao.ConcurrencyFailureException; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.query.PagingRequest; +import org.alfresco.query.PagingResults; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.site.SiteModel; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.security.AccessPermission; import org.alfresco.service.cmr.security.AccessStatus; +import org.alfresco.service.cmr.security.AuthorityType; import org.alfresco.service.cmr.site.SiteService; import org.alfresco.service.cmr.site.SiteVisibility; import org.alfresco.util.GUID; @@ -206,7 +215,8 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase final NodeRef record = doTestInTransaction(new Test() { public NodeRef run() throws Exception { - NodeRef record = fileFolderService.create(documentLibrary, GUID.generate(), ContentModel.TYPE_CONTENT).getNodeRef(); + NodeRef record = fileFolderService.create(documentLibrary, GUID.generate(), ContentModel.TYPE_CONTENT) + .getNodeRef(); recordService.createRecord(filePlan, record); return record; } @@ -279,4 +289,238 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase } }); } + + public void testConcurrentSetWithRetry() + { + Set extendedReaders = new HashSet<>(2); + Set extendedWriters = new HashSet<>(2); + + Set documents = setupConcurrentTestCase(10, extendedReaders, extendedWriters); + + // For each record created previously, spawn a thread to set extended security so we cause concurrency + // failure trying to create IPR groups with the same name + fireParallelExecutionOfSetExtendedSecurity(documents, extendedReaders, extendedWriters, true); + + // Look for duplicated IPR groups and verify all documents have the same groups assigned + verifyCreatedGroups(documents, false); + + AuthenticationUtil.clearCurrentSecurityContext(); + } + + public void testConcurrentSetWithoutRetry() + { + Set extendedReaders = new HashSet<>(2); + Set extendedWriters = new HashSet<>(2); + + Set documents = setupConcurrentTestCase(10, extendedReaders, extendedWriters); + + // For each record created previously, spawn a thread to set extended security so we cause concurrency + // failure trying to create IPR groups with the same name. + // Since there is no retry, we expect to get a ConcurrencyFailureException + Assert.assertThrows(ConcurrencyFailureException.class, () -> { + fireParallelExecutionOfSetExtendedSecurity(documents, extendedReaders, extendedWriters, false); + }); + + // Look for duplicated IPR groups and verify all documents have the same groups assigned + // Since there was a ConcurrencyFailureException some threads failed to set extended security so some + // documents may not have IPR groups created. + verifyCreatedGroups(documents, true); + + AuthenticationUtil.clearCurrentSecurityContext(); + } + + private Set setupConcurrentTestCase(int concurrentThreads, Set extendedReaders, Set extendedWriters) + { + final String usera = createTestUser(); + final String userb = createTestUser(); + final String owner = createTestUser(); + + extendedReaders.add(usera); + extendedReaders.add(userb); + extendedWriters.add(usera); + extendedWriters.add(userb); + + AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser(); + + // Create a site + NodeRef documentLib = createSite(new HashSet<>(), new HashSet<>()); + + // Create records in the site document library + return createRecords(concurrentThreads, documentLib, owner); + } + + private NodeRef createSite(Set readers, Set writers) + { + return retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback() { + @Override + public NodeRef execute() throws Throwable + { + final String siteShortName = GUID.generate(); + siteService.createSite(null, siteShortName, "test", "test", SiteVisibility.PRIVATE); + readers.forEach(reader -> siteService.setMembership(siteShortName, reader, SiteModel.SITE_CONSUMER)); + writers.forEach(writer -> siteService.setMembership(siteShortName, writer, SiteModel.SITE_COLLABORATOR)); + return siteService.createContainer(siteShortName, SiteService.DOCUMENT_LIBRARY, null, null); + } + }, false, true); + } + + private Set createRecords(int numRecords, NodeRef parent, String owner) + { + return retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback>() { + @Override + public Set execute() throws Throwable + { + int createdRecords = 0; + Set documents = new HashSet<>(); + while (createdRecords < numRecords) + { + final NodeRef doc = fileFolderService.create(parent, GUID.generate(), ContentModel.TYPE_CONTENT).getNodeRef(); + ownableService.setOwner(doc, owner); + recordService.createRecord(filePlan, doc, rmFolder, true); + recordService.file(doc); + recordService.complete(doc); + documents.add(doc); + createdRecords++; + } + return documents; + } + }, false, true); + } + + private void setExtendedSecurity(NodeRef doc, Set readers, Set writers, boolean useRetry) + { + if (!useRetry) + { + setExtendedSecurity(doc, readers, writers); + return; + } + + retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback() { + @Override + public Void execute() throws Throwable + { + setExtendedSecurity(doc, readers, writers); + return null; + } + }, false, true); + } + + private void setExtendedSecurity(NodeRef doc, Set readers, Set writers) + { + AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser(); + extendedSecurityService.set(doc, readers, writers); + } + + private void fireParallelExecutionOfSetExtendedSecurity(Set documents, Set extendedReaders, Set extendedWriters, boolean useRetry) + { + CompletableFuture[] futures = documents.stream() + .map(doc -> CompletableFuture.runAsync(() -> setExtendedSecurity(doc, extendedReaders, extendedWriters, useRetry))) + .toArray(CompletableFuture[]::new); + + try + { + CompletableFuture.allOf(futures).join(); + } + catch (Exception e) + { + Throwable cause = e.getCause(); + if (cause instanceof ConcurrencyFailureException) + { + throw (ConcurrencyFailureException) cause; + } + throw new RuntimeException("Error during parallel execution", e); + } + } + + private void verifyCreatedGroups(Set documents, boolean onlyDuplicatesValidation) + { + retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback() { + @Override + public Void execute() throws Throwable + { + Set expectedAuthorities = null; + Set> errors = new HashSet<>(); + for (NodeRef doc : documents) + { + Set permissions = permissionService.getAllSetPermissions(doc); + Set authorities = getDocumentAuthorities(permissions); + Set authoritiesById = getAuthorityIds(authorities); + + verifyIPRGroups(authorities, onlyDuplicatesValidation); + + if (onlyDuplicatesValidation) + { + // Some documents may not have IPR groups created if there was a ConcurrencyFailureException + continue; + } + + // All documents should have the same exact set of groups assigned + if (expectedAuthorities == null) + { + expectedAuthorities = authoritiesById; + } + + if (!expectedAuthorities.equals(authoritiesById)) + { + errors.add(authoritiesById); + } + } + + assertTrue("Unexpected authorities linked to document", errors.isEmpty()); + + return null; + } + }, false, true); + } + + private Set getDocumentAuthorities(Set permissions) + { + Set authorities = new HashSet<>(); + + for (AccessPermission accessPermission : permissions) + { + String authority = accessPermission.getAuthority(); + String authName = authorityService.getName(AuthorityType.GROUP, authority); + authorities.add(authName); + + } + return authorities; + } + + private Set getAuthorityIds(Set authorities) + { + Set authorityIds = new HashSet<>(); + for (String authority : authorities) + { + String authId = authorityService.getAuthorityNodeRef(authority) != null + ? authorityService.getAuthorityNodeRef(authority).getId() + : null; + authorityIds.add(authId); + } + return authorityIds; + } + + private void verifyIPRGroups(Set authorities, boolean onlyDuplicatesValidation) + { + boolean hasGroupIPR = false; + + for (String authorityName : authorities) + { + String shortName = authorityService.getShortName(authorityName); + + if (authorityName.startsWith("GROUP_IPR")) + { + hasGroupIPR = true; + PagingResults results = authorityService.getAuthorities(AuthorityType.GROUP, null, shortName, false, + false, new PagingRequest(0, 10)); + + assertEquals("No duplicated IPR group expected", 1, results.getPage().size()); + } + } + + if (!onlyDuplicatesValidation) + { + assertTrue("No IPR Groups created", hasGroupIPR); + } + } }