CLARIN-DSpace v9/Port #1350 + #1347 + #1338 (DOI dedup, Shib special groups, tgz preview) to the v9 base

dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java

@@ -277,7 +277,7 @@ public int authenticate(Context context, String username, String password,
 
             // Step 4: Log the user in.
             context.setCurrentUser(eperson);
-            request.getSession().setAttribute("shib.authenticated", true);
+            request.setAttribute("shib.authenticated", true);
             AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson);
 
             log.info(eperson.getEmail() + " has been authenticated via shibboleth.");
@@ -330,42 +330,35 @@ public int authenticate(Context context, String username, String password,
     @Override
     public List<Group> getSpecialGroups(Context context, HttpServletRequest request) {
         try {
-            // User has not successfuly authenticated via shibboleth.
-            if (request == null ||
-                    context.getCurrentUser() == null ||
-                    request.getSession().getAttribute("shib.authenticated") == null) {
-                return Collections.EMPTY_LIST;
+            // User has not successfully authenticated via shibboleth.
+            if (request == null || context.getCurrentUser() == null) {
+                return Collections.emptyList();
             }
 
-            // If we have already calculated the special groups then return them.
-            if (request.getSession().getAttribute("shib.specialgroup") != null) {
-                log.debug("Returning cached special groups.");
-                List<UUID> sessionGroupIds = (List<UUID>) request.getSession().getAttribute("shib.specialgroup");
-                List<Group> result = new ArrayList<>();
-                for (UUID uuid : sessionGroupIds) {
-                    result.add(groupService.find(context, uuid));
-                }
-                return result;
+            List<Group> specialGroups = context.getSpecialGroups();
+            if (!specialGroups.isEmpty()) {
+                log.debug("Returning special groups from context.");
+                return specialGroups;
             }
 
+            if (request.getAttribute("shib.authenticated") == null) {
+                log.debug("User has not been authenticated via shibboleth, returning empty list of special groups.");
+                return Collections.emptyList();
+            }
 
             List<UUID> groupIds = new ShibGroup(new ShibHeaders(request), context).get();
-            // Cache the special groups, so we don't have to recalculate them again
-            // for this session.
-            request.getSession().setAttribute("shib.specialgroup", groupIds);
 
             List<Group> groups = new ArrayList<>();
             for (UUID uuid : groupIds) {
                 Group foundGroup = groupService.find(context, uuid);
-                if (Objects.isNull(foundGroup)) {
-                    continue;
+                if (foundGroup != null) {
+                    groups.add(foundGroup);
                 }
-                groups.add(foundGroup);
             }
             return groups;
         } catch (Throwable t) {
-            log.error("Unable to validate any sepcial groups this user may belong too because of an exception.", t);
-            return Collections.EMPTY_LIST;
+            log.error("Unable to validate any special groups this user may belong to because of an exception.", t);
+            return Collections.emptyList();
         }
     }
 
@@ -1291,7 +1284,7 @@ private String getShibURL(HttpServletRequest request) {
     public boolean isUsed(final Context context, final HttpServletRequest request) {
         if (request != null &&
                 context.getCurrentUser() != null &&
-                request.getSession().getAttribute("shib.authenticated") != null) {
+                request.getAttribute("shib.authenticated") != null) {
             return true;
         }
         return false;

dspace-api/src/main/java/org/dspace/content/PreviewContentServiceImpl.java

@@ -369,7 +369,7 @@ private void processGzipFile(List<String> filePaths, File file, Bitstream bitstr
         if (fileName == null) {
             logBitstreamNameIsNull();
         } else {
-            if (fileName.toLowerCase().endsWith("tar.gz")) {
+            if (fileName.toLowerCase().endsWith(".tar.gz") || fileName.toLowerCase().endsWith(".tgz")) {
                 processTarGzipFile(filePaths, file, bitstream);
             } else {
                 try (InputStream is = new GzipCompressorInputStream(new FileInputStream(file))) {

dspace-api/src/main/java/org/dspace/core/Context.java

@@ -686,7 +686,12 @@ public boolean inSpecialGroup(UUID groupID) {
     public List<Group> getSpecialGroups() throws SQLException {
         List<Group> myGroups = new ArrayList<>();
         for (UUID groupId : specialGroups) {
-            myGroups.add(EPersonServiceFactory.getInstance().getGroupService().find(this, groupId));
+            Group group = EPersonServiceFactory.getInstance().getGroupService().find(this, groupId);
+            // A special group UUID may reference a group that has since been deleted; skip nulls
+            // so callers never receive a list containing null (avoids NPE downstream).
+            if (group != null) {
+                myGroups.add(group);
+            }
         }
 
         return myGroups;

dspace-api/src/main/java/org/dspace/ctask/general/ItemMetadataQAChecker.java

@@ -87,6 +87,7 @@ public void init(Curator curator, String taskId) throws IOException {
                     "dc.rights.label",
                     "dc.date.available",
                     "dc.source.uri",
+                    "dc.identifier.doi",
                     "metashare.ResourceInfo#DistributionInfo#LicenseInfo.license"
                 });
         strangeMetadata = configurationService.getArrayProperty("lr.curation.metadata.strange", new String[]{

dspace-api/src/main/java/org/dspace/identifier/DOIIdentifierProvider.java

@@ -1069,13 +1069,26 @@ protected void saveDOIToObject(Context context, DSpaceObject dso, String doi)
         }
         Item item = (Item) dso;
 
-        itemService.addMetadata(context, item, MD_SCHEMA, DOI_ELEMENT, DOI_QUALIFIER, null,
-            doiService.DOIToExternalForm(doi));
-        try {
-            itemService.update(context, item);
-        } catch (SQLException | AuthorizeException ex) {
-            throw ex;
+        String doiURL = doiService.DOIToExternalForm(doi);
+
+        // Add the DOI to the metadata only if this exact value is not present yet. This keeps the operation
+        // idempotent (re-registration does not create duplicate values) without ever deleting metadata: a
+        // pre-existing, different DOI is left untouched. This method is called after the DOI has already been
+        // registered with the external agency, so destroying metadata here would be lossy and irreversible.
+        // Items that end up with more than one dc.identifier.doi value are surfaced by the ItemMetadataQAChecker
+        // curation task for manual review.
+        List<MetadataValue> existing = itemService.getMetadata(item, MD_SCHEMA, DOI_ELEMENT, DOI_QUALIFIER, Item.ANY);
+        boolean alreadyPresent = existing.stream()
+                .anyMatch(metadataValue -> doiURL.equals(metadataValue.getValue()));
+
+        if (alreadyPresent) {
+            log.debug("The DOI {} is already part of the metadata of Item {}. Not adding it again.",
+                    doi, item.getID());
+            return;
         }
+
+        itemService.addMetadata(context, item, MD_SCHEMA, DOI_ELEMENT, DOI_QUALIFIER, null, doiURL);
+        itemService.update(context, item);
     }
 
     /**

dspace-api/src/main/java/org/dspace/scripts/filepreview/FilePreview.java

@@ -16,12 +16,10 @@
 
 import org.apache.commons.cli.ParseException;
 import org.apache.commons.lang3.StringUtils;
-import org.dspace.authenticate.AuthenticationMethod;
-import org.dspace.authenticate.factory.AuthenticateServiceFactory;
-import org.dspace.authenticate.service.AuthenticationService;
 import org.dspace.content.Bitstream;
 import org.dspace.content.Bundle;
 import org.dspace.content.Item;
+import org.dspace.content.PreviewContent;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.ItemService;
 import org.dspace.content.service.PreviewContentService;
@@ -46,21 +44,19 @@ public class FilePreview extends DSpaceRunnable<FilePreviewConfiguration> {
             ContentServiceFactory.getInstance().getPreviewContentService();
     private EPersonService ePersonService = EPersonServiceFactory.getInstance()
             .getEPersonService();
-    private AuthenticationService authenticateService = AuthenticateServiceFactory.getInstance()
-            .getAuthenticationService();
 
     /**
      * `-i`: Info, show help information.
      */
     private boolean info = false;
+    private boolean force = false;
 
     /**
      * `-u`: UUID of the Item for which to create a preview of its bitstreams.
      */
     private String specificItemUUID = null;
 
     private String epersonMail = null;
-    private String epersonPassword = null;
 
     @Override
     public FilePreviewConfiguration getScriptConfiguration() {
@@ -84,11 +80,14 @@ public void setup() throws ParseException {
                     specificItemUUID);
         }
 
+        if (commandLine.hasOption('f')) {
+            force = true;
+        }
+
         epersonMail = commandLine.getOptionValue('e');
-        epersonPassword = commandLine.getOptionValue('p');
 
-        if (getEpersonIdentifier() == null && (epersonMail == null || epersonPassword == null)) {
-            throw new ParseException("Provide both -e/--email and -p/--password when no eperson is supplied.");
+        if (getEpersonIdentifier() == null && epersonMail == null) {
+            throw new ParseException("Provide -e/--email when no eperson is supplied.");
         }
     }
 
@@ -101,7 +100,7 @@ public void internalRun() throws Exception {
 
         Context context = new Context();
         try {
-            context.setCurrentUser(getAuthenticatedEperson((context)));
+            context.setCurrentUser(getEperson(context));
             handler.logInfo("Authentication by user: " + context.getCurrentUser().getEmail());
             if (StringUtils.isNotBlank(specificItemUUID)) {
                 // Generate the preview only for a specific item
@@ -152,7 +151,17 @@ private void generateItemFilePreviews(Context context, UUID itemUUID) throws Exc
                 }
                 // Generate new content if we didn't find any
                 if (previewContentService.hasPreview(context, bitstream)) {
-                    continue;
+                    if (force) {
+                        List<PreviewContent> previewContents = previewContentService
+                                .findByBitstream(context, bitstream.getID());
+                        for (PreviewContent content : previewContents) {
+                            handler.logInfo("Deleting existing preview content: '" + content.getName() +
+                                    "', for bitstream: '" + bitstream.getName() + "'");
+                            previewContentService.delete(context, content);
+                        }
+                    } else {
+                        continue;
+                    }
                 }
 
                 List<FileInfo> fileInfos = previewContentService.getFilePreviewContent(context, bitstream);
@@ -162,6 +171,7 @@ private void generateItemFilePreviews(Context context, UUID itemUUID) throws Exc
                     continue;
                 }
 
+                handler.logInfo("Generating file preview for bitstream: " + bitstream.getName());
                 for (FileInfo fi : fileInfos) {
                     previewContentService.createPreviewContent(context, bitstream, fi);
                 }
@@ -176,38 +186,30 @@ public void printHelp() {
                 "You can choose from these available options:\n" +
                 "  -i, --info            Show help information\n" +
                 "  -u, --uuid            The UUID of the ITEM for which to create a preview of its bitstreams\n" +
-                "  -e, --email           Email for authentication\n" +
-                "  -p, --password        Password for authentication\n");
+                "  -f, --force           Force to create preview, even when the preview exists\n" +
+                "  -e, --email           Email of the eperson to run the script as\n");
 
     }
 
     /**
-     * Retrieves an EPerson object either by its identifier or by performing an email-based lookup.
-     * It then authenticates the EPerson using the provided email and password.
-     * If the authentication is successful, it returns the EPerson object; otherwise,
-     * it throws an AuthenticationException.
+     * Resolves the EPerson the script runs as: the eperson supplied by the launching context
+     * (e.g. the logged-in user when started from the admin UI) if present, otherwise the eperson
+     * looked up by the {@code -e}/--email option. Like other CLI scripts, command-line invocation
+     * is trusted (shell access implies full server access), so no password is verified here;
+     * admin-only operations remain guarded by authorization checks in the service layer.
      *
      * @param context The Context object used for interacting with the DSpace database and service layer.
-     * @return The authenticated EPerson object corresponding to the provided email,
-     *         if authentication is successful.
-     * @throws SQLException If a database error occurs while retrieving or interacting with the EPerson data.
-     * @throws AuthenticationException If no EPerson is found for the provided email
-     *         or if the authentication fails.
+     * @return The EPerson the script should run as.
+     * @throws SQLException If a database error occurs while retrieving the EPerson data.
+     * @throws AuthenticationException If no EPerson is found for the provided email.
      */
-    private EPerson getAuthenticatedEperson(Context context) throws SQLException, AuthenticationException {
+    private EPerson getEperson(Context context) throws SQLException, AuthenticationException {
         if (getEpersonIdentifier() != null) {
             return ePersonService.find(context, getEpersonIdentifier());
         }
-        String msg;
         EPerson ePerson = ePersonService.findByEmail(context, epersonMail);
         if (ePerson == null) {
-            msg = "No EPerson found for this email: " + epersonMail;
-            handler.logError(msg);
-            throw new AuthenticationException(msg);
-        }
-        int authenticated = authenticateService.authenticate(context, epersonMail, epersonPassword, null, null);
-        if (AuthenticationMethod.SUCCESS != authenticated) {
-            msg = "Authentication failed for email: " + epersonMail;
+            String msg = "No EPerson found for this email: " + epersonMail;
             handler.logError(msg);
             throw new AuthenticationException(msg);
         }

dspace-api/src/main/java/org/dspace/scripts/filepreview/FilePreviewConfiguration.java

@@ -39,15 +39,11 @@ public Options getOptions() {
             options.getOption("u").setType(String.class);
             options.getOption("u").setRequired(false);
 
+            options.addOption("f", "force", false, "Force to create preview, even when the preview exists.");
+
             options.addOption("e", "email", true,
-                    "Email for authentication.");
+                    "Email of the eperson to run the script as.");
             options.getOption("e").setType(String.class);
-            options.getOption("e").setRequired(true);
-
-            options.addOption("p", "password", true,
-                    "Password for authentication.");
-            options.getOption("p").setType(String.class);
-            options.getOption("p").setRequired(true);
 
             super.options = options;
         }

dspace-api/src/test/java/org/dspace/curate/ItemMetadataQACheckerIT.java

@@ -63,6 +63,7 @@ public class ItemMetadataQACheckerIT extends AbstractIntegrationTestWithDatabase
     Item itemWithIncorrectLanguageName;
     Item itemWithTwoAvailableDates;
     Item itemWithTwoAvailableDatesAndLang;
+    Item itemWithTwoDois;
     Item itemVersion1;
     Item itemVersion2;
     Item itemVersion3;
@@ -146,6 +147,13 @@ public void setUp() throws Exception {
             itemService.addMetadata(context, itemWithTwoAvailableDatesAndLang,"dc", "date",
                     "available", "en_US", "2021-01-01");
 
+            itemWithTwoDois = ItemBuilder.createItem(context, collection)
+                    .withTitle("Item With Two DOIs")
+                    .withMetadata("dc", "type", null, "corpus")
+                    .withMetadata("dc", "identifier", "doi", "https://doi.org/10.5072/test-1")
+                    .withMetadata("dc", "identifier", "doi", "https://doi.org/10.5072/test-2")
+                    .build();
+
             itemVersion1 = ItemBuilder.createItem(context, collection)
                     .withTitle("Item Version 1")
                     .withMetadata("dc", "type", null, "corpus")
@@ -233,6 +241,20 @@ public void testItemWithTwoAvailableDatesAndLang() throws IOException {
         assertTrue("Result should mention multiple dc.date.available", result.contains("dc.date.available"));
     }
 
+    @Test
+    public void testItemWithTwoDois() throws IOException {
+        Curator curator = new Curator();
+        curator.addTask(TASK_NAME);
+        context.setCurrentUser(admin);
+
+        // Run curator task for item with two dc.identifier.doi - should fail
+        curator.curate(context, itemWithTwoDois.getHandle());
+        int status = curator.getStatus(TASK_NAME);
+        assertEquals("Curation should fail for item with two dc.identifier.doi", Curator.CURATE_FAIL, status);
+        String result = curator.getResult(TASK_NAME);
+        assertTrue("Result should mention multiple dc.identifier.doi", result.contains("dc.identifier.doi"));
+    }
+
     @Test
     public void testValidItem() throws IOException {
         Curator curator = new Curator();