From 96a5cbb90b1750758b984c1cbc434bf66d04a5e4 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Fri, 1 Mar 2024 08:52:55 +0100 Subject: [PATCH] DSC-1561 avoid tracking of placeholder for virtual metadata enhancement --- .../impl/RelatedEntityItemEnhancer.java | 382 ++++++++---------- .../app/matcher/MetadataValueMatcher.java | 7 +- .../consumer/ItemEnhancerConsumerIT.java | 101 +++-- .../enhancer/script/ItemEnhancerScriptIT.java | 69 ++-- 4 files changed, 251 insertions(+), 308 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/content/enhancer/impl/RelatedEntityItemEnhancer.java b/dspace-api/src/main/java/org/dspace/content/enhancer/impl/RelatedEntityItemEnhancer.java index f17e36ee90a..284f9b8bff8 100644 --- a/dspace-api/src/main/java/org/dspace/content/enhancer/impl/RelatedEntityItemEnhancer.java +++ b/dspace-api/src/main/java/org/dspace/content/enhancer/impl/RelatedEntityItemEnhancer.java @@ -11,10 +11,14 @@ import java.sql.SQLException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; -import java.util.Objects; +import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.dspace.content.Item; @@ -45,10 +49,19 @@ public class RelatedEntityItemEnhancer extends AbstractItemEnhancer { @Autowired private ItemService itemService; + /** + * the entity that can be extended by this enhancer, i.e. Publication + */ private String sourceEntityType; + /** + * the metadata used to navigate the relation, i.e. dc.contributor.author + */ private String sourceItemMetadataField; + /** + * the metadata that is copied from the linked entity, i.e. person.identifier.orcid + */ private String relatedItemMetadataField; @Override @@ -62,18 +75,17 @@ public boolean enhance(Context context, Item item, boolean deepMode) { if (!deepMode) { try { result = cleanObsoleteVirtualFields(context, item); - result = updateVirtualFieldsPlaces(context, item) || result; result = performEnhancement(context, item) || result; } catch (SQLException e) { LOGGER.error("An error occurs enhancing item with id {}: {}", item.getID(), e.getMessage(), e); throw new SQLRuntimeException(e); } } else { - List currMetadataValues = getCurrentVirtualMetadata(context, item); - List toBeMetadataValues = getToBeVirtualMetadata(context, item); + Map> currMetadataValues = getCurrentVirtualsMap(item); + Map> toBeMetadataValues = getToBeVirtualMetadata(context, item); if (!equivalent(currMetadataValues, toBeMetadataValues)) { try { - itemService.removeMetadataValues(context, item, currMetadataValues); + clearAllVirtualMetadata(context, item); addMetadata(context, item, toBeMetadataValues); } catch (SQLException e) { throw new SQLRuntimeException(e); @@ -84,20 +96,44 @@ public boolean enhance(Context context, Item item, boolean deepMode) { return result; } - private void addMetadata(Context context, Item item, List toBeMetadataValues) + private void clearAllVirtualMetadata(Context context, Item item) throws SQLException { + itemService.clearMetadata(context, item, VIRTUAL_METADATA_SCHEMA, VIRTUAL_SOURCE_METADATA_ELEMENT, + getVirtualQualifier(), Item.ANY); + itemService.clearMetadata(context, item, VIRTUAL_METADATA_SCHEMA, VIRTUAL_METADATA_ELEMENT, + getVirtualQualifier(), Item.ANY); + } + + private void addMetadata(Context context, Item item, Map> toBeMetadataValues) throws SQLException { - for (MetadataValueDTO dto : toBeMetadataValues) { - itemService.addMetadata(context, item, dto.getSchema(), dto.getElement(), dto.getQualifier(), null, - dto.getValue(), dto.getAuthority(), dto.getConfidence()); + for (Entry> metadataValues : toBeMetadataValues.entrySet()) { + addVirtualSourceField(context, item, metadataValues.getKey()); + for (MetadataValueDTO dto : metadataValues.getValue()) { + addVirtualField(context, item, dto.getValue(), dto.getAuthority(), dto.getLanguage(), + dto.getConfidence()); + } } } - private boolean equivalent(List currMetadataValues, List toBeMetadataValues) { + private boolean equivalent(Map> currMetadataValues, + Map> toBeMetadataValues) { if (currMetadataValues.size() != toBeMetadataValues.size()) { return false; } else { - for (int idx = 0; idx < currMetadataValues.size(); idx++) { - if (!equivalent(currMetadataValues.get(idx), toBeMetadataValues.get(idx))) { + for (String key : currMetadataValues.keySet()) { + if (!equivalent(currMetadataValues.get(key), toBeMetadataValues.get(key))) { + return false; + } + } + } + return true; + } + + private boolean equivalent(List metadataValue, List metadataValueDTO) { + if (metadataValue.size() != metadataValueDTO.size()) { + return false; + } else { + for (int i = 0; i < metadataValue.size(); i++) { + if (!equivalent(metadataValue.get(i), metadataValueDTO.get(i))) { return false; } } @@ -114,23 +150,14 @@ private boolean equivalent(MetadataValue metadataValue, MetadataValueDTO metadat && StringUtils.equals(metadataValue.getAuthority(), metadataValueDTO.getAuthority()); } - private List getToBeVirtualMetadata(Context context, Item item) { - List tobeVirtualMetadata = new ArrayList<>(); - List virtualSourceFields = getEnhanceableMetadataValue(item); - for (MetadataValue virtualSourceField : virtualSourceFields) { - MetadataValueDTO mv = new MetadataValueDTO(); - mv.setSchema(VIRTUAL_METADATA_SCHEMA); - mv.setElement(VIRTUAL_SOURCE_METADATA_ELEMENT); - mv.setQualifier(getVirtualQualifier()); - String authority = virtualSourceField.getAuthority(); + private Map> getToBeVirtualMetadata(Context context, Item item) { + Map> tobeVirtualMetadataMap = new HashMap>(); + + Set virtualSources = getVirtualSources(item); + for (String authority : virtualSources) { + List tobeVirtualMetadata = new ArrayList<>(); Item relatedItem = null; - if (StringUtils.isNotBlank(authority)) { - mv.setValue(authority); - relatedItem = findRelatedEntityItem(context, virtualSourceField); - } else { - mv.setValue(PLACEHOLDER_PARENT_METADATA_VALUE); - } - tobeVirtualMetadata.add(mv); + relatedItem = findRelatedEntityItem(context, authority); if (relatedItem == null) { MetadataValueDTO mvRelated = new MetadataValueDTO(); mvRelated.setSchema(VIRTUAL_METADATA_SCHEMA); @@ -138,44 +165,35 @@ private List getToBeVirtualMetadata(Context context, Item item mvRelated.setQualifier(getVirtualQualifier()); mvRelated.setValue(PLACEHOLDER_PARENT_METADATA_VALUE); tobeVirtualMetadata.add(mvRelated); - continue; - } - - List relatedItemMetadataValues = getMetadataValues(relatedItem, relatedItemMetadataField); - if (relatedItemMetadataValues.isEmpty()) { - MetadataValueDTO mvRelated = new MetadataValueDTO(); - mvRelated.setSchema(VIRTUAL_METADATA_SCHEMA); - mvRelated.setElement(VIRTUAL_METADATA_ELEMENT); - mvRelated.setQualifier(getVirtualQualifier()); - mvRelated.setValue(PLACEHOLDER_PARENT_METADATA_VALUE); - tobeVirtualMetadata.add(mvRelated); - continue; - } - for (MetadataValue relatedItemMetadataValue : relatedItemMetadataValues) { - MetadataValueDTO mvRelated = new MetadataValueDTO(); - mvRelated.setSchema(VIRTUAL_METADATA_SCHEMA); - mvRelated.setElement(VIRTUAL_METADATA_ELEMENT); - mvRelated.setQualifier(getVirtualQualifier()); - mvRelated.setValue(relatedItemMetadataValue.getValue()); - String authorityRelated = relatedItemMetadataValue.getAuthority(); - if (StringUtils.isNotBlank(authorityRelated)) { - mvRelated.setAuthority(authorityRelated); - mvRelated.setConfidence(Choices.CF_ACCEPTED); + } else { + List relatedItemMetadataValues = getMetadataValues(relatedItem, + relatedItemMetadataField); + if (relatedItemMetadataValues.isEmpty()) { + MetadataValueDTO mvRelated = new MetadataValueDTO(); + mvRelated.setSchema(VIRTUAL_METADATA_SCHEMA); + mvRelated.setElement(VIRTUAL_METADATA_ELEMENT); + mvRelated.setQualifier(getVirtualQualifier()); + mvRelated.setValue(PLACEHOLDER_PARENT_METADATA_VALUE); + tobeVirtualMetadata.add(mvRelated); + } else { + for (MetadataValue relatedItemMetadataValue : relatedItemMetadataValues) { + MetadataValueDTO mvRelated = new MetadataValueDTO(); + mvRelated.setSchema(VIRTUAL_METADATA_SCHEMA); + mvRelated.setElement(VIRTUAL_METADATA_ELEMENT); + mvRelated.setQualifier(getVirtualQualifier()); + mvRelated.setValue(relatedItemMetadataValue.getValue()); + String authorityRelated = relatedItemMetadataValue.getAuthority(); + if (StringUtils.isNotBlank(authorityRelated)) { + mvRelated.setAuthority(authorityRelated); + mvRelated.setConfidence(Choices.CF_ACCEPTED); + } + tobeVirtualMetadata.add(mvRelated); + } } - tobeVirtualMetadata.add(mvRelated); } + tobeVirtualMetadataMap.put(authority, tobeVirtualMetadata); } - return tobeVirtualMetadata; - } - - private List getCurrentVirtualMetadata(Context context, Item item) { - List currentVirtualMetadata = new ArrayList<>(); - List virtualSourceFields = getVirtualSourceFields(item); - for (MetadataValue virtualSourceField : virtualSourceFields) { - currentVirtualMetadata.add(virtualSourceField); - getRelatedVirtualField(item, virtualSourceField).ifPresent(currentVirtualMetadata::add); - } - return currentVirtualMetadata; + return tobeVirtualMetadataMap; } private boolean cleanObsoleteVirtualFields(Context context, Item item) throws SQLException { @@ -188,66 +206,17 @@ private boolean cleanObsoleteVirtualFields(Context context, Item item) throws SQ return result; } - private boolean updateVirtualFieldsPlaces(Context context, Item item) { - boolean result = false; - List virtualSourceFields = getVirtualSourceFields(item); - List enhanceableMetadataValue = getEnhanceableMetadataValue(item); - for (MetadataValue virtualSourceField : virtualSourceFields) { - Optional metadataWithPlaceToUpdate = metadataWithPlaceToUpdate(item, - enhanceableMetadataValue, virtualSourceField); - if (metadataWithPlaceToUpdate.isPresent()) { - updatePlaces(item, metadataWithPlaceToUpdate.get(), virtualSourceField); - result = true; - } - } - return result; - } - - private Optional metadataWithPlaceToUpdate(Item item, List enhanceableMetadataValue, - MetadataValue virtualSourceField) { - return findMetadataValueToUpdatePlace(enhanceableMetadataValue, virtualSourceField, - item); - } - - private boolean hasToUpdatePlace(MetadataValue metadataValue, MetadataValue virtualSourceField) { - return metadataValue.getPlace() != virtualSourceField.getPlace(); - } - - private void updatePlaces(Item item, MetadataValue mv, MetadataValue virtualSourceField) { - virtualSourceField.setPlace(mv.getPlace()); - getRelatedVirtualField(item, mv) - .ifPresent(relatedMv -> relatedMv.setPlace(mv.getPlace())); - } - - private Optional findMetadataValueToUpdatePlace(List enhanceableMetadataValue, - MetadataValue virtualSourceField, Item item) { - Optional exactMatch = enhanceableMetadataValue.stream() - .filter(metadataValue -> hasAuthorityEqualsTo(metadataValue, - virtualSourceField.getValue()) && !hasToUpdatePlace(metadataValue, virtualSourceField)) - .findFirst(); - if (exactMatch.isPresent()) { - enhanceableMetadataValue.remove(exactMatch.get()); - return Optional.empty(); - } else { - Optional authorityOnlyMatch = enhanceableMetadataValue.stream() - .filter(metadataValue -> hasAuthorityEqualsTo(metadataValue, - virtualSourceField.getValue()) && hasToUpdatePlace(metadataValue, virtualSourceField)) - .findFirst(); - enhanceableMetadataValue.remove(authorityOnlyMatch.get()); - return authorityOnlyMatch; - } - } - private List getObsoleteVirtualFields(Item item) { List obsoleteVirtualFields = new ArrayList<>(); - - List virtualSourceFields = getVirtualSourceFields(item); - List enhanceableMetadata = getEnhanceableMetadataValue(item); - for (MetadataValue virtualSourceField : virtualSourceFields) { - if (isRelatedSourceNoMorePresent(item, enhanceableMetadata, virtualSourceField)) { - obsoleteVirtualFields.add(virtualSourceField); - getRelatedVirtualField(item, virtualSourceField).ifPresent(obsoleteVirtualFields::add); + Map> currentVirtualsMap = getCurrentVirtualsMap(item); + Set virtualSources = getVirtualSources(item); + for (String authority : currentVirtualsMap.keySet()) { + if (!virtualSources.contains(authority)) { + for (MetadataValue mv : getVirtualSourceFields(item, authority)) { + obsoleteVirtualFields.add(mv); + getRelatedVirtualField(item, mv.getPlace()).ifPresent(obsoleteVirtualFields::add); + } } } @@ -255,142 +224,111 @@ private List getObsoleteVirtualFields(Item item) { } - /** - * This method will look in the enhanceableMetadata if the source metadata is still present. If so, it will remove - * form the list as it would not be used to validate other potential duplicate source metadata - * - * @param item - * @param enhanceableMetadata - * @param virtualSourceField - * @return true if the metadata containing a source of enhancement is still present in the list of the metadata to - * use to enhance the item - */ - private boolean isRelatedSourceNoMorePresent(Item item, List enhanceableMetadata, - MetadataValue virtualSourceField) { - Optional mv = enhanceableMetadata.stream() - .filter(metadataValue -> hasAuthorityEqualsTo(metadataValue, virtualSourceField.getValue())) - .findFirst(); - if (mv.isPresent()) { - enhanceableMetadata.remove(mv.get()); - return false; - } - return true; + private Set getVirtualSources(Item item) { + return itemService.getMetadataByMetadataString(item, sourceItemMetadataField).stream() + .filter(mv -> UUIDUtils.fromString(mv.getAuthority()) != null).map(mv -> mv.getAuthority()) + .collect(Collectors.toSet()); } - private Optional getRelatedVirtualField(Item item, MetadataValue virtualSourceField) { - return getVirtualFields(item).stream() - .filter(metadataValue -> metadataValue.getPlace() == virtualSourceField.getPlace()) - .findFirst(); - } + private Map> getCurrentVirtualsMap(Item item) { + Map> currentVirtualsMap = new HashMap>(); + List sources = itemService.getMetadata(item, VIRTUAL_METADATA_SCHEMA, + VIRTUAL_SOURCE_METADATA_ELEMENT, getVirtualQualifier(), Item.ANY); + List generated = itemService.getMetadata(item, VIRTUAL_METADATA_SCHEMA, VIRTUAL_METADATA_ELEMENT, + getVirtualQualifier(), Item.ANY); - private boolean performEnhancement(Context context, Item item) throws SQLException { - boolean result = false; - if (noEnhanceableMetadata(context, item)) { - return false; + if (sources.size() != generated.size()) { + LOGGER.error( + "inconsistent virtual metadata for the item {} got {} sources and {} generated virtual metadata", + item.getID().toString(), sources.size(), generated.size()); } - for (MetadataValue metadataValue : getEnhanceableMetadataValue(item)) { - - if (wasValueAlreadyUsedForEnhancement(item, metadataValue)) { - continue; - } - - Item relatedItem = findRelatedEntityItem(context, metadataValue); - if (relatedItem == null) { - addVirtualField(context, item, PLACEHOLDER_PARENT_METADATA_VALUE); - addVirtualSourceField(context, item, metadataValue); - continue; + for (int i = 0; i < Integer.max(sources.size(), generated.size()); i++) { + String authority; + if (i < sources.size()) { + authority = sources.get(i).getValue(); + } else { + // we have less source than virtual metadata let's generate a random uuid to + // associate with these extra metadata so that they will be managed as obsolete + // value + authority = UUID.randomUUID().toString(); } - - List relatedItemMetadataValues = getMetadataValues(relatedItem, relatedItemMetadataField); - if (relatedItemMetadataValues.isEmpty()) { - addVirtualField(context, item, PLACEHOLDER_PARENT_METADATA_VALUE); - addVirtualSourceField(context, item, metadataValue); - continue; + List mvalues = currentVirtualsMap.get(authority); + if (mvalues == null) { + mvalues = new ArrayList(); } - for (MetadataValue relatedItemMetadataValue : relatedItemMetadataValues) { - addVirtualField(context, item, relatedItemMetadataValue.getValue()); - addVirtualSourceField(context, item, metadataValue); + if (i < generated.size()) { + mvalues.add(generated.get(i)); } - result = true; + currentVirtualsMap.put(authority, mvalues); } - return result; - } - - private boolean noEnhanceableMetadata(Context context, Item item) { - - return getEnhanceableMetadataValue(item) - .stream() - .noneMatch(metadataValue -> validAuthority(context, metadataValue)); - } - - private boolean validAuthority(Context context, MetadataValue metadataValue) { - Item relatedItem = findRelatedEntityItem(context, metadataValue); - return Objects.nonNull(relatedItem); + return currentVirtualsMap; } - private List getEnhanceableMetadataValue(Item item) { - return getMetadataValues(item, sourceItemMetadataField); + private Optional getRelatedVirtualField(Item item, int pos) { + return getVirtualFields(item).stream() + .skip(pos) + .findFirst(); } - private boolean wasValueAlreadyUsedForEnhancement(Item item, MetadataValue metadataValue) { + private boolean performEnhancement(Context context, Item item) throws SQLException { + boolean result = false; + Map> currentVirtualsMap = getCurrentVirtualsMap(item); + Set virtualSources = getVirtualSources(item); + for (String authority : virtualSources) { + if (!currentVirtualsMap.containsKey(authority)) { + Item relatedItem = findRelatedEntityItem(context, authority); + if (relatedItem == null) { + addVirtualField(context, item, PLACEHOLDER_PARENT_METADATA_VALUE, null, null, Choices.CF_UNSET); + addVirtualSourceField(context, item, authority); + continue; + } - if (isPlaceholderAtPlace(getVirtualFields(item), metadataValue.getPlace())) { - return true; + List relatedItemMetadataValues = getMetadataValues(relatedItem, + relatedItemMetadataField); + if (relatedItemMetadataValues.isEmpty()) { + addVirtualField(context, item, PLACEHOLDER_PARENT_METADATA_VALUE, null, null, Choices.CF_UNSET); + addVirtualSourceField(context, item, authority); + continue; + } + for (MetadataValue relatedItemMetadataValue : relatedItemMetadataValues) { + addVirtualField(context, item, relatedItemMetadataValue.getValue(), + relatedItemMetadataValue.getAuthority(), relatedItemMetadataValue.getLanguage(), + relatedItemMetadataValue.getConfidence()); + addVirtualSourceField(context, item, authority); + } + result = true; + } } - - return getVirtualSourceFields(item).stream() - .anyMatch(virtualSourceField -> virtualSourceField.getPlace() == metadataValue.getPlace() - && hasAuthorityEqualsTo(metadataValue, virtualSourceField.getValue())); - - } - - private boolean isPlaceholderAtPlace(List metadataValues, int place) { - return place < metadataValues.size() ? isPlaceholder(metadataValues.get(place)) : false; - } - - private boolean hasAuthorityEqualsTo(MetadataValue metadataValue, String authority) { - return Objects.equals(metadataValue.getAuthority(), authority) - || (StringUtils.isBlank(metadataValue.getAuthority()) - && Objects.equals(PLACEHOLDER_PARENT_METADATA_VALUE, authority)); + return result; } - private Item findRelatedEntityItem(Context context, MetadataValue metadataValue) { + private Item findRelatedEntityItem(Context context, String authority) { try { - UUID relatedItemUUID = UUIDUtils.fromString(metadataValue.getAuthority()); + UUID relatedItemUUID = UUIDUtils.fromString(authority); return relatedItemUUID != null ? itemService.find(context, relatedItemUUID) : null; } catch (SQLException e) { throw new SQLRuntimeException(e); } } - private boolean isPlaceholder(MetadataValue metadataValue) { - return PLACEHOLDER_PARENT_METADATA_VALUE.equals(metadataValue.getValue()); - } - private List getMetadataValues(Item item, String metadataField) { return itemService.getMetadataByMetadataString(item, metadataField); } - private List getVirtualSourceFields(Item item) { - return getMetadataValues(item, getVirtualSourceMetadataField()); + private List getVirtualSourceFields(Item item, String authority) { + return getMetadataValues(item, getVirtualSourceMetadataField()).stream() + .filter(mv -> StringUtils.equals(authority, mv.getValue())).collect(Collectors.toList()); } private List getVirtualFields(Item item) { return getMetadataValues(item, getVirtualMetadataField()); } - private void addVirtualField(Context context, Item item, String value) throws SQLException { - itemService.addMetadata(context, item, VIRTUAL_METADATA_SCHEMA, VIRTUAL_METADATA_ELEMENT, - getVirtualQualifier(), null, value); - } - - private void addVirtualSourceField(Context context, Item item, MetadataValue sourceValue) throws SQLException { - if (StringUtils.isNotBlank(sourceValue.getAuthority())) { - addVirtualSourceField(context, item, sourceValue.getAuthority()); - } else { - addVirtualSourceField(context, item, PLACEHOLDER_PARENT_METADATA_VALUE); - } + private void addVirtualField(Context context, Item item, String value, String authority, String lang, + int confidence) throws SQLException { + itemService.addMetadata(context, item, VIRTUAL_METADATA_SCHEMA, VIRTUAL_METADATA_ELEMENT, getVirtualQualifier(), + lang, value, authority, confidence); } private void addVirtualSourceField(Context context, Item item, String sourceValueAuthority) throws SQLException { diff --git a/dspace-api/src/test/java/org/dspace/app/matcher/MetadataValueMatcher.java b/dspace-api/src/test/java/org/dspace/app/matcher/MetadataValueMatcher.java index 1439c9c37fa..55ceb779ba0 100644 --- a/dspace-api/src/test/java/org/dspace/app/matcher/MetadataValueMatcher.java +++ b/dspace-api/src/test/java/org/dspace/app/matcher/MetadataValueMatcher.java @@ -72,7 +72,8 @@ protected boolean matchesSafely(MetadataValue metadataValue) { Objects.equals(metadataValue.getMetadataField().toString('.'), field) && Objects.equals(metadataValue.getLanguage(), language) && Objects.equals(metadataValue.getAuthority(), authority) && - Objects.equals(metadataValue.getPlace(), place) && + (Objects.isNull(place) + || Objects.equals(metadataValue.getPlace(), place)) && Objects.equals(metadataValue.getConfidence(), confidence) && Objects.equals(metadataValue.getSecurityLevel(), securityLevel); } @@ -91,6 +92,10 @@ public static MetadataValueMatcher with(String field, String value) { return with(field, value, null, null, 0, -1); } + public static MetadataValueMatcher withNoPlace(String field, String value) { + return with(field, value, null, null, null, -1); + } + public static MetadataValueMatcher withSecurity(String field, String value, Integer securityLevel) { return with(field, value, null, null, 0, -1, securityLevel); } diff --git a/dspace-api/src/test/java/org/dspace/content/enhancer/consumer/ItemEnhancerConsumerIT.java b/dspace-api/src/test/java/org/dspace/content/enhancer/consumer/ItemEnhancerConsumerIT.java index 176f055a446..aa440574c07 100644 --- a/dspace-api/src/test/java/org/dspace/content/enhancer/consumer/ItemEnhancerConsumerIT.java +++ b/dspace-api/src/test/java/org/dspace/content/enhancer/consumer/ItemEnhancerConsumerIT.java @@ -8,6 +8,7 @@ package org.dspace.content.enhancer.consumer; import static org.dspace.app.matcher.MetadataValueMatcher.with; +import static org.dspace.app.matcher.MetadataValueMatcher.withNoPlace; import static org.dspace.core.CrisConstants.PLACEHOLDER_PARENT_METADATA_VALUE; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; @@ -141,31 +142,29 @@ public void testManyMetadataValuesEnhancement() throws Exception { publication = commitAndReload(publication); List values = publication.getMetadata(); - assertThat(values, hasSize(26)); + assertThat(values, hasSize(22)); assertThat(values, hasItem(with("dc.contributor.author", "Red Smith"))); assertThat(values, hasItem(with("dc.contributor.author", "Walter White", person1.getID().toString(), 1, 600))); assertThat(values, hasItem(with("dc.contributor.author", "John Smith", person2.getID().toString(), 2, 600))); assertThat(values, hasItem(with("dc.contributor.author", "Jesse Pinkman", person3.getID().toString(), 3, 600))); - assertThat(values, hasItem(with("cris.virtual.department", PLACEHOLDER_PARENT_METADATA_VALUE, 0))); - assertThat(values, hasItem(with("cris.virtualsource.department", PLACEHOLDER_PARENT_METADATA_VALUE, 0))); - assertThat(values, hasItem(with("cris.virtual.department", "4Science", 1))); - assertThat(values, hasItem(with("cris.virtualsource.department", person1.getID().toString(), 1))); - assertThat(values, hasItem(with("cris.virtual.department", PLACEHOLDER_PARENT_METADATA_VALUE, 2))); - assertThat(values, hasItem(with("cris.virtualsource.department", person2.getID().toString(), 2))); - assertThat(values, hasItem(with("cris.virtual.department", "University of Rome", 3))); - assertThat(values, hasItem(with("cris.virtualsource.department", person3.getID().toString(), 3))); + // virtual source and virtual metadata are not required to respect the order of the source metadata + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person1.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person2.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person3.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", PLACEHOLDER_PARENT_METADATA_VALUE))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "4Science"))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "University of Rome"))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person1.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person2.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person3.getID().toString()))); + // we can check with the position as all the values are expected to be placeholder assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 0))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 0))); assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person1.getID().toString(), 1))); assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 2))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person2.getID().toString(), 2))); - assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 3))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person3.getID().toString(), 3))); - assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(4)); - assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(4)); - assertThat(getMetadataValues(publication, "cris.virtual.author-orcid"), hasSize(4)); - assertThat(getMetadataValues(publication, "cris.virtualsource.author-orcid"), hasSize(4)); + assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(3)); + assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(3)); + assertThat(getMetadataValues(publication, "cris.virtual.author-orcid"), hasSize(3)); + assertThat(getMetadataValues(publication, "cris.virtualsource.author-orcid"), hasSize(3)); } @@ -246,22 +245,28 @@ public void testEnhancementWithMetadataRemoval() throws Exception { assertThat(values, hasItem(with("dc.contributor.author", "Walter White", person1.getID().toString(), 0, 600))); assertThat(values, hasItem(with("dc.contributor.author", "John Smith", person2.getID().toString(), 1, 600))); assertThat(values, hasItem(with("dc.contributor.author", "Jesse Pinkman", person3.getID().toString(), 2, 600))); - assertThat(values, hasItem(with("cris.virtual.department", "4Science"))); - assertThat(values, hasItem(with("cris.virtualsource.department", person1.getID().toString()))); - assertThat(values, hasItem(with("cris.virtual.department", "Company", 1))); - assertThat(values, hasItem(with("cris.virtualsource.department", person2.getID().toString(), 1))); - assertThat(values, hasItem(with("cris.virtual.department", "University of Rome", 2))); - assertThat(values, hasItem(with("cris.virtualsource.department", person3.getID().toString(), 2))); - assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person1.getID().toString()))); - assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person2.getID().toString(), 1))); - assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person3.getID().toString(), 2))); + // virtual source and virtual metadata are not required to respect the order of the source metadata + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person1.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person2.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person3.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "4Science"))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "Company"))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "University of Rome"))); assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(3)); assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(3)); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person1.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person2.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person3.getID().toString()))); + // we can check with the position as all the values are expected to be placeholder + assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 0))); + assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1))); + assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 2))); + assertThat(getMetadataValues(publication, "cris.virtual.author-orcid"), hasSize(3)); + assertThat(getMetadataValues(publication, "cris.virtualsource.author-orcid"), hasSize(3)); + + MetadataValue authorToRemove = getMetadataValues(publication, "dc.contributor.author").get(1); context.turnOffAuthorisationSystem(); @@ -274,14 +279,16 @@ public void testEnhancementWithMetadataRemoval() throws Exception { assertThat(values, hasSize(16)); assertThat(values, hasItem(with("dc.contributor.author", "Walter White", person1.getID().toString(), 0, 600))); assertThat(values, hasItem(with("dc.contributor.author", "Jesse Pinkman", person3.getID().toString(), 1, 600))); - assertThat(values, hasItem(with("cris.virtual.department", "4Science"))); - assertThat(values, hasItem(with("cris.virtualsource.department", person1.getID().toString()))); - assertThat(values, hasItem(with("cris.virtual.department", "University of Rome", 1))); - assertThat(values, hasItem(with("cris.virtualsource.department", person3.getID().toString(), 1))); + // virtual source and virtual metadata are not required to respect the order of the source metadata + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person1.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.department", person3.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "4Science"))); + assertThat(values, hasItem(withNoPlace("cris.virtual.department", "University of Rome"))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person1.getID().toString()))); + assertThat(values, hasItem(withNoPlace("cris.virtualsource.author-orcid", person3.getID().toString()))); + // we can check with the position as all the values are expected to be placeholder assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 0))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person1.getID().toString(), 0))); assertThat(values, hasItem(with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1))); - assertThat(values, hasItem(with("cris.virtualsource.author-orcid", person3.getID().toString(), 1))); assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(2)); assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(2)); assertThat(getMetadataValues(publication, "cris.virtual.author-orcid"), hasSize(2)); @@ -348,16 +355,10 @@ public void testEnhancementAfterItemUpdate() throws Exception { with("dc.contributor.author", "Gus Fring", 3))); assertThat(getMetadataValues(publication, "cris.virtual.author-orcid"), contains( - with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE), - with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1), - with("cris.virtual.author-orcid", "0000-0000-1111-2222", 2), - with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 3))); + with("cris.virtual.author-orcid", "0000-0000-1111-2222"))); assertThat(getMetadataValues(publication, "cris.virtualsource.author-orcid"), contains( - with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE), - with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1), - with("cris.virtualsource.author-orcid", personId, 2), - with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 3))); + with("cris.virtualsource.author-orcid", personId))); context.turnOffAuthorisationSystem(); itemService.addMetadata(context, publication, "dc", "title", "alternative", null, "Other name"); @@ -372,16 +373,10 @@ public void testEnhancementAfterItemUpdate() throws Exception { with("dc.contributor.author", "Gus Fring", 3))); assertThat(getMetadataValues(publication, "cris.virtual.author-orcid"), contains( - with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE), - with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1), - with("cris.virtual.author-orcid", "0000-0000-1111-2222", 2), - with("cris.virtual.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 3))); + with("cris.virtual.author-orcid", "0000-0000-1111-2222"))); - assertThat(getMetadataValues(publication, "cris.virtualsource.author-orcid"), contains( - with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE), - with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 1), - with("cris.virtualsource.author-orcid", personId, 2), - with("cris.virtualsource.author-orcid", PLACEHOLDER_PARENT_METADATA_VALUE, 3))); + assertThat(getMetadataValues(publication, "cris.virtualsource.author-orcid"), contains( + with("cris.virtualsource.author-orcid", personId))); } diff --git a/dspace-api/src/test/java/org/dspace/content/enhancer/script/ItemEnhancerScriptIT.java b/dspace-api/src/test/java/org/dspace/content/enhancer/script/ItemEnhancerScriptIT.java index 33913368b0a..f8b2b0d2d77 100644 --- a/dspace-api/src/test/java/org/dspace/content/enhancer/script/ItemEnhancerScriptIT.java +++ b/dspace-api/src/test/java/org/dspace/content/enhancer/script/ItemEnhancerScriptIT.java @@ -8,11 +8,13 @@ package org.dspace.content.enhancer.script; import static org.dspace.app.matcher.MetadataValueMatcher.with; +import static org.dspace.app.matcher.MetadataValueMatcher.withNoPlace; import static org.dspace.content.Item.ANY; import static org.dspace.content.enhancer.consumer.ItemEnhancerConsumer.ITEMENHANCER_ENABLED; import static org.dspace.core.CrisConstants.PLACEHOLDER_PARENT_METADATA_VALUE; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; @@ -171,11 +173,14 @@ public void testItemsEnhancement() throws Exception { assertThat(firstPublication.getMetadata(), hasItem(with("cris.virtual.department", "4Science"))); assertThat(firstPublication.getMetadata(), hasItem(with("cris.virtualsource.department", firstAuthorId))); - assertThat(secondPublication.getMetadata(), hasItem(with("cris.virtual.department", "4Science"))); - assertThat(secondPublication.getMetadata(), hasItem(with("cris.virtualsource.department", firstAuthorId))); - assertThat(secondPublication.getMetadata(), hasItem(with("cris.virtual.department", "Company", 1))); - assertThat(secondPublication.getMetadata(), hasItem(with("cris.virtualsource.department", secondAuthorId, 1))); - + assertThat(getMetadataValues(secondPublication, "cris.virtual.department"), + containsInAnyOrder( + withNoPlace("cris.virtual.department", "4Science"), + withNoPlace("cris.virtual.department", "Company"))); + assertThat(getMetadataValues(secondPublication, "cris.virtualsource.department"), + containsInAnyOrder( + withNoPlace("cris.virtualsource.department", firstAuthorId), + withNoPlace("cris.virtualsource.department", secondAuthorId))); assertThat(getMetadataValues(thirdPublication, "cris.virtual.department"), empty()); assertThat(getMetadataValues(thirdPublication, "cris.virtualsource.department"), empty()); @@ -223,10 +228,13 @@ public void testItemEnhancementWithoutForce() throws Exception { assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(2)); assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(2)); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "4Science"))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", firstAuthorId))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "Company", 1))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", secondAuthorId, 1))); + assertThat(getMetadataValues(publication, "cris.virtual.department"), containsInAnyOrder( + withNoPlace("cris.virtual.department", "4Science"), + withNoPlace("cris.virtual.department", "Company"))); + + assertThat(getMetadataValues(publication, "cris.virtualsource.department"), containsInAnyOrder( + withNoPlace("cris.virtualsource.department", firstAuthorId), + withNoPlace("cris.virtualsource.department", secondAuthorId))); context.turnOffAuthorisationSystem(); @@ -293,10 +301,12 @@ public void testItemEnhancementWithForce() throws Exception { assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(2)); assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(2)); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "4Science"))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", firstAuthorId))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "Company", 1))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", secondAuthorId, 1))); + assertThat(getMetadataValues(publication, "cris.virtual.department"), containsInAnyOrder( + withNoPlace("cris.virtual.department", "4Science"), + withNoPlace("cris.virtual.department", "Company"))); + assertThat(getMetadataValues(publication, "cris.virtualsource.department"), containsInAnyOrder( + withNoPlace("cris.virtualsource.department", firstAuthorId), + withNoPlace("cris.virtualsource.department", secondAuthorId))); context.turnOffAuthorisationSystem(); @@ -375,17 +385,16 @@ public void testItemEnhancementMetadataPositions() throws Exception { assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(4)); assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(4)); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", - PLACEHOLDER_PARENT_METADATA_VALUE, 0))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", firstAuthorId,0))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "4Science", 1))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", secondAuthorId,1))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "Company", 2))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", thirdAuthorId, 2))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", - PLACEHOLDER_PARENT_METADATA_VALUE, 3))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", fourthAuthorId,3))); - + assertThat(getMetadataValues(publication, "cris.virtual.department"), containsInAnyOrder( + withNoPlace("cris.virtual.department", PLACEHOLDER_PARENT_METADATA_VALUE), + withNoPlace("cris.virtual.department", "4Science"), + withNoPlace("cris.virtual.department", "Company"), + withNoPlace("cris.virtual.department", PLACEHOLDER_PARENT_METADATA_VALUE))); + assertThat(getMetadataValues(publication, "cris.virtualsource.department"), containsInAnyOrder( + withNoPlace("cris.virtualsource.department", firstAuthorId), + withNoPlace("cris.virtualsource.department", secondAuthorId), + withNoPlace("cris.virtualsource.department", thirdAuthorId), + withNoPlace("cris.virtualsource.department", fourthAuthorId))); } @Test @@ -420,15 +429,11 @@ public void testItemEnhancementSourceWithoutAuthority() throws Exception { publication = reload(publication); - assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(2)); - assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(2)); + assertThat(getMetadataValues(publication, "cris.virtual.department"), hasSize(1)); + assertThat(getMetadataValues(publication, "cris.virtualsource.department"), hasSize(1)); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", - PLACEHOLDER_PARENT_METADATA_VALUE, 0))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", - PLACEHOLDER_PARENT_METADATA_VALUE,0))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "4Science", 1))); - assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", secondAuthorId,1))); + assertThat(publication.getMetadata(), hasItem(with("cris.virtual.department", "4Science", 0))); + assertThat(publication.getMetadata(), hasItem(with("cris.virtualsource.department", secondAuthorId,0))); }