From 956617cee4831c745a50e9dadee3c3b33805c018 Mon Sep 17 00:00:00 2001 From: milanmajchrak <90026355+milanmajchrak@users.noreply.github.com> Date: Mon, 4 Sep 2023 09:18:27 +0200 Subject: [PATCH] ufal/be-wrong-citation-author-sequence (#417) * Created test and updated MetadataConverter.java * Fixed checkstyle issues. * Place is used only by the ItemService. * Refactored code. --- .../app/rest/converter/MetadataConverter.java | 33 ++++++- .../rest/ClarinItemImportControllerIT.java | 85 ++++++++++++++++++- 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/MetadataConverter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/MetadataConverter.java index 76aca4be231..ea7e6577c1d 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/MetadataConverter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/MetadataConverter.java @@ -26,6 +26,7 @@ import org.dspace.content.MetadataValue; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.DSpaceObjectService; +import org.dspace.content.service.ItemService; import org.dspace.core.Context; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; @@ -138,11 +139,39 @@ private void persistMetadataRest(Context context, T dso String element = seq[1]; String qualifier = seq.length == 3 ? seq[2] : null; for (MetadataValueRest mvr: entry.getValue()) { - dsoService.addMetadata(context, dso, schema, element, qualifier, mvr.getLanguage(), - mvr.getValue(), mvr.getAuthority(), mvr.getConfidence()); + addMetadataByService(dsoService, context, dso, schema, element, qualifier, mvr); } } dsoService.update(context, dso); } + /** + * Call `addMetadata` method by arbitrary dso service. ItemService calls `addMetadata` method with more + * arguments than other services. + * + * @param dsoService service with implemented `addMetadata` method + * @param context the context to use. + * @param dso the DSpace object. + * @param schema name of the metadata schema + * @param element name of the metadata element + * @param qualifier name of the metadata qualifier + * @param mvr MetadataValueRest object + * @throws SQLException if a database error occurs. + */ + private void addMetadataByService(DSpaceObjectService dsoService, Context context, + T dso, String schema, String element, String qualifier, + MetadataValueRest mvr) throws SQLException { + // Use `place` argument only for ItemService because some services doesn't have implemented + // `addMetadata` method with `place` argument. + // `place` is important because of importing Items. Without it the Items metadata could be in the + // wrong sequence e.g., authors of the item. + if (dsoService instanceof ItemService) { + dsoService.addMetadata(context, dso, schema, element, qualifier, mvr.getLanguage(), + mvr.getValue(), mvr.getAuthority(), mvr.getConfidence(), mvr.getPlace()); + } else { + dsoService.addMetadata(context, dso, schema, element, qualifier, mvr.getLanguage(), + mvr.getValue(), mvr.getAuthority(), mvr.getConfidence()); + } + } + } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinItemImportControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinItemImportControllerIT.java index 9872bb8dd03..8e1ad2a697e 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinItemImportControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinItemImportControllerIT.java @@ -18,6 +18,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.util.List; +import java.util.Objects; import java.util.UUID; import javax.ws.rs.core.MediaType; @@ -327,4 +328,86 @@ public void importArchivedItemTest() throws Exception { ItemBuilder.deleteItem(uuid); context.restoreAuthSystemState(); } -} \ No newline at end of file + + // Fix of this issue: https://github.com/dataquest-dev/DSpace/issues/409 + // Authors sequence was changed after a data import. + @Test + public void importItemWithUnsortedAuthors() throws Exception { + String FIRST_AUTHOR_VALUE = "First author"; + int FIRST_AUTHOR_PLACE = 1; + String SECOND_AUTHOR_VALUE = "Second author"; + int SECOND_AUTHOR_PLACE = 0; + + context.turnOffAuthorisationSystem(); + ObjectNode node = jsonNodeFactory.objectNode(); + node.set("withdrawn", jsonNodeFactory.textNode("false")); + node.set("inArchive", jsonNodeFactory.textNode("false")); + node.set("discoverable", jsonNodeFactory.textNode("false")); + + // Metadata which should be kept after installing the new Item. + ObjectNode metadataNode = jsonNodeFactory.objectNode(); + + // `dc.contributor.author` metadata added into `metadata` of the ItemRest object + ObjectNode firstAuthorMetadataNode = jsonNodeFactory.objectNode(); + firstAuthorMetadataNode.set("value", jsonNodeFactory.textNode(FIRST_AUTHOR_VALUE)); + firstAuthorMetadataNode.set("language", jsonNodeFactory.textNode("en_US")); + firstAuthorMetadataNode.set("authority", jsonNodeFactory.nullNode()); + firstAuthorMetadataNode.set("confidence", jsonNodeFactory.numberNode(-1)); + firstAuthorMetadataNode.set("place", jsonNodeFactory.numberNode(FIRST_AUTHOR_PLACE)); + + // `dc.contributor.author` metadata added into `metadata` of the ItemRest object + ObjectNode secondAuthorMetadataNode = jsonNodeFactory.objectNode(); + secondAuthorMetadataNode.set("value", jsonNodeFactory.textNode(SECOND_AUTHOR_VALUE)); + secondAuthorMetadataNode.set("language", jsonNodeFactory.textNode("en_US")); + secondAuthorMetadataNode.set("authority", jsonNodeFactory.nullNode()); + secondAuthorMetadataNode.set("confidence", jsonNodeFactory.numberNode(-1)); + secondAuthorMetadataNode.set("place", jsonNodeFactory.numberNode(SECOND_AUTHOR_PLACE)); + metadataNode.set("dc.contributor.author", jsonNodeFactory.arrayNode() + .add(firstAuthorMetadataNode) + .add(secondAuthorMetadataNode)); + + node.set("metadata", metadataNode); + context.restoreAuthSystemState(); + + ObjectMapper mapper = new ObjectMapper(); + String token = getAuthToken(admin.getEmail(), password); + + UUID uuid = UUID.fromString(read(getClient(token).perform(post("/api/clarin/import/item") + .content(mapper.writeValueAsBytes(node)) + .contentType(org.springframework.http.MediaType.APPLICATION_JSON) + .param("owningCollection", col.getID().toString()) + .param("epersonUUID", submitter.getID().toString())) + .andExpect(status().isOk()).andReturn().getResponse().getContentAsString(), + "$.id")); + + // workspaceitem should nt exist + List workflowItems = workspaceItemService.findAll(context); + assertEquals(workflowItems.size(), 0); + // contoling of the created item + Item item = itemService.find(context, uuid); + assertFalse(item.isWithdrawn()); + assertFalse(item.isArchived()); + assertFalse(item.isDiscoverable()); + assertEquals(item.getSubmitter().getID(), submitter.getID()); + assertEquals(item.getOwningCollection().getID(), col.getID()); + + // get all `dc.contributor.author`metadata values + List authorValues = + itemService.getMetadata(item, "dc", "contributor", "author", "en_US"); + assertEquals(authorValues.size(), 2); + + int indexFirstAuthor = Objects.equals(authorValues.get(0).getValue(), FIRST_AUTHOR_VALUE) ? 0 : 1; + int indexSecondAuthor = Objects.equals(indexFirstAuthor, 0) ? 1 : 0; + // first metadata value should be FIRST_AUTHOR with place 1 + assertEquals(authorValues.get(indexFirstAuthor).getValue(), FIRST_AUTHOR_VALUE); + assertEquals(authorValues.get(indexFirstAuthor).getPlace(), FIRST_AUTHOR_PLACE); + // second metadata value should be SECOND_AUTHOR with place 0 + assertEquals(authorValues.get(indexSecondAuthor).getValue(), SECOND_AUTHOR_VALUE); + assertEquals(authorValues.get(indexSecondAuthor).getPlace(), SECOND_AUTHOR_PLACE); + + // clean all + context.turnOffAuthorisationSystem(); + ItemBuilder.deleteItem(uuid); + context.restoreAuthSystemState(); + } +}