Merge branch 'master' of https://github.com/DSpace/DSpace into authz

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

@@ -11,8 +11,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
-import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrInputDocument;
@@ -140,8 +140,8 @@ public class PersonAuthorityValue extends AuthorityValue {
     @Override
     public void setValues(SolrDocument document) {
         super.setValues(document);
-        this.firstName = ObjectUtils.toString(document.getFieldValue("first_name"));
+        this.firstName = Objects.toString(document.getFieldValue("first_name"), "");
-        this.lastName = ObjectUtils.toString(document.getFieldValue("last_name"));
+        this.lastName = Objects.toString(document.getFieldValue("last_name"), "");
         nameVariants = new ArrayList<String>();
         Collection<Object> document_name_variant = document.getFieldValues("name_variant");
         if (document_name_variant != null) {

dspace-api/src/main/java/org/dspace/authorize/ResourcePolicy.java

@@ -8,6 +8,7 @@
 package org.dspace.authorize;
 
 import java.util.Date;
+import java.util.Objects;
 import javax.persistence.CascadeType;
 import javax.persistence.Column;
 import javax.persistence.Entity;
@@ -23,7 +24,6 @@ import javax.persistence.Table;
 import javax.persistence.Temporal;
 import javax.persistence.TemporalType;
 
-import org.apache.commons.lang3.ObjectUtils;
 import org.dspace.content.DSpaceObject;
 import org.dspace.core.Context;
 import org.dspace.core.ReloadableEntity;
@@ -123,16 +123,16 @@ public class ResourcePolicy implements ReloadableEntity<Integer> {
         if (getAction() != other.getAction()) {
             return false;
         }
-        if (!ObjectUtils.equals(getEPerson(), other.getEPerson())) {
+        if (!Objects.equals(getEPerson(), other.getEPerson())) {
             return false;
         }
-        if (!ObjectUtils.equals(getGroup(), other.getGroup())) {
+        if (!Objects.equals(getGroup(), other.getGroup())) {
             return false;
         }
-        if (!ObjectUtils.equals(getStartDate(), other.getStartDate())) {
+        if (!Objects.equals(getStartDate(), other.getStartDate())) {
             return false;
         }
-        if (!ObjectUtils.equals(getEndDate(), other.getEndDate())) {
+        if (!Objects.equals(getEndDate(), other.getEndDate())) {
             return false;
         }
         return true;
@@ -185,7 +185,7 @@ public class ResourcePolicy implements ReloadableEntity<Integer> {
     /**
      * set the action this policy authorizes
      *
-     * @param myid action ID from {@link org.dspace.core.Constants#Constants Constants}
+     * @param myid action ID from {@link org.dspace.core.Constants Constants}
      */
     public void setAction(int myid) {
         this.actionId = myid;

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

@@ -694,9 +694,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);
@@ -757,4 +759,9 @@ public abstract class DSpaceObjectServiceImpl<T extends DSpaceObject> implements
         }
     }
 
+    @Override
+    public void setMetadataModified(T dso) {
+        dso.setMetadataModified();
+    }
+
 }

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

@@ -12,11 +12,13 @@ import java.sql.SQLException;
 import java.util.List;
 
 import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.content.dao.MetadataFieldDAO;
 import org.dspace.content.service.MetadataFieldService;
+import org.dspace.content.service.MetadataSchemaService;
 import org.dspace.content.service.MetadataValueService;
 import org.dspace.core.Context;
 import org.dspace.core.LogManager;
@@ -42,6 +44,8 @@ public class MetadataFieldServiceImpl implements MetadataFieldService {
     protected AuthorizeService authorizeService;
     @Autowired(required = true)
     protected MetadataValueService metadataValueService;
+    @Autowired(required = true)
+    protected MetadataSchemaService metadataSchemaService;
 
     protected MetadataFieldServiceImpl() {
 
@@ -87,13 +91,25 @@ public class MetadataFieldServiceImpl implements MetadataFieldService {
         return metadataFieldDAO.findByElement(context, metadataSchema, element, qualifier);
     }
 
-
     @Override
     public MetadataField findByElement(Context context, String metadataSchemaName, String element, String qualifier)
         throws SQLException {
         return metadataFieldDAO.findByElement(context, metadataSchemaName, element, qualifier);
     }
 
+    @Override
+    public MetadataField findByString(Context context, String mdString, char separator) throws SQLException {
+        String[] seq = StringUtils.split(mdString, separator);
+        String schema = seq.length > 1 ? seq[0] : null;
+        String element = seq.length > 1 ? seq[1] : null;
+        String qualifier = seq.length == 3 ? seq[2] : null;
+        if (schema == null || element == null) {
+            return null;
+        } else {
+            return this.findByElement(context, schema, element, qualifier);
+        }
+    }
+
     @Override
     public List<MetadataField> findFieldsByElementNameUnqualified(Context context, String metadataSchemaName,
                                                                   String element) throws SQLException {

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

@@ -182,7 +182,7 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Add metadata fields. These are appended to existing values.
-     * Use <code>clearDC</code> to remove values. The ordering of values
+     * Use <code>clearMetadata</code> to remove values. The ordering of values
      * passed in is maintained.
      * <p>
      * If metadata authority control is available, try to get authority
@@ -207,7 +207,7 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Add metadata fields. These are appended to existing values.
-     * Use <code>clearDC</code> to remove values. The ordering of values
+     * Use <code>clearMetadata</code> to remove values. The ordering of values
      * passed in is maintained.
      *
      * @param context     DSpace context
@@ -231,7 +231,7 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Add metadata fields. These are appended to existing values.
-     * Use <code>clearDC</code> to remove values. The ordering of values
+     * Use <code>clearMetadata</code> to remove values. The ordering of values
      * passed in is maintained.
      *
      * @param context       DSpace context
@@ -272,7 +272,7 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Add a single metadata field. This is appended to existing
-     * values. Use <code>clearDC</code> to remove values.
+     * values. Use <code>clearMetadata</code> to remove values.
      *
      * @param context   DSpace context
      * @param dso       DSpaceObject
@@ -292,7 +292,7 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Add a single metadata field. This is appended to existing
-     * values. Use <code>clearDC</code> to remove values.
+     * values. Use <code>clearMetadata</code> to remove values.
      *
      * @param context    DSpace context
      * @param dso        DSpaceObject
@@ -314,10 +314,10 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Clear metadata values. As with <code>getDC</code> above,
-     * passing in <code>null</code> only matches fields where the qualifier or
+     * passing in <code>null</code> only matches fields where the qualifier orr
      * language is actually <code>null</code>.<code>Item.ANY</code> will
      * match any element, qualifier or language, including <code>null</code>.
-     * Thus, <code>dspaceobject.clearDC(Item.ANY, Item.ANY, Item.ANY)</code> will
+     * Thus, <code>dspaceobject.clearMetadata(Item.ANY, Item.ANY, Item.ANY)</code> will
      * remove all Dublin Core metadata associated with an DSpaceObject.
      *
      * @param context   DSpace context
@@ -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>clearMetadata</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;
 
@@ -385,4 +405,10 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
      * @return a org.dspace.core.Constants that represents a IndexableObject type
      */
     public int getSupportsTypeConstant();
+
+    /**
+     * Trigger the modifiedMetadata variable in DSpaceObject
+     * @param dso   DSpaceObject whose metadata has been modified
+     */
+    public void setMetadataModified(T dso);
 }

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

@@ -71,6 +71,16 @@ public interface MetadataFieldService {
     public MetadataField findByElement(Context context, String metadataSchemaName, String element, String qualifier)
         throws SQLException;
 
+    /**
+     * Separates an mdString in schema, element and qualifier parts, separated by a given separator
+     *      And returns it's matching metadataField if found
+     * @param context       dspace context
+     * @param mdString      String being separated to find corresponding mdField (ex dc.contributor)
+     * @param separator     Separator being used to separate the mdString
+     * @return  Corresponding MetadataField if found
+     */
+    public MetadataField findByString(Context context, String mdString, char separator) throws SQLException;
+
     public List<MetadataField> findFieldsByElementNameUnqualified(Context context, String metadataSchema,
                                                                   String element)
         throws SQLException;

dspace-api/src/main/java/org/dspace/core/Context.java

@@ -361,20 +361,18 @@ public class Context implements AutoCloseable {
         // If Context is no longer open/valid, just note that it has already been closed
         if (!isValid()) {
             log.info("complete() was called on a closed Context object. No changes to commit.");
+            return;
         }
 
         try {
             // As long as we have a valid, writeable database connection,
-            // rollback any changes if we are in read-only mode,
+            // commit changes. Otherwise, we'll just close the DB connection (see below)
-            // otherwise, commit any changes made as part of the transaction
+            if (!isReadOnly()) {
-            if (isReadOnly()) {
-                abort();
-            } else {
                 commit();
             }
         } finally {
             if (dbConnection != null) {
-                // Free the DB connection
+                // Free the DB connection and invalidate the Context
                 dbConnection.closeDBConnection();
                 dbConnection = null;
             }
@@ -395,29 +393,24 @@ public class Context implements AutoCloseable {
         // If Context is no longer open/valid, just note that it has already been closed
         if (!isValid()) {
             log.info("commit() was called on a closed Context object. No changes to commit.");
+            return;
         }
 
         if (isReadOnly()) {
             throw new UnsupportedOperationException("You cannot commit a read-only context");
         }
 
-        // Our DB Connection (Hibernate) will decide if an actual commit is required or not
         try {
-            // As long as we have a valid, writeable database connection,
+            // Dispatch events before committing changes to the database,
-            // commit any changes made as part of the transaction
+            // as the consumers may change something too
-            if (isValid()) {
+            dispatchEvents();
-                // Dispatch events before committing changes to the database,
-                // as the consumers may change something too
-                dispatchEvents();
-            }
-
         } finally {
             if (log.isDebugEnabled()) {
                 log.debug("Cache size on commit is " + getCacheSize());
             }
 
             if (dbConnection != null) {
-                //Commit our changes
+                // Commit our changes (this closes the transaction but leaves database connection open)
                 dbConnection.commit();
                 reloadContextBoundEntities();
             }
@@ -425,8 +418,12 @@ public class Context implements AutoCloseable {
     }
 
 
+    /**
+     * Dispatch any events (cached in current Context) to configured EventListeners (consumers)
+     * in the EventService. This should be called prior to any commit as some consumers may add
+     * to the current transaction. Once events are dispatched, the Context's event cache is cleared.
+     */
     public void dispatchEvents() {
-        // Commit any changes made as part of the transaction
         Dispatcher dispatcher = null;
 
         try {
@@ -462,6 +459,7 @@ public class Context implements AutoCloseable {
 
     /**
      * Add an event to be dispatched when this context is committed.
+     * NOTE: Read-only Contexts cannot add events, as they cannot modify objects.
      *
      * @param event event to be dispatched
      */
@@ -490,6 +488,10 @@ public class Context implements AutoCloseable {
         return events;
     }
 
+    /**
+     * Whether or not the context has events cached.
+     * @return true or false
+     */
     public boolean hasEvents() {
         return !CollectionUtils.isEmpty(events);
     }
@@ -521,22 +523,25 @@ public class Context implements AutoCloseable {
         // If Context is no longer open/valid, just note that it has already been closed
         if (!isValid()) {
             log.info("abort() was called on a closed Context object. No changes to abort.");
+            return;
         }
 
         try {
-            // Rollback ONLY if we have a database connection, and it is NOT Read Only
+            // Rollback ONLY if we have a database transaction, and it is NOT Read Only
-            if (isValid() && !isReadOnly()) {
+            if (!isReadOnly() && isTransactionAlive()) {
                 dbConnection.rollback();
             }
         } catch (SQLException se) {
-            log.error(se.getMessage(), se);
+            log.error("Error rolling back transaction during an abort()", se);
         } finally {
             try {
-                if (!dbConnection.isSessionAlive()) {
+                if (dbConnection != null) {
+                    // Free the DB connection & invalidate the Context
                     dbConnection.closeDBConnection();
+                    dbConnection = null;
                 }
             } catch (Exception ex) {
-                log.error("Exception aborting context", ex);
+                log.error("Error closing the database connection", ex);
             }
             events = null;
         }
@@ -558,7 +563,22 @@ public class Context implements AutoCloseable {
      */
     public boolean isValid() {
         // Only return true if our DB connection is live
-        return dbConnection != null && dbConnection.isTransActionAlive();
+        // NOTE: A transaction need not exist for our Context to be valid, as a Context may use multiple transactions.
+        return dbConnection != null && dbConnection.isSessionAlive();
+    }
+
+    /**
+     * Find out whether our context includes an open database transaction.
+     * Returns <code>true</code> if there is an open transaction. Returns
+     * <code>false</code> if the context is invalid (e.g. abort() or complete())
+     * was called OR no current transaction exists (e.g. commit() was just called
+     * and no new transaction has begun)
+     *
+     * @return
+     */
+    protected boolean isTransactionAlive() {
+        // Only return true if both Context is valid *and* transaction is alive
+        return isValid() && dbConnection.isTransActionAlive();
     }
 
     /**
@@ -571,21 +591,22 @@ public class Context implements AutoCloseable {
         return mode != null && mode == Mode.READ_ONLY;
     }
 
+    /**
+     * Add a group's UUID to the list of special groups cached in Context
+     * @param groupID UUID of group
+     */
     public void setSpecialGroup(UUID groupID) {
         specialGroups.add(groupID);
-
-        // System.out.println("Added " + groupID);
     }
 
     /**
-     * test if member of special group
+     * Test if a group is a special group
      *
      * @param groupID ID of special group to test
      * @return true if member
      */
     public boolean inSpecialGroup(UUID groupID) {
         if (specialGroups.contains(groupID)) {
-            // System.out.println("Contains " + groupID);
             return true;
         }
 
@@ -593,10 +614,9 @@ public class Context implements AutoCloseable {
     }
 
     /**
-     * Get an array of all of the special groups that current user is a member
+     * Get an array of all of the special groups that current user is a member of.
-     * of.
      *
-     * @return list of groups
+     * @return list of special groups
      * @throws SQLException if database error
      */
     public List<Group> getSpecialGroups() throws SQLException {
@@ -608,6 +628,10 @@ public class Context implements AutoCloseable {
         return myGroups;
     }
 
+    /**
+     *  Close the context, aborting any open transactions (if any).
+     * @throws Throwable
+     */
     @Override
     protected void finalize() throws Throwable {
         /*

dspace-api/src/main/java/org/dspace/core/HibernateDBConnection.java

@@ -35,6 +35,23 @@ import org.springframework.orm.hibernate5.SessionFactoryUtils;
 
 /**
  * Hibernate implementation of the DBConnection.
+ * <P>
+ * NOTE: This class does NOT represent a single Hibernate database connection. Instead, it wraps
+ * Hibernate's Session object to obtain access to a database connection in order to execute one or more
+ * transactions.
+ * <P>
+ * Per DSpace's current Hibernate configuration ([dspace]/config/core-hibernate.xml), we use the one-session-per-thread
+ * approach (ThreadLocalSessionContext). This means that Hibernate creates a single Session per thread (request), at the
+ * time when getCurrentSession() is first called.
+ * <P>
+ * This Session may be reused for multiple Transactions, but if commit() is called, any objects (Entities) in
+ * the Session become disconnected and MUST be reloaded into the Session (see reloadEntity() method below).
+ * <P>
+ * If an Error occurs, the Session itself is invalidated. No further Transactions can be run on that Session.
+ * <P>
+ * DSpace generally follows the "Session-per-request" transactional pattern described here:
+ * https://docs.jboss.org/hibernate/orm/5.0/userguide/en-US/html/ch06.html#session-per-request
+ *
  *
  * @author kevinvandevelde at atmire.com
  */
@@ -47,32 +64,61 @@ public class HibernateDBConnection implements DBConnection<Session> {
     private boolean batchModeEnabled = false;
     private boolean readOnlyEnabled = false;
 
+    /**
+     * Retrieves the current Session from Hibernate (per our settings, Hibernate is configured to create one Session
+     * per thread). If Session doesn't yet exist, it is created. A Transaction is also initialized (or reinintialized)
+     * in the Session if one doesn't exist, or was previously closed (e.g. if commit() was previously called)
+     * @return Hibernate current Session object
+     * @throws SQLException
+     */
     @Override
     public Session getSession() throws SQLException {
+        // If we don't yet have a live transaction, start a new one
+        // NOTE: a Session cannot be used until a Transaction is started.
         if (!isTransActionAlive()) {
             sessionFactory.getCurrentSession().beginTransaction();
             configureDatabaseMode();
         }
+        // Return the current Hibernate Session object (Hibernate will create one if it doesn't yet exist)
         return sessionFactory.getCurrentSession();
     }
 
+    /**
+     * Check if the connection has a currently active Transaction. A Transaction is active if it has not yet been
+     * either committed or rolled back.
+     * @return
+     */
     @Override
     public boolean isTransActionAlive() {
         Transaction transaction = getTransaction();
         return transaction != null && transaction.isActive();
     }
 
+    /**
+     * Retrieve the current Hibernate Transaction object from our Hibernate Session.
+     * @return current Transaction (may be active or inactive) or null
+     */
     protected Transaction getTransaction() {
         return sessionFactory.getCurrentSession().getTransaction();
     }
 
+    /**
+     * Check if Hibernate Session is still "alive" / open. An open Session may or may not have an open Transaction
+     * (so isTransactionAlive() may return false even if isSessionAlive() returns true). A Session may be reused for
+     * multiple transactions (e.g. if commit() is called, the Session remains alive while the Transaction is closed)
+     *
+     * @return true if Session is alive, false otherwise
+     */
     @Override
     public boolean isSessionAlive() {
-        return sessionFactory.getCurrentSession() != null && sessionFactory.getCurrentSession()
+        return sessionFactory.getCurrentSession() != null && sessionFactory.getCurrentSession().isOpen();
-                                                                           .getTransaction() != null && sessionFactory
-            .getCurrentSession().getTransaction().getStatus().isOneOf(TransactionStatus.ACTIVE);
     }
 
+    /**
+     * Rollback any changes applied to the current Transaction. This also closes the Transaction. A new Transaction
+     * may be opened the next time getSession() is called.
+     * @throws SQLException
+     */
     @Override
     public void rollback() throws SQLException {
         if (isTransActionAlive()) {
@@ -80,6 +126,14 @@ public class HibernateDBConnection implements DBConnection<Session> {
         }
     }
 
+    /**
+     * Close our current Database connection. This also closes & unbinds the Hibernate Session from our thread.
+     * <P>
+     * NOTE: Because DSpace configures Hibernate to automatically create a Session per thread, a Session may still
+     * exist after this method is called (as Hibernate may automatically create a new Session for the current thread).
+     * However, Hibernate will automatically clean up any existing Session when the thread closes.
+     * @throws SQLException
+     */
     @Override
     public void closeDBConnection() throws SQLException {
         if (sessionFactory.getCurrentSession() != null && sessionFactory.getCurrentSession().isOpen()) {
@@ -87,11 +141,23 @@ public class HibernateDBConnection implements DBConnection<Session> {
         }
     }
 
+    /**
+     * Commits any current changes cached in the Hibernate Session to the database & closes the Transaction.
+     * To open a new Transaction, you may call getSession().
+     * <P>
+     * WARNING: When commit() is called, while the Session is still "alive", all previously loaded objects (entities)
+     * become disconnected from the Session. Therefore, if you continue to use the Session, you MUST reload any needed
+     * objects (entities) using reloadEntity() method.
+     *
+     * @throws SQLException
+     */
     @Override
     public void commit() throws SQLException {
         if (isTransActionAlive() && !getTransaction().getStatus().isOneOf(TransactionStatus.MARKED_ROLLBACK,
                                                                           TransactionStatus.ROLLING_BACK)) {
+            // Flush synchronizes the database with in-memory objects in Session (and frees up that memory)
             getSession().flush();
+            // Commit those results to the database & ends the Transaction
             getTransaction().commit();
         }
     }
@@ -132,6 +198,16 @@ public class HibernateDBConnection implements DBConnection<Session> {
         return getSession().getStatistics().getEntityCount();
     }
 
+    /**
+     * Reload an entity into the Hibernate cache. This can be called after a call to commit() to re-cache an object
+     * in the Hibernate Session (see commit()). Failing to reload objects into the cache may result in a Hibernate
+     * throwing a "LazyInitializationException" if you attempt to use an object that has been disconnected from the
+     * Session cache.
+     * @param entity The DSpace object to reload
+     * @param <E> The class of the entity. The entity must implement the {@link ReloadableEntity} interface.
+     * @return the newly cached object.
+     * @throws SQLException
+     */
     @Override
     @SuppressWarnings("unchecked")
     public <E extends ReloadableEntity> E reloadEntity(final E entity) throws SQLException {
@@ -167,10 +243,13 @@ public class HibernateDBConnection implements DBConnection<Session> {
     }
 
     /**
-     * Evict an entity from the hibernate cache. This is necessary when batch processing a large number of items.
+     * Evict an entity from the hibernate cache.
+     * <P>
+     * When an entity is evicted, it frees up the memory used by that entity in the cache. This is often
+     * necessary when batch processing a large number of objects (to avoid out-of-memory exceptions).
      *
-     * @param entity The entity to reload
+     * @param entity The entity to evict
-     * @param <E>    The class of the enity. The entity must implement the {@link ReloadableEntity} interface.
+     * @param <E>    The class of the entity. The entity must implement the {@link ReloadableEntity} interface.
      * @throws SQLException When reloading the entity from the database fails.
      */
     @Override

dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java

@@ -15,9 +15,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.sql.SQLException;
-import java.util.Arrays;
 import java.util.Iterator;
-import java.util.List;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.Source;
 import javax.xml.transform.TransformerException;
@@ -47,7 +45,6 @@ import org.xmlunit.diff.ComparisonFormatter;
 import org.xmlunit.diff.DefaultComparisonFormatter;
 import org.xmlunit.diff.Diff;
 import org.xmlunit.diff.Difference;
-import org.xmlunit.util.Predicate;
 
 /**
  * Tests of {@link StructBuilder}.
@@ -314,9 +311,9 @@ public class StructBuilderIT
     }
 
     /**
-     * Reject uninteresting nodes.
+     * Reject uninteresting nodes. (currently commented out of tests above)
      */
-    private static class MyNodeFilter implements Predicate<Node> {
+    /*private static class MyNodeFilter implements Predicate<Node> {
         private static final List<String> dontCare = Arrays.asList(
             "description",
             "intro",
@@ -330,5 +327,5 @@ public class StructBuilderIT
             String type = node.getLocalName();
             return ! dontCare.contains(type);
         }
-    }
+    }*/
 }

dspace-api/src/test/java/org/dspace/app/util/GoogleBitstreamComparatorTest.java

@@ -8,7 +8,7 @@
 package org.dspace.app.util;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;

dspace-api/src/test/java/org/dspace/app/util/GoogleMetadataTest.java

@@ -14,7 +14,9 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.sql.SQLException;
+import java.util.List;
 
+import com.google.common.base.Splitter;
 import org.apache.logging.log4j.Logger;
 import org.dspace.AbstractUnitTest;
 import org.dspace.authorize.AuthorizeException;
@@ -85,7 +87,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
             log.error("SQL Error in init", ex);
             fail("SQL Error in init: " + ex.getMessage());
         } catch (IOException e) {
-            e.printStackTrace();
+            log.error("IO Error in init", e);
+            fail("IO Error in init: " + e.getMessage());
         }
     }
 
@@ -119,8 +122,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
         context.restoreAuthSystemState();
         context.commit();
         GoogleMetadata gm = new GoogleMetadata(this.context, it);
-        String[] urlSplitted = gm.getPDFURL().get(0).split("/");
+        List<String> urlSplitted = Splitter.on("/").splitToList(gm.getPDFURL().get(0));
-        assertEquals("Pdf", urlSplitted[urlSplitted.length - 1]);
+        assertEquals("Pdf", urlSplitted.get(urlSplitted.size() - 1));
     }
 
     /**
@@ -154,8 +157,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
         context.restoreAuthSystemState();
         context.commit();
         GoogleMetadata gm = new GoogleMetadata(this.context, it);
-        String[] urlSplitted = gm.getPDFURL().get(0).split("/");
+        List<String> urlSplitted = Splitter.on("/").splitToList(gm.getPDFURL().get(0));
-        assertEquals("size9", urlSplitted[urlSplitted.length - 1]);
+        assertEquals("size9", urlSplitted.get(urlSplitted.size() - 1));
     }
 
     /**
@@ -189,8 +192,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
         context.restoreAuthSystemState();
         context.commit();
         GoogleMetadata gm = new GoogleMetadata(this.context, it);
-        String[] urlSplitted = gm.getPDFURL().get(0).split("/");
+        List<String> urlSplitted = Splitter.on("/").splitToList(gm.getPDFURL().get(0));
-        assertEquals("first", urlSplitted[urlSplitted.length - 1]);
+        assertEquals("first", urlSplitted.get(urlSplitted.size() - 1));
     }
 
     /**
@@ -225,8 +228,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
         context.restoreAuthSystemState();
         context.commit();
         GoogleMetadata gm = new GoogleMetadata(this.context, it);
-        String[] urlSplitted = gm.getPDFURL().get(0).split("/");
+        List<String> urlSplitted = Splitter.on("/").splitToList(gm.getPDFURL().get(0));
-        assertEquals("primary", urlSplitted[urlSplitted.length - 1]);
+        assertEquals("primary", urlSplitted.get(urlSplitted.size() - 1));
     }
 
     /**
@@ -261,8 +264,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
         context.restoreAuthSystemState();
         context.commit();
         GoogleMetadata gm = new GoogleMetadata(this.context, it);
-        String[] urlSplitted = gm.getPDFURL().get(0).split("/");
+        List<String> urlSplitted = Splitter.on("/").splitToList(gm.getPDFURL().get(0));
-        assertEquals("large", urlSplitted[urlSplitted.length - 1]);
+        assertEquals("large", urlSplitted.get(urlSplitted.size() - 1));
     }
 
 
@@ -285,7 +288,7 @@ public class GoogleMetadataTest extends AbstractUnitTest {
     @Test
     public void testGetPDFURLWithNoBitstreams() throws Exception {
         context.turnOffAuthorisationSystem();
-        Bundle bundle = ContentServiceFactory.getInstance().getBundleService().create(context, it, "ORIGINAL");
+        ContentServiceFactory.getInstance().getBundleService().create(context, it, "ORIGINAL");
 
         context.restoreAuthSystemState();
         context.commit();
@@ -319,8 +322,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
         context.restoreAuthSystemState();
         context.commit();
         GoogleMetadata gm = new GoogleMetadata(this.context, it);
-        String[] urlSplitted = gm.getPDFURL().get(0).split("/");
+        List<String> urlSplitted = Splitter.on("/").splitToList(gm.getPDFURL().get(0));
-        assertEquals("small", urlSplitted[urlSplitted.length - 1]);
+        assertEquals("small", urlSplitted.get(urlSplitted.size() - 1));
     }
 
     @After
@@ -334,12 +337,8 @@ public class GoogleMetadataTest extends AbstractUnitTest {
             community = context.reloadEntity(community);
             ContentServiceFactory.getInstance().getCommunityService().delete(context, community);
             community = null;
-        } catch (SQLException e) {
+        } catch (Exception e) {
-            e.printStackTrace();
+            throw new AssertionError("Error occurred in destroy()", e);
-        } catch (AuthorizeException e) {
-            e.printStackTrace();
-        } catch (IOException e) {
-            e.printStackTrace();
         }
         it = null;
         super.destroy();

dspace-api/src/test/java/org/dspace/authenticate/IPMatcherTest.java

@@ -264,8 +264,8 @@ public class IPMatcherTest {
         assertFalse(ipMatcher.match("192.1.2.2"));
     }
 
-
+    // Commented out as this is currently not used in tests
-    private ArrayList<String> getAllIp4Except(ArrayList<String> exceptions) {
+    /*private ArrayList<String> getAllIp4Except(ArrayList<String> exceptions) {
         int d1 = 0;
         int d2 = 0;
         int d3 = 0;
@@ -284,7 +284,7 @@ public class IPMatcherTest {
             }
         }
         return ips;
-    }
+    }*/
 
     private void verifyAllIp4Except(ArrayList<String> exceptions, boolean asserted, IPMatcher ipMatcher)
         throws IPMatcherException {

dspace-api/src/test/java/org/dspace/authorize/AuthorizeServiceTest.java

@@ -90,8 +90,7 @@ public class AuthorizeServiceTest extends AbstractUnitTest {
     @Test
     public void testauthorizeMethodRespectSpecialGroups() {
 
-        EPerson eperson1;
+        EPerson eperson;
-        EPerson eperson2;
         Group group1;
 
         Community dso;
@@ -99,7 +98,7 @@ public class AuthorizeServiceTest extends AbstractUnitTest {
             context.turnOffAuthorisationSystem();
 
             // create an eperson and a group
-            eperson1 = ePersonService.create(context);
+            eperson = ePersonService.create(context);
             group1 = groupService.create(context);
             // A group has to have a name, otherwise there are queries that break
             groupService.setName(group1, "My test group 2");
@@ -111,19 +110,19 @@ public class AuthorizeServiceTest extends AbstractUnitTest {
             // special group to the user. Then test if the action on the DSO
             // is allowed for the user
             authorizeService.addPolicy(context, dso, Constants.ADD, group1);
-            context.setCurrentUser(eperson1);
+            context.setCurrentUser(eperson);
             context.setSpecialGroup(group1.getID());
             context.commit();
         } catch (SQLException | AuthorizeException ex) {
-            throw new RuntimeException(ex);
+            throw new AssertionError(ex);
         } finally {
             context.restoreAuthSystemState();
         }
 
         try {
-            Assert.assertTrue(authorizeService.authorizeActionBoolean(context, eperson1, dso, Constants.ADD, true));
+            Assert.assertTrue(authorizeService.authorizeActionBoolean(context, eperson, dso, Constants.ADD, true));
         } catch (SQLException ex) {
-            throw new RuntimeException(ex);
+            throw new AssertionError(ex);
         }
     }
 }

dspace-api/src/test/java/org/dspace/content/BitstreamFormatTest.java

@@ -232,7 +232,7 @@ public class BitstreamFormatTest extends AbstractUnitTest {
         // Disalow full Admin perms
         when(authorizeServiceSpy.isAdmin(context)).thenReturn(false);
 
-        BitstreamFormat found = bitstreamFormatService.create(context);
+        bitstreamFormatService.create(context);
         fail("Exception should have been thrown");
     }
 

dspace-api/src/test/java/org/dspace/content/BundleTest.java

@@ -363,7 +363,7 @@ public class BundleTest extends AbstractDSpaceObjectTest {
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, b, Constants.ADD);
 
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream bs = bitstreamService.create(context, b, new FileInputStream(f));
+        bitstreamService.create(context, b, new FileInputStream(f));
         fail("Exception should be thrown");
     }
 
@@ -399,7 +399,7 @@ public class BundleTest extends AbstractDSpaceObjectTest {
 
         int assetstore = 0; //default assetstore
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream bs = bitstreamService.register(context, b, assetstore, f.getAbsolutePath());
+        bitstreamService.register(context, b, assetstore, f.getAbsolutePath());
         fail("Exception should be thrown");
     }
 

dspace-api/src/test/java/org/dspace/content/CollectionTest.java

@@ -196,7 +196,7 @@ public class CollectionTest extends AbstractDSpaceObjectTest {
 
         // test creating collection with a specified handle which IS already in use
         // This should throw an exception
-        Collection created = collectionService.create(context, owningCommunity, inUseHandle);
+        collectionService.create(context, owningCommunity, inUseHandle);
         fail("Exception expected");
     }
 
@@ -291,7 +291,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest {
         String itext = "introductory text";
         String copy = "copyright declaration";
         String sidebar = "side bar text";
-        String tempItem = "3";
         String provDesc = "provenance description";
         String license = "license text";
 
@@ -370,7 +369,7 @@ public class CollectionTest extends AbstractDSpaceObjectTest {
     @Test(expected = AuthorizeException.class)
     public void testSetLogoNoAuth() throws Exception {
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream logo = collectionService.setLogo(context, collection, new FileInputStream(f));
+        collectionService.setLogo(context, collection, new FileInputStream(f));
         fail("Exception expected");
     }
 
@@ -393,7 +392,7 @@ public class CollectionTest extends AbstractDSpaceObjectTest {
     @Test(expected = AuthorizeException.class)
     public void testCreateWorkflowGroupNoAuth() throws Exception {
         int step = 1;
-        Group result = collectionService.createWorkflowGroup(context, collection, step);
+        collectionService.createWorkflowGroup(context, collection, step);
         fail("Exception expected");
     }
 
@@ -461,7 +460,7 @@ public class CollectionTest extends AbstractDSpaceObjectTest {
      */
     @Test(expected = AuthorizeException.class)
     public void testCreateSubmittersNoAuth() throws Exception {
-        Group result = collectionService.createSubmitters(context, collection);
+        collectionService.createSubmitters(context, collection);
         fail("Exception expected");
     }
 
@@ -511,7 +510,7 @@ public class CollectionTest extends AbstractDSpaceObjectTest {
      */
     @Test(expected = AuthorizeException.class)
     public void testCreateAdministratorsNoAuth() throws Exception {
-        Group result = collectionService.createAdministrators(context, collection);
+        collectionService.createAdministrators(context, collection);
         fail("Exception expected");
     }
 

dspace-api/src/test/java/org/dspace/content/CommunityTest.java

@@ -195,7 +195,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
 
         // test creating community with no parent (as a non-admin)
         // this should throw an exception
-        Community created = communityService.create(null, context);
+        communityService.create(null, context);
         fail("Exception expected");
     }
 
@@ -230,7 +230,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
 
         // test creating community with a specified handle which IS already in use
         // This should throw an exception
-        Community created = communityService.create(null, context, inUseHandle);
+        communityService.create(null, context, inUseHandle);
         fail("Exception expected");
     }
 
@@ -378,7 +378,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, c, Constants.WRITE);
 
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream logo = communityService.setLogo(context, c, new FileInputStream(f));
+        communityService.setLogo(context, c, new FileInputStream(f));
         fail("Exception expected");
     }
 
@@ -425,7 +425,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
      */
     @Test(expected = AuthorizeException.class)
     public void testCreateAdministratorsNoAuth() throws Exception {
-        Group result = communityService.createAdministrators(context, c);
+        communityService.createAdministrators(context, c);
         fail("Exception should have been thrown");
     }
 
@@ -617,7 +617,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         // Disallow current Community ADD perms
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, c, Constants.ADD);
 
-        Collection result = collectionService.create(context, c);
+        collectionService.create(context, c);
         fail("Exception expected");
     }
 
@@ -672,7 +672,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         // Disallow current Community ADD perms
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, c, Constants.ADD);
 
-        Community result = communityService.createSubcommunity(context, c);
+        communityService.createSubcommunity(context, c);
         fail("Exception expected");
     }
 
@@ -872,8 +872,9 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         Collection grandchildCol = collectionService.create(context, grandchild);
         // Create two separate items
         WorkspaceItem wsItem = workspaceItemService.create(context, childCol, false);
-        wsItem = workspaceItemService.create(context, childCol, false);
         Item item = installItemService.installItem(context, wsItem);
+        wsItem = workspaceItemService.create(context, grandchildCol, false);
+        Item item2 = installItemService.installItem(context, wsItem);
 
         // Done creating the objects. Turn auth system back on
         context.restoreAuthSystemState();
@@ -885,6 +886,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         UUID childColId = childCol.getID();
         UUID grandchildColId = grandchildCol.getID();
         UUID itemId = item.getID();
+        UUID item2Id = item2.getID();
 
         // Delete the parent of this entire hierarchy
         communityService.delete(context, parent);
@@ -902,6 +904,8 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
                    collectionService.find(context, grandchildColId), nullValue());
         assertThat("Item not deleted",
                    itemService.find(context, itemId), nullValue());
+        assertThat("Item not deleted",
+                   itemService.find(context, item2Id), nullValue());
     }
 
     /**
@@ -1030,7 +1034,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
             assertThat("testGetParentObject 1", communityService.getParentObject(context, son), notNullValue());
             assertThat("testGetParentObject 2", (Community) communityService.getParentObject(context, son), equalTo(c));
         } catch (AuthorizeException ex) {
-            fail("Authorize exception catched");
+            throw new AssertionError("AuthorizeException occurred", ex);
         }
     }
 

dspace-api/src/test/java/org/dspace/content/DCDateTest.java

@@ -19,7 +19,6 @@ import java.util.Locale;
 import java.util.TimeZone;
 
 import org.apache.commons.lang3.time.DateUtils;
-import org.apache.logging.log4j.Logger;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -28,11 +27,6 @@ import org.junit.Test;
  * @author pvillega
  */
 public class DCDateTest {
-    /**
-     * log4j category
-     */
-    private static Logger log = org.apache.logging.log4j.LogManager.getLogger(DCDateTest.class);
-
     /**
      * Object to use in the tests
      */

dspace-api/src/test/java/org/dspace/content/EntityServiceImplTest.java

@@ -8,16 +8,14 @@
 package org.dspace.content;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
 
-import org.dspace.content.dao.RelationshipTypeDAO;
 import org.dspace.content.service.EntityTypeService;
 import org.dspace.content.service.ItemService;
 import org.dspace.content.service.RelationshipService;
@@ -60,10 +58,8 @@ public class EntityServiceImplTest  {
     public void testfindByItemId() throws Exception {
         // Declare objects utilized in unit test
         Item item = mock(Item.class);
-        List<Relationship> relationshipList = new ArrayList<>();
         Relationship relationship = mock(Relationship.class);
         relationship.setId(9);
-        relationshipList.add(relationship);
 
         // Mock the state of objects utilized in findByItemId() to meet the success criteria of an invocation
         when(itemService.find(any(), any())).thenReturn(item);
@@ -79,10 +75,6 @@ public class EntityServiceImplTest  {
         // Declare objects utilized in unit test
         Entity entity = mock(Entity.class);
         EntityTypeService entityTypeService = mock(EntityTypeService.class);
-        Item item = mock(Item.class);
-        List<MetadataValue> list = new ArrayList<>();
-        MetadataValue metadataValue = mock(MetadataValue.class);
-        list.add(metadataValue);
         EntityType entityType = entityTypeService.findByEntityType(context, "testType");
 
         // The returned EntityType should equal our defined entityType case
@@ -151,11 +143,7 @@ public class EntityServiceImplTest  {
     @Test
     public void testGetAllRelationshipTypes() throws Exception {
         // Declare objects utilized for this test
-        List<MetadataValue> list = new ArrayList<>();
-        MetadataValue metadataValue = mock(MetadataValue.class);
-        list.add(metadataValue);
         Item item = mock(Item.class);
-        RelationshipTypeDAO relationshipTypeDAO = mock(RelationshipTypeDAO.class);
         Entity entity = mock(Entity.class);
         RelationshipType relationshipType = mock(RelationshipType.class);
         relationshipType.setLeftType(leftType);
@@ -185,7 +173,7 @@ public class EntityServiceImplTest  {
         RelationshipType relationshipType = mock(RelationshipType.class);
 
         // Currently this unit test will only test one case with one relationshipType
-        List<RelationshipType> relationshipTypeList = new LinkedList<>();
+        List<RelationshipType> relationshipTypeList = new ArrayList<>();
         relationshipTypeList.add(relationshipType);
         List<MetadataValue> metsList = new ArrayList<>();
         MetadataValue metadataValue = mock(MetadataValue.class);
@@ -213,7 +201,7 @@ public class EntityServiceImplTest  {
         RelationshipType relationshipType = mock(RelationshipType.class);
 
         // Currently this unit test will only test one case with one relationshipType
-        List<RelationshipType> relationshipTypeList = new LinkedList<>();
+        List<RelationshipType> relationshipTypeList = new ArrayList<>();
         relationshipTypeList.add(relationshipType);
         List<MetadataValue> metsList = new ArrayList<>();
         MetadataValue metadataValue = mock(MetadataValue.class);
@@ -236,7 +224,7 @@ public class EntityServiceImplTest  {
     @Test
     public void testGetRelationshipTypesByTypeName() throws Exception {
         // Declare objects utilized in unit test
-        List<RelationshipType> list = new LinkedList<>();
+        List<RelationshipType> list = new ArrayList<>();
         RelationshipType relationshipType = mock(RelationshipType.class);
         list.add(relationshipType);
 

dspace-api/src/test/java/org/dspace/content/EntityTypeServiceImplTest.java

@@ -8,7 +8,7 @@
 package org.dspace.content;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.when;
 

dspace-api/src/test/java/org/dspace/content/FormatIdentifierTest.java

@@ -14,7 +14,6 @@ import static org.junit.Assert.assertThat;
 import java.io.File;
 import java.io.FileInputStream;
 
-import org.apache.logging.log4j.Logger;
 import org.dspace.AbstractUnitTest;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.BitstreamFormatService;
@@ -30,11 +29,6 @@ import org.junit.Test;
  */
 public class FormatIdentifierTest extends AbstractUnitTest {
 
-    /**
-     * log4j category
-     */
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(FormatIdentifierTest.class);
-
     protected BitstreamService bitstreamService = ContentServiceFactory.getInstance().getBitstreamService();
     protected BitstreamFormatService bitstreamFormatService = ContentServiceFactory.getInstance()
                                                                                    .getBitstreamFormatService();
@@ -71,8 +65,8 @@ public class FormatIdentifierTest extends AbstractUnitTest {
     @Test
     public void testGuessFormat() throws Exception {
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream bs = null;
+        Bitstream bs;
-        BitstreamFormat result = null;
+        BitstreamFormat result;
         BitstreamFormat pdf = bitstreamFormatService.findByShortDescription(context, "Adobe PDF");
 
         //test null filename

dspace-api/src/test/java/org/dspace/content/ITCommunityCollection.java

@@ -19,7 +19,6 @@ import java.io.IOException;
 import java.sql.SQLException;
 import java.util.UUID;
 
-import org.apache.logging.log4j.Logger;
 import org.dspace.AbstractIntegrationTest;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.factory.ContentServiceFactory;
@@ -48,11 +47,6 @@ import org.junit.Test;
  * @author tdonohue
  */
 public class ITCommunityCollection extends AbstractIntegrationTest {
-    /**
-     * log4j category
-     */
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(ITCommunityCollection.class);
-
     protected CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService();
     protected CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService();
     protected ItemService itemService = ContentServiceFactory.getInstance().getItemService();
@@ -107,8 +101,8 @@ public class ITCommunityCollection extends AbstractIntegrationTest {
         //verify it works as expected
         assertThat("testCreateTree 0", parent.getParentCommunities().size(), is(0));
         assertThat("testCreateTree 1", child1.getParentCommunities().get(0), equalTo(parent));
-        assertThat("testCreateTree 2", (Community) collectionService.getParentObject(context, col1), equalTo(child1));
+        assertThat("testCreateTree 2", collectionService.getParentObject(context, col1), equalTo(child1));
-        assertThat("testCreateTree 3", (Community) collectionService.getParentObject(context, col2), equalTo(child1));
+        assertThat("testCreateTree 3", collectionService.getParentObject(context, col2), equalTo(child1));
 
         context.turnOffAuthorisationSystem();
         communityService.delete(context, parent);
@@ -133,8 +127,8 @@ public class ITCommunityCollection extends AbstractIntegrationTest {
         context.restoreAuthSystemState();
 
         //verify it works as expected
-        assertThat("testCreateItems 0", (Collection) itemService.getParentObject(context, item1), equalTo(col1));
+        assertThat("testCreateItems 0", itemService.getParentObject(context, item1), equalTo(col1));
-        assertThat("testCreateItems 1", (Collection) itemService.getParentObject(context, item2), equalTo(col2));
+        assertThat("testCreateItems 1", itemService.getParentObject(context, item2), equalTo(col2));
 
         context.turnOffAuthorisationSystem();
         communityService.delete(context, parent);
@@ -158,8 +152,8 @@ public class ITCommunityCollection extends AbstractIntegrationTest {
 
         // Add same number of items to each collection
         for (int count = 0; count < items_per_collection; count++) {
-            Item item1 = installItemService.installItem(context, workspaceItemService.create(context, col1, false));
+            installItemService.installItem(context, workspaceItemService.create(context, col1, false));
-            Item item2 = installItemService.installItem(context, workspaceItemService.create(context, col2, false));
+            installItemService.installItem(context, workspaceItemService.create(context, col2, false));
         }
 
         // Finally, let's throw in a small wrench and add a mapped item
@@ -229,7 +223,6 @@ public class ITCommunityCollection extends AbstractIntegrationTest {
         context.setCurrentUser(commAdmin);
 
         // Test deletion of single Bitstream as a Community Admin (delete just flags as deleted)
-        UUID bitstreamId = bitstream.getID();
         bitstreamService.delete(context, bitstream);
         assertTrue("Community Admin unable to flag Bitstream as deleted",
                    bitstream.isDeleted());
@@ -312,7 +305,6 @@ public class ITCommunityCollection extends AbstractIntegrationTest {
         context.setCurrentUser(collAdmin);
 
         // Test deletion of single Bitstream as a Collection Admin (delete just flags as deleted)
-        UUID bitstreamId = bitstream2.getID();
         bitstreamService.delete(context, bitstream2);
         assertTrue("Collection Admin unable to flag Bitstream as deleted",
                    bitstream2.isDeleted());
@@ -327,7 +319,6 @@ public class ITCommunityCollection extends AbstractIntegrationTest {
         // Test deletion of single Item as a Collection Admin
         UUID itemId = item.getID();
         bundleId = bundle.getID();
-        bitstreamId = bitstream.getID();
         itemService.delete(context, item);
         assertThat("Collection Admin unable to delete sub-Item",
                    itemService.find(context, itemId), nullValue());

dspace-api/src/test/java/org/dspace/content/ITMetadata.java

@@ -16,7 +16,6 @@ import java.io.IOException;
 import java.sql.SQLException;
 import java.util.List;
 
-import org.apache.logging.log4j.Logger;
 import org.dspace.AbstractIntegrationTest;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.factory.ContentServiceFactory;
@@ -37,11 +36,6 @@ import org.junit.Test;
  * @author pvillega
  */
 public class ITMetadata extends AbstractIntegrationTest {
-    /**
-     * log4j category
-     */
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(ITMetadata.class);
-
 
     protected CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService();
     protected CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService();

dspace-api/src/test/java/org/dspace/content/InProgressSubmissionTest.java

@@ -7,7 +7,6 @@
  */
 package org.dspace.content;
 
-import org.apache.logging.log4j.Logger;
 import org.dspace.AbstractUnitTest;
 import org.junit.After;
 import org.junit.Before;
@@ -23,11 +22,6 @@ import org.junit.Test;
  */
 public class InProgressSubmissionTest extends AbstractUnitTest {
 
-    /**
-     * log4j category
-     */
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(InProgressSubmissionTest.class);
-
     /**
      * This method will be run before every test as per @Before. It will
      * initialize resources required for the tests.

dspace-api/src/test/java/org/dspace/content/ItemComparatorTest.java

@@ -132,8 +132,8 @@ public class ItemComparatorTest extends AbstractUnitTest {
      */
     @Test
     public void testCompare() throws SQLException {
-        int result = 0;
+        int result;
-        ItemComparator ic = null;
+        ItemComparator ic;
 
         //one of the tiems has no value
         ic = new ItemComparator("test", "one", Item.ANY, true);
@@ -246,7 +246,7 @@ public class ItemComparatorTest extends AbstractUnitTest {
     @SuppressWarnings( {"ObjectEqualsNull", "IncompatibleEquals"})
     public void testEquals() {
         ItemComparator ic = new ItemComparator("test", "one", Item.ANY, true);
-        ItemComparator target = null;
+        ItemComparator target;
 
         assertFalse("testEquals 0", ic.equals(null));
         assertFalse("testEquals 1", ic.equals("test one"));

dspace-api/src/test/java/org/dspace/content/ItemTest.java

@@ -41,7 +41,6 @@ import org.dspace.authorize.factory.AuthorizeServiceFactory;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.BitstreamFormatService;
-import org.dspace.content.service.CollectionService;
 import org.dspace.content.service.MetadataFieldService;
 import org.dspace.content.service.MetadataSchemaService;
 import org.dspace.core.Constants;
@@ -76,8 +75,6 @@ public class ItemTest extends AbstractDSpaceObjectTest {
                                                                                  .getBitstreamFormatService();
     private MetadataFieldService metadataFieldService = ContentServiceFactory.getInstance().getMetadataFieldService();
 
-    private CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService();
-
     private Collection collection;
     private Community owningCommunity;
 
@@ -839,7 +836,7 @@ public class ItemTest extends AbstractDSpaceObjectTest {
     @Test(expected = AuthorizeException.class)
     public void testCreateBundleNoAuth() throws Exception {
         String name = "bundle";
-        Bundle created = bundleService.create(context, it, name);
+        bundleService.create(context, it, name);
         fail("Exception expected");
     }
 
@@ -941,7 +938,7 @@ public class ItemTest extends AbstractDSpaceObjectTest {
     public void testCreateSingleBitstream_InputStream_StringNoAuth() throws Exception {
         String name = "new bundle";
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream result = itemService.createSingleBitstream(context, new FileInputStream(f), it, name);
+        itemService.createSingleBitstream(context, new FileInputStream(f), it, name);
         fail("Exception expected");
     }
 
@@ -972,7 +969,7 @@ public class ItemTest extends AbstractDSpaceObjectTest {
     @Test(expected = AuthorizeException.class)
     public void testCreateSingleBitstream_InputStreamNoAuth() throws Exception {
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream result = itemService.createSingleBitstream(context, new FileInputStream(f), it);
+        itemService.createSingleBitstream(context, new FileInputStream(f), it);
         fail("Expected exception");
     }
 
@@ -1624,7 +1621,7 @@ public class ItemTest extends AbstractDSpaceObjectTest {
             assertThat("testGetParentObject 1", itemService.getParentObject(context, it), notNullValue());
             assertThat("testGetParentObject 2", (Collection) itemService.getParentObject(context, it), equalTo(parent));
         } catch (AuthorizeException ex) {
-            fail("Authorize exception catched");
+            throw new AssertionError("Authorize Exception occurred", ex);
         }
     }
 

dspace-api/src/test/java/org/dspace/content/LicenseUtilsTest.java

@@ -104,11 +104,11 @@ public class LicenseUtilsTest extends AbstractUnitTest {
     @Test
     public void testGetLicenseText_5args() throws SQLException, AuthorizeException, IOException {
         //parameters for the test
-        Locale locale = null;
+        Locale locale;
-        Collection collection = null;
+        Collection collection;
-        Item item = null;
+        Item item;
-        EPerson person = null;
+        EPerson person;
-        Map<String, Object> additionalInfo = null;
+        Map<String, Object> additionalInfo;
 
         // We don't test attribute 4 as this is the date, and the date often differs between when the test
         // is executed, and when the LicenceUtils code gets the current date/time which causes the test to fail
@@ -195,10 +195,10 @@ public class LicenseUtilsTest extends AbstractUnitTest {
     @Test
     public void testGetLicenseText_4args() throws SQLException, AuthorizeException, IOException {
         //parameters for the test
-        Locale locale = null;
+        Locale locale;
-        Collection collection = null;
+        Collection collection;
-        Item item = null;
+        Item item;
-        EPerson person = null;
+        EPerson person;
 
         String template = "Template license: %1$s %2$s %3$s %5$s %6$s";
         String templateResult = "Template license: first name last name testgetlicensetext_4args@email.com  ";

dspace-api/src/test/java/org/dspace/content/NonUniqueMetadataExceptionTest.java

@@ -9,8 +9,6 @@ package org.dspace.content;
 
 import static org.junit.Assert.assertTrue;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 import org.junit.Test;
 
 /**
@@ -21,12 +19,6 @@ import org.junit.Test;
  */
 public class NonUniqueMetadataExceptionTest {
 
-    /**
-     * log4j category
-     */
-    private static final Logger log = LogManager
-            .getLogger(NonUniqueMetadataExceptionTest.class);
-
     /**
      * Dummy test to avoid initialization errors
      */

dspace-api/src/test/java/org/dspace/content/RelationshipMetadataServiceTest.java

@@ -463,8 +463,8 @@ public class RelationshipMetadataServiceTest extends AbstractUnitTest {
         WorkspaceItem is = workspaceItemService.create(context, col, false);
         Item secondItem = installItemService.installItem(context, is);
         itemService.addMetadata(context, secondItem, "relationship", "type", null, null, "Publication");
-        Relationship secondRelationship = relationshipService.create(context, secondItem, rightItem,
+        relationshipService.create(context, secondItem, rightItem,
-                                                                     isAuthorOfPublicationRelationshipType, 0, 0);
+                                   isAuthorOfPublicationRelationshipType, 0, 0);
         context.restoreAuthSystemState();
 
         assertThat(relationshipService.findNextRightPlaceByRightItem(context, rightItem), equalTo(2));
@@ -489,8 +489,8 @@ public class RelationshipMetadataServiceTest extends AbstractUnitTest {
         itemService.addMetadata(context, secondAuthor, "relationship", "type", null, null, "Author");
         itemService.addMetadata(context, secondAuthor, "person", "familyName", null, null, "familyName");
         itemService.addMetadata(context, secondAuthor, "person", "givenName", null, null, "firstName");
-        Relationship secondRelationship = relationshipService.create(context, leftItem, secondAuthor,
+        relationshipService.create(context, leftItem, secondAuthor,
-                                                                     isAuthorOfPublicationRelationshipType, 0, 0);
+                                   isAuthorOfPublicationRelationshipType, 0, 0);
         context.restoreAuthSystemState();
 
         assertThat(relationshipService.findNextLeftPlaceByLeftItem(context, leftItem), equalTo(2));

dspace-api/src/test/java/org/dspace/content/RelationshipServiceImplTest.java

@@ -8,12 +8,11 @@
 package org.dspace.content;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 
 import org.dspace.authorize.service.AuthorizeService;
@@ -137,7 +136,7 @@ public class RelationshipServiceImplTest {
     @Test
     public void testFindByItemAndRelationshipType() throws Exception {
         // Declare objects utilized in unit test
-        List<Relationship> relList = new LinkedList<>();
+        List<Relationship> relList = new ArrayList<>();
         Item item = mock(Item.class);
         RelationshipType testRel = new RelationshipType();
 
@@ -152,7 +151,7 @@ public class RelationshipServiceImplTest {
     @Test
     public void testFindByRelationshipType() throws Exception {
         // Declare objects utilized in unit test
-        List<Relationship> relList = new LinkedList<>();
+        List<Relationship> relList = new ArrayList<>();
         RelationshipType testRel = new RelationshipType();
 
         // The Relationship(s) reported should match our our relList
@@ -231,8 +230,6 @@ public class RelationshipServiceImplTest {
     public void testDelete() throws Exception {
 
         // Declare objects utilized in unit test
-        MetadataValue metVal = mock(MetadataValue.class);
-        List<MetadataValue> metsList = new ArrayList<>();
         List<Relationship> leftTypelist = new ArrayList<>();
         List<Relationship> rightTypelist = new ArrayList<>();
         Item leftItem = mock(Item.class);
@@ -246,7 +243,6 @@ public class RelationshipServiceImplTest {
         testRel.setRightwardType("Entitylabel");
         testRel.setLeftMinCardinality(0);
         testRel.setRightMinCardinality(0);
-        metsList.add(metVal);
         relationship = getRelationship(leftItem, rightItem, testRel, 0,0);
         leftTypelist.add(relationship);
         rightTypelist.add(relationship);

dspace-api/src/test/java/org/dspace/content/RelationshipTypeTest.java

@@ -10,15 +10,14 @@ package org.dspace.content;
 import static org.hamcrest.core.IsEqual.equalTo;
 import static org.hamcrest.core.IsNull.notNullValue;
 import static org.junit.Assert.assertThat;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Random;
 
-import org.apache.logging.log4j.Logger;
 import org.dspace.content.dao.RelationshipTypeDAO;
 import org.dspace.core.Context;
 import org.junit.Before;
@@ -30,9 +29,6 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 @RunWith(MockitoJUnitRunner.class)
 public class RelationshipTypeTest {
-
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(RelationshipTypeTest.class);
-
     @InjectMocks
     private RelationshipTypeServiceImpl relationshipTypeService;
 
@@ -102,7 +98,7 @@ public class RelationshipTypeTest {
     @Test
     public void testRelationshipTypeFindAll() throws Exception {
         // Declare objects utilized for this test
-        List<RelationshipType> mockedList = new LinkedList<>();
+        List<RelationshipType> mockedList = new ArrayList<>();
         mockedList.add(firstRelationshipType);
         mockedList.add(secondRelationshipType);
 
@@ -120,7 +116,7 @@ public class RelationshipTypeTest {
     @Test
     public void testRelationshipTypeFindByLeftOrRightwardType() throws Exception {
         // Declare objects utilized for this test
-        List<RelationshipType> mockedList = new LinkedList<>();
+        List<RelationshipType> mockedList = new ArrayList<>();
         mockedList.add(firstRelationshipType);
 
         // Mock DAO to return our mockedList
@@ -138,7 +134,7 @@ public class RelationshipTypeTest {
     @Test
     public void testRelationshipTypefindByEntityType() throws Exception {
         // Declare objects utilized for this test
-        List<RelationshipType> mockedList = new LinkedList<>();
+        List<RelationshipType> mockedList = new ArrayList<>();
         mockedList.add(firstRelationshipType);
 
         // Mock DAO to return our mockedList

dspace-api/src/test/java/org/dspace/content/SiteTest.java

@@ -58,7 +58,7 @@ public class SiteTest extends AbstractUnitTest {
         try {
             //we have to create a new community in the database
             context.turnOffAuthorisationSystem();
-            this.s = (Site) siteService.findSite(context);
+            this.s = siteService.findSite(context);
             //we need to commit the changes so we don't block the table for testing
             context.restoreAuthSystemState();
         } catch (SQLException ex) {
@@ -120,8 +120,7 @@ public class SiteTest extends AbstractUnitTest {
      */
     @Test
     public void testSiteFind() throws Exception {
-        int id = 0;
+        Site found = siteService.findSite(context);
-        Site found = (Site) siteService.findSite(context);
         assertThat("testSiteFind 0", found, notNullValue());
         assertThat("testSiteFind 1", found, equalTo(s));
     }

dspace-api/src/test/java/org/dspace/content/ThumbnailTest.java

@@ -7,6 +7,7 @@
  */
 package org.dspace.content;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -48,11 +49,6 @@ public class ThumbnailTest extends AbstractUnitTest {
      */
     private Bitstream orig;
 
-    /**
-     * Thumbnail instance for the tests, original copy
-     */
-    private Thumbnail t;
-
     /**
      * This method will be run before every test as per @Before. It will
      * initialize resources required for the tests.
@@ -69,7 +65,9 @@ public class ThumbnailTest extends AbstractUnitTest {
             File f = new File(testProps.get("test.bitstream").toString());
             thumb = bitstreamService.create(context, new FileInputStream(f));
             orig = bitstreamService.create(context, new FileInputStream(f));
-            t = new Thumbnail(thumb, orig);
+            Thumbnail t = new Thumbnail(thumb, orig);
+            assertEquals(orig, t.getOriginal());
+            assertEquals(thumb, t.getThumb());
         } catch (IOException ex) {
             log.error("IO Error in init", ex);
             fail("SQL Error in init: " + ex.getMessage());
@@ -89,9 +87,16 @@ public class ThumbnailTest extends AbstractUnitTest {
     @After
     @Override
     public void destroy() {
-        thumb = null;
+        try {
-        orig = null;
+            context.turnOffAuthorisationSystem();
-        t = null;
+            bitstreamService.delete(context, thumb);
+            bitstreamService.delete(context, orig);
+            context.restoreAuthSystemState();
+            thumb = null;
+            orig = null;
+        } catch (Exception e) {
+            throw new AssertionError("Error in destroy()", e);
+        }
         super.destroy();
     }
 

dspace-api/src/test/java/org/dspace/content/VersioningTest.java

@@ -25,7 +25,6 @@ import org.dspace.content.service.CommunityService;
 import org.dspace.content.service.InstallItemService;
 import org.dspace.content.service.ItemService;
 import org.dspace.content.service.WorkspaceItemService;
-import org.dspace.core.ConfigurationManager;
 import org.dspace.handle.factory.HandleServiceFactory;
 import org.dspace.handle.service.HandleService;
 import org.dspace.versioning.Version;
@@ -49,7 +48,6 @@ public class VersioningTest extends AbstractUnitTest {
 
     private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(VersioningTest.class);
 
-    private String originalHandle;
     private Item originalItem;
     private Item versionedItem;
     private String summary = "Unit test version";
@@ -65,7 +63,7 @@ public class VersioningTest extends AbstractUnitTest {
 
     //A regex that can be used to see if a handle contains the format of handle created by the org.dspace.identifier
     // .VersionedHandleIdentifierProvider*
-    protected String versionedHandleRegex = ConfigurationManager.getProperty("handle.prefix") + "\\/[0-9]*\\.[0-9]";
+    //protected String versionedHandleRegex = ConfigurationManager.getProperty("handle.prefix") + "\\/[0-9]*\\.[0-9]";
 
     /**
      * This method will be run before every test as per @Before. It will
@@ -86,7 +84,6 @@ public class VersioningTest extends AbstractUnitTest {
             WorkspaceItem is = workspaceItemService.create(context, col, false);
 
             originalItem = installItemService.installItem(context, is);
-            originalHandle = originalItem.getHandle();
 
             Version version = versionService.createNewVersion(context, originalItem, summary);
             WorkspaceItem wsi = workspaceItemService.findByItem(context, version.getItem());

dspace-api/src/test/java/org/dspace/content/WorkspaceItemTest.java

@@ -160,8 +160,8 @@ public class WorkspaceItemTest extends AbstractUnitTest {
         // Allow Collection ADD perms
         doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADD);
 
-        boolean template = false;
+        boolean template;
-        WorkspaceItem created = null;
+        WorkspaceItem created;
 
         template = false;
         created = workspaceItemService.create(context, collection, template);

dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java

@@ -22,6 +22,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 
+import com.google.common.base.Splitter;
 import org.apache.logging.log4j.Logger;
 import org.dspace.AbstractIntegrationTest;
 import org.dspace.authorize.AuthorizeException;
@@ -1194,9 +1195,9 @@ public class ITDSpaceAIP extends AbstractIntegrationTest {
 
             // Get the typeText & name of this object from the values
             String info = infoMap.get(key);
-            String[] values = info.split(valueseparator);
+            List<String> values = Splitter.on(valueseparator).splitToList(info);
-            String typeText = values[0];
+            String typeText = values.get(0);
-            String name = values[1];
+            String name = values.get(1);
 
             // Also assert type and name are correct
             assertEquals("assertObjectsExist object " + key + " type",

dspace-api/src/test/java/org/dspace/content/virtual/CollectedTest.java

@@ -14,6 +14,7 @@ import static org.mockito.Mockito.when;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.google.common.base.Splitter;
 import org.dspace.content.Item;
 import org.dspace.content.MetadataValue;
 import org.dspace.content.service.ItemService;
@@ -85,18 +86,15 @@ public class CollectedTest {
         metadataValueList.add(metadataValue);
         String s = "dc.title";
         list.add(s);
-        String[] splittedString = s.split("\\.");
+        List<String> splittedString = Splitter.on('.').splitToList(s);
         collected.setFields(list);
         valueList.add("TestValue");
 
         // Mock the state of objects utilized in getValues() to meet the success criteria of an invocation
-        when(itemService.getMetadata(item, splittedString.length > 0 ? splittedString[0] :
+        when(itemService.getMetadata(item, splittedString.size() > 0 ? splittedString.get(0) : null,
-                        null,
+                                     splittedString.size() > 1 ? splittedString.get(1) : null,
-                splittedString.length > 1 ? splittedString[1] :
+                                     splittedString.size() > 2 ? splittedString.get(2) : null,
-                        null,
+                                     Item.ANY, false)).thenReturn(metadataValueList);
-                splittedString.length > 2 ? splittedString[2] :
-                        null,
-                Item.ANY, false)).thenReturn(metadataValueList);
         when(metadataValue.getValue()).thenReturn("TestValue");
 
         // The reported value(s) should match our valueList

dspace-api/src/test/java/org/dspace/content/virtual/ConcatenateTest.java

@@ -14,6 +14,7 @@ import static org.mockito.Mockito.when;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.google.common.base.Splitter;
 import org.dspace.content.Item;
 import org.dspace.content.MetadataValue;
 import org.dspace.content.service.ItemService;
@@ -107,18 +108,15 @@ public class ConcatenateTest {
         metadataValueList.add(metadataValue);
         String s = "dc.title";
         list.add(s);
-        String[] splittedString = s.split("\\.");
+        List<String> splittedString = Splitter.on(".").splitToList(s);
         concatenate.setFields(list);
         valueList.add("TestValue");
 
         // Mock the state of objects utilized in getValues() to meet the success criteria of an invocation
-        when(itemService.getMetadata(item, splittedString.length > 0 ? splittedString[0] :
+        when(itemService.getMetadata(item, splittedString.size() > 0 ? splittedString.get(0) : null,
-                        null,
+                                     splittedString.size() > 1 ? splittedString.get(1) : null,
-                splittedString.length > 1 ? splittedString[1] :
+                                     splittedString.size() > 2 ? splittedString.get(2) : null,
-                        null,
+                                     Item.ANY, false)).thenReturn(metadataValueList);
-                splittedString.length > 2 ? splittedString[2] :
-                        null,
-                Item.ANY, false)).thenReturn(metadataValueList);
         when(metadataValue.getValue()).thenReturn("TestValue");
 
 

dspace-api/src/test/java/org/dspace/content/virtual/RelatedTest.java

@@ -13,7 +13,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
 
@@ -150,8 +149,8 @@ public class RelatedTest {
         assertEquals("TestGetValues 1", virtualMetadataConfiguration.getValues(context, item),
                 related.getValues(context, item));
         related.setPlace(2);
-        // No match should return empty LinkedList
+        // No match should return empty List
-        assertEquals("TestGetValues 2", new LinkedList<>(), related.getValues(context, item));
+        assertEquals("TestGetValues 2", new ArrayList<>(), related.getValues(context, item));
     }
 
 

dspace-api/src/test/java/org/dspace/content/virtual/UUIDValueTest.java

@@ -11,7 +11,7 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
 
@@ -28,7 +28,7 @@ import org.mockito.junit.MockitoJUnitRunner;
 public class UUIDValueTest {
 
     @InjectMocks
-    private UUIDValue UUIDValue;
+    private UUIDValue uuidValue;
 
     @Mock
     private Context context;
@@ -36,32 +36,32 @@ public class UUIDValueTest {
     @Test
     public void testGetValues() throws Exception {
         // Setup objects utilized in unit test
-        List<String> list = new LinkedList<>();
+        List<String> list = new ArrayList<>();
         Item item = mock(Item.class);
         UUID uuid = UUID.randomUUID();
         when(item.getID()).thenReturn(uuid);
         list.add(String.valueOf(uuid));
 
         // The reported value(s) should match our defined list
-        assertEquals("TestGetValues 0", list, UUIDValue.getValues(context, item));
+        assertEquals("TestGetValues 0", list, uuidValue.getValues(context, item));
     }
 
     @Test
     public void testSetUseForPlace() {
         // Setup objects utilized in unit test
-        UUIDValue.setUseForPlace(true);
+        uuidValue.setUseForPlace(true);
 
         // The reported boolean should return true
-        assertEquals("TestSetUseForPlace 0", true, UUIDValue.getUseForPlace());
+        assertEquals("TestSetUseForPlace 0", true, uuidValue.getUseForPlace());
 
     }
 
     @Test
     public void testGetUseForPlace() {
         // Setup objects utilized in unit test
-        UUIDValue.setUseForPlace(true);
+        uuidValue.setUseForPlace(true);
 
         // The reported boolean should return true
-        assertEquals("TestGetUseForPlace 0", true, UUIDValue.getUseForPlace());
+        assertEquals("TestGetUseForPlace 0", true, uuidValue.getUseForPlace());
     }
 }

dspace-api/src/test/java/org/dspace/core/ContextTest.java

@@ -10,12 +10,15 @@ package org.dspace.core;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
+import java.io.IOException;
 import java.sql.SQLException;
 import java.util.List;
 import java.util.Locale;
@@ -84,7 +87,7 @@ public class ContextTest extends AbstractUnitTest {
      * Test of setCurrentUser method, of class Context.
      */
     @Test
-    public void testSetCurrentUser() throws SQLException, AuthorizeException {
+    public void testSetCurrentUser() throws SQLException, AuthorizeException, IOException {
         // Allow full Admin perms
         when(authorizeServiceSpy.isAdmin(context)).thenReturn(true);
 
@@ -105,6 +108,9 @@ public class ContextTest extends AbstractUnitTest {
 
         // Restore the previous current user
         context.setCurrentUser(oldUser);
+
+        // Cleanup our new user
+        ePersonService.delete(context, newUser);
     }
 
     /**
@@ -255,6 +261,60 @@ public class ContextTest extends AbstractUnitTest {
         cleanupContext(instance);
     }
 
+    /**
+     * Test of commit method, of class Context.
+     */
+    @Test
+    public void testCommit() throws SQLException, AuthorizeException, IOException {
+        // To test commit() we need a new Context object
+        Context instance = new Context();
+
+        // By default, we should have a new DB connection, so let's make sure it is there
+        assertThat("HibernateDBConnection should exist", instance.getDBConnection(), notNullValue());
+        assertTrue("Context should be valid", instance.isValid());
+        assertTrue("Transaction should be open", instance.isTransactionAlive());
+
+        // Allow full Admin perms (in new context)
+        when(authorizeServiceSpy.isAdmin(instance)).thenReturn(true);
+
+        // Create a new EPerson (to be committed)
+        String createdEmail = "myfakeemail@example.com";
+        EPerson newUser = ePersonService.create(instance);
+        newUser.setFirstName(instance, "Tim");
+        newUser.setLastName(instance, "Smith");
+        newUser.setEmail(createdEmail);
+        newUser.setCanLogIn(true);
+        newUser.setLanguage(instance, I18nUtil.getDefaultLocale().getLanguage());
+
+        // Now, call commit()
+        instance.commit();
+
+        // We expect our DB connection to still exist
+        assertThat("HibernateDBConnection should still be open", instance.getDBConnection(), notNullValue());
+        // We expect the Context to be valid
+        assertTrue("Context should still be valid", instance.isValid());
+        // However, the transaction should now be closed
+        assertFalse("DB transaction should be closed", instance.isTransactionAlive());
+
+        // ReloadEntity and verify changes saved
+        // NOTE: reloadEntity() is required, see commit() method Javadocs
+        newUser = instance.reloadEntity(newUser);
+        assertEquals("New user should be created", newUser.getEmail(), createdEmail);
+
+        // Change the email and commit again (a Context should support multiple commit() calls)
+        String newEmail = "myrealemail@example.com";
+        newUser.setEmail(newEmail);
+        instance.commit();
+
+        // Reload entity and new value should be there.
+        newUser = instance.reloadEntity(newUser);
+        assertEquals("New email address should be saved", newUser.getEmail(), newEmail);
+
+        // Cleanup our new object & context
+        ePersonService.delete(instance, newUser);
+        cleanupContext(instance);
+    }
+
     /**
      * Test of abort method, of class Context.
      */
@@ -269,11 +329,11 @@ public class ContextTest extends AbstractUnitTest {
         // Create a new EPerson (DO NOT COMMIT IT)
         String createdEmail = "susie@email.com";
         EPerson newUser = ePersonService.create(instance);
-        newUser.setFirstName(context, "Susan");
+        newUser.setFirstName(instance, "Susan");
-        newUser.setLastName(context, "Doe");
+        newUser.setLastName(instance, "Doe");
         newUser.setEmail(createdEmail);
         newUser.setCanLogIn(true);
-        newUser.setLanguage(context, I18nUtil.getDefaultLocale().getLanguage());
+        newUser.setLanguage(instance, I18nUtil.getDefaultLocale().getLanguage());
 
         // Abort our context
         instance.abort();
@@ -304,12 +364,11 @@ public class ContextTest extends AbstractUnitTest {
 
             // Create a new EPerson (DO NOT COMMIT IT)
             EPerson newUser = ePersonService.create(instance);
-            newUser.setFirstName(context, "Susan");
+            newUser.setFirstName(instance, "Susan");
-            newUser.setLastName(context, "Doe");
+            newUser.setLastName(instance, "Doe");
             newUser.setEmail(createdEmail);
             newUser.setCanLogIn(true);
-            newUser.setLanguage(context, I18nUtil.getDefaultLocale().getLanguage());
+            newUser.setLanguage(instance, I18nUtil.getDefaultLocale().getLanguage());
-
         }
 
         // Open a new context, let's make sure that EPerson isn't there
@@ -433,7 +492,7 @@ public class ContextTest extends AbstractUnitTest {
      * Test of getSpecialGroups method, of class Context.
      */
     @Test
-    public void testGetSpecialGroups() throws SQLException, AuthorizeException {
+    public void testGetSpecialGroups() throws SQLException, AuthorizeException, IOException {
         // To test special groups we need a new Context object
         Context instance = new Context();
 
@@ -456,7 +515,8 @@ public class ContextTest extends AbstractUnitTest {
         assertThat("testGetSpecialGroup 1", specialGroups.get(0), equalTo(group));
         assertThat("testGetSpecialGroup 1", specialGroups.get(1), equalTo(adminGroup));
 
-        // Cleanup our context
+        // Cleanup our context & group
+        groupService.delete(instance, group);
         cleanupContext(instance);
     }
 

dspace-api/src/test/java/org/dspace/core/HibernateDBConnectionTest.java

@@ -0,0 +1,229 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.core;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.SQLException;
+
+import org.dspace.AbstractUnitTest;
+import org.dspace.eperson.EPerson;
+import org.dspace.utils.DSpace;
+import org.hibernate.Session;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Perform some basic unit tests for HibernateDBConnection
+ *
+ * @author tdonohue
+ */
+public class HibernateDBConnectionTest extends AbstractUnitTest {
+
+    private HibernateDBConnection connection;
+
+    /**
+     * This method will be run before every test as per @Before. It will
+     * initialize resources required for the tests.
+     *
+     * Other methods can be annotated with @Before here or in subclasses
+     * but no execution order is guaranteed
+     */
+    @Before
+    @Override
+    public void init() {
+        super.init();
+        // Get a DB connection to test with
+        connection = new DSpace().getServiceManager()
+                                 .getServiceByName(null, HibernateDBConnection.class);
+    }
+
+    /**
+     * Test of getSession method
+     */
+    @Test
+    public void testGetSession() throws SQLException {
+        assertNotNull("DB connection should not be null", connection);
+        // Connection should begin with an active transaction
+        assertTrue("A transaction should be open by default", connection.getTransaction().isActive());
+
+        // Rollback current transaction
+        connection.getTransaction().rollback();
+
+        // Transaction should be closed
+        assertFalse("Transaction should be closed after rollback", connection.getTransaction().isActive());
+
+        //Now call getSession(), saving a reference to the session
+        Session currentSession = connection.getSession();
+
+        // New transaction should be initialized
+        assertTrue("New transaction should be open after getSession() call",
+                   connection.getTransaction().isActive());
+
+        // Call getSession again. The same Session should still be returned
+        assertEquals("Multiple calls to getSession should return same Session", currentSession,
+                     connection.getSession());
+    }
+
+    /**
+     * Test of isTransactionAlive method
+     */
+    @Test
+    public void testIsTransactionAlive() {
+        assertNotNull("DB connection should not be null", connection);
+        assertNotNull("Transaction should not be null", connection.getTransaction());
+        // Connection should begin with a transaction
+        assertTrue("A transaction should be open by default", connection.isTransActionAlive());
+
+        // Rollback current transaction
+        connection.getTransaction().rollback();
+
+        // Transaction should be closed
+        assertFalse("Transaction should be closed after rollback", connection.isTransActionAlive());
+    }
+
+    /**
+     * Test of isSessionAlive method
+     */
+    @Test
+    public void testIsSessionAlive() throws SQLException {
+        assertNotNull("DB connection should not be null", connection);
+        assertNotNull("Session should not be null", connection.getSession());
+        assertTrue("A Session should be alive by default", connection.isSessionAlive());
+
+        // Rollback current transaction, closing it
+        connection.getTransaction().rollback();
+
+        // Session should still be alive even after transaction closes
+        assertTrue("A Session should still be alive if transaction closes", connection.isSessionAlive());
+
+        // NOTE: Because we configure Hibernate Session objects to be bound to a thread
+        // (see 'hibernate.current_session_context_class' in hibernate.cfg.xml), a Session is ALWAYS ALIVE until
+        // the thread closes (at which point Hibernate will clean it up automatically).
+        // This means that essentially isSessionAlive() will always return true, unless the connection is severed
+        // in some unexpected way.  See also "testCloseDBConnection()"
+    }
+
+    /**
+     * Test of closeDBConnection method
+     */
+    @Test
+    public void testCloseDBConnection() throws SQLException {
+        // Get a reference to the current Session
+        Session initialSession = connection.getSession();
+
+        // Close the DB connection / Session
+        // NOTE: Because of our Hibernate configuration, Hibernate automatically creates a new Session per thread.
+        // Even though we "close" the connection, Hibernate will reopen a new one immediately. So, all this actually
+        // does is create a *new* Session
+        connection.closeDBConnection();
+
+        Session newSession = connection.getSession();
+        assertNotEquals("New Session expected",initialSession, newSession);
+    }
+
+    /**
+     * Test of commit method
+     */
+    @Test
+    public void testCommit() throws SQLException {
+        // Ensure a transaction exists
+        connection.getSession();
+        assertTrue("Transaction should be active", connection.getTransaction().isActive());
+
+        connection.commit();
+        assertFalse("Commit should close transaction", connection.getTransaction().isActive());
+
+        // A second commit should be a no-op (no error thrown)
+        connection.commit();
+    }
+
+    /**
+     * Test of rollback method
+     */
+    @Test
+    public void testRollback() throws SQLException {
+        // Ensure a transaction exists
+        connection.getSession();
+        assertTrue("Transaction should be active", connection.getTransaction().isActive());
+
+        connection.rollback();
+        assertFalse("Rollback should close transaction", connection.getTransaction().isActive());
+
+        // A second rollback should be a no-op (no error thrown)
+        connection.rollback();
+    }
+
+    /**
+     * Test of reloadEntity method
+     */
+    @Test
+    public void testReloadEntityAfterRollback() throws SQLException {
+        // Get DBConnection  associated with DSpace Context
+        HibernateDBConnection dbConnection = (HibernateDBConnection) context.getDBConnection();
+        EPerson person = context.getCurrentUser();
+
+        assertTrue("Current user should be cached in session", dbConnection.getSession()
+                                                                           .contains(person));
+
+        dbConnection.rollback();
+        assertFalse("Current user should be gone from cache", dbConnection.getSession()
+                                                                           .contains(person));
+
+        person = dbConnection.reloadEntity(person);
+        assertTrue("Current user should be cached back in session", dbConnection.getSession()
+                                                                           .contains(person));
+    }
+
+    /**
+     * Test of reloadEntity method
+     */
+    @Test
+    public void testReloadEntityAfterCommit() throws SQLException {
+        // Get DBConnection associated with DSpace Context
+        HibernateDBConnection dbConnection = (HibernateDBConnection) context.getDBConnection();
+        EPerson person = context.getCurrentUser();
+
+        assertTrue("Current user should be cached in session", dbConnection.getSession()
+                                                                           .contains(person));
+
+        dbConnection.commit();
+        assertFalse("Current user should be gone from cache", dbConnection.getSession()
+                                                                          .contains(person));
+
+        person = dbConnection.reloadEntity(person);
+        assertTrue("Current user should be cached back in session", dbConnection.getSession()
+                                                                                .contains(person));
+    }
+
+    /**
+     * Test of uncacheEntity method
+     */
+    @Test
+    public void testUncacheEntity() throws SQLException {
+        // Get DBConnection associated with DSpace Context
+        HibernateDBConnection dbConnection = (HibernateDBConnection) context.getDBConnection();
+        EPerson person = context.getCurrentUser();
+
+        assertTrue("Current user should be cached in session", dbConnection.getSession()
+                                                                           .contains(person));
+
+        dbConnection.uncacheEntity(person);
+        assertFalse("Current user should be gone from cache", dbConnection.getSession()
+                                                                          .contains(person));
+
+        // Test ability to reload an uncached entity
+        person = dbConnection.reloadEntity(person);
+        assertTrue("Current user should be cached back in session", dbConnection.getSession()
+                                                                                .contains(person));
+    }
+}

dspace-api/src/test/java/org/dspace/core/PathsClassLoaderTest.java

@@ -8,7 +8,7 @@
 package org.dspace.core;
 
 import static org.apache.bcel.Const.ACC_PUBLIC;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
 
 import java.io.File;
 import java.io.FileOutputStream;
@@ -136,12 +136,12 @@ public class PathsClassLoaderTest {
             jarFile.getCanonicalPath()};
         PathsClassLoader instance = new PathsClassLoader(parentCL, classpath);
         Class result = instance.findClass(className);
-        assertTrue("Should return a Class from file", result instanceof Class);
+        assertNotNull("Should return a Class from file", result);
 
         classpath[0] = jarFile.getCanonicalPath();
         instance = new PathsClassLoader(parentCL, classpath);
         result = instance.findClass(jarClassName);
-        assertTrue("Should return a Class from JAR", result instanceof Class);
+        assertNotNull("Should return a Class from JAR", result);
     }
 
 }

dspace-api/src/test/java/org/dspace/external/provider/impl/MockDataProvider.java

@@ -8,8 +8,8 @@
 package org.dspace.external.provider.impl;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -28,10 +28,12 @@ public class MockDataProvider implements ExternalDataProvider {
      * Generic getter for the sourceIdentifier
      * @return the sourceIdentifier value of this MockDataProvider
      */
+    @Override
     public String getSourceIdentifier() {
         return sourceIdentifier;
     }
 
+    @Override
     public Optional<ExternalDataObject> getExternalDataObject(String id) {
         ExternalDataObject externalDataObject = mockLookupMap.get(id);
         if (externalDataObject == null) {
@@ -41,8 +43,9 @@ public class MockDataProvider implements ExternalDataProvider {
         }
     }
 
+    @Override
     public List<ExternalDataObject> searchExternalDataObjects(String query, int start, int limit) {
-        List<ExternalDataObject> listToReturn = new LinkedList<>();
+        List<ExternalDataObject> listToReturn = new ArrayList<>();
         for (Map.Entry<String, ExternalDataObject> entry : mockLookupMap.entrySet()) {
             if (StringUtils.containsIgnoreCase(entry.getKey(), query)) {
                 listToReturn.add(entry.getValue());
@@ -72,7 +75,7 @@ public class MockDataProvider implements ExternalDataProvider {
 
     public void init() throws IOException {
         mockLookupMap = new HashMap<>();
-        List<String> externalDataObjectsToMake = new LinkedList<>();
+        List<String> externalDataObjectsToMake = new ArrayList<>();
         externalDataObjectsToMake.add("one");
         externalDataObjectsToMake.add("two");
         externalDataObjectsToMake.add("three");
@@ -83,7 +86,7 @@ public class MockDataProvider implements ExternalDataProvider {
             externalDataObject.setId(id);
             externalDataObject.setValue(id);
             externalDataObject.setDisplayValue(id);
-            List<MetadataValueDTO> list = new LinkedList<>();
+            List<MetadataValueDTO> list = new ArrayList<>();
             list.add(new MetadataValueDTO("dc", "contributor", "author", null, "Donald, Smith"));
             externalDataObject.setMetadata(list);
 

dspace-api/src/test/java/org/dspace/handle/dao/impl/HandleDAOImplTest.java

@@ -128,12 +128,8 @@ public class HandleDAOImplTest extends AbstractUnitTest {
             owningCommunity = context.reloadEntity(owningCommunity);
             ContentServiceFactory.getInstance().getCommunityService().delete(context, owningCommunity);
             owningCommunity = null;
-        } catch (SQLException e) {
+        } catch (Exception e) {
-            e.printStackTrace();
+            throw new AssertionError("Error occurred in destroy()", e);
-        } catch (AuthorizeException e) {
-            e.printStackTrace();
-        } catch (IOException e) {
-            e.printStackTrace();
         }
         item1 = null;
         item2 = null;

dspace-api/src/test/java/org/dspace/scripts/DSpaceCommandLineParameterTest.java

@@ -11,7 +11,7 @@ import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 
 import org.dspace.AbstractUnitTest;
@@ -121,7 +121,7 @@ public class DSpaceCommandLineParameterTest extends AbstractUnitTest {
         String value3 = null;
         DSpaceCommandLineParameter dSpaceCommandLineParameter3 = new DSpaceCommandLineParameter(key3, value3);
 
-        List<DSpaceCommandLineParameter> dSpaceCommandLineParameterList = new LinkedList<>();
+        List<DSpaceCommandLineParameter> dSpaceCommandLineParameterList = new ArrayList<>();
         dSpaceCommandLineParameterList.add(dSpaceCommandLineParameter);
         dSpaceCommandLineParameterList.add(dSpaceCommandLineParameter1);
         dSpaceCommandLineParameterList.add(dSpaceCommandLineParameter2);

dspace-api/src/test/java/org/dspace/statistics/util/DummyHttpServletRequest.java

@@ -11,11 +11,11 @@ package org.dspace.statistics.util;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.security.Principal;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -114,7 +114,7 @@ public class DummyHttpServletRequest implements HttpServletRequest {
      * @param headerValue The value of the header
      */
     public void addHeader(String headerName, String headerValue) {
-        List<String> values = headers.computeIfAbsent(headerName, k -> new LinkedList<>());
+        List<String> values = headers.computeIfAbsent(headerName, k -> new ArrayList<>());
         values.add(headerValue);
     }
     /* (non-Javadoc)
@@ -292,6 +292,7 @@ public class DummyHttpServletRequest implements HttpServletRequest {
      * @see javax.servlet.http.HttpServletRequest#isRequestedSessionIdFromUrl()
      */
     @Override
+    @Deprecated
     public boolean isRequestedSessionIdFromUrl() {
         // TODO Auto-generated method stub
         return false;
@@ -502,6 +503,7 @@ public class DummyHttpServletRequest implements HttpServletRequest {
      * @see javax.servlet.ServletRequest#getRealPath(java.lang.String)
      */
     @Override
+    @Deprecated
     public String getRealPath(String arg0) {
         // TODO Auto-generated method stub
         return null;

dspace-api/src/test/java/org/dspace/workflowbasic/BasicWorkflowAuthorizationIT.java

@@ -271,7 +271,7 @@ public class BasicWorkflowAuthorizationIT
             Item item = wsi.getItem();
             Bundle bundle = bundleService.create(context, item, "ORIGINAL");
             File f = new File(AbstractDSpaceTest.testProps.get("test.bitstream").toString());
-            Bitstream bs = bitstreamService.create(context, bundle, new FileInputStream(f));
+            bitstreamService.create(context, bundle, new FileInputStream(f));
             bundleService.update(context, bundle);
             itemService.update(context, item);
             workspaceItemService.update(context, wsi);
@@ -323,7 +323,7 @@ public class BasicWorkflowAuthorizationIT
             Item item = wsi.getItem();
             Bundle bundle = bundleService.create(context, item, "ORIGINAL");
             File f = new File(AbstractDSpaceTest.testProps.get("test.bitstream").toString());
-            Bitstream bs = bitstreamService.create(context, bundle, new FileInputStream(f));
+            bitstreamService.create(context, bundle, new FileInputStream(f));
             bundleService.update(context, bundle);
             itemService.update(context, item);
             workspaceItemService.update(context, wsi);
@@ -365,7 +365,7 @@ public class BasicWorkflowAuthorizationIT
             item.setSubmitter(submitter);
             Bundle bundle = bundleService.create(context, item, "ORIGINAL");
             File f = new File(AbstractDSpaceTest.testProps.get("test.bitstream").toString());
-            Bitstream bs = bitstreamService.create(context, bundle, new FileInputStream(f));
+            bitstreamService.create(context, bundle, new FileInputStream(f));
             bundleService.update(context, bundle);
             itemService.update(context, item);
             workspaceItemService.update(context, wsi);

dspace-api/src/test/java/org/dspace/xmlworkflow/RoleTest.java

@@ -31,49 +31,48 @@ public class RoleTest extends AbstractUnitTest {
     @Test
     public void defaultWorkflow_RoleReviewer() {
         Role role = defaultWorkflow.getRoles().get("Reviewer");
-        assertEquals(role.getDescription(),
+        assertEquals("The people responsible for this step are able to edit the metadata of incoming submissions, " +
-                "The people responsible for this step are able to edit the metadata of incoming submissions, " +
+                        "and then accept or reject them.", role.getDescription());
-                        "and then accept or reject them.");
+        assertEquals("Reviewer", role.getName());
-        assertEquals(role.getName(), "Reviewer");
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
     }
 
     @Test
     public void defaultWorkflow_RoleEditor() {
         Role role = defaultWorkflow.getRoles().get("Editor");
-        assertEquals(role.getDescription(), "The people responsible for this step are able to edit the " +
+        assertEquals("The people responsible for this step are able to edit the " +
-                "metadata of incoming submissions, and then accept or reject them.");
+                "metadata of incoming submissions, and then accept or reject them.", role.getDescription());
-        assertEquals(role.getName(), "Editor");
+        assertEquals("Editor", role.getName());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
     }
 
     @Test
     public void defaultWorkflow_RoleFinalEditor() {
         Role role = defaultWorkflow.getRoles().get("Final Editor");
-        assertEquals(role.getDescription(), "The people responsible for this step are able to edit the " +
+        assertEquals("The people responsible for this step are able to edit the " +
-                "metadata of incoming submissions, but will not be able to reject them.");
+                "metadata of incoming submissions, but will not be able to reject them.", role.getDescription());
-        assertEquals(role.getName(), "Final Editor");
+        assertEquals("Final Editor", role.getName());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
     }
 
     @Test
     public void selectSingleReviewer_RoleReviewManagers() {
         Role role = selectSingleReviewer.getRoles().get("ReviewManagers");
-        assertEquals(role.getName(), "ReviewManagers");
+        assertEquals("ReviewManagers", role.getName());
-        assertEquals(role.getScope(), Role.Scope.REPOSITORY);
+        assertEquals(Role.Scope.REPOSITORY, role.getScope());
     }
 
     @Test
     public void selectSingleReviewer_RoleReviewer() {
         Role role = selectSingleReviewer.getRoles().get("Reviewer");
-        assertEquals(role.getName(), "Reviewer");
+        assertEquals("Reviewer", role.getName());
-        assertEquals(role.getScope(), Role.Scope.ITEM);
+        assertEquals(Role.Scope.ITEM, role.getScope());
     }
 
     @Test
     public void scoreReview_RoleScoreReviewers() {
         Role role = scoreReview.getRoles().get("ScoreReviewers");
-        assertEquals(role.getName(), "ScoreReviewers");
+        assertEquals("ScoreReviewers", role.getName());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
     }
 }

dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java

@@ -78,14 +78,14 @@ public class XmlWorkflowFactoryTest extends AbstractUnitTest {
     public void workflowMapping_NonMappedCollection() throws WorkflowConfigurationException {
         Collection collection = this.findOrCreateCollectionWithHandle("123456789/6");
         Workflow workflow = xmlWorkflowFactory.getWorkflow(collection);
-        assertEquals(workflow.getID(), "defaultWorkflow");
+        assertEquals("defaultWorkflow", workflow.getID());
     }
 
     @Test
     public void workflowMapping_MappedCollection() throws WorkflowConfigurationException {
         Collection collection = this.findOrCreateCollectionWithHandle("123456789/4");
         Workflow workflow = xmlWorkflowFactory.getWorkflow(collection);
-        assertEquals(workflow.getID(), "selectSingleReviewer");
+        assertEquals("selectSingleReviewer", workflow.getID());
     }
 
     private Collection findOrCreateCollectionWithHandle(String handle) {

dspace-api/src/test/java/org/dspace/xmlworkflow/state/StepTest.java

@@ -7,8 +7,9 @@
  */
 package org.dspace.xmlworkflow.state;
 
-import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assert.assertEquals;
-import static junit.framework.TestCase.assertNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.util.List;
 
@@ -35,70 +36,70 @@ public class StepTest extends AbstractUnitTest {
     @Test
     public void defaultWorkflow_ReviewStep() throws WorkflowConfigurationException {
         Step step = defaultWorkflow.getStep("reviewstep");
-        assertEquals(step.getUserSelectionMethod().getId(), "claimaction");
+        assertEquals("claimaction", step.getUserSelectionMethod().getId());
-        assertEquals(step.getRole().getName(), "Reviewer");
+        assertEquals("Reviewer", step.getRole().getName());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "reviewaction"));
+        assertTrue(this.containsActionNamed(actions, "reviewaction"));
-        assertEquals(step.getNextStep(0).getId(), "editstep");
+        assertEquals("editstep", step.getNextStep(0).getId());
     }
 
     @Test
     public void defaultWorkflow_EditStep() throws WorkflowConfigurationException {
         Step step = defaultWorkflow.getStep("editstep");
-        assertEquals(step.getUserSelectionMethod().getId(), "claimaction");
+        assertEquals("claimaction", step.getUserSelectionMethod().getId());
-        assertEquals(step.getRole().getName(), "Editor");
+        assertEquals("Editor", step.getRole().getName());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "editaction"));
+        assertTrue(this.containsActionNamed(actions, "editaction"));
-        assertEquals(step.getNextStep(0).getId(), "finaleditstep");
+        assertEquals("finaleditstep", step.getNextStep(0).getId());
     }
 
     @Test
     public void defaultWorkflow_FinalEditStep() throws WorkflowConfigurationException {
         Step step = defaultWorkflow.getStep("finaleditstep");
-        assertEquals(step.getUserSelectionMethod().getId(), "claimaction");
+        assertEquals("claimaction", step.getUserSelectionMethod().getId());
-        assertEquals(step.getRole().getName(), "Final Editor");
+        assertEquals("Final Editor", step.getRole().getName());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "finaleditaction"));
+        assertTrue(this.containsActionNamed(actions, "finaleditaction"));
         assertNull(step.getNextStep(0));
     }
 
     @Test
     public void selectSingleReviewer_SelectReviewerStep() throws WorkflowConfigurationException {
         Step step = selectSingleReviewer.getStep("selectReviewerStep");
-        assertEquals(step.getUserSelectionMethod().getId(), "claimaction");
+        assertEquals("claimaction", step.getUserSelectionMethod().getId());
-        assertEquals(step.getRole().getName(), "ReviewManagers");
+        assertEquals("ReviewManagers", step.getRole().getName());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "selectrevieweraction"));
+        assertTrue(this.containsActionNamed(actions, "selectrevieweraction"));
-        assertEquals(step.getNextStep(0).getId(), "singleUserReviewStep");
+        assertEquals("singleUserReviewStep", step.getNextStep(0).getId());
     }
 
     @Test
     public void selectSingleReviewer_SingleUserReviewStep() throws WorkflowConfigurationException {
         Step step = selectSingleReviewer.getStep("singleUserReviewStep");
-        assertEquals(step.getUserSelectionMethod().getId(), "autoassignAction");
+        assertEquals("autoassignAction", step.getUserSelectionMethod().getId());
-        assert (step.getRole().getName().equals("Reviewer"));
+        assertEquals("Reviewer", step.getRole().getName());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "singleuserreviewaction"));
+        assertTrue(this.containsActionNamed(actions, "singleuserreviewaction"));
-        assertEquals(step.getNextStep(1).getId(), "selectReviewerStep");
+        assertEquals("selectReviewerStep", step.getNextStep(1).getId());
     }
 
     @Test
     public void scoreReview_ScoreReviewStep() throws WorkflowConfigurationException {
         Step step = scoreReview.getStep("scoreReviewStep");
-        assertEquals(step.getUserSelectionMethod().getId(), "claimaction");
+        assertEquals("claimaction", step.getUserSelectionMethod().getId());
-        assertEquals(step.getRole().getName(), "ScoreReviewers");
+        assertEquals("ScoreReviewers", step.getRole().getName());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "scorereviewaction"));
+        assertTrue(this.containsActionNamed(actions, "scorereviewaction"));
-        assertEquals(step.getNextStep(0).getId(), "evaluationStep");
+        assertEquals("evaluationStep", step.getNextStep(0).getId());
-        assertEquals(step.getRequiredUsers(), 2);
+        assertEquals(2, step.getRequiredUsers());
     }
 
     @Test
     public void scoreReview_EvaluationStep() throws WorkflowConfigurationException {
         Step step = scoreReview.getStep("evaluationStep");
-        assertEquals(step.getUserSelectionMethod().getId(), "noUserSelectionAction");
+        assertEquals("noUserSelectionAction", step.getUserSelectionMethod().getId());
         List<WorkflowActionConfig> actions = step.getActions();
-        assert (this.containsActionNamed(actions, "evaluationaction"));
+        assertTrue(this.containsActionNamed(actions, "evaluationaction"));
         assertNull(step.getNextStep(0));
     }
 

dspace-api/src/test/java/org/dspace/xmlworkflow/state/WorkflowTest.java

@@ -7,7 +7,8 @@
  */
 package org.dspace.xmlworkflow.state;
 
-import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.List;
 
@@ -31,30 +32,30 @@ public class WorkflowTest extends AbstractUnitTest {
 
     @Test
     public void defaultWorkflow() {
-        assertEquals(defaultWorkflow.getFirstStep().getId(), "reviewstep");
+        assertEquals("reviewstep", defaultWorkflow.getFirstStep().getId());
         List<Step> steps = defaultWorkflow.getSteps();
-        assertEquals(steps.size(), 3);
+        assertEquals(3, steps.size());
-        assert (this.containsStepNamed(steps, "reviewstep"));
+        assertTrue(this.containsStepNamed(steps, "reviewstep"));
-        assert (this.containsStepNamed(steps, "editstep"));
+        assertTrue(this.containsStepNamed(steps, "editstep"));
-        assert (this.containsStepNamed(steps, "finaleditstep"));
+        assertTrue(this.containsStepNamed(steps, "finaleditstep"));
     }
 
     @Test
     public void selectSingleReviewer() {
-        assertEquals(selectSingleReviewer.getFirstStep().getId(), "selectReviewerStep");
+        assertEquals("selectReviewerStep", selectSingleReviewer.getFirstStep().getId());
         List<Step> steps = selectSingleReviewer.getSteps();
-        assertEquals(steps.size(), 2);
+        assertEquals(2, steps.size());
-        assert (this.containsStepNamed(steps, "selectReviewerStep"));
+        assertTrue(this.containsStepNamed(steps, "selectReviewerStep"));
-        assert (this.containsStepNamed(steps, "singleUserReviewStep"));
+        assertTrue(this.containsStepNamed(steps, "singleUserReviewStep"));
     }
 
     @Test
     public void scoreReview() {
-        assertEquals(scoreReview.getFirstStep().getId(), "scoreReviewStep");
+        assertEquals("scoreReviewStep", scoreReview.getFirstStep().getId());
         List<Step> steps = scoreReview.getSteps();
-        assertEquals(steps.size(), 2);
+        assertEquals(2, steps.size());
-        assert (this.containsStepNamed(steps, "scoreReviewStep"));
+        assertTrue(this.containsStepNamed(steps, "scoreReviewStep"));
-        assert (this.containsStepNamed(steps, "evaluationStep"));
+        assertTrue(this.containsStepNamed(steps, "evaluationStep"));
     }
 
     private boolean containsStepNamed(List<Step> steps, String stepName) {

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

@@ -648,8 +648,7 @@ public class RestResourceController implements InitializingBean {
     @RequestMapping(method = RequestMethod.PATCH, value = REGEX_REQUESTMAPPING_IDENTIFIER_AS_DIGIT)
     public ResponseEntity<ResourceSupport> patch(HttpServletRequest request, @PathVariable String apiCategory,
                                                  @PathVariable String model, @PathVariable Integer id,
-                                                 @RequestBody(required = true) JsonNode jsonNode)
+                                                 @RequestBody(required = true) JsonNode jsonNode) {
-        throws HttpRequestMethodNotSupportedException {
         return patchInternal(request, apiCategory, model, id, jsonNode);
     }
 
@@ -671,8 +670,7 @@ public class RestResourceController implements InitializingBean {
     public ResponseEntity<ResourceSupport> patch(HttpServletRequest request, @PathVariable String apiCategory,
                                                  @PathVariable String model,
                                                  @PathVariable(name = "uuid") UUID id,
-                                                 @RequestBody(required = true) JsonNode jsonNode)
+                                                 @RequestBody(required = true) JsonNode jsonNode) {
-        throws HttpRequestMethodNotSupportedException {
         return patchInternal(request, apiCategory, model, id, jsonNode);
     }
 
@@ -690,8 +688,7 @@ public class RestResourceController implements InitializingBean {
     public <ID extends Serializable> ResponseEntity<ResourceSupport> patchInternal(HttpServletRequest request,
                                                                                    String apiCategory,
                                                                                    String model, ID id,
-                                                                                   JsonNode jsonNode)
+                                                                                   JsonNode jsonNode) {
-        throws HttpRequestMethodNotSupportedException {
         checkModelPluralForm(apiCategory, model);
         DSpaceRestRepository<RestAddressableModel, ID> repository = utils.getResourceRepository(apiCategory, model);
         RestAddressableModel modelObject = null;

dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java

@@ -92,34 +92,37 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH
                 HttpStatus.UNPROCESSABLE_ENTITY.value());
     }
 
-    @ExceptionHandler( {MissingParameterException.class, QueryMethodParameterConversionException.class})
+    @ExceptionHandler(QueryMethodParameterConversionException.class)
     protected void ParameterConversionException(HttpServletRequest request, HttpServletResponse response, Exception ex)
         throws IOException {
-
+        // we want the 400 status for missing parameters, see https://jira.lyrasis.org/browse/DS-4428
-        //422 is not defined in HttpServletResponse.  Its meaning is "Unprocessable Entity".
-        //Using the value from HttpStatus.
-        //Since this is a handled exception case, the stack trace will not be returned.
         sendErrorResponse(request, response, null,
                           ex.getMessage(),
-                          HttpStatus.UNPROCESSABLE_ENTITY.value());
+                          HttpStatus.BAD_REQUEST.value());
+    }
+
+    @ExceptionHandler(MissingParameterException.class)
+    protected void MissingParameterException(HttpServletRequest request, HttpServletResponse response, Exception ex)
+        throws IOException {
+        // we want the 400 status for missing parameters, see https://jira.lyrasis.org/browse/DS-4428
+        sendErrorResponse(request, response, null,
+                          ex.getMessage(),
+                          HttpStatus.BAD_REQUEST.value());
     }
 
     @Override
     protected ResponseEntity<Object> handleMissingServletRequestParameter(MissingServletRequestParameterException ex,
                                                                           HttpHeaders headers, HttpStatus status,
                                                                           WebRequest request) {
-        // we want the 422 status for missing parameter as it seems to be the common behavior for REST application, see
+        // we want the 400 status for missing parameters, see https://jira.lyrasis.org/browse/DS-4428
-        // https://stackoverflow.com/questions/3050518/what-http-status-response-code-should-i-use-if-the-request-is-missing-a-required
+        return super.handleMissingServletRequestParameter(ex, headers, HttpStatus.BAD_REQUEST, request);
-        return super.handleMissingServletRequestParameter(ex, headers, HttpStatus.UNPROCESSABLE_ENTITY, request);
     }
 
     @Override
     protected ResponseEntity<Object> handleTypeMismatch(TypeMismatchException ex, HttpHeaders headers,
                                                         HttpStatus status, WebRequest request) {
-        // we want the 422 status for type mismatch on parameters as it seems to be the common behavior for REST
+        // we want the 400 status for missing parameters, see https://jira.lyrasis.org/browse/DS-4428
-        // application, see
+        return super.handleTypeMismatch(ex, headers, HttpStatus.BAD_REQUEST, request);
-        // https://stackoverflow.com/questions/3050518/what-http-status-response-code-should-i-use-if-the-request-is-missing-a-required
-        return super.handleTypeMismatch(ex, headers, HttpStatus.UNPROCESSABLE_ENTITY, request);
     }
 
     @ExceptionHandler(Exception.class)

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

@@ -21,7 +21,6 @@ import org.dspace.app.rest.model.BitstreamRest;
 import org.dspace.app.rest.model.BundleRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.content.Bitstream;
@@ -67,7 +66,7 @@ public class BitstreamRestRepository extends DSpaceObjectRestRepository<Bitstrea
 
     @Autowired
     public BitstreamRestRepository(BitstreamService dsoService) {
-        super(dsoService, new DSpaceObjectPatch<BitstreamRest>() { });
+        super(dsoService);
         this.bs = dsoService;
     }
 

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

@@ -24,7 +24,6 @@ import org.dspace.app.rest.model.BitstreamRest;
 import org.dspace.app.rest.model.BundleRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.BundlePatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.content.Bitstream;
@@ -71,8 +70,8 @@ public class BundleRestRepository extends DSpaceObjectRestRepository<Bundle, Bun
     @Autowired
     private BitstreamFormatService bitstreamFormatService;
 
-    public BundleRestRepository(BundleService dsoService, BundlePatch dsoPatch) {
+    public BundleRestRepository(BundleService dsoService) {
-        super(dsoService, dsoPatch);
+        super(dsoService);
         this.bundleService = dsoService;
     }
 

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

@@ -28,7 +28,6 @@ import org.dspace.app.rest.model.TemplateItemRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.model.wrapper.TemplateItem;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.app.rest.utils.CollectionRestEqualityUtils;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Bitstream;
@@ -74,8 +73,7 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
     private ItemService itemService;
 
     public CollectionRestRepository(CollectionService dsoService) {
-        super(dsoService, new DSpaceObjectPatch<CollectionRest>() {
+        super(dsoService);
-        });
     }
 
     @Override
@@ -98,7 +96,7 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
         try {
             long total = cs.countTotal(context);
             List<Collection> collections = cs.findAll(context, pageable.getPageSize(),
-                    Math.toIntExact(pageable.getOffset()));
+                Math.toIntExact(pageable.getOffset()));
             return converter.toRestPage(collections, pageable, total, utils.obtainProjection());
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
@@ -157,7 +155,7 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
 
         if (id == null) {
             throw new DSpaceBadRequestException("Parent Community UUID is null. " +
-                                                    "Cannot create a Collection without providing a parent Community");
+                "Cannot create a Collection without providing a parent Community");
         }
 
         HttpServletRequest req = getRequestService().getCurrentRequest().getHttpServletRequest();
@@ -175,7 +173,7 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
             Community parent = communityService.find(context, id);
             if (parent == null) {
                 throw new UnprocessableEntityException("Parent community for id: "
-                                                           + id + " not found");
+                    + id + " not found");
             }
             collection = cs.create(context, parent);
             cs.update(context, collection);
@@ -207,8 +205,8 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
             metadataConverter.setMetadata(context, collection, collectionRest.getMetadata());
         } else {
             throw new IllegalArgumentException("The UUID in the Json and the UUID in the url do not match: "
-                                                   + id + ", "
+                + id + ", "
-                                                   + collectionRest.getId());
+                + collectionRest.getId());
         }
         return converter.toRest(collection, Projection.DEFAULT);
     }
@@ -233,9 +231,10 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
     /**
      * Method to install a logo on a Collection which doesn't have a logo
      * Called by request mappings in CollectionLogoController
+     *
      * @param context
-     * @param collection    The collection on which to install the logo
+     * @param collection The collection on which to install the logo
-     * @param uploadfile    The new logo
+     * @param uploadfile The new logo
      * @return The created bitstream containing the new logo
      * @throws IOException
      * @throws AuthorizeException
@@ -268,7 +267,7 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
         throws SQLException, AuthorizeException {
         if (collection.getTemplateItem() != null) {
             throw new UnprocessableEntityException("Collection with ID " + collection.getID()
-                                                       + " already contains a template item");
+                + " already contains a template item");
         }
         cs.createTemplateItem(context, collection);
         Item item = collection.getTemplateItem();
@@ -284,7 +283,7 @@ public class CollectionRestRepository extends DSpaceObjectRestRepository<Collect
     /**
      * This method looks up the template Item associated with a Collection
      *
-     * @param collection    The Collection for which to find the template
+     * @param collection The Collection for which to find the template
      * @return The template Item from the Collection
      * @throws SQLException
      */

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

@@ -26,7 +26,6 @@ import org.dspace.app.rest.model.BitstreamRest;
 import org.dspace.app.rest.model.CommunityRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.app.rest.utils.CommunityRestEqualityUtils;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Bitstream;
@@ -54,16 +53,17 @@ public class CommunityRestRepository extends DSpaceObjectRestRepository<Communit
     private static final Logger log = org.apache.logging.log4j.LogManager
             .getLogger(CommunityRestRepository.class);
 
-    private final CommunityService cs;
-
     @Autowired
     BitstreamService bitstreamService;
 
     @Autowired
     CommunityRestEqualityUtils communityRestEqualityUtils;
 
+    @Autowired
+    private CommunityService cs;
+
     public CommunityRestRepository(CommunityService dsoService) {
-        super(dsoService, new DSpaceObjectPatch<CommunityRest>() {});
+        super(dsoService);
         this.cs = dsoService;
     }
 

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

@@ -14,8 +14,7 @@ import org.dspace.app.rest.converter.MetadataConverter;
 import org.dspace.app.rest.exception.UnprocessableEntityException;
 import org.dspace.app.rest.model.DSpaceObjectRest;
 import org.dspace.app.rest.model.patch.Patch;
-import org.dspace.app.rest.projection.Projection;
+import org.dspace.app.rest.repository.patch.ResourcePatch;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.DSpaceObject;
 import org.dspace.content.service.DSpaceObjectService;
@@ -33,15 +32,14 @@ public abstract class DSpaceObjectRestRepository<M extends DSpaceObject, R exten
         extends DSpaceRestRepository<R, UUID> implements FindableObjectRepository<M, UUID> {
 
     final DSpaceObjectService<M> dsoService;
-    final DSpaceObjectPatch<R> dsoPatch;
 
+    @Autowired
+    ResourcePatch<M> resourcePatch;
     @Autowired
     MetadataConverter metadataConverter;
 
-    DSpaceObjectRestRepository(DSpaceObjectService<M> dsoService,
+    DSpaceObjectRestRepository(DSpaceObjectService<M> dsoService) {
-                               DSpaceObjectPatch<R> dsoPatch) {
         this.dsoService = dsoService;
-        this.dsoPatch = dsoPatch;
     }
 
     /**
@@ -63,24 +61,8 @@ public abstract class DSpaceObjectRestRepository<M extends DSpaceObject, R exten
         if (dso == null) {
             throw new ResourceNotFoundException(apiCategory + "." + model + " with id: " + id + " not found");
         }
-        R dsoRest = dsoPatch.patch(findOne(context, 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 = converter.toRest(dso, Projection.DEFAULT);
-        if (!origDsoRest.getMetadata().equals(dsoRest.getMetadata())) {
-            metadataConverter.setMetadata(obtainContext(), dso, dsoRest.getMetadata());
-        }
     }
 
     @Override

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

@@ -25,7 +25,6 @@ import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException;
 import org.dspace.app.rest.exception.UnprocessableEntityException;
 import org.dspace.app.rest.model.RestAddressableModel;
 import org.dspace.app.rest.model.patch.Patch;
-import org.dspace.app.util.DCInputsReaderException;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.service.MetadataFieldService;
 import org.dspace.core.Context;
@@ -34,7 +33,6 @@ import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.repository.PagingAndSortingRepository;
-import org.springframework.web.HttpRequestMethodNotSupportedException;
 import org.springframework.web.multipart.MultipartFile;
 
 /**
@@ -392,30 +390,24 @@ public abstract class DSpaceRestRepository<T extends RestAddressableModel, ID ex
     /**
      * Apply a partial update to the REST object via JSON Patch
      *
-     * @param request
+     * @param request     the http request
-     *            the http request
      * @param apiCategory
      * @param model
-     * @param id
+     * @param id          the ID of the target REST object
-     *            the ID of the target REST object
+     * @param patch       the JSON Patch (https://tools.ietf.org/html/rfc6902) operation
-     * @param patch
-     * the JSON Patch (https://tools.ietf.org/html/rfc6902) operation
      * @return
-     * @throws HttpRequestMethodNotSupportedException
      * @throws UnprocessableEntityException
      * @throws DSpaceBadRequestException
      */
     public T patch(HttpServletRequest request, String apiCategory, String model, ID id, Patch patch)
-        throws HttpRequestMethodNotSupportedException, UnprocessableEntityException, DSpaceBadRequestException {
+        throws UnprocessableEntityException, DSpaceBadRequestException {
         Context context = obtainContext();
-
         try {
             thisRepository.patch(context, request, apiCategory, model, id, patch);
             context.commit();
-
         } catch (AuthorizeException ae) {
             throw new RESTAuthorizationException(ae);
-        } catch (SQLException | DCInputsReaderException e) {
+        } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }
         return findById(id).orElse(null);
@@ -433,19 +425,15 @@ public abstract class DSpaceRestRepository<T extends RestAddressableModel, ID ex
      * @param patch
      *            the JSON Patch (https://tools.ietf.org/html/rfc6902) operation
      * @return the full new state of the REST object after patching
-     * @throws HttpRequestMethodNotSupportedException
-     * @throws UnprocessableEntityException
-     * @throws DSpaceBadRequestException
      * @throws RepositoryMethodNotImplementedException
      *             returned by the default implementation when the operation is not supported for the entity
      *
      * @throws SQLException
      * @throws AuthorizeException
-     * @throws DCInputsReaderException
      */
     protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, ID id,
                          Patch patch)
-        throws RepositoryMethodNotImplementedException, SQLException, AuthorizeException, DCInputsReaderException {
+        throws RepositoryMethodNotImplementedException, SQLException, AuthorizeException {
         throw new RepositoryMethodNotImplementedException(apiCategory, model);
     }
 

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;
 
@@ -22,7 +21,6 @@ import org.dspace.app.rest.exception.UnprocessableEntityException;
 import org.dspace.app.rest.model.EPersonRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.EPersonPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.core.Context;
@@ -49,12 +47,9 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
 
     private final EPersonService es;
 
-    @Autowired
-    EPersonPatch epersonPatch;
 
-    public EPersonRestRepository(EPersonService dsoService,
+    public EPersonRestRepository(EPersonService dsoService) {
-                                 EPersonPatch dsoPatch) {
+        super(dsoService);
-        super(dsoService, dsoPatch);
         this.es = dsoService;
     }
 
@@ -150,7 +145,7 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
      *
      * @param email
      *            is the *required* email address
-     * @return a Page of EPersonRest instances matching the user query
+     * @return the EPersonRest instance, if any, matching the user query
      */
     @SearchRestMethod(name = "byEmail")
     public EPersonRest findByEmail(@Parameter(value = "email", required = true) String email) {
@@ -174,31 +169,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/GroupRestRepository.java

@@ -20,7 +20,6 @@ import org.dspace.app.rest.exception.UnprocessableEntityException;
 import org.dspace.app.rest.model.GroupRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.core.Context;
 import org.dspace.eperson.Group;
@@ -44,7 +43,7 @@ public class GroupRestRepository extends DSpaceObjectRestRepository<Group, Group
 
     @Autowired
     GroupRestRepository(GroupService dsoService) {
-        super(dsoService, new DSpaceObjectPatch<GroupRest>() {});
+        super(dsoService);
         this.gs = dsoService;
     }
 

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

@@ -30,7 +30,6 @@ import org.dspace.app.rest.model.ItemRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
 import org.dspace.app.rest.repository.handler.service.UriListHandlerService;
-import org.dspace.app.rest.repository.patch.ItemPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Bundle;
 import org.dspace.content.Collection;
@@ -96,9 +95,8 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
     @Autowired
     private UriListHandlerService uriListHandlerService;
 
-
+    public ItemRestRepository(ItemService dsoService) {
-    public ItemRestRepository(ItemService dsoService, ItemPatch dsoPatch) {
+        super(dsoService);
-        super(dsoService, dsoPatch);
     }
 
     @Override
@@ -125,7 +123,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
         try {
             long total = itemService.countTotal(context);
             Iterator<Item> it = itemService.findAll(context, pageable.getPageSize(),
-                    Math.toIntExact(pageable.getOffset()));
+                Math.toIntExact(pageable.getOffset()));
             List<Item> items = new ArrayList<>();
             while (it.hasNext()) {
                 items.add(it.next());
@@ -140,50 +138,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
     @PreAuthorize("hasPermission(#id, 'ITEM', #patch)")
     protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, UUID id,
                          Patch patch) throws AuthorizeException, SQLException {
-
+        patchDSpaceObject(apiCategory, model, id, patch);
-        Item item = itemService.find(obtainContext(), id);
-        if (item == null) {
-            throw new ResourceNotFoundException(apiCategory + "." + model + " with id: " + id + " not found");
-        }
-        if (item.getTemplateItemOf() != null) {
-            throw new DSpaceBadRequestException("The ID: " + id + " resolved to a template item");
-        }
-        ItemRest itemRest = dsoPatch.patch(findOne(context, id), patch.getOperations());
-        updateDSpaceObject(item, itemRest);
-    }
-
-    @Override
-    protected void updateDSpaceObject(Item item, ItemRest itemRest)
-        throws AuthorizeException, SQLException {
-        super.updateDSpaceObject(item, itemRest);
-        if (item.getTemplateItemOf() != null) {
-            if (itemRest.getWithdrawn() != item.isWithdrawn()) {
-                throw new UnprocessableEntityException("Cannot apply a withdrawn patch to a templateItem");
-            }
-            if (itemRest.getDiscoverable() != item.isDiscoverable()) {
-                throw new UnprocessableEntityException("Cannot apply a discoverable patch to a templateItem");
-            }
-            return;
-        }
-        Context context = obtainContext();
-        if (itemRest.getWithdrawn() != item.isWithdrawn()) {
-            if (itemRest.getWithdrawn()) {
-                if (item.getTemplateItemOf() != null) {
-                    throw new UnprocessableEntityException("A template item cannot be withdrawn.");
-                }
-                itemService.withdraw(context, item);
-            } else {
-                itemService.reinstate(context, item);
-            }
-        }
-
-        if (itemRest.getDiscoverable() != item.isDiscoverable()) {
-            if (itemRest.getDiscoverable() && item.getTemplateItemOf() != null) {
-                throw new UnprocessableEntityException("A template item cannot be discoverable.");
-            }
-            item.setDiscoverable(itemRest.getDiscoverable());
-            itemService.update(context, item);
-        }
     }
 
     @Override
@@ -203,15 +158,15 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
             item = itemService.find(context, id);
             if (item == null) {
                 throw new ResourceNotFoundException(ItemRest.CATEGORY + "." + ItemRest.NAME +
-                                                            " with id: " + id + " not found");
+                    " with id: " + id + " not found");
             }
             if (itemService.isInProgressSubmission(context, item)) {
                 throw new UnprocessableEntityException("The item cannot be deleted. "
-                                                           + "It's part of a in-progress submission.");
+                    + "It's part of a in-progress submission.");
             }
             if (item.getTemplateItemOf() != null) {
                 throw new UnprocessableEntityException("The item cannot be deleted. "
-                                                           + "It's a template for a collection");
+                    + "It's a template for a collection");
             }
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
@@ -228,9 +183,9 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
      * Deletes relationships of an item which need virtual metadata to be copied to actual metadata
      * This ensures a delete call is used which can copy the metadata prior to deleting the item
      *
-     * @param context           The relevant DSpace context
+     * @param context     The relevant DSpace context
-     * @param copyVirtual       The value(s) of the copyVirtualMetadata parameter
+     * @param copyVirtual The value(s) of the copyVirtualMetadata parameter
-     * @param item              The item to be deleted
+     * @param item        The item to be deleted
      */
     private void deleteMultipleRelationshipsCopyVirtualMetadata(Context context, String[] copyVirtual, Item item)
         throws SQLException, AuthorizeException {
@@ -273,7 +228,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
 
     private List<Integer> parseVirtualMetadataTypes(String[] copyVirtual) {
         List<Integer> types = new ArrayList<>();
-        for (String typeString: copyVirtual) {
+        for (String typeString : copyVirtual) {
             if (!StringUtils.isNumeric(typeString)) {
                 throw new DSpaceBadRequestException("parameter " + REQUESTPARAMETER_COPYVIRTUALMETADATA
                     + " should only contain a single value '" + COPYVIRTUAL_ALL[0] + "', '" + COPYVIRTUAL_CONFIGURED[0]
@@ -286,8 +241,9 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
 
     /**
      * Deletes the relationship while copying the virtual metadata to the item which is **NOT** deleted
-     * @param itemToDelete              The item to be deleted
+     *
-     * @param relationshipToDelete      The relationship to be deleted
+     * @param itemToDelete         The item to be deleted
+     * @param relationshipToDelete The relationship to be deleted
      */
     private void deleteRelationshipCopyVirtualMetadata(Item itemToDelete, Relationship relationshipToDelete)
         throws SQLException, AuthorizeException {
@@ -325,7 +281,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
         Collection collection = collectionService.find(context, owningCollectionUuid);
         if (collection == null) {
             throw new DSpaceBadRequestException("The given owningCollection parameter is invalid: "
-                                                    + owningCollectionUuid);
+                + owningCollectionUuid);
         }
         WorkspaceItem workspaceItem = workspaceItemService.create(context, collection, false);
         Item item = workspaceItem.getItem();
@@ -363,8 +319,8 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
             metadataConverter.setMetadata(context, item, itemRest.getMetadata());
         } else {
             throw new IllegalArgumentException("The UUID in the Json and the UUID in the url do not match: "
-                                                       + uuid + ", "
+                + uuid + ", "
-                                                       + itemRest.getId());
+                + itemRest.getId());
         }
         return converter.toRest(item, Projection.DEFAULT);
     }
@@ -378,7 +334,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
      * @return The added bundle
      */
     public Bundle addBundleToItem(Context context, Item item, BundleRest bundleRest)
-            throws SQLException, AuthorizeException {
+        throws SQLException, AuthorizeException {
         if (item.getBundles(bundleRest.getName()).size() > 0) {
             throw new DSpaceBadRequestException("The bundle name already exists in the item");
         }

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

@@ -8,7 +8,6 @@
 package org.dspace.app.rest.repository;
 
 import java.io.IOException;
-
 import java.sql.SQLException;
 import java.util.List;
 import java.util.UUID;
@@ -24,13 +23,11 @@ import org.dspace.app.rest.exception.RESTAuthorizationException;
 import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException;
 import org.dspace.app.rest.exception.UnprocessableEntityException;
 import org.dspace.app.rest.model.ResourcePolicyRest;
-import org.dspace.app.rest.model.patch.Operation;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.factories.ResourcePolicyOperationFactory;
+import org.dspace.app.rest.repository.patch.ResourcePatch;
 import org.dspace.app.rest.utils.DSpaceObjectUtils;
 import org.dspace.app.rest.utils.Utils;
-import org.dspace.app.util.DCInputsReaderException;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.ResourcePolicy;
 import org.dspace.authorize.service.ResourcePolicyService;
@@ -59,9 +56,6 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
     @Autowired
     ResourcePolicyService resourcePolicyService;
 
-    @Autowired
-    ResourcePolicyOperationFactory resourcePolicyOperationPatchFactory;
-
     @Autowired
     Utils utils;
 
@@ -74,6 +68,8 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
     @Autowired
     DSpaceObjectUtils dspaceObjectUtils;
 
+    @Autowired
+    ResourcePatch<ResourcePolicy> resourcePatch;
 
     @Override
     @PreAuthorize("hasPermission(#id, 'resourcepolicy', 'READ')")
@@ -103,21 +99,18 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
 
     /**
      * Find the resource policies matching the uuid of the resource object and/or the specified action
-     * 
+     *
-     * @param resourceUuid
+     * @param resourceUuid mandatory, the uuid of the resource object of the policy
-     *            mandatory, the uuid of the resource object of the policy
+     * @param action       optional, limit the returned policies to the specified action
-     * @param action
+     * @param pageable     contains the pagination information
-     *            optional, limit the returned policies to the specified action
-     * @param pageable
-     *            contains the pagination information
      * @return a Page of ResourcePolicyRest instances matching the uuid of the resource object and/or the specified
-     *         action
+     * action
      */
     @PreAuthorize("hasPermission(#resourceUuid, 'dspaceObject', 'ADMIN')")
     @SearchRestMethod(name = "resource")
     public Page<ResourcePolicyRest> findByResource(@Parameter(value = "uuid", required = true) UUID resourceUuid,
-                                      @Parameter(value = "action", required = false) String action, Pageable pageable) {
+                                                   @Parameter(value = "action", required = false) String action,
-
+                                                   Pageable pageable) {
         List<ResourcePolicy> resourcePolisies = null;
         int total = 0;
         try {
@@ -125,13 +118,13 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
             if (action != null) {
                 int actionId = Constants.getActionID(action);
                 resourcePolisies = resourcePolicyService.findByResouceUuidAndActionId(context, resourceUuid, actionId,
-                        Math.toIntExact(pageable.getOffset()),
+                    Math.toIntExact(pageable.getOffset()),
-                        Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
+                    Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
                 total = resourcePolicyService.countByResouceUuidAndActionId(context, resourceUuid, actionId);
             } else {
                 resourcePolisies = resourcePolicyService.findByResouceUuid(context, resourceUuid,
-                        Math.toIntExact(pageable.getOffset()),
+                    Math.toIntExact(pageable.getOffset()),
-                        Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
+                    Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
                 total = resourcePolicyService.countByResourceUuid(context, resourceUuid);
             }
         } catch (SQLException e) {
@@ -142,22 +135,18 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
 
     /**
      * Find the resource policies matching uuid of the eperson and/or the one specified resource object
-     * 
-     * @param epersonUuid
-     *            mandatory, the uuid of the eperson that benefit of the policy
-     * @param resourceUuid
-     *            optional, limit the returned policies to the ones related to the specified resource
-     * @param pageable
-     *            contains the pagination information
      *
+     * @param epersonUuid  mandatory, the uuid of the eperson that benefit of the policy
+     * @param resourceUuid optional, limit the returned policies to the ones related to the specified resource
+     * @param pageable     contains the pagination information
      * @return It returns the list of explicit matching resource policies, no inherited or broader resource policies
-     *         will be included in the list nor policies derived by groups' membership
+     * will be included in the list nor policies derived by groups' membership
      */
     @PreAuthorize("hasPermission(#epersonUuid, 'EPERSON', 'READ')")
     @SearchRestMethod(name = "eperson")
     public Page<ResourcePolicyRest> findByEPerson(@Parameter(value = "uuid", required = true) UUID epersonUuid,
-                                @Parameter(value = "resource", required = false) UUID resourceUuid, Pageable pageable) {
+                                                  @Parameter(value = "resource", required = false) UUID resourceUuid,
-
+                                                  Pageable pageable) {
         List<ResourcePolicy> resourcePolisies = null;
         int total = 0;
         try {
@@ -168,14 +157,14 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
             }
             if (resourceUuid != null) {
                 resourcePolisies = resourcePolicyService.findByEPersonAndResourceUuid(context, eperson, resourceUuid,
-                        Math.toIntExact(pageable.getOffset()),
+                    Math.toIntExact(pageable.getOffset()),
-                        Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
+                    Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
                 total = resourcePolicyService.countResourcePoliciesByEPersonAndResourceUuid(context,
-                        eperson, resourceUuid);
+                    eperson, resourceUuid);
             } else {
                 resourcePolisies = resourcePolicyService.findByEPerson(context, eperson,
-                        Math.toIntExact(pageable.getOffset()),
+                    Math.toIntExact(pageable.getOffset()),
-                        Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
+                    Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
                 total = resourcePolicyService.countByEPerson(context, eperson);
             }
         } catch (SQLException e) {
@@ -186,22 +175,18 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
 
     /**
      * Find the resource policies matching uuid of the group and/or the ones specified resource object
-     * 
+     *
-     * @param groupUuid
+     * @param groupUuid    mandatory, the uuid of the group that benefit of the policy
-     *            mandatory, the uuid of the group that benefit of the policy
+     * @param resourceUuid optional, limit the returned policies to the ones related to the specified resource
-     * @param resourceUuid
+     * @param pageable     contains the pagination information
-     *            optional, limit the returned policies to the ones related to the specified resource
-     * @param pageable
-     *            contains the pagination information
-     * 
      * @return It returns the list of explicit matching resource policies, no inherited or broader resource policies
-     *         will be included in the list nor policies derived by groups' membership
+     * will be included in the list nor policies derived by groups' membership
      */
     @PreAuthorize("hasPermission(#groupUuid, 'GROUP', 'READ')")
     @SearchRestMethod(name = "group")
     public Page<ResourcePolicyRest> findByGroup(@Parameter(value = "uuid", required = true) UUID groupUuid,
-                                @Parameter(value = "resource", required = false) UUID resourceUuid, Pageable pageable) {
+                                                @Parameter(value = "resource", required = false) UUID resourceUuid,
-
+                                                Pageable pageable) {
         List<ResourcePolicy> resourcePolisies = null;
         int total = 0;
         try {
@@ -215,13 +200,13 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
             }
             if (resourceUuid != null) {
                 resourcePolisies = resourcePolicyService.findByGroupAndResourceUuid(context, group, resourceUuid,
-                        Math.toIntExact(pageable.getOffset()),
+                    Math.toIntExact(pageable.getOffset()),
-                        Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
+                    Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
                 total = resourcePolicyService.countByGroupAndResourceUuid(context, group, resourceUuid);
             } else {
                 resourcePolisies = resourcePolicyService.findByGroup(context, group,
-                        Math.toIntExact(pageable.getOffset()),
+                    Math.toIntExact(pageable.getOffset()),
-                        Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
+                    Math.toIntExact(pageable.getOffset() + pageable.getPageSize()));
                 total = resourcePolicyService.countResourcePolicyByGroup(context, group);
             }
 
@@ -310,7 +295,7 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
             resourcePolicy = resourcePolicyService.find(context, id);
             if (resourcePolicy == null) {
                 throw new ResourceNotFoundException(
-                        ResourcePolicyRest.CATEGORY + "." + ResourcePolicyRest.NAME + " with id: " + id + " not found");
+                    ResourcePolicyRest.CATEGORY + "." + ResourcePolicyRest.NAME + " with id: " + id + " not found");
             }
             resourcePolicyService.delete(context, resourcePolicy);
         } catch (SQLException e) {
@@ -321,27 +306,13 @@ public class ResourcePolicyRestRepository extends DSpaceRestRepository<ResourceP
     @Override
     @PreAuthorize("hasPermission(#id, 'resourcepolicy', 'ADMIN')")
     protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, Integer id,
-            Patch patch)
+                         Patch patch) throws RepositoryMethodNotImplementedException, SQLException, AuthorizeException {
-            throws RepositoryMethodNotImplementedException, SQLException, AuthorizeException, DCInputsReaderException {
+        ResourcePolicy resourcePolicy = resourcePolicyService.find(context, id);
-        ResourcePolicyRest rest = findOne(context,id);
+        if (resourcePolicy == null) {
-        if (rest == null) {
             throw new ResourceNotFoundException(
-                    ResourcePolicyRest.CATEGORY + "." + ResourcePolicyRest.NAME + " with id: " + id + " not found");
+                ResourcePolicyRest.CATEGORY + "." + ResourcePolicyRest.NAME + " with id: " + id + " not found");
-        }
-        for (Operation op : patch.getOperations()) {
-            rest = resourcePolicyOperationPatchFactory.getOperationForPath(op.getPath()).perform(rest, op);
-        }
-
-        ResourcePolicy resourcePolicy = null;
-        try {
-            resourcePolicy = resourcePolicyService.find(context, id);
-            resourcePolicy.setStartDate(rest.getStartDate());
-            resourcePolicy.setEndDate(rest.getEndDate());
-            resourcePolicy.setRpDescription(rest.getDescription());
-            resourcePolicy.setRpName(rest.getName());
-            resourcePolicyService.update(context, resourcePolicy);
-        } catch (SQLException e) {
-            throw new RuntimeException("Unable to patch ResourcePolicy with id = " + id, e);
         }
+        resourcePatch.patch(obtainContext(), resourcePolicy, patch.getOperations());
+        resourcePolicyService.update(context, resourcePolicy);
     }
 }

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

@@ -15,7 +15,6 @@ import javax.servlet.http.HttpServletRequest;
 
 import org.dspace.app.rest.model.SiteRest;
 import org.dspace.app.rest.model.patch.Patch;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Site;
 import org.dspace.content.service.SiteService;
@@ -39,7 +38,7 @@ public class SiteRestRepository extends DSpaceObjectRestRepository<Site, SiteRes
 
     @Autowired
     public SiteRestRepository(SiteService dsoService) {
-        super(dsoService, new DSpaceObjectPatch<SiteRest>() {});
+        super(dsoService);
         this.sitesv = dsoService;
     }
 

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

@@ -14,12 +14,11 @@ import java.util.UUID;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.dspace.app.rest.converter.JsonPatchConverter;
-import org.dspace.app.rest.model.ItemRest;
 import org.dspace.app.rest.model.TemplateItemRest;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.model.wrapper.TemplateItem;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.app.rest.repository.patch.ItemPatch;
+import org.dspace.app.rest.repository.patch.ResourcePatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
 import org.dspace.content.Item;
@@ -45,10 +44,10 @@ public class TemplateItemRestRepository extends DSpaceRestRepository<TemplateIte
     private ItemRestRepository itemRestRepository;
 
     @Autowired
-    private ItemPatch itemPatch;
+    private CollectionService collectionService;
 
     @Autowired
-    private CollectionService collectionService;
+    ResourcePatch<Item> resourcePatch;
 
     @Override
     public TemplateItemRest findOne(Context context, UUID uuid) {
@@ -81,24 +80,23 @@ public class TemplateItemRestRepository extends DSpaceRestRepository<TemplateIte
 
     /**
      * Modify a template Item which is a template Item
-     * @param templateItem          The Item to be modified
+     *
-     * @param jsonNode      The patch to be applied
+     * @param templateItem The Item to be modified
+     * @param jsonNode     The patch to be applied
      * @return The Item as it is after applying the patch
      * @throws SQLException
      * @throws AuthorizeException
      */
     public TemplateItemRest patchTemplateItem(TemplateItem templateItem, JsonNode jsonNode)
         throws SQLException, AuthorizeException {
-
         ObjectMapper mapper = new ObjectMapper();
         JsonPatchConverter patchConverter = new JsonPatchConverter(mapper);
         Patch patch = patchConverter.convert(jsonNode);
 
-        ItemRest patchedItemRest =
+        Item item = templateItem.getItem();
-            itemPatch.patch(converter.toRest(templateItem.getItem(), Projection.DEFAULT), patch.getOperations());
+        resourcePatch.patch(obtainContext(), item, patch.getOperations());
-        itemRestRepository.updateDSpaceObject(templateItem.getItem(), patchedItemRest);
+        itemService.update(obtainContext(), item);
-
+        return findById(templateItem.getID()).orElse(null);
-        return converter.toRest(templateItem, Projection.DEFAULT);
     }
 
     /**
@@ -107,7 +105,7 @@ public class TemplateItemRestRepository extends DSpaceRestRepository<TemplateIte
      * Note: The caller is responsible for checking that this item is in fact a template item.
      *
      * @param context
-     * @param templateItem          The item to be removed
+     * @param templateItem The item to be removed
      * @throws SQLException
      * @throws IOException
      * @throws AuthorizeException

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

@@ -169,7 +169,7 @@ public class WorkspaceItemRestRepository extends DSpaceRestRepository<WorkspaceI
     @Override
     protected WorkspaceItemRest createAndReturn(Context context) throws SQLException, AuthorizeException {
         WorkspaceItem source = submissionService.createWorkspaceItem(context, getRequestService().getCurrentRequest());
-        return converter.toRest(source, Projection.DEFAULT);
+        return converter.toRest(source, converter.getProjection("full"));
     }
 
     @Override

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

@@ -1,101 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch;
-
-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;
-
-/**
- * The base class for resource PATCH operations.
- *
- * @author Michael Spalti
- */
-public abstract class AbstractResourcePatch<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 operations list of patch operations
-     * @throws UnprocessableEntityException
-     * @throws DSpaceBadRequestException
-     */
-    public R patch(R restModel, List<Operation> operations) {
-
-        // Note: the list of possible operations is taken from JsonPatchConverter class. Does not implement
-        // test https://tools.ietf.org/html/rfc6902#section-4.6
-        ops: for (Operation op : operations) {
-            switch (op.getOp()) {
-                case "add":
-                    restModel = add(restModel, op);
-                    continue ops;
-                case "replace":
-                    restModel = replace(restModel, op);
-                    continue ops;
-                case "remove":
-                    restModel = remove(restModel, op);
-                    continue ops;
-                case "copy":
-                    restModel = copy(restModel, op);
-                    continue ops;
-                case "move":
-                    restModel = move(restModel, op);
-                    continue ops;
-                default:
-                    // JsonPatchConverter should have thrown error before this point.
-                    throw new DSpaceBadRequestException("Missing or illegal patch operation: " + op.getOp());
-            }
-        }
-
-        return restModel;
-
-    }
-    // The default patch methods throw an error when no sub-class implementation is provided.
-
-    protected R add(R restModel, Operation operation)
-            throws UnprocessableEntityException, DSpaceBadRequestException {
-        throw new UnprocessableEntityException(
-                "The add operation is not supported."
-        );
-    }
-
-    protected R replace(R restModel, Operation operation)
-            throws UnprocessableEntityException, DSpaceBadRequestException {
-        throw new UnprocessableEntityException(
-                "The replace operation is not supported."
-        );
-    }
-
-    protected R remove(R restModel, Operation operation)
-
-            throws UnprocessableEntityException, DSpaceBadRequestException {
-        throw new UnprocessableEntityException(
-                "The remove operation is not supported."
-        );
-    }
-
-    protected R copy(R restModel, Operation operation)
-            throws UnprocessableEntityException, DSpaceBadRequestException {
-        throw new UnprocessableEntityException(
-                "The copy operation is not supported."
-        );
-    }
-
-    protected R move(R restModel, Operation operation)
-            throws UnprocessableEntityException, DSpaceBadRequestException {
-        throw new UnprocessableEntityException(
-                "The move operation is not supported."
-        );
-    }
-
-}

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

@@ -1,39 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.exception.UnprocessableEntityException;
-import org.dspace.app.rest.model.BundleRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.dspace.app.rest.repository.patch.factories.BundleOperationFactory;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides PATCH operations for bundle updates.
- */
-@Component
-public class BundlePatch extends DSpaceObjectPatch<BundleRest> {
-
-    @Autowired
-    BundleOperationFactory patchFactory;
-
-    /**
-     * Performs the move operation.
-     * @param restModel the rest representation of the bundle
-     * @param operation the move operation
-     * @throws UnprocessableEntityException
-     * @throws DSpaceBadRequestException
-     */
-    protected BundleRest move(BundleRest restModel, Operation operation) {
-        ResourcePatchOperation<BundleRest> patchOperation = patchFactory.getMoveOperation();
-        return patchOperation.perform(restModel, operation);
-    }
-}

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

@@ -1,81 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.ObjectNode;
-import com.flipkart.zjsonpatch.JsonPatch;
-import org.dspace.app.rest.converter.JsonPatchConverter;
-import org.dspace.app.rest.model.DSpaceObjectRest;
-import org.dspace.app.rest.model.MetadataRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.dspace.app.rest.model.patch.Patch;
-
-/**
- * Base class for DSpaceObject-based PATCH operations, providing common functionality.
- *
- * @param <R> the type of DSpaceObjectRest object the class is applicable to.
- */
-public abstract class DSpaceObjectPatch<R extends DSpaceObjectRest> extends AbstractResourcePatch<R> {
-
-    private static final String METADATA_PATH = "/metadata";
-
-    private ObjectMapper objectMapper = new ObjectMapper();
-
-    private JsonPatchConverter jsonPatchConverter = new JsonPatchConverter(objectMapper);
-
-    /**
-     * Applies the given patch operations to the given DSpaceObjectRest instance.
-     *
-     * This extends the default implementation by first applying metadata-based patch operations,
-     * then applying any others.
-     *
-     * @param dsoRest the instance to apply the changes to.
-     * @param operations the list of patch operations.
-     * @return the modified DSpaceObectRest instance.
-     */
-    @Override
-    public R patch(R dsoRest, List<Operation> operations) {
-        List<Operation> metadataOperations = new ArrayList<>();
-        List<Operation> otherOperations = new ArrayList<>();
-
-        for (Operation operation : operations) {
-            String path = operation.getPath();
-            if (path.equals(METADATA_PATH) || path.startsWith(METADATA_PATH + "/")) {
-                metadataOperations.add(operation);
-            } else {
-                otherOperations.add(operation);
-            }
-        }
-
-        if (!metadataOperations.isEmpty()) {
-            dsoRest.setMetadata(applyMetadataPatch(
-                    jsonPatchConverter.convert(new Patch(metadataOperations)),
-                    dsoRest.getMetadata()));
-        }
-
-        return super.patch(dsoRest, otherOperations);
-    }
-
-    private MetadataRest applyMetadataPatch(JsonNode patch, MetadataRest metadataRest) {
-        try {
-            ObjectNode objectNode = objectMapper.createObjectNode();
-            JsonNode metadataNode = objectMapper.valueToTree(metadataRest);
-            objectNode.replace("metadata", metadataNode);
-            JsonPatch.applyInPlace(patch, objectNode);
-            return objectMapper.treeToValue(objectNode.get("metadata"), MetadataRest.class);
-        } catch (IOException e) {
-            throw new IllegalArgumentException(e);
-        }
-    }
-}

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

@@ -1,44 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.exception.UnprocessableEntityException;
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.dspace.app.rest.repository.patch.factories.EPersonOperationFactory;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides patch operations for eperson updates.
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonPatch extends DSpaceObjectPatch<EPersonRest> {
-
-    @Autowired
-    EPersonOperationFactory patchFactory;
-
-    /**
-     * Performs the replace operation.
-     * @param eperson the eperson rest representation
-     * @param operation the replace operation
-     * @throws UnprocessableEntityException
-     * @throws DSpaceBadRequestException
-     */
-    protected EPersonRest replace(EPersonRest eperson, Operation operation) {
-        ResourcePatchOperation<EPersonRest> patchOperation =
-                patchFactory.getReplaceOperationForPath(operation.getPath());
-
-        return patchOperation.perform(eperson, operation);
-
-    }
-}

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

@@ -1,45 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch;
-
-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.patch.Operation;
-import org.dspace.app.rest.repository.patch.factories.ItemOperationFactory;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides PATCH operations for item updates.
- *
- * @author Michael Spalti
- */
-@Component
-public class ItemPatch extends DSpaceObjectPatch<ItemRest> {
-
-    @Autowired
-    ItemOperationFactory patchFactory;
-
-    /**
-     * Performs the replace operation.
-     * @param item the rest representation of the item
-     * @param operation the replace operation
-     * @throws UnprocessableEntityException
-     * @throws DSpaceBadRequestException
-     */
-    protected ItemRest replace(ItemRest item, Operation operation) {
-
-        ResourcePatchOperation<ItemRest> patchOperation =
-                patchFactory.getReplaceOperationForPath(operation.getPath());
-
-        return patchOperation.perform(item, operation);
-
-    }
-}

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

@@ -0,0 +1,67 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch;
+
+import java.sql.SQLException;
+import java.util.List;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.exception.UnprocessableEntityException;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.app.rest.repository.patch.operation.PatchOperation;
+import org.dspace.core.Context;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ * The base class for resource PATCH operations.
+ */
+@Component
+public class ResourcePatch<M> {
+
+    @Autowired
+    private List<PatchOperation> patchOperations;
+
+    /**
+     * Handles the patch operations. Patch implementations are provided by subclasses.
+     * The default methods throw an UnprocessableEntityException.
+     *
+     * @param context       Context of patch operation
+     * @param dso           the dso resource to patch
+     * @param operations    list of patch operations
+     * @throws UnprocessableEntityException
+     * @throws DSpaceBadRequestException
+     */
+    public void patch(Context context, M dso, List<Operation> operations) throws SQLException {
+        for (Operation operation: operations) {
+            performPatchOperation(context, dso, operation);
+        }
+    }
+
+    /**
+     * Checks with all possible patch operations whether they support this operation
+     *      (based on instanceof dso and operation.path)
+     * @param context       Context of patch operation
+     * @param object        the resource to patch
+     * @param operation     the patch operation
+     * @throws DSpaceBadRequestException
+     */
+    protected void performPatchOperation(Context context, M object, Operation operation)
+            throws DSpaceBadRequestException, SQLException {
+        for (PatchOperation patchOperation: patchOperations) {
+            if (patchOperation.supports(object, operation)) {
+                patchOperation.perform(context,object, operation);
+                return;
+            }
+        }
+        throw new DSpaceBadRequestException(
+                "This operation is not supported."
+        );
+    }
+
+}

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

@@ -1,31 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories;
-
-import org.dspace.app.rest.model.BundleRest;
-import org.dspace.app.rest.repository.patch.factories.impl.BundleMoveOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides factory methods for obtaining instances of bundle patch operations.
- */
-@Component
-public class BundleOperationFactory {
-
-    @Autowired
-    BundleMoveOperation bundleMoveOperation;
-
-    /**
-     * Returns the patch instance for the move operation
-     */
-    public ResourcePatchOperation<BundleRest> getMoveOperation() {
-        return bundleMoveOperation;
-    }
-}

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

@@ -1,74 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.repository.patch.factories.impl.EPersonCertificateReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.EPersonEmailReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.EPersonLoginReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.EPersonNetidReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.EPersonPasswordReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides factory methods for obtaining instances of eperson patch operations.
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonOperationFactory {
-
-    @Autowired
-    EPersonPasswordReplaceOperation passwordReplaceOperation;
-
-    @Autowired
-    EPersonLoginReplaceOperation loginReplaceOperation;
-
-    @Autowired
-    EPersonCertificateReplaceOperation certificateReplaceOperation;
-
-    @Autowired
-    EPersonNetidReplaceOperation netIdReplaceOperation;
-
-    @Autowired
-    EPersonEmailReplaceOperation emailReplaceOperation;
-
-    public static final String OPERATION_PASSWORD_CHANGE = "/password";
-    public static final String OPERATION_CAN_LOGIN = "/canLogin";
-    public static final String OPERATION_REQUIRE_CERTIFICATE = "/certificate";
-    public static final String OPERATION_SET_NETID = "/netid";
-    public static final String OPERATION_SET_EMAIL = "/email";
-    /**
-     * Returns the patch instance for the replace operation (based on the operation path).
-     *
-     * @param path the operation path
-     * @return the patch operation implementation
-     * @throws DSpaceBadRequestException
-     */
-    public ResourcePatchOperation<EPersonRest> getReplaceOperationForPath(String path) {
-
-        switch (path) {
-            case OPERATION_PASSWORD_CHANGE:
-                return passwordReplaceOperation;
-            case OPERATION_CAN_LOGIN:
-                return loginReplaceOperation;
-            case OPERATION_REQUIRE_CERTIFICATE:
-                return certificateReplaceOperation;
-            case OPERATION_SET_NETID:
-                return netIdReplaceOperation;
-            case OPERATION_SET_EMAIL:
-                return emailReplaceOperation;
-            default:
-                throw new DSpaceBadRequestException("Missing patch operation for: " + path);
-        }
-    }
-
-}

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

@@ -1,53 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ItemRest;
-import org.dspace.app.rest.repository.patch.factories.impl.ItemDiscoverableReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.ItemWithdrawReplaceOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides factory methods for obtaining instances of item patch operations.
- *
- * @author Michael Spalti
- */
-@Component
-public class ItemOperationFactory {
-
-    @Autowired
-    ItemDiscoverableReplaceOperation itemDiscoverableReplaceOperation;
-
-    @Autowired
-    ItemWithdrawReplaceOperation itemWithdrawReplaceOperation;
-
-    private static final String OPERATION_PATH_WITHDRAW = "/withdrawn";
-    private static final String OPERATION_PATH_DISCOVERABLE = "/discoverable";
-
-    /**
-     * Returns the patch instance for the replace operation (based on the operation path).
-     *
-     * @param path the operation path
-     * @return the patch operation implementation
-     * @throws DSpaceBadRequestException
-     */
-    public ResourcePatchOperation<ItemRest> getReplaceOperationForPath(String path) {
-
-        switch (path) {
-            case OPERATION_PATH_DISCOVERABLE:
-                return itemDiscoverableReplaceOperation;
-            case OPERATION_PATH_WITHDRAW:
-                return itemWithdrawReplaceOperation;
-            default:
-                throw new DSpaceBadRequestException("Missing patch operation for: " + path);
-        }
-    }
-}

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

@@ -1,67 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ResourcePolicyRest;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePatchOperation;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePolicyDescriptionOperations;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePolicyEndDateOperations;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePolicyNameOperations;
-import org.dspace.app.rest.repository.patch.factories.impl.ResourcePolicyStartDateOperations;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-
-/**
- * Provides factory methods for obtaining instances of ResourcePolicy patch operations.
- *
- * @author Andrea Bollini (andrea.bollini at 4science.it)
- */
-@Component
-public class ResourcePolicyOperationFactory {
-
-    @Autowired
-    ResourcePolicyStartDateOperations resourcePolicyStartDateOperations;
-
-    @Autowired
-    ResourcePolicyEndDateOperations resourcePolicyEndDateOperations;
-
-    @Autowired
-    ResourcePolicyNameOperations resourcePolicyNameOperations;
-
-    @Autowired
-    ResourcePolicyDescriptionOperations resourcePolicyDescriptionOperations;
-
-    private static final String OPERATION_PATH_STARTDATE = "/startDate";
-    private static final String OPERATION_PATH_ENDDATE = "/endDate";
-    private static final String OPERATION_PATH_DESCRIPTION = "/description";
-    private static final String OPERATION_PATH_NAME = "/name";
-
-    /**
-     * Returns the patch instance for the operation (based on the operation path).
-     *
-     * @param path the operation path
-     * @return the patch operation implementation
-     * @throws DSpaceBadRequestException
-     */
-    public ResourcePatchOperation<ResourcePolicyRest> getOperationForPath(String path) {
-
-        switch (path) {
-            case OPERATION_PATH_STARTDATE:
-                return resourcePolicyStartDateOperations;
-            case OPERATION_PATH_ENDDATE:
-                return resourcePolicyEndDateOperations;
-            case OPERATION_PATH_DESCRIPTION:
-                return resourcePolicyDescriptionOperations;
-            case OPERATION_PATH_NAME:
-                return resourcePolicyNameOperations;
-            default:
-                throw new DSpaceBadRequestException("Missing patch operation for: " + path);
-        }
-    }
-}

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

@@ -1,58 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-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.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for EPerson requires certificate patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /certificate", "value": true|false]'
- * </code>
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonCertificateReplaceOperation extends ReplacePatchOperation<EPersonRest, Boolean>
-        implements ResourcePatchOperation<EPersonRest> {
-
-    @Override
-    public EPersonRest replace(EPersonRest eperson, Operation operation) {
-
-        Boolean requireCert = getBooleanOperationValue(operation.getValue());
-        eperson.setRequireCertificate(requireCert);
-        return eperson;
-
-    }
-
-    @Override
-    void checkModelForExistingValue(EPersonRest resource, Operation operation) {
-        // 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) {
-            throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
-        }
-    }
-
-    @Override
-    protected Class<Boolean[]> getArrayClassForEvaluation() {
-        return Boolean[].class;
-    }
-
-    @Override
-    protected Class<Boolean> getClassForEvaluation() {
-        return Boolean.class;
-    }
-}

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

@@ -1,54 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-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.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for EPerson password patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /email", "value": "new@email"]'
- * </code>
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonEmailReplaceOperation extends ReplacePatchOperation<EPersonRest, String>
-        implements ResourcePatchOperation<EPersonRest> {
-    @Override
-    EPersonRest replace(EPersonRest eperson, Operation operation) {
-
-        eperson.setEmail((String) operation.getValue());
-        return eperson;
-    }
-
-    @Override
-    void checkModelForExistingValue(EPersonRest resource, Operation operation) {
-        if (resource.getEmail() == null) {
-            throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
-        }
-    }
-
-    @Override
-    protected Class<String[]> getArrayClassForEvaluation() {
-
-        return String[].class;
-    }
-
-    @Override
-    protected Class<String> getClassForEvaluation() {
-
-        return String.class;
-    }
-}

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

@@ -1,54 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-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.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for EPerson canLogin patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /canLogin", "value": true|false]'
- * </code>
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonLoginReplaceOperation extends ReplacePatchOperation<EPersonRest, Boolean>
-        implements ResourcePatchOperation<EPersonRest> {
-
-    @Override
-    public EPersonRest replace(EPersonRest eperson, Operation operation) {
-
-        Boolean canLogin = getBooleanOperationValue(operation.getValue());
-        eperson.setCanLogIn(canLogin);
-        return eperson;
-    }
-
-    @Override
-    void checkModelForExistingValue(EPersonRest resource, Operation operation) {
-        if ((Object) resource.isCanLogIn() == null) {
-            throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
-        }
-    }
-
-    @Override
-    protected Class<Boolean[]> getArrayClassForEvaluation() {
-        return Boolean[].class;
-    }
-
-    @Override
-    protected Class<Boolean> getClassForEvaluation() {
-        return Boolean.class;
-    }
-}

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

@@ -1,54 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-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.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for EPerson netid patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /netid", "value": "newNetId"]'
- * </code>
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonNetidReplaceOperation extends ReplacePatchOperation<EPersonRest, String>
-        implements ResourcePatchOperation<EPersonRest> {
-
-    @Override
-    EPersonRest replace(EPersonRest eperson, Operation operation) {
-
-        eperson.setNetid((String) operation.getValue());
-        return eperson;
-    }
-
-
-    @Override
-    void checkModelForExistingValue(EPersonRest resource, Operation operation) {
-        if (resource.getNetid() == null) {
-            throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
-        }
-    }
-
-    @Override
-    protected Class<String[]> getArrayClassForEvaluation() {
-        return String[].class;
-    }
-
-    @Override
-    protected Class<String> getClassForEvaluation() {
-        return String.class;
-    }
-}

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

@@ -1,57 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import org.dspace.app.rest.model.EPersonRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for EPerson password patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /password", "value": "newpassword"]'
- * </code>
- *
- * @author Michael Spalti
- */
-@Component
-public class EPersonPasswordReplaceOperation extends ReplacePatchOperation<EPersonRest, String> {
-
-    @Override
-    EPersonRest replace(EPersonRest eperson, Operation operation) {
-
-        eperson.setPassword((String) operation.getValue());
-        return eperson;
-    }
-
-    @Override
-    void checkModelForExistingValue(EPersonRest resource, Operation operation) {
-        /*
-         * FIXME: the password field in eperson rest model is always null because
-         * the value is not set in the rest converter.
-         * We would normally throw an exception here since replace
-         * operations are not allowed on non-existent values, but that
-         * would prevent the password update from ever taking place.
-         */
-    }
-
-    @Override
-    protected Class<String[]> getArrayClassForEvaluation() {
-
-        return String[].class;
-    }
-
-    @Override
-    protected Class<String> getClassForEvaluation() {
-
-        return String.class;
-    }
-}

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

@@ -1,55 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import org.apache.log4j.Logger;
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ItemRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * This is the implementation for Item resource patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/item/<:id-item> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /discoverable", "value": true|false]'
- * </code>
- *
- *  @author Michael Spalti
- */
-@Component
-public class ItemDiscoverableReplaceOperation extends ReplacePatchOperation<ItemRest, Boolean> {
-
-    private static final Logger log = Logger.getLogger(ItemDiscoverableReplaceOperation.class);
-
-    @Override
-    public ItemRest replace(ItemRest item, Operation operation) {
-
-        item.setDiscoverable(getBooleanOperationValue(operation.getValue()));
-        return item;
-
-    }
-
-    @Override
-    void checkModelForExistingValue(ItemRest resource, Operation operation) {
-        if ((Object) resource.getDiscoverable() == null) {
-            throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
-        }
-    }
-
-    protected Class<Boolean[]> getArrayClassForEvaluation() {
-        return Boolean[].class;
-    }
-
-    protected Class<Boolean> getClassForEvaluation() {
-        return Boolean.class;
-    }
-
-}

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

@@ -1,80 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import org.apache.log4j.Logger;
-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.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * This is the implementation for Item resource patches.
- * <p>
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/item/<:id-item> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /withdrawn", "value": true|false]'
- * </code>
- *
- * @author Michael Spalti
- */
-@Component
-public class ItemWithdrawReplaceOperation extends ReplacePatchOperation<ItemRest, Boolean> {
-
-    private static final Logger log = Logger.getLogger(ItemWithdrawReplaceOperation.class);
-
-    @Override
-    public ItemRest replace(ItemRest item, Operation operation) {
-
-        Boolean withdraw = getBooleanOperationValue(operation.getValue());
-
-        // 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()) {
-                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()) {
-                return item;
-            }
-            item.setWithdrawn(true);
-            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()) {
-                return item;
-            }
-            item.setWithdrawn(false);
-            return item;
-        }
-
-    }
-
-    @Override
-    void checkModelForExistingValue(ItemRest resource, Operation operation) {
-        if ((Object) resource.getWithdrawn() == null) {
-            throw new DSpaceBadRequestException("Attempting to replace a non-existent value.");
-        }
-    }
-
-    protected Class<Boolean[]> getArrayClassForEvaluation() {
-        return Boolean[].class;
-    }
-
-    protected Class<Boolean> getClassForEvaluation() {
-        return Boolean.class;
-    }
-
-}

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

@@ -1,106 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import org.apache.commons.lang3.StringUtils;
-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.MoveOperation;
-import org.dspace.app.rest.model.patch.Operation;
-
-/**
- * Base class for move patch operations.
- */
-public abstract class MovePatchOperation<R extends RestModel, T> extends PatchOperation<R, T> {
-
-    /**
-     * The index to move the object from
-     */
-    protected int from;
-
-    /**
-     * The index to move the object to
-     */
-    protected int to;
-
-    /**
-     * Implements the patch operation for move operations.
-     * Before performing the move operation this method checks
-     * if the arguments provided are valid
-     *
-     * @param resource  the rest model.
-     * @param operation the move patch operation.
-     * @return the updated rest model.
-     * @throws DSpaceBadRequestException
-     * @throws UnprocessableEntityException
-     */
-    @Override
-    public R perform(R resource, Operation operation) {
-        checkMoveOperation(operation);
-        return move(resource, operation);
-    }
-
-    /**
-     * Executes the move patch operation.
-     *
-     * @param resource  the rest model.
-     * @param operation the move patch operation.
-     * @return the updated rest model.
-     * @throws DSpaceBadRequestException
-     * @throws UnprocessableEntityException
-     */
-    abstract R move(R resource, Operation operation);
-
-    /**
-     * This method checks if the operation contains any invalid arguments. Invalid arguments include:
-     * - from and path point to the same index
-     * - either of the indexes are negative
-     * @param operation the move patch operation.
-     */
-    private void checkMoveOperation(Operation operation) {
-        if (!(operation instanceof MoveOperation)) {
-            throw new DSpaceBadRequestException(
-                    "Expected a MoveOperation, but received " + operation.getClass().getName() + " instead."
-            );
-        }
-        from = getLocationFromPath(((MoveOperation)operation).getFrom());
-        to = getLocationFromPath(operation.getPath());
-        if (from == to) {
-            throw new DSpaceBadRequestException(
-                    "The \"from\" location must be different from the \"to\" location."
-            );
-        }
-        if (from < 0) {
-            throw new DSpaceBadRequestException("A negative \"from\" location was provided: " + from);
-        }
-        if (to < 0) {
-            throw new DSpaceBadRequestException("A negative \"to\" location was provided: " + to);
-        }
-    }
-
-    /**
-     * Fetches and returns the index from a path argument
-     * @param path the provided path (e.g. "/_links/bitstreams/1/href")
-     */
-    protected int getLocationFromPath(String path) {
-        String[] parts = StringUtils.split(path, "/");
-        String locationStr;
-        if (parts.length > 1) {
-            if (StringUtils.equals(parts[parts.length - 1], "href")) {
-                locationStr = parts[parts.length - 2];
-            } else {
-                locationStr = parts[parts.length - 1];
-            }
-        } else {
-            locationStr = parts[0];
-        }
-        return Integer.parseInt(locationStr);
-    }
-}
-

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

@@ -1,82 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-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;
-
-/**
- * Base class for all resource patch operations.
- *
- * @author Michael Spalti
- */
-public abstract class PatchOperation<R extends RestModel, T>
-        implements ResourcePatchOperation<R> {
-
-    /**
-     * Updates the rest model by applying the patch operation.
-     *
-     * @param resource  the rest model.
-     * @param operation the patch operation.
-     * @return the updated rest model.
-     * @throws DSpaceBadRequestException
-     */
-    public abstract R perform(R resource, Operation operation);
-
-    /**
-     * Throws PatchBadRequestException for missing operation value.
-     *
-     * @param value the value to test
-     */
-    void checkOperationValue(Object value) {
-        if (value == null) {
-            throw new DSpaceBadRequestException("No value provided for the operation.");
-        }
-    }
-
-    /**
-     * Allows clients to use either a boolean or a string representation of boolean value.
-     *
-     * @param value the operation value
-     * @return the original or derived boolean value
-     * @throws DSpaceBadRequestException
-     */
-    Boolean getBooleanOperationValue(Object value) {
-        Boolean bool;
-
-        if (value instanceof String) {
-            bool = BooleanUtils.toBooleanObject((String) value);
-            if (bool == null) {
-                // make sure the string was converted to boolean.
-                throw new DSpaceBadRequestException("Boolean value not provided.");
-            }
-        } else {
-            bool = (Boolean) value;
-        }
-        return bool;
-    }
-
-    /**
-     * This method should return the typed array to be used in the
-     * LateObjectEvaluator evaluation of json arrays.
-     *
-     * @return
-     */
-    protected abstract Class<T[]> getArrayClassForEvaluation();
-
-    /**
-     * This method should return the object type to be used in the
-     * LateObjectEvaluator evaluation of json objects.
-     *
-     * @return
-     */
-    protected abstract Class<T> getClassForEvaluation();
-
-}

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

@@ -1,67 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-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;
-
-/**
- * Base class for replace patch operations.
- *
- * @author Michael Spalti
- */
-public abstract class ReplacePatchOperation<R extends RestModel, T>
-        extends PatchOperation<R, T> {
-
-    /**
-     * Implements the patch operation for replace operations.
-     * Before performing the replace operation this method checks
-     * for a non-null operation value and a non-null value on the rest model
-     * (replace operations should only be applied to an existing value).
-     *
-     * @param resource  the rest model.
-     * @param operation the replace patch operation.
-     * @return the updated rest model.
-     * @throws DSpaceBadRequestException
-     * @throws UnprocessableEntityException
-     */
-    @Override
-    public R perform(R resource, Operation operation) {
-
-        checkOperationValue(operation.getValue());
-        checkModelForExistingValue(resource, operation);
-        return replace(resource, operation);
-
-    }
-
-    /**
-     * Executes the replace patch operation.
-     *
-     * @param resource  the rest model.
-     * @param operation the replace patch operation.
-     * @return the updated rest model.
-     * @throws DSpaceBadRequestException
-     * @throws UnprocessableEntityException
-     */
-    abstract R replace(R resource, Operation operation);
-
-    /**
-     * Replace operations are not allowed on non-existent values.
-     * Null values may exist in the RestModel for certain fields
-     * (usually non-boolean). This method should be implemented
-     * to assure that the replace operation acts only on an existing value.
-     *
-     * @param resource the rest model.
-     * @throws DSpaceBadRequestException
-     */
-    abstract void checkModelForExistingValue(R resource, Operation operation);
-
-
-}

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

@@ -1,23 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-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;
-
-/**
- * The patch interface used by repository classes.
- * @param <R>
- */
-public interface ResourcePatchOperation<R extends RestModel> {
-
-    R perform(R resource, Operation operation)
-            throws DSpaceBadRequestException;
-
-}

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

@@ -1,107 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ResourcePolicyRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for ResourcePolicy name patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/authz/resourcepolicies/<:id-resourcepolicy> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /description", "value": "my description"]'
- * </code>
- *
- * @author Andrea Bollini (andrea.bollini at 4science.it)
- */
-@Component
-public class ResourcePolicyDescriptionOperations implements ResourcePatchOperation<ResourcePolicyRest> {
-
-    @Override
-    public ResourcePolicyRest perform(ResourcePolicyRest resource, Operation operation)
-            throws DSpaceBadRequestException {
-        switch (operation.getOp()) {
-            case "replace":
-                checkOperationValue(operation.getValue());
-                checkModelForExistingValue(resource, operation);
-                return replace(resource, operation);
-            case "add":
-                checkOperationValue(operation.getValue());
-                checkModelForNotExistingValue(resource, operation);
-                return add(resource, operation);
-            case "remove":
-                checkModelForExistingValue(resource, operation);
-                return delete(resource, operation);
-            default:
-                throw new DSpaceBadRequestException("Unsupported operation " + operation.getOp());
-        }
-    }
-
-    public ResourcePolicyRest replace(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String newDescription = (String) operation.getValue();
-        resourcePolicy.setDescription(newDescription);
-        return resourcePolicy;
-    }
-
-    public ResourcePolicyRest add(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String description = (String) operation.getValue();
-        resourcePolicy.setDescription(description);
-        return resourcePolicy;
-    }
-
-    public ResourcePolicyRest delete(ResourcePolicyRest resourcePolicy, Operation operation) {
-        resourcePolicy.setDescription(null);
-        return resourcePolicy;
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing operation value.
-     *
-     * @param value
-     *            the value to test
-     */
-    void checkOperationValue(Object value) {
-        if (value == null || value.equals("")) {
-            throw new DSpaceBadRequestException("No value provided for the operation.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing value in the /description path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getDescription() == null) {
-            throw new DSpaceBadRequestException("Attempting to " + operation.getOp() + " a non-existent value.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException if a value is already set in the /description path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForNotExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getDescription() != null) {
-            throw new DSpaceBadRequestException("Attempting to add a value to an already existing path.");
-        }
-    }
-}

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

@@ -1,147 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.util.Date;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ResourcePolicyRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for ResourcePolicy endDate patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/authz/resourcepolicies/<:id-resourcepolicy> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /endDate", "value": "YYYY-MM-DD"]'
- * </code>
- *
- * @author Andrea Bollini (andrea.bollini at 4science.it)
- */
-@Component
-public class ResourcePolicyEndDateOperations implements ResourcePatchOperation<ResourcePolicyRest> {
-
-    final SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-MM-dd");
-
-    @Override
-    public ResourcePolicyRest perform(ResourcePolicyRest resource, Operation operation)
-            throws DSpaceBadRequestException {
-        switch (operation.getOp()) {
-            case "replace":
-                checkOperationValue(operation.getValue());
-                checkModelForExistingValue(resource, operation);
-                checkModelForConsistentValue(resource, operation);
-                return replace(resource, operation);
-            case "add":
-                checkOperationValue(operation.getValue());
-                checkModelForNotExistingValue(resource, operation);
-                checkModelForConsistentValue(resource, operation);
-                return add(resource, operation);
-            case "remove":
-                checkModelForExistingValue(resource, operation);
-                return delete(resource, operation);
-            default:
-                throw new DSpaceBadRequestException("Unsupported operation " + operation.getOp());
-        }
-    }
-
-    ResourcePolicyRest add(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String dateS = (String) operation.getValue();
-        try {
-            Date date = simpleDateFormat.parse(dateS);
-            resourcePolicy.setEndDate(date);
-        } catch (ParseException e) {
-            throw new DSpaceBadRequestException("Invalid endDate value", e);
-        }
-        return resourcePolicy;
-    }
-
-    ResourcePolicyRest delete(ResourcePolicyRest resourcePolicy, Operation operation) {
-        resourcePolicy.setEndDate(null);
-        return resourcePolicy;
-    }
-
-    ResourcePolicyRest replace(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String dateS = (String) operation.getValue();
-        try {
-            Date date = simpleDateFormat.parse(dateS);
-            resourcePolicy.setEndDate(date);
-        } catch (ParseException e) {
-            throw new DSpaceBadRequestException("Invalid endDate value", e);
-        }
-        return resourcePolicy;
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing operation value.
-     *
-     * @param value
-     *            the value to test
-     */
-    void checkOperationValue(Object value) {
-        if (value == null) {
-            throw new DSpaceBadRequestException("No value provided for the operation.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing value in the /endDate path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getEndDate() == null) {
-            throw new DSpaceBadRequestException("Attempting to " + operation.getOp() + " a non-existent value.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException if a value is already set in the /endDate path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForNotExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getEndDate() != null) {
-            throw new DSpaceBadRequestException("Attempting to add a value to an already existing path.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException if the value for endDate is not consistent with the startDate value, if present
-     * (smaller than).
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForConsistentValue(ResourcePolicyRest resource, Operation operation) {
-        String dateS = (String) operation.getValue();
-        try {
-            Date date = simpleDateFormat.parse(dateS);
-            if (resource.getEndDate() != null && resource.getStartDate().after(date)) {
-                throw new DSpaceBadRequestException("Attempting to set an invalid endDate smaller than the startDate.");
-            }
-        } catch (ParseException e) {
-            throw new DSpaceBadRequestException("Invalid endDate value", e);
-        }
-    }
-}

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

@@ -1,107 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ResourcePolicyRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for ResourcePolicy name patches.
- *
- * Example: <code>
- * curl -X PATCH http://${dspace.server.url}/api/authz/resourcepolicies/<:id-resourcepolicy> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /name", "value": "New Name"]'
- * </code>
- *
- * @author Andrea Bollini (andrea.bollini at 4science.it)
- */
-@Component
-public class ResourcePolicyNameOperations implements ResourcePatchOperation<ResourcePolicyRest> {
-
-    @Override
-    public ResourcePolicyRest perform(ResourcePolicyRest resource, Operation operation)
-            throws DSpaceBadRequestException {
-        switch (operation.getOp()) {
-            case "replace":
-                checkOperationValue(operation.getValue());
-                checkModelForExistingValue(resource, operation);
-                return replace(resource, operation);
-            case "add":
-                checkOperationValue(operation.getValue());
-                checkModelForNotExistingValue(resource, operation);
-                return add(resource, operation);
-            case "remove":
-                checkModelForExistingValue(resource, operation);
-                return delete(resource, operation);
-            default:
-                throw new DSpaceBadRequestException("Unsupported operation " + operation.getOp());
-        }
-    }
-
-    public ResourcePolicyRest replace(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String newName = (String) operation.getValue();
-        resourcePolicy.setName(newName);
-        return resourcePolicy;
-    }
-
-    public ResourcePolicyRest add(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String name = (String) operation.getValue();
-        resourcePolicy.setName(name);
-        return resourcePolicy;
-    }
-
-    public ResourcePolicyRest delete(ResourcePolicyRest resourcePolicy, Operation operation) {
-        resourcePolicy.setName(null);
-        return resourcePolicy;
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing operation value.
-     *
-     * @param value
-     *            the value to test
-     */
-    void checkOperationValue(Object value) {
-        if (value == null || value.equals("")) {
-            throw new DSpaceBadRequestException("No value provided for the operation.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing value in the /name path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getName() == null) {
-            throw new DSpaceBadRequestException("Attempting to " + operation.getOp() + " a non-existent value.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException if a value is already set in the /name path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForNotExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getName() != null) {
-            throw new DSpaceBadRequestException("Attempting to add a value to an already existing path.");
-        }
-    }
-}

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

@@ -1,161 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.repository.patch.factories.impl;
-
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.util.Date;
-
-import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.ResourcePolicyRest;
-import org.dspace.app.rest.model.patch.Operation;
-import org.springframework.stereotype.Component;
-
-/**
- * Implementation for ResourcePolicy startDate patches.
- *
- * Examples:
- *
- * <code>
- * curl -X PATCH http://${dspace.server.url}/api/authz/resourcepolicies/<:id-resourcepolicy> -H "
- * Content-Type: application/json" -d '[{ "op": "replace", "path": "
- * /startDate", "value": "YYYY-MM-DD"]'
- * </code>
- *
- * <code>
- * curl -X PATCH http://${dspace.server.url}/api/authz/resourcepolicies/<:id-resourcepolicy> -H "
- * Content-Type: application/json" -d '[{ "op": "add", "path": "
- * /startDate", "value": "YYYY-MM-DD"]'
- * </code>
- *
- * <code>
- * curl -X PATCH http://${dspace.server.url}/api/authz/resourcepolicies/<:id-resourcepolicy> -H "
- * Content-Type: application/json" -d '[{ "op": "delete", "path": "
- * /startDate"]'
- * </code>
- *
- * @author Andrea Bollini (andrea.bollini at 4science.it)
- */
-@Component
-public class ResourcePolicyStartDateOperations implements ResourcePatchOperation<ResourcePolicyRest> {
-
-    final SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-MM-dd");
-
-    @Override
-    public ResourcePolicyRest perform(ResourcePolicyRest resource, Operation operation)
-            throws DSpaceBadRequestException {
-        switch (operation.getOp()) {
-            case "replace":
-                checkOperationValue(operation.getValue());
-                checkModelForExistingValue(resource, operation);
-                checkModelForConsistentValue(resource, operation);
-                return replace(resource, operation);
-            case "add":
-                checkOperationValue(operation.getValue());
-                checkModelForNotExistingValue(resource, operation);
-                checkModelForConsistentValue(resource, operation);
-                return add(resource, operation);
-            case "remove":
-                checkModelForExistingValue(resource, operation);
-                return delete(resource, operation);
-            default:
-                throw new DSpaceBadRequestException("Unsupported operation " + operation.getOp());
-        }
-    }
-
-    ResourcePolicyRest add(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String dateS = (String) operation.getValue();
-        try {
-            Date date = simpleDateFormat.parse(dateS);
-            resourcePolicy.setStartDate(date);
-        } catch (ParseException e) {
-            throw new DSpaceBadRequestException("Invalid startDate value", e);
-        }
-        return resourcePolicy;
-    }
-
-    ResourcePolicyRest delete(ResourcePolicyRest resourcePolicy, Operation operation) {
-        resourcePolicy.setStartDate(null);
-        return resourcePolicy;
-    }
-
-    ResourcePolicyRest replace(ResourcePolicyRest resourcePolicy, Operation operation) {
-        String dateS = (String) operation.getValue();
-        try {
-            Date date = simpleDateFormat.parse(dateS);
-            resourcePolicy.setStartDate(date);
-        } catch (ParseException e) {
-            throw new DSpaceBadRequestException("Invalid startDate value", e);
-        }
-        return resourcePolicy;
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing operation value.
-     *
-     * @param value
-     *            the value to test
-     */
-    void checkOperationValue(Object value) {
-        if (value == null) {
-            throw new DSpaceBadRequestException("No value provided for the operation.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException for missing value in the /startDate path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getStartDate() == null) {
-            throw new DSpaceBadRequestException("Attempting to " + operation.getOp() + " a non-existent value.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException if a value is already set in the /startDate path.
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForNotExistingValue(ResourcePolicyRest resource, Operation operation) {
-        if (resource.getStartDate() != null) {
-            throw new DSpaceBadRequestException("Attempting to add a value to an already existing path.");
-        }
-    }
-
-    /**
-     * Throws PatchBadRequestException if the value for startDate is not consistent with the endDate value, if present
-     * (greater than).
-     *
-     * @param resource
-     *            the resource to update
-     * @param operation
-     *            the operation to apply
-     *
-     */
-    void checkModelForConsistentValue(ResourcePolicyRest resource, Operation operation) {
-        String dateS = (String) operation.getValue();
-        try {
-            Date date = simpleDateFormat.parse(dateS);
-            if (resource.getEndDate() != null && resource.getEndDate().before(date)) {
-                throw new DSpaceBadRequestException("Attempting to set an invalid startDate greater than the endDate.");
-            }
-        } catch (ParseException e) {
-            throw new DSpaceBadRequestException("Invalid startDate value", e);
-        }
-    }
-}

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

@@ -5,19 +5,17 @@
  *
  * http://www.dspace.org/license/
  */
-package org.dspace.app.rest.repository.patch.factories.impl;
+package org.dspace.app.rest.repository.patch.operation;
 
 import java.sql.SQLException;
 
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
-import org.dspace.app.rest.model.BundleRest;
+import org.dspace.app.rest.model.patch.MoveOperation;
 import org.dspace.app.rest.model.patch.Operation;
-import org.dspace.app.rest.utils.ContextUtil;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Bundle;
 import org.dspace.content.service.BundleService;
 import org.dspace.core.Context;
-import org.dspace.services.RequestService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -32,27 +30,31 @@ import org.springframework.stereotype.Component;
  * </code>
  */
 @Component
-public class BundleMoveOperation extends MovePatchOperation<BundleRest, Integer> {
+public class BundleMoveOperation extends PatchOperation<Bundle> {
 
     @Autowired
     BundleService bundleService;
 
     @Autowired
-    RequestService requestService;
+    DSpaceObjectMetadataPatchUtils dspaceObjectMetadataPatchUtils;
+
+    private static final String OPERATION_PATH_BUNDLE_MOVE = "/_links/bitstreams/";
 
     /**
      * Executes the move patch operation.
      *
-     * @param resource  the rest model.
+     * @param bundle  the bundle in which we want to move files around.
      * @param operation the move patch operation.
      * @return the updated rest model.
      * @throws DSpaceBadRequestException
      */
     @Override
-    public BundleRest move(BundleRest resource, Operation operation) {
+    public Bundle perform(Context context, Bundle bundle, Operation operation) {
-        Context context = ContextUtil.obtainContext(requestService.getCurrentRequest().getServletRequest());
         try {
-            Bundle bundle = bundleService.findByIdOrLegacyId(context, resource.getId());
+            MoveOperation moveOperation = (MoveOperation) operation;
+            final int from = Integer.parseInt(dspaceObjectMetadataPatchUtils.getIndexFromPath(moveOperation.getFrom()));
+            final int to = Integer.parseInt(dspaceObjectMetadataPatchUtils.getIndexFromPath(moveOperation.getPath()));
+
             int totalAmount = bundle.getBitstreams().size();
 
             if (totalAmount < 1) {
@@ -78,29 +80,13 @@ public class BundleMoveOperation extends MovePatchOperation<BundleRest, Integer>
             throw new DSpaceBadRequestException(e.getMessage(), e);
         }
 
-        return resource;
+        return bundle;
     }
 
-    /**
-     * This method should return the typed array to be used in the
-     * LateObjectEvaluator evaluation of json arrays.
-     *
-     * @return
-     */
     @Override
-    protected Class<Integer[]> getArrayClassForEvaluation() {
+    public boolean supports(Object objectToMatch, Operation operation) {
-        return Integer[].class;
+        return (objectToMatch instanceof Bundle && operation.getOp().trim().equalsIgnoreCase(OPERATION_MOVE)
-    }
+                && operation.getPath().trim().startsWith(OPERATION_PATH_BUNDLE_MOVE));
-
-    /**
-     * This method should return the object type to be used in the
-     * LateObjectEvaluator evaluation of json objects.
-     *
-     * @return
-     */
-    @Override
-    protected Class<Integer> getClassForEvaluation() {
-        return Integer.class;
     }
 
     /**

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

@@ -0,0 +1,85 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import java.sql.SQLException;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.model.MetadataValueRest;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.MetadataField;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.DSpaceObjectService;
+import org.dspace.core.Context;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ Class for PATCH ADD operations on Dspace Objects' metadata
+ * Usage: (can be done on other dso than Item also):
+ * - ADD metadata (with schema.identifier.qualifier) value of a dso (here: Item) to end of list of md
+ * <code>
+ * curl -X PATCH http://${dspace.server.url}/api/core/items/<:id-item> -H "
+ * Content-Type: application/json" -d '[{ "op": "add", "path": "
+ * /metadata/schema.identifier.qualifier(/0|-)}", "value": "metadataValue"]'
+ * </code>
+ * @author Maria Verdonck (Atmire) on 18/11/2019
+ */
+@Component
+public class DSpaceObjectMetadataAddOperation<R extends DSpaceObject> extends PatchOperation<R> {
+
+    @Autowired
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
+
+    @Override
+    public R perform(Context context, R resource, Operation operation) throws SQLException {
+        DSpaceObjectService dsoService = ContentServiceFactory.getInstance().getDSpaceObjectService(resource);
+        MetadataValueRest metadataValueToAdd = metadataPatchUtils.extractMetadataValueFromOperation(operation);
+        MetadataField metadataField = metadataPatchUtils.getMetadataField(context, operation);
+        String indexInPath = metadataPatchUtils.getIndexFromPath(operation.getPath());
+
+        add(context, resource, dsoService, metadataField, metadataValueToAdd, indexInPath);
+        return resource;
+    }
+
+    /**
+     * 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 metadataField 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, MetadataField metadataField,
+                     MetadataValueRest metadataValue, String index) {
+        metadataPatchUtils.checkMetadataFieldNotNull(metadataField);
+        int indexInt = 0;
+        if (index != null && index.equals("-")) {
+            indexInt = -1;
+        }
+        try {
+            dsoService.addAndShiftRightMetadata(context, dso, metadataField.getMetadataSchema().getName(),
+                    metadataField.getElement(), metadataField.getQualifier(), metadataValue.getLanguage(),
+                    metadataValue.getValue(), metadataValue.getAuthority(), metadataValue.getConfidence(), indexInt);
+        } catch (SQLException e) {
+            throw new DSpaceBadRequestException("SQLException in DspaceObjectMetadataAddOperation.add trying to add " +
+                    "metadata to dso.", e);
+        }
+    }
+
+    @Override
+    public boolean supports(Object objectToMatch, Operation operation) {
+        return ((operation.getPath().startsWith(metadataPatchUtils.OPERATION_METADATA_PATH)
+                || operation.getPath().equals(metadataPatchUtils.OPERATION_METADATA_PATH))
+                && operation.getOp().trim().equalsIgnoreCase(OPERATION_ADD)
+                && objectToMatch instanceof DSpaceObject);
+    }
+}

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

@@ -0,0 +1,102 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import java.sql.SQLException;
+import java.util.List;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.exception.UnprocessableEntityException;
+import org.dspace.app.rest.model.MetadataValueRest;
+import org.dspace.app.rest.model.patch.CopyOperation;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
+import org.dspace.content.MetadataField;
+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.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ * Class for PATCH COPY operations on Dspace Objects' metadata
+ * Usage: (can be done on other dso than Item also):
+ * - COPY metadata (with schema.identifier.qualifier) value of a dso (here: Item) from given index to end of list of md
+ * <code>
+ * curl -X PATCH http://${dspace.server.url}/api/core/items/<:id-item> -H "
+ * Content-Type: application/json" -d '[{ "op": "copy",
+ * "from": "/metadata/schema.identifier.qualifier/indexToCopyFrom"
+ * "path": "/metadata/schema.identifier.qualifier/-"}]'
+ * </code>
+ *
+ * @author Maria Verdonck (Atmire) on 18/11/2019
+ */
+@Component
+public class DSpaceObjectMetadataCopyOperation<R extends DSpaceObject> extends PatchOperation<R> {
+
+    @Autowired
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
+
+    @Override
+    public R perform(Context context, R resource, Operation operation) throws SQLException {
+        DSpaceObjectService dsoService = ContentServiceFactory.getInstance().getDSpaceObjectService(resource);
+        MetadataField metadataField = metadataPatchUtils.getMetadataField(context, operation);
+        String[] partsFromCopy = ((CopyOperation) operation).getFrom().split("/");
+        String indexToCopyFrom = (partsFromCopy.length > 3) ? partsFromCopy[3] : null;
+
+        copy(context, resource, dsoService, metadataField, indexToCopyFrom);
+        return resource;
+    }
+
+    /**
+     * Copies metadata of the dso from indexFrom to new index at end of md
+     *
+     * @param context         context patch is being performed in
+     * @param dso             dso being patched
+     * @param dsoService      service doing the patch in db
+     * @param metadataField   md field being patched
+     * @param indexToCopyFrom index we're copying metadata from
+     */
+    private void copy(Context context, DSpaceObject dso, DSpaceObjectService dsoService, MetadataField metadataField,
+                      String indexToCopyFrom) {
+        metadataPatchUtils.checkMetadataFieldNotNull(metadataField);
+        List<MetadataValue> metadataValues = dsoService.getMetadata(dso, metadataField.getMetadataSchema().getName(),
+                metadataField.getElement(), metadataField.getQualifier(), Item.ANY);
+        try {
+            int indexToCopyFromInt = Integer.parseInt(indexToCopyFrom);
+            if (indexToCopyFromInt >= 0 && metadataValues.size() > indexToCopyFromInt
+                    && metadataValues.get(indexToCopyFromInt) != null) {
+                MetadataValue metadataValueToCopy = metadataValues.get(indexToCopyFromInt);
+                MetadataValueRest metadataValueRestToCopy
+                        = metadataPatchUtils.convertMdValueToRest(metadataValueToCopy);
+                // Add metadata value to end of md list
+                dsoService.addAndShiftRightMetadata(context, dso, metadataField.getMetadataSchema().getName(),
+                        metadataField.getElement(), metadataField.getQualifier(), metadataValueRestToCopy.getLanguage(),
+                        metadataValueRestToCopy.getValue(), metadataValueRestToCopy.getAuthority(),
+                        metadataValueRestToCopy.getConfidence(), -1);
+            } else {
+                throw new UnprocessableEntityException("There is no metadata of this type at that index");
+            }
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("This index (" + indexToCopyFrom + ") is not valid number.", e);
+        } catch (SQLException e) {
+            throw new DSpaceBadRequestException("SQLException in DspaceObjectMetadataCopyOperation.copy trying to " +
+                    "add metadata to dso.", e);
+        }
+    }
+
+    @Override
+    public boolean supports(Object objectToMatch, Operation operation) {
+        return ((operation.getPath().startsWith(metadataPatchUtils.OPERATION_METADATA_PATH)
+                || operation.getPath().equals(metadataPatchUtils.OPERATION_METADATA_PATH))
+                && operation.getOp().trim().equalsIgnoreCase(OPERATION_COPY)
+                && objectToMatch instanceof DSpaceObject);
+    }
+}

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

@@ -0,0 +1,84 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import java.sql.SQLException;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.model.patch.MoveOperation;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.MetadataField;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.DSpaceObjectService;
+import org.dspace.core.Context;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ * Class for PATCH MOVE operations on Dspace Objects' metadata
+ * Usage: (can be done on other dso than Item also):
+ * - MOVE metadata (with schema.identifier.qualifier) value of a dso (here: Item)
+ * from given index in from to given index in path
+ * <code>
+ * curl -X PATCH http://${dspace.server.url}/api/core/items/<:id-item> -H "
+ * Content-Type: application/json" -d '[{ "op": "move",
+ * "from": "/metadata/schema.identifier.qualifier/indexToCopyFrom"
+ * "path": "/metadata/schema.identifier.qualifier/indexToCopyTo"}]'
+ * </code>
+ *
+ * @author Maria Verdonck (Atmire) on 18/11/2019
+ */
+@Component
+public class DSpaceObjectMetadataMoveOperation<R extends DSpaceObject> extends PatchOperation<R> {
+
+    @Autowired
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
+
+    @Override
+    public R perform(Context context, R resource, Operation operation) throws SQLException {
+        DSpaceObjectService dsoService = ContentServiceFactory.getInstance().getDSpaceObjectService(resource);
+        MetadataField metadataField = metadataPatchUtils.getMetadataField(context, operation);
+        String indexInPath = metadataPatchUtils.getIndexFromPath(operation.getPath());
+        String indexToMoveFrom = metadataPatchUtils.getIndexFromPath(((MoveOperation) operation).getFrom());
+
+        move(context, resource, dsoService, metadataField, indexInPath, indexToMoveFrom);
+        return resource;
+    }
+
+    /**
+     * 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 metadataField 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, MetadataField metadataField, String indexFrom, String indexTo) {
+        metadataPatchUtils.checkMetadataFieldNotNull(metadataField);
+        try {
+            dsoService.moveMetadata(context, dso, metadataField.getMetadataSchema().getName(),
+                    metadataField.getElement(), metadataField.getQualifier(), Integer.parseInt(indexFrom),
+                    Integer.parseInt(indexTo));
+        } catch (SQLException e) {
+            throw new DSpaceBadRequestException("SQLException in DspaceObjectMetadataMoveOperation.move trying to " +
+                    "move metadata in dso.", e);
+        }
+    }
+
+    @Override
+    public boolean supports(Object objectToMatch, Operation operation) {
+        return ((operation.getPath().startsWith(metadataPatchUtils.OPERATION_METADATA_PATH)
+                || operation.getPath().equals(metadataPatchUtils.OPERATION_METADATA_PATH))
+                && operation.getOp().trim().equalsIgnoreCase(OPERATION_MOVE)
+                && objectToMatch instanceof DSpaceObject);
+    }
+}

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

@@ -0,0 +1,160 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import java.io.IOException;
+import java.sql.SQLException;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.lang3.StringUtils;
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.model.MetadataValueRest;
+import org.dspace.app.rest.model.patch.JsonValueEvaluator;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.MetadataField;
+import org.dspace.content.MetadataValue;
+import org.dspace.content.service.MetadataFieldService;
+import org.dspace.core.Context;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ * Util class for shared methods between the Metadata Operations
+ * @author Maria Verdonck (Atmire) on 18/11/2019
+ */
+@Component
+public final class DSpaceObjectMetadataPatchUtils {
+
+    private ObjectMapper objectMapper = new ObjectMapper();
+
+    @Autowired
+    private MetadataFieldService metadataFieldService;
+
+    /**
+     * Path in json body of patch that uses these metadata operations
+     */
+    protected static final String OPERATION_METADATA_PATH = "/metadata";
+
+    private DSpaceObjectMetadataPatchUtils() {
+    }
+
+    /**
+     * Extract metadataValue from Operation by parsing the json and mapping it to a MetadataValueRest
+     * @param operation     Operation whose value is begin parsed
+     * @return MetadataValueRest extracted from json in operation value
+     */
+    protected MetadataValueRest extractMetadataValueFromOperation(Operation operation) {
+        MetadataValueRest metadataValue = null;
+        try {
+            if (operation.getValue() != null) {
+                if (operation.getValue() instanceof JsonValueEvaluator) {
+                    JsonNode valueNode = ((JsonValueEvaluator) operation.getValue()).getValueNode();
+                    if (valueNode.isArray()) {
+                        metadataValue = objectMapper.treeToValue(valueNode.get(0), MetadataValueRest.class);
+                    } else {
+                        metadataValue = objectMapper.treeToValue(valueNode, MetadataValueRest.class);
+                    }
+                }
+                if (operation.getValue() instanceof String) {
+                    String valueString = (String) operation.getValue();
+                    metadataValue = new MetadataValueRest();
+                    metadataValue.setValue(valueString);
+                }
+            }
+        } catch (IOException e) {
+            throw new DSpaceBadRequestException("IOException in " +
+                    "DspaceObjectMetadataOperation.extractMetadataValueFromOperation trying to map json from " +
+                    "operation.value to MetadataValue class.", e);
+        }
+        if (metadataValue == null) {
+            throw new DSpaceBadRequestException("Could not extract MetadataValue Object from Operation");
+        }
+        return metadataValue;
+    }
+
+    /**
+     * Extracts the mdField String (schema.element.qualifier) from the operation and returns it
+     * @param operation The patch operation
+     * @return The mdField (schema.element.qualifier) patch is being performed on
+     */
+    protected String extractMdFieldStringFromOperation(Operation operation) {
+        String mdElement = StringUtils.substringBetween(operation.getPath(), OPERATION_METADATA_PATH + "/", "/");
+        if (mdElement == null) {
+            mdElement = StringUtils.substringAfter(operation.getPath(), OPERATION_METADATA_PATH + "/");
+            if (mdElement == null) {
+                throw new DSpaceBadRequestException("No metadata field string found in path: " + operation.getPath());
+            }
+        }
+        return mdElement;
+    }
+
+    /**
+     * Converts a metadataValue (database entity) to a REST equivalent of it
+     * @param md    Original metadataValue
+     * @return The REST equivalent
+     */
+    protected MetadataValueRest convertMdValueToRest(MetadataValue md) {
+        MetadataValueRest dto = new MetadataValueRest();
+        dto.setAuthority(md.getAuthority());
+        dto.setConfidence(md.getConfidence());
+        dto.setLanguage(md.getLanguage());
+        dto.setPlace(md.getPlace());
+        dto.setValue(md.getValue());
+        return dto;
+    }
+
+    /**
+     * Extracts which property of the metadata is being changed in the replace patch operation
+     * @param partsOfPath   Parts of the path of the operation, separated with /
+     * @return The property that is begin replaced of the metadata
+     */
+    protected String extractPropertyOfMdFromPath(String[] partsOfPath) {
+        return (partsOfPath.length > 4) ? partsOfPath[4] : null;
+    }
+
+    /**
+     * Extracts the new value of the metadata from the operation for the replace patch operation
+     * @param operation     The patch operation
+     * @return The new value of the metadata being replaced in the patch operation
+     */
+    protected String extractNewValueOfMd(Operation operation) {
+        if (operation.getValue() instanceof String) {
+            return (String) operation.getValue();
+        }
+        return null;
+    }
+
+    /**
+     * Retrieves metadataField based on the metadata element found in the operation
+     * @param context       Context the retrieve metadataField from service with string
+     * @param operation     Operation of the patch
+     * @return              The metadataField corresponding to the md element string of the operation
+     */
+    protected MetadataField getMetadataField(Context context, Operation operation) throws SQLException {
+        String mdElement = this.extractMdFieldStringFromOperation(operation);
+        return metadataFieldService.findByString(context, mdElement, '.');
+    }
+
+    /**
+     * Retrieved the index from the path of the patch operation, if one can be found
+     * @param path          The string from the operation
+     * @return              The index in the path if there is one (path ex: /metadata/dc.title/1 (1 being the index))
+     */
+    protected String getIndexFromPath(String path) {
+        String[] partsOfPath = path.split("/");
+        // Index of md being patched
+        return (partsOfPath.length > 3) ? partsOfPath[3] : null;
+    }
+
+    protected void checkMetadataFieldNotNull(MetadataField metadataField) {
+        if (metadataField == null) {
+            throw new DSpaceBadRequestException("There was no metadataField found in path of operation");
+        }
+    }
+}

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

@@ -0,0 +1,107 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.List;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.exception.UnprocessableEntityException;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
+import org.dspace.content.MetadataField;
+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.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ * Class for PATCH REMOVE operations on Dspace Objects' metadata
+ * Usage: (can be done on other dso than Item also):
+ * - REMOVE metadata (with schema.identifier.qualifier) value of a dso (here: Item)
+ * > Without index: removes all md values of that schema.identifier.qualifier type
+ * > With index: removes only that select md value
+ * <code>
+ * curl -X PATCH http://${dspace.server.url}/api/core/items/<:id-item> -H "
+ * Content-Type: application/json" -d '[{ "op": "remove",
+ * "path": "/metadata/schema.identifier.qualifier(/indexOfSpecificMdToRemove)"}]'
+ * </code>
+ *
+ * @author Maria Verdonck (Atmire) on 18/11/2019
+ */
+@Component
+public class DSpaceObjectMetadataRemoveOperation<R extends DSpaceObject> extends PatchOperation<R> {
+
+    @Autowired
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
+
+    @Override
+    public R perform(Context context, R resource, Operation operation) throws SQLException {
+        DSpaceObjectService dsoService = ContentServiceFactory.getInstance().getDSpaceObjectService(resource);
+        String indexInPath = metadataPatchUtils.getIndexFromPath(operation.getPath());
+        MetadataField metadataField = metadataPatchUtils.getMetadataField(context, operation);
+
+        remove(context, resource, dsoService, metadataField, indexInPath);
+        return resource;
+    }
+
+    /**
+     * 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 metadataField md field being patched
+     * @param index         index at where we want to delete metadata
+     */
+    private void remove(Context context, DSpaceObject dso, DSpaceObjectService dsoService, MetadataField metadataField,
+                        String index) {
+        metadataPatchUtils.checkMetadataFieldNotNull(metadataField);
+        try {
+            if (index == null) {
+                // remove all metadata of this type
+                dsoService.clearMetadata(context, dso, metadataField.getMetadataSchema().getName(),
+                        metadataField.getElement(), metadataField.getQualifier(), Item.ANY);
+            } else {
+                // remove metadata at index
+                List<MetadataValue> metadataValues = dsoService.getMetadata(dso,
+                        metadataField.getMetadataSchema().getName(), metadataField.getElement(),
+                        metadataField.getQualifier(), Item.ANY);
+                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(indexInt)));
+                } else {
+                    throw new UnprocessableEntityException("UnprocessableEntityException - There is no metadata of " +
+                            "this type at that index");
+                }
+            }
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("This index (" + index + ") is not valid number.", e);
+        } catch (ArrayIndexOutOfBoundsException e) {
+            throw new UnprocessableEntityException("There is no metadata of this type at that index");
+        } catch (SQLException e) {
+            throw new DSpaceBadRequestException("SQLException in DspaceObjectMetadataRemoveOperation.remove " +
+                    "trying to remove metadata from dso.", e);
+        }
+    }
+
+    @Override
+    public boolean supports(Object objectToMatch, Operation operation) {
+        return ((operation.getPath().startsWith(metadataPatchUtils.OPERATION_METADATA_PATH)
+                || operation.getPath().equals(metadataPatchUtils.OPERATION_METADATA_PATH))
+                && operation.getOp().trim().equalsIgnoreCase(OPERATION_REMOVE)
+                && objectToMatch instanceof DSpaceObject);
+    }
+}

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

@@ -0,0 +1,222 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import java.sql.SQLException;
+import java.util.List;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.exception.UnprocessableEntityException;
+import org.dspace.app.rest.model.MetadataValueRest;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
+import org.dspace.content.MetadataField;
+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.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ Class for PATCH REPLACE operations on Dspace Objects' metadata
+ * Usage: (can be done on other dso than Item also):
+ * - REPLACE metadata (with schema.identifier.qualifier) value of a dso (here: Item)
+ *      from existing value to new given value
+ * <code>
+ * curl -X PATCH http://${dspace.server.url}/api/core/items/<:id-item> -H "
+ * Content-Type: application/json" -d '[{ "op": "replace", "path": "
+ * /metadata/schema.identifier.qualifier}", "value": "newMetadataValue"]'
+ * </code>
+ * @author Maria Verdonck (Atmire) on 18/11/2019
+ */
+@Component
+public class DSpaceObjectMetadataReplaceOperation<R extends DSpaceObject> extends PatchOperation<R> {
+
+    @Autowired
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
+
+    @Override
+    public R perform(Context context, R resource, Operation operation) throws SQLException {
+        DSpaceObjectService dsoService = ContentServiceFactory.getInstance().getDSpaceObjectService(resource);
+        MetadataField metadataField = metadataPatchUtils.getMetadataField(context, operation);
+        String[] partsOfPath = operation.getPath().split("/");
+        // Index of md being patched
+        String indexInPath = (partsOfPath.length > 3) ? partsOfPath[3] : null;
+        MetadataValueRest metadataValueToReplace = metadataPatchUtils.extractMetadataValueFromOperation(operation);
+        // Property of md being altered
+        String propertyOfMd = metadataPatchUtils.extractPropertyOfMdFromPath(partsOfPath);
+        String newValueMdAttribute = metadataPatchUtils.extractNewValueOfMd(operation);
+
+        replace(context, resource, dsoService, metadataField, metadataValueToReplace, indexInPath, propertyOfMd,
+                newValueMdAttribute);
+        return resource;
+    }
+
+    /**
+     * Replaces metadata in the dso; 4 cases:
+     *      * - If we replace everything: clears all metadata
+     *      * - 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 alterations directly on this object
+     *      * - A single existing metadata property:
+     *      * Retrieve the metadatavalue object & make alterations 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 metadataField     possible md field being patched (if null all md gets cleared)
+     * @param metadataValue     value of md element
+     * @param index             possible index of md being replaced
+     * @param propertyOfMd      possible property of md being replaced
+     * @param valueMdProperty   possible new value of property of md being replaced
+     */
+    private void replace(Context context, DSpaceObject dso, DSpaceObjectService dsoService, MetadataField metadataField,
+                         MetadataValueRest metadataValue, String index, String propertyOfMd, String valueMdProperty) {
+        // replace entire set of metadata
+        if (metadataField == null) {
+            this.replaceAllMetadata(context, dso, dsoService);
+            return;
+        }
+
+        // replace all metadata for existing key
+        if (index == null) {
+            this.replaceMetadataFieldMetadata(context, dso, dsoService, metadataField, metadataValue);
+            return;
+        }
+        // replace single existing metadata value
+        if (propertyOfMd == null) {
+            this.replaceSingleMetadataValue(dso, dsoService, metadataField, metadataValue, index);
+            return;
+        }
+        // replace single property of exiting metadata value
+        this.replaceSinglePropertyOfMdValue(dso, dsoService, metadataField, index, propertyOfMd, valueMdProperty);
+    }
+
+    /**
+     * Clears all metadata of dso
+     * @param context           context patch is being performed in
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     */
+    private void replaceAllMetadata(Context context, DSpaceObject dso, DSpaceObjectService dsoService) {
+        try {
+            dsoService.clearMetadata(context, dso, Item.ANY, Item.ANY, Item.ANY, Item.ANY);
+        } catch (SQLException e) {
+            throw new DSpaceBadRequestException("SQLException in DspaceObjectMetadataOperation.replace trying to " +
+                    "remove and replace metadata from dso.", e);
+        }
+    }
+
+    /**
+     * Replaces all metadata for an existing single mdField with new value(s)
+     * @param context           context patch is being performed in
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param metadataField     md field being patched
+     * @param metadataValue     value of md element
+     */
+    private void replaceMetadataFieldMetadata(Context context, DSpaceObject dso, DSpaceObjectService dsoService,
+                                              MetadataField metadataField, MetadataValueRest metadataValue) {
+        try {
+            dsoService.clearMetadata(context, dso, metadataField.getMetadataSchema().getName(),
+                    metadataField.getElement(), metadataField.getQualifier(), Item.ANY);
+            dsoService.addAndShiftRightMetadata(context, dso, metadataField.getMetadataSchema().getName(),
+                    metadataField.getElement(), metadataField.getQualifier(), metadataValue.getLanguage(),
+                    metadataValue.getValue(), metadataValue.getAuthority(), metadataValue.getConfidence(), -1);
+        } catch (SQLException e) {
+            throw new DSpaceBadRequestException("SQLException in DspaceObjectMetadataOperation.replace trying to " +
+                    "remove and replace metadata from dso.", e);
+        }
+    }
+
+    /**
+     * Replaces metadata value of a single metadataValue object
+     *      Retrieve the metadatavalue object & make alerations directly on this object
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param metadataField     md field being patched
+     * @param metadataValue     new value of md element
+     * @param index             index of md being replaced
+     */
+    // replace single existing metadata value
+    private void replaceSingleMetadataValue(DSpaceObject dso, DSpaceObjectService dsoService,
+                                            MetadataField metadataField, MetadataValueRest metadataValue,
+                                            String index) {
+        try {
+            List<MetadataValue> metadataValues = dsoService.getMetadata(dso,
+                    metadataField.getMetadataSchema().getName(), metadataField.getElement(),
+                    metadataField.getQualifier(), 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());
+                dsoService.setMetadataModified(dso);
+            } 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 number.", e);
+        }
+    }
+
+    /**
+     * Replaces single property of a specific mdValue object
+     * @param dso               dso being patched
+     * @param dsoService        service doing the patch in db
+     * @param metadataField     md field being patched
+     * @param index             index of md being replaced
+     * @param propertyOfMd      property of md being replaced
+     * @param valueMdProperty   new value of property of md being replaced
+     */
+    private void replaceSinglePropertyOfMdValue(DSpaceObject dso, DSpaceObjectService dsoService,
+                                                MetadataField metadataField,
+                                                String index, String propertyOfMd, String valueMdProperty) {
+        try {
+            List<MetadataValue> metadataValues = dsoService.getMetadata(dso,
+                    metadataField.getMetadataSchema().getName(), metadataField.getElement(),
+                    metadataField.getQualifier(), 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(valueMdProperty);
+                }
+                if (propertyOfMd.equals("confidence")) {
+                    existingMdv.setConfidence(Integer.valueOf(valueMdProperty));
+                }
+                if (propertyOfMd.equals("language")) {
+                    existingMdv.setLanguage(valueMdProperty);
+                }
+                if (propertyOfMd.equals("value")) {
+                    existingMdv.setValue(valueMdProperty);
+                }
+                dsoService.setMetadataModified(dso);
+            } else {
+                throw new UnprocessableEntityException("There is no metadata of this type at that index");
+            }
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("Not all numbers are valid numbers. " +
+                    "(Index and confidence should be nr)", e);
+        }
+    }
+
+    @Override
+    public boolean supports(Object objectToMatch, Operation operation) {
+        return ((operation.getPath().startsWith(metadataPatchUtils.OPERATION_METADATA_PATH)
+                || operation.getPath().equals(metadataPatchUtils.OPERATION_METADATA_PATH))
+                && operation.getOp().trim().equalsIgnoreCase(OPERATION_REPLACE)
+                && objectToMatch instanceof DSpaceObject);
+    }
+}

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

@@ -0,0 +1,51 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.repository.patch.operation;
+
+import org.dspace.app.rest.exception.DSpaceBadRequestException;
+import org.dspace.app.rest.model.patch.Operation;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
+import org.springframework.stereotype.Component;
+
+/**
+ * Implementation for EPerson requires certificate patches.
+ *
+ * Example: <code>
+ * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
+ * Content-Type: application/json" -d '[{ "op": "replace", "path": "
+ * /certificate", "value": true|false]'
+ * </code>
+ */
+@Component
+public class EPersonCertificateReplaceOperation<R> extends PatchOperation<R> {
+
+    /**
+     * Path in json body of patch that uses this operation
+     */
+    private static final String OPERATION_PATH_CERTIFICATE = "/certificate";
+
+    @Override
+    public R perform(Context context, R object, Operation operation) {
+        checkOperationValue(operation.getValue());
+        Boolean requireCert = getBooleanOperationValue(operation.getValue());
+        if (supports(object, operation)) {
+            EPerson eperson = (EPerson) object;
+            eperson.setRequireCertificate(requireCert);
+            return object;
+        } else {
+            throw new DSpaceBadRequestException("EPersonCertificateReplaceOperation does not support this operation.");
+        }
+    }
+
+    @Override
+    public boolean supports(Object objectToMatch, Operation operation) {
+        return (objectToMatch instanceof EPerson && operation.getOp().trim().equalsIgnoreCase(OPERATION_REPLACE)
+                && operation.getPath().trim().equalsIgnoreCase(OPERATION_PATH_CERTIFICATE));
+    }
+}