GND_8: Update AuthorityValue and GNDAuthorityValue

Improve AuthorityValue:
- Sets labels and otherMetadataMap
- Doesn't require field to be equal when
  comparing 'hasSameInformationAs'
- reordered some code and added javadoc
- took out 'item' param from updateItem

GND_8: Update AuthorityValue and GNDAuthorityValue

GNDAuthorityValue was not init'ing properly.
In fact, it is not required for normal
GND usage, but it does let us
do things like last mod comparison

GND_8: Authority Value models

Author: kshepherd

dspace-api/src/main/java/org/dspace/authority/AuthorityValue.java

@@ -27,7 +27,6 @@
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrInputDocument;
 import org.dspace.authorize.AuthorizeException;
-import org.dspace.content.Item;
 import org.dspace.content.MetadataValue;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.core.Context;
@@ -43,34 +42,39 @@ public class AuthorityValue {
     /**
      * The id of the record in solr
      */
-    private String id;
+    protected String id;
 
     /**
      * The metadata field that this authority value is for
      */
-    private String field;
+    protected String field;
 
     /**
      * The text value of this authority
      */
-    private String value;
+    protected String value;
 
     /**
      * When this authority record has been created
      */
-    private Date creationDate;
+    protected Date creationDate;
 
     /**
      * If this authority has been removed
      */
-    private boolean deleted;
+    protected boolean deleted;
 
     /**
      * represents the last time that DSpace got updated information from its external source
      */
-    private Date lastModified;
+    protected Date lastModified;
 
-    private Map<String, List<String>> otherMetadataMap;
+    protected Map<String, List<String>> otherMetadataMap;
+
+    /**
+     * log4j logger
+     */
+    private static final Logger log = LogManager.getLogger();
 
     public AuthorityValue() {
         this.otherMetadataMap = new LinkedHashMap<>();
@@ -150,7 +154,8 @@ public void delete() {
     }
 
     /**
-     * Generate a solr record from this instance
+     * Generate a Solr record from this instance. The otherMetadataMap values will be iterated
+     * and added with a label_ prefix
      *
      * @return SolrInputDocument
      */
@@ -176,7 +181,9 @@ public SolrInputDocument getSolrInputDocument() {
     }
 
     /**
-     * Initialize this instance based on a solr record
+     * Set values for this AuthorityValue based on a Solr record.
+     * Any supplementary information saved with the label_ prefix will be added to the
+     * otherMetadataMap with the label_ prefix stripped.
      *
      * @param document SolrDocument
      */
@@ -190,34 +197,33 @@ public void setValues(SolrDocument document) {
         // Set other metadata
         for (String key : document.getFieldNames()) {
             if (key.startsWith("label_")) {
-                String gndKey = key.replace("label_", "");
+                String otherMetadataKey = key.replace("label_", "");
                 Collection<Object> values = document.getFieldValues(key);
                 if (values != null) {
                     Collection<String> stringValues = (Collection<String>) (Object) values;
-                    otherMetadataMap.put(gndKey, new ArrayList<>(stringValues));
+                    otherMetadataMap.put(otherMetadataKey, new ArrayList<>(stringValues));
                 }
             }
         }
     }
 
     /**
-     * Replace an item's DCValue with this authority
+     * Replace an item's DCValue with this authority value and key
      *
      * @param context     context
      * @param value       metadata value
-     * @param currentItem item
      * @throws SQLException       if database error
      * @throws AuthorizeException if authorization error
      */
-    public void updateItem(Context context, Item currentItem, MetadataValue value)
+    public void updateItem(Context context, MetadataValue value)
         throws SQLException, AuthorizeException {
         value.setValue(getValue());
         value.setAuthority(getId());
         ContentServiceFactory.getInstance().getMetadataValueService().update(context, value, true);
     }
 
     /**
-     * Information that can be used the choice ui.
+     * Information that can be used with the ChoiceAuthority UI
      *
      * @return map
      */
@@ -229,7 +235,7 @@ public Map<String, String> choiceSelectMap() {
      * Build a list of ISO date formatters to parse various forms.
      *
      * <p><strong>Note:</strong>  any formatter which does not parse a zone or
-     * offset must have a default zone set.  See {@link stringToDate}.
+     * offset must have a default zone set.  See {@link #stringToDate}.
      *
      * @return the formatters.
      */
@@ -267,20 +273,39 @@ static public Date stringToDate(String date) {
     }
 
     /**
-     * log4j logger
+     * Set a key-value pair in the otherMetadataMap.
+     *
+     * @param key   the key for the metadata entry
+     * @param value the value for the metadata entry
      */
-    private static Logger log = LogManager.getLogger();
+    public void setMetadataMapEntry(String key, String value) {
+        List<String> values;
+        if (!this.otherMetadataMap.containsKey(key)) {
+            values = new ArrayList<>();
+        } else {
+            values = this.otherMetadataMap.get(key);
+        }
+        values.add(value);
+        this.otherMetadataMap.put(key, values);
+    }
 
-    @Override
-    public String toString() {
-        return "AuthorityValue{" +
-            "id='" + id + '\'' +
-            ", field='" + field + '\'' +
-            ", value='" + value + '\'' +
-            ", creationDate=" + creationDate +
-            ", deleted=" + deleted +
-            ", lastModified=" + lastModified +
-            '}';
+    /**
+     * Retrieves the other metadata map of the AuthorityValue object.
+     * In the solr document this is in the form of label_{dc.example.field}
+     *
+     * @return the other metadata map of the AuthorityValue object
+     */
+    public Map<String, List<String>> getOtherMetadataMap() {
+        return otherMetadataMap;
+    }
+
+    /**
+     * Set the other metadata map
+     *
+     * @param otherMetadataMap the map of fields and values
+     */
+    public void setOtherMetadataMap(Map<String, List<String>> otherMetadataMap) {
+        this.otherMetadataMap = otherMetadataMap;
     }
 
     /**
@@ -335,9 +360,6 @@ public boolean hasTheSameInformationAs(Object o) {
         if (deleted != that.deleted) {
             return false;
         }
-        if (field != null ? !field.equals(that.field) : that.field != null) {
-            return false;
-        }
         if (id != null ? !id.equals(that.id) : that.id != null) {
             return false;
         }
@@ -348,22 +370,15 @@ public boolean hasTheSameInformationAs(Object o) {
         return true;
     }
 
-    public void setMetadataMapEntry(String key, String value) {
-        List<String> values;
-        if (!this.otherMetadataMap.containsKey(key)) {
-            values = new ArrayList<>();
-        } else {
-            values = this.otherMetadataMap.get(key);
-        }
-        values.add(value);
-        this.otherMetadataMap.put(key, values);
-    }
-
-    public Map<String, List<String>> getOtherMetadataMap() {
-        return otherMetadataMap;
-    }
-
-    public void setOtherMetadataMap(Map<String, List<String>> otherMetadataMap) {
-        this.otherMetadataMap = otherMetadataMap;
+    @Override
+    public String toString() {
+        return "AuthorityValue{" +
+                "id='" + id + '\'' +
+                ", field='" + field + '\'' +
+                ", value='" + value + '\'' +
+                ", creationDate=" + creationDate +
+                ", deleted=" + deleted +
+                ", lastModified=" + lastModified +
+                '}';
     }
 }

dspace-api/src/main/java/org/dspace/authority/GNDAuthorityValue.java

@@ -7,21 +7,10 @@
  */
 package org.dspace.authority;
 
-import java.sql.SQLException;
-import java.util.ArrayList;
 import java.util.Date;
-import java.util.List;
-import java.util.Map;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.apache.solr.common.SolrDocument;
-import org.apache.solr.common.SolrInputDocument;
-import org.dspace.authorize.AuthorizeException;
-import org.dspace.content.Item;
-import org.dspace.content.MetadataValue;
-import org.dspace.content.factory.ContentServiceFactory;
-import org.dspace.core.Context;
 
 /**
  * GND Authority Value is a simple extension of authority value that allows arbitrary
@@ -30,11 +19,12 @@
  * @author Kim Shepherd
  */
 public class GNDAuthorityValue extends AuthorityValue {
+    // Logger
+    private static final Logger log = LogManager.getLogger();
 
-    private Map<String, List<String>> otherMetadataMap;
+    public GNDAuthorityValue() {
 
-    // Logger
-    private static final Logger log = LogManager.getLogger(GNDAuthorityValue.class);
+    }
 
     @Override
     public String getAuthorityType() {
@@ -43,88 +33,18 @@ public String getAuthorityType() {
 
     @Override
     public Date getLastModified() {
-        if (this.otherMetadataMap.containsKey("gnd.date.modified")
-                && this.otherMetadataMap.get("gnd.date.modified").size() > 0) {
-            return stringToDate(this.otherMetadataMap.get("gnd.date.modified").get(0));
+        if (otherMetadataMap.containsKey("gnd.date.modified")
+                && otherMetadataMap.get("gnd.date.modified").size() > 0) {
+            return stringToDate(otherMetadataMap.get("gnd.date.modified").get(0));
         }
         return super.getLastModified();
     }
 
-    public void setMetadataMapEntry(String key, String value) {
-        List<String> values;
-        if (!this.otherMetadataMap.containsKey(key)) {
-            values = new ArrayList<>();
-        } else {
-            values = this.otherMetadataMap.get(key);
-        }
-        values.add(value);
-        this.otherMetadataMap.put(key, values);
-    }
-
-    public Map<String, List<String>> getOtherMetadataMap() {
-        return otherMetadataMap;
-    }
-
-    public void setOtherMetadataMap(Map<String, List<String>> otherMetadataMap) {
-        this.otherMetadataMap = otherMetadataMap;
-    }
-
-    /**
-     * Generate a solr record from this instance
-     * @return SolrInputDocument
-     */
-    @Override
-    public SolrInputDocument getSolrInputDocument() {
-        SolrInputDocument doc = super.getSolrInputDocument();
-
-        // Other metadata - set dynamic fields for each key
-//        for (String metadataKey : otherMetadataMap.keySet()) {
-//            List<String> metadata = otherMetadataMap.get(metadataKey);
-//            for (String value : metadata) {
-//                doc.addField("label_" + metadataKey, value);
-//            }
-//        }
-
-        return doc;
-    }
-
-    /**
-     * Set the values of this AuthorityValue based on the existing Solr record
-     *
-     * @param document SolrDocument
-     */
-    @Override
-    public void setValues(SolrDocument document) {
-        super.setValues(document);
-        // Set other metadata
-//        for (String key : document.getFieldNames()) {
-//            if (key.startsWith("label_")) {
-//                String gndKey = key.replace("label_", "");
-//                Collection<Object> values = document.getFieldValues(key);
-//                if (values != null) {
-//                    Collection<String> stringValues = (Collection<String>) (Object) values;
-//                    otherMetadataMap.put(gndKey, new ArrayList<>(stringValues));
-//                }
-//            }
-//        }
-    }
-
-    /**
-     * Replace an item's DCValue with this authority
-     * Unlike base authority, we are not going to actually update the id at this stage. Or are we?
-     *
-     * @param context     context
-     * @param value       metadata value
-     * @param currentItem item
-     * @throws SQLException       if database error
-     * @throws AuthorizeException if authorization error
-     */
     @Override
-    public void updateItem(Context context, Item currentItem, MetadataValue value)
-            throws SQLException, AuthorizeException {
-        value.setValue(getValue());
-        value.setAuthority(getId());
-        ContentServiceFactory.getInstance().getMetadataValueService().update(context, value, true);
+    public AuthorityValue newInstance(String info) {
+        GNDAuthorityValue authorityValue = new GNDAuthorityValue();
+        authorityValue.setValue(info);
+        return authorityValue;
     }
 
 }

dspace-api/src/main/java/org/dspace/authority/UpdateAuthorities.java

@@ -168,7 +168,7 @@ protected void updateItems(AuthorityValue authority) {
             while (itemIterator.hasNext()) {
                 Item next = itemIterator.next();
                 List<MetadataValue> metadata = itemService.getMetadata(next, authority.getField(), authority.getId());
-                authority.updateItem(context, next, metadata.get(0)); //should be only one
+                authority.updateItem(context, metadata.get(0)); //should be only one
                 List<MetadataValue> metadataAfter = itemService
                     .getMetadata(next, authority.getField(), authority.getId());
                 if (!metadata.get(0).getValue().equals(metadataAfter.get(0).getValue())) {

dspace-api/src/main/java/org/dspace/authority/indexer/DSpaceAuthorityIndexer.java

@@ -107,7 +107,7 @@ public List<AuthorityValue> getAuthorityValues(Context context, Item item, Map<S
                 }
                 if (value != null) {
                     if (requiresItemUpdate) {
-                        value.updateItem(context, item, metadataValue);
+                        value.updateItem(context, metadataValue);
                         try {
                             itemService.update(context, item);
                         } catch (Exception e) {