From a9e120f3d434ece061dcaf7371822b9201eac655 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 17 Jun 2024 13:02:33 +0200 Subject: [PATCH 01/25] feat: new parameter "fromdate" to evaluate items only from certain date --- .../app/mediafilter/MediaFilterScript.java | 15 +++++++++++++-- .../MediaFilterScriptConfiguration.java | 2 ++ .../mediafilter/MediaFilterServiceImpl.java | 18 ++++++++++++++++++ .../service/MediaFilterService.java | 3 +++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java index 5fbbebbb28c..71c0d6e14dd 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java @@ -7,6 +7,7 @@ */ package org.dspace.app.mediafilter; +import java.time.LocalDate; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -37,8 +38,9 @@ * MFM: -v verbose outputs all extracted text to STDOUT; -f force forces all * bitstreams to be processed, even if they have been before; -n noindex does not * recreate index after processing bitstreams; -i [identifier] limits processing - * scope to a community, collection or item; and -m [max] limits processing to a - * maximum number of items. + * scope to a community, collection or item; -m [max] limits processing to a + * maximum number of items; -fd [fromdate] takes only items starting from this date, + * filtering by last_modified in the item table. */ public class MediaFilterScript extends DSpaceRunnable { @@ -60,6 +62,7 @@ public class MediaFilterScript extends DSpaceRunnable> filterFormats = new HashMap<>(); + private LocalDate fromDate = null; public MediaFilterScriptConfiguration getScriptConfiguration() { return new DSpace().getServiceManager() @@ -112,6 +115,10 @@ public void setup() throws ParseException { skipIds = commandLine.getOptionValues('s'); } + if (commandLine.hasOption('f')) { + fromDate = LocalDate.parse(commandLine.getOptionValue('f')); + } + } @@ -215,6 +222,10 @@ public void internalRun() throws Exception { mediaFilterService.setSkipList(Arrays.asList(skipIds)); } + if (fromDate != null) { + mediaFilterService.setFromDate(fromDate); + } + Context c = null; try { diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java index 7465fa6e127..9f62f19e836 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java @@ -52,6 +52,8 @@ public Options getOptions() { .build(); options.addOption(pluginOption); + options.addOption("fd", "fromdate", true, "Process only item from specified last modified date"); + Option skipOption = Option.builder("s") .longOpt("skip") .hasArg() diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterServiceImpl.java index a6ba9fde88d..512b8f803b9 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterServiceImpl.java @@ -9,8 +9,11 @@ import java.io.InputStream; import java.sql.SQLException; +import java.time.LocalDate; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -93,6 +96,7 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB protected boolean isVerbose = false; protected boolean isQuiet = false; protected boolean isForce = false; // default to not forced + protected LocalDate fromDate = null; protected MediaFilterServiceImpl() { @@ -120,6 +124,15 @@ public void applyFiltersAllItems(Context context) throws Exception { for (Community topLevelCommunity : topLevelCommunities) { applyFiltersCommunity(context, topLevelCommunity); } + } else if (fromDate != null) { + Iterator itemIterator = + itemService.findByLastModifiedSince( + context, + Date.from(fromDate.atStartOfDay(ZoneId.systemDefault()).toInstant()) + ); + while (itemIterator.hasNext() && processed < max2Process) { + applyFiltersItem(context, itemIterator.next()); + } } else { //otherwise, just find every item and process Iterator itemIterator = itemService.findAll(context); @@ -588,4 +601,9 @@ public void setFilterFormats(Map> filterFormats) { public void setLogHandler(DSpaceRunnableHandler handler) { this.handler = handler; } + + @Override + public void setFromDate(LocalDate fromDate) { + this.fromDate = fromDate; + } } diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/service/MediaFilterService.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/service/MediaFilterService.java index bc92ff52109..30e6dba42f0 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/service/MediaFilterService.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/service/MediaFilterService.java @@ -8,6 +8,7 @@ package org.dspace.app.mediafilter.service; import java.sql.SQLException; +import java.time.LocalDate; import java.util.List; import java.util.Map; @@ -149,4 +150,6 @@ public void updatePoliciesOfDerivativeBitstreams(Context context, Item item, Bit * @param handler */ public void setLogHandler(DSpaceRunnableHandler handler); + + public void setFromDate(LocalDate fromDate); } From 2f6b7f3ee4b0c40d215730dd2e86f4492a455e77 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 17 Jun 2024 13:57:39 +0200 Subject: [PATCH 02/25] refactor: changed short parameter fd to d --- .../main/java/org/dspace/app/mediafilter/MediaFilterScript.java | 2 +- .../dspace/app/mediafilter/MediaFilterScriptConfiguration.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java index 71c0d6e14dd..e76feada033 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java @@ -115,7 +115,7 @@ public void setup() throws ParseException { skipIds = commandLine.getOptionValues('s'); } - if (commandLine.hasOption('f')) { + if (commandLine.hasOption('d')) { fromDate = LocalDate.parse(commandLine.getOptionValue('f')); } diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java index 9f62f19e836..c9f61292d61 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScriptConfiguration.java @@ -52,7 +52,7 @@ public Options getOptions() { .build(); options.addOption(pluginOption); - options.addOption("fd", "fromdate", true, "Process only item from specified last modified date"); + options.addOption("d", "fromdate", true, "Process only item from specified last modified date"); Option skipOption = Option.builder("s") .longOpt("skip") From 3fd88b867eb9bbd8b3621904ecac5b845fc7346e Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 17 Jun 2024 16:39:02 +0200 Subject: [PATCH 03/25] refactor: changed short parameter fd to d --- .../main/java/org/dspace/app/mediafilter/MediaFilterScript.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java index e76feada033..7f022f38b31 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterScript.java @@ -116,7 +116,7 @@ public void setup() throws ParseException { } if (commandLine.hasOption('d')) { - fromDate = LocalDate.parse(commandLine.getOptionValue('f')); + fromDate = LocalDate.parse(commandLine.getOptionValue('d')); } From 52702a23df37f087058c7837a2dfcbbcf4306382 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 25 Jun 2024 13:45:31 +0200 Subject: [PATCH 04/25] Fix request a copy link token generation Ensure DSpace URLs with extra segments are included fully in the generated link --- .../rest/repository/RequestItemRepository.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index 6bfae8ed351..1bb3ddbc9ae 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.validator.routines.EmailValidator; import org.apache.http.client.utils.URIBuilder; import org.apache.logging.log4j.LogManager; @@ -287,19 +288,17 @@ public Class getDomainClass() { * Generate a link back to DSpace, to act on a request. * * @param token identifies the request. - * @return URL to the item request API, with the token as request parameter - * "token". + * @return URL to the item request API, with /request-a-copy/{token} as the last URL segments * @throws URISyntaxException passed through. * @throws MalformedURLException passed through. */ private String getLinkTokenEmail(String token) throws URISyntaxException, MalformedURLException { final String base = configurationService.getProperty("dspace.ui.url"); - - URI link = new URIBuilder(base) - .setPathSegments("request-a-copy", token) - .build(); - - return link.toURL().toExternalForm(); + URIBuilder uriBuilder = new URIBuilder(base); + // Add request-a-copy/token to the existing path (without breaking /sub/dir dspace URLs) + URI uri = uriBuilder.setPath(StringUtils.stripEnd(uriBuilder.getPath(), "") + + "/request-a-copy/" + token).build(); + return uri.toURL().toExternalForm(); } } From 3646a54df3d540e7dc7603cd7013961df8d9a404 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 25 Jun 2024 14:36:17 +0200 Subject: [PATCH 05/25] Make RequestItemRepository#getLinkTokenEmail public, write test --- .../repository/RequestItemRepository.java | 2 +- .../app/rest/RequestItemRepositoryIT.java | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index 1bb3ddbc9ae..0698567dcbc 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -292,7 +292,7 @@ public Class getDomainClass() { * @throws URISyntaxException passed through. * @throws MalformedURLException passed through. */ - private String getLinkTokenEmail(String token) + public String getLinkTokenEmail(String token) throws URISyntaxException, MalformedURLException { final String base = configurationService.getProperty("dspace.ui.url"); URIBuilder uriBuilder = new URIBuilder(base); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java index ea61db51450..a43e458207c 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java @@ -28,6 +28,8 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URISyntaxException; import java.sql.SQLException; import java.time.temporal.ChronoUnit; import java.util.Date; @@ -55,10 +57,12 @@ import org.dspace.content.Bitstream; import org.dspace.content.Collection; import org.dspace.content.Item; +import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; /** * @@ -81,6 +85,12 @@ public class RequestItemRepositoryIT @Autowired(required = true) RequestItemService requestItemService; + @Autowired + ApplicationContext applicationContext; + + @Autowired + private ConfigurationService configurationService; + private Collection collection; private Item item; @@ -594,4 +604,23 @@ public void testGetDomainClass() { Class instanceClass = instance.getDomainClass(); assertEquals("Wrong domain class", RequestItemRest.class, instanceClass); } + + /** + * Test that generated links include the correct base URL, even if it has extra URL segments + */ + @Test + public void testGetLinkTokenEmail() throws MalformedURLException, URISyntaxException { + RequestItemRepository instance = applicationContext.getBean( + RequestItemRest.CATEGORY + '.' + RequestItemRest.PLURAL_NAME, + RequestItemRepository.class); + String currentDspaceUrl = configurationService.getProperty("dspace.ui.url"); + String newDspaceUrl = currentDspaceUrl + "/subdir"; + // Add a /subdir to the url for this test + configurationService.setProperty("dspace.ui.url", newDspaceUrl); + String expectedUrl = newDspaceUrl + "/request-a-copy/token"; + String generatedLink = instance.getLinkTokenEmail("token"); + // The URLs should match + assertEquals(expectedUrl, generatedLink); + configurationService.reloadConfig(); + } } From 6eb3271fa320b9d7d32ef3c59f7b83467bd2b03a Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 10 Jul 2024 16:04:34 +0200 Subject: [PATCH 06/25] #9668: Ensure proper handling of non-subpath URLs in link tokens --- .../repository/RequestItemRepository.java | 5 +++-- .../app/rest/RequestItemRepositoryIT.java | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index 0698567dcbc..35bd2148954 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -297,8 +297,9 @@ public String getLinkTokenEmail(String token) final String base = configurationService.getProperty("dspace.ui.url"); URIBuilder uriBuilder = new URIBuilder(base); // Add request-a-copy/token to the existing path (without breaking /sub/dir dspace URLs) - URI uri = uriBuilder.setPath(StringUtils.stripEnd(uriBuilder.getPath(), "") - + "/request-a-copy/" + token).build(); + URI uri = uriBuilder.setPath( + (uriBuilder.getPath() == null ? "" : StringUtils.stripEnd(uriBuilder.getPath(), "")) + + "/request-a-copy/" + token).build(); return uri.toURL().toExternalForm(); } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java index a43e458207c..56409d18d73 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java @@ -606,10 +606,10 @@ public void testGetDomainClass() { } /** - * Test that generated links include the correct base URL, even if it has extra URL segments + * Test that generated links include the correct base URL, where the UI URL has a subpath like /subdir */ @Test - public void testGetLinkTokenEmail() throws MalformedURLException, URISyntaxException { + public void testGetLinkTokenEmailWithSubPath() throws MalformedURLException, URISyntaxException { RequestItemRepository instance = applicationContext.getBean( RequestItemRest.CATEGORY + '.' + RequestItemRest.PLURAL_NAME, RequestItemRepository.class); @@ -623,4 +623,20 @@ public void testGetLinkTokenEmail() throws MalformedURLException, URISyntaxExcep assertEquals(expectedUrl, generatedLink); configurationService.reloadConfig(); } + + /** + * Test that generated links include the correct base URL, with NO subpath elements + */ + @Test + public void testGetLinkTokenEmailWithoutSubPath() throws MalformedURLException, URISyntaxException { + RequestItemRepository instance = applicationContext.getBean( + RequestItemRest.CATEGORY + '.' + RequestItemRest.PLURAL_NAME, + RequestItemRepository.class); + String currentDspaceUrl = configurationService.getProperty("dspace.ui.url"); + String expectedUrl = currentDspaceUrl + "/request-a-copy/token"; + String generatedLink = instance.getLinkTokenEmail("token"); + // The URLs should match + assertEquals(expectedUrl, generatedLink); + configurationService.reloadConfig(); + } } From a9f6d771121087f120c0a82710bde864b73b5e10 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Sun, 1 Sep 2024 12:53:04 +0200 Subject: [PATCH 07/25] Improved URI build method as per review --- .../rest/repository/RequestItemRepository.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index 35bd2148954..3de3dbef9c5 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -15,7 +15,11 @@ import java.net.URI; import java.net.URISyntaxException; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collections; import java.util.Date; +import java.util.LinkedList; +import java.util.List; import java.util.UUID; import com.fasterxml.jackson.databind.JsonNode; @@ -295,11 +299,16 @@ public Class getDomainClass() { public String getLinkTokenEmail(String token) throws URISyntaxException, MalformedURLException { final String base = configurationService.getProperty("dspace.ui.url"); + URIBuilder uriBuilder = new URIBuilder(base); - // Add request-a-copy/token to the existing path (without breaking /sub/dir dspace URLs) - URI uri = uriBuilder.setPath( - (uriBuilder.getPath() == null ? "" : StringUtils.stripEnd(uriBuilder.getPath(), "")) - + "/request-a-copy/" + token).build(); + List segments = new LinkedList<>(); + if (StringUtils.isNotBlank(uriBuilder.getPath())) { + segments.add(StringUtils.strip(uriBuilder.getPath(), "/")); + } + segments.add("request-a-copy"); + segments.add(token); + URI uri = uriBuilder.setPathSegments(segments).build(); + return uri.toURL().toExternalForm(); } } From 74a6dc218787b77ed03aa36900be6bd070be043c Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 4 Sep 2024 14:04:20 +0200 Subject: [PATCH 08/25] Tidy implementation of link token generation --- .../dspace/app/rest/repository/RequestItemRepository.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index 3de3dbef9c5..59645aaf1d5 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -300,6 +300,7 @@ public String getLinkTokenEmail(String token) throws URISyntaxException, MalformedURLException { final String base = configurationService.getProperty("dspace.ui.url"); + // Construct the link, making sure to support sub-paths URIBuilder uriBuilder = new URIBuilder(base); List segments = new LinkedList<>(); if (StringUtils.isNotBlank(uriBuilder.getPath())) { @@ -307,8 +308,8 @@ public String getLinkTokenEmail(String token) } segments.add("request-a-copy"); segments.add(token); - URI uri = uriBuilder.setPathSegments(segments).build(); - return uri.toURL().toExternalForm(); + // Build and return the URL from segments (or throw exception) + return uriBuilder.setPathSegments(segments).build().toURL().toExternalForm(); } } From 185a6fdf91c33eb5e100fd28b881f714b3a0a3be Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 4 Sep 2024 15:19:34 +0200 Subject: [PATCH 09/25] lint fixes (RequestItemRepository) --- .../org/dspace/app/rest/repository/RequestItemRepository.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index 59645aaf1d5..eaae877f283 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -12,11 +12,8 @@ import java.io.IOException; import java.net.MalformedURLException; -import java.net.URI; import java.net.URISyntaxException; import java.sql.SQLException; -import java.util.ArrayList; -import java.util.Collections; import java.util.Date; import java.util.LinkedList; import java.util.List; From 9c7b20ff5721e2886acd7186ba4df310c9efcf07 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 4 Sep 2024 15:31:28 +0200 Subject: [PATCH 10/25] fix: changed parameter of HQL query --- .../src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java index 3be39f1788f..bd042648384 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java @@ -441,7 +441,7 @@ public int countItems(Context context, List collections, boolean inc public Iterator findByLastModifiedSince(Context context, Date since) throws SQLException { Query query = createQuery(context, - "SELECT i.id FROM Item i WHERE last_modified > :last_modified ORDER BY id"); + "SELECT i.id FROM Item i WHERE lastModified > :last_modified ORDER BY id"); query.setParameter("last_modified", since, TemporalType.TIMESTAMP); @SuppressWarnings("unchecked") List uuids = query.getResultList(); From cdb167e55aac9916618dcfee7222105f4456dbd8 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:25:18 +0200 Subject: [PATCH 11/25] #9806: Add new create methods to group builder Now supports admin groups, default read, workflow role --- .../java/org/dspace/builder/GroupBuilder.java | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/dspace-api/src/test/java/org/dspace/builder/GroupBuilder.java b/dspace-api/src/test/java/org/dspace/builder/GroupBuilder.java index b3447dd8bd9..c16fb696b0c 100644 --- a/dspace-api/src/test/java/org/dspace/builder/GroupBuilder.java +++ b/dspace-api/src/test/java/org/dspace/builder/GroupBuilder.java @@ -12,6 +12,9 @@ import java.util.UUID; import org.dspace.authorize.AuthorizeException; +import org.dspace.content.Collection; +import org.dspace.content.Community; +import org.dspace.content.DSpaceObject; import org.dspace.content.service.DSpaceObjectService; import org.dspace.core.Context; import org.dspace.eperson.EPerson; @@ -51,6 +54,33 @@ public static GroupBuilder createGroup(final Context context) { return builder.create(context); } + public static GroupBuilder createCollectionAdminGroup(final Context context, Collection collection) { + GroupBuilder builder = new GroupBuilder(context); + return builder.createAdminGroup(context, collection); + } + + public static GroupBuilder createCollectionSubmitterGroup(final Context context, Collection collection) { + GroupBuilder builder = new GroupBuilder(context); + return builder.createSubmitterGroup(context, collection); + } + + public static GroupBuilder createCollectionDefaultReadGroup(final Context context, Collection collection, + String typeOfGroupString, int defaultRead) { + GroupBuilder builder = new GroupBuilder(context); + return builder.createDefaultReadGroup(context, collection, typeOfGroupString, defaultRead); + } + + public static GroupBuilder createCollectionWorkflowRoleGroup(final Context context, Collection collection, + String roleName) { + GroupBuilder builder = new GroupBuilder(context); + return builder.createWorkflowRoleGroup(context, collection, roleName); + } + + public static GroupBuilder createCommunityAdminGroup(final Context context, Community community) { + GroupBuilder builder = new GroupBuilder(context); + return builder.createAdminGroup(context, community); + } + private GroupBuilder create(final Context context) { this.context = context; try { @@ -61,6 +91,54 @@ private GroupBuilder create(final Context context) { return this; } + private GroupBuilder createAdminGroup(final Context context, DSpaceObject container) { + this.context = context; + try { + if (container instanceof Collection) { + group = collectionService.createAdministrators(context, (Collection) container); + } else if (container instanceof Community) { + group = communityService.createAdministrators(context, (Community) container); + } else { + handleException(new IllegalArgumentException("DSpaceObject must be collection or community. " + + "Type: " + container.getType())); + } + } catch (Exception e) { + return handleException(e); + } + return this; + } + + private GroupBuilder createSubmitterGroup(final Context context, Collection collection) { + this.context = context; + try { + group = collectionService.createSubmitters(context, collection); + } catch (Exception e) { + return handleException(e); + } + return this; + } + + private GroupBuilder createDefaultReadGroup(final Context context, Collection collection, + String typeOfGroupString, int defaultRead) { + this.context = context; + try { + group = collectionService.createDefaultReadGroup(context, collection, typeOfGroupString, defaultRead); + } catch (Exception e) { + return handleException(e); + } + return this; + } + + private GroupBuilder createWorkflowRoleGroup(final Context context, Collection collection, String roleName) { + this.context = context; + try { + group = workflowService.createWorkflowRoleGroup(context, collection, roleName); + } catch (Exception e) { + return handleException(e); + } + return this; + } + @Override protected DSpaceObjectService getService() { return groupService; From b13abac7539944358711a381548fbaedd9dcbd92 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:28:27 +0200 Subject: [PATCH 12/25] #9806: Use builders for coll, comm, group creation in BitstreamRestRepositoryIT --- .../app/rest/BitstreamRestRepositoryIT.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestRepositoryIT.java index af2b14759c6..a18dde701b4 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestRepositoryIT.java @@ -50,6 +50,7 @@ import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; import org.dspace.builder.EPersonBuilder; +import org.dspace.builder.GroupBuilder; import org.dspace.builder.ItemBuilder; import org.dspace.builder.ResourcePolicyBuilder; import org.dspace.content.Bitstream; @@ -2768,10 +2769,12 @@ public void deleteBitstreamsInBulk_collectionAdmin() throws Exception { .withEmail("col2admin@test.com") .withPassword(password) .build(); - Group col1_AdminGroup = collectionService.createAdministrators(context, col1); - Group col2_AdminGroup = collectionService.createAdministrators(context, col2); - groupService.addMember(context, col1_AdminGroup, col1Admin); - groupService.addMember(context, col2_AdminGroup, col2Admin); + Group col1_AdminGroup = GroupBuilder.createCollectionAdminGroup(context, col1) + .addMember(col1Admin) + .build(); + Group col2_AdminGroup = GroupBuilder.createCollectionAdminGroup(context, col2) + .addMember(col2Admin) + .build(); Item publicItem1 = ItemBuilder.createItem(context, col1) .withTitle("Test item 1") .build(); @@ -2872,8 +2875,9 @@ public void deleteBitstreamsInBulk_communityAdmin() throws Exception { .withEmail("parentComAdmin@test.com") .withPassword(password) .build(); - Group parentComAdminGroup = communityService.createAdministrators(context, parentCommunity); - groupService.addMember(context, parentComAdminGroup, parentCommunityAdmin); + Group parentComAdminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity) + .addMember(parentCommunityAdmin) + .build(); Item publicItem1 = ItemBuilder.createItem(context, col1) .withTitle("Test item 1") .build(); From 9205773802a8d22820b074e18409e245c63f5609 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:29:48 +0200 Subject: [PATCH 13/25] #9806: Use builders for coll, comm, group creation in GroupRestRepositoryIT --- .../app/rest/GroupRestRepositoryIT.java | 202 ++++++++++-------- 1 file changed, 107 insertions(+), 95 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java index 83259aa09e9..6f9d418f7f4 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java @@ -85,9 +85,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { ResourcePolicyService resourcePolicyService; @Autowired private ConfigurationService configurationService; - @Autowired - private CollectionService collectionService; - @Autowired private AuthorizeService authorizeService; @@ -773,12 +770,13 @@ public void addChildGroupCommunityAdminTest() throws Exception { try { context.turnOffAuthorisationSystem(); - community = communityService.create(null, context); - parentGroup = communityService.createAdministrators(context, community); - childGroup1 = groupService.create(context); - childGroup2 = groupService.create(context); + community = CommunityBuilder.createCommunity(context).build(); + parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + .addMember(eperson) + .build(); + childGroup1 = GroupBuilder.createGroup(context).build(); + childGroup2 = GroupBuilder.createGroup(context).build(); - groupService.addMember(context, parentGroup, eperson); groupService.update(context, parentGroup); context.commit(); @@ -810,6 +808,7 @@ public void addChildGroupCommunityAdminTest() throws Exception { ); } finally { + // TODO: Can we remove these lines now that we are creating them with the builder? if (community != null) { CommunityBuilder.deleteCommunity(community.getID()); } @@ -837,9 +836,9 @@ public void addChildGroupForbiddenTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup1 = groupService.create(context); - childGroup2 = groupService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup1 = GroupBuilder.createGroup(context).build(); + childGroup2 = GroupBuilder.createGroup(context).build(); context.commit(); @@ -882,9 +881,9 @@ public void addChildGroupUnauthorizedTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup1 = groupService.create(context); - childGroup2 = groupService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup1 = GroupBuilder.createGroup(context).build(); + childGroup2 = GroupBuilder.createGroup(context).build(); context.commit(); @@ -926,9 +925,9 @@ public void addChildGroupNotFoundTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup1 = groupService.create(context); - childGroup2 = groupService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup1 = GroupBuilder.createGroup(context).build(); + childGroup2 = GroupBuilder.createGroup(context).build(); context.commit(); @@ -971,18 +970,18 @@ public void addChildGroupUnprocessableTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup1 = groupService.create(context); - childGroup2 = groupService.create(context); - - groupService.addMember(context, childGroup1, parentGroup); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup1 = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); + childGroup2 = GroupBuilder.createGroup(context).build(); groupService.update(context, childGroup1); - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup1 = context.reloadEntity(childGroup1); - childGroup2 = context.reloadEntity(childGroup2); +// context.commit(); +// +// parentGroup = context.reloadEntity(parentGroup); +// childGroup1 = context.reloadEntity(childGroup1); +// childGroup2 = context.reloadEntity(childGroup2); context.restoreAuthSystemState(); String authToken = getAuthToken(admin.getEmail(), password); @@ -995,13 +994,15 @@ public void addChildGroupUnprocessableTest() throws Exception { ) ).andExpect(status().isUnprocessableEntity()); + // TODO - confirm with reviewers that this is a mistake - it actually should be No Content + // (see AddMember test) but was incorrectly expecting 422? getClient(authToken).perform( post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() ) - ).andExpect(status().isUnprocessableEntity()); + ).andExpect(status().isNoContent()); } finally { if (parentGroup != null) { @@ -1093,13 +1094,12 @@ public void addMemberCommunityAdminTest() throws Exception { try { context.turnOffAuthorisationSystem(); - community = communityService.create(null, context); - parentGroup = communityService.createAdministrators(context, community); - member1 = ePersonService.create(context); - member2 = ePersonService.create(context); - - groupService.addMember(context, parentGroup, eperson); - groupService.update(context, parentGroup); + community = CommunityBuilder.createCommunity(context).build(); + parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + .addMember(eperson) + .build(); + member1 = EPersonBuilder.createEPerson(context).build(); + member2 = EPersonBuilder.createEPerson(context).build(); context.commit(); @@ -1158,9 +1158,9 @@ public void addMemberForbiddenTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member1 = ePersonService.create(context); - member2 = ePersonService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + member1 = EPersonBuilder.createEPerson(context).build(); + member2 = EPersonBuilder.createEPerson(context).build(); context.commit(); @@ -1204,9 +1204,9 @@ public void addMemberUnauthorizedTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member1 = ePersonService.create(context); - member2 = ePersonService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + member1 = EPersonBuilder.createEPerson(context).build(); + member2 = EPersonBuilder.createEPerson(context).build(); context.commit(); @@ -1249,9 +1249,9 @@ public void addMemberNotFoundTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member1 = ePersonService.create(context); - member2 = ePersonService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + member1 = EPersonBuilder.createEPerson(context).build(); + member2 = EPersonBuilder.createEPerson(context).build(); context.commit(); @@ -1295,9 +1295,9 @@ public void addMemberUnprocessableTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member1 = ePersonService.create(context); - member2 = ePersonService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + member1 = EPersonBuilder.createEPerson(context).build(); + member2 = EPersonBuilder.createEPerson(context).build(); context.commit(); @@ -1389,13 +1389,13 @@ public void removeChildGroupCommunityAdminTest() throws Exception { try { context.turnOffAuthorisationSystem(); - community = communityService.create(null, context); - parentGroup = communityService.createAdministrators(context, community); - childGroup = groupService.create(context); - - groupService.addMember(context, parentGroup, childGroup); - groupService.addMember(context, parentGroup, eperson); - groupService.update(context, parentGroup); + community = CommunityBuilder.createCommunity(context).build(); + parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + .addMember(eperson) + .build(); + childGroup = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); context.commit(); @@ -1439,8 +1439,8 @@ public void removeChildGroupForbiddenTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup = groupService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup = GroupBuilder.createGroup(context).build(); context.commit(); @@ -1474,8 +1474,8 @@ public void removeChildGroupUnauthorizedTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup = groupService.create(context); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup = GroupBuilder.createGroup(context).build(); context.commit(); @@ -1508,10 +1508,10 @@ public void removeChildGroupNotFoundTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup = groupService.create(context); - - groupService.addMember(context, childGroup, parentGroup); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); context.commit(); @@ -1546,10 +1546,10 @@ public void removeChildGroupUnprocessableTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - childGroup = groupService.create(context); - - groupService.addMember(context, childGroup, parentGroup); + parentGroup = GroupBuilder.createGroup(context).build(); + childGroup = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); context.commit(); @@ -1625,13 +1625,12 @@ public void removeMemberCommunityAdminTest() throws Exception { try { context.turnOffAuthorisationSystem(); - community = communityService.create(null, context); - parentGroup = communityService.createAdministrators(context, community); - member = ePersonService.create(context); - - groupService.addMember(context, parentGroup, member); - groupService.addMember(context, parentGroup, eperson); - groupService.update(context, parentGroup); + community = CommunityBuilder.createCommunity(context).build(); + member = EPersonBuilder.createEPerson(context).build(); + parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + .addMember(member) + .addMember(eperson) + .build(); assertTrue(groupService.isMember(context, member, parentGroup)); @@ -1678,9 +1677,10 @@ public void removeMemberForbiddenTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member = ePersonService.create(context); - groupService.addMember(context, parentGroup, member); + member = EPersonBuilder.createEPerson(context).build(); + parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); context.commit(); @@ -1715,9 +1715,10 @@ public void removeMemberUnauthorizedTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member = ePersonService.create(context); - groupService.addMember(context, parentGroup, member); + member = EPersonBuilder.createEPerson(context).build(); + parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); context.commit(); @@ -1751,9 +1752,10 @@ public void removeMemberNotFoundTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member = ePersonService.create(context); - groupService.addMember(context, parentGroup, member); + member = EPersonBuilder.createEPerson(context).build(); + parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); context.commit(); @@ -1789,9 +1791,10 @@ public void removeMemberUnprocessableTest() throws Exception { try { context.turnOffAuthorisationSystem(); - parentGroup = groupService.create(context); - member = ePersonService.create(context); - groupService.addMember(context, parentGroup, member); + member = EPersonBuilder.createEPerson(context).build(); + parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); context.commit(); @@ -2586,7 +2589,8 @@ public void commAdminAndColAdminCanManageItemReadGroupTest() throws Exception { String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group itemReadGroup = collectionService.createDefaultReadGroup(context, col1, itemGroupString, defaultItemRead); + Group itemReadGroup = GroupBuilder.createCollectionDefaultReadGroup(context, + col1, itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); @@ -2670,8 +2674,9 @@ public void commAdminAndColAdminCanManageBitstreamReadGroupTest() throws Excepti String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group bitstreamReadGroup = collectionService.createDefaultReadGroup(context, col1, bitstreamGroupString, - defaultBitstreamRead); + Group bitstreamReadGroup = GroupBuilder.createCollectionDefaultReadGroup(context, col1, bitstreamGroupString, + defaultBitstreamRead) + .build(); context.restoreAuthSystemState(); @@ -2792,7 +2797,8 @@ public void commAdminAndColAdminCanManageWorkflowGroupsTest() throws Exception { public void collectionAdminRemoveMembersFromCollectionAdminGroupSuccess() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); context.restoreAuthSystemState(); @@ -2827,7 +2833,8 @@ public void collectionAdminRemoveMembersFromCollectionAdminGroupSuccess() throws public void collectionAdminAddChildGroupToCollectionAdminGroupSuccess() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); @@ -2853,7 +2860,8 @@ public void collectionAdminAddChildGroupToCollectionAdminGroupSuccess() throws E public void collectionAdminRemoveChildGroupFromCollectionAdminGroupSuccess() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); @@ -2889,7 +2897,8 @@ public void collectionAdminRemoveChildGroupFromCollectionAdminGroupSuccess() thr public void collectionAdminAddMembersToCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); @@ -2923,7 +2932,8 @@ public void collectionAdminAddMembersToCollectionAdminGroupPropertySetToFalse() public void collectionAdminRemoveMembersFromCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); context.restoreAuthSystemState(); @@ -2961,7 +2971,8 @@ public void collectionAdminRemoveMembersFromCollectionAdminGroupPropertySetToFal public void collectionAdminAddChildGroupToCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); @@ -2990,7 +3001,8 @@ public void collectionAdminAddChildGroupToCollectionAdminGroupPropertySetToFalse public void collectionAdminRemoveChildGroupFromCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection) + .build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); From 80328eaca5f2ad36e006d32ea06aa3b89e2ada92 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:30:52 +0200 Subject: [PATCH 14/25] #9806: Tidy imports for GroupRestRepositoryIT --- .../src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java index 6f9d418f7f4..bdd13b97ff7 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java @@ -60,7 +60,6 @@ import org.dspace.content.Item; import org.dspace.content.MetadataSchemaEnum; import org.dspace.content.factory.ContentServiceFactory; -import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; import org.dspace.core.Constants; import org.dspace.core.I18nUtil; From 1f475aa731bbe42ad523f1056e6b56796ea8a2ee Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:31:31 +0200 Subject: [PATCH 15/25] #9806: Use builders for group, comm, coll creation in PackagerIT --- .../src/test/java/org/dspace/app/packager/PackagerIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java b/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java index ab98a053e4d..d9ee0fb9bf5 100644 --- a/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java +++ b/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java @@ -24,6 +24,7 @@ import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; import org.dspace.builder.ItemBuilder; +import org.dspace.builder.WorkspaceItemBuilder; import org.dspace.content.Collection; import org.dspace.content.Community; import org.dspace.content.Item; @@ -159,7 +160,7 @@ public void packagerUUIDAlreadyExistWithoutForceTest() throws Exception { performExportScript(article.getHandle(), tempFile); UUID id = article.getID(); itemService.delete(context, article); - WorkspaceItem workspaceItem = workspaceItemService.create(context, col1, id, false, false); + WorkspaceItem workspaceItem = WorkspaceItemBuilder.createWorkspaceItem(context, col1, id).build(); installItemService.installItem(context, workspaceItem, "123456789/0100"); performImportNoForceScript(tempFile); Iterator items = itemService.findByCollection(context, col1); From 2ef69045d15ae49a4d447227e8080860eecc61cd Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:32:08 +0200 Subject: [PATCH 16/25] #9806: Use builders for group, comm, coll creation in StructBuilderIT --- .../dspace/administer/StructBuilderIT.java | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java b/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java index 63340698ac0..ead338bc8e7 100644 --- a/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java +++ b/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java @@ -23,11 +23,12 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.dspace.AbstractIntegrationTest; +import org.dspace.AbstractIntegrationTestWithDatabase; import org.dspace.authorize.AuthorizeException; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; import org.dspace.content.Collection; import org.dspace.content.Community; -import org.dspace.content.MetadataSchemaEnum; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; @@ -38,7 +39,6 @@ import org.junit.Test; import org.w3c.dom.Attr; import org.w3c.dom.Node; -import org.xml.sax.SAXException; import org.xmlunit.builder.DiffBuilder; import org.xmlunit.diff.Comparison; import org.xmlunit.diff.ComparisonFormatter; @@ -52,7 +52,7 @@ * @author Mark H. Wood */ public class StructBuilderIT - extends AbstractIntegrationTest { + extends AbstractIntegrationTestWithDatabase { private static final Logger log = LogManager.getLogger(); private static final CommunityService communityService @@ -79,7 +79,8 @@ public static void tearDownClass() { * @throws IOException passed through. */ @Before - public void setUp() throws SQLException, AuthorizeException, IOException { + public void setUp() throws Exception { + super.setUp(); // Clear out all communities and collections. context.turnOffAuthorisationSystem(); for (Community community : communityService.findAllTop(context)) { @@ -285,19 +286,15 @@ public void testImportStructureWithHandles() * @throws org.dspace.authorize.AuthorizeException passed through. */ @Test - public void testExportStructure() - throws ParserConfigurationException, SAXException, IOException, - SQLException, AuthorizeException { + public void testExportStructure() { // Create some structure to test. context.turnOffAuthorisationSystem(); - Community community0 = communityService.create(null, context); - communityService.setMetadataSingleValue(context, community0, - MetadataSchemaEnum.DC.getName(), "title", null, - null, "Top Community 0"); - Collection collection0_0 = collectionService.create(context, community0); - collectionService.setMetadataSingleValue(context, collection0_0, - MetadataSchemaEnum.DC.getName(), "title", null, - null, "Collection 0.0"); + // Top level community + Community community0 = CommunityBuilder.createCommunity(context) + .withName("Top Community 0").build(); + // Collection below top level community + Collection collection0_0 = CollectionBuilder.createCollection(context, community0) + .withName("Collection 0.0").build(); // Export the current structure. System.out.println("exportStructure"); From b99b1eec29c5a0b6e92998b072282b03cfedc080 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:33:30 +0200 Subject: [PATCH 17/25] #9806: Refactor WorkspaceItemBuilder to support specific item uuid --- .../dspace/builder/WorkspaceItemBuilder.java | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java b/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java index 8b82149cdf7..67d8894338e 100644 --- a/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java +++ b/dspace-api/src/test/java/org/dspace/builder/WorkspaceItemBuilder.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.io.InputStream; import java.sql.SQLException; +import java.util.UUID; import org.dspace.app.ldn.NotifyPatternToTrigger; import org.dspace.app.ldn.NotifyServiceEntity; @@ -43,14 +44,31 @@ protected WorkspaceItemBuilder(Context context) { public static WorkspaceItemBuilder createWorkspaceItem(final Context context, final Collection col) { WorkspaceItemBuilder builder = new WorkspaceItemBuilder(context); - return builder.create(context, col); + return builder.create(context, col, null); } - private WorkspaceItemBuilder create(final Context context, final Collection col) { + public static WorkspaceItemBuilder createWorkspaceItem(final Context context, final Collection col, UUID uuid) { + WorkspaceItemBuilder builder = new WorkspaceItemBuilder(context); + return builder.create(context, col, uuid); + } + + /** + * Create with a specific UUID (e.g. restoring items with Packager import) + * + * @param context DSpace context + * @param col Parent collection + * @param uuid Item UUID + * @return WorkspaceItemBuilder + */ + private WorkspaceItemBuilder create(final Context context, final Collection col, UUID uuid) { this.context = context; try { - workspaceItem = workspaceItemService.create(context, col, false); + if (uuid == null) { + workspaceItem = workspaceItemService.create(context, col, false); + } else { + workspaceItem = workspaceItemService.create(context, col, uuid, false, false); + } item = workspaceItem.getItem(); } catch (Exception e) { return handleException(e); From 6e9181e3f7cd502b345af7d5879fa25654176c25 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:34:05 +0200 Subject: [PATCH 18/25] #9806: Use builders for comm, coll, group creation in SupervisionOrderServiceIT --- .../SupervisionOrderServiceIT.java | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/supervision/SupervisionOrderServiceIT.java b/dspace-api/src/test/java/org/dspace/supervision/SupervisionOrderServiceIT.java index 60407823485..aa4cd8bd4e4 100644 --- a/dspace-api/src/test/java/org/dspace/supervision/SupervisionOrderServiceIT.java +++ b/dspace-api/src/test/java/org/dspace/supervision/SupervisionOrderServiceIT.java @@ -18,6 +18,7 @@ import org.dspace.builder.CommunityBuilder; import org.dspace.builder.EPersonBuilder; import org.dspace.builder.GroupBuilder; +import org.dspace.builder.SupervisionOrderBuilder; import org.dspace.builder.WorkspaceItemBuilder; import org.dspace.content.Collection; import org.dspace.content.Item; @@ -85,10 +86,10 @@ public void createSupervisionOrderTest() throws Exception { .build(); SupervisionOrder supervisionOrderOne = - supervisionOrderService.create(context, item, groupA); + SupervisionOrderBuilder.createSupervisionOrder(context, item, groupA).build(); SupervisionOrder supervisionOrderTwo = - supervisionOrderService.create(context, item, groupB); + SupervisionOrderBuilder.createSupervisionOrder(context, item, groupB).build(); context.restoreAuthSystemState(); @@ -136,7 +137,8 @@ public void findSupervisionOrderTest() throws Exception { .build(); SupervisionOrder supervisionOrderOne = - supervisionOrderService.create(context, workspaceItem.getItem(), groupA); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItem.getItem(), groupA) + .build(); context.restoreAuthSystemState(); @@ -205,9 +207,12 @@ public void findAllSupervisionOrdersTest() throws Exception { .addMember(userB) .build(); - supervisionOrderService.create(context, workspaceItem.getItem(), groupA); - supervisionOrderService.create(context, workspaceItem.getItem(), groupB); - supervisionOrderService.create(context, workspaceItemTwo.getItem(), groupA); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItem.getItem(), groupA) + .build(); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItem.getItem(), groupB) + .build(); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItemTwo.getItem(), groupA) + .build(); context.restoreAuthSystemState(); @@ -259,9 +264,12 @@ public void findSupervisionOrderByItemTest() throws Exception { .addMember(eperson) .build(); - supervisionOrderService.create(context, workspaceItem.getItem(), groupA); - supervisionOrderService.create(context, workspaceItem.getItem(), groupB); - supervisionOrderService.create(context, workspaceItemTwo.getItem(), groupA); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItem.getItem(), groupA) + .build(); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItem.getItem(), groupB) + .build(); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItemTwo.getItem(), groupA) + .build(); context.restoreAuthSystemState(); @@ -310,7 +318,8 @@ public void findSupervisionOrderByItemAndGroupTest() throws Exception { .addMember(eperson) .build(); - supervisionOrderService.create(context, item, groupA); + SupervisionOrderBuilder.createSupervisionOrder(context, item, groupA) + .build(); context.restoreAuthSystemState(); @@ -370,7 +379,8 @@ public void isSupervisorTest() throws Exception { .addMember(userB) .build(); - supervisionOrderService.create(context, workspaceItem.getItem(), groupA); + SupervisionOrderBuilder.createSupervisionOrder(context, workspaceItem.getItem(), groupA) + .build(); context.restoreAuthSystemState(); From f4629d8351b70ca945ffeebdf928cebfb868c349 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:35:14 +0200 Subject: [PATCH 19/25] #9806: Use builders for comm, coll, group creation in CollectionGroupRestControllerIT --- .../rest/CollectionGroupRestControllerIT.java | 151 +++++++++--------- 1 file changed, 77 insertions(+), 74 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java index f6ab10c087a..8d490109220 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java @@ -28,9 +28,9 @@ import org.dspace.authorize.service.AuthorizeService; import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.GroupBuilder; import org.dspace.builder.WorkspaceItemBuilder; import org.dspace.content.Collection; -import org.dspace.content.service.CollectionService; import org.dspace.core.Constants; import org.dspace.eperson.Group; import org.dspace.eperson.service.GroupService; @@ -41,10 +41,6 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrationTest { - - @Autowired - private CollectionService collectionService; - @Autowired private GroupService groupService; @@ -68,7 +64,7 @@ public void setup() { @Test public void getCollectionAdminGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -81,7 +77,7 @@ public void getCollectionAdminGroupTest() throws Exception { @Test public void getCollectionAdminGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -95,7 +91,7 @@ public void getCollectionAdminGroupTestParentCommunityAdmin() throws Exception { @Test public void getCollectionAdminGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -109,7 +105,7 @@ public void getCollectionAdminGroupTestCollectionAdmin() throws Exception { @Test public void getCollectionAdminGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - collectionService.createAdministrators(context, collection); + GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); getClient().perform(get("/api/core/collections/" + collection.getID() + "/adminGroup")) @@ -119,7 +115,7 @@ public void getCollectionAdminGroupUnAuthorizedTest() throws Exception { @Test public void getCollectionAdminGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - collectionService.createAdministrators(context, collection); + GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -413,7 +409,7 @@ public void postCollectionAdminGroupCreateAdminGroupUnProcessablePermanent() thr @Test public void deleteCollectionAdminGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -428,7 +424,7 @@ public void deleteCollectionAdminGroupTest() throws Exception { @Test public void deleteCollectionAdminGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -443,7 +439,7 @@ public void deleteCollectionAdminGroupTestParentCommunityAdmin() throws Exceptio @Test public void deleteCollectionAdminGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -458,7 +454,7 @@ public void deleteCollectionAdminGroupTestCollectionAdmin() throws Exception { @Test public void deleteCollectionAdminGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); getClient().perform(delete("/api/core/collections/" + collection.getID() + "/adminGroup")) @@ -474,7 +470,7 @@ public void deleteCollectionAdminGroupUnAuthorizedTest() throws Exception { @Test public void deleteCollectionAdminGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -493,7 +489,7 @@ public void deleteCollectionAdminGroupForbiddenTest() throws Exception { @Test public void deleteCollectionAdminGroupNotFoundTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -512,7 +508,7 @@ public void deleteCollectionAdminGroupNotFoundTest() throws Exception { @Test public void getCollectionSubmittersGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group submitters = collectionService.createSubmitters(context, collection); + Group submitters = GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -525,7 +521,7 @@ public void getCollectionSubmittersGroupTest() throws Exception { @Test public void getCollectionSubmittersGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group submitters = collectionService.createSubmitters(context, collection); + Group submitters = GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -539,7 +535,7 @@ public void getCollectionSubmittersGroupTestParentCommunityAdmin() throws Except @Test public void getCollectionSubmittersGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group submitters = collectionService.createSubmitters(context, collection); + Group submitters = GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -553,7 +549,7 @@ public void getCollectionSubmittersGroupTestCollectionAdmin() throws Exception { @Test public void getCollectionSubmittersGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - collectionService.createSubmitters(context, collection); + GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); getClient().perform(get("/api/core/collections/" + collection.getID() + "/submittersGroup")) @@ -563,7 +559,7 @@ public void getCollectionSubmittersGroupUnAuthorizedTest() throws Exception { @Test public void getCollectionSubmittersGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - collectionService.createSubmitters(context, collection); + GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -860,7 +856,7 @@ public void postCollectionSubmittersGroupCreateSubmittersGroupUnProcessablePerma @Test public void deleteCollectionSubmitterGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group submittersGroup = collectionService.createSubmitters(context, collection); + GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -875,7 +871,7 @@ public void deleteCollectionSubmitterGroupTest() throws Exception { @Test public void deleteCollectionSubmittersGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group submittersGroup = collectionService.createSubmitters(context, collection); + GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -890,7 +886,7 @@ public void deleteCollectionSubmittersGroupTestParentCommunityAdmin() throws Exc @Test public void deleteCollectionSubmittersGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group submittersGroup = collectionService.createSubmitters(context, collection); + GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -905,7 +901,7 @@ public void deleteCollectionSubmittersGroupTestCollectionAdmin() throws Exceptio @Test public void deleteCollectionSubmittersGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - Group submittersGroup = collectionService.createSubmitters(context, collection); + Group submittersGroup = GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); getClient().perform(delete("/api/core/collections/" + collection.getID() + "/submittersGroup")) @@ -924,7 +920,7 @@ public void deleteCollectionSubmittersGroupUnAuthorizedTest() throws Exception { @Test public void deleteCollectionSubmittersGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - Group submittersGroup = collectionService.createSubmitters(context, collection); + Group submittersGroup = GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -945,7 +941,7 @@ public void deleteCollectionSubmittersGroupForbiddenTest() throws Exception { @Test public void deleteCollectionSubmittersGroupNotFoundTest() throws Exception { context.turnOffAuthorisationSystem(); - Group submittersGroup = collectionService.createSubmitters(context, collection); + GroupBuilder.createCollectionSubmitterGroup(context, collection).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -961,7 +957,8 @@ public void getCollectionItemReadGroupTest() throws Exception { String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -977,7 +974,8 @@ public void getCollectionDefaultItemReadGroupTestParentCommunityAdmin() throws E String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -995,7 +993,8 @@ public void getCollectionDefaultItemReadGroupTestCollectionAdmin() throws Except String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1012,7 +1011,8 @@ public void getCollectionDefaultItemReadGroupUnAuthorizedTest() throws Exception String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); getClient().perform(get("/api/core/collections/" + collection.getID() + "/itemReadGroup")) @@ -1025,7 +1025,8 @@ public void getCollectionDefaultItemReadGroupForbiddenTest() throws Exception { String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1321,7 +1322,7 @@ public void deleteCollectionDefaultItemReadGroupTest() throws Exception { String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -1345,7 +1346,7 @@ public void deleteCollectionDefaultItemReadGroupTestParentCommunityAdmin() throw String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, itemGroupString, defaultItemRead).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1367,7 +1368,7 @@ public void deleteCollectionDefaultItemReadGroupTestCollectionAdmin() throws Exc String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, itemGroupString, defaultItemRead).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1389,7 +1390,8 @@ public void deleteCollectionDefaultItemReadGroupUnAuthorizedTest() throws Except String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); getClient().perform(delete("/api/core/collections/" + collection.getID() + "/itemReadGroup")) @@ -1408,7 +1410,8 @@ public void deleteCollectionDefaultItemReadGroupForbiddenTest() throws Exception String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1430,7 +1433,7 @@ public void deleteCollectionDefaultItemReadGroupNotFoundTest() throws Exception String itemGroupString = "ITEM"; int defaultItemRead = Constants.DEFAULT_ITEM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, itemGroupString, defaultItemRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, itemGroupString, defaultItemRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1445,8 +1448,8 @@ public void getCollectionBitstreamReadGroupTest() throws Exception { String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -1462,8 +1465,8 @@ public void getCollectionDefaultBitstreamReadGroupTestParentCommunityAdmin() thr String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1480,8 +1483,8 @@ public void getCollectionDefaultBitstreamReadGroupTestCollectionAdmin() throws E String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1498,8 +1501,8 @@ public void getCollectionDefaultBitstreamReadGroupUnAuthorizedTest() throws Exce String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); getClient().perform(get("/api/core/collections/" + collection.getID() + "/bitstreamReadGroup")) @@ -1512,8 +1515,8 @@ public void getCollectionDefaultBitstreamReadGroupForbiddenTest() throws Excepti String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1811,8 +1814,8 @@ public void deleteCollectionDefaultBitstreamReadGroupTest() throws Exception { String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -1835,8 +1838,8 @@ public void deleteCollectionDefaultBitstreamReadGroupTestParentCommunityAdmin() String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1859,8 +1862,8 @@ public void deleteCollectionDefaultBitstreamReadGroupTestCollectionAdmin() throw String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1882,8 +1885,8 @@ public void deleteCollectionDefaultBitstreamReadGroupUnAuthorizedTest() throws E String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + Group role = GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); getClient().perform(delete("/api/core/collections/" + collection.getID() + "/bitstreamReadGroup")) @@ -1902,8 +1905,8 @@ public void deleteCollectionDefaultBitstreamReadGroupForbiddenTest() throws Exce String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1918,8 +1921,8 @@ public void deleteCollectionDefaultBitstreamReadGroupNotFoundTest() throws Excep String bitstreamGroupString = "BITSTREAM"; int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; - Group role = collectionService.createDefaultReadGroup(context, collection, bitstreamGroupString, - defaultBitstreamRead); + GroupBuilder.createCollectionDefaultReadGroup(context, collection, + bitstreamGroupString, defaultBitstreamRead).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1931,7 +1934,7 @@ public void deleteCollectionDefaultBitstreamReadGroupNotFoundTest() throws Excep public void getWorkflowGroupForCollectionAndRole() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group group = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -1944,7 +1947,7 @@ public void getWorkflowGroupForCollectionAndRole() throws Exception { @Test public void getWorkflowGroupForCollectionAndRoleParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group group = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1958,7 +1961,7 @@ public void getWorkflowGroupForCollectionAndRoleParentCommunityAdmin() throws Ex @Test public void getWorkflowGroupForCollectionAndRoleWrongUUIDCollectionNotFound() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -1979,7 +1982,7 @@ public void getWorkflowGroupForCollectionAndRoleWrongRoleNotFound() throws Excep public void getWorkflowGroupCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group group = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -1994,7 +1997,7 @@ public void getWorkflowGroupCommunityAdmin() throws Exception { @Test public void getWorkflowGroupCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group group = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -2009,7 +2012,7 @@ public void getWorkflowGroupCollectionAdmin() throws Exception { @Test public void getWorkflowGroupUnAuthorized() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); getClient().perform(get("/api/core/collections/" + collection.getID() + "/workflowGroups/reviewer")) @@ -2019,7 +2022,7 @@ public void getWorkflowGroupUnAuthorized() throws Exception { @Test public void getWorkflowGroupForbidden() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -2324,7 +2327,7 @@ public void postCollectionWorkflowGroupCreateWorkflowGroupUnProcessablePermanent @Test public void deleteCollectionWorkflowGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -2339,7 +2342,7 @@ public void deleteCollectionWorkflowGroupTest() throws Exception { @Test public void deleteCollectionWorkflowGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -2355,7 +2358,7 @@ public void deleteCollectionWorkflowGroupTestParentCommunityAdmin() throws Excep @Test public void deleteCollectionWorkflowGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -2371,7 +2374,7 @@ public void deleteCollectionWorkflowGroupTestCollectionAdmin() throws Exception @Test public void deleteCollectionWorkflowGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group group = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); getClient().perform(delete("/api/core/collections/" + collection.getID() + "/workflowGroups/reviewer")) @@ -2387,7 +2390,7 @@ public void deleteCollectionWorkflowGroupUnAuthorizedTest() throws Exception { @Test public void deleteCollectionWorkflowGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group group = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -2406,7 +2409,7 @@ public void deleteCollectionWorkflowGroupForbiddenTest() throws Exception { @Test public void deleteCollectionWorkflowGroupNotFoundTest() throws Exception { context.turnOffAuthorisationSystem(); - Group group = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -2418,7 +2421,7 @@ public void deleteCollectionWorkflowGroupNotFoundTest() throws Exception { @Test public void deleteCollectionWorkflowGroupWithPooledTaskTest() throws Exception { context.turnOffAuthorisationSystem(); - Group reviewer = workflowService.createWorkflowRoleGroup(context, collection, "reviewer"); + Group reviewer = GroupBuilder.createCollectionWorkflowRoleGroup(context, collection, "reviewer").build(); // Submit an Item into the workflow -> moves to the "reviewer" step's pool. // The role must have at least one EPerson, otherwise the WSI gets archived immediately From 2d9988f77c52537f46888f27e35dbc0360299835 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:35:43 +0200 Subject: [PATCH 20/25] #9806: Builders for comm, coll, group in CommunityAdminGroupRestControllerIT --- .../CommunityAdminGroupRestControllerIT.java | 52 +++++++++---------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java index 37548553b14..074a7e6a355 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java @@ -34,8 +34,6 @@ import org.dspace.builder.GroupBuilder; import org.dspace.content.Collection; import org.dspace.content.Community; -import org.dspace.content.service.CollectionService; -import org.dspace.content.service.CommunityService; import org.dspace.core.Constants; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; @@ -49,19 +47,12 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerIntegrationTest { - - @Autowired - private CommunityService communityService; - @Autowired private GroupService groupService; @Autowired private AuthorizeService authorizeService; - @Autowired - private CollectionService collectionService; - @Autowired private ConfigurationService configurationService; @@ -78,7 +69,7 @@ public void setup() { @Test public void getCommunityAdminGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -91,7 +82,8 @@ public void getCommunityAdminGroupTest() throws Exception { @Test public void getCommunityAdminGroupTestCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); + // TODO: this should actually be "add member", not directly setting a policy, right? authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -106,7 +98,7 @@ public void getCommunityAdminGroupTestCommunityAdmin() throws Exception { @Test public void getCommunityAdminGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - communityService.createAdministrators(context, parentCommunity); + GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); getClient().perform(get("/api/core/communities/" + parentCommunity.getID() + "/adminGroup")) @@ -116,7 +108,7 @@ public void getCommunityAdminGroupUnAuthorizedTest() throws Exception { @Test public void getCommunityAdminGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - communityService.createAdministrators(context, parentCommunity); + GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); getClient(token).perform(get("/api/core/communities/" + parentCommunity.getID() + "/adminGroup")) @@ -379,7 +371,7 @@ public void postCommunityAdminGroupCreateAdminGroupUnProcessablePermanent() thro @Test public void deleteCommunityAdminGroupTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); String token = getAuthToken(admin.getEmail(), password); @@ -397,7 +389,7 @@ public void deleteCommunityAdminGroupTestCommunityAdmin() throws Exception { Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) .withName("Sub Community") .build(); - Group adminGroup = communityService.createAdministrators(context, child1); + GroupBuilder.createCommunityAdminGroup(context, child1).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); context.restoreAuthSystemState(); @@ -412,7 +404,7 @@ public void deleteCommunityAdminGroupTestCommunityAdmin() throws Exception { @Test public void deleteCommunityAdminGroupUnAuthorizedTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); getClient().perform(delete("/api/core/communities/" + parentCommunity.getID() + "/adminGroup")) @@ -429,7 +421,7 @@ public void deleteCommunityAdminGroupUnAuthorizedTest() throws Exception { @Test public void deleteCommunityAdminGroupForbiddenTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -449,7 +441,7 @@ public void deleteCommunityAdminGroupForbiddenTest() throws Exception { @Test public void deleteCommunityAdminGroupNotFoundTest() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); context.restoreAuthSystemState(); String token = getAuthToken(eperson.getEmail(), password); @@ -462,7 +454,7 @@ public void deleteCommunityAdminGroupNotFoundTest() throws Exception { public void communityAdminAddMembersToCommunityAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); configurationService.setProperty("core.authorization.community-admin.admin-group", false); @@ -489,7 +481,7 @@ public void communityAdminAddMembersToCommunityAdminGroupPropertySetToFalse() th public void communityAdminRemoveMembersFromCommunityAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); context.restoreAuthSystemState(); @@ -526,7 +518,7 @@ public void communityAdminRemoveMembersFromCommunityAdminGroupPropertySetToFalse public void communityAdminAddChildGroupToCommunityAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); configurationService.setProperty("core.authorization.community-admin.admin-group", false); @@ -554,7 +546,7 @@ public void communityAdminAddChildGroupToCommunityAdminGroupPropertySetToFalse() public void communityAdminRemoveChildGroupFromCommunityAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = communityService.createAdministrators(context, parentCommunity); + Group adminGroup = GroupBuilder.createCommunityAdminGroup(context, parentCommunity).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); @@ -591,7 +583,9 @@ public void communityAdminRemoveChildGroupFromCommunityAdminGroupPropertySetToFa public void communityAdminAddChildGroupToCollectionAdminGroupSuccess() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + // TODO: Why is this test in CommunityAdmin? it seems to purely be a collection group test? + // copy paste gone wrong and we should actually be testing for community admin group sub? + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); @@ -617,7 +611,9 @@ public void communityAdminAddChildGroupToCollectionAdminGroupSuccess() throws Ex public void communityAdminRemoveChildGroupFromCollectionAdminGroupSuccess() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + // TODO: Why is this test in CommunityAdmin? it seems to purely be a collection group test? + // copy paste gone wrong and we should actually be testing for community admin group sub? + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); @@ -653,7 +649,7 @@ public void communityAdminRemoveChildGroupFromCollectionAdminGroupSuccess() thro public void communityAdminAddMembersToCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); @@ -681,7 +677,7 @@ public void communityAdminAddMembersToCollectionAdminGroupPropertySetToFalse() t public void communityAdminRemoveMembersFromCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); context.restoreAuthSystemState(); @@ -719,7 +715,7 @@ public void communityAdminRemoveMembersFromCollectionAdminGroupPropertySetToFals public void communityAdminAddChildGroupToCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); @@ -748,7 +744,7 @@ public void communityAdminAddChildGroupToCollectionAdminGroupPropertySetToFalse( public void communityAdminRemoveChildGroupFromCollectionAdminGroupPropertySetToFalse() throws Exception { context.turnOffAuthorisationSystem(); - Group adminGroup = collectionService.createAdministrators(context, collection); + Group adminGroup = GroupBuilder.createCollectionAdminGroup(context, collection).build(); authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); context.restoreAuthSystemState(); From 8cfb433c40ad01f348461a2c656665d1891de398 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 15:58:59 +0200 Subject: [PATCH 21/25] #9806: Update object cleanup in GroupRestRepositoryIT --- .../app/rest/GroupRestRepositoryIT.java | 952 +++++------------- 1 file changed, 239 insertions(+), 713 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java index bdd13b97ff7..ff8aea493ae 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java @@ -59,14 +59,11 @@ import org.dspace.content.Community; import org.dspace.content.Item; import org.dspace.content.MetadataSchemaEnum; -import org.dspace.content.factory.ContentServiceFactory; -import org.dspace.content.service.CommunityService; import org.dspace.core.Constants; import org.dspace.core.I18nUtil; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; -import org.dspace.eperson.service.EPersonService; import org.dspace.eperson.service.GroupService; import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; @@ -757,263 +754,136 @@ public void addChildGroupTest() throws Exception { @Test public void addChildGroupCommunityAdminTest() throws Exception { - CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - Community community = null; - - Group parentGroup = null; - Group childGroup1 = null; - Group childGroup2 = null; - - try { - context.turnOffAuthorisationSystem(); - - community = CommunityBuilder.createCommunity(context).build(); - parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) - .addMember(eperson) - .build(); - childGroup1 = GroupBuilder.createGroup(context).build(); - childGroup2 = GroupBuilder.createGroup(context).build(); - - groupService.update(context, parentGroup); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup1 = context.reloadEntity(childGroup1); - childGroup2 = context.reloadEntity(childGroup2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(eperson.getEmail(), password); - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() - ) - ).andExpect(status().isNoContent()); + Community community = CommunityBuilder.createCommunity(context).build(); + Group parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + .addMember(eperson) + .build(); + Group childGroup1 = GroupBuilder.createGroup(context).build(); + Group childGroup2 = GroupBuilder.createGroup(context).build(); - parentGroup = context.reloadEntity(parentGroup); - childGroup1 = context.reloadEntity(childGroup1); - childGroup2 = context.reloadEntity(childGroup2); + context.restoreAuthSystemState(); + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() + ) + ).andExpect(status().isNoContent()); - assertTrue( - groupService.isMember(parentGroup, childGroup1) - ); + parentGroup = context.reloadEntity(parentGroup); + childGroup1 = context.reloadEntity(childGroup1); + childGroup2 = context.reloadEntity(childGroup2); - assertTrue( - groupService.isMember(parentGroup, childGroup2) - ); + assertTrue( + groupService.isMember(parentGroup, childGroup1) + ); - } finally { - // TODO: Can we remove these lines now that we are creating them with the builder? - if (community != null) { - CommunityBuilder.deleteCommunity(community.getID()); - } - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup1 != null) { - GroupBuilder.deleteGroup(childGroup1.getID()); - } - if (childGroup2 != null) { - GroupBuilder.deleteGroup(childGroup2.getID()); - } - } + assertTrue( + groupService.isMember(parentGroup, childGroup2) + ); } @Test public void addChildGroupForbiddenTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup1 = null; - Group childGroup2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup1 = GroupBuilder.createGroup(context).build(); - childGroup2 = GroupBuilder.createGroup(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup1 = context.reloadEntity(childGroup1); - childGroup2 = context.reloadEntity(childGroup2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(eperson.getEmail(), password); - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() - ) - ).andExpect(status().isForbidden()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup1 = GroupBuilder.createGroup(context).build(); + Group childGroup2 = GroupBuilder.createGroup(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup1 != null) { - GroupBuilder.deleteGroup(childGroup1.getID()); - } - if (childGroup2 != null) { - GroupBuilder.deleteGroup(childGroup2.getID()); - } - } + context.restoreAuthSystemState(); + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() + ) + ).andExpect(status().isForbidden()); } @Test public void addChildGroupUnauthorizedTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup1 = null; - Group childGroup2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup1 = GroupBuilder.createGroup(context).build(); - childGroup2 = GroupBuilder.createGroup(context).build(); + context.turnOffAuthorisationSystem(); - context.commit(); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup1 = GroupBuilder.createGroup(context).build(); + Group childGroup2 = GroupBuilder.createGroup(context).build(); - parentGroup = context.reloadEntity(parentGroup); - childGroup1 = context.reloadEntity(childGroup1); - childGroup2 = context.reloadEntity(childGroup2); + context.commit(); - context.restoreAuthSystemState(); - getClient().perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() - ) - ).andExpect(status().isUnauthorized()); + parentGroup = context.reloadEntity(parentGroup); + childGroup1 = context.reloadEntity(childGroup1); + childGroup2 = context.reloadEntity(childGroup2); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup1 != null) { - GroupBuilder.deleteGroup(childGroup1.getID()); - } - if (childGroup2 != null) { - GroupBuilder.deleteGroup(childGroup2.getID()); - } - } + context.restoreAuthSystemState(); + getClient().perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() + ) + ).andExpect(status().isUnauthorized()); } @Test public void addChildGroupNotFoundTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup1 = null; - Group childGroup2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup1 = GroupBuilder.createGroup(context).build(); - childGroup2 = GroupBuilder.createGroup(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup1 = context.reloadEntity(childGroup1); - childGroup2 = context.reloadEntity(childGroup2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform( - post("/api/eperson/groups/" + UUID.randomUUID() + "/subgroups") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() - ) - ).andExpect(status().isNotFound()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup1 = GroupBuilder.createGroup(context).build(); + Group childGroup2 = GroupBuilder.createGroup(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup1 != null) { - GroupBuilder.deleteGroup(childGroup1.getID()); - } - if (childGroup2 != null) { - GroupBuilder.deleteGroup(childGroup2.getID()); - } - } + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform( + post("/api/eperson/groups/" + UUID.randomUUID() + "/subgroups") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() + ) + ).andExpect(status().isNotFound()); } @Test public void addChildGroupUnprocessableTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup1 = null; - Group childGroup2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup1 = GroupBuilder.createGroup(context) - .withParent(parentGroup) - .build(); - childGroup2 = GroupBuilder.createGroup(context).build(); - groupService.update(context, childGroup1); - -// context.commit(); -// -// parentGroup = context.reloadEntity(parentGroup); -// childGroup1 = context.reloadEntity(childGroup1); -// childGroup2 = context.reloadEntity(childGroup2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup1 = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); + Group childGroup2 = GroupBuilder.createGroup(context).build(); - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/123456789\n" - + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() - ) - ).andExpect(status().isUnprocessableEntity()); + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); - // TODO - confirm with reviewers that this is a mistake - it actually should be No Content - // (see AddMember test) but was incorrectly expecting 422? - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() - ) - ).andExpect(status().isNoContent()); + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/123456789\n" + + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() + ) + ).andExpect(status().isUnprocessableEntity()); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup1 != null) { - GroupBuilder.deleteGroup(childGroup1.getID()); - } - if (childGroup2 != null) { - GroupBuilder.deleteGroup(childGroup2.getID()); - } - } + // TODO - confirm with reviewers that this is a mistake - it actually should be No Content + // (see AddMember test) but was incorrectly expecting 422? + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID() + ) + ).andExpect(status().isNoContent()); } @Test @@ -1081,251 +951,118 @@ public void addMemberTest() throws Exception { @Test public void addMemberCommunityAdminTest() throws Exception { - CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Community community = null; - Group parentGroup = null; - EPerson member1 = null; - EPerson member2 = null; - - try { - context.turnOffAuthorisationSystem(); - - community = CommunityBuilder.createCommunity(context).build(); - parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) - .addMember(eperson) - .build(); - member1 = EPersonBuilder.createEPerson(context).build(); - member2 = EPersonBuilder.createEPerson(context).build(); - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member1 = context.reloadEntity(member1); - member2 = context.reloadEntity(member2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(eperson.getEmail(), password); - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + member2.getID() - ) - ).andExpect(status().isNoContent()); + Community community = CommunityBuilder.createCommunity(context).build(); + Group parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + .addMember(eperson) + .build(); + EPerson member1 = EPersonBuilder.createEPerson(context).build(); + EPerson member2 = EPersonBuilder.createEPerson(context).build(); - parentGroup = context.reloadEntity(parentGroup); - member1 = context.reloadEntity(member1); - member2 = context.reloadEntity(member2); + context.restoreAuthSystemState(); + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + member2.getID() + ) + ).andExpect(status().isNoContent()); - assertTrue( - groupService.isMember(context, member1, parentGroup) - ); + parentGroup = context.reloadEntity(parentGroup); + member1 = context.reloadEntity(member1); + member2 = context.reloadEntity(member2); - assertTrue( - groupService.isMember(context, member2, parentGroup) - ); + assertTrue( + groupService.isMember(context, member1, parentGroup) + ); - } finally { - if (community != null) { - CommunityBuilder.deleteCommunity(community.getID()); - } - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member1 != null) { - EPersonBuilder.deleteEPerson(member1.getID()); - } - if (member2 != null) { - EPersonBuilder.deleteEPerson(member2.getID()); - } - } + assertTrue( + groupService.isMember(context, member2, parentGroup) + ); } @Test public void addMemberForbiddenTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member1 = null; - EPerson member2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - member1 = EPersonBuilder.createEPerson(context).build(); - member2 = EPersonBuilder.createEPerson(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member1 = context.reloadEntity(member1); - member2 = context.reloadEntity(member2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(eperson.getEmail(), password); - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + member2.getID() - ) - ).andExpect(status().isForbidden()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + EPerson member1 = EPersonBuilder.createEPerson(context).build(); + EPerson member2 = EPersonBuilder.createEPerson(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member1 != null) { - EPersonBuilder.deleteEPerson(member1.getID()); - } - if (member2 != null) { - EPersonBuilder.deleteEPerson(member2.getID()); - } - } + context.restoreAuthSystemState(); + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + member2.getID() + ) + ).andExpect(status().isForbidden()); } @Test public void addMemberUnauthorizedTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member1 = null; - EPerson member2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - member1 = EPersonBuilder.createEPerson(context).build(); - member2 = EPersonBuilder.createEPerson(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member1 = context.reloadEntity(member1); - member2 = context.reloadEntity(member2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - getClient().perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + member2.getID() - ) - ).andExpect(status().isUnauthorized()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + EPerson member1 = EPersonBuilder.createEPerson(context).build(); + EPerson member2 = EPersonBuilder.createEPerson(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member1 != null) { - EPersonBuilder.deleteEPerson(member1.getID()); - } - if (member2 != null) { - EPersonBuilder.deleteEPerson(member2.getID()); - } - } + context.restoreAuthSystemState(); + getClient().perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + member2.getID() + ) + ).andExpect(status().isUnauthorized()); } @Test public void addMemberNotFoundTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member1 = null; - EPerson member2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - member1 = EPersonBuilder.createEPerson(context).build(); - member2 = EPersonBuilder.createEPerson(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member1 = context.reloadEntity(member1); - member2 = context.reloadEntity(member2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform( - post("/api/eperson/groups/" + UUID.randomUUID() + "/epersons") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" - + REST_SERVER_URL + "eperson/groups/" + member2.getID() - ) - ).andExpect(status().isNotFound()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + EPerson member1 = EPersonBuilder.createEPerson(context).build(); + EPerson member2 = EPersonBuilder.createEPerson(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member1 != null) { - EPersonBuilder.deleteEPerson(member1.getID()); - } - if (member2 != null) { - EPersonBuilder.deleteEPerson(member2.getID()); - } - } + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform( + post("/api/eperson/groups/" + UUID.randomUUID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + member2.getID() + ) + ).andExpect(status().isNotFound()); } @Test public void addMemberUnprocessableTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member1 = null; - EPerson member2 = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - member1 = EPersonBuilder.createEPerson(context).build(); - member2 = EPersonBuilder.createEPerson(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member1 = context.reloadEntity(member1); - member2 = context.reloadEntity(member2); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); + Group parentGroup = GroupBuilder.createGroup(context).build(); + EPerson member1 = EPersonBuilder.createEPerson(context).build(); + EPerson member2 = EPersonBuilder.createEPerson(context).build(); - getClient(authToken).perform( - post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/123456789\n" - + REST_SERVER_URL + "eperson/groups/" + member2.getID() - ) - ).andExpect(status().isUnprocessableEntity()); + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member1 != null) { - EPersonBuilder.deleteEPerson(member1.getID()); - } - if (member2 != null) { - EPersonBuilder.deleteEPerson(member2.getID()); - } - } + getClient(authToken).perform( + post("/api/eperson/groups/" + parentGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/123456789\n" + + REST_SERVER_URL + "eperson/groups/" + member2.getID() + ) + ).andExpect(status().isUnprocessableEntity()); } @Test @@ -1378,29 +1115,18 @@ public void removeChildGroupTest() throws Exception { @Test public void removeChildGroupCommunityAdminTest() throws Exception { - CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - Community community = null; - Group parentGroup = null; - Group childGroup = null; - - try { context.turnOffAuthorisationSystem(); - community = CommunityBuilder.createCommunity(context).build(); - parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + Community community = CommunityBuilder.createCommunity(context).build(); + Group parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) .addMember(eperson) .build(); - childGroup = GroupBuilder.createGroup(context) + Group childGroup = GroupBuilder.createGroup(context) .withParent(parentGroup) .build(); - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup = context.reloadEntity(childGroup); - context.restoreAuthSystemState(); String authToken = getAuthToken(eperson.getEmail(), password); getClient(authToken).perform( @@ -1413,163 +1139,71 @@ public void removeChildGroupCommunityAdminTest() throws Exception { assertFalse( groupService.isMember(parentGroup, childGroup) ); - - } finally { - if (community != null) { - CommunityBuilder.deleteCommunity(community.getID()); - } - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup != null) { - GroupBuilder.deleteGroup(childGroup.getID()); - } - } } @Test public void removeChildGroupForbiddenTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup = GroupBuilder.createGroup(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup = context.reloadEntity(childGroup); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(eperson.getEmail(), password); - getClient(authToken).perform( - delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroup.getID()) - ).andExpect(status().isForbidden()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup = GroupBuilder.createGroup(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup != null) { - GroupBuilder.deleteGroup(childGroup.getID()); - } - } + context.restoreAuthSystemState(); + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform( + delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroup.getID()) + ).andExpect(status().isForbidden()); } @Test public void removeChildGroupUnauthorizedTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup = GroupBuilder.createGroup(context).build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup = context.reloadEntity(childGroup); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - getClient().perform( - delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroup.getID()) - ).andExpect(status().isUnauthorized()); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup = GroupBuilder.createGroup(context).build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup != null) { - GroupBuilder.deleteGroup(childGroup.getID()); - } - } + context.restoreAuthSystemState(); + getClient().perform( + delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroup.getID()) + ).andExpect(status().isUnauthorized()); } @Test public void removeChildGroupNotFoundTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup = GroupBuilder.createGroup(context) - .withParent(parentGroup) - .build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup = context.reloadEntity(childGroup); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); - getClient(authToken).perform( - delete("/api/eperson/groups/" + UUID.randomUUID() + "/subgroups/" + childGroup.getID()) - ).andExpect(status().isNotFound()); + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup != null) { - GroupBuilder.deleteGroup(childGroup.getID()); - } - } + getClient(authToken).perform( + delete("/api/eperson/groups/" + UUID.randomUUID() + "/subgroups/" + childGroup.getID()) + ).andExpect(status().isNotFound()); } @Test public void removeChildGroupUnprocessableTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - - Group parentGroup = null; - Group childGroup = null; - - try { - context.turnOffAuthorisationSystem(); - - parentGroup = GroupBuilder.createGroup(context).build(); - childGroup = GroupBuilder.createGroup(context) - .withParent(parentGroup) - .build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - childGroup = context.reloadEntity(childGroup); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); + Group parentGroup = GroupBuilder.createGroup(context).build(); + Group childGroup = GroupBuilder.createGroup(context) + .withParent(parentGroup) + .build(); - getClient(authToken).perform( - delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + UUID.randomUUID()) - ).andExpect(status().isUnprocessableEntity()); + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (childGroup != null) { - GroupBuilder.deleteGroup(childGroup.getID()); - } - } + getClient(authToken).perform( + delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + UUID.randomUUID()) + ).andExpect(status().isUnprocessableEntity()); } @Test @@ -1613,31 +1247,19 @@ public void removeMemberTest() throws Exception { @Test public void removeMemberCommunityAdminTest() throws Exception { - CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Community community = null; - Group parentGroup = null; - EPerson member = null; - try { context.turnOffAuthorisationSystem(); - community = CommunityBuilder.createCommunity(context).build(); - member = EPersonBuilder.createEPerson(context).build(); - parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) + Community community = CommunityBuilder.createCommunity(context).build(); + EPerson member = EPersonBuilder.createEPerson(context).build(); + Group parentGroup = GroupBuilder.createCommunityAdminGroup(context, community) .addMember(member) .addMember(eperson) .build(); assertTrue(groupService.isMember(context, member, parentGroup)); - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member = context.reloadEntity(member); - context.restoreAuthSystemState(); String authToken = getAuthToken(eperson.getEmail(), password); getClient(authToken).perform( @@ -1650,171 +1272,75 @@ public void removeMemberCommunityAdminTest() throws Exception { assertFalse( groupService.isMember(context, member, parentGroup) ); - - } finally { - if (community != null) { - CommunityBuilder.deleteCommunity(community.getID()); - } - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member != null) { - EPersonBuilder.deleteEPerson(member.getID()); - } - } } @Test public void removeMemberForbiddenTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member = null; - - try { - context.turnOffAuthorisationSystem(); - - member = EPersonBuilder.createEPerson(context).build(); - parentGroup = GroupBuilder.createGroup(context) - .addMember(member) - .build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member = context.reloadEntity(member); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(eperson.getEmail(), password); - getClient(authToken).perform( - delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + member.getID()) - ).andExpect(status().isForbidden()); + EPerson member = EPersonBuilder.createEPerson(context).build(); + Group parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member != null) { - EPersonBuilder.deleteEPerson(member.getID()); - } - } + context.restoreAuthSystemState(); + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform( + delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + member.getID()) + ).andExpect(status().isForbidden()); } @Test public void removeMemberUnauthorizedTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member = null; - - try { - context.turnOffAuthorisationSystem(); - - member = EPersonBuilder.createEPerson(context).build(); - parentGroup = GroupBuilder.createGroup(context) - .addMember(member) - .build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member = context.reloadEntity(member); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - getClient().perform( - delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + member.getID()) - ).andExpect(status().isUnauthorized()); + EPerson member = EPersonBuilder.createEPerson(context).build(); + Group parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member != null) { - EPersonBuilder.deleteEPerson(member.getID()); - } - } + context.restoreAuthSystemState(); + getClient().perform( + delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + member.getID()) + ).andExpect(status().isUnauthorized()); } @Test public void removeMemberNotFoundTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member = null; - - try { - context.turnOffAuthorisationSystem(); - - member = EPersonBuilder.createEPerson(context).build(); - parentGroup = GroupBuilder.createGroup(context) - .addMember(member) - .build(); - - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member = context.reloadEntity(member); + context.turnOffAuthorisationSystem(); - context.restoreAuthSystemState(); - String authToken = getAuthToken(admin.getEmail(), password); + EPerson member = EPersonBuilder.createEPerson(context).build(); + Group parentGroup = GroupBuilder.createGroup(context) + .addMember(member) + .build(); - getClient(authToken).perform( - delete("/api/eperson/groups/" + UUID.randomUUID() + "/epersons/" + member.getID()) - ).andExpect(status().isNotFound()); + context.restoreAuthSystemState(); + String authToken = getAuthToken(admin.getEmail(), password); - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member != null) { - EPersonBuilder.deleteEPerson(member.getID()); - } - } + getClient(authToken).perform( + delete("/api/eperson/groups/" + UUID.randomUUID() + "/epersons/" + member.getID()) + ).andExpect(status().isNotFound()); } @Test public void removeMemberUnprocessableTest() throws Exception { - GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); - - Group parentGroup = null; - EPerson member = null; - - try { context.turnOffAuthorisationSystem(); - member = EPersonBuilder.createEPerson(context).build(); - parentGroup = GroupBuilder.createGroup(context) + EPerson member = EPersonBuilder.createEPerson(context).build(); + Group parentGroup = GroupBuilder.createGroup(context) .addMember(member) .build(); - context.commit(); - - parentGroup = context.reloadEntity(parentGroup); - member = context.reloadEntity(member); - context.restoreAuthSystemState(); String authToken = getAuthToken(admin.getEmail(), password); getClient(authToken).perform( delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + UUID.randomUUID()) ).andExpect(status().isUnprocessableEntity()); - - } finally { - if (parentGroup != null) { - GroupBuilder.deleteGroup(parentGroup.getID()); - } - if (member != null) { - EPersonBuilder.deleteEPerson(member.getID()); - } - } } @Test From 90536e443b7fedef0481a34326cafc58fb334396 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 16:43:36 +0200 Subject: [PATCH 22/25] #9806: Align provider reg in CreateMissingIdentifiersIT with other tests VersionedHandlerIdentifierProviderIT uses this registerProvider method which looks more reliable and doesn't do a refresh/reload of applicationContext after (which I suspected might have an odd interaction with VersioningWithRelationshipsIT and its createBean() calls?) --- .../general/CreateMissingIdentifiersIT.java | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/ctask/general/CreateMissingIdentifiersIT.java b/dspace-api/src/test/java/org/dspace/ctask/general/CreateMissingIdentifiersIT.java index 2a07799deee..8038a715332 100644 --- a/dspace-api/src/test/java/org/dspace/ctask/general/CreateMissingIdentifiersIT.java +++ b/dspace-api/src/test/java/org/dspace/ctask/general/CreateMissingIdentifiersIT.java @@ -10,6 +10,8 @@ import static org.junit.Assert.assertEquals; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import org.dspace.AbstractIntegrationTestWithDatabase; import org.dspace.builder.CollectionBuilder; @@ -19,7 +21,10 @@ import org.dspace.content.Item; import org.dspace.core.factory.CoreServiceFactory; import org.dspace.curate.Curator; +import org.dspace.identifier.IdentifierProvider; +import org.dspace.identifier.IdentifierServiceImpl; import org.dspace.identifier.VersionedHandleIdentifierProviderWithCanonicalHandles; +import org.dspace.kernel.ServiceManager; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; import org.junit.After; @@ -32,10 +37,23 @@ */ public class CreateMissingIdentifiersIT extends AbstractIntegrationTestWithDatabase { + private ServiceManager serviceManager; + private IdentifierServiceImpl identifierService; private static final String P_TASK_DEF = "plugin.named.org.dspace.curate.CurationTask"; private static final String TASK_NAME = "test"; + @Override + public void setUp() throws Exception { + super.setUp(); + context.turnOffAuthorisationSystem(); + + serviceManager = DSpaceServicesFactory.getInstance().getServiceManager(); + identifierService = serviceManager.getServicesByType(IdentifierServiceImpl.class).get(0); + // Clean out providers to avoid any being used for creation of community and collection + identifierService.setProviders(new ArrayList<>()); + } + @Test public void testPerform() throws IOException { @@ -67,11 +85,7 @@ public void testPerform() /* * Now install an incompatible provider to make the task fail. */ - DSpaceServicesFactory.getInstance() - .getServiceManager() - .registerServiceClass( - VersionedHandleIdentifierProviderWithCanonicalHandles.class.getCanonicalName(), - VersionedHandleIdentifierProviderWithCanonicalHandles.class); + registerProvider(VersionedHandleIdentifierProviderWithCanonicalHandles.class); curator.curate(context, item); System.out.format("With incompatible provider, result is '%s'.\n", @@ -86,4 +100,14 @@ public void destroy() throws Exception { super.destroy(); DSpaceServicesFactory.getInstance().getServiceManager().getApplicationContext().refresh(); } + + private void registerProvider(Class type) { + // Register our new provider + serviceManager.registerServiceClass(type.getName(), type); + IdentifierProvider identifierProvider = + (IdentifierProvider) serviceManager.getServiceByName(type.getName(), type); + + // Overwrite the identifier-service's providers with the new one to ensure only this provider is used + identifierService.setProviders(List.of(identifierProvider)); + } } From 3521ab6d3598e716cc9fe8e938e883b302006580 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 16:45:05 +0200 Subject: [PATCH 23/25] #9806: Use builders for creation in VersioningWithRelationshipsIT I am a bit uncertain about the createBean() calls here, why do we not simply *get* the configured beans using the service manager instead, but will look at that in a separate change --- .../VersioningWithRelationshipsIT.java | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java b/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java index 44653300e0d..accc52d0d83 100644 --- a/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java +++ b/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java @@ -49,6 +49,7 @@ import org.dspace.builder.ItemBuilder; import org.dspace.builder.RelationshipBuilder; import org.dspace.builder.RelationshipTypeBuilder; +import org.dspace.builder.VersionBuilder; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.InstallItemService; import org.dspace.content.service.ItemService; @@ -62,8 +63,6 @@ import org.dspace.kernel.ServiceManager; import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.versioning.Version; -import org.dspace.versioning.factory.VersionServiceFactory; -import org.dspace.versioning.service.VersioningService; import org.hamcrest.Matcher; import org.junit.Assert; import org.junit.Before; @@ -74,8 +73,6 @@ public class VersioningWithRelationshipsIT extends AbstractIntegrationTestWithDa private final RelationshipService relationshipService = ContentServiceFactory.getInstance().getRelationshipService(); - private final VersioningService versioningService = - VersionServiceFactory.getInstance().getVersionService(); private final WorkspaceItemService workspaceItemService = ContentServiceFactory.getInstance().getWorkspaceItemService(); private final InstallItemService installItemService = @@ -291,7 +288,7 @@ public void test_createNewVersionOfItemOnLeftSideOfRelationships() throws Except // create a new version of the publication // ///////////////////////////////////////////// - Version newVersion = versioningService.createNewVersion(context, originalPublication); + Version newVersion = VersionBuilder.createVersion(context, originalPublication, "test").build(); Item newPublication = newVersion.getItem(); assertNotSame(originalPublication, newPublication); @@ -567,7 +564,7 @@ public void test_createNewVersionOfItemAndModifyRelationships() throws Exception // create a new version of the publication // ///////////////////////////////////////////// - Version newVersion = versioningService.createNewVersion(context, originalPublication); + Version newVersion = VersionBuilder.createVersion(context, originalPublication, "test").build(); Item newPublication = newVersion.getItem(); assertNotSame(originalPublication, newPublication); @@ -927,7 +924,7 @@ public void test_createNewVersionOfItemOnRightSideOfRelationships() throws Excep // create a new version of the person // //////////////////////////////////////// - Version newVersion = versioningService.createNewVersion(context, originalPerson); + Version newVersion = VersionBuilder.createVersion(context, originalPerson, "test").build(); Item newPerson = newVersion.getItem(); assertNotSame(originalPerson, newPerson); @@ -1300,7 +1297,7 @@ public void test_createNewVersionOfItemAndVerifyMetadataOrder() throws Exception // create new version of publication // /////////////////////////////////////// - Version newVersion = versioningService.createNewVersion(context, originalPublication); + Version newVersion = VersionBuilder.createVersion(context, originalPublication, "test").build(); Item newPublication = newVersion.getItem(); assertNotSame(originalPublication, newPublication); @@ -1463,7 +1460,7 @@ public void test_createNewVersionOfItemWithAddRemoveMove() throws Exception { // create a new version of the publication // ///////////////////////////////////////////// - Version newVersion = versioningService.createNewVersion(context, originalPublication); + Version newVersion = VersionBuilder.createVersion(context, originalPublication, "test").build(); Item newPublication = newVersion.getItem(); assertNotSame(originalPublication, newPublication); @@ -1782,7 +1779,7 @@ public void test_placeRecalculationAfterDelete() throws Exception { // create new version - volume 1.2 // ///////////////////////////////////// - Item v1_2 = versioningService.createNewVersion(context, v1_1).getItem(); + Item v1_2 = VersionBuilder.createVersion(context, v1_1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, v1_2)); context.commit(); @@ -1790,7 +1787,7 @@ public void test_placeRecalculationAfterDelete() throws Exception { // create new version - issue 3.2 // //////////////////////////////////// - Item i3_2 = versioningService.createNewVersion(context, i3_1).getItem(); + Item i3_2 = VersionBuilder.createVersion(context, i3_1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, i3_2)); context.commit(); @@ -2316,7 +2313,7 @@ public void test_placeRecalculationAfterDelete_complex() throws Exception { // create new version - person 3.2 // ///////////////////////////////////// - Item pe3_2 = versioningService.createNewVersion(context, pe3_1).getItem(); + Item pe3_2 = VersionBuilder.createVersion(context, pe3_1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, pe3_2)); context.commit(); @@ -2324,7 +2321,7 @@ public void test_placeRecalculationAfterDelete_complex() throws Exception { // create new version - project 3.2 // ////////////////////////////////////// - Item pr3_2 = versioningService.createNewVersion(context, pr3_1).getItem(); + Item pr3_2 = VersionBuilder.createVersion(context, pr3_1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, pr3_2)); context.commit(); @@ -3056,7 +3053,7 @@ public void test_placeRecalculationNoUseForPlace() throws Exception { // create new version - volume 1.2 // ///////////////////////////////////// - Item v1_2 = versioningService.createNewVersion(context, v1_1).getItem(); + Item v1_2 = VersionBuilder.createVersion(context, v1_1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, v1_2)); context.commit(); @@ -3064,7 +3061,7 @@ public void test_placeRecalculationNoUseForPlace() throws Exception { // create new version - issue 3.2 // //////////////////////////////////// - Item i3_2 = versioningService.createNewVersion(context, i3_1).getItem(); + Item i3_2 = VersionBuilder.createVersion(context, i3_1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, i3_2)); context.commit(); @@ -3509,7 +3506,7 @@ public void test_virtualMetadataPreserved() throws Exception { // create a new version of publication 1 and archive // /////////////////////////////////////////////////////// - Item publication1V2 = versioningService.createNewVersion(context, publication1V1).getItem(); + Item publication1V2 = VersionBuilder.createVersion(context, publication1V1, "test").build().getItem(); installItemService.installItem(context, workspaceItemService.findByItem(context, publication1V2)); context.dispatchEvents(); @@ -3517,7 +3514,7 @@ public void test_virtualMetadataPreserved() throws Exception { // create new version of person 1 // //////////////////////////////////// - Item person1V2 = versioningService.createNewVersion(context, person1V1).getItem(); + Item person1V2 = VersionBuilder.createVersion(context, person1V1, "test").build().getItem(); // update "Smith, Donald" to "Smith, D." itemService.replaceMetadata( context, person1V2, "person", "givenName", null, null, "D.", @@ -3853,7 +3850,7 @@ public void test_virtualMetadataPreserved() throws Exception { // create new version of person 2 // //////////////////////////////////// - Item person2V2 = versioningService.createNewVersion(context, person2V1).getItem(); + Item person2V2 = VersionBuilder.createVersion(context, person2V1, "test").build().getItem(); Relationship rel1 = getRelationship(publication1V2, isAuthorOfPublication, person2V2); assertNotNull(rel1); rel1.setRightwardValue("Doe, Jane Jr"); From 4af690065038d9cb401d0fa1fc5fbfe0fa1fe2c7 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 17:15:15 +0200 Subject: [PATCH 24/25] #9806: Set explicit id provider before VersioningWithRelationshipsIT --- .../VersioningWithRelationshipsIT.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java b/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java index accc52d0d83..10cb30cd52b 100644 --- a/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java +++ b/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java @@ -60,6 +60,9 @@ import org.dspace.content.virtual.VirtualMetadataPopulator; import org.dspace.core.Constants; import org.dspace.discovery.SolrSearchCore; +import org.dspace.identifier.IdentifierProvider; +import org.dspace.identifier.IdentifierServiceImpl; +import org.dspace.identifier.VersionedHandleIdentifierProvider; import org.dspace.kernel.ServiceManager; import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.versioning.Version; @@ -81,7 +84,7 @@ public class VersioningWithRelationshipsIT extends AbstractIntegrationTestWithDa ContentServiceFactory.getInstance().getItemService(); private final SolrSearchCore solrSearchCore = DSpaceServicesFactory.getInstance().getServiceManager().getServicesByType(SolrSearchCore.class).get(0); - + private IdentifierServiceImpl identifierService; protected Community community; protected Collection collection; protected EntityType publicationEntityType; @@ -98,6 +101,22 @@ public class VersioningWithRelationshipsIT extends AbstractIntegrationTestWithDa protected RelationshipType isIssueOfJournalVolume; protected RelationshipType isProjectOfPerson; + private void registerProvider(Class type) { + // Register our new provider + IdentifierProvider identifierProvider = + (IdentifierProvider) DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName(type.getName(), type); + if (identifierProvider == null) { + DSpaceServicesFactory.getInstance().getServiceManager().registerServiceClass(type.getName(), type); + identifierProvider = (IdentifierProvider) DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName(type.getName(), type); + } + + // Overwrite the identifier-service's providers with the new one to ensure only this provider is used + identifierService = DSpaceServicesFactory.getInstance().getServiceManager() + .getServicesByType(IdentifierServiceImpl.class).get(0); + identifierService.setProviders(new ArrayList<>()); + identifierService.setProviders(List.of(identifierProvider)); + } + @Override @Before public void setUp() throws Exception { @@ -105,6 +124,9 @@ public void setUp() throws Exception { context.turnOffAuthorisationSystem(); + + registerProvider(VersionedHandleIdentifierProvider.class); + community = CommunityBuilder.createCommunity(context) .withName("community") .build(); From f6cabe648dfcda786afdf7d5411dc3d7ad85c405 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Fri, 6 Sep 2024 17:58:23 +0200 Subject: [PATCH 25/25] #9806: Move cleanup of handle provider to destroy in VersionedHandleIdentifierProviderIT --- .../VersioningWithRelationshipsIT.java | 23 ------------------- .../VersionedHandleIdentifierProviderIT.java | 22 ++++++++++++++++-- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java b/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java index 10cb30cd52b..3acc4ca146e 100644 --- a/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java +++ b/dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsIT.java @@ -60,9 +60,6 @@ import org.dspace.content.virtual.VirtualMetadataPopulator; import org.dspace.core.Constants; import org.dspace.discovery.SolrSearchCore; -import org.dspace.identifier.IdentifierProvider; -import org.dspace.identifier.IdentifierServiceImpl; -import org.dspace.identifier.VersionedHandleIdentifierProvider; import org.dspace.kernel.ServiceManager; import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.versioning.Version; @@ -84,7 +81,6 @@ public class VersioningWithRelationshipsIT extends AbstractIntegrationTestWithDa ContentServiceFactory.getInstance().getItemService(); private final SolrSearchCore solrSearchCore = DSpaceServicesFactory.getInstance().getServiceManager().getServicesByType(SolrSearchCore.class).get(0); - private IdentifierServiceImpl identifierService; protected Community community; protected Collection collection; protected EntityType publicationEntityType; @@ -101,22 +97,6 @@ public class VersioningWithRelationshipsIT extends AbstractIntegrationTestWithDa protected RelationshipType isIssueOfJournalVolume; protected RelationshipType isProjectOfPerson; - private void registerProvider(Class type) { - // Register our new provider - IdentifierProvider identifierProvider = - (IdentifierProvider) DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName(type.getName(), type); - if (identifierProvider == null) { - DSpaceServicesFactory.getInstance().getServiceManager().registerServiceClass(type.getName(), type); - identifierProvider = (IdentifierProvider) DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName(type.getName(), type); - } - - // Overwrite the identifier-service's providers with the new one to ensure only this provider is used - identifierService = DSpaceServicesFactory.getInstance().getServiceManager() - .getServicesByType(IdentifierServiceImpl.class).get(0); - identifierService.setProviders(new ArrayList<>()); - identifierService.setProviders(List.of(identifierProvider)); - } - @Override @Before public void setUp() throws Exception { @@ -124,9 +104,6 @@ public void setUp() throws Exception { context.turnOffAuthorisationSystem(); - - registerProvider(VersionedHandleIdentifierProvider.class); - community = CommunityBuilder.createCommunity(context) .withName("community") .build(); diff --git a/dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java b/dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java index 7e549f6cae3..57acf1f1c45 100644 --- a/dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java +++ b/dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java @@ -24,6 +24,7 @@ import org.dspace.content.Item; import org.dspace.kernel.ServiceManager; import org.dspace.services.factory.DSpaceServicesFactory; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -57,13 +58,30 @@ public void setUp() throws Exception { .build(); } + @After + @Override + public void destroy() throws Exception { + super.destroy(); + // After this test has finished running, refresh application context and + // set the expected 'default' versioned handle provider back to ensure other tests don't fail + DSpaceServicesFactory.getInstance().getServiceManager().getApplicationContext().refresh(); + } + private void registerProvider(Class type) { // Register our new provider - serviceManager.registerServiceClass(type.getName(), type); IdentifierProvider identifierProvider = - (IdentifierProvider) serviceManager.getServiceByName(type.getName(), type); + (IdentifierProvider) DSpaceServicesFactory.getInstance().getServiceManager() + .getServiceByName(type.getName(), type); + if (identifierProvider == null) { + DSpaceServicesFactory.getInstance().getServiceManager().registerServiceClass(type.getName(), type); + identifierProvider = (IdentifierProvider) DSpaceServicesFactory.getInstance().getServiceManager() + .getServiceByName(type.getName(), type); + } // Overwrite the identifier-service's providers with the new one to ensure only this provider is used + identifierService = DSpaceServicesFactory.getInstance().getServiceManager() + .getServicesByType(IdentifierServiceImpl.class).get(0); + identifierService.setProviders(new ArrayList<>()); identifierService.setProviders(List.of(identifierProvider)); }