DS-4389 improving patch system framework - Minor feedback changes PR 2591

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

@@ -91,7 +91,6 @@ 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 {
@@ -107,8 +106,7 @@ public class MetadataFieldServiceImpl implements MetadataFieldService {
         if (schema == null || element == null) {
             return null;
         } else {
-            MetadataSchema metadataSchema = metadataSchemaService.find(context, schema);
+            return this.findByElement(context, schema, element, qualifier);
-            return this.findByElement(context, metadataSchema, element, qualifier);
         }
     }
 

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
@@ -372,7 +372,7 @@ public interface DSpaceObjectService<T extends DSpaceObject> {
 
     /**
      * Add a single metadata field. Whether it's appended or prepended depends on index parameter.
-     * Use <code>clearDC</code> to remove values.
+     * Use <code>clearMetadata</code> to remove values.
      *
      * @param context    DSpace context
      * @param dso        DSpaceObject

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

@@ -46,7 +46,7 @@ public class ResourcePatch<M extends DSpaceObject> {
 
     /**
      * Checks with all possible patch operations whether they support this operation
-     *      (based on instanceof restModel and operation.path
+     *      (based on instanceof dso and operation.path)
      * @param context       Context of patch operation
      * @param dso           the dso resource to patch
      * @param operation     the patch operation

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

@@ -36,7 +36,7 @@ public class BundleMoveOperation extends PatchOperation<Bundle> {
     BundleService bundleService;
 
     @Autowired
-    DspaceObjectMetadataPatchUtils dspaceObjectMetadataPatchUtils;
+    DSpaceObjectMetadataPatchUtils dspaceObjectMetadataPatchUtils;
 
     private static final String OPERATION_PATH_BUNDLE_MOVE = "/_links/bitstreams/";
 

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

@@ -32,10 +32,10 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 18/11/2019
  */
 @Component
-public class DspaceObjectMetadataAddOperation<R extends DSpaceObject> extends PatchOperation<R> {
+public class DSpaceObjectMetadataAddOperation<R extends DSpaceObject> extends PatchOperation<R> {
 
     @Autowired
-    DspaceObjectMetadataPatchUtils metadataPatchUtils;
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
 
     @Override
     public R perform(Context context, R resource, Operation operation) throws SQLException {

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

@@ -39,10 +39,10 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 18/11/2019
  */
 @Component
-public class DspaceObjectMetadataCopyOperation<R extends DSpaceObject> extends PatchOperation<R> {
+public class DSpaceObjectMetadataCopyOperation<R extends DSpaceObject> extends PatchOperation<R> {
 
     @Autowired
-    DspaceObjectMetadataPatchUtils metadataPatchUtils;
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
 
     @Override
     public R perform(Context context, R resource, Operation operation) throws SQLException {
@@ -85,7 +85,7 @@ public class DspaceObjectMetadataCopyOperation<R extends DSpaceObject> extends P
                 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 nr", 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);

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

@@ -35,10 +35,10 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 18/11/2019
  */
 @Component
-public class DspaceObjectMetadataMoveOperation<R extends DSpaceObject> extends PatchOperation<R> {
+public class DSpaceObjectMetadataMoveOperation<R extends DSpaceObject> extends PatchOperation<R> {
 
     @Autowired
-    DspaceObjectMetadataPatchUtils metadataPatchUtils;
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
 
     @Override
     public R perform(Context context, R resource, Operation operation) throws SQLException {

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

@@ -29,7 +29,7 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 18/11/2019
  */
 @Component
-public final class DspaceObjectMetadataPatchUtils {
+public final class DSpaceObjectMetadataPatchUtils {
 
     private ObjectMapper objectMapper = new ObjectMapper();
 
@@ -41,7 +41,7 @@ public final class DspaceObjectMetadataPatchUtils {
      */
     protected static final String METADATA_PATH = "/metadata";
 
-    private DspaceObjectMetadataPatchUtils() {
+    private DSpaceObjectMetadataPatchUtils() {
     }
 
     /**
@@ -149,8 +149,7 @@ public final class DspaceObjectMetadataPatchUtils {
     protected String getIndexFromPath(String path) {
         String[] partsOfPath = path.split("/");
         // Index of md being patched
-        String indexInPath = (partsOfPath.length > 3) ? partsOfPath[3] : null;
+        return (partsOfPath.length > 3) ? partsOfPath[3] : null;
-        return indexInPath;
     }
 
     protected void checkMetadataFieldNotNull(MetadataField metadataField) {

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

@@ -39,10 +39,10 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 18/11/2019
  */
 @Component
-public class DspaceObjectMetadataRemoveOperation<R extends DSpaceObject> extends PatchOperation<R> {
+public class DSpaceObjectMetadataRemoveOperation<R extends DSpaceObject> extends PatchOperation<R> {
 
     @Autowired
-    DspaceObjectMetadataPatchUtils metadataPatchUtils;
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
 
     @Override
     public R perform(Context context, R resource, Operation operation) throws SQLException {
@@ -88,7 +88,7 @@ public class DspaceObjectMetadataRemoveOperation<R extends DSpaceObject> extends
                 }
             }
         } catch (NumberFormatException e) {
-            throw new IllegalArgumentException("This index (" + index + ") is not valid nr", 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) {

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

@@ -37,10 +37,10 @@ import org.springframework.stereotype.Component;
  * @author Maria Verdonck (Atmire) on 18/11/2019
  */
 @Component
-public class DspaceObjectMetadataReplaceOperation<R extends DSpaceObject> extends PatchOperation<R> {
+public class DSpaceObjectMetadataReplaceOperation<R extends DSpaceObject> extends PatchOperation<R> {
 
     @Autowired
-    DspaceObjectMetadataPatchUtils metadataPatchUtils;
+    DSpaceObjectMetadataPatchUtils metadataPatchUtils;
 
     @Override
     public R perform(Context context, R resource, Operation operation) throws SQLException {
@@ -166,7 +166,7 @@ public class DspaceObjectMetadataReplaceOperation<R extends DSpaceObject> extend
                 throw new UnprocessableEntityException("There is no metadata of this type at that index");
             }
         } catch (NumberFormatException e) {
-            throw new IllegalArgumentException("This index (" + index + ") is not valid nr", e);
+            throw new IllegalArgumentException("This index (" + index + ") is not valid number.", e);
         }
     }
 

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

@@ -15,7 +15,7 @@ import org.dspace.core.Context;
 import org.springframework.stereotype.Component;
 
 /**
- * This is the implementation for Item resource patches.
+ * This is the implementation for Item 'discoverable' patches.
  *
  * Example: <code>
  * curl -X PATCH http://${dspace.url}/api/core/items/<:id-item> -H "
@@ -37,10 +37,8 @@ public class ItemDiscoverableReplaceOperation<R> extends PatchOperation<R> {
         Boolean discoverable = getBooleanOperationValue(operation.getValue());
         if (supports(object, operation)) {
             Item item = (Item) object;
-            if (discoverable) {
+            if (discoverable && item.getTemplateItemOf() != null) {
-                if (item.getTemplateItemOf() != null) {
+                throw new UnprocessableEntityException("A template item cannot be discoverable.");
-                    throw new UnprocessableEntityException("A template item cannot be discoverable.");
-                }
             }
             item.setDiscoverable(discoverable);
             return object;

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

@@ -21,7 +21,7 @@ import org.dspace.core.Context;
 import org.springframework.stereotype.Component;
 
 /**
- * This is the implementation for Item resource patches.
+ * This is the implementation for Item 'withdrawn' patches.
  * <p>
  * Example: <code>
  * curl -X PATCH http://${dspace.url}/api/core/items/<:id-item> -H "

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

@@ -71,7 +71,7 @@ public abstract class PatchOperation<M> {
     }
 
     /**
-     * Determines whether or not this Patch Operation can do this patch (RestModel and path gets checked)
+     * Determines whether or not this Patch Operation can do this patch (Object of operation and path gets checked)
      * @param objectToMatch    Object whose class must be instance of type object
      *                              for which this PatchOperation was created
      * @param operation        Operation of the patch, should match this type of Patch Operation

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

@@ -37,7 +37,6 @@ import org.springframework.stereotype.Component;
 public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin {
 
     private static final Logger log = LoggerFactory.getLogger(EPersonRestPermissionEvaluatorPlugin.class);
-    public static final String OPERATION_PASSWORD_CHANGE = "/password";
 
     @Autowired
     AuthorizeService authorizeService;