diff --git a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java index 9c853613a94..1925b1d48df 100644 --- a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java @@ -50,6 +50,7 @@ import org.dspace.content.service.MetadataSchemaService; import org.dspace.content.service.RelationshipService; import org.dspace.content.service.WorkspaceItemService; +import org.dspace.content.service.clarin.ClarinItemService; import org.dspace.content.virtual.VirtualMetadataPopulator; import org.dspace.core.Constants; import org.dspace.core.Context; @@ -180,6 +181,9 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl implements It @Autowired private VersionHistoryService versionHistoryService; + @Autowired(required = true) + private ClarinItemService clarinItemService; + @Autowired(required = true) ClarinMatomoBitstreamTracker matomoBitstreamTracker; @@ -686,6 +690,11 @@ public void update(Context context, Item item) throws SQLException, AuthorizeExc } if (item.isMetadataModified() || item.isModified()) { + // Derive dc.date.issued from local.approximateDate.issued when metadata changes + if (item.isMetadataModified()) { + clarinItemService.updateItemDatesMetadata(context, item); + } + // Set the last modified date item.setLastModified(new Date()); diff --git a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinItemServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinItemServiceImpl.java index 018964b4cbf..faeb0dacce8 100644 --- a/dspace-api/src/main/java/org/dspace/content/clarin/ClarinItemServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/clarin/ClarinItemServiceImpl.java @@ -223,12 +223,33 @@ public void updateItemDatesMetadata(Context context, Item item) throws SQLExcept return; } + String derivedDate = deriveDateIssuedFromApproximateDate(item); + if (derivedDate == null) { + log.debug("Cannot update item dates metadata because the approximate date is empty."); + return; + } + + // Skip the write only when dc.date.issued already holds exactly the single derived value. + // A multi-valued field must still be normalized down to the single derived value. + List currentDateIssued = + itemService.getMetadata(item, "dc", "date", "issued", Item.ANY, false); + if (currentDateIssued.size() == 1 + && derivedDate.equals(currentDateIssued.get(0).getValue())) { + return; + } + + // Clear the current `dc.date.issued` metadata and set it to the derived value + itemService.clearMetadata(context, item, "dc", "date", "issued", Item.ANY); + itemService.addMetadata(context, item, "dc", "date", "issued", Item.ANY, derivedDate); + } + + @Override + public String deriveDateIssuedFromApproximateDate(Item item) { List approximatedDates = itemService.getMetadata(item, "local", "approximateDate", "issued", Item.ANY, false); if (CollectionUtils.isEmpty(approximatedDates) || StringUtils.isBlank(approximatedDates.get(0).getValue())) { - log.debug("Cannot update item dates metadata because the approximate date is empty."); - return; + return null; } // Get the approximate date value from the metadata @@ -239,21 +260,11 @@ public void updateItemDatesMetadata(Context context, Item item) throws SQLExcept // Trim the list of years - remove leading and trailing whitespaces listOfYearValues.replaceAll(String::trim); - try { - // Clear the current `dc.date.issued` metadata - itemService.clearMetadata(context, item, "dc", "date", "issued", Item.ANY); - - // Update the `dc.date.issued` metadata with a new value: `0000` or the last year from the sequence - if (CollectionUtils.isNotEmpty(listOfYearValues) && isListOfNumbers(listOfYearValues)) { - // Take the last year from the list of years and add it to the `dc.date.issued` metadata - itemService.addMetadata(context, item, "dc", "date", "issued", Item.ANY, - getLastNumber(listOfYearValues)); - } else { - // Add the `0000` value to the `dc.date.issued` metadata - itemService.addMetadata(context, item, "dc", "date", "issued", Item.ANY, NO_YEAR); - } - } catch (SQLException e) { - log.error("Cannot remove `dc.date.issued` metadata because: {}", e.getMessage()); + // `0000` when the approximate date is not a list of numbers, otherwise the last year in the sequence + if (CollectionUtils.isNotEmpty(listOfYearValues) && isListOfNumbers(listOfYearValues)) { + return getLastNumber(listOfYearValues); + } else { + return NO_YEAR; } } diff --git a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinItemService.java b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinItemService.java index 0559b10e137..6009cbf94cc 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinItemService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinItemService.java @@ -102,4 +102,16 @@ public interface ClarinItemService { */ void updateItemDatesMetadata(Context context, Item item) throws SQLException; + /** + * Derive the display value for {@code dc.date.issued} from the item's + * {@code local.approximateDate.issued} metadata, without touching the database. + * Returns the last year for a numeric sequence (e.g. "1938, 1945" -> "1945"), + * {@code "0000"} for a non-numeric approximate value, or {@code null} when no + * approximate date is present. + * + * @param item the item to derive the date from + * @return the derived {@code dc.date.issued} value, or {@code null} if none applies + */ + String deriveDateIssuedFromApproximateDate(Item item); + } diff --git a/dspace-api/src/test/java/org/dspace/content/clarin/ClarinItemServiceImplTest.java b/dspace-api/src/test/java/org/dspace/content/clarin/ClarinItemServiceImplTest.java new file mode 100644 index 00000000000..52f065c922e --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/content/clarin/ClarinItemServiceImplTest.java @@ -0,0 +1,146 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.content.clarin; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.sql.SQLException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.dspace.content.Item; +import org.dspace.content.MetadataValue; +import org.dspace.content.service.ItemService; +import org.dspace.core.Context; +import org.junit.Before; +import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; + +/** + * Pure unit tests for {@link ClarinItemServiceImpl}: the {@code dc.date.issued} derivation + * from {@code local.approximateDate.issued}, and the normalization guard in + * {@code updateItemDatesMetadata}. Fully mocked — no DSpace kernel or database. + * + * @author dataquest + */ +public class ClarinItemServiceImplTest { + + private ItemService itemService; + private ClarinItemServiceImpl clarinItemService; + private Item item; + private Context context; + + @Before + public void setUp() { + itemService = mock(ItemService.class); + item = mock(Item.class); + context = mock(Context.class); + clarinItemService = new ClarinItemServiceImpl(); + ReflectionTestUtils.setField(clarinItemService, "itemService", itemService); + } + + private MetadataValue mv(String value) { + MetadataValue metadataValue = mock(MetadataValue.class); + when(metadataValue.getValue()).thenReturn(value); + return metadataValue; + } + + private void mockApproximateDate(String value) { + List values = value == null ? Collections.emptyList() : Collections.singletonList(mv(value)); + when(itemService.getMetadata(item, "local", "approximateDate", "issued", Item.ANY, false)) + .thenReturn(values); + } + + private void mockCurrentDateIssued(List values) { + when(itemService.getMetadata(item, "dc", "date", "issued", Item.ANY, false)).thenReturn(values); + } + + // ---- deriveDateIssuedFromApproximateDate (pure, no DB) ---- + + @Test + public void derive_returnsNull_whenNoApproximateDate() { + mockApproximateDate(null); + assertNull(clarinItemService.deriveDateIssuedFromApproximateDate(item)); + } + + @Test + public void derive_returnsNull_whenApproximateDateBlank() { + mockApproximateDate(" "); + assertNull(clarinItemService.deriveDateIssuedFromApproximateDate(item)); + } + + @Test + public void derive_returnsNoYear_whenNonNumeric() { + mockApproximateDate("spring 1945"); + assertEquals("0000", clarinItemService.deriveDateIssuedFromApproximateDate(item)); + } + + @Test + public void derive_returnsLastYear_whenNumericSequence() { + mockApproximateDate("1938, 1945, 2022"); + assertEquals("2022", clarinItemService.deriveDateIssuedFromApproximateDate(item)); + } + + @Test + public void derive_returnsSingleYear() { + mockApproximateDate("1990"); + assertEquals("1990", clarinItemService.deriveDateIssuedFromApproximateDate(item)); + } + + // ---- updateItemDatesMetadata: skip-write / normalization guard ---- + + @Test + public void update_skipsWrite_whenSingleValueAlreadyDerived() throws SQLException { + mockApproximateDate("2022"); + mockCurrentDateIssued(Collections.singletonList(mv("2022"))); + + clarinItemService.updateItemDatesMetadata(context, item); + + verify(itemService, never()).clearMetadata(context, item, "dc", "date", "issued", Item.ANY); + verify(itemService, never()).addMetadata(context, item, "dc", "date", "issued", Item.ANY, "2022"); + } + + @Test + public void update_normalizesMultiValue_evenWhenFirstMatchesDerived() throws SQLException { + // Regression guard: a multi-valued dc.date.issued must still be collapsed to the single derived value, + // even if the first stored value already equals the derived one. + mockApproximateDate("2022"); + mockCurrentDateIssued(Arrays.asList(mv("2022"), mv("1999"))); + + clarinItemService.updateItemDatesMetadata(context, item); + + verify(itemService).clearMetadata(context, item, "dc", "date", "issued", Item.ANY); + verify(itemService).addMetadata(context, item, "dc", "date", "issued", Item.ANY, "2022"); + } + + @Test + public void update_writes_whenSingleValueDiffers() throws SQLException { + mockApproximateDate("2022"); + mockCurrentDateIssued(Collections.singletonList(mv("1900"))); + + clarinItemService.updateItemDatesMetadata(context, item); + + verify(itemService).clearMetadata(context, item, "dc", "date", "issued", Item.ANY); + verify(itemService).addMetadata(context, item, "dc", "date", "issued", Item.ANY, "2022"); + } + + @Test + public void update_skips_whenApproximateDateEmpty() throws SQLException { + mockApproximateDate(null); + + clarinItemService.updateItemDatesMetadata(context, item); + + verify(itemService, never()).clearMetadata(context, item, "dc", "date", "issued", Item.ANY); + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ItemConverter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ItemConverter.java index d4513db54ca..e6059db9f6f 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ItemConverter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ItemConverter.java @@ -18,6 +18,7 @@ import org.apache.logging.log4j.Logger; import org.dspace.app.rest.model.ItemRest; import org.dspace.app.rest.model.MetadataValueList; +import org.dspace.app.rest.model.MetadataValueRest; import org.dspace.app.rest.projection.Projection; import org.dspace.app.rest.utils.ContextUtil; import org.dspace.content.Item; @@ -27,7 +28,6 @@ import org.dspace.content.service.clarin.ClarinItemService; import org.dspace.core.Context; import org.dspace.discovery.IndexableObject; -import org.dspace.services.model.Request; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.springframework.util.ObjectUtils; @@ -53,18 +53,6 @@ public class ItemConverter @Override public ItemRest convert(Item obj, Projection projection) { - Context context = null; - Request currentRequest = requestService.getCurrentRequest(); - if (currentRequest != null) { - context = ContextUtil.obtainContext(currentRequest.getHttpServletRequest()); - } - try { - clarinItemService.updateItemDatesMetadata(context, obj); - } catch (SQLException e) { - log.error("Error updating item dates metadata", e); - throw new RuntimeException(e); - } - ItemRest item = super.convert(obj, projection); item.setInArchive(obj.isArchived()); item.setDiscoverable(obj.isDiscoverable()); @@ -77,9 +65,40 @@ public ItemRest convert(Item obj, Projection projection) { item.setEntityType(entityTypes.get(0).getValue()); } + // Override dc.date.issued on the REST DTO with the value derived from + // local.approximateDate.issued. Display-only: it does not modify the entity or the database. + overrideDateIssuedFromApproximateDate(obj, item); + return item; } + /** + * If the item has a {@code local.approximateDate.issued} value, override {@code dc.date.issued} + * on the REST DTO using {@link ClarinItemService#deriveDateIssuedFromApproximateDate(Item)}. + * Display-only (no database writes); skipped when {@code dc.date.issued} is hidden. + */ + private void overrideDateIssuedFromApproximateDate(Item source, ItemRest target) { + String derivedValue = clarinItemService.deriveDateIssuedFromApproximateDate(source); + if (derivedValue == null) { + return; + } + + Context context = ContextUtil.obtainCurrentRequestContext(); + try { + if (metadataExposureService.isHidden(context, "dc", "date", "issued", source)) { + return; + } + } catch (SQLException e) { + log.error("Error checking metadata visibility for dc.date.issued", e); + return; + } + + MetadataValueRest dateRest = new MetadataValueRest(derivedValue); + dateRest.setConfidence(-1); + // MetadataRest#put normalizes place and keeps Arrays.asList semantics consistent with other fields + target.getMetadata().put("dc.date.issued", dateRest); + } + /** * Retrieves the metadata list filtered according to the hidden metadata configuration * When the context is null, it will return the metadatalist as for an anonymous user