diff --git a/dspace-api/src/main/java/org/dspace/app/util/OptimizeSelectCollection.java b/dspace-api/src/main/java/org/dspace/app/util/OptimizeSelectCollection.java index c81e20c6ac..0c4b878a64 100644 --- a/dspace-api/src/main/java/org/dspace/app/util/OptimizeSelectCollection.java +++ b/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 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(); + 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"); diff --git a/dspace-api/src/test/java/org/dspace/content/CollectionTest.java b/dspace-api/src/test/java/org/dspace/content/CollectionTest.java index 9750463878..e6b5942f4d 100644 --- a/dspace-api/src/test/java/org/dspace/content/CollectionTest.java +++ b/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 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 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 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 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. */ diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPSelectCollectionStep.java b/dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPSelectCollectionStep.java index 91c5b9c265..d69eb346c3 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPSelectCollectionStep.java +++ b/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); } diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPStartSubmissionLookupStep.java b/dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPStartSubmissionLookupStep.java index b99f503c09..4f20e81cf4 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/submit/step/JSPStartSubmissionLookupStep.java +++ b/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); } diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/item/MoveItemForm.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/item/MoveItemForm.java index eeb4a91b8e..0b00725935 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/item/MoveItemForm.java +++ b/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"); diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/Submissions.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/Submissions.java index e80ca69d7f..7a61cfdcd3 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/Submissions.java +++ b/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) { diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/SelectCollectionStep.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/SelectCollectionStep.java index 41cf7e59fb..939c8dec55 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/SelectCollectionStep.java +++ b/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