Skip to content

Commit a7d8c71

Browse files
CLARIN-DSpace v9/Port #1350 + #1347 + #1338 (DOI dedup, Shib special groups, tgz preview) to the v9 base (#1378)
* UFAL/fix: DOI Organizer creates duplicate dc.identifier.doi metadata (ufal#1368) (#1350) (cherry picked from commit e9392ae on dtq-dev) v9 adaptations: - DOIIdentifierProviderTest: the two new tests' 'new Date().getTime()' rewritten to 'Instant.now().toEpochMilli()' — v9-base's Date->Instant migration removed the java.util.Date import, so the clean cherry-pick would not compile (known tripwire, sync plan card e9392ae). Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / e9392ae (BE-1, Vlna 1). * UFAL/Obtain special groups from user context when new token is generated (on token refresh) (ufal#1378) (#1347) (cherry picked from commit 74f5862 on dtq-dev) v9 adaptations / additions: - ClarinShibbolethSpecialGroupsIT.java added verbatim from the zcu backport branch head b7d3c78 (origin/zcu-pub/backport-1347-shib-special-groups) — the regression IT exists only on the 7.6 customer backport branches, not on dtq-dev; fix sourced from 74f5862 (canonical), test from b7d3c78 per the sync plan card. - No code adaptations: the pick applied conflict-free (method bodies only, jakarta imports untouched); Context.getSpecialGroups null-guard included. Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / 74f5862 (BE-1, Vlna 1). * [Port to dtq-dev] Issue 1364: tgz file preview fix (#1338) (cherry picked from commit 15b296a on dtq-dev) v9 adaptations / conflict resolution: - FilePreviewIT.java: single conflict resolved toward the v9-base deletion of testPreviewWithSyncStorage + SyncBitstreamStorageServiceImpl import/SYNC_STORE_NUMBER (that class does not exist on v9-base); the commit's modernization hunk for that test dropped, everything else applied (testUnauthorizedPassword removed, -p args dropped, checkHandlerMessages helper + testPreviewWithForce added). Resulting file contains exactly 6 tests. - Intentional semantics change carried from the fork commit: the file-preview CLI no longer requires -p/--password; EPerson resolved from context or -e email, consistent with other DSpace CLI scripts. Admin-only operations stay guarded server-side. Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / 15b296a (BE-1, Vlna 1). --------- Co-authored-by: Ondřej Košarko <ko_ok@centrum.cz>
1 parent e738a32 commit a7d8c71

12 files changed

Lines changed: 435 additions & 95 deletions

File tree

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

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ public int authenticate(Context context, String username, String password,
277277

278278
// Step 4: Log the user in.
279279
context.setCurrentUser(eperson);
280-
request.getSession().setAttribute("shib.authenticated", true);
280+
request.setAttribute("shib.authenticated", true);
281281
AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson);
282282

283283
log.info(eperson.getEmail() + " has been authenticated via shibboleth.");
@@ -330,42 +330,35 @@ public int authenticate(Context context, String username, String password,
330330
@Override
331331
public List<Group> getSpecialGroups(Context context, HttpServletRequest request) {
332332
try {
333-
// User has not successfuly authenticated via shibboleth.
334-
if (request == null ||
335-
context.getCurrentUser() == null ||
336-
request.getSession().getAttribute("shib.authenticated") == null) {
337-
return Collections.EMPTY_LIST;
333+
// User has not successfully authenticated via shibboleth.
334+
if (request == null || context.getCurrentUser() == null) {
335+
return Collections.emptyList();
338336
}
339337

340-
// If we have already calculated the special groups then return them.
341-
if (request.getSession().getAttribute("shib.specialgroup") != null) {
342-
log.debug("Returning cached special groups.");
343-
List<UUID> sessionGroupIds = (List<UUID>) request.getSession().getAttribute("shib.specialgroup");
344-
List<Group> result = new ArrayList<>();
345-
for (UUID uuid : sessionGroupIds) {
346-
result.add(groupService.find(context, uuid));
347-
}
348-
return result;
338+
List<Group> specialGroups = context.getSpecialGroups();
339+
if (!specialGroups.isEmpty()) {
340+
log.debug("Returning special groups from context.");
341+
return specialGroups;
349342
}
350343

344+
if (request.getAttribute("shib.authenticated") == null) {
345+
log.debug("User has not been authenticated via shibboleth, returning empty list of special groups.");
346+
return Collections.emptyList();
347+
}
351348

352349
List<UUID> groupIds = new ShibGroup(new ShibHeaders(request), context).get();
353-
// Cache the special groups, so we don't have to recalculate them again
354-
// for this session.
355-
request.getSession().setAttribute("shib.specialgroup", groupIds);
356350

357351
List<Group> groups = new ArrayList<>();
358352
for (UUID uuid : groupIds) {
359353
Group foundGroup = groupService.find(context, uuid);
360-
if (Objects.isNull(foundGroup)) {
361-
continue;
354+
if (foundGroup != null) {
355+
groups.add(foundGroup);
362356
}
363-
groups.add(foundGroup);
364357
}
365358
return groups;
366359
} catch (Throwable t) {
367-
log.error("Unable to validate any sepcial groups this user may belong too because of an exception.", t);
368-
return Collections.EMPTY_LIST;
360+
log.error("Unable to validate any special groups this user may belong to because of an exception.", t);
361+
return Collections.emptyList();
369362
}
370363
}
371364

@@ -1291,7 +1284,7 @@ private String getShibURL(HttpServletRequest request) {
12911284
public boolean isUsed(final Context context, final HttpServletRequest request) {
12921285
if (request != null &&
12931286
context.getCurrentUser() != null &&
1294-
request.getSession().getAttribute("shib.authenticated") != null) {
1287+
request.getAttribute("shib.authenticated") != null) {
12951288
return true;
12961289
}
12971290
return false;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ private void processGzipFile(List<String> filePaths, File file, Bitstream bitstr
369369
if (fileName == null) {
370370
logBitstreamNameIsNull();
371371
} else {
372-
if (fileName.toLowerCase().endsWith("tar.gz")) {
372+
if (fileName.toLowerCase().endsWith(".tar.gz") || fileName.toLowerCase().endsWith(".tgz")) {
373373
processTarGzipFile(filePaths, file, bitstream);
374374
} else {
375375
try (InputStream is = new GzipCompressorInputStream(new FileInputStream(file))) {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,12 @@ public boolean inSpecialGroup(UUID groupID) {
686686
public List<Group> getSpecialGroups() throws SQLException {
687687
List<Group> myGroups = new ArrayList<>();
688688
for (UUID groupId : specialGroups) {
689-
myGroups.add(EPersonServiceFactory.getInstance().getGroupService().find(this, groupId));
689+
Group group = EPersonServiceFactory.getInstance().getGroupService().find(this, groupId);
690+
// A special group UUID may reference a group that has since been deleted; skip nulls
691+
// so callers never receive a list containing null (avoids NPE downstream).
692+
if (group != null) {
693+
myGroups.add(group);
694+
}
690695
}
691696

692697
return myGroups;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public void init(Curator curator, String taskId) throws IOException {
8787
"dc.rights.label",
8888
"dc.date.available",
8989
"dc.source.uri",
90+
"dc.identifier.doi",
9091
"metashare.ResourceInfo#DistributionInfo#LicenseInfo.license"
9192
});
9293
strangeMetadata = configurationService.getArrayProperty("lr.curation.metadata.strange", new String[]{

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,13 +1069,26 @@ protected void saveDOIToObject(Context context, DSpaceObject dso, String doi)
10691069
}
10701070
Item item = (Item) dso;
10711071

1072-
itemService.addMetadata(context, item, MD_SCHEMA, DOI_ELEMENT, DOI_QUALIFIER, null,
1073-
doiService.DOIToExternalForm(doi));
1074-
try {
1075-
itemService.update(context, item);
1076-
} catch (SQLException | AuthorizeException ex) {
1077-
throw ex;
1072+
String doiURL = doiService.DOIToExternalForm(doi);
1073+
1074+
// Add the DOI to the metadata only if this exact value is not present yet. This keeps the operation
1075+
// idempotent (re-registration does not create duplicate values) without ever deleting metadata: a
1076+
// pre-existing, different DOI is left untouched. This method is called after the DOI has already been
1077+
// registered with the external agency, so destroying metadata here would be lossy and irreversible.
1078+
// Items that end up with more than one dc.identifier.doi value are surfaced by the ItemMetadataQAChecker
1079+
// curation task for manual review.
1080+
List<MetadataValue> existing = itemService.getMetadata(item, MD_SCHEMA, DOI_ELEMENT, DOI_QUALIFIER, Item.ANY);
1081+
boolean alreadyPresent = existing.stream()
1082+
.anyMatch(metadataValue -> doiURL.equals(metadataValue.getValue()));
1083+
1084+
if (alreadyPresent) {
1085+
log.debug("The DOI {} is already part of the metadata of Item {}. Not adding it again.",
1086+
doi, item.getID());
1087+
return;
10781088
}
1089+
1090+
itemService.addMetadata(context, item, MD_SCHEMA, DOI_ELEMENT, DOI_QUALIFIER, null, doiURL);
1091+
itemService.update(context, item);
10791092
}
10801093

10811094
/**

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

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@
1616

1717
import org.apache.commons.cli.ParseException;
1818
import org.apache.commons.lang3.StringUtils;
19-
import org.dspace.authenticate.AuthenticationMethod;
20-
import org.dspace.authenticate.factory.AuthenticateServiceFactory;
21-
import org.dspace.authenticate.service.AuthenticationService;
2219
import org.dspace.content.Bitstream;
2320
import org.dspace.content.Bundle;
2421
import org.dspace.content.Item;
22+
import org.dspace.content.PreviewContent;
2523
import org.dspace.content.factory.ContentServiceFactory;
2624
import org.dspace.content.service.ItemService;
2725
import org.dspace.content.service.PreviewContentService;
@@ -46,21 +44,19 @@ public class FilePreview extends DSpaceRunnable<FilePreviewConfiguration> {
4644
ContentServiceFactory.getInstance().getPreviewContentService();
4745
private EPersonService ePersonService = EPersonServiceFactory.getInstance()
4846
.getEPersonService();
49-
private AuthenticationService authenticateService = AuthenticateServiceFactory.getInstance()
50-
.getAuthenticationService();
5147

5248
/**
5349
* `-i`: Info, show help information.
5450
*/
5551
private boolean info = false;
52+
private boolean force = false;
5653

5754
/**
5855
* `-u`: UUID of the Item for which to create a preview of its bitstreams.
5956
*/
6057
private String specificItemUUID = null;
6158

6259
private String epersonMail = null;
63-
private String epersonPassword = null;
6460

6561
@Override
6662
public FilePreviewConfiguration getScriptConfiguration() {
@@ -84,11 +80,14 @@ public void setup() throws ParseException {
8480
specificItemUUID);
8581
}
8682

83+
if (commandLine.hasOption('f')) {
84+
force = true;
85+
}
86+
8787
epersonMail = commandLine.getOptionValue('e');
88-
epersonPassword = commandLine.getOptionValue('p');
8988

90-
if (getEpersonIdentifier() == null && (epersonMail == null || epersonPassword == null)) {
91-
throw new ParseException("Provide both -e/--email and -p/--password when no eperson is supplied.");
89+
if (getEpersonIdentifier() == null && epersonMail == null) {
90+
throw new ParseException("Provide -e/--email when no eperson is supplied.");
9291
}
9392
}
9493

@@ -101,7 +100,7 @@ public void internalRun() throws Exception {
101100

102101
Context context = new Context();
103102
try {
104-
context.setCurrentUser(getAuthenticatedEperson((context)));
103+
context.setCurrentUser(getEperson(context));
105104
handler.logInfo("Authentication by user: " + context.getCurrentUser().getEmail());
106105
if (StringUtils.isNotBlank(specificItemUUID)) {
107106
// Generate the preview only for a specific item
@@ -152,7 +151,17 @@ private void generateItemFilePreviews(Context context, UUID itemUUID) throws Exc
152151
}
153152
// Generate new content if we didn't find any
154153
if (previewContentService.hasPreview(context, bitstream)) {
155-
continue;
154+
if (force) {
155+
List<PreviewContent> previewContents = previewContentService
156+
.findByBitstream(context, bitstream.getID());
157+
for (PreviewContent content : previewContents) {
158+
handler.logInfo("Deleting existing preview content: '" + content.getName() +
159+
"', for bitstream: '" + bitstream.getName() + "'");
160+
previewContentService.delete(context, content);
161+
}
162+
} else {
163+
continue;
164+
}
156165
}
157166

158167
List<FileInfo> fileInfos = previewContentService.getFilePreviewContent(context, bitstream);
@@ -162,6 +171,7 @@ private void generateItemFilePreviews(Context context, UUID itemUUID) throws Exc
162171
continue;
163172
}
164173

174+
handler.logInfo("Generating file preview for bitstream: " + bitstream.getName());
165175
for (FileInfo fi : fileInfos) {
166176
previewContentService.createPreviewContent(context, bitstream, fi);
167177
}
@@ -176,38 +186,30 @@ public void printHelp() {
176186
"You can choose from these available options:\n" +
177187
" -i, --info Show help information\n" +
178188
" -u, --uuid The UUID of the ITEM for which to create a preview of its bitstreams\n" +
179-
" -e, --email Email for authentication\n" +
180-
" -p, --password Password for authentication\n");
189+
" -f, --force Force to create preview, even when the preview exists\n" +
190+
" -e, --email Email of the eperson to run the script as\n");
181191

182192
}
183193

184194
/**
185-
* Retrieves an EPerson object either by its identifier or by performing an email-based lookup.
186-
* It then authenticates the EPerson using the provided email and password.
187-
* If the authentication is successful, it returns the EPerson object; otherwise,
188-
* it throws an AuthenticationException.
195+
* Resolves the EPerson the script runs as: the eperson supplied by the launching context
196+
* (e.g. the logged-in user when started from the admin UI) if present, otherwise the eperson
197+
* looked up by the {@code -e}/--email option. Like other CLI scripts, command-line invocation
198+
* is trusted (shell access implies full server access), so no password is verified here;
199+
* admin-only operations remain guarded by authorization checks in the service layer.
189200
*
190201
* @param context The Context object used for interacting with the DSpace database and service layer.
191-
* @return The authenticated EPerson object corresponding to the provided email,
192-
* if authentication is successful.
193-
* @throws SQLException If a database error occurs while retrieving or interacting with the EPerson data.
194-
* @throws AuthenticationException If no EPerson is found for the provided email
195-
* or if the authentication fails.
202+
* @return The EPerson the script should run as.
203+
* @throws SQLException If a database error occurs while retrieving the EPerson data.
204+
* @throws AuthenticationException If no EPerson is found for the provided email.
196205
*/
197-
private EPerson getAuthenticatedEperson(Context context) throws SQLException, AuthenticationException {
206+
private EPerson getEperson(Context context) throws SQLException, AuthenticationException {
198207
if (getEpersonIdentifier() != null) {
199208
return ePersonService.find(context, getEpersonIdentifier());
200209
}
201-
String msg;
202210
EPerson ePerson = ePersonService.findByEmail(context, epersonMail);
203211
if (ePerson == null) {
204-
msg = "No EPerson found for this email: " + epersonMail;
205-
handler.logError(msg);
206-
throw new AuthenticationException(msg);
207-
}
208-
int authenticated = authenticateService.authenticate(context, epersonMail, epersonPassword, null, null);
209-
if (AuthenticationMethod.SUCCESS != authenticated) {
210-
msg = "Authentication failed for email: " + epersonMail;
212+
String msg = "No EPerson found for this email: " + epersonMail;
211213
handler.logError(msg);
212214
throw new AuthenticationException(msg);
213215
}

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,11 @@ public Options getOptions() {
3939
options.getOption("u").setType(String.class);
4040
options.getOption("u").setRequired(false);
4141

42+
options.addOption("f", "force", false, "Force to create preview, even when the preview exists.");
43+
4244
options.addOption("e", "email", true,
43-
"Email for authentication.");
45+
"Email of the eperson to run the script as.");
4446
options.getOption("e").setType(String.class);
45-
options.getOption("e").setRequired(true);
46-
47-
options.addOption("p", "password", true,
48-
"Password for authentication.");
49-
options.getOption("p").setType(String.class);
50-
options.getOption("p").setRequired(true);
5147

5248
super.options = options;
5349
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public class ItemMetadataQACheckerIT extends AbstractIntegrationTestWithDatabase
6363
Item itemWithIncorrectLanguageName;
6464
Item itemWithTwoAvailableDates;
6565
Item itemWithTwoAvailableDatesAndLang;
66+
Item itemWithTwoDois;
6667
Item itemVersion1;
6768
Item itemVersion2;
6869
Item itemVersion3;
@@ -146,6 +147,13 @@ public void setUp() throws Exception {
146147
itemService.addMetadata(context, itemWithTwoAvailableDatesAndLang,"dc", "date",
147148
"available", "en_US", "2021-01-01");
148149

150+
itemWithTwoDois = ItemBuilder.createItem(context, collection)
151+
.withTitle("Item With Two DOIs")
152+
.withMetadata("dc", "type", null, "corpus")
153+
.withMetadata("dc", "identifier", "doi", "https://doi.org/10.5072/test-1")
154+
.withMetadata("dc", "identifier", "doi", "https://doi.org/10.5072/test-2")
155+
.build();
156+
149157
itemVersion1 = ItemBuilder.createItem(context, collection)
150158
.withTitle("Item Version 1")
151159
.withMetadata("dc", "type", null, "corpus")
@@ -233,6 +241,20 @@ public void testItemWithTwoAvailableDatesAndLang() throws IOException {
233241
assertTrue("Result should mention multiple dc.date.available", result.contains("dc.date.available"));
234242
}
235243

244+
@Test
245+
public void testItemWithTwoDois() throws IOException {
246+
Curator curator = new Curator();
247+
curator.addTask(TASK_NAME);
248+
context.setCurrentUser(admin);
249+
250+
// Run curator task for item with two dc.identifier.doi - should fail
251+
curator.curate(context, itemWithTwoDois.getHandle());
252+
int status = curator.getStatus(TASK_NAME);
253+
assertEquals("Curation should fail for item with two dc.identifier.doi", Curator.CURATE_FAIL, status);
254+
String result = curator.getResult(TASK_NAME);
255+
assertTrue("Result should mention multiple dc.identifier.doi", result.contains("dc.identifier.doi"));
256+
}
257+
236258
@Test
237259
public void testValidItem() throws IOException {
238260
Curator curator = new Curator();

0 commit comments

Comments
 (0)