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.

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 <T extends DSpaceObject> 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 <T extends DSpaceObject> void addMetadataByService(DSpaceObjectService<T> 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());
+        }
+    }
+
 }

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();
     }
-}
+
+    // 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<WorkspaceItem> 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<MetadataValue> 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();
+    }
+}