65997: ReplacePatchOperation function moved to subclasses &

EPersonOperationFactory removed, no longer useful
TODO: fix failing tests of replace in two operations

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

@@ -691,9 +691,11 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
 
         List<MetadataValue> list = getMetadata(dso, schema, element, qualifier);
 
-        if (from >= list.size()) {
+        if (from >= list.size() || to >= list.size() || to < 0 || from < 0) {
             throw new IllegalArgumentException(
-                "The \"from\" location MUST exist for the operation to be successful. Idx:" + from);
+                "The \"from\" and \"to\" locations MUST exist for the operation to be successful." +
+                        "\n To and from indices must be between 0 and " + (list.size() - 1) +
+                        "\n Idx from:" + from + " Idx to: " + to);
         }
 
         clearMetadata(context, dso, schema, element, qualifier, Item.ANY);

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

@@ -370,6 +370,26 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     public void delete(Context context, T dso) throws SQLException, AuthorizeException, IOException;
 
+    /**
+     * Add a single metadata field. Whether it's appended or prepended depends on index parameter.
+     * Use <code>clearDC</code> to remove values.
+     *
+     * @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 index      the index at which this metadata is added (0: first place, -1 for last)
+     * @throws SQLException if database error
+     */
     void addAndShiftRightMetadata(Context context, T dso, String schema, String element, String qualifier, String lang,
                                   String value, String authority, int confidence, int index) throws SQLException;
 

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DSpaceObjectRestRepository.java

@@ -35,7 +35,7 @@ public abstract class DSpaceObjectRestRepository<M extends DSpaceObject, R exten
     final DSpaceObjectConverter<M, R> dsoConverter;
 
     @Autowired
-    ResourcePatch<R> resourcePatch;
+    ResourcePatch<M> resourcePatch;
     @Autowired
     MetadataConverter metadataConverter;
 
@@ -63,23 +63,7 @@ public abstract class DSpaceObjectRestRepository<M extends DSpaceObject, R exten
         if (dso == null) {
             throw new ResourceNotFoundException(apiCategory + "." + model + " with id: " + id + " not found");
         }
-        R dsoRest = resourcePatch.patch(findOne(id), patch.getOperations());
+        resourcePatch.patch(obtainContext(), dso, patch.getOperations());
-        updateDSpaceObject(dso, dsoRest);
+        dsoService.update(obtainContext(), dso);
-    }
-
-    /**
-     * Applies the changes in the given rest DSpace object to the model DSpace object.
-     * The default implementation updates metadata if needed. Subclasses should extend
-     * to support updates of additional properties.
-     *
-     * @param dso the dso to apply changes to.
-     * @param dsoRest the rest representation of the new desired state.
-     */
-    protected void updateDSpaceObject(M dso, R dsoRest)
-            throws AuthorizeException, SQLException {
-        R origDsoRest = dsoConverter.fromModel(dso);
-        if (!origDsoRest.getMetadata().equals(dsoRest.getMetadata())) {
-            metadataConverter.setMetadata(obtainContext(), dso, dsoRest.getMetadata());
-        }
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java

@@ -10,7 +10,6 @@ package org.dspace.app.rest.repository;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.List;
-import java.util.Objects;
 import java.util.UUID;
 import javax.servlet.http.HttpServletRequest;
 
@@ -187,31 +186,6 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
         patchDSpaceObject(apiCategory, model, uuid, patch);
     }
 
-    @Override
-    protected void updateDSpaceObject(EPerson ePerson, EPersonRest ePersonRest)
-            throws AuthorizeException, SQLException {
-        super.updateDSpaceObject(ePerson, ePersonRest);
-
-        Context context = obtainContext();
-        if (ePersonRest.getPassword() != null) {
-            es.setPassword(ePerson, ePersonRest.getPassword());
-        }
-        if (ePersonRest.isRequireCertificate() != ePerson.getRequireCertificate()) {
-            ePerson.setRequireCertificate(ePersonRest.isRequireCertificate());
-        }
-        if (ePersonRest.isCanLogIn() != ePerson.canLogIn()) {
-            ePerson.setCanLogIn(ePersonRest.isCanLogIn());
-        }
-        if (!Objects.equals(ePersonRest.getEmail(), ePerson.getEmail())) {
-            ePerson.setEmail(ePersonRest.getEmail());
-        }
-        if (!Objects.equals(ePersonRest.getNetid(), ePerson.getNetid())) {
-            ePerson.setNetid(ePersonRest.getNetid());
-        }
-
-        es.update(context, ePerson);
-    }
-
     @Override
     protected void delete(Context context, UUID id) throws AuthorizeException {
         EPerson eperson = null;

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java

@@ -122,25 +122,6 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
         patchDSpaceObject(apiCategory, model, id, patch);
     }
 
-    @Override
-    protected void updateDSpaceObject(Item item, ItemRest itemRest)
-            throws AuthorizeException, SQLException  {
-        super.updateDSpaceObject(item, itemRest);
-
-        Context context = obtainContext();
-        if (itemRest.getWithdrawn() != item.isWithdrawn()) {
-            if (itemRest.getWithdrawn()) {
-                is.withdraw(context, item);
-            } else {
-                is.reinstate(context, item);
-            }
-        }
-        if (itemRest.getDiscoverable() != item.isDiscoverable()) {
-            item.setDiscoverable(itemRest.getDiscoverable());
-            is.update(context, item);
-        }
-    }
-
     @Override
     public Class<ItemRest> getDomainClass() {
         return ItemRest.class;

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/ResourcePatch.java

@@ -11,19 +11,18 @@ import java.util.List;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
 import org.dspace.app.rest.exception.UnprocessableEntityException;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
 import org.dspace.app.rest.repository.patch.factories.impl.PatchOperation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 /**
  * The base class for resource PATCH operations.
- *
- * @author Michael Spalti
  */
 @Component
-public class ResourcePatch<R extends RestModel> {
+public class ResourcePatch<M extends DSpaceObject> {
 
     @Autowired
     private List<PatchOperation> patchOperations;
@@ -32,31 +31,31 @@ public class ResourcePatch<R extends RestModel> {
      * Handles the patch operations. Patch implementations are provided by subclasses.
      * The default methods throw an UnprocessableEntityException.
      *
-     * @param restModel     the rest resource to patch
+     * @param context       Context of patch operation
+     * @param dso           the dso resource to patch
      * @param operations    list of patch operations
      * @throws UnprocessableEntityException
      * @throws DSpaceBadRequestException
      */
-    public R patch(R restModel, List<Operation> operations) {
+    public void patch(Context context, M dso, List<Operation> operations) {
         for (Operation operation: operations) {
-            performPatchOperation(restModel, operation);
+            performPatchOperation(context, dso, operation);
         }
-        return restModel;
-
     }
 
     /**
      * Checks with all possible patch operations whether they support this operation
      *      (based on instanceof restModel and operation.path
-     * @param restModel     the rest resource to patch
+     * @param context       Context of patch operation
+     * @param dso           the dso resource to patch
      * @param operation     the patch operation
      * @throws DSpaceBadRequestException
      */
-    protected void performPatchOperation(R restModel, Operation operation)
+    protected void performPatchOperation(Context context, M dso, Operation operation)
             throws DSpaceBadRequestException {
         for (PatchOperation patchOperation: patchOperations) {
-            if (patchOperation.supports(restModel, operation.getPath())) {
+            if (patchOperation.supports(dso, operation.getPath())) {
-                patchOperation.perform(restModel, operation);
+                patchOperation.perform(context, dso, operation);
                 return;
             }
         }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/DspaceObjectMetadataOperation.java

@@ -8,19 +8,26 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import java.io.IOException;
-import java.util.ArrayList;
+import java.sql.SQLException;
+import java.util.Arrays;
 import java.util.List;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.ObjectNode;
+import org.apache.commons.lang3.StringUtils;
-import com.flipkart.zjsonpatch.JsonPatch;
+import org.apache.logging.log4j.Logger;
 import org.dspace.app.rest.converter.JsonPatchConverter;
-import org.dspace.app.rest.model.DSpaceObjectRest;
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.MetadataRest;
+import org.dspace.app.rest.exception.UnprocessableEntityException;
-import org.dspace.app.rest.model.RestModel;
+import org.dspace.app.rest.model.patch.JsonValueEvaluator;
+import org.dspace.app.rest.model.patch.MoveOperation;
 import org.dspace.app.rest.model.patch.Operation;
-import org.dspace.app.rest.model.patch.Patch;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
+import org.dspace.content.MetadataValue;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.DSpaceObjectService;
+import org.dspace.core.Context;
 import org.springframework.stereotype.Component;
 
 /**
@@ -55,7 +62,9 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 30/10/2019
  */
 @Component
-public class DspaceObjectMetadataOperation<R extends RestModel> extends PatchOperation<R> {
+public class DspaceObjectMetadataOperation<R extends DSpaceObject> extends PatchOperation<R> {
+    private static final Logger log
+            = org.apache.logging.log4j.LogManager.getLogger(DspaceObjectMetadataOperation.class);
 
     /**
      * Path in json body of patch that uses this operation
@@ -66,43 +75,261 @@ public class DspaceObjectMetadataOperation<R extends RestModel> extends PatchOpe
 
     /**
      * Implements the patch operation for metadata operations.
-     *
+     * @param context   context we're performing patch in
-     * @param resource  the rest model.
+     * @param resource  the dso.
      * @param operation the metadata patch operation.
-     * @return the updated rest model.
+     * @return the updated dso
      */
     @Override
-    public R perform(R resource, Operation operation) {
+    public R perform(Context context, R resource, Operation operation) {
-        DSpaceObjectRest dSpaceObjectRest = (DSpaceObjectRest) resource;
+        DSpaceObject dSpaceObject = (DSpaceObject) resource;
-        List<Operation> operations = new ArrayList<Operation>();
+        performPatchOperation(context, dSpaceObject, operation);
-        operations.add(operation);
+        return (R) dSpaceObject;
-        dSpaceObjectRest.setMetadata(applyMetadataPatch(
-                jsonPatchConverter.convert(new Patch(operations)),
-                dSpaceObjectRest.getMetadata()));
-        return (R) dSpaceObjectRest;
     }
 
     /**
-     * Apply the actual metadata patch by replacing the original metadata node
+     * Gets all the info about the metadata we're patching from the operation and sends it to the
-     *  with the newly created one based on the patch body
+     *      appropriate method to perform the actual patch
-     * @param patch             Metadata patch used for the replacement
+     * @param context       Context we're performing patch in
-     * @param metadataRest      Original metadata rest object
+     * @param dso           object we're performing metadata patch on
-     * @return  Newly created metadata node
+     * @param operation     patch operation
      */
-    private MetadataRest applyMetadataPatch(JsonNode patch, MetadataRest metadataRest) {
+    private void performPatchOperation(Context context, DSpaceObject dso, Operation operation) {
+        DSpaceObjectService dsoService = ContentServiceFactory.getInstance().getDSpaceObjectService(dso);
+        String mdElement = StringUtils.substringBetween(operation.getPath(), METADATA_PATH + "/", "/");
+        if (mdElement == null) {
+            mdElement = StringUtils.substringAfter(operation.getPath(), METADATA_PATH + "/");
+        }
+        String[] seq = mdElement.split("\\.");
+        String schema = seq.length > 1 ? seq[0] : null;
+        String element = seq.length > 1 ? seq[1] : null;
+        String qualifier = seq.length == 3 ? seq[2] : null;
+
+        String[] parts = operation.getPath().split("/");
+        String indexInPath = (parts.length > 3) ? parts[3] : null;
+        String propertyOfMd = (parts.length > 4) ? parts[4] : null;
+
         try {
-            ObjectNode objectNode = objectMapper.createObjectNode();
+            MetadataValue metadataValue = null;
-            JsonNode metadataNode = objectMapper.valueToTree(metadataRest);
+            if (operation.getValue() != null) {
-            objectNode.replace("metadata", metadataNode);
+                JsonNode valueNode = ((JsonValueEvaluator) operation.getValue()).getValueNode();
-            JsonPatch.applyInPlace(patch, objectNode);
+                if (valueNode.isArray()) {
-            return objectMapper.treeToValue(objectNode.get("metadata"), MetadataRest.class);
+                    metadataValue = objectMapper.treeToValue(valueNode.get(0), MetadataValue.class);
+                } else {
+                    metadataValue = objectMapper.treeToValue(valueNode, MetadataValue.class);
+                }
+            }
+
+            switch (operation.getOp()) {
+                case "add":
+                    add(context, dso, dsoService, schema, element, qualifier, metadataValue, indexInPath);
+                    return;
+                case "remove":
+                    remove(context, dso, dsoService, schema, element, qualifier, indexInPath);
+                    return;
+                case "replace":
+                    replace(context, dso, dsoService, schema, element, qualifier,
+                            metadataValue, indexInPath, propertyOfMd);
+                    return;
+                case "move":
+                    String[] partsFrom = ((MoveOperation)operation).getFrom().split("/");
+                    String indexTo = (partsFrom.length > 3) ? partsFrom[3] : null;
+                    move(context, dso, dsoService, schema, element, qualifier, indexInPath, indexTo);
+                    return;
+                default:
+                    throw new DSpaceBadRequestException(
+                            "This operation is not supported."
+                    );
+            }
         } catch (IOException e) {
-            throw new IllegalArgumentException(e);
+            log.error("IOException in DspaceObjectMetadataOperation.performPatchOperation trying " +
+                    "to map json from operation.value to MetadataValue class.", e);
+        }
+    }
+
+    /**
+     * Adds metadata to the dso (appending if index is 0 or left out, prepending if -)
+     * @param context           context patch is being performed in
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param schema            schema of md field being patched
+     * @param element           element of md field being patched
+     * @param qualifier         qualifier of md field being patched
+     * @param metadataValue     value of md element
+     * @param index             determines whether we're prepending (-) or appending (0) md value
+     */
+    private void add(Context context, DSpaceObject dso,
+                     DSpaceObjectService dsoService, String schema, String element,
+                     String qualifier, MetadataValue metadataValue, String index) {
+        int indexInt = 0;
+        if (index != null && index.equals("-")) {
+            indexInt = -1;
+        }
+        try {
+            dsoService.addAndShiftRightMetadata(context, dso, schema, element, qualifier,
+                    metadataValue.getLanguage(), metadataValue.getValue(),
+                    metadataValue.getAuthority(), metadataValue.getConfidence(), indexInt);
+        } catch (SQLException e) {
+            log.error("SQLException in DspaceObjectMetadataOperation.add trying to add metadata to dso.", e);
+        }
+    }
+
+    /**
+     * Removes a metadata from the dso at a given index (or all of that type if no index was given)
+     * @param context           context patch is being performed in
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param schema            schema of md field being patched
+     * @param element           element of md field being patched
+     * @param qualifier         qualifier of md field being patched
+     * @param index             index at where we want to delete metadata
+     */
+    private void remove(Context context, DSpaceObject dso,
+                     DSpaceObjectService dsoService, String schema, String element,
+                     String qualifier, String index) {
+        if (index == null) {
+            //remove all metadata of this type
+            try {
+                dsoService.clearMetadata(context, dso, schema, element, qualifier, Item.ANY);
+            } catch (SQLException e) {
+                log.error("SQLException in DspaceObjectMetadataOperation.remove trying to " +
+                        "remove metadata from dso.", e);
+            }
+        } else {
+            //remove metadata at index
+            List<MetadataValue> metadataValues = dsoService.getMetadata(dso, schema, element, qualifier, Item.ANY);
+            try {
+                int indexInt = Integer.parseInt(index);
+                if (indexInt >= 0 && metadataValues.size() > indexInt
+                        && metadataValues.get(indexInt) != null) {
+                    //remove that metadata
+                    dsoService.removeMetadataValues(context, dso,
+                            Arrays.asList(metadataValues.get(Integer.parseInt(index))));
+                } else {
+                    throw new UnprocessableEntityException("There is no metadata of this type at that index");
+                }
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("This index (" + index + ") is not valid nr", e);
+            } catch (ArrayIndexOutOfBoundsException e) {
+                throw new UnprocessableEntityException("There is no metadata of this type at that index");
+            } catch (SQLException e) {
+                log.error("SQLException in DspaceObjectMetadataOperation.remove trying to remove " +
+                        "metadata from dso.", e);
+            }
+        }
+    }
+
+    /**
+     * Replaces metadata in the dso; 4 cases:
+     *      - If we replace everything: clearMetadata & add the new ones
+     *      - If we replace for a single field: clearMetadata on the field & add the new ones
+     *      - A single existing metadata value: Retrieve the metadatavalue object & make alerations directly on this object
+     *      - A single existing metadata property: Retrieve the metadatavalue object & make alerations directly on this object
+     * @param context           context patch is being performed in
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param schema            schema of md field being patched
+     * @param element           element of md field being patched
+     * @param qualifier         qualifier of md field being patched
+     * @param metadataValue     value of md element
+     */
+    private void replace(Context context, DSpaceObject dso,
+                     DSpaceObjectService dsoService, String schema, String element,
+                     String qualifier, MetadataValue metadataValue, String index, String propertyOfMd) {
+        // replace entire set of metadata
+        if (schema == null) {
+            try {
+                dsoService.clearMetadata(context, dso, Item.ANY, Item.ANY, Item.ANY, Item.ANY);
+                // TODO How to add new md value if exists? No knowledge of seq
+            } catch (SQLException e) {
+                log.error("SQLException in DspaceObjectMetadataOperation.replace trying to remove" +
+                        "and replace metadata from dso.", e);
+            }
+        }
+        // replace all metadata for existing key
+        if (schema != null && index == null) {
+            try {
+                dsoService.clearMetadata(context, dso, schema, element, qualifier, Item.ANY);
+                this.add(context, dso, dsoService, schema, element, qualifier, metadataValue, null);
+            } catch (SQLException e) {
+                log.error("SQLException in DspaceObjectMetadataOperation.replace trying to remove " +
+                        "and replace metadata from dso.", e);
+            }
+        }
+        // replace single existing metadata value
+        if (schema != null && index != null && propertyOfMd == null) {
+            try {
+                List<MetadataValue> metadataValues = dsoService.getMetadata(dso, schema, element,
+                        qualifier, Item.ANY);
+                int indexInt = Integer.parseInt(index);
+                if (indexInt >= 0 && metadataValues.size() > indexInt
+                        && metadataValues.get(indexInt) != null) {
+                    // Alter this existing md
+                    MetadataValue existingMdv = metadataValues.get(indexInt);
+                    existingMdv.setAuthority(metadataValue.getAuthority());
+                    existingMdv.setConfidence(metadataValue.getConfidence());
+                    existingMdv.setLanguage(metadataValue.getLanguage());
+                    existingMdv.setValue(metadataValue.getValue());
+                } else {
+                    throw new UnprocessableEntityException("There is no metadata of this type at that index");
+                }
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("This index (" + index + ") is not valid nr", e);
+            }
+        }
+        // replace single property of exiting metadata value
+        if (schema != null && index != null && propertyOfMd != null) {
+            try {
+                List<MetadataValue> metadataValues = dsoService.getMetadata(dso, schema, element, qualifier, Item.ANY);
+                int indexInt = Integer.parseInt(index);
+                if (indexInt >= 0 && metadataValues.size() > indexInt && metadataValues.get(indexInt) != null) {
+                    // Alter only asked propertyOfMd
+                    MetadataValue existingMdv = metadataValues.get(indexInt);
+                    if (propertyOfMd.equals("authority")) {
+                        existingMdv.setAuthority(metadataValue.getAuthority());
+                    }
+                    if (propertyOfMd.equals("confidence")) {
+                        existingMdv.setConfidence(metadataValue.getConfidence());
+                    }
+                    if (propertyOfMd.equals("language")) {
+                        existingMdv.setLanguage(metadataValue.getLanguage());
+                    }
+                    if (propertyOfMd.equals("value")) {
+                        existingMdv.setValue(metadataValue.getValue());
+                    }
+                } else {
+                    throw new UnprocessableEntityException("There is no metadata of this type at that index");
+                }
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("This index (" + index + ") is not valid nr", e);
+            }
+        }
+    }
+
+    /**
+     * Moves metadata of the dso from indexFrom to indexTo
+     * @param context           context patch is being performed in
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param schema            schema of md field being patched
+     * @param element           element of md field being patched
+     * @param qualifier         qualifier of md field being patched
+     * @param indexFrom         index we're moving metadata from
+     * @param indexTo         index we're moving metadata to
+     */
+    private void move(Context context, DSpaceObject dso,
+                     DSpaceObjectService dsoService, String schema, String element,
+                     String qualifier, String indexFrom, String indexTo) {
+        try {
+            dsoService.moveMetadata(context, dso, schema, element, qualifier,
+                    Integer.parseInt(indexFrom), Integer.parseInt(indexTo));
+        } catch (SQLException e) {
+            log.error("SQLException in DspaceObjectMetadataOperation.move trying to move metadata in dso.", e);
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return ((path.equals(METADATA_PATH) || path.startsWith(METADATA_PATH + "/")) && R instanceof DSpaceObjectRest);
+        return ((path.startsWith(METADATA_PATH) || path.equals(METADATA_PATH)) && R instanceof DSpaceObject);
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonCertificateReplaceOperation.java

@@ -8,9 +8,10 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Component;
 
 /**
@@ -21,11 +22,9 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /certificate", "value": true|false]'
  * </code>
- *
- * @author Michael Spalti
  */
 @Component
-public class EPersonCertificateReplaceOperation extends PatchOperation<EPersonRest> {
+public class EPersonCertificateReplaceOperation extends PatchOperation<EPerson> {
 
     /**
      * Path in json body of patch that uses this operation
@@ -33,7 +32,7 @@ public class EPersonCertificateReplaceOperation extends PatchOperation<EPersonRe
     private static final String OPERATION_PATH_CERTIFICATE = "/certificate";
 
     @Override
-    public EPersonRest perform(EPersonRest eperson, Operation operation) {
+    public EPerson perform(Context context, EPerson eperson, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(eperson);
         Boolean requireCert = getBooleanOperationValue(operation.getValue());
@@ -42,17 +41,17 @@ public class EPersonCertificateReplaceOperation extends PatchOperation<EPersonRe
 
     }
 
-    void checkModelForExistingValue(EPersonRest resource) {
+    void checkModelForExistingValue(EPerson resource) {
         // TODO: many (all?) boolean values on the rest model should never be null.
         // So perhaps the error to throw in this case is different...IllegalStateException?
         // Or perhaps do nothing (no check is required).
-        if ((Object) resource.isRequireCertificate() == null) {
+        if ((Object) resource.getRequireCertificate() == null) {
             throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return (R instanceof EPersonRest && path.trim().equalsIgnoreCase(OPERATION_PATH_CERTIFICATE));
+        return (R instanceof EPerson && path.trim().equalsIgnoreCase(OPERATION_PATH_CERTIFICATE));
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonEmailReplaceOperation.java

@@ -8,9 +8,10 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Component;
 
 /**
@@ -21,11 +22,9 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /email", "value": "new@email"]'
  * </code>
- *
- * @author Michael Spalti
  */
 @Component
-public class EPersonEmailReplaceOperation extends PatchOperation<EPersonRest> {
+public class EPersonEmailReplaceOperation extends PatchOperation<EPerson> {
 
     /**
      * Path in json body of patch that uses this operation
@@ -33,21 +32,21 @@ public class EPersonEmailReplaceOperation extends PatchOperation<EPersonRest> {
     private static final String OPERATION_PATH_EMAIL = "/email";
 
     @Override
-    public EPersonRest perform(EPersonRest eperson, Operation operation) {
+    public EPerson perform(Context context, EPerson eperson, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(eperson);
         eperson.setEmail((String) operation.getValue());
         return eperson;
     }
 
-    void checkModelForExistingValue(EPersonRest resource) {
+    void checkModelForExistingValue(EPerson resource) {
         if (resource.getEmail() == null) {
             throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return (R instanceof EPersonRest && path.trim().equalsIgnoreCase(OPERATION_PATH_EMAIL));
+        return (R instanceof EPerson && path.trim().equalsIgnoreCase(OPERATION_PATH_EMAIL));
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonLoginReplaceOperation.java

@@ -8,9 +8,10 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Component;
 
 /**
@@ -21,11 +22,9 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /canLogin", "value": true|false]'
  * </code>
- *
- * @author Michael Spalti
  */
 @Component
-public class EPersonLoginReplaceOperation extends PatchOperation<EPersonRest> {
+public class EPersonLoginReplaceOperation extends PatchOperation<EPerson> {
 
     /**
      * Path in json body of patch that uses this operation
@@ -33,7 +32,7 @@ public class EPersonLoginReplaceOperation extends PatchOperation<EPersonRest> {
     private static final String OPERATION_PATH_PASSWORD = "/canLogin";
 
     @Override
-    public EPersonRest perform(EPersonRest eperson, Operation operation) {
+    public EPerson perform(Context context, EPerson eperson, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(eperson);
         Boolean canLogin = getBooleanOperationValue(operation.getValue());
@@ -41,14 +40,14 @@ public class EPersonLoginReplaceOperation extends PatchOperation<EPersonRest> {
         return eperson;
     }
 
-    void checkModelForExistingValue(EPersonRest resource) {
+    void checkModelForExistingValue(EPerson resource) {
-        if ((Object) resource.isCanLogIn() == null) {
+        if ((Object) resource.canLogIn() == null) {
             throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return (R instanceof EPersonRest && path.trim().equalsIgnoreCase(OPERATION_PATH_PASSWORD));
+        return (R instanceof EPerson && path.trim().equalsIgnoreCase(OPERATION_PATH_PASSWORD));
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonNetidReplaceOperation.java

@@ -8,9 +8,10 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Component;
 
 /**
@@ -21,11 +22,9 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /netid", "value": "newNetId"]'
  * </code>
- *
- * @author Michael Spalti
  */
 @Component
-public class EPersonNetidReplaceOperation extends PatchOperation<EPersonRest> {
+public class EPersonNetidReplaceOperation extends PatchOperation<EPerson> {
 
     /**
      * Path in json body of patch that uses this operation
@@ -33,21 +32,21 @@ public class EPersonNetidReplaceOperation extends PatchOperation<EPersonRest> {
     private static final String OPERATION_PATH_NETID = "/netid";
 
     @Override
-    public EPersonRest perform(EPersonRest eperson, Operation operation) {
+    public EPerson perform(Context context, EPerson eperson, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(eperson);
         eperson.setNetid((String) operation.getValue());
         return eperson;
     }
 
-    void checkModelForExistingValue(EPersonRest resource) {
+    void checkModelForExistingValue(EPerson resource) {
         if (resource.getNetid() == null) {
             throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return (R instanceof EPersonRest && path.trim().equalsIgnoreCase(OPERATION_PATH_NETID));
+        return (R instanceof EPerson && path.trim().equalsIgnoreCase(OPERATION_PATH_NETID));
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonPasswordReplaceOperation.java

@@ -7,9 +7,12 @@
  */
 package org.dspace.app.rest.repository.patch.factories.impl;
 
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
+import org.dspace.eperson.factory.EPersonServiceFactory;
+import org.dspace.eperson.service.EPersonService;
 import org.springframework.stereotype.Component;
 
 /**
@@ -20,26 +23,25 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /password", "value": "newpassword"]'
  * </code>
- *
- * @author Michael Spalti
  */
 @Component
-public class EPersonPasswordReplaceOperation extends PatchOperation<EPersonRest> {
+public class EPersonPasswordReplaceOperation extends PatchOperation<EPerson> {
 
     /**
      * Path in json body of patch that uses this operation
      */
     public static final String OPERATION_PASSWORD_CHANGE = "/password";
+    protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
 
     @Override
-    public EPersonRest perform(EPersonRest eperson, Operation operation) {
+    public EPerson perform(Context context, EPerson eperson, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(eperson);
-        eperson.setPassword((String) operation.getValue());
+        ePersonService.setPassword(eperson, (String) operation.getValue());
         return eperson;
     }
 
-    void checkModelForExistingValue(EPersonRest resource) {
+    void checkModelForExistingValue(EPerson resource) {
         /*
          * FIXME: the password field in eperson rest model is always null because
          * the value is not set in the rest converter.
@@ -50,7 +52,7 @@ public class EPersonPasswordReplaceOperation extends PatchOperation<EPersonRest>
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return (R instanceof EPersonRest && path.trim().equalsIgnoreCase(OPERATION_PASSWORD_CHANGE));
+        return (R instanceof EPerson && path.trim().equalsIgnoreCase(OPERATION_PASSWORD_CHANGE));
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ItemDiscoverableReplaceOperation.java

@@ -8,9 +8,10 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ItemRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
+import org.dspace.core.Context;
 import org.springframework.stereotype.Component;
 
 /**
@@ -21,11 +22,9 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /discoverable", "value": true|false]'
  * </code>
- *
- *  @author Michael Spalti
  */
 @Component
-public class ItemDiscoverableReplaceOperation extends PatchOperation<ItemRest> {
+public class ItemDiscoverableReplaceOperation extends PatchOperation<Item> {
 
     /**
      * Path in json body of patch that uses this operation
@@ -33,7 +32,7 @@ public class ItemDiscoverableReplaceOperation extends PatchOperation<ItemRest> {
     private static final String OPERATION_PATH_DISCOVERABLE = "/discoverable";
 
     @Override
-    public ItemRest perform(ItemRest item, Operation operation) {
+    public Item perform(Context context, Item item, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(item);
         Boolean discoverable = getBooleanOperationValue(operation.getValue());
@@ -42,15 +41,15 @@ public class ItemDiscoverableReplaceOperation extends PatchOperation<ItemRest> {
 
     }
 
-    void checkModelForExistingValue(ItemRest resource) {
+    void checkModelForExistingValue(Item resource) {
-        if ((Object) resource.getDiscoverable() == null) {
+        if ((Object) resource.isDiscoverable() == null) {
             throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject R, String path) {
-        return (R instanceof ItemRest && path.trim().equalsIgnoreCase(OPERATION_PATH_DISCOVERABLE));
+        return (R instanceof Item && path.trim().equalsIgnoreCase(OPERATION_PATH_DISCOVERABLE));
     }
 
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ItemWithdrawReplaceOperation.java

@@ -7,11 +7,17 @@
  */
 package org.dspace.app.rest.repository.patch.factories.impl;
 
+import java.sql.SQLException;
+
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
 import org.dspace.app.rest.exception.UnprocessableEntityException;
-import org.dspace.app.rest.model.ItemRest;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.ItemService;
+import org.dspace.core.Context;
 import org.springframework.stereotype.Component;
 
 /**
@@ -22,19 +28,18 @@ import org.springframework.stereotype.Component;
  * Content-Type: application/json" -d '[{ "op": "replace", "path": "
  * /withdrawn", "value": true|false]'
  * </code>
- *
- * @author Michael Spalti
  */
 @Component
-public class ItemWithdrawReplaceOperation extends PatchOperation<ItemRest> {
+public class ItemWithdrawReplaceOperation extends PatchOperation<Item> {
 
     /**
      * Path in json body of patch that uses this operation
      */
     private static final String OPERATION_PATH_WITHDRAW = "/withdrawn";
+    private static final ItemService itemService = ContentServiceFactory.getInstance().getItemService();
 
     @Override
-    public ItemRest perform(ItemRest item, Operation operation) {
+    public Item perform(Context context, Item item, Operation operation) {
         checkOperationValue(operation.getValue());
         checkModelForExistingValue(item);
 
@@ -43,39 +48,55 @@ public class ItemWithdrawReplaceOperation extends PatchOperation<ItemRest> {
         // This is a request to withdraw the item.
         if (withdraw) {
             // The item is currently not withdrawn and also not archived. Is this a possible situation?
-            if (!item.getWithdrawn() && !item.getInArchive()) {
+            if (!item.isWithdrawn() && !item.isArchived()) {
                 throw new UnprocessableEntityException("Cannot withdraw item when it is not in archive.");
             }
             // Item is already withdrawn. No-op, 200 response.
             // (The operation is not idempotent since it results in a provenance note in the record.)
-            if (item.getWithdrawn()) {
+            if (item.isWithdrawn()) {
                 return item;
             }
-            item.setWithdrawn(true);
+            try {
+                itemService.withdraw(context, item);
+            } catch (SQLException e) {
+                // TODO
+                e.printStackTrace();
+            } catch (AuthorizeException e) {
+                // TODO
+                e.printStackTrace();
+            }
             return item;
 
         } else {
             // No need to reinstate item if it has not previously been not withdrawn.
             // No-op, 200 response. (The operation is not idempotent since it results
             // in a provenance note in the record.)
-            if (!item.getWithdrawn()) {
+            if (!item.isWithdrawn()) {
                 return item;
             }
-            item.setWithdrawn(false);
+            try {
+                itemService.reinstate(context, item);
+            } catch (SQLException e) {
+                // TODO
+                e.printStackTrace();
+            } catch (AuthorizeException e) {
+                // TODO
+                e.printStackTrace();
+            }
             return item;
         }
 
     }
 
-    void checkModelForExistingValue(ItemRest resource) {
+    void checkModelForExistingValue(Item resource) {
-        if ((Object) resource.getWithdrawn() == null) {
+        if ((Object) resource.isWithdrawn() == null) {
             throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
         }
     }
 
     @Override
-    public boolean supports(RestModel R, String path) {
+    public boolean supports(DSpaceObject dso, String path) {
-        return (R instanceof ItemRest && path.trim().equalsIgnoreCase(OPERATION_PATH_WITHDRAW));
+        return (dso instanceof Item && path.trim().equalsIgnoreCase(OPERATION_PATH_WITHDRAW));
     }
 
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/PatchOperation.java

@@ -9,26 +9,26 @@ package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.apache.commons.lang3.BooleanUtils;
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
 
 /**
  * Base class for all resource patch operations.
- *
- * @author Michael Spalti
  */
-public abstract class PatchOperation<R extends RestModel>
+public abstract class PatchOperation<M extends DSpaceObject>
-        implements ResourcePatchOperation<R> {
+        implements ResourcePatchOperation<M> {
 
     /**
      * Updates the rest model by applying the patch operation.
      *
-     * @param resource  the rest model.
+     * @param context   context of patch operation
+     * @param resource  the dso.
      * @param operation the patch operation.
-     * @return the updated rest model.
+     * @return the patched dso
      * @throws DSpaceBadRequestException
      */
-    public abstract R perform(R resource, Operation operation);
+    public abstract M perform(Context context, M resource, Operation operation);
 
     /**
      * Throws PatchBadRequestException for missing operation value.
@@ -65,10 +65,10 @@ public abstract class PatchOperation<R extends RestModel>
 
     /**
      * Determines whether or not this Patch Operation can do this patch (RestModel and path gets checked)
-     * @param R         RestModel, whose class must be instance of dso for which this PatchOperation was created
+     * @param M         dso, whose class must be instance of dso for which this PatchOperation was created
      * @param path      Path given to the patch body, should match this type of Patch Operation
-     * @return          True if this PatchOperation class can do the patch for this given RestModel and Path
+     * @return          True if this PatchOperation class can do the patch for this given dso type and Path
      */
-    public abstract boolean supports(RestModel R, String path);
+    public abstract boolean supports(DSpaceObject M, String path);
 
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ResourcePatchOperation.java

@@ -8,16 +8,17 @@
 package org.dspace.app.rest.repository.patch.factories.impl;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.core.Context;
 
 /**
  * The patch interface used by repository classes.
- * @param <R>
+ * @param <M>
  */
-public interface ResourcePatchOperation<R extends RestModel> {
+public interface ResourcePatchOperation<M extends DSpaceObject> {
 
-    R perform(R resource, Operation operation)
+    M perform(Context context, M resource, Operation operation)
             throws DSpaceBadRequestException;
 
 }