[CST-4039] Patch Add entire array with virtual values does not work

dspace-api/src/main/java/org/dspace/content/DSpaceObjectServiceImpl.java

@@ -17,8 +17,10 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.StringTokenizer;
+import java.util.function.Supplier;
 
 import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.lang.NotImplementedException;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
@@ -238,6 +240,21 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
     public List<MetadataValue> addMetadata(Context context, T dso, MetadataField metadataField, String lang,
                                            List<String> values, List<String> authorities, List<Integer> confidences)
         throws SQLException {
+
+        //Set place to list length of all metadatavalues for the given schema.element.qualifier combination.
+        // Subtract one to adhere to the 0 as first element rule
+        final Supplier<Integer> placeSupplier =  () ->
+            this.getMetadata(dso, metadataField.getMetadataSchema().getName(), metadataField.getElement(),
+                metadataField.getQualifier(), Item.ANY).size() - 1;
+
+        return addMetadata(context, dso, metadataField, lang, values, authorities, confidences, placeSupplier);
+
+    }
+
+    public List<MetadataValue> addMetadata(Context context, T dso, MetadataField metadataField, String lang,
+            List<String> values, List<String> authorities, List<Integer> confidences, Supplier<Integer> placeSupplier)
+                    throws SQLException {
+
         boolean authorityControlled = metadataAuthorityService.isAuthorityControlled(metadataField);
         boolean authorityRequired = metadataAuthorityService.isAuthorityRequired(metadataField);
         List<MetadataValue> newMetadata = new ArrayList<>(values.size());
@@ -252,11 +269,8 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
             }
             MetadataValue metadataValue = metadataValueService.create(context, dso, metadataField);
             newMetadata.add(metadataValue);
-            //Set place to list length of all metadatavalues for the given schema.element.qualifier combination.
+
-            // Subtract one to adhere to the 0 as first element rule
+            metadataValue.setPlace(placeSupplier.get());
-            metadataValue.setPlace(
-                this.getMetadata(dso, metadataField.getMetadataSchema().getName(), metadataField.getElement(),
-                                 metadataField.getQualifier(), Item.ANY).size() - 1);
 
             metadataValue.setLanguage(lang == null ? null : lang.trim());
 
@@ -359,7 +373,7 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
     public MetadataValue addMetadata(Context context, T dso, String schema, String element, String qualifier,
                             String lang, String value, String authority, int confidence) throws SQLException {
         return addMetadata(context, dso, schema, element, qualifier, lang, Arrays.asList(value),
-                Arrays.asList(authority), Arrays.asList(confidence)).get(0);
+                Arrays.asList(authority), Arrays.asList(confidence)).stream().findFirst().orElse(null);
     }
 
     @Override
@@ -805,4 +819,12 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
         dso.setMetadataModified();
     }
 
+    @Override
+    public MetadataValue addMetadata(Context context, T dso, String schema, String element, String qualifier,
+            String lang, String value, String authority, int confidence, int place) throws SQLException {
+
+        throw new NotImplementedException();
+
+    }
+
 }

dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java

@@ -11,12 +11,14 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Comparator;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -1361,7 +1363,7 @@ prevent the generation of resource policy entry values with null dspace_object a
      * Supports moving metadata by adding the metadata value or updating the place of the relationship
      */
     @Override
-    protected void moveSingleMetadataValue(Context context, Item dso, int place, MetadataValue rr) {
+    public void moveSingleMetadataValue(Context context, Item dso, int place, MetadataValue rr) {
         if (rr instanceof RelationshipMetadataValue) {
             try {
                 //Retrieve the applicable relationship
@@ -1405,5 +1407,25 @@ prevent the generation of resource policy entry values with null dspace_object a
         return listToReturn;
     }
 
+    @Override
+    public MetadataValue addMetadata(Context context, Item dso, String schema, String element, String qualifier,
+            String lang, String value, String authority, int confidence, int place) throws SQLException {
+
+        // We will not verify that they are valid entries in the registry
+        // until update() is called.
+        MetadataField metadataField = metadataFieldService.findByElement(context, schema, element, qualifier);
+        if (metadataField == null) {
+            throw new SQLException(
+                "bad_dublin_core schema=" + schema + "." + element + "." + qualifier + ". Metadata field does not " +
+                "exist!");
+        }
+
+        final Supplier<Integer> placeSupplier =  () -> place;
+
+        return addMetadata(context, dso, metadataField, lang, Arrays.asList(value),
+                Arrays.asList(authority), Arrays.asList(confidence), placeSupplier)
+                .stream().findFirst().orElse(null);
+    }
+
 
 }

dspace-api/src/main/java/org/dspace/content/service/DSpaceObjectService.java

@@ -367,6 +367,30 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
     public MetadataValue addMetadata(Context context, T dso, String schema, String element, String qualifier,
                              String lang, String value) throws SQLException;
 
+    /**
+     * Add a single metadata field at the given place position.
+     *
+     * @param context    DSpace context
+     * @param dso        DSpaceObject
+     * @param schema     the schema for the metadata field. <em>Must</em> match
+     *                   the <code>name</code> of an existing metadata schema.
+     * @param element    the metadata element name
+     * @param qualifier  the metadata qualifier, or <code>null</code> for
+     *                   unqualified
+     * @param lang       the ISO639 language code, optionally followed by an underscore
+     *                   and the ISO3166 country code. <code>null</code> means the
+     *                   value has no language (for example, a date).
+     * @param value      the value to add.
+     * @param authority  the external authority key for this value (or null)
+     * @param confidence the authority confidence (default 0)
+     * @param place      the metadata position
+     * @return the MetadataValue added ot the object
+     * @throws SQLException if database error
+     */
+    public MetadataValue addMetadata(Context context, T dso, String schema, String element, String qualifier,
+                           String lang, String value, String authority, int confidence, int place) throws SQLException;
+
+
     /**
      * Add a single metadata field. This is appended to existing
      * values. Use <code>clearMetadata</code> to remove values.

dspace-api/src/main/java/org/dspace/content/service/ItemService.java

@@ -741,5 +741,13 @@ public interface ItemService
     public List<MetadataValue> getMetadata(Item item, String schema, String element, String qualifier,
                                            String lang, boolean enableVirtualMetadata);
 
+    /**
+     * 
+     * @param context DSpace context object
+     * @param item Item
+     * @param place new metadata position
+     * @param rr MetadataValue to move
+     */
+    public void moveSingleMetadataValue(Context context, Item item, int place, MetadataValue rr);
 
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/submit/factory/impl/ItemMetadataValueAddPatchOperation.java

@@ -8,15 +8,25 @@
 package org.dspace.app.rest.submit.factory.impl;
 
 import java.sql.SQLException;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 
+import org.apache.commons.lang3.StringUtils;
 import org.dspace.app.rest.model.MetadataValueRest;
 import org.dspace.app.rest.model.patch.LateObjectEvaluator;
+import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.InProgressSubmission;
 import org.dspace.content.Item;
 import org.dspace.content.MetadataValue;
+import org.dspace.content.Relationship;
+import org.dspace.content.RelationshipMetadataValue;
 import org.dspace.content.service.ItemService;
+import org.dspace.content.service.RelationshipService;
+import org.dspace.core.Constants;
 import org.dspace.core.Context;
+import org.dspace.core.Utils;
 import org.dspace.services.model.Request;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.util.Assert;
@@ -69,9 +79,12 @@ public class ItemMetadataValueAddPatchOperation extends MetadataValueAddPatchOpe
     @Autowired
     ItemService itemService;
 
+    @Autowired
+    RelationshipService relationshipService;
+
     @Override
     void add(Context context, Request currentRequest, InProgressSubmission source, String path, Object value)
-        throws SQLException {
+            throws SQLException {
         String[] split = getAbsolutePath(path).split("/");
         // if split size is one so we have a call to initialize or replace
         if (split.length == 1) {
@@ -84,7 +97,7 @@ public class ItemMetadataValueAddPatchOperation extends MetadataValueAddPatchOpe
             MetadataValueRest object = evaluateSingleObject((LateObjectEvaluator) value);
             // check if is not empty
             List<MetadataValue> metadataByMetadataString = itemService.getMetadataByMetadataString(source.getItem(),
-                                                                                                   split[0]);
+                    split[0]);
             Assert.notEmpty(metadataByMetadataString);
             if (split.length > 1) {
                 String controlChar = split[1];
@@ -98,7 +111,7 @@ public class ItemMetadataValueAddPatchOperation extends MetadataValueAddPatchOpe
                         int index = Integer.parseInt(controlChar);
                         if (index > metadataByMetadataString.size()) {
                             throw new IllegalArgumentException(
-                                "The specified index MUST NOT be greater than the number of elements in the array");
+                                    "The specified index MUST NOT be greater than the number of elements in the array");
                         }
                         addValue(context, source.getItem(), split[0], object, index);
 
@@ -109,8 +122,88 @@ public class ItemMetadataValueAddPatchOperation extends MetadataValueAddPatchOpe
 
     }
 
+    protected void replaceValue(Context context, Item source, String target, List<MetadataValueRest> list)
+            throws SQLException {
+        String[] metadata = Utils.tokenize(target);
+
+        // fetch pre-existent metadata
+        List<MetadataValue> preExistentMetadata =
+                getDSpaceObjectService().getMetadata(source, metadata[0], metadata[1], metadata[2], Item.ANY);
+
+        // fetch pre-existent relationships
+        Map<Integer, Relationship> preExistentRelationships = preExistentRelationships(context, preExistentMetadata);
+
+        // clear all plain metadata
+        getDSpaceObjectService().clearMetadata(context, source, metadata[0], metadata[1], metadata[2], Item.ANY);
+        // remove all deleted relationships
+        for (Relationship rel : preExistentRelationships.values()) {
+            try {
+                Optional<MetadataValueRest> stillPresent = list.stream()
+                    .filter(ll -> ll.getAuthority() != null &&  rel.getID().equals(getRelId(ll.getAuthority())))
+                    .findAny();
+                if (stillPresent.isEmpty()) {
+                    relationshipService.delete(context, rel);
+                }
+            } catch (AuthorizeException e) {
+                e.printStackTrace();
+                throw new RuntimeException("Authorize Exception during relationship deletion.");
+            }
+        }
+
+        // create plain metadata / move relationships in the list order
+
+        // if a virtual value is present in the list, it must be present in preExistentRelationships too.
+        // (with this operator virtual value can only be moved or deleted).
+        // a not present virtual value will be discarded
+        int idx = 0;
+        for (MetadataValueRest ll : list) {
+            if (StringUtils.startsWith(ll.getAuthority(), Constants.VIRTUAL_AUTHORITY_PREFIX)) {
+
+                Optional<MetadataValue> preExistentMvr = preExistentMetadata.stream().filter(mvr ->
+                    StringUtils.equals(ll.getAuthority(), mvr.getAuthority())).findFirst();
+
+                if (!preExistentMvr.isPresent()) {
+                    idx++;
+                    continue;
+                }
+
+                this.itemService.moveSingleMetadataValue(context, source, idx, preExistentMvr.get());
+
+            } else {
+                getDSpaceObjectService()
+                        .addMetadata(context, source, metadata[0], metadata[1], metadata[2],
+                                ll.getLanguage(), ll.getValue(), ll.getAuthority(), ll.getConfidence(), idx);
+            }
+            idx++;
+        }
+    }
+
+    /**
+     * Retrieve Relationship Objects from a List of MetadataValue.
+     */
+    private Map<Integer, Relationship> preExistentRelationships(Context context,
+            List<MetadataValue> preExistentMetadata) throws SQLException {
+        Map<Integer, Relationship> relationshipsMap = new HashMap<Integer, Relationship>();
+        for (MetadataValue ll : preExistentMetadata) {
+            if (ll instanceof RelationshipMetadataValue) {
+                Relationship relationship = relationshipService
+                        .find(context, ((RelationshipMetadataValue) ll).getRelationshipId());
+                if (relationship != null) {
+                    relationshipsMap.put(relationship.getID(), relationship);
+                }
+            }
+        }
+        return relationshipsMap;
+    }
+
+    private Integer getRelId(String authority) {
+        final int relId = Integer.parseInt(authority.split(Constants.VIRTUAL_AUTHORITY_PREFIX)[1]);
+        return relId;
+    }
+
     @Override
     protected ItemService getDSpaceObjectService() {
         return itemService;
     }
+
 }

dspace-server-webapp/src/test/java/org/dspace/app/rest/PatchMetadataIT.java

@@ -25,7 +25,6 @@ import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import org.dspace.app.rest.matcher.MetadataMatcher;
@@ -1184,27 +1183,11 @@ public class PatchMetadataIT extends AbstractEntityIntegrationTest {
         // "Linton, Oliver" (virtual)
         initPersonPublicationWorkspace();
 
-        // "Linton, Oliver" (virtual)
+        List<MetadataValue> expectedValues = new ArrayList<MetadataValue>();
-        // "Perotti, Enrico"
+        expectedValues.add(this.authorsMetadataOriginalOrder.get(2)); // "Peterson, Karrie"
-        // "Peterson, Karrie"
+        expectedValues.add(this.authorsMetadataOriginalOrder.get(4)); // "Linton, Oliver" (virtual)
-        // "Dahlen, Sarah" (virtual)
+        expectedValues.add(this.authorsMetadataOriginalOrder.get(0)); // "Whyte, William"
-        // "Whyte, William"
+        patchAddEntireArray(expectedValues);
-        List<MetadataValue> reverse = new ArrayList<MetadataValue>();
-        reverse.add(this.authorsMetadataOriginalOrder.get(4));
-        reverse.add(this.authorsMetadataOriginalOrder.get(3));
-        reverse.add(this.authorsMetadataOriginalOrder.get(2));
-        reverse.add(this.authorsMetadataOriginalOrder.get(1));
-        reverse.add(this.authorsMetadataOriginalOrder.get(0));
-        patchAddEntireArray(reverse);
-
-        // "Peterson, Karrie"
-        // "Linton, Oliver" (virtual)
-        // "Whyte, William"
-        List<MetadataValue> deletionAndReorder = new ArrayList<MetadataValue>();
-        deletionAndReorder.add(this.authorsMetadataOriginalOrder.get(2));
-        deletionAndReorder.add(this.authorsMetadataOriginalOrder.get(4));
-        deletionAndReorder.add(this.authorsMetadataOriginalOrder.get(0));
-        patchAddEntireArray(deletionAndReorder);
 
     }
 
@@ -1383,7 +1366,7 @@ public class PatchMetadataIT extends AbstractEntityIntegrationTest {
         final String authorField = "dc.contributor.author";
         final List<Matcher<? super Object>> matchers = new ArrayList<>();
         IntStream.range(0, metadataValues.size()).forEach((i) -> {
-            matchers.add(Matchers.is(MetadataMatcher.matchMetadata(authorField, metadataValues.get(i).getValue(), 0)));
+            matchers.add(Matchers.is(MetadataMatcher.matchMetadata(authorField, metadataValues.get(i).getValue(), i)));
         });