Skip to content

Commit 90a4679

Browse files
milanmajchrakmilanmajchrak
authored
Merge dtq-dev into ufal/fix-content-disposition-rfc5987 (pick up scheduled-IT de-flake)
2 parents 1e1b1fe + 23f3786 commit 90a4679

17 files changed

Lines changed: 622 additions & 92 deletions

File tree

‎dspace-api/src/main/java/org/dspace/app/mediafilter/PDFBoxThumbnail.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package org.dspace.app.mediafilter;
99

1010
import java.awt.image.BufferedImage;
11+
import java.io.IOException;
1112
import java.io.InputStream;
1213

1314
import org.apache.logging.log4j.Logger;
@@ -79,6 +80,13 @@ public InputStream getDestinationStream(Item currentItem, InputStream source, bo
7980
} catch (InvalidPasswordException ex) {
8081
log.error("PDF is encrypted. Cannot create thumbnail (item: {})", currentItem::getHandle);
8182
return null;
83+
} catch (IOException ex) {
84+
// A malformed/non-standard PDF (bad %PDF- header, missing xref, truncated file, etc.)
85+
// is a data-quality issue in the source bitstream, not a DSpace fault. Skip the
86+
// thumbnail instead of failing the whole filter-media run.
87+
log.warn("PDF could not be parsed by PDFBox. Cannot create thumbnail (item: {}): {}",
88+
currentItem::getHandle, ex::getMessage);
89+
return null;
8290
}
8391

8492
// Generate thumbnail derivative and return as IO stream.

‎dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,13 @@ public InputStream getDestinationStream(Item currentItem, InputStream source, bo
8585
tika.setMaxStringLength(maxChars); // Tell Tika the maximum number of characters to extract
8686
extractedText = tika.parseToString(source);
8787
} catch (IOException e) {
88-
System.err.format("Unable to extract text from bitstream in Item %s%n", currentItem.getID().toString());
89-
e.printStackTrace(System.err);
90-
log.error("Unable to extract text from bitstream in Item {}", currentItem.getID().toString(), e);
91-
throw e;
88+
// A malformed/non-standard source file (e.g. a PDF with a corrupt header, missing
89+
// xref, or truncated content) is a data-quality issue in the bitstream, not a
90+
// DSpace fault. Skip text extraction for it instead of failing the whole
91+
// filter-media run.
92+
log.warn("Unable to extract text from bitstream in Item {}: {}",
93+
currentItem.getHandle(), e.getMessage());
94+
return null;
9295
} catch (OutOfMemoryError oe) {
9396
System.err.format("OutOfMemoryError occurred when extracting text from bitstream in Item %s. " +
9497
"You may wish to enable 'textextractor.use-temp-file'.%n", currentItem.getID().toString());

‎dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ public int authenticate(Context context,
322322
log.info(LogHelper.getHeader(context,
323323
"type=ldap-login", "type=ldap_but_already_email"));
324324
context.turnOffAuthorisationSystem();
325-
setEpersonAttributes(context, eperson, ldap, Optional.of(netid));
325+
setEpersonAttributes(context, eperson, ldap, Optional.of(netid), email);
326326
ePersonService.update(context, eperson);
327327
context.dispatchEvents();
328328
context.restoreAuthSystemState();
@@ -339,7 +339,7 @@ public int authenticate(Context context,
339339
try {
340340
context.turnOffAuthorisationSystem();
341341
eperson = ePersonService.create(context);
342-
setEpersonAttributes(context, eperson, ldap, Optional.of(netid));
342+
setEpersonAttributes(context, eperson, ldap, Optional.of(netid), email);
343343
eperson.setCanLogIn(true);
344344
authenticationService.initEPerson(context, request, eperson);
345345
ePersonService.update(context, eperson);
@@ -382,9 +382,22 @@ public int authenticate(Context context,
382382
*/
383383
private void setEpersonAttributes(Context context, EPerson eperson, SpeakerToLDAP ldap, Optional<String> netid)
384384
throws SQLException {
385+
setEpersonAttributes(context, eperson, ldap, netid, null);
386+
}
387+
388+
/**
389+
* Update eperson's attributes, falling back to the supplied login e-mail when LDAP has none.
390+
*/
391+
private void setEpersonAttributes(Context context, EPerson eperson, SpeakerToLDAP ldap,
392+
Optional<String> netid, String email) throws SQLException {
385393

394+
// Set the e-mail: prefer the LDAP-provided address, otherwise fall back to the
395+
// login e-mail when one was supplied. If neither is available, the existing
396+
// e-mail is left unchanged.
386397
if (StringUtils.isNotEmpty(ldap.ldapEmail)) {
387398
eperson.setEmail(ldap.ldapEmail);
399+
} else if (StringUtils.isNotEmpty(email)) {
400+
eperson.setEmail(email);
388401
}
389402
if (StringUtils.isNotEmpty(ldap.ldapGivenName)) {
390403
eperson.setFirstName(context, ldap.ldapGivenName);
@@ -734,7 +747,7 @@ public String getName() {
734747
*/
735748
private void assignGroups(String dn, ArrayList<String> group, Context context) {
736749
if (StringUtils.isNotBlank(dn)) {
737-
System.out.println("dn:" + dn);
750+
log.debug(LogHelper.getHeader(context, "assignGroups", "dn=" + dn));
738751
int groupmapIndex = 1;
739752
String groupMap = configurationService.getProperty("authentication-ldap.login.groupmap." + groupmapIndex);
740753
boolean cmp;
@@ -743,7 +756,16 @@ private void assignGroups(String dn, ArrayList<String> group, Context context) {
743756
// groupmap contains the mapping of LDAP groups to DSpace groups
744757
// outer loop with the DSpace groups
745758
while (groupMap != null) {
746-
String t[] = groupMap.split(":");
759+
String t[] = groupMap.split(":", 2);
760+
if (t.length < 2 || StringUtils.isBlank(t[0]) || StringUtils.isBlank(t[1])) {
761+
log.error(LogHelper.getHeader(context, "assignGroups",
762+
"malformed groupmap entry at index " + groupmapIndex + ": " + groupMap +
763+
" - expected '<ldapSearchFragment>:<dspaceGroupName>' with both parts non-empty"));
764+
groupMap = configurationService.getProperty(
765+
"authentication-ldap.login.groupmap." + ++groupmapIndex);
766+
continue;
767+
}
768+
747769
String ldapSearchString = t[0];
748770
String dspaceGroupName = t[1];
749771

‎dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.dspace.content.service.MetadataSchemaService;
5151
import org.dspace.content.service.RelationshipService;
5252
import org.dspace.content.service.WorkspaceItemService;
53+
import org.dspace.content.service.clarin.ClarinItemService;
5354
import org.dspace.content.virtual.VirtualMetadataPopulator;
5455
import org.dspace.core.Constants;
5556
import org.dspace.core.Context;
@@ -180,6 +181,9 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
180181
@Autowired
181182
private VersionHistoryService versionHistoryService;
182183

184+
@Autowired(required = true)
185+
private ClarinItemService clarinItemService;
186+
183187
@Autowired(required = true)
184188
ClarinMatomoBitstreamTracker matomoBitstreamTracker;
185189

@@ -686,6 +690,11 @@ public void update(Context context, Item item) throws SQLException, AuthorizeExc
686690
}
687691

688692
if (item.isMetadataModified() || item.isModified()) {
693+
// Derive dc.date.issued from local.approximateDate.issued when metadata changes
694+
if (item.isMetadataModified()) {
695+
clarinItemService.updateItemDatesMetadata(context, item);
696+
}
697+
689698
// Set the last modified date
690699
item.setLastModified(new Date());
691700

‎dspace-api/src/main/java/org/dspace/content/clarin/ClarinItemServiceImpl.java‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,33 @@ public void updateItemDatesMetadata(Context context, Item item) throws SQLExcept
223223
return;
224224
}
225225

226+
String derivedDate = deriveDateIssuedFromApproximateDate(item);
227+
if (derivedDate == null) {
228+
log.debug("Cannot update item dates metadata because the approximate date is empty.");
229+
return;
230+
}
231+
232+
// Skip the write only when dc.date.issued already holds exactly the single derived value.
233+
// A multi-valued field must still be normalized down to the single derived value.
234+
List<MetadataValue> currentDateIssued =
235+
itemService.getMetadata(item, "dc", "date", "issued", Item.ANY, false);
236+
if (currentDateIssued.size() == 1
237+
&& derivedDate.equals(currentDateIssued.get(0).getValue())) {
238+
return;
239+
}
240+
241+
// Clear the current `dc.date.issued` metadata and set it to the derived value
242+
itemService.clearMetadata(context, item, "dc", "date", "issued", Item.ANY);
243+
itemService.addMetadata(context, item, "dc", "date", "issued", Item.ANY, derivedDate);
244+
}
245+
246+
@Override
247+
public String deriveDateIssuedFromApproximateDate(Item item) {
226248
List<MetadataValue> approximatedDates =
227249
itemService.getMetadata(item, "local", "approximateDate", "issued", Item.ANY, false);
228250

229251
if (CollectionUtils.isEmpty(approximatedDates) || StringUtils.isBlank(approximatedDates.get(0).getValue())) {
230-
log.debug("Cannot update item dates metadata because the approximate date is empty.");
231-
return;
252+
return null;
232253
}
233254

234255
// Get the approximate date value from the metadata
@@ -239,21 +260,11 @@ public void updateItemDatesMetadata(Context context, Item item) throws SQLExcept
239260
// Trim the list of years - remove leading and trailing whitespaces
240261
listOfYearValues.replaceAll(String::trim);
241262

242-
try {
243-
// Clear the current `dc.date.issued` metadata
244-
itemService.clearMetadata(context, item, "dc", "date", "issued", Item.ANY);
245-
246-
// Update the `dc.date.issued` metadata with a new value: `0000` or the last year from the sequence
247-
if (CollectionUtils.isNotEmpty(listOfYearValues) && isListOfNumbers(listOfYearValues)) {
248-
// Take the last year from the list of years and add it to the `dc.date.issued` metadata
249-
itemService.addMetadata(context, item, "dc", "date", "issued", Item.ANY,
250-
getLastNumber(listOfYearValues));
251-
} else {
252-
// Add the `0000` value to the `dc.date.issued` metadata
253-
itemService.addMetadata(context, item, "dc", "date", "issued", Item.ANY, NO_YEAR);
254-
}
255-
} catch (SQLException e) {
256-
log.error("Cannot remove `dc.date.issued` metadata because: {}", e.getMessage());
263+
// `0000` when the approximate date is not a list of numbers, otherwise the last year in the sequence
264+
if (CollectionUtils.isNotEmpty(listOfYearValues) && isListOfNumbers(listOfYearValues)) {
265+
return getLastNumber(listOfYearValues);
266+
} else {
267+
return NO_YEAR;
257268
}
258269
}
259270

‎dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinItemService.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,16 @@ public interface ClarinItemService {
102102
*/
103103
void updateItemDatesMetadata(Context context, Item item) throws SQLException;
104104

105+
/**
106+
* Derive the display value for {@code dc.date.issued} from the item's
107+
* {@code local.approximateDate.issued} metadata, without touching the database.
108+
* Returns the last year for a numeric sequence (e.g. "1938, 1945" -&gt; "1945"),
109+
* {@code "0000"} for a non-numeric approximate value, or {@code null} when no
110+
* approximate date is present.
111+
*
112+
* @param item the item to derive the date from
113+
* @return the derived {@code dc.date.issued} value, or {@code null} if none applies
114+
*/
115+
String deriveDateIssuedFromApproximateDate(Item item);
116+
105117
}

‎dspace-api/src/test/java/org/dspace/content/clarin/ClarinItemServiceImplTest.java‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/**
2+
* The contents of this file are subject to the license and copyright
3+
* detailed in the LICENSE and NOTICE files at the root of the source
4+
* tree and available online at
5+
*
6+
* http://www.dspace.org/license/
7+
*/
8+
package org.dspace.content.clarin;
9+
10+
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertNull;
12+
import static org.mockito.Mockito.mock;
13+
import static org.mockito.Mockito.never;
14+
import static org.mockito.Mockito.verify;
15+
import static org.mockito.Mockito.when;
16+
17+
import java.sql.SQLException;
18+
import java.util.Arrays;
19+
import java.util.Collections;
20+
import java.util.List;
21+
22+
import org.dspace.content.Item;
23+
import org.dspace.content.MetadataValue;
24+
import org.dspace.content.service.ItemService;
25+
import org.dspace.core.Context;
26+
import org.junit.Before;
27+
import org.junit.Test;
28+
import org.springframework.test.util.ReflectionTestUtils;
29+
30+
/**
31+
* Pure unit tests for {@link ClarinItemServiceImpl}: the {@code dc.date.issued} derivation
32+
* from {@code local.approximateDate.issued}, and the normalization guard in
33+
* {@code updateItemDatesMetadata}. Fully mocked — no DSpace kernel or database.
34+
*
35+
* @author dataquest
36+
*/
37+
public class ClarinItemServiceImplTest {
38+
39+
private ItemService itemService;
40+
private ClarinItemServiceImpl clarinItemService;
41+
private Item item;
42+
private Context context;
43+
44+
@Before
45+
public void setUp() {
46+
itemService = mock(ItemService.class);
47+
item = mock(Item.class);
48+
context = mock(Context.class);
49+
clarinItemService = new ClarinItemServiceImpl();
50+
ReflectionTestUtils.setField(clarinItemService, "itemService", itemService);
51+
}
52+
53+
private MetadataValue mv(String value) {
54+
MetadataValue metadataValue = mock(MetadataValue.class);
55+
when(metadataValue.getValue()).thenReturn(value);
56+
return metadataValue;
57+
}
58+
59+
private void mockApproximateDate(String value) {
60+
List<MetadataValue> values = value == null ? Collections.emptyList() : Collections.singletonList(mv(value));
61+
when(itemService.getMetadata(item, "local", "approximateDate", "issued", Item.ANY, false))
62+
.thenReturn(values);
63+
}
64+
65+
private void mockCurrentDateIssued(List<MetadataValue> values) {
66+
when(itemService.getMetadata(item, "dc", "date", "issued", Item.ANY, false)).thenReturn(values);
67+
}
68+
69+
// ---- deriveDateIssuedFromApproximateDate (pure, no DB) ----
70+
71+
@Test
72+
public void derive_returnsNull_whenNoApproximateDate() {
73+
mockApproximateDate(null);
74+
assertNull(clarinItemService.deriveDateIssuedFromApproximateDate(item));
75+
}
76+
77+
@Test
78+
public void derive_returnsNull_whenApproximateDateBlank() {
79+
mockApproximateDate(" ");
80+
assertNull(clarinItemService.deriveDateIssuedFromApproximateDate(item));
81+
}
82+
83+
@Test
84+
public void derive_returnsNoYear_whenNonNumeric() {
85+
mockApproximateDate("spring 1945");
86+
assertEquals("0000", clarinItemService.deriveDateIssuedFromApproximateDate(item));
87+
}
88+
89+
@Test
90+
public void derive_returnsLastYear_whenNumericSequence() {
91+
mockApproximateDate("1938, 1945, 2022");
92+
assertEquals("2022", clarinItemService.deriveDateIssuedFromApproximateDate(item));
93+
}
94+
95+
@Test
96+
public void derive_returnsSingleYear() {
97+
mockApproximateDate("1990");
98+
assertEquals("1990", clarinItemService.deriveDateIssuedFromApproximateDate(item));
99+
}
100+
101+
// ---- updateItemDatesMetadata: skip-write / normalization guard ----
102+
103+
@Test
104+
public void update_skipsWrite_whenSingleValueAlreadyDerived() throws SQLException {
105+
mockApproximateDate("2022");
106+
mockCurrentDateIssued(Collections.singletonList(mv("2022")));
107+
108+
clarinItemService.updateItemDatesMetadata(context, item);
109+
110+
verify(itemService, never()).clearMetadata(context, item, "dc", "date", "issued", Item.ANY);
111+
verify(itemService, never()).addMetadata(context, item, "dc", "date", "issued", Item.ANY, "2022");
112+
}
113+
114+
@Test
115+
public void update_normalizesMultiValue_evenWhenFirstMatchesDerived() throws SQLException {
116+
// Regression guard: a multi-valued dc.date.issued must still be collapsed to the single derived value,
117+
// even if the first stored value already equals the derived one.
118+
mockApproximateDate("2022");
119+
mockCurrentDateIssued(Arrays.asList(mv("2022"), mv("1999")));
120+
121+
clarinItemService.updateItemDatesMetadata(context, item);
122+
123+
verify(itemService).clearMetadata(context, item, "dc", "date", "issued", Item.ANY);
124+
verify(itemService).addMetadata(context, item, "dc", "date", "issued", Item.ANY, "2022");
125+
}
126+
127+
@Test
128+
public void update_writes_whenSingleValueDiffers() throws SQLException {
129+
mockApproximateDate("2022");
130+
mockCurrentDateIssued(Collections.singletonList(mv("1900")));
131+
132+
clarinItemService.updateItemDatesMetadata(context, item);
133+
134+
verify(itemService).clearMetadata(context, item, "dc", "date", "issued", Item.ANY);
135+
verify(itemService).addMetadata(context, item, "dc", "date", "issued", Item.ANY, "2022");
136+
}
137+
138+
@Test
139+
public void update_skips_whenApproximateDateEmpty() throws SQLException {
140+
mockApproximateDate(null);
141+
142+
clarinItemService.updateItemDatesMetadata(context, item);
143+
144+
verify(itemService, never()).clearMetadata(context, item, "dc", "date", "issued", Item.ANY);
145+
}
146+
}

‎dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.io.IOException;
2020
import java.sql.SQLException;
2121
import java.util.List;
22-
import java.util.Random;
2322

2423
import okhttp3.mockwebserver.Dispatcher;
2524
import okhttp3.mockwebserver.MockResponse;
@@ -58,7 +57,9 @@
5857
public class ItemHandleCheckerIT extends AbstractIntegrationTestWithDatabase {
5958
private static final String TASK_NAME = "checkhandles";
6059

61-
private static final String HANDLE_COLLECTION = "123456789/" + randomString();
60+
// Non-numeric suffix the handle sequence can never mint, unique to this class so it cannot
61+
// clash with leftover handle rows of other tests either.
62+
private static final String HANDLE_COLLECTION = "123456789/handle-checker-test";
6263

6364
private static final String HANDLE_ITEM1 = HANDLE_COLLECTION + "-1";
6465
private static final String HANDLE_ITEM2 = HANDLE_COLLECTION + "-2";
@@ -280,10 +281,4 @@ private String getIdentifierUri(Item item) {
280281
private List<MetadataValue> getIdentifierUris(Item item) {
281282
return itemService.getMetadata(item, "dc", "identifier", "uri", Item.ANY);
282283
}
283-
284-
private static String randomString() {
285-
Random r = new Random();
286-
// Generate random integers in range 1000 to 1999
287-
return String.valueOf(1000 + r.nextInt(1000));
288-
}
289284
}

0 commit comments

Comments
 (0)