DS-3127: Prevent database updates when directly manipulating the bistream list of a bundle

dspace-api/src/main/java/org/dspace/app/util/GoogleMetadata.java

@@ -7,34 +7,30 @@
  */
 package org.dspace.app.util;
 
-import java.sql.SQLException;
-
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ListMultimap;
+import org.apache.log4j.Logger;
 import org.dspace.authorize.factory.AuthorizeServiceFactory;
 import org.dspace.content.*;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.*;
-
-import org.apache.log4j.Logger;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.ItemService;
 import org.dspace.core.ConfigurationManager;
-
-import java.io.File;
-import java.io.UnsupportedEncodingException;
-import java.util.Collection;
-import java.util.Map.Entry;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
-
 import org.dspace.handle.factory.HandleServiceFactory;
 import org.jdom.Element;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.sql.SQLException;
+import java.util.*;
+import java.util.Collection;
+import java.util.Map.Entry;
+
 /**
  * Configuration and mapping for Google Scholar output metadata
  * @author Sands Fish
@@ -1057,11 +1053,12 @@ public class GoogleMetadata
      */
     protected Bitstream findLinkableFulltext(Item item) throws SQLException {
         Bitstream bestSoFar = null;
-        int bitstreamCount = 0;
+
         List<Bundle> contentBundles = itemService.getBundles(item, "ORIGINAL");
         for (Bundle bundle : contentBundles) {
             List<Bitstream> bitstreams = bundle.getBitstreams();
             Collections.sort(bitstreams, googleBitstreamComparator);
+
             for (Bitstream candidate : bitstreams) {
                 if (candidate.equals(bundle.getPrimaryBitstream())) { // is primary -> use this one
                     if (isPublic(candidate)) {

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

@@ -261,7 +261,7 @@ public class BitstreamServiceImpl extends DSpaceObjectServiceImpl<Bitstream> imp
         //Remove our bitstream from all our bundles
         final List<Bundle> bundles = bitstream.getBundles();
         for (Bundle bundle : bundles) {
-            bundle.getBitstreams().remove(bitstream);
+            bundle.removeBitstream(bitstream);
         }
 
         //Remove all bundles from the bitstream object, clearing the connection in 2 ways

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

@@ -10,6 +10,7 @@ package org.dspace.content;
 import java.sql.SQLException;
 import java.util.*;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.BundleService;
 import org.dspace.core.Constants;
@@ -130,18 +131,40 @@ public class Bundle extends DSpaceObject implements DSpaceObjectLegacySupport
     }
     
     /**
-     * Get the bitstreams in this bundle
+     * Get a copy of the bitstream list of this bundle
+     * Note that this is a copy and if you wish to manipulate the bistream list, you should use
+     * {@ref Bundle.addBitstream}, {@ref Bundle.removeBitstream} or {@ref Bundle.clearBitstreams}
      * 
      * @return the bitstreams
      */
     public List<Bitstream> getBitstreams() {
-        return bitstreams;
+        List<Bitstream> bitstreamList = new LinkedList<>(this.bitstreams);
+        return bitstreamList;
     }
 
+    /**
+     * Add a new bitstream to this bundle.
+     * @param bitstream
+     */
     void addBitstream(Bitstream bitstream){
         bitstreams.add(bitstream);
     }
 
+    /**
+     * Clear the list of bitstream of this bundle
+     */
+    public void clearBitstreams() {
+        bitstreams.clear();
+    }
+
+    /**
+     * Remove the given bitstream from this bundles bitstream list
+     * @param bitstream The bitstream to remove
+     */
+    public void removeBitstream(Bitstream bitstream) {
+        bitstreams.remove(bitstream);
+    }
+
     /**
      * Get the items this bundle appears in
      * 
@@ -215,5 +238,4 @@ public class Bundle extends DSpaceObject implements DSpaceObjectLegacySupport
         }
         return bundleService;
     }
-
 }

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

@@ -198,7 +198,7 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
             // We don't need to remove the link between bundle & bitstream, this will be handled in the delete() method.
             bitstreamService.delete(context, bitstream);
         }else{
-            bundle.getBitstreams().remove(bitstream);
+            bundle.removeBitstream(bitstream);
             bitstream.getBundles().remove(bundle);
         }
     }
@@ -304,13 +304,13 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
         if(CollectionUtils.isNotEmpty(updatedBitstreams) && !updatedBitstreams.equals(currentBitstreams))
         {
             //First clear out the existing list of bitstreams
-            bundle.getBitstreams().clear();
+            bundle.clearBitstreams();
 
             // Now add them back in the proper order
             for (Bitstream bitstream : updatedBitstreams)
             {
                 bitstream.getBundles().remove(bundle);
-                bundle.getBitstreams().add(bitstream);
+                bundle.addBitstream(bitstream);
                 bitstream.getBundles().add(bundle);
                 bitstreamService.update(context, bitstream);
             }
@@ -430,8 +430,8 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
                 bundle.getName(), getIdentifiers(context, bundle)));
 
         // Remove bitstreams
-        List<Bitstream> bitstreams = new LinkedList<>(bundle.getBitstreams());
+        List<Bitstream> bitstreams = bundle.getBitstreams();
-        bundle.getBitstreams().clear();
+        bundle.clearBitstreams();
         for (Bitstream bitstream : bitstreams) {
             removeBitstream(context, bundle, bitstream);
         }