ZCU-PUB/SWORDv2 - Cannot update bitstream of archived Item (#944)

* Using for instead of Iterator in removeBundle

* Added a test to test file change in the Item

* Use a default value for the plugin.single.org.dspace.sword2.WorkflowManager

* Added a doc why we did that change

---------

Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk>
Co-authored-by: milanmajchrak <milan.majchrak@dataquest.sk>

Author: 3 people

dspace-server-webapp/src/test/java/org/dspace/app/sword2/Swordv2IT.java

@@ -13,6 +13,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
+import java.io.InputStream;
 import java.nio.file.Path;
 import java.sql.SQLException;
 import java.util.List;
@@ -620,5 +621,54 @@ public void statementTest() throws Exception {
         assertThat(response.getBody(),
                    containsString("<category term=\"http://dspace.org/state/archived\""));
     }
-}
 
+    @Test
+    public void updateFileTest() throws Exception {
+        context.turnOffAuthorisationSystem();
+        // Create all content as the SAME EPERSON we will use to authenticate on this endpoint.
+        // THIS IS REQUIRED as the /statements endpoint will only show YOUR ITEM SUBMISSIONS.
+        context.setCurrentUser(eperson);
+        // Create a top level community and one Collection
+        parentCommunity = CommunityBuilder.createCommunity(context)
+                .withName("Parent Community")
+                .build();
+        Collection collection = CollectionBuilder.createCollection(context, parentCommunity)
+                .withName("Test SWORDv2 Collection")
+                .build();
+
+        // Add one Item into that Collection.
+        InputStream pdf = getClass().getResourceAsStream("simple-article.pdf");
+        WorkspaceItem witem = WorkspaceItemBuilder.createWorkspaceItem(context, collection)
+                .withTitle("Test WorkspaceItem")
+                .withIssueDate("2017-10-17")
+                .withFulltext("simple-article.pdf", "/local/path/simple-article.pdf", pdf)
+                .build();
+        Item item = witem.getItem();
+
+        // Above changes MUST be committed to the database for SWORDv2 to see them.
+        context.commit();
+        context.restoreAuthSystemState();
+
+        String editLink = getURL(MEDIA_RESOURCE_PATH + "/" + item.getID().toString());
+        // Load the ZIP file from the filesystem
+        FileSystemResource zipFile = new FileSystemResource(
+                Path.of("src", "test", "resources",
+                        "org", "dspace", "app", "sword2", "example.zip").toFile()
+        );
+
+        // Set up headers
+        HttpHeaders headers = new HttpHeaders();
+        headers.setBasicAuth(admin.getEmail(), password);
+        headers.setContentType(MediaType.valueOf("application/zip"));
+        headers.setContentDisposition(ContentDisposition.attachment().filename("example.zip").build());
+
+        RequestEntity<FileSystemResource> request = RequestEntity
+                .put(editLink)
+                .headers(headers)
+                .body(zipFile);
+        ResponseEntity<String> response = responseAsString(request);
+
+        // Expect a 200 response with ATOM feed content returned
+        assertEquals(HttpStatus.NO_CONTENT, response.getStatusCode());
+    }
+}

dspace-swordv2/src/main/java/org/dspace/sword2/VersionManager.java

@@ -10,6 +10,7 @@
 import java.io.IOException;
 import java.sql.SQLException;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
@@ -43,14 +44,17 @@ public void removeBundle(Context context, Item item, String name)
         throws SQLException, AuthorizeException, IOException {
         boolean keep = configurationService
             .getBooleanProperty("swordv2-server.versions.keep");
-        Iterator<Bundle> bundles = item.getBundles().iterator();
-        while (bundles.hasNext()) {
-            Bundle b = bundles.next();
+        // Add the bundle to the list of bundles to remove because the method `this.removeBundle`
+        // modifies the item's bundles, which can cause a ConcurrentModificationException.
+        List<Bundle> bundlesToRemove = new ArrayList<>();
+        for (Bundle b : item.getBundles()) {
             if (name.equals(b.getName())) {
-                bundles.remove();
-                this.removeBundle(context, item, b, keep);
+                bundlesToRemove.add(b);
             }
         }
+        for (Bundle b : bundlesToRemove) {
+            this.removeBundle(context, item, b, keep);
+        }
     }
 
     public void removeBundle(Context context, Item item, Bundle source)