64640: BundleRestRepository IT, JavaDocs and CheckStyle fixes

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V6.0_2015.03.07__DS-2701_Hibernate_migration.sql

@@ -189,16 +189,19 @@ ALTER TABLE item2bundle add primary key (item_id,bundle_id);
 --Migrate Bundle2Bitsteam
 ALTER TABLE bundle2bitstream ALTER COLUMN bundle_id rename to bundle_legacy_id;
 ALTER TABLE bundle2bitstream ALTER COLUMN bitstream_id rename to bitstream_legacy_id;
+ALTER TABLE bundle2bitstream ALTER COLUMN bitstream_order rename to bitstream_order_legacy;
 ALTER TABLE bundle2bitstream ADD COLUMN bundle_id UUID NOT NULL;
 ALTER TABLE bundle2bitstream ADD CONSTRAINT bundle2bitstream_bundle_id_fk FOREIGN KEY (bundle_id) REFERENCES bundle;
 ALTER TABLE bundle2bitstream ADD COLUMN bitstream_id UUID NOT NULL;
 ALTER TABLE bundle2bitstream ADD CONSTRAINT bundle2bitstream_bitstream_id_fk FOREIGN KEY (bitstream_id) REFERENCES bitstream;
+ALTER TABLE bundle2bitstream ADD COLUMN bitstream_order INTEGER NOT NULL;
 UPDATE bundle2bitstream SET bundle_id = (SELECT bundle.uuid FROM bundle WHERE bundle2bitstream.bundle_legacy_id = bundle.bundle_id);
 UPDATE bundle2bitstream SET bitstream_id = (SELECT bitstream.uuid FROM bitstream WHERE bundle2bitstream.bitstream_legacy_id = bitstream.bitstream_id);
 ALTER TABLE bundle2bitstream DROP COLUMN bundle_legacy_id;
 ALTER TABLE bundle2bitstream DROP COLUMN bitstream_legacy_id;
+ALTER TABLE bundle2bitstream DROP COLUMN bitstream_order_legacy;
 ALTER TABLE bundle2bitstream DROP COLUMN id;
-ALTER TABLE bundle2bitstream add primary key (bitstream_id,bundle_id);
+ALTER TABLE bundle2bitstream add primary key (bitstream_id,bundle_id,bitstream_order);
 
 
 -- Migrate item

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

@@ -18,7 +18,6 @@ import org.dspace.app.rest.model.hateoas.BundleResource;
 import org.dspace.app.rest.model.hateoas.DSpaceResource;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.app.rest.repository.patch.BundlePatch;
-import org.dspace.app.rest.repository.patch.DSpaceObjectPatch;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Bundle;
 import org.dspace.content.service.BundleService;

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

@@ -1,5 +1,14 @@
+/**
+ * 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;
@@ -8,7 +17,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 /**
- * Created by kristof on 24/09/2019
+ * Provides PATCH operations for bundle updates.
  */
 @Component
 public class BundlePatch extends DSpaceObjectPatch<BundleRest> {
@@ -16,6 +25,13 @@ 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/factories/BundleOperationFactory.java

@@ -1,3 +1,10 @@
+/**
+ * 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;
@@ -7,7 +14,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 /**
- * Created by kristof on 24/09/2019
+ * Provides factory methods for obtaining instances of bundle patch operations.
  */
 @Component
 public class BundleOperationFactory {
@@ -15,6 +22,9 @@ 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/impl/BundleMoveOperation.java

@@ -1,3 +1,10 @@
+/**
+ * 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.sql.SQLException;
@@ -15,7 +22,14 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 /**
- * Created by kristof on 24/09/2019
+ * This is the implementation for Bundle move patches.
+ * This operation moves bitstreams within a bundle from one index to another.
+ *
+ * Example: <code>
+ * curl -X PATCH http://${dspace.url}/api/bundles/<:id-bundle> -H "
+ * Content-Type: application/json" -d '[{ "op": "move", "path": "
+ * /_links/bitstreams/1/href", "from": "/_links/bitstreams/0/href"]'
+ * </code>
  */
 @Component
 public class BundleMoveOperation extends MovePatchOperation<BundleRest, Integer> {
@@ -33,14 +47,22 @@ public class BundleMoveOperation extends MovePatchOperation<BundleRest, Integer>
             Bundle bundle = bundleService.findByIdOrLegacyId(context, resource.getId());
             int totalAmount = bundle.getBitstreams().size();
 
-            if(totalAmount < 1) {
+            if (totalAmount < 1) {
-                throw new DSpaceBadRequestException(createMoveExceptionMessage(bundle, from, to, "No sub-communities found."));
+                throw new DSpaceBadRequestException(
+                        createMoveExceptionMessage(bundle, from, to, "No sub-communities found.")
+                );
             }
-            if(from >= totalAmount) {
+            if (from >= totalAmount) {
-                throw new DSpaceBadRequestException(createMoveExceptionMessage(bundle, from, to, "\"from\" location out of bounds. Latest available position: " + (totalAmount-1)));
+                throw new DSpaceBadRequestException(
+                        createMoveExceptionMessage(bundle, from, to,
+                                "\"from\" location out of bounds. Latest available position: " + (totalAmount - 1))
+                );
             }
-            if(to >= totalAmount) {
+            if (to >= totalAmount) {
-                throw new DSpaceBadRequestException(createMoveExceptionMessage(bundle, from, to, "\"to\" location out of bounds. Latest available position: " + (totalAmount-1)));
+                throw new DSpaceBadRequestException(
+                        createMoveExceptionMessage(bundle, from, to,
+                                "\"to\" location out of bounds. Latest available position: " + (totalAmount - 1))
+                );
             }
 
             bundleService.moveBitstream(context, bundle, from, to);
@@ -62,7 +84,8 @@ public class BundleMoveOperation extends MovePatchOperation<BundleRest, Integer>
     }
 
     private String createMoveExceptionMessage(Bundle bundle, int from, int to, String message) {
-        return "Failed moving bitstreams of bundle with id " + bundle.getID() + " from location " + from + " to " + to + ": " + message;
+        return "Failed moving bitstreams of bundle with id " +
+                bundle.getID() + " from location " + from + " to " + to + ": " + message;
     }
 
 }

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

@@ -1,52 +1,101 @@
+/**
+ * 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;
 
 /**
- * Created by kristof on 24/09/2019
+ * 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)) {
+        if (!(operation instanceof MoveOperation)) {
-            throw new DSpaceBadRequestException("Expected a MoveOperation, but received " + operation.getClass().getName() + " instead.");
+            throw new DSpaceBadRequestException(
+                    "Expected a MoveOperation, but received " + operation.getClass().getName() + " instead."
+            );
         }
         from = getLocationFromPath(((MoveOperation)operation).getFrom());
         to = getLocationFromPath(operation.getPath());
-        if(from == to) {
+        if (from == to) {
-            throw new DSpaceBadRequestException("The \"from\" location must be different from the \"to\" location.");
+            throw new DSpaceBadRequestException(
+                    "The \"from\" location must be different from the \"to\" location."
+            );
         }
-        if(from < 0) {
+        if (from < 0) {
             throw new DSpaceBadRequestException("A negative \"from\" location was provided: " + from);
         }
-        if(to < 0) {
+        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 (parts.length > 1) {
-            if(StringUtils.equals(parts[parts.length-1], "href")) {
+            if (StringUtils.equals(parts[parts.length - 1], "href")) {
-                locationStr = parts[parts.length-2];
+                locationStr = parts[parts.length - 2];
             } else {
-                locationStr = parts[parts.length-1];
+                locationStr = parts[parts.length - 1];
             }
         } else {
             locationStr = parts[0];

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

@@ -12,6 +12,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.is;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
@@ -19,9 +20,12 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
+import javax.ws.rs.core.MediaType;
+
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.codec.CharEncoding;
 import org.apache.commons.io.IOUtils;
@@ -38,6 +42,8 @@ import org.dspace.app.rest.matcher.MetadataMatcher;
 import org.dspace.app.rest.model.BundleRest;
 import org.dspace.app.rest.model.MetadataRest;
 import org.dspace.app.rest.model.MetadataValueRest;
+import org.dspace.app.rest.model.patch.MoveOperation;
+import org.dspace.app.rest.model.patch.Operation;
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
 import org.dspace.authorize.ResourcePolicy;
 import org.dspace.content.Bitstream;
@@ -361,5 +367,60 @@ public class BundleRestRepositoryIT extends AbstractControllerIntegrationTest {
                    )));
     }
 
+    @Test
+    public void patchMoveBitstreams() throws Exception {
+        context.turnOffAuthorisationSystem();
+
+        String bitstreamContent = "Dummy content";
+        try (InputStream is = IOUtils.toInputStream(bitstreamContent, CharEncoding.UTF_8)) {
+            bitstream1 = BitstreamBuilder.createBitstream(context, item, is)
+                    .withName("Bitstream")
+                    .withDescription("Description")
+                    .withMimeType("text/plain")
+                    .build();
+            bitstream2 = BitstreamBuilder.createBitstream(context, item, is)
+                    .withName("Bitstream2")
+                    .withDescription("Description2")
+                    .withMimeType("text/plain")
+                    .build();
+        }
+
+        bundle1 = BundleBuilder.createBundle(context, item)
+                .withName("testname")
+                .withBitstream(bitstream1)
+                .withBitstream(bitstream2)
+                .build();
+
+        context.restoreAuthSystemState();
+
+        getClient().perform(get("/api/core/bundles/" + bundle1.getID() + "/bitstreams"))
+                .andExpect(status().isOk())
+                .andExpect(content().contentType(contentType))
+                .andExpect(jsonPath("$._embedded.bitstreams", Matchers.contains(
+                        BitstreamMatcher.matchBitstreamEntry(bitstream1),
+                        BitstreamMatcher.matchBitstreamEntry(bitstream2)
+                )));
+
+        List<Operation> ops = new ArrayList<Operation>();
+        MoveOperation moveOperation = new MoveOperation("/_links/bitstreams/0/href",
+                "/_links/bitstreams/1/href");
+        ops.add(moveOperation);
+        String patchBody = getPatchContent(ops);
+
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(patch("/api/core/bundles/" + bundle1.getID())
+                .content(patchBody)
+                .contentType(MediaType.APPLICATION_JSON_PATCH_JSON))
+                    .andExpect(status().isOk());
+
+        getClient().perform(get("/api/core/bundles/" + bundle1.getID() + "/bitstreams"))
+                .andExpect(status().isOk())
+                .andExpect(content().contentType(contentType))
+                .andExpect(jsonPath("$._embedded.bitstreams", Matchers.contains(
+                        BitstreamMatcher.matchBitstreamEntry(bitstream2),
+                        BitstreamMatcher.matchBitstreamEntry(bitstream1)
+                )));
+    }
 
 }