Enable optimized selectcollection logic and add tests

You can run a standalone test via:
/dspace/bin/dspace dsrun org.dspace.app.util.OptimizeSelectCollection

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

@@ -22,6 +22,7 @@ public class OptimizeSelectCollection {
     private static Context context;
 
     private static ArrayList<EPerson> brokenPeople;
+    private static Long timeSavedMS = 0L;
 
     public static void main(String[] argv) throws Exception
     {
@@ -31,17 +32,21 @@ public class OptimizeSelectCollection {
 
         context = new Context();
         brokenPeople = new ArrayList<EPerson>();
+        int peopleChecked = 0;
+        timeSavedMS = 0L;
 
         if(argv != null && argv.length > 0) {
             for(String email : argv) {
                 EPerson person = EPerson.findByEmail(context, email);
                 checkSelectCollectionForUser(person);
+                peopleChecked++;
             }
         } else {
             //default case, run as specific user, or run all...
             EPerson[] people = EPerson.findAll(context, EPerson.EMAIL);
             for(EPerson person : people) {
                 checkSelectCollectionForUser(person);
+                peopleChecked++;
             }
         }
 
@@ -50,6 +55,8 @@ public class OptimizeSelectCollection {
             for(EPerson person : brokenPeople) {
                 System.out.println("-- " + person.getEmail());
             }
+        } else {
+            System.out.println("All Good: " + peopleChecked + " people have been checked, with same submission powers. TimeSaved(ms): " + timeSavedMS);
         }
 
     }
@@ -63,7 +70,7 @@ public class OptimizeSelectCollection {
         stopWatch.start("findAuthorized");
         Collection[] collections = Collection.findAuthorized(context, null, Constants.ADD);
         stopWatch.stop();
-
+        Long defaultMS = stopWatch.getLastTaskTimeMillis();
 
         stopWatch.start("ListingCollections");
         System.out.println("Legacy Find Authorized");
@@ -73,6 +80,9 @@ public class OptimizeSelectCollection {
         stopWatch.start("findAuthorizedOptimized");
         Collection[] collectionsOptimized = Collection.findAuthorizedOptimized(context, Constants.ADD);
         stopWatch.stop();
+        Long optimizedMS = stopWatch.getLastTaskTimeMillis();
+        timeSavedMS += defaultMS - optimizedMS;
+
 
         stopWatch.start("ListingCollectionsWithOptimizedCollections");
         System.out.println("Find Authorized Optimized");

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

@@ -10,10 +10,17 @@ package org.dspace.content;
 import java.io.File;
 import java.io.FileInputStream;
 import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.hadoop.security.authorize.ServiceAuthorizationManager;
 import org.dspace.authorize.AuthorizeException;
 import org.apache.log4j.Logger;
 import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
+import org.elasticsearch.common.collect.Lists;
 import org.junit.*;
 import static org.junit.Assert.* ;
 import static org.hamcrest.CoreMatchers.*;
@@ -1821,6 +1828,76 @@ public class CollectionTest extends AbstractDSpaceObjectTest
         assertTrue("testFindAuthorized 10",found.length >= 1);
     }
 
+    /**
+     * Test of findAuthorizedOptimized method, of class Collection.
+     * We create some collections and some users with varying auth, and ensure we can access them all properly.
+     */
+    @Test
+    public void testFindAuthorizedOptimized() throws Exception
+    {
+        context.turnOffAuthorisationSystem();
+        Community com = Community.create(null, context);
+        Collection collectionA = Collection.create(context);
+        Collection collectionB = Collection.create(context);
+        Collection collectionC = Collection.create(context);
+
+        com.addCollection(collectionA);
+        com.addCollection(collectionB);
+        com.addCollection(collectionC);
+
+        EPerson epersonA = EPerson.create(context);
+        EPerson epersonB = EPerson.create(context);
+        EPerson epersonC = EPerson.create(context);
+        EPerson epersonD = EPerson.create(context);
+
+        //personA can submit to collectionA and collectionC
+        AuthorizeManager.addPolicy(context, collectionA, Constants.ADD, epersonA);
+        AuthorizeManager.addPolicy(context, collectionC, Constants.ADD, epersonA);
+
+        //personB can submit to collectionB and collectionC
+        AuthorizeManager.addPolicy(context, collectionB, Constants.ADD, epersonB);
+        AuthorizeManager.addPolicy(context, collectionC, Constants.ADD, epersonB);
+
+        //personC can only submit to collectionC
+        AuthorizeManager.addPolicy(context, collectionC, Constants.ADD, epersonC);
+
+        //personD no submission powers
+
+        context.restoreAuthSystemState();
+
+        context.setCurrentUser(epersonA);
+        Collection[] personACollections = Collection.findAuthorizedOptimized(context, Constants.ADD);
+        assertTrue("testFindAuthorizeOptimized A", personACollections.length == 2);
+        List<Collection> aList = Arrays.asList(personACollections);
+        assertTrue("testFindAuthorizeOptimized A.A", aList.contains(collectionA));
+        assertFalse("testFindAuthorizeOptimized A.A", aList.contains(collectionB));
+        assertTrue("testFindAuthorizeOptimized A.A", aList.contains(collectionC));
+
+        context.setCurrentUser(epersonB);
+        Collection[] personBCollections = Collection.findAuthorizedOptimized(context, Constants.ADD);
+        assertTrue("testFindAuthorizeOptimized B", personBCollections.length == 2);
+        List<Collection> bList = Arrays.asList(personBCollections);
+        assertFalse("testFindAuthorizeOptimized B.A", bList.contains(collectionA));
+        assertTrue("testFindAuthorizeOptimized B.B", bList.contains(collectionB));
+        assertTrue("testFindAuthorizeOptimized B.C", bList.contains(collectionC));
+
+        context.setCurrentUser(epersonC);
+        Collection[] personCCollections = Collection.findAuthorizedOptimized(context, Constants.ADD);
+        assertTrue("testFindAuthorizeOptimized C", personCCollections.length == 1);
+        List<Collection> cList = Arrays.asList(personCCollections);
+        assertFalse("testFindAuthorizeOptimized C.A", cList.contains(collectionA));
+        assertFalse("testFindAuthorizeOptimized C.B", cList.contains(collectionB));
+        assertTrue("testFindAuthorizeOptimized C.C", cList.contains(collectionC));
+
+        context.setCurrentUser(epersonD);
+        Collection[] personDCollections = Collection.findAuthorizedOptimized(context, Constants.ADD);
+        assertTrue("testFindAuthorizeOptimized D", personDCollections.length == 0);
+        List<Collection> dList = Arrays.asList(personDCollections);
+        assertFalse("testFindAuthorizeOptimized D.A", dList.contains(collectionA));
+        assertFalse("testFindAuthorizeOptimized D.B", dList.contains(collectionB));
+        assertFalse("testFindAuthorizeOptimized D.C", dList.contains(collectionC));
+    }
+
     /**
      * Test of countItems method, of class Collection.
      */

dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPSelectCollectionStep.java

@@ -136,7 +136,7 @@ public class JSPSelectCollectionStep extends JSPStep
             else
             {
                 // Show all collections
-                collections = Collection.findAuthorized(context, null,
+                collections = Collection.findAuthorizedOptimized(context,
                         Constants.ADD);
             }
 

dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPStartSubmissionLookupStep.java

@@ -153,7 +153,7 @@ public class JSPStartSubmissionLookupStep extends JSPStep
             else
             {
                 // Show all collections
-                collections = Collection.findAuthorized(context, null,
+                collections = Collection.findAuthorizedOptimized(context,
                         Constants.ADD);
             }
 

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/item/MoveItemForm.java

@@ -67,7 +67,7 @@ public class MoveItemForm extends AbstractDSpaceTransformer {
         Division main = body.addInteractiveDivision("move-item", contextPath+"/admin/item", Division.METHOD_POST, "primary administrative item");
         main.setHead(T_head1.parameterize(item.getHandle()));
 
-        Collection[] collections = Collection.findAuthorized(context, null, Constants.ADD);
+        Collection[] collections = Collection.findAuthorizedOptimized(context, Constants.ADD);
 
         List list = main.addList("select-collection", List.TYPE_FORM);
         Select select = list.addItem().addSelect("collectionID");

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/Submissions.java

@@ -172,7 +172,7 @@ public class Submissions extends AbstractDSpaceTransformer
 
     	if (unfinishedItems.length <= 0 && supervisedItems.length <= 0)
     	{
-            Collection[] collections = Collection.findAuthorized(context, null, Constants.ADD);
+            Collection[] collections = Collection.findAuthorizedOptimized(context, Constants.ADD);
 
             if (collections.length > 0)
             {

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/SelectCollectionStep.java

@@ -82,7 +82,7 @@ public class SelectCollectionStep extends AbstractSubmissionStep
         } 
         else
         {
-            collections = Collection.findAuthorized(context, null, Constants.ADD);
+            collections = Collection.findAuthorizedOptimized(context, Constants.ADD);
         }
         
         // Basic form with a drop down list of all the collections