Merge pull request #798 from antoine-atmire/DS-2330

DS-2330 bug fix: lost authors when using ORCID column

dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSV.java

@@ -7,6 +7,7 @@
  */
 package org.dspace.app.bulkedit;
 
+import org.apache.commons.lang3.StringUtils;
 import org.dspace.authority.AuthorityValue;
 import org.dspace.app.bulkedit.DSpaceCSVLine;
 import org.dspace.app.bulkedit.MetadataImport;
@@ -14,6 +15,7 @@ import org.dspace.app.bulkedit.MetadataImportInvalidHeadingException;
 import org.dspace.content.Collection;
 import org.dspace.content.*;
 import org.dspace.content.Collection;
+import org.dspace.content.authority.Choices;
 import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
 
@@ -185,7 +187,7 @@ public class DSpaceCSV implements Serializable
             StringBuilder lineBuilder = new StringBuilder();
             String lineRead;
 
-            while ((lineRead = input.readLine()) != null)
+            while (StringUtils.isNotBlank(lineRead = input.readLine()))
             {
                 if (lineBuilder.length() > 0) {
                     // Already have a previously read value - add this line
@@ -449,7 +451,7 @@ public class DSpaceCSV implements Serializable
                 String mdValue = value.value;
                 if (value.authority != null && !"".equals(value.authority))
                 {
-                    mdValue += authoritySeparator + value.authority + authoritySeparator + value.confidence;
+                    mdValue += authoritySeparator + value.authority + authoritySeparator +  (value.confidence != -1 ? value.confidence : Choices.CF_ACCEPTED);
                 }
                 line.add(key, mdValue);
                 if (!headings.contains(key))
@@ -610,8 +612,9 @@ public class DSpaceCSV implements Serializable
         // Create the headings line
         String[] csvLines = new String[counter + 1];
         csvLines[0] = "id" + fieldSeparator + "collection";
-        Collections.sort(headings);
+        List<String> headingsCopy = new ArrayList<String>(headings);
-        for (String value : headings)
+        Collections.sort(headingsCopy);
+        for (String value : headingsCopy)
         {
             csvLines[0] = csvLines[0] + fieldSeparator + value;
         }
@@ -620,7 +623,7 @@ public class DSpaceCSV implements Serializable
         int c = 1;
         while (i.hasNext())
         {
-            csvLines[c++] = i.next().toCSV(headings);
+            csvLines[c++] = i.next().toCSV(headingsCopy);
         }
 
         return csvLines;

dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSVLine.java

@@ -7,12 +7,10 @@
  */
 package org.dspace.app.bulkedit;
 
+import org.dspace.authority.AuthorityValue;
+
 import java.io.Serializable;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 /**
  * Utility class to store a line from a CSV file
@@ -21,12 +19,35 @@ import java.util.Set;
  */
 public class DSpaceCSVLine implements Serializable
 {
+
     /** The item id of the item represented by this line. -1 is for a new item */
     private int id;
 
     /** The elements in this line in a hashtable, keyed by the metadata type */
     private Map<String, ArrayList> items;
 
+    /** ensuring that the order-sensible columns of the csv are processed in the correct order */
+    private final Comparator<? super String> headerComparator = new Comparator<String>() {
+        @Override
+        public int compare(String md1, String md2) {
+            // The metadata coming from an external source should be processed after the others
+            AuthorityValue source1 = MetadataImport.getAuthorityValueType(md1);
+            AuthorityValue source2 = MetadataImport.getAuthorityValueType(md2);
+
+            int compare;
+            if (source1 == null && source2 != null) {
+                compare = -1;
+            }
+            else if (source1 != null && source2 == null) {
+                compare = 1;
+            } else {
+                // the order of the rest does not matter
+                compare = md1.compareTo(md2);
+            }
+            return compare;
+        }
+    };
+
     /**
      * Create a new CSV line
      *
@@ -36,7 +57,8 @@ public class DSpaceCSVLine implements Serializable
     {
         // Store the ID + separator, and initialise the hashtable
         this.id = itemId;
-        items = new HashMap<String, ArrayList>();
+        items = new TreeMap<String, ArrayList>(headerComparator);
+//        this.items = new HashMap<String, ArrayList>();
     }
 
     /**
@@ -46,7 +68,8 @@ public class DSpaceCSVLine implements Serializable
     {
         // Set the ID to be -1, and initialise the hashtable
         this.id = -1;
-        this.items = new HashMap<String, ArrayList>();
+        this.items = new TreeMap<String, ArrayList>(headerComparator);
+//        this.items = new HashMap<String, ArrayList>();
     }
 
     /**

dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImport.java

@@ -166,7 +166,7 @@ public class MetadataImport
                             }
 
                             // Compare
-                            compare(item, fromCSV, change, md, whatHasChanged);
+                            compare(item, fromCSV, change, md, whatHasChanged, line);
                         }
                     }
 
@@ -412,11 +412,12 @@ public class MetadataImport
      * @param md The element to compare
      * @param changes The changes object to populate
      *
+     * @param line
      * @throws SQLException if there is a problem accessing a Collection from the database, from its handle
      * @throws AuthorizeException if there is an authorization problem with permissions
      */
     private void compare(Item item, String[] fromCSV, boolean change,
-                         String md, BulkEditChange changes) throws SQLException, AuthorizeException
+                         String md, BulkEditChange changes, DSpaceCSVLine line) throws SQLException, AuthorizeException
     {
         // Log what metadata element we're looking at
         String all = "";
@@ -473,18 +474,15 @@ public class MetadataImport
                                        ",looking_for_element=" + element +
                                        ",looking_for_qualifier=" + qualifier +
                                        ",looking_for_language=" + language));
+        String[] dcvalues = new String[0];
+        if(fromAuthority==null) {
             Metadatum[] current = item.getMetadata(schema, element, qualifier, language);
-        
+            dcvalues = new String[current.length];
-        String[] dcvalues = new String[current.length];
             int i = 0;
-        for (Metadatum dcv : current)
+            for (Metadatum dcv : current) {
-        {
+                if (dcv.authority == null || !isAuthorityControlledField(md)) {
-            if (dcv.authority == null || !isAuthorityControlledField(md))
-            {
                     dcvalues[i] = dcv.value;
-            }
+                } else {
-            else
-            {
                     dcvalues[i] = dcv.value + DSpaceCSV.authoritySeparator + dcv.authority;
                     dcvalues[i] += DSpaceCSV.authoritySeparator + (dcv.confidence != -1 ? dcv.confidence : Choices.CF_ACCEPTED);
                 }
@@ -493,6 +491,9 @@ public class MetadataImport
                         "item_id=" + item.getID() + ",fromCSV=" + all +
                                 ",found=" + dcv.value));
             }
+        }else{
+            dcvalues = line.get(md).toArray(new String[line.get(md).size()]);
+        }
 
         // Compare from current->csv
         for (int v = 0; v < fromCSV.length; v++) {
@@ -530,7 +531,9 @@ public class MetadataImport
                 dcv.confidence = (parts.length > 2 ? Integer.valueOf(parts[2]) : Choices.CF_ACCEPTED);
             }
 
-            if ((value != null) && (!"".equals(value)) && (!contains(value, fromCSV)))
+            if ((value != null) && (!"".equals(value)) && (!contains(value, fromCSV)) && fromAuthority==null)
+            // fromAuthority==null: with the current implementation metadata values from external authority sources can only be used to add metadata, not to change or remove them
+            // because e.g. an author that is not in the column "ORCID:dc.contributor.author" could still be in the column "dc.contributor.author" so don't remove it
             {
                 // Remove it
                 log.debug(LogManager.getHeader(c, "metadata_import",
@@ -862,7 +865,8 @@ public class MetadataImport
             }
 
             // look up the value and authority in solr
-            List<AuthorityValue> byValue = authorityValueFinder.findByValue(c, schema, element, qualifier, value);
+            AuthorityValue example = fromAuthority.newInstance(value);
+            List<AuthorityValue> byValue = authorityValueFinder.findByValue(c, schema, element, qualifier, example.getValue());
             AuthorityValue authorityValue = null;
             if (byValue.isEmpty()) {
                 String toGenerate = fromAuthority.generateString() + value;
@@ -1167,6 +1171,8 @@ public class MetadataImport
     {
         int pos = md.indexOf("[");
         String mdf = (pos > -1 ? md.substring(0, pos) : md);
+        pos = md.indexOf(":");
+        mdf = (pos > -1 ? md.substring(pos+1) : md);
         return authorityControlled.contains(mdf);
     }