Merge branch 'dspace-6_x' into DS-3579_Context-mode-and-cache-management-CLI-commands

dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportCLITool.java

@@ -189,7 +189,7 @@ public class ItemImportCLITool {
             String zipfilename = "";
             if (line.hasOption('z')) {
                 zip = true;
-                zipfilename = sourcedir + System.getProperty("file.separator") + line.getOptionValue('z');
+                zipfilename = line.getOptionValue('z');
             }
 
             //By default assume collections will be given on the command line

dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickPdfThumbnailFilter.java

@@ -46,11 +46,4 @@ public class ImageMagickPdfThumbnailFilter extends ImageMagickThumbnailFilter {
 	    }
     }
 
-   public static final String[] PDF = {"Adobe PDF"};
-   @Override
-   public String[] getInputMIMETypes()
-  {
-      return PDF;
-  }
-
 }

dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickThumbnailFilter.java

@@ -34,7 +34,7 @@ import org.dspace.core.ConfigurationManager;
  * thumbnail.maxwidth, thumbnail.maxheight, the size we want our thumbnail to be
  * no bigger than. Creates only JPEGs.
  */
-public abstract class ImageMagickThumbnailFilter extends MediaFilter implements SelfRegisterInputFormats
+public abstract class ImageMagickThumbnailFilter extends MediaFilter
 {
 	protected static int width = 180;
     protected static int height = 120;
@@ -188,21 +188,4 @@ public abstract class ImageMagickThumbnailFilter extends MediaFilter implements
         return true; //assume that the thumbnail is a custom one
     }
 
-     @Override
-     public String[] getInputMIMETypes()
-    {
-        return ImageIO.getReaderMIMETypes();
-    }
-
-    @Override
-    public String[] getInputDescriptions()
-    {
-        return null;
-    }
-
-    @Override
-    public String[] getInputExtensions()
-    {
-        return ImageIO.getReaderFileSuffixes();
-    }
 }

dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java

@@ -260,7 +260,7 @@ public class AuthorizeServiceImpl implements AuthorizeService
             // if user is an Admin on this object
             DSpaceObject adminObject = useInheritance ? serviceFactory.getDSpaceObjectService(o).getAdminObject(c, o, action) : null;
 
-            if (isAdmin(c, adminObject))
+            if (isAdmin(c, e, adminObject))
             {
                 c.cacheAuthorizedAction(o, action, e, true, null);
                 return true;
@@ -322,7 +322,7 @@ public class AuthorizeServiceImpl implements AuthorizeService
                 }
 
                 if ((rp.getGroup() != null)
-                        && (groupService.isMember(c, rp.getGroup())))
+                        && (groupService.isMember(c, e, rp.getGroup())))
                 {
                     // group was set, and eperson is a member
                     // of that group
@@ -370,9 +370,14 @@ public class AuthorizeServiceImpl implements AuthorizeService
     @Override
     public boolean isAdmin(Context c, DSpaceObject o) throws SQLException
     {
+        return this.isAdmin(c, c.getCurrentUser(), o);
+    }
 
+    @Override
+    public boolean isAdmin(Context c, EPerson e, DSpaceObject o) throws SQLException
+    {
         // return true if user is an Administrator
-        if (isAdmin(c))
+        if (isAdmin(c, e))
         {
             return true;
         }
@@ -382,7 +387,7 @@ public class AuthorizeServiceImpl implements AuthorizeService
             return false;
         }
 
-        Boolean cachedResult = c.getCachedAuthorizationResult(o, Constants.ADMIN, c.getCurrentUser());
+        Boolean cachedResult = c.getCachedAuthorizationResult(o, Constants.ADMIN, e);
         if (cachedResult != null) {
             return cachedResult.booleanValue();
         }
@@ -397,18 +402,18 @@ public class AuthorizeServiceImpl implements AuthorizeService
             // check policies for date validity
             if (resourcePolicyService.isDateValid(rp))
             {
-                if (rp.getEPerson() != null && rp.getEPerson().equals(c.getCurrentUser()))
+                if (rp.getEPerson() != null && rp.getEPerson().equals(e))
                 {
-                    c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), true, rp);
+                    c.cacheAuthorizedAction(o, Constants.ADMIN, e, true, rp);
                     return true; // match
                 }
 
                 if ((rp.getGroup() != null)
-                        && (groupService.isMember(c, rp.getGroup())))
+                        && (groupService.isMember(c, e, rp.getGroup())))
                 {
                     // group was set, and eperson is a member
                     // of that group
-                    c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), true, rp);
+                    c.cacheAuthorizedAction(o, Constants.ADMIN, e, true, rp);
                     return true;
                 }
             }
@@ -427,12 +432,12 @@ public class AuthorizeServiceImpl implements AuthorizeService
         DSpaceObject parent = serviceFactory.getDSpaceObjectService(o).getParentObject(c, o);
         if (parent != null)
         {
-            boolean admin = isAdmin(c, parent);
+            boolean admin = isAdmin(c, e, parent);
-            c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), admin, null);
+            c.cacheAuthorizedAction(o, Constants.ADMIN, e, admin, null);
             return admin;
         }
 
-        c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), false, null);
+        c.cacheAuthorizedAction(o, Constants.ADMIN, e, false, null);
         return false;
     }
 
@@ -456,6 +461,24 @@ public class AuthorizeServiceImpl implements AuthorizeService
         }
     }
 
+    @Override
+    public boolean isAdmin(Context c, EPerson e) throws SQLException
+    {
+        // if we're ignoring authorization, user is member of admin
+        if (c.ignoreAuthorization())
+        {
+            return true;
+        }
+
+        if (e == null)
+        {
+            return false; // anonymous users can't be admins....
+        } else
+        {
+            return groupService.isMember(c, e, Group.ADMIN);
+        }
+    }
+
     public boolean isCommunityAdmin(Context c) throws SQLException 
     {
         EPerson e = c.getCurrentUser();
@@ -679,13 +702,14 @@ public class AuthorizeServiceImpl implements AuthorizeService
     @Override
     public boolean isAnIdenticalPolicyAlreadyInPlace(Context c, DSpaceObject dso, Group group, int action, int policyID) throws SQLException
     {
-        return findByTypeIdGroupAction(c, dso, group, action, policyID) != null;
+        return !resourcePolicyService.findByTypeGroupActionExceptId(c, dso, group, action, policyID).isEmpty();
     }
 
     @Override
-    public ResourcePolicy findByTypeIdGroupAction(Context c, DSpaceObject dso, Group group, int action, int policyID) throws SQLException
+    public ResourcePolicy findByTypeGroupAction(Context c, DSpaceObject dso, Group group, int action)
+            throws SQLException
     {
-        List<ResourcePolicy> policies = resourcePolicyService.find(c, dso, group, action, policyID);
+        List<ResourcePolicy> policies = resourcePolicyService.find(c, dso, group, action);
 
         if (CollectionUtils.isNotEmpty(policies))
         {
@@ -695,7 +719,6 @@ public class AuthorizeServiceImpl implements AuthorizeService
         }
     }
 
-
     /**
      * Generate Policies policies READ for the date in input adding reason. New policies are assigned automatically at the groups that
      * have right on the collection. E.g., if the anonymous can access the collection policies are assigned to anonymous.
@@ -771,12 +794,19 @@ public class AuthorizeServiceImpl implements AuthorizeService
     public ResourcePolicy createOrModifyPolicy(ResourcePolicy policy, Context context, String name, Group group, EPerson ePerson,
                                                       Date embargoDate, int action, String reason, DSpaceObject dso) throws AuthorizeException, SQLException
     {
-
+        ResourcePolicy policyTemp = null;
-        int policyID = -1;
+        if (policy != null)
-        if (policy != null) policyID = policy.getID();
+        {
-
+            List<ResourcePolicy> duplicates = resourcePolicyService.findByTypeGroupActionExceptId(context, dso, group, action, policy.getID());
+            if (!duplicates.isEmpty())
+            {
+                policy = duplicates.get(0);
+            }
+        } else {
             // if an identical policy (same Action and same Group) is already in place modify it...
-        ResourcePolicy policyTemp = findByTypeIdGroupAction(context, dso, group, action, policyID);
+            policyTemp = findByTypeGroupAction(context, dso, group, action);
+        }
+
         if (policyTemp != null)
         {
             policy = policyTemp;

dspace-api/src/main/java/org/dspace/authorize/ResourcePolicyServiceImpl.java

@@ -104,14 +104,23 @@ public class ResourcePolicyServiceImpl implements ResourcePolicyService
     }
 
     @Override
-    public List<ResourcePolicy> find(Context c, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException {
+    public List<ResourcePolicy> find(Context c, DSpaceObject dso, Group group, int action) throws SQLException {
-        return resourcePolicyDAO.findByTypeIdGroupAction(c, dso, group, action, notPolicyID);
+        return resourcePolicyDAO.findByTypeGroupAction(c, dso, group, action);
     }
     
+    @Override
     public List<ResourcePolicy> find(Context c, EPerson e, List<Group> groups, int action, int type_id) throws SQLException{
         return resourcePolicyDAO.findByEPersonGroupTypeIdAction(c, e, groups, action, type_id);
     }
     
+    @Override
+    public List<ResourcePolicy> findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID)
+            throws SQLException
+    {
+        return resourcePolicyDAO.findByTypeGroupActionExceptId(context, dso, group, action, notPolicyID);
+    }
+            
+
     /**
      * Delete an ResourcePolicy
      *

dspace-api/src/main/java/org/dspace/authorize/dao/ResourcePolicyDAO.java

@@ -34,7 +34,16 @@ public interface ResourcePolicyDAO extends GenericDAO<ResourcePolicy> {
 
     public List<ResourcePolicy> findByDSoAndAction(Context context, DSpaceObject dso, int actionId) throws SQLException;
 
-    public List<ResourcePolicy> findByTypeIdGroupAction(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException;
+    public List<ResourcePolicy> findByTypeGroupAction(Context context, DSpaceObject dso, Group group, int action) throws SQLException;
+    
+    /**
+     * Look for ResourcePolicies by DSpaceObject, Group, and action, ignoring IDs with a specific PolicyID.
+     * This method can be used to detect duplicate ResourcePolicies.
+     * @param notPolicyID ResourcePolicies with this ID will be ignored while looking out for equal ResourcePolicies.
+     * @return List of resource policies for the same DSpaceObject, group and action but other policyID.
+     * @throws SQLException 
+     */
+    public List<ResourcePolicy> findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException;
     
     public List<ResourcePolicy> findByEPersonGroupTypeIdAction(Context context, EPerson e, List<Group> groups, int action, int type_id) throws SQLException;
 

dspace-api/src/main/java/org/dspace/authorize/dao/impl/ResourcePolicyDAOImpl.java

@@ -75,7 +75,7 @@ public class ResourcePolicyDAOImpl extends AbstractHibernateDAO<ResourcePolicy>
     }
 
     @Override
-    public List<ResourcePolicy> findByTypeIdGroupAction(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException {
+    public List<ResourcePolicy> findByTypeGroupAction(Context context, DSpaceObject dso, Group group, int action) throws SQLException {
         Criteria criteria = createCriteria(context, ResourcePolicy.class);
         criteria.add(Restrictions.and(
                 Restrictions.eq("dSpaceObject", dso),
@@ -83,15 +83,21 @@ public class ResourcePolicyDAOImpl extends AbstractHibernateDAO<ResourcePolicy>
                 Restrictions.eq("actionId", action)
         ));
         criteria.setMaxResults(1);
-
-        List<ResourcePolicy> results;
-        if (notPolicyID != -1)
-        {
-            criteria.add(Restrictions.and(Restrictions.not(Restrictions.eq("id", notPolicyID))));
-        }
-
         return list(criteria);
     }
+    
+    @Override
+    public List<ResourcePolicy> findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException {
+        Criteria criteria = createCriteria(context, ResourcePolicy.class);
+        criteria.add(Restrictions.and(
+                Restrictions.eq("dSpaceObject", dso),
+                Restrictions.eq("epersonGroup", group),
+                Restrictions.eq("actionId", action)
+        ));
+        criteria.add(Restrictions.and(Restrictions.not(Restrictions.eq("id", notPolicyID))));
+        return list(criteria);
+    }
+    
      public List<ResourcePolicy> findByEPersonGroupTypeIdAction(Context context, EPerson e, List<Group> groups, int action, int type_id) throws SQLException
      {
          Criteria criteria = createCriteria(context, ResourcePolicy.class);

dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java

@@ -167,11 +167,28 @@ public interface AuthorizeService {
      */
     public boolean isAdmin(Context c, DSpaceObject o) throws SQLException;
 
+    /**
+     * Check to see if a specific user is an Administrator of a given object
+     * within DSpace. Always return {@code true} if the user is a System
+     * Admin
+     *
+     * @param c current context
+     * @param e the user to check
+     * @param o current DSpace Object, if <code>null</code> the call will be
+     *         equivalent to a call to the <code>isAdmin(Context c)</code>
+     *         method
+     * @return {@code true} if the user has administrative privileges on the
+     *         given DSpace object
+     * @throws SQLException if database error
+     */
+    public boolean isAdmin(Context c, EPerson e, DSpaceObject o) throws SQLException;
+
 
     /**
      * Check to see if the current user is a System Admin. Always return
-     * {@code true} if c.ignoreAuthorization is set. Anonymous users
+     * {@code true} if c.ignoreAuthorization is set. If no EPerson is
-     * can't be Admins (EPerson set to NULL)
+     * logged in and context.getCurrentUser() returns null, this method
+     * returns false as anonymous users can never be administrators.
      *
      * @param c current context
      * @return {@code true} if user is an admin or ignore authorization
@@ -180,6 +197,17 @@ public interface AuthorizeService {
      */
     public boolean isAdmin(Context c) throws SQLException;
 
+    /**
+     * Check to see if a specific user is system admin. Always return
+     * {@code true} if c.ignoreAuthorization is set.
+     *
+     * @param c current context
+     * @return {@code true} if user is an admin or ignore authorization
+     *         flag set
+     * @throws SQLException if database error
+     */
+    public boolean isAdmin(Context c, EPerson e) throws SQLException;
+    
     public boolean isCommunityAdmin(Context c) throws SQLException;
     
     public boolean isCollectionAdmin(Context c) throws SQLException; 
@@ -411,7 +439,7 @@ public interface AuthorizeService {
      */
     public boolean isAnIdenticalPolicyAlreadyInPlace(Context c, DSpaceObject o, Group group, int actionID, int policyID) throws SQLException;
     
-    public ResourcePolicy findByTypeIdGroupAction(Context c, DSpaceObject dso, Group group, int action, int policyID) throws SQLException;
+    public ResourcePolicy findByTypeGroupAction(Context c, DSpaceObject dso, Group group, int action) throws SQLException;
 
 
     /**

dspace-api/src/main/java/org/dspace/authorize/service/ResourcePolicyService.java

@@ -33,12 +33,22 @@ public interface ResourcePolicyService extends DSpaceCRUDService<ResourcePolicy>
 
     public List<ResourcePolicy> find(Context c, DSpaceObject o, int actionId) throws SQLException;
 
-    public List<ResourcePolicy> find(Context c, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException;
+    public List<ResourcePolicy> find(Context c, DSpaceObject dso, Group group, int action) throws SQLException;
 
     public List<ResourcePolicy> find(Context context, Group group) throws SQLException;
     
     public List<ResourcePolicy> find(Context c, EPerson e, List<Group> groups, int action, int type_id) throws SQLException;
     
+    /**
+     * Look for ResourcePolicies by DSpaceObject, Group, and action, ignoring IDs with a specific PolicyID.
+     * This method can be used to detect duplicate ResourcePolicies.
+     * @param notPolicyID ResourcePolicies with this ID will be ignored while looking out for equal ResourcePolicies.
+     * @return List of resource policies for the same DSpaceObject, group and action but other policyID.
+     * @throws SQLException 
+     */
+    public List<ResourcePolicy> findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID)
+            throws SQLException;
+
     public String getActionText(ResourcePolicy resourcePolicy);
 
     public boolean isDateValid(ResourcePolicy resourcePolicy);

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

@@ -269,19 +269,49 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
     public void setOrder(Context context, Bundle bundle, UUID[] bitstreamIds) throws AuthorizeException, SQLException {
         authorizeService.authorizeAction(context, bundle, Constants.WRITE);
 
-        bundle.getBitstreams().clear();
+        List<Bitstream> currentBitstreams = bundle.getBitstreams();
+        List<Bitstream> updatedBitstreams = new ArrayList<Bitstream>();
+
+        // Loop through and ensure these Bitstream IDs are all valid. Add them to list of updatedBitstreams.
         for (int i = 0; i < bitstreamIds.length; i++) {
             UUID bitstreamId = bitstreamIds[i];
             Bitstream bitstream = bitstreamService.find(context, bitstreamId);
+
+            // If we have an invalid Bitstream ID, just ignore it, but log a warning
             if(bitstream == null) {
                 //This should never occur but just in case
                 log.warn(LogManager.getHeader(context, "Invalid bitstream id while changing bitstream order", "Bundle: " + bundle.getID() + ", bitstream id: " + bitstreamId));
                 continue;
             }
+
+            // If we have a Bitstream not in the current list, log a warning & exit immediately
+            if(!currentBitstreams.contains(bitstream))
+            {
+                log.warn(LogManager.getHeader(context, "Encountered a bitstream not in this bundle while changing bitstream order. Bitstream order will not be changed.", "Bundle: " + bundle.getID() + ", bitstream id: " + bitstreamId));
+                return;
+            }
+            updatedBitstreams.add(bitstream);
+        }
+
+        // If our lists are different sizes, exit immediately
+        if(updatedBitstreams.size()!=currentBitstreams.size())
+        {
+            log.warn(LogManager.getHeader(context, "Size of old list and new list do not match. Bitstream order will not be changed.", "Bundle: " + bundle.getID()));
+            return;
+        }
+
+        // As long as the order has changed, update it
+        if(CollectionUtils.isNotEmpty(updatedBitstreams) && !updatedBitstreams.equals(currentBitstreams))
+        {
+            //First clear out the existing list of bitstreams
+            bundle.getBitstreams().clear();
+
+            // Now add them back in the proper order
+            for (Bitstream bitstream : updatedBitstreams)
+            {
                 bitstream.getBundles().remove(bundle);
                 bundle.getBitstreams().add(bitstream);
                 bitstream.getBundles().add(bundle);
-
                 bitstreamService.update(context, bitstream);
             }
 
@@ -294,6 +324,7 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
 
             }
         }
+    }
 
     @Override
     public DSpaceObject getAdminObject(Context context, Bundle bundle, int action) throws SQLException {

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

@@ -7,6 +7,7 @@
  */
 package org.dspace.content;
 
+import org.dspace.content.comparator.NameAscendingComparator;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.CollectionService;
 import org.dspace.core.*;
@@ -16,8 +17,7 @@ import org.hibernate.proxy.HibernateProxyHelper;
 
 import javax.persistence.*;
 import java.sql.SQLException;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.List;
 
 /**
  * Class representing a collection.
@@ -86,7 +86,7 @@ public class Collection extends DSpaceObject implements DSpaceObjectLegacySuppor
             joinColumns = {@JoinColumn(name = "collection_id") },
             inverseJoinColumns = {@JoinColumn(name = "community_id") }
     )
-    private final List<Community> communities = new ArrayList<>();
+    private Set<Community> communities = new HashSet<>();
 
     @Transient
     private transient CollectionService collectionService;
@@ -266,7 +266,11 @@ public class Collection extends DSpaceObject implements DSpaceObjectLegacySuppor
      */
     public List<Community> getCommunities() throws SQLException
     {
-        return communities;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Community[] output = communities.toArray(new Community[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addCommunity(Community community) {

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

@@ -749,8 +749,8 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
         while (owningCommunities.hasNext())
         {
             Community owningCommunity = owningCommunities.next();
-            owningCommunities.remove();
+            collection.removeCommunity(owningCommunity);
-            owningCommunity.getCollections().remove(collection);
+            owningCommunity.removeCollection(collection);
         }
 
         collectionDAO.delete(context, collection);

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

@@ -9,6 +9,7 @@ package org.dspace.content;
 
 import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.log4j.Logger;
+import org.dspace.content.comparator.NameAscendingComparator;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.CommunityService;
 import org.dspace.core.*;
@@ -47,13 +48,13 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport
             joinColumns = {@JoinColumn(name = "parent_comm_id") },
             inverseJoinColumns = {@JoinColumn(name = "child_comm_id") }
     )
-    private final List<Community> subCommunities = new ArrayList<>();
+    private Set<Community> subCommunities = new HashSet<>();
 
     @ManyToMany(fetch = FetchType.LAZY, mappedBy = "subCommunities")
-    private List<Community> parentCommunities = new ArrayList<>();
+    private Set<Community> parentCommunities = new HashSet<>();
 
     @ManyToMany(fetch = FetchType.LAZY, mappedBy = "communities", cascade = {CascadeType.PERSIST})
-    private final List<Collection> collections = new ArrayList<>();
+    private Set<Collection> collections = new HashSet<>();
 
     @OneToOne
     @JoinColumn(name = "admin")
@@ -88,13 +89,13 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport
 
     void addSubCommunity(Community subCommunity)
     {
-        getSubcommunities().add(subCommunity);
+        subCommunities.add(subCommunity);
         setModified();
     }
 
     void removeSubCommunity(Community subCommunity)
     {
-        getSubcommunities().remove(subCommunity);
+        subCommunities.remove(subCommunity);
         setModified();
     }
 
@@ -143,17 +144,21 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport
      */
     public List<Collection> getCollections()
     {
-        return collections;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Collection[] output = collections.toArray(new Collection[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addCollection(Collection collection)
     {
-        getCollections().add(collection);
+        collections.add(collection);
     }
 
     void removeCollection(Collection collection)
     {
-        getCollections().remove(collection);
+        collections.remove(collection);
     }
 
     /**
@@ -165,7 +170,11 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport
      */
     public List<Community> getSubcommunities()
     {
-        return subCommunities;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Community[] output = subCommunities.toArray(new Community[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     /**
@@ -176,16 +185,25 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport
      */
     public List<Community> getParentCommunities()
     {
-        return parentCommunities;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Community[] output = parentCommunities.toArray(new Community[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addParentCommunity(Community parentCommunity) {
-        getParentCommunities().add(parentCommunity);
+        parentCommunities.add(parentCommunity);
     }
 
     void clearParentCommunities(){
-        this.parentCommunities.clear();
+        parentCommunities.clear();
-        this.parentCommunities = null;
+    }
+
+    public void removeParentCommunity(Community parentCommunity)
+    {
+        parentCommunities.remove(parentCommunity);
+        setModified();
     }
 
     /**

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

@@ -455,7 +455,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
 
         rawDelete(context, childCommunity);
 
-        childCommunity.getParentCommunities().remove(parentCommunity);
+        childCommunity.removeParentCommunity(parentCommunity);
         parentCommunity.removeSubCommunity(childCommunity);
 
         log.info(LogManager.getHeader(context, "remove_subcommunity",
@@ -492,7 +492,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
             Iterator<Community> subcommunities = community.getSubcommunities().iterator();
             while (subcommunities.hasNext()) {
                 Community subCommunity = subcommunities.next();
-                subcommunities.remove();
+                community.removeSubCommunity(subCommunity);
                 delete(context, subCommunity);
             }
             // now let the parent remove the community
@@ -535,7 +535,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
         while (collections.hasNext())
         {
             Collection collection = collections.next();
-            collections.remove();
+            community.removeCollection(collection);
             removeCollection(context, community, collection);
         }
         // delete subcommunities
@@ -544,7 +544,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
         while (subCommunities.hasNext())
         {
             Community subComm = subCommunities.next();
-            subCommunities.remove();
+            community.removeSubCommunity(subComm);
             delete(context, subComm);
         }
 

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

@@ -7,17 +7,18 @@
  */
 package org.dspace.content;
 
+import org.dspace.content.comparator.NameAscendingComparator;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.ItemService;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
+import org.hibernate.annotations.Sort;
+import org.hibernate.annotations.SortType;
 import org.hibernate.proxy.HibernateProxyHelper;
 
 import javax.persistence.*;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Date;
-import java.util.List;
 
 /**
  * Class representing an item in DSpace.
@@ -78,7 +79,7 @@ public class Item extends DSpaceObject implements DSpaceObjectLegacySupport
             joinColumns = {@JoinColumn(name = "item_id") },
             inverseJoinColumns = {@JoinColumn(name = "collection_id") }
     )
-    private final List<Collection> collections = new ArrayList<>();
+    private final Set<Collection> collections = new HashSet<>();
 
     @ManyToMany(fetch = FetchType.LAZY, mappedBy = "items")
     private final List<Bundle> bundles = new ArrayList<>();
@@ -224,23 +225,31 @@ public class Item extends DSpaceObject implements DSpaceObjectLegacySupport
     }
 
     /**
-     * Get the collections this item is in. The order is indeterminate.
+     * Get the collections this item is in. The order is sorted ascending by collection name.
      *
      * @return the collections this item is in, if any.
      */
     public List<Collection> getCollections()
     {
-        return collections;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Collection[] output = collections.toArray(new Collection[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addCollection(Collection collection)
     {
-        getCollections().add(collection);
+        collections.add(collection);
     }
 
     void removeCollection(Collection collection)
     {
-        getCollections().remove(collection);
+        collections.remove(collection);
+    }
+
+    public void clearCollections(){
+        collections.clear();
     }
 
     public Collection getTemplateItemOf() {

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

@@ -651,7 +651,7 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
         }
 
         //Only clear collections after we have removed everything else from the item
-        item.getCollections().clear();
+        item.clearCollections();
         item.setOwningCollection(null);
 
         // Finally remove item row

dspace-api/src/main/java/org/dspace/content/comparator/NameAscendingComparator.java

@@ -0,0 +1,39 @@
+/**
+ * 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.content.comparator;
+
+import org.apache.commons.lang.ObjectUtils;
+import org.apache.commons.lang.StringUtils;
+import org.dspace.content.DSpaceObject;
+
+import java.util.Comparator;
+
+public class NameAscendingComparator implements Comparator<DSpaceObject>{
+
+    @Override
+    public int compare(DSpaceObject dso1, DSpaceObject dso2) {
+        if (dso1 == dso2){
+            return 0;
+        }else if (dso1 == null){
+            return -1;
+        }else if (dso2 == null){
+            return 1;
+        }else {
+            String name1 = StringUtils.trimToEmpty(dso1.getName());
+            String name2 = StringUtils.trimToEmpty(dso2.getName());
+
+            //When two DSO's have the same name, use their UUID to put them in an order
+            if(name1.equals(name2)) {
+                return ObjectUtils.compare(dso1.getID(), dso2.getID());
+            } else {
+                return name1.compareToIgnoreCase(name2);
+            }
+        }
+    }
+
+}

dspace-api/src/main/java/org/dspace/core/Context.java

@@ -9,8 +9,7 @@ package org.dspace.core;
 
 import org.apache.log4j.Logger;
 import org.dspace.authorize.ResourcePolicy;
-import org.dspace.content.*;
+import org.dspace.content.DSpaceObject;
-import org.dspace.content.Collection;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 import org.dspace.eperson.factory.EPersonServiceFactory;
@@ -692,8 +691,14 @@ public class Context
             log.warn("Unable to set database connection mode", ex);
         }
 
+        //Always clear the cache, except when going from READ_ONLY to READ_ONLY
+        if(mode != Mode.READ_ONLY || newMode != Mode.READ_ONLY) {
+            //clear our read-only cache to prevent any inconsistencies
+            readOnlyCache.clear();
+        }
+
         //save the new mode
-        this.mode = newMode;
+        mode = newMode;
     }
 
     /**

dspace-api/src/main/java/org/dspace/core/ContextReadOnlyCache.java

@@ -79,6 +79,12 @@ public class ContextReadOnlyCache {
         return allMemberGroupsCache.get(buildAllMembersGroupKey(ePerson));
     }
 
+    public void clear() {
+        authorizedActionsCache.clear();
+        groupMembershipCache.clear();
+        allMemberGroupsCache.clear();
+    }
+
     private String buildAllMembersGroupKey(EPerson ePerson) {
         return ePerson == null ? "" : ePerson.getID().toString();
     }
@@ -93,4 +99,5 @@ public class ContextReadOnlyCache {
         return new ImmutablePair<>(group == null ? "" : group.getName(),
                 eperson == null ? "" : eperson.getID().toString());
     }
+
 }

dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java

@@ -8,6 +8,7 @@
 package org.dspace.eperson;
 
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.dspace.authorize.AuthorizeConfiguration;
@@ -157,9 +158,19 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
     }
 
     @Override
-    public boolean isMember(Context context, Group group) throws SQLException {
+    public boolean isParentOf(Context context, Group parentGroup, Group childGroup) throws SQLException {
-        EPerson currentUser = context.getCurrentUser();
+        return group2GroupCacheDAO.findByParentAndChild(context, parentGroup, childGroup) != null;
+    }
 
+    @Override
+    public boolean isMember(Context context, Group group) throws SQLException {
+        return isMember(context, context.getCurrentUser(), group);
+    }
+
+    @Override
+    public boolean isMember(Context context, EPerson ePerson, Group group)
+            throws SQLException
+    {
         if(group == null) {
             return false;
 
@@ -167,34 +178,59 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
         } else if (StringUtils.equals(group.getName(), Group.ANONYMOUS)) {
             return true;
 
-        } else if(currentUser != null) {
+        } else {
+            Boolean cachedGroupMembership = context.getCachedGroupMembership(group, ePerson);
 
-            Boolean cachedGroupMembership = context.getCachedGroupMembership(group, currentUser);
             if(cachedGroupMembership != null) {
                 return cachedGroupMembership.booleanValue();
 
-            } else if(CollectionUtils.isNotEmpty(context.getSpecialGroups())) {
+            } else {
-                Set<Group> allMemberGroups = allMemberGroupsSet(context, currentUser);
+                boolean isMember = false;
-                boolean result = allMemberGroups.contains(group);
 
-                context.cacheGroupMembership(group, currentUser, result);
+                //If we have an ePerson, check we can find membership in the database
-                return result;
+                if(ePerson != null) {
-            } else {
+                    //lookup eperson in normal groups and subgroups with 1 query
-                //lookup eperson in normal groups and subgroups
+                    isMember = isEPersonInGroup(context, group.getName(), ePerson);
-                boolean result = epersonInGroup(context, group.getName(), currentUser);
-                context.cacheGroupMembership(group, currentUser, result);
-                return result;
                 }
-        } else {
+
-            return false;
+                //If we did not find the group membership in the database, check the special groups.
+                //If there are special groups we need to check direct membership or check if the
+                //special group is a subgroup of the provided group.
+                //Note that special groups should only be checked if the current user == the ePerson.
+                //This also works for anonymous users (ePerson == null) if IP authentication used
+                if(!isMember && CollectionUtils.isNotEmpty(context.getSpecialGroups()) && isAuthenticatedUser(context, ePerson)) {
+                    Iterator<Group> it = context.getSpecialGroups().iterator();
+
+                    while (it.hasNext() && !isMember) {
+                        Group specialGroup = it.next();
+                        //Check if the special group matches the given group or if it is a subgroup (with 1 query)
+                        if (specialGroup.equals(group) || isParentOf(context, group, specialGroup)) {
+                            isMember = true;
                         }
                     }
+                }
+
+                context.cacheGroupMembership(group, ePerson, isMember);
+                return isMember;
+
+            }
+        }
+    }
+
+    private boolean isAuthenticatedUser(final Context context, final EPerson ePerson) {
+        return ObjectUtils.equals(context.getCurrentUser(), ePerson);
+    }
 
     @Override
     public boolean isMember(final Context context, final String groupName) throws SQLException {
         return isMember(context, findByName(context, groupName));
     }
 
+    @Override
+    public boolean isMember(final Context context, EPerson eperson, final String groupName) throws SQLException {
+        return isMember(context, eperson, findByName(context, groupName));
+    }
+
     @Override
     public List<Group> allMemberGroups(Context context, EPerson ePerson) throws SQLException {
         return new ArrayList<>(allMemberGroupsSet(context, ePerson));
@@ -478,7 +514,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
 
 
 
-    protected boolean epersonInGroup(Context context, String groupName, EPerson ePerson)
+    protected boolean isEPersonInGroup(Context context, String groupName, EPerson ePerson)
             throws SQLException
     {
         return groupDAO.findByNameAndMembership(context, groupName, ePerson) != null;

dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java

@@ -14,7 +14,6 @@ import org.dspace.eperson.Group2GroupCache;
 
 import java.sql.SQLException;
 import java.util.List;
-import java.util.Set;
 
 /**
  * Database Access Object interface class for the Group2GroupCache object.
@@ -27,7 +26,9 @@ public interface Group2GroupCacheDAO extends GenericDAO<Group2GroupCache> {
 
     public List<Group2GroupCache> findByParent(Context context, Group group) throws SQLException;
 
-    public List<Group2GroupCache> findByChildren(Context context, Set<Group> groups) throws SQLException;
+    public List<Group2GroupCache> findByChildren(Context context, Iterable<Group> groups) throws SQLException;
+
+    public Group2GroupCache findByParentAndChild(Context context, Group parent, Group child) throws SQLException;
 
     public Group2GroupCache find(Context context, Group parent, Group child) throws SQLException;
 

dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java

@@ -13,6 +13,7 @@ import org.dspace.eperson.Group;
 import org.dspace.eperson.Group2GroupCache;
 import org.dspace.eperson.dao.Group2GroupCacheDAO;
 import org.hibernate.Criteria;
+import org.hibernate.Query;
 import org.hibernate.criterion.Disjunction;
 import org.hibernate.criterion.Restrictions;
 
@@ -44,7 +45,7 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO<Group2GroupCac
     }
 
     @Override
-    public List<Group2GroupCache> findByChildren(Context context, Set<Group> groups) throws SQLException {
+    public List<Group2GroupCache> findByChildren(Context context, Iterable<Group> groups) throws SQLException {
         Criteria criteria = createCriteria(context, Group2GroupCache.class);
 
         Disjunction orDisjunction = Restrictions.or();
@@ -59,6 +60,19 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO<Group2GroupCac
         return list(criteria);
     }
 
+    @Override
+    public Group2GroupCache findByParentAndChild(Context context, Group parent, Group child) throws SQLException {
+        Query query = createQuery(context,
+                "FROM Group2GroupCache g WHERE g.parent = :parentGroup AND g.child = :childGroup");
+
+        query.setParameter("parentGroup", parent);
+        query.setParameter("childGroup", child);
+
+        query.setCacheable(true);
+
+        return singleResult(query);
+    }
+
     @Override
     public Group2GroupCache find(Context context, Group parent, Group child) throws SQLException {
         Criteria criteria = createCriteria(context, Group2GroupCache.class);

dspace-api/src/main/java/org/dspace/eperson/dao/impl/GroupDAOImpl.java

@@ -112,10 +112,10 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO<Group> implements Grou
                             "(p.id = :eperson_id OR " +
                             "EXISTS ( " +
                                 "SELECT 1 FROM Group2GroupCache gc " +
-                                "JOIN gc.parent p " +
+                                "JOIN gc.parent parent " +
-                                "JOIN gc.child c " +
+                                "JOIN gc.child child " +
-                                "JOIN c.epeople cp " +
+                                "JOIN child.epeople cp " +
-                                "WHERE p.id = g.id AND cp.id = :eperson_id " +
+                                "WHERE parent.id = g.id AND cp.id = :eperson_id " +
                                 ") " +
                             ")");
 

dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java

@@ -7,10 +7,6 @@
  */
 package org.dspace.eperson.service;
 
-import java.sql.SQLException;
-import java.util.List;
-import java.util.Set;
-
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.MetadataField;
 import org.dspace.content.service.DSpaceObjectLegacySupportService;
@@ -19,6 +15,10 @@ import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 
+import java.sql.SQLException;
+import java.util.List;
+import java.util.Set;
+
 /**
  * Service interface class for the Group object.
  * The implementation of this class is responsible for all business logic calls for the Group object and is autowired by spring
@@ -117,6 +117,15 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      */
     public boolean isMember(Group owningGroup, Group childGroup);
 
+    /**
+     * Check to see if parentGroup is a direct or in-direct parent of a childGroup.
+     *
+     * @param parentGroup parent group
+     * @param childGroup child group
+     * @return true or false
+     */
+    public boolean isParentOf(Context context, Group parentGroup, Group childGroup) throws SQLException;
+
     /**
      * fast check to see if an eperson is a member called with eperson id, does
      * database lookup without instantiating all of the epeople objects and is
@@ -134,7 +143,8 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
     /**
      * fast check to see if an eperson is a member called with eperson id, does
      * database lookup without instantiating all of the epeople objects and is
-     * thus a static method
+     * thus a static method. This method uses context.getCurrentUser() as
+     * eperson whos membership should be checked.
      *
      * @param context
      *            context
@@ -145,6 +155,34 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      */
     public boolean isMember(Context context, String groupName) throws SQLException;
 
+    /**
+     * fast check to see if an eperson is a member called with eperson id, does
+     * database lookup without instantiating all of the epeople objects and is
+     * thus a static method. The eperson whos membership should be checked must
+     * be defined as method attribute.
+     *
+     * @param context
+     *            context
+     * @param groupName
+     *            the name of the group to check
+     * @return true or false
+     * @throws SQLException if database error
+     */
+    public boolean isMember(Context context, EPerson epersonToCheck, String groupName) throws SQLException;
+
+    /**
+     * fast check to see if an eperson is a member called with eperson id, does
+     * database lookup without instantiating all of the epeople objects and is
+     * thus a static method
+     *
+     * @param context DSpace context object.
+     * @param eperson EPerson whos membership should be checked.
+     * @param group The group to check against.
+     * @return true or false
+     * @throws SQLException if database error
+     */
+    public boolean isMember(Context context, EPerson eperson, Group group) throws SQLException;
+
     /**
      * Get all of the groups that an eperson is a member of.
      *

dspace-api/src/main/java/org/dspace/statistics/content/StatisticsDataVisits.java

@@ -511,7 +511,7 @@ public class StatisticsDataVisits extends StatisticsData
                         {
                             break;
                         }
-                        return value;
+                        return bit.getName();
                     case Constants.ITEM:
                         Item item = itemService.findByIdOrLegacyId(context, dsoId);
                         if(item == null)

dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactory.java

@@ -10,6 +10,8 @@ package org.dspace.statistics.factory;
 import org.dspace.services.factory.DSpaceServicesFactory;
 import org.dspace.statistics.service.ElasticSearchLoggerService;
 import org.dspace.statistics.service.SolrLoggerService;
+import org.dspace.statistics.util.SpiderDetector;
+import org.dspace.statistics.util.SpiderDetectorService;
 
 /**
  * Abstract factory to get services for the statistics package, use StatisticsServiceFactory.getInstance() to retrieve an implementation
@@ -22,6 +24,8 @@ public abstract class StatisticsServiceFactory {
 
     public abstract ElasticSearchLoggerService getElasticSearchLoggerService();
 
+    public abstract SpiderDetectorService getSpiderDetectorService();
+
     public static StatisticsServiceFactory getInstance()
     {
         return DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName("statisticsServiceFactory", StatisticsServiceFactory.class);

dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactoryImpl.java

@@ -10,6 +10,8 @@ package org.dspace.statistics.factory;
 import org.dspace.services.factory.DSpaceServicesFactory;
 import org.dspace.statistics.service.ElasticSearchLoggerService;
 import org.dspace.statistics.service.SolrLoggerService;
+import org.dspace.statistics.util.SpiderDetector;
+import org.dspace.statistics.util.SpiderDetectorService;
 
 /**
  * Factory implementation to get services for the statistics package, use StatisticsServiceFactory.getInstance() to retrieve an implementation
@@ -29,4 +31,9 @@ public class StatisticsServiceFactoryImpl extends StatisticsServiceFactory {
         // In order to lazy load, we cannot autowire it and instead load it by name
         return DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName("elasticSearchLoggerService", ElasticSearchLoggerService.class);
     }
+
+    @Override
+    public SpiderDetectorService getSpiderDetectorService() {
+        return DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName("spiderDetectorService", SpiderDetectorService.class);
+    }
 }

dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetector.java

@@ -7,48 +7,40 @@
  */
 package org.dspace.statistics.util;
 
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileReader;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
-import java.util.Collections;
-import java.util.regex.Pattern;
 import javax.servlet.http.HttpServletRequest;
-import org.dspace.services.factory.DSpaceServicesFactory;
+
+import org.dspace.statistics.factory.StatisticsServiceFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * SpiderDetector is used to find IP's that are spiders...
+ * SpiderDetector delegates static methods to SpiderDetectorService, which is used to find IP's that are spiders...
- * In future someone may add Host Domains
- * to the detection criteria here.
  *
  * @author kevinvandevelde at atmire.com
  * @author ben at atmire.com
  * @author Mark Diggory (mdiggory at atmire.com)
+ * @author frederic at atmire.com
  */
 public class SpiderDetector {
 
     private static final Logger log = LoggerFactory.getLogger(SpiderDetector.class);
 
-    private static Boolean useProxies;
+    //Service where all methods get delegated to, this is instantiated by a spring-bean defined in core-services.xml
+    private static SpiderDetectorService spiderDetectorService = StatisticsServiceFactory.getInstance().getSpiderDetectorService();
 
     /**
-     * Sparse HashTable structure to hold IP address ranges.
+     * Get an immutable Set representing all the Spider Addresses here
+     *
+     * @return a set of IP addresses as strings
      */
-    private static IPTable table = null;
+    public static Set<String> getSpiderIpAddresses() {
 
-    /** Collection of regular expressions to match known spiders' agents. */
+        spiderDetectorService.loadSpiderIpAddresses();
-    private static final List<Pattern> agents
+        return spiderDetectorService.getTable().toSet();
-            = Collections.synchronizedList(new ArrayList<Pattern>());
+    }
-
-    /** Collection of regular expressions to match known spiders' domain names. */
-    private static final List<Pattern> domains
-            = Collections.synchronizedList(new ArrayList<Pattern>());
 
     /**
      * Utility method which reads lines from a file & returns them in a Set.
@@ -60,130 +52,7 @@ public class SpiderDetector {
     public static Set<String> readPatterns(File patternFile)
             throws IOException
     {
-        Set<String> patterns = new HashSet<>();
+        return spiderDetectorService.readPatterns(patternFile);
-
-        if (!patternFile.exists() || !patternFile.isFile())
-        {
-            return patterns;
-        }
-
-        //Read our file & get all them patterns.
-        try (BufferedReader in = new BufferedReader(new FileReader(patternFile)))
-        {
-            String line;
-            while ((line = in.readLine()) != null) {
-                if (!line.startsWith("#")) {
-                    line = line.trim();
-
-                    if (!line.equals("")) {
-                        patterns.add(line);
-                    }
-                } else {
-                    //   ua.add(line.replaceFirst("#","").replaceFirst("UA","").trim());
-                    // ... add this functionality later
-                }
-            }
-        }
-        return patterns;
-    }
-
-    /**
-     * Get an immutable Set representing all the Spider Addresses here
-     *
-     * @return a set of IP addresses as strings
-     */
-    public static Set<String> getSpiderIpAddresses() {
-
-        loadSpiderIpAddresses();
-        return table.toSet();
-    }
-
-    /*
-     *  private loader to populate the table from files.
-     */
-
-    private synchronized static void loadSpiderIpAddresses() {
-
-        if (table == null) {
-            table = new IPTable();
-
-            String filePath = DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.dir");
-
-            try {
-                File spidersDir = new File(filePath, "config/spiders");
-
-                if (spidersDir.exists() && spidersDir.isDirectory()) {
-                    for (File file : spidersDir.listFiles()) {
-                        if (file.isFile())
-                        {
-                            for (String ip : readPatterns(file)) {
-                                log.debug("Loading {}", ip);
-                                if (!Character.isDigit(ip.charAt(0)))
-                                {
-                                    try {
-                                        ip = DnsLookup.forward(ip);
-                                        log.debug("Resolved to {}", ip);
-                                    } catch (IOException e) {
-                                        log.warn("Not loading {}:  {}", ip, e.getMessage());
-                                        continue;
-                                    }
-                                }
-                                table.add(ip);
-                            }
-                            log.info("Loaded Spider IP file: " + file);
-                        }
-                    }
-                } else {
-                    log.info("No spider file loaded");
-                }
-            }
-            catch (IOException | IPTable.IPFormatException e) {
-                log.error("Error Loading Spiders:" + e.getMessage(), e);
-            }
-
-        }
-
-    }
-
-    /**
-     * Load agent name patterns from all files in a single subdirectory of config/spiders.
-     *
-     * @param directory simple directory name (e.g. "agents").
-     *      "${dspace.dir}/config/spiders" will be prepended to yield the path to
-     *      the directory of pattern files.
-     * @param patternList patterns read from the files in {@code directory} will
-     *      be added to this List.
-     */
-    private static void loadPatterns(String directory, List<Pattern> patternList)
-    {
-        String dspaceHome = DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.dir");
-        File spidersDir = new File(dspaceHome, "config/spiders");
-        File patternsDir = new File(spidersDir, directory);
-        if (patternsDir.exists() && patternsDir.isDirectory())
-        {
-            for (File file : patternsDir.listFiles())
-            {
-                Set<String> patterns;
-                try
-                {
-                    patterns = readPatterns(file);
-                } catch (IOException ex)
-                {
-                    log.error("Patterns not read from {}:  {}",
-                            file.getPath(), ex.getMessage());
-                    continue;
-                }
-                for (String pattern : patterns)
-                {
-                    patternList.add(Pattern.compile(pattern,Pattern.CASE_INSENSITIVE));
-                }
-                log.info("Loaded pattern file:  {}", file.getPath());
-            }
-        }
-        else
-        {
-            log.info("No patterns loaded from {}", patternsDir.getPath());
-        }
     }
 
     /**
@@ -201,58 +70,7 @@ public class SpiderDetector {
     public static boolean isSpider(String clientIP, String proxyIPs,
                                    String hostname, String agent)
     {
-        // See if any agent patterns match
+        return spiderDetectorService.isSpider(clientIP, proxyIPs, hostname, agent);
-        if (null != agent)
-        {
-            synchronized(agents)
-            {
-                if (agents.isEmpty())
-                    loadPatterns("agents", agents);
-            }
-            for (Pattern candidate : agents)
-            {
-		// prevent matcher() invocation from a null Pattern object
-                if (null != candidate && candidate.matcher(agent).find())
-                {
-                    return true;
-                }
-            }
-        }
-
-        // No.  See if any IP addresses match
-        if (isUseProxies() && proxyIPs != null) {
-            /* This header is a comma delimited list */
-            for (String xfip : proxyIPs.split(",")) {
-                if (isSpider(xfip))
-                {
-                    return true;
-                }
-            }
-        }
-
-        if (isSpider(clientIP))
-            return true;
-
-        // No.  See if any DNS names match
-        if (null != hostname)
-        {
-            synchronized(domains)
-            {
-                if (domains.isEmpty())
-                    loadPatterns("domains", domains);
-            }
-            for (Pattern candidate : domains)
-            {
-		// prevent matcher() invocation from a null Pattern object
-		if (null != candidate && candidate.matcher(hostname).find())
-                {
-                    return true;
-                }
-            }
-        }
-
-        // Not a known spider.
-        return false;
     }
 
     /**
@@ -263,10 +81,7 @@ public class SpiderDetector {
      */
     public static boolean isSpider(HttpServletRequest request)
     {
-        return isSpider(request.getRemoteAddr(),
+        return spiderDetectorService.isSpider(request);
-                request.getHeader("X-Forwarded-For"),
-                request.getRemoteHost(),
-                request.getHeader("User-Agent"));
     }
 
     /**
@@ -276,30 +91,7 @@ public class SpiderDetector {
      * @return if is spider IP
      */
     public static boolean isSpider(String ip) {
-
+        return spiderDetectorService.isSpider(ip);
-        if (table == null) {
-            SpiderDetector.loadSpiderIpAddresses();
-        }
-
-        try {
-            if (table.contains(ip)) {
-                return true;
-            }
-        } catch (Exception e) {
-            return false;
-        }
-
-        return false;
-
-
-    }
-
-    private static boolean isUseProxies() {
-        if(useProxies == null) {
-            useProxies = DSpaceServicesFactory.getInstance().getConfigurationService().getBooleanProperty("useProxies");
-        }
-
-        return useProxies;
     }
 
 }

dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorService.java

@@ -0,0 +1,34 @@
+/**
+ * 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.statistics.util;
+
+import javax.servlet.http.HttpServletRequest;
+import java.io.File;
+import java.io.IOException;
+import java.util.Set;
+
+/**
+ * Interface to implement a SpiderDetectorService
+ * @author frederic at atmire.com
+ */
+public interface SpiderDetectorService {
+
+    public boolean isSpider(String clientIP, String proxyIPs, String hostname, String agent);
+
+    public boolean isSpider(HttpServletRequest request);
+
+    public boolean isSpider(String ip);
+
+    public void loadSpiderIpAddresses();
+
+    public Set<String> readPatterns(File patternFile)
+            throws IOException;
+
+    public IPTable getTable();
+
+}

dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorServiceImpl.java

@@ -0,0 +1,329 @@
+/**
+ * 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.statistics.util;
+
+import org.apache.commons.configuration.ConversionException;
+import org.apache.commons.lang.StringUtils;
+import org.dspace.services.ConfigurationService;
+import org.dspace.services.factory.DSpaceServicesFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+
+import javax.servlet.http.HttpServletRequest;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.util.*;
+import java.util.regex.Pattern;
+
+/**
+ * SpiderDetectorServiceImpl is used to find IP's that are spiders...
+ * In future someone may add Host Domains
+ * to the detection criteria here.
+ *
+ * @author kevinvandevelde at atmire.com
+ * @author ben at atmire.com
+ * @author Mark Diggory (mdiggory at atmire.com)
+ * @author frederic at atmire.com
+ */
+public class SpiderDetectorServiceImpl implements SpiderDetectorService {
+
+    private static final Logger log = LoggerFactory.getLogger(SpiderDetectorServiceImpl.class);
+
+    private Boolean useProxies;
+
+    private Boolean useCaseInsensitiveMatching;
+
+    private final List<Pattern> agents
+            = Collections.synchronizedList(new ArrayList<Pattern>());
+
+    private final List<Pattern> domains
+            = Collections.synchronizedList(new ArrayList<Pattern>());
+
+    private ConfigurationService configurationService;
+
+    /**
+     * Sparse HashTable structure to hold IP address ranges.
+     */
+    private IPTable table = null;
+
+    @Autowired(required = true)
+    public SpiderDetectorServiceImpl(ConfigurationService configurationService) {
+        this.configurationService = configurationService;
+    }
+
+    public IPTable getTable() {
+        return table;
+    }
+
+    /**
+     * Service Method for testing spiders against existing spider files.
+     * <p>
+     * In future spiders HashSet may be optimized as byte offset array to
+     * improve performance and memory footprint further.
+     *
+     * @param clientIP address of the client.
+     * @param proxyIPs comma-list of X-Forwarded-For addresses, or null.
+     * @param hostname domain name of host, or null.
+     * @param agent User-Agent header value, or null.
+     * @return true if the client matches any spider characteristics list.
+     */
+    public boolean isSpider(String clientIP, String proxyIPs, String hostname, String agent) {
+        // See if any agent patterns match
+        if (null != agent)
+        {
+            synchronized(agents)
+            {
+                if (agents.isEmpty())
+                    loadPatterns("agents", agents);
+            }
+
+            if(isUseCaseInsensitiveMatching()) {
+                agent = StringUtils.lowerCase(agent);
+                hostname = StringUtils.lowerCase(hostname);
+            }
+
+            for (Pattern candidate : agents) {
+
+                // prevent matcher() invocation from a null Pattern object
+                if (null != candidate && candidate.matcher(agent).find()) {
+                    return true;
+                }
+
+
+            }
+        }
+
+        // No.  See if any IP addresses match
+        if (isUseProxies() && proxyIPs != null) {
+            /* This header is a comma delimited list */
+            for (String xfip : proxyIPs.split(",")) {
+                if (isSpider(xfip))
+                {
+                    return true;
+                }
+            }
+        }
+
+        if (isSpider(clientIP))
+            return true;
+
+        // No.  See if any DNS names match
+        if (null != hostname)
+        {
+            synchronized(domains)
+            {
+                if (domains.isEmpty())
+                    loadPatterns("domains", domains);
+            }
+            for (Pattern candidate : domains)
+            {
+                // prevent matcher() invocation from a null Pattern object
+                if (null != candidate && candidate.matcher(hostname).find())
+                {
+                    return true;
+                }
+            }
+        }
+
+        // Not a known spider.
+        return false;
+    }
+
+    /**
+     * Utility method which reads lines from a file & returns them in a Set.
+     *
+     * @param patternFile the location of our spider file
+     * @return a vector full of patterns
+     * @throws IOException could not happen since we check the file be4 we use it
+     */
+    public Set<String> readPatterns(File patternFile)
+            throws IOException
+    {
+        Set<String> patterns = new HashSet<>();
+
+        if (!patternFile.exists() || !patternFile.isFile())
+        {
+            return patterns;
+        }
+
+        //Read our file & get all them patterns.
+        try (BufferedReader in = new BufferedReader(new FileReader(patternFile)))
+        {
+            String line;
+            while ((line = in.readLine()) != null) {
+                if (!line.startsWith("#")) {
+                    line = line.trim();
+
+                    if (!line.equals("")) {
+                        patterns.add(line);
+                    }
+                } else {
+                    //   ua.add(line.replaceFirst("#","").replaceFirst("UA","").trim());
+                    // ... add this functionality later
+                }
+            }
+        }
+        return patterns;
+    }
+
+    /**
+     * Load agent name patterns from all files in a single subdirectory of config/spiders.
+     *
+     * @param directory simple directory name (e.g. "agents").
+     *      "${dspace.dir}/config/spiders" will be prepended to yield the path to
+     *      the directory of pattern files.
+     * @param patternList patterns read from the files in {@code directory} will
+     *      be added to this List.
+     */
+    private void loadPatterns(String directory, List<Pattern> patternList)
+    {
+        String dspaceHome = configurationService.getProperty("dspace.dir");
+        File spidersDir = new File(dspaceHome, "config/spiders");
+        File patternsDir = new File(spidersDir, directory);
+        if (patternsDir.exists() && patternsDir.isDirectory())
+        {
+            for (File file : patternsDir.listFiles())
+            {
+                Set<String> patterns;
+                try
+                {
+                    patterns = readPatterns(file);
+                } catch (IOException ex)
+                {
+                    log.error("Patterns not read from {}:  {}",
+                            file.getPath(), ex.getMessage());
+                    continue;
+                }
+                //If case insensitive matching is enabled, lowercase the patterns so they can be lowercase matched
+                for (String pattern : patterns) {
+                    if(isUseCaseInsensitiveMatching()) {
+                        pattern = StringUtils.lowerCase(pattern);
+                    }
+                    patternList.add(Pattern.compile(pattern));
+                }
+
+
+                log.info("Loaded pattern file:  {}", file.getPath());
+            }
+        }
+        else
+        {
+            log.info("No patterns loaded from {}", patternsDir.getPath());
+        }
+    }
+
+    /**
+     * Service Method for testing spiders against existing spider files.
+     *
+     * @param request
+     * @return true|false if the request was detected to be from a spider.
+     */
+    public boolean isSpider(HttpServletRequest request) {
+        return isSpider(request.getRemoteAddr(),
+                request.getHeader("X-Forwarded-For"),
+                request.getRemoteHost(),
+                request.getHeader("User-Agent"));
+    }
+
+    /**
+     * Check individual IP is a spider.
+     *
+     * @param ip
+     * @return if is spider IP
+     */
+    public boolean isSpider(String ip) {
+        if (table == null) {
+            loadSpiderIpAddresses();
+        }
+
+        try {
+            if (table.contains(ip)) {
+                return true;
+            }
+        } catch (Exception e) {
+            return false;
+        }
+
+        return false;
+    }
+
+    /*
+     *  loader to populate the table from files.
+     */
+    public synchronized void loadSpiderIpAddresses() {
+
+        if (table == null) {
+            table = new IPTable();
+
+            String filePath = configurationService.getProperty("dspace.dir");
+
+            try {
+                File spidersDir = new File(filePath, "config/spiders");
+
+                if (spidersDir.exists() && spidersDir.isDirectory()) {
+                    for (File file : spidersDir.listFiles()) {
+                        if (file.isFile())
+                        {
+                            for (String ip : readPatterns(file)) {
+                                log.debug("Loading {}", ip);
+                                if (!Character.isDigit(ip.charAt(0)))
+                                {
+                                    try {
+                                        ip = DnsLookup.forward(ip);
+                                        log.debug("Resolved to {}", ip);
+                                    } catch (IOException e) {
+                                        log.warn("Not loading {}:  {}", ip, e.getMessage());
+                                        continue;
+                                    }
+                                }
+                                table.add(ip);
+                            }
+                            log.info("Loaded Spider IP file: " + file);
+                        }
+                    }
+                } else {
+                    log.info("No spider file loaded");
+                }
+            }
+            catch (IOException | IPTable.IPFormatException e) {
+                log.error("Error Loading Spiders:" + e.getMessage(), e);
+            }
+
+        }
+
+    }
+
+    /**
+     * checks if case insensitive matching is enabled
+     * @return true if it's enabled, false if not
+     */
+    private boolean isUseCaseInsensitiveMatching() {
+        if (useCaseInsensitiveMatching == null) {
+            try {
+                useCaseInsensitiveMatching = configurationService.getBooleanProperty("usage-statistics.bots.case-insensitive");
+            } catch (ConversionException e) {
+                useCaseInsensitiveMatching = false;
+                log.warn("Please use a boolean value for usage-statistics.bots.case-insensitive");
+            }
+        }
+
+        return useCaseInsensitiveMatching;
+    }
+
+    private boolean isUseProxies() {
+        if(useProxies == null) {
+            useProxies = configurationService.getBooleanProperty("useProxies");
+        }
+
+        return useProxies;
+    }
+
+}

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql

@@ -0,0 +1,16 @@
+--
+-- 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/
+--
+
+------------------------------------------------------
+-- DS-3563 Missing database index on metadatavalue.resource_type_id
+------------------------------------------------------
+-- Create an index on the metadata value resource_type_id column so that it can be searched efficiently.
+
+DROP INDEX IF EXISTS metadatavalue_resource_type_id_idx;
+
+CREATE INDEX metadatavalue_resource_type_id_idx ON metadatavalue (resource_type_id);

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql

@@ -0,0 +1,23 @@
+--
+-- 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/
+--
+
+------------------------------------------------------
+-- DS-3563 Missing database index on metadatavalue.resource_type_id
+------------------------------------------------------
+-- Create an index on the metadata value resource_type_id column so that it can be searched efficiently.
+declare
+    index_not_exists EXCEPTION;
+  PRAGMA EXCEPTION_INIT(index_not_exists, -1418);
+begin
+
+  execute immediate 'DROP INDEX metadatavalue_type_id_idx';
+  exception
+  when index_not_exists then null;
+end;
+
+CREATE INDEX metadatavalue_type_id_idx ON metadatavalue (resource_type_id);

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql

@@ -0,0 +1,16 @@
+--
+-- 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/
+--
+
+------------------------------------------------------
+-- DS-3563 Missing database index on metadatavalue.resource_type_id
+------------------------------------------------------
+-- Create an index on the metadata value resource_type_id column so that it can be searched efficiently.
+
+DROP INDEX IF EXISTS metadatavalue_resource_type_id_idx;
+
+CREATE INDEX metadatavalue_resource_type_id_idx ON metadatavalue (resource_type_id);

dspace-api/src/test/java/org/dspace/authorize/AuthorizeServiceTest.java

@@ -0,0 +1,145 @@
+/**
+ * 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.authorize;
+
+import org.dspace.AbstractUnitTest;
+import org.dspace.authorize.factory.AuthorizeServiceFactory;
+import org.dspace.authorize.service.ResourcePolicyService;
+import org.dspace.content.Community;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.CommunityService;
+import org.dspace.core.Constants;
+import org.dspace.eperson.EPerson;
+import org.dspace.eperson.Group;
+import org.dspace.eperson.factory.EPersonServiceFactory;
+import org.dspace.eperson.service.EPersonService;
+import org.dspace.eperson.service.GroupService;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.sql.SQLException;
+
+/**
+ * Created by pbecker as he wanted to write a test against DS-3572.
+ * This definitely needs to be extended, but it's at least a start.
+ */
+public class AuthorizeServiceTest  extends AbstractUnitTest
+{
+
+    protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
+    protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
+    protected ResourcePolicyService resourcePolicyService = AuthorizeServiceFactory.getInstance().getResourcePolicyService();
+    protected CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService();
+
+    public AuthorizeServiceTest()
+    {}
+
+    @Test
+    public void testauthorizeMethodDoesNotConfuseEPersonWithCurrentUser()
+    {
+        Community dso;
+        EPerson eperson1;
+        EPerson eperson2;
+        Group group;
+
+        try
+        {
+            context.turnOffAuthorisationSystem();
+
+            // create two epersons: one to test a permission the other one to be used as currentUser
+            eperson1 = ePersonService.create(context);
+            eperson2 = ePersonService.create(context);
+            // create a group as the bug described in DS-3572 contains a wrong group membership check
+            group = groupService.create(context);
+            // A group has to have a name, otherwise there are queries that break
+            groupService.setName(group, "My test group");
+            // add eperson1 to the group.
+            groupService.addMember(context, group, eperson1);
+            groupService.update(context, group);
+
+            // Use a top level community as DSpaceObject to test permissions
+            dso = communityService.create(null, context);
+
+            // grant write permission to the eperson1 by its group membership
+            authorizeService.addPolicy(context, dso, Constants.WRITE, group);
+            context.commit();
+
+            // set the other eperson as the current user
+            // Notice that it is not a member of the group, and does not have write permission
+            context.setCurrentUser(eperson2);
+        }
+        catch (SQLException | AuthorizeException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+        finally
+        {
+            context.restoreAuthSystemState();
+        }
+
+        try {
+            // eperson1 should be able to write as he is member of a group that has write permissions
+            Assert.assertTrue(authorizeService.authorizeActionBoolean(context, eperson1, dso, Constants.WRITE, true));
+            // person2 shouldn't have write access
+            Assert.assertFalse(authorizeService.authorizeActionBoolean(context, eperson2, dso, Constants.WRITE, true));
+        }
+        catch (SQLException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+    }
+
+    @Test
+    public void testauthorizeMethodRespectSpecialGroups()
+    {
+
+        EPerson eperson1;
+        EPerson eperson2;
+        Group group1;
+
+        Community dso;
+        try
+        {
+            context.turnOffAuthorisationSystem();
+
+            // create an eperson and a group
+            eperson1 = ePersonService.create(context);
+            group1 = groupService.create(context);
+            // A group has to have a name, otherwise there are queries that break
+            groupService.setName(group1, "My test group 2");
+
+            // Use a top level community as DSpaceObject to test permissions
+            dso = communityService.create(null, context);
+
+            // allow the group some action on a DSpaceObject and set it as
+            // special group to the user. Then test if the action on the DSO
+            // is allowed for the user
+            authorizeService.addPolicy(context, dso, Constants.ADD, group1);
+            context.setCurrentUser(eperson1);
+            context.setSpecialGroup(group1.getID());
+            context.commit();
+        }
+        catch (SQLException | AuthorizeException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+        finally
+        {
+            context.restoreAuthSystemState();
+        }
+
+        try {
+            Assert.assertTrue(authorizeService.authorizeActionBoolean(context, eperson1, dso, Constants.ADD, true));
+        }
+        catch (SQLException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+    }
+}

dspace-api/src/test/java/org/dspace/checker/dao/impl/ChecksumHistoryDAOImplTest.java

@@ -97,7 +97,7 @@ public class ChecksumHistoryDAOImplTest
         qry.setInteger("id", checkId);
         qry.setDate("date", matchDate);
         qry.setString("result", ChecksumResultCode.CHECKSUM_MATCH.name());
-        qry.setString("bitstream", bs.getID().toString()); // FIXME identifier not being set???
+        qry.setParameter("bitstream", bs.getID()); // FIXME identifier not being set???
         qry.executeUpdate();
 
         // Row with nonmatching result code
@@ -107,7 +107,7 @@ public class ChecksumHistoryDAOImplTest
         qry.setInteger("id", checkId);
         qry.setDate("date", noMatchDate);
         qry.setString("result", ChecksumResultCode.CHECKSUM_NO_MATCH.name());
-        qry.setString("bitstream", bs.getID().toString());
+        qry.setParameter("bitstream", bs.getID()); // FIXME identifier not being set???
         qry.executeUpdate();
 
         // Create one newer row
@@ -117,7 +117,7 @@ public class ChecksumHistoryDAOImplTest
         qry.setInteger("id", checkId);
         qry.setDate("date", new java.sql.Date(futureDate.getTime()));
         qry.setString("result", ChecksumResultCode.CHECKSUM_MATCH.name());
-        qry.setString("bitstream", bs.getID().toString());
+        qry.setParameter("bitstream", bs.getID()); // FIXME identifier not being set???
         qry.executeUpdate();
 
         // Test!

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

@@ -694,6 +694,69 @@ public class BundleTest extends AbstractDSpaceObjectTest
         assertTrue("testGetBitstreamPolicies 0", bspolicies.isEmpty());
     }
 
+    @Test
+    public void testSetOrder() throws SQLException, AuthorizeException, FileNotFoundException, IOException
+    {
+        new NonStrictExpectations(authorizeService.getClass())
+        {{
+            // Allow Bundle ADD perms
+            authorizeService.authorizeAction((Context) any, (Bundle) any,
+                Constants.ADD); result = null;
+            authorizeService.authorizeAction((Context) any, (Bitstream) any,
+                Constants.WRITE); result = null;
+        }};
+
+        // Create three Bitstreams to test ordering with. Give them different names
+        File f = new File(testProps.get("test.bitstream").toString());
+        Bitstream bs = bitstreamService.create(context, new FileInputStream(f));
+        bs.setName(context, "bitstream1");
+        bundleService.addBitstream(context, b, bs);
+        Bitstream bs2 = bitstreamService.create(context, new FileInputStream(f));
+        bs2.setName(context, "bitstream2");
+        bundleService.addBitstream(context, b, bs2);
+        Bitstream bs3 = bitstreamService.create(context, new FileInputStream(f));
+        bs3.setName(context, "bitstream3");
+        bundleService.addBitstream(context, b, bs3);
+
+        // Assert Bitstreams are in the order added
+        Bitstream[] bitstreams = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
+        assertTrue("testSetOrder: starting count correct", bitstreams.length == 3);
+        assertThat("testSetOrder: Bitstream 1 is first", bitstreams[0].getName(), equalTo(bs.getName()));
+        assertThat("testSetOrder: Bitstream 2 is second", bitstreams[1].getName(), equalTo(bs2.getName()));
+        assertThat("testSetOrder: Bitstream 3 is third", bitstreams[2].getName(), equalTo(bs3.getName()));
+
+        UUID bsID1 = bs.getID();
+        UUID bsID2 = bs2.getID();
+        UUID bsID3 = bs3.getID();
+
+        // Now define a new order and call setOrder()
+        UUID[] newBitstreamOrder = new UUID[] { bsID3, bsID1, bsID2 };
+        bundleService.setOrder(context, b, newBitstreamOrder);
+
+        // Assert Bitstreams are in the new order
+        bitstreams = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
+        assertTrue("testSetOrder: new count correct", bitstreams.length == 3);
+        assertThat("testSetOrder: Bitstream 3 is now first", bitstreams[0].getName(), equalTo(bs3.getName()));
+        assertThat("testSetOrder: Bitstream 1 is now second", bitstreams[1].getName(), equalTo(bs.getName()));
+        assertThat("testSetOrder: Bitstream 2 is now third", bitstreams[2].getName(), equalTo(bs2.getName()));
+
+        // Now give only a partial list of bitstreams
+        newBitstreamOrder = new UUID[] { bsID1, bsID2 };
+        bundleService.setOrder(context, b, newBitstreamOrder);
+
+        // Assert Bitstream order is unchanged
+        Bitstream[] bitstreamsAfterPartialData = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
+        assertThat("testSetOrder: Partial data doesn't change order", bitstreamsAfterPartialData, equalTo(bitstreams));
+
+        // Now give bad data in the list of bitstreams
+        newBitstreamOrder = new UUID[] { bsID1, null, bsID2 };
+        bundleService.setOrder(context, b, newBitstreamOrder);
+
+        // Assert Bitstream order is unchanged
+        Bitstream[] bitstreamsAfterBadData = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
+        assertThat("testSetOrder: Partial data doesn't change order", bitstreamsAfterBadData, equalTo(bitstreams));
+    }
+
     /**
      * Test of getAdminObject method, of class Bundle.
      */

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

@@ -7,6 +7,20 @@
  */
 package org.dspace.content;
 
+import mockit.NonStrictExpectations;
+import org.apache.log4j.Logger;
+import org.dspace.app.util.AuthorizeUtil;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.core.Constants;
+import org.dspace.core.Context;
+import org.dspace.core.factory.CoreServiceFactory;
+import org.dspace.core.service.LicenseService;
+import org.dspace.eperson.EPerson;
+import org.dspace.eperson.Group;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -15,20 +29,8 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.UUID;
 
-
-import org.dspace.authorize.AuthorizeException;
-import org.apache.log4j.Logger;
-import org.dspace.core.Context;
-import org.dspace.core.factory.CoreServiceFactory;
-import org.dspace.core.service.LicenseService;
-import org.dspace.eperson.EPerson;
-import org.dspace.eperson.Group;
-import org.junit.*;
-import static org.junit.Assert.* ;
 import static org.hamcrest.CoreMatchers.*;
-import mockit.NonStrictExpectations;
+import static org.junit.Assert.*;
-import org.dspace.app.util.AuthorizeUtil;
-import org.dspace.core.Constants;
 
 /**
  * Unit Tests for class Collection
@@ -1823,8 +1825,22 @@ public class CollectionTest extends AbstractDSpaceObjectTest
     @Test
     public void testGetCommunities() throws Exception
     {
-        assertThat("testGetCommunities 0",collection.getCommunities(), notNullValue());
+        context.turnOffAuthorisationSystem();
-        assertTrue("testGetCommunities 1",collection.getCommunities().size() == 1);
+        Community community = communityService.create(null, context);
+        communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "community 3");
+        this.collection.addCommunity(community);
+        community = communityService.create(null, context);
+        communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "community 1");
+        this.collection.addCommunity(community);
+        community = communityService.create(null, context);
+        communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "community 2");
+        this.collection.addCommunity(community);
+        context.restoreAuthSystemState();
+        assertTrue("testGetCommunities 0",collection.getCommunities().size() == 4);
+        //Communities should be sorted by name
+        assertTrue("testGetCommunities 1",collection.getCommunities().get(1).getName().equals("community 1"));
+        assertTrue("testGetCommunities 1",collection.getCommunities().get(2).getName().equals("community 2"));
+        assertTrue("testGetCommunities 1",collection.getCommunities().get(3).getName().equals("community 3"));
     }
 
     /**

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

@@ -8,22 +8,25 @@
 
 package org.dspace.content;
 
-import java.io.File;
+import mockit.NonStrictExpectations;
-import java.io.FileInputStream;
-import java.sql.SQLException;
-import java.util.List;
-import java.util.UUID;
-
 import org.apache.log4j.Logger;
 import org.dspace.app.util.AuthorizeUtil;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.eperson.Group;
-import org.junit.*;
+import org.junit.After;
-import static org.junit.Assert.* ;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.sql.SQLException;
+import java.util.List;
+import java.util.UUID;
+
 import static org.hamcrest.CoreMatchers.*;
-import mockit.NonStrictExpectations;
+import static org.junit.Assert.*;
 
 /**
  * Unit Tests for class Community
@@ -681,10 +684,20 @@ public class CommunityTest extends AbstractDSpaceObjectTest
         assertThat("testGetCollections 0",c.getCollections(), notNullValue());
         assertTrue("testGetCollections 1", c.getCollections().size() == 0);
 
-        Collection result = collectionService.create(context, c);
+        context.turnOffAuthorisationSystem();
-        assertThat("testGetCollections 2",c.getCollections(), notNullValue());
+        Collection collection = collectionService.create(context, c);
-        assertTrue("testGetCollections 3", c.getCollections().size() == 1);
+        collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection B");
-        assertThat("testGetCollections 4",c.getCollections().get(0), equalTo(result));
+        collection = collectionService.create(context, c);
+        collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection C");
+        collection = collectionService.create(context, c);
+        collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection A");
+        //we need to commit the changes so we don't block the table for testing
+        context.restoreAuthSystemState();
+
+        //sorted
+        assertTrue("testGetCollections 2",c.getCollections().get(0).getName().equals("collection A"));
+        assertTrue("testGetCollections 3",c.getCollections().get(1).getName().equals("collection B"));
+        assertTrue("testGetCollections 4",c.getCollections().get(2).getName().equals("collection C"));
     }
 
     /**
@@ -707,11 +720,20 @@ public class CommunityTest extends AbstractDSpaceObjectTest
         assertThat("testGetSubcommunities 0",c.getSubcommunities(), notNullValue());
         assertTrue("testGetSubcommunities 1", c.getSubcommunities().size() == 0);
 
-        //community with parent
+        context.turnOffAuthorisationSystem();
-        Community son = communityService.create(c, context);
+        Community community = communityService.create(c, context);
-        assertThat("testGetSubcommunities 2",c.getSubcommunities(), notNullValue());
+        communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "subcommunity B");
-        assertTrue("testGetSubcommunities 3", c.getSubcommunities().size() == 1);
+        community = communityService.create(c, context);
-        assertThat("testGetSubcommunities 4", c.getSubcommunities().get(0), equalTo(son));
+        communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "subcommunity A");
+        community = communityService.create(c, context);
+        communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "subcommunity C");
+        //we need to commit the changes so we don't block the table for testing
+        context.restoreAuthSystemState();
+
+        //get Subcommunities sorted
+        assertTrue("testGetCollections 2",c.getSubcommunities().get(0).getName().equals("subcommunity A"));
+        assertTrue("testGetCollections 3",c.getSubcommunities().get(1).getName().equals("subcommunity B"));
+        assertTrue("testGetCollections 4",c.getSubcommunities().get(2).getName().equals("subcommunity C"));
     }
 
     /**

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

@@ -15,6 +15,7 @@ import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.*;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -122,7 +123,7 @@ public class InstallItemTest extends AbstractUnitTest
     public void testInstallItem_validHandle() throws Exception
     {
         context.turnOffAuthorisationSystem();
-        String handle = "123456789/567";
+        String handle = "123456789/56789";
         WorkspaceItem is = workspaceItemService.create(context, collection, false);
       
         //Test assigning a specified handle to an item
@@ -147,9 +148,10 @@ public class InstallItemTest extends AbstractUnitTest
                     Constants.ADD); result = false;
             // Allow full Admin perms
                 authorizeService.isAdmin((Context) any); result = true;
+                authorizeService.isAdmin((Context) any, (EPerson) any); result = true;
         }};
 
-        String handle = "123456789/567";
+        String handle = "123456789/56789";
         WorkspaceItem is = workspaceItemService.create(context, collection, false);
         WorkspaceItem is2 = workspaceItemService.create(context, collection, false);
         
@@ -170,7 +172,7 @@ public class InstallItemTest extends AbstractUnitTest
     public void testRestoreItem() throws Exception
     {
         context.turnOffAuthorisationSystem();
-        String handle = "123456789/567";
+        String handle = "123456789/56789";
         WorkspaceItem is = workspaceItemService.create(context, collection, false);
 
         //get current date
@@ -240,7 +242,7 @@ public class InstallItemTest extends AbstractUnitTest
     {
         //create a dummy WorkspaceItem
         context.turnOffAuthorisationSystem();
-        String handle = "123456789/567";
+        String handle = "123456789/56789";
         WorkspaceItem is = workspaceItemService.create(context, collection, false);
 
         // Set "today" as "dc.date.issued"
@@ -274,7 +276,7 @@ public class InstallItemTest extends AbstractUnitTest
     {
         //create a dummy WorkspaceItem with no dc.date.issued
         context.turnOffAuthorisationSystem();
-        String handle = "123456789/567";
+        String handle = "123456789/56789";
         WorkspaceItem is = workspaceItemService.create(context, collection, false);
 
         Item result = installItemService.installItem(context, is, handle);
@@ -293,7 +295,7 @@ public class InstallItemTest extends AbstractUnitTest
     {
         //create a dummy WorkspaceItem
         context.turnOffAuthorisationSystem();
-        String handle = "123456789/567";
+        String handle = "123456789/56789";
         WorkspaceItem is = workspaceItemService.create(context, collection, false);
 
         // Set "today" as "dc.date.issued"

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

@@ -7,32 +7,33 @@
  */
 package org.dspace.content;
 
+import mockit.NonStrictExpectations;
+import org.apache.commons.lang.time.DateUtils;
+import org.apache.log4j.Logger;
+import org.dspace.app.util.AuthorizeUtil;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.authorize.ResourcePolicy;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.content.service.BitstreamFormatService;
+import org.dspace.content.service.MetadataFieldService;
+import org.dspace.content.service.MetadataSchemaService;
+import org.dspace.core.Constants;
+import org.dspace.core.Context;
+import org.dspace.eperson.EPerson;
+import org.dspace.eperson.Group;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.sql.SQLException;
-
-import org.apache.commons.lang.time.DateUtils;
-import org.dspace.authorize.AuthorizeException;
-import org.apache.log4j.Logger;
-
 import java.util.*;
 
-import org.dspace.content.factory.ContentServiceFactory;
-import org.dspace.content.service.BitstreamFormatService;
-import org.dspace.content.service.MetadataFieldService;
-import org.dspace.content.service.MetadataSchemaService;
-import org.dspace.core.Context;
-import org.dspace.eperson.EPerson;
-import org.dspace.eperson.Group;
-import org.junit.*;
-import static org.junit.Assert.* ;
 import static org.hamcrest.CoreMatchers.*;
-import mockit.*;
+import static org.junit.Assert.*;
-import org.dspace.app.util.AuthorizeUtil;
-import org.dspace.authorize.ResourcePolicy;
-import org.dspace.core.Constants;
 
 /**
  * Unit Tests for class Item
@@ -107,28 +108,30 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     @Override
     public void destroy()
     {
-//        try {
         context.turnOffAuthorisationSystem();
-            //Get new instances, god knows what happended before
+        try {
-//            it = itemService.find(context, it.getID());
+            itemService.delete(context, it);
-//            collection = collectionService.find(context, collection.getID());
+        } catch(Exception e){
-//            owningCommunity = communityService.find(context, owningCommunity.getID());
+        }
-//
+        
-//            communityService.delete(context, owningCommunity);
+        try {
-//            context.commit();
+            collectionService.delete(context, collection);
-//            context.restoreAuthSystemState();
+        } catch(Exception e){
+        }
+        
+        try {
+            communityService.delete(context, owningCommunity);
+        } catch(Exception e){
+        }
+        
+        context.restoreAuthSystemState();
         it = null;
         collection = null;
         owningCommunity = null;
+        try {
             super.destroy();
-//        } catch (SQLException | AuthorizeException | IOException ex) {
+        } catch(Exception e){
-//            if(context.isValid())
+        }
-//            {
-//                context.abort();
-//            }
-//            log.error("Error in destroy", ex);
-//            fail("Error in destroy: " + ex.getMessage());
-//        }
     }
 
 
@@ -612,8 +615,18 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     @Test
     public void testGetCollections() throws Exception
     {
+        context.turnOffAuthorisationSystem();
+        Collection collection = collectionService.create(context, owningCommunity);
+        collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection B");
+        it.addCollection(collection);
+        collection = collectionService.create(context, owningCommunity);
+        collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection A");
+        it.addCollection(collection);
+        context.restoreAuthSystemState();
         assertThat("testGetCollections 0", it.getCollections(), notNullValue());
-        assertTrue("testGetCollections 1", it.getCollections().size() == 1);
+        assertTrue("testGetCollections 1", it.getCollections().size() == 3);
+        assertTrue("testGetCollections 2", it.getCollections().get(1).getName().equals("collection A"));
+        assertTrue("testGetCollections 3", it.getCollections().get(2).getName().equals("collection B"));
     }
 
     /**
@@ -1478,6 +1491,7 @@ public class ItemTest  extends AbstractDSpaceObjectTest
         context.turnOffAuthorisationSystem();
         Collection from = createCollection();
         Collection to = createCollection();
+        it.addCollection(from);
         it.setOwningCollection(from);
 
         itemService.move(context, it, from, to);

dspace-api/src/test/java/org/dspace/content/comparator/NameAscendingComparatorTest.java

@@ -0,0 +1,97 @@
+/**
+ * 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.content.comparator;
+
+import org.dspace.content.DSpaceObject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class NameAscendingComparatorTest {
+
+    private NameAscendingComparator comparator = new NameAscendingComparator();
+
+    @Mock
+    private DSpaceObject dso1;
+
+    @Mock
+    private DSpaceObject dso2;
+
+
+    @Test
+    public void testCompareLessThan() throws Exception {
+        when(dso1.getName()).thenReturn("a");
+        when(dso2.getName()).thenReturn("b");
+
+        assertTrue(comparator.compare(dso1, dso2) < 0);
+    }
+
+    @Test
+    public void testCompareGreaterThan() throws Exception {
+        when(dso1.getName()).thenReturn("b");
+        when(dso2.getName()).thenReturn("a");
+
+        assertTrue(comparator.compare(dso1, dso2) > 0);
+    }
+
+    @Test
+    public void testCompareEqual() throws Exception {
+        when(dso1.getName()).thenReturn("b");
+        when(dso2.getName()).thenReturn("b");
+
+        assertTrue(comparator.compare(dso1, dso2) == 0);
+    }
+
+    @Test
+    public void testCompareFirstNull() throws Exception {
+        when(dso2.getName()).thenReturn("b");
+
+        assertTrue(comparator.compare(null, dso2) < 0);
+    }
+
+    @Test
+    public void testCompareSecondNull() throws Exception {
+        when(dso1.getName()).thenReturn("a");
+
+        assertTrue(comparator.compare(dso1, null) > 0);
+    }
+
+    @Test
+    public void testCompareBothNull() throws Exception {
+        assertTrue(comparator.compare(null, null) == 0);
+    }
+
+    @Test
+    public void testCompareNameNull() throws Exception {
+        when(dso1.getName()).thenReturn(null);
+        when(dso2.getName()).thenReturn("b");
+
+        assertTrue(comparator.compare(dso1, dso2) < 0);
+    }
+
+    @Test
+    public void testCompareCaseInsensitive() throws Exception {
+        when(dso1.getName()).thenReturn("a");
+        when(dso2.getName()).thenReturn("B");
+
+        assertTrue(comparator.compare(dso1, dso2) < 0);
+    }
+
+    @Test
+    public void testCompareCaseTrimmed() throws Exception {
+        when(dso1.getName()).thenReturn("a");
+        when(dso2.getName()).thenReturn(" b ");
+
+        assertTrue(comparator.compare(dso1, dso2) < 0);
+    }
+}

dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java

@@ -167,6 +167,11 @@ public class ITDSpaceAIP extends AbstractUnitTest
             ePersonService.update(context, submitter);
             context.setCurrentUser(submitter);
 
+            //Make our test ePerson an admin so he can perform deletes and restores
+            GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
+            Group adminGroup = groupService.findByName(context, Group.ADMIN);
+            groupService.addMember(context, adminGroup, submitter);
+
             // Create our primary Test Item
             WorkspaceItem wsItem = workspaceItemService.create(context, grandchildCol, false);
             Item item = installItemService.installItem(context, wsItem);

dspace-api/src/test/java/org/dspace/core/ContextReadOnlyCacheTest.java

@@ -0,0 +1,121 @@
+/**
+ * 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.core;
+
+import org.dspace.content.Item;
+import org.dspace.eperson.EPerson;
+import org.dspace.eperson.Group;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.UUID;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.when;
+
+/**
+ * Class to test the read-only Context cache
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ContextReadOnlyCacheTest {
+
+    private ContextReadOnlyCache readOnlyCache;
+
+    @Mock
+    private EPerson ePerson;
+
+    @Before
+    public void init() {
+        readOnlyCache = new ContextReadOnlyCache();
+        when(ePerson.getID()).thenReturn(UUID.randomUUID());
+    }
+
+    @Test
+    public void cacheAuthorizedAction() throws Exception {
+        Item item = Mockito.mock(Item.class);
+        when(item.getID()).thenReturn(UUID.randomUUID());
+
+        readOnlyCache.cacheAuthorizedAction(item, Constants.READ, ePerson, true);
+        readOnlyCache.cacheAuthorizedAction(item, Constants.WRITE, ePerson, false);
+
+        assertTrue(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, ePerson));
+        assertFalse(readOnlyCache.getCachedAuthorizationResult(item, Constants.WRITE, ePerson));
+
+        assertNull(readOnlyCache.getCachedAuthorizationResult(item, Constants.ADMIN, ePerson));
+        assertNull(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, null));
+        assertNull(readOnlyCache.getCachedAuthorizationResult(null, Constants.READ, ePerson));
+    }
+
+    @Test
+    public void cacheGroupMembership() throws Exception {
+        Group group1 = buildGroupMock("Test Group 1");
+        Group group2 = buildGroupMock("Test Group 2");
+        Group group3 = buildGroupMock("Test Group 3");
+
+        readOnlyCache.cacheGroupMembership(group1, ePerson, true);
+        readOnlyCache.cacheGroupMembership(group2, ePerson, false);
+
+        assertTrue(readOnlyCache.getCachedGroupMembership(group1, ePerson));
+        assertFalse(readOnlyCache.getCachedGroupMembership(group2, ePerson));
+
+        assertNull(readOnlyCache.getCachedGroupMembership(group3, ePerson));
+        assertNull(readOnlyCache.getCachedGroupMembership(null, ePerson));
+        assertNull(readOnlyCache.getCachedGroupMembership(group2, null));
+    }
+
+    @Test
+    public void cacheAllMemberGroupsSet() throws Exception {
+        Group group1 = buildGroupMock("Test Group 1");
+        Group group2 = buildGroupMock("Test Group 2");
+        Group group3 = buildGroupMock("Test Group 3");
+
+        readOnlyCache.cacheAllMemberGroupsSet(ePerson, new HashSet<>(Arrays.asList(group1, group2)));
+
+        assertTrue(readOnlyCache.getCachedGroupMembership(group1, ePerson));
+        assertTrue(readOnlyCache.getCachedGroupMembership(group2, ePerson));
+        assertFalse(readOnlyCache.getCachedGroupMembership(group3, ePerson));
+        assertFalse(readOnlyCache.getCachedGroupMembership(null, ePerson));
+
+        assertNull(readOnlyCache.getCachedGroupMembership(group2, null));
+    }
+
+    @Test
+    public void clear() throws Exception {
+        Item item = Mockito.mock(Item.class);
+        when(item.getID()).thenReturn(UUID.randomUUID());
+        Group group1 = buildGroupMock("Test Group 1");
+
+        //load data into the cache
+        readOnlyCache.cacheAuthorizedAction(item, Constants.READ, ePerson, true);
+        readOnlyCache.cacheGroupMembership(group1, ePerson, true);
+
+        //double check the data is there
+        assertTrue(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, ePerson));
+        assertTrue(readOnlyCache.getCachedGroupMembership(group1, ePerson));
+
+        //clear the cache
+        readOnlyCache.clear();
+
+        //check that the data is not present anymore
+        assertNull(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, ePerson));
+        assertNull(readOnlyCache.getCachedGroupMembership(group1, ePerson));
+    }
+
+    private Group buildGroupMock(final String name) {
+        Group group = Mockito.mock(Group.class);
+        when(group.getName()).thenReturn(name);
+        return group;
+    }
+
+}

dspace-api/src/test/java/org/dspace/eperson/GroupTest.java

@@ -206,6 +206,10 @@ public class GroupTest extends AbstractUnitTest {
         List<String> names = new ArrayList<>();
         List<String> sortedNames = new ArrayList<>();
         for (Group group : groups) {
+            // Ignore any unnamed groups. This is only necessary when running unit tests via a persistent database (e.g. Postgres) as unnamed groups may be created by other tests.
+            if (group.getName() == null) {
+                continue;
+            }
             names.add(group.getName());
             sortedNames.add(group.getName());
         }
@@ -331,6 +335,18 @@ public class GroupTest extends AbstractUnitTest {
         assertFalse("isMemberGroup 4", groupService.isMember(level2Group, level1Group));
     }
 
+    @Test
+    public void isSubgroupOf() throws SQLException {
+        assertTrue("isMemberGroup 1", groupService.isParentOf(context, topGroup, level1Group));
+        assertTrue("isMemberGroup 2", groupService.isParentOf(context, level1Group, level2Group));
+        assertFalse("isMemberGroup 3", groupService.isParentOf(context, level1Group, topGroup));
+        assertFalse("isMemberGroup 4", groupService.isParentOf(context, level2Group, level1Group));
+
+        //Also check ancestor relations
+        assertTrue("isMemberGroup 5", groupService.isParentOf(context, topGroup, level2Group));
+        assertFalse("isMemberGroup 6", groupService.isParentOf(context, level2Group, topGroup));
+    }
+
     @Test
     public void isMemberEPerson() throws SQLException, AuthorizeException, EPersonDeletionException, IOException {
         EPerson ePerson = null;
@@ -355,9 +371,9 @@ public class GroupTest extends AbstractUnitTest {
             ePerson = createEPersonAndAddToGroup("isMemberContext@dspace.org", level2Group);
 
             context.setCurrentUser(ePerson);
-            assertTrue(groupService.isMember(context, topGroup));
+            assertTrue(groupService.isMember(context, ePerson, topGroup));
-            assertTrue(groupService.isMember(context, level1Group));
+            assertTrue(groupService.isMember(context, ePerson, level1Group));
-            assertTrue(groupService.isMember(context, level2Group));
+            assertTrue(groupService.isMember(context, ePerson, level2Group));
         } finally {
             if(ePerson != null)
             {
@@ -373,10 +389,9 @@ public class GroupTest extends AbstractUnitTest {
         try {
             ePerson = createEPersonAndAddToGroup("isMemberContextGroupId@dspace.org", level2Group);
 
-            context.setCurrentUser(ePerson);
+            assertTrue(groupService.isMember(context, ePerson, topGroup.getName()));
-            assertTrue(groupService.isMember(context, topGroup));
+            assertTrue(groupService.isMember(context, ePerson, level1Group.getName()));
-            assertTrue(groupService.isMember(context, level1Group));
+            assertTrue(groupService.isMember(context, ePerson, level2Group.getName()));
-            assertTrue(groupService.isMember(context, level2Group));
         } finally {
             if(ePerson != null)
             {
@@ -386,6 +401,117 @@ public class GroupTest extends AbstractUnitTest {
         }
     }
 
+    @Test
+    public void isMemberContextSpecialGroup() throws SQLException, AuthorizeException, EPersonDeletionException, IOException {
+        EPerson ePerson = null;
+        Group specialGroup = null;
+        try {
+            specialGroup = createGroup("specialGroup");
+            groupService.addMember(context, level1Group, specialGroup);
+            groupService.update(context, level1Group);
+
+            ePerson = createEPerson("isMemberContextGroupSpecial@dspace.org");
+
+            context.setCurrentUser(ePerson);
+            context.setSpecialGroup(specialGroup.getID());
+
+            assertTrue(groupService.isMember(context, topGroup));
+            assertTrue(groupService.isMember(context, level1Group));
+            assertFalse(groupService.isMember(context, level2Group));
+            assertTrue(groupService.isMember(context, specialGroup));
+
+        } finally {
+            if(ePerson != null)
+            {
+                context.turnOffAuthorisationSystem();
+                ePersonService.delete(context, ePerson);
+            }
+            if(specialGroup != null)
+            {
+                context.turnOffAuthorisationSystem();
+                groupService.delete(context, specialGroup);
+            }
+        }
+    }
+
+    @Test
+    public void isMemberContextSpecialGroupOtherUser() throws SQLException, AuthorizeException, EPersonDeletionException, IOException {
+        EPerson ePerson1 = null;
+        EPerson ePerson2 = null;
+        Group specialGroup = null;
+        try {
+            specialGroup = createGroup("specialGroup");
+            groupService.addMember(context, level2Group, specialGroup);
+            groupService.update(context, level2Group);
+
+            //The authenticated user has a special group
+            ePerson1 = createEPerson("isMemberContextGroupSpecial@dspace.org");
+            context.setCurrentUser(ePerson1);
+            context.setSpecialGroup(specialGroup.getID());
+
+            //Or second user is member of the level 1 group
+            ePerson2 = createEPersonAndAddToGroup("isMemberContextSpecialGroupOtherUser@dspace.org", level1Group);
+
+            assertTrue(groupService.isMember(context, ePerson2, topGroup));
+            assertTrue(groupService.isMember(context, ePerson2, level1Group));
+            assertFalse(groupService.isMember(context, ePerson2, level2Group));
+            assertFalse(groupService.isMember(context, ePerson2, specialGroup));
+
+            assertTrue(groupService.isMember(context, ePerson1, level2Group));
+            assertTrue(groupService.isMember(context, ePerson1, specialGroup));
+
+        } finally {
+            if(ePerson1 != null)
+            {
+                context.turnOffAuthorisationSystem();
+                ePersonService.delete(context, ePerson1);
+            }
+            if(ePerson2 != null)
+            {
+                context.turnOffAuthorisationSystem();
+                ePersonService.delete(context, ePerson2);
+            }
+            if(specialGroup != null)
+            {
+                context.turnOffAuthorisationSystem();
+                groupService.delete(context, specialGroup);
+            }
+        }
+    }
+
+    @Test
+    public void isMemberContextSpecialGroupDbMembership() throws SQLException, AuthorizeException, EPersonDeletionException, IOException {
+        EPerson ePerson = null;
+        Group specialGroup = null;
+        try {
+            specialGroup = createGroup("specialGroup");
+            groupService.addMember(context, level1Group, specialGroup);
+            groupService.update(context, level1Group);
+
+            ePerson = createEPersonAndAddToGroup("isMemberContextGroupSpecialDbMembership@dspace.org", level2Group);
+
+            context.setCurrentUser(ePerson);
+            context.setSpecialGroup(specialGroup.getID());
+
+            assertTrue(groupService.isMember(context, topGroup));
+            assertTrue(groupService.isMember(context, level1Group));
+            assertTrue(groupService.isMember(context, level2Group));
+            assertTrue(groupService.isMember(context, specialGroup));
+
+        } finally {
+            if(ePerson != null)
+            {
+                context.turnOffAuthorisationSystem();
+                ePersonService.delete(context, ePerson);
+            }
+            if(specialGroup != null)
+            {
+                context.turnOffAuthorisationSystem();
+                groupService.delete(context, specialGroup);
+            }
+        }
+    }
+
     @Test
     public void isPermanent()
             throws SQLException
@@ -443,10 +569,15 @@ public class GroupTest extends AbstractUnitTest {
     }
 
     @Test
-    public void removeMemberGroup() throws SQLException {
+    public void removeMemberGroup() throws SQLException, AuthorizeException {
         assertTrue(groupService.isMember(topGroup, level1Group));
+        assertTrue(groupService.isParentOf(context, topGroup, level1Group));
+
         groupService.removeMember(context, topGroup, level1Group);
+        groupService.update(context, topGroup);
+
         assertFalse(groupService.isMember(topGroup, level1Group));
+        assertFalse(groupService.isParentOf(context, topGroup, level1Group));
     }
 
     @Test

dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorServiceImplTest.java

@@ -0,0 +1,372 @@
+/**
+ * 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.statistics.util;
+
+
+import mockit.Mock;
+import mockit.MockUp;
+import org.dspace.AbstractDSpaceTest;
+import org.dspace.services.ConfigurationService;
+import org.dspace.services.factory.DSpaceServicesFactory;
+import org.dspace.statistics.SolrLoggerServiceImpl;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import static org.mockito.Mockito.mock;
+
+/**
+ * @author mwood
+ * @author frederic at atmire.com
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class SpiderDetectorServiceImplTest extends AbstractDSpaceTest
+{
+    private static final String NOT_A_BOT_ADDRESS = "192.168.0.1";
+
+    private ConfigurationService configurationService;
+
+
+    private SpiderDetectorService spiderDetectorService;
+
+    @Before
+    public void init() {
+        configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
+        spiderDetectorService = new SpiderDetectorServiceImpl(configurationService);
+
+    }
+
+    @Test
+    public void testReadPatterns()
+    {
+// FIXME        fail("Not yet implemented");
+    }
+
+    @Test
+    public void testGetSpiderIpAddresses()
+    {
+// FIXME        fail("Not yet implemented");
+    }
+
+    /**
+     * Test if Case Insitive matching option works
+     * @throws Exception
+     */
+    @Test
+    public void testCaseInsensitiveMatching() throws Exception
+    {
+        configurationService.setProperty("usage-statistics.bots.case-insensitive", true);
+        spiderDetectorService = new SpiderDetectorServiceImpl(configurationService);
+
+        DummyHttpServletRequest req = new DummyHttpServletRequest();
+        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
+        req.setRemoteHost("notabot.example.com"); // avoid surprises
+        req.setAgent("Firefox"); // avoid surprises
+
+        String candidate;
+
+        // Test agent patterns
+        req.setAgent("msnboT Is WaTching you");
+        assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req));
+
+        req.setAgent("FirefOx");
+        assertFalse("'Firefox' matched a pattern", spiderDetectorService.isSpider(req));
+
+        // Test IP patterns
+        candidate = "192.168.2.1";
+        req.setAddress(candidate);
+        assertTrue(candidate + " did not match IP patterns", spiderDetectorService.isSpider(req));
+
+        req.setAddress(NOT_A_BOT_ADDRESS);
+        assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", spiderDetectorService.isSpider(req));
+
+        // Test DNS patterns
+        candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com";
+        req.setRemoteHost(candidate);
+        assertTrue(candidate + " did match DNS patterns", spiderDetectorService.isSpider(req));
+
+        candidate = "wIki.dsPace.oRg";
+        req.setRemoteHost(candidate);
+        assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req));
+
+    }
+
+    /**
+     * Test method for {@link org.dspace.statistics.util.SpiderDetectorService#isSpider(javax.servlet.http.HttpServletRequest)}.
+     */
+    @Test
+    public void testIsSpiderHttpServletRequest()
+    {
+        DummyHttpServletRequest req = new DummyHttpServletRequest();
+        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
+        req.setRemoteHost("notabot.example.com"); // avoid surprises
+        req.setAgent("Firefox"); // avoid surprises
+
+        String candidate;
+
+        // Test agent patterns
+        req.setAgent("msnbot is watching you");
+        assertTrue("'msnbot' did not match any pattern", spiderDetectorService.isSpider(req));
+
+        req.setAgent("Firefox");
+        assertFalse("'Firefox' matched a pattern", spiderDetectorService.isSpider(req));
+
+        // Test IP patterns
+        candidate = "192.168.2.1";
+        req.setAddress(candidate);
+        assertTrue(candidate + " did not match IP patterns", spiderDetectorService.isSpider(req));
+
+        req.setAddress(NOT_A_BOT_ADDRESS);
+        assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", spiderDetectorService.isSpider(req));
+
+        // Test DNS patterns
+        candidate = "baiduspider-dspace-test.crawl.baidu.com";
+        req.setRemoteHost(candidate);
+        assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req));
+
+        candidate = "wiki.dspace.org";
+        req.setRemoteHost(candidate);
+        assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req));
+    }
+
+    /**
+     * Test method for {@link org.dspace.statistics.util.SpiderDetectorService#isSpider(java.lang.String, java.lang.String, java.lang.String, java.lang.String)}.
+     */
+    @Test
+    public void testIsSpiderStringStringStringString()
+    {
+        String candidate;
+
+        // Test IP patterns
+        candidate = "192.168.2.1";
+        assertTrue(candidate + " did not match IP patterns",
+                spiderDetectorService.isSpider(candidate, null, null, null));
+
+        candidate = NOT_A_BOT_ADDRESS;
+        assertFalse(candidate + " matched IP patterns",
+                spiderDetectorService.isSpider(candidate, null, null, null));
+
+        // Test DNS patterns
+        candidate = "baiduspider-dspace-test.crawl.baidu.com";
+        assertTrue(candidate + " did not match DNS patterns",
+                spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null));
+
+        candidate = "wiki.dspace.org";
+        assertFalse(candidate + " matched DNS patterns",
+                spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null));
+
+        // Test agent patterns
+        candidate = "msnbot is watching you";
+        assertTrue("'" + candidate + "' did not match agent patterns",
+                spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate));
+
+        candidate = "Firefox";
+        assertFalse("'" + candidate + "' matched agent patterns",
+                spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate));
+    }
+
+    /**
+     * Test method for {@link org.dspace.statistics.util.SpiderDetectorService#isSpider(java.lang.String)}.
+     */
+    @Test
+    public void testIsSpiderString()
+    {
+        String candidate;
+
+        candidate = "192.168.2.1";
+        assertTrue(candidate + " did not match IP patterns",
+                spiderDetectorService.isSpider(candidate, null, null, null));
+
+        candidate = NOT_A_BOT_ADDRESS;
+        assertFalse(candidate + " matched IP patterns",
+                spiderDetectorService.isSpider(candidate, null, null, null));
+
+    }
+
+
+    /**
+     * Test if Case Sensitive matching still works after adding the option
+     * @throws Exception
+     */
+    @Test
+    public void testCaseSensitiveMatching() throws Exception
+    {
+
+        DummyHttpServletRequest req = new DummyHttpServletRequest();
+        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
+        req.setRemoteHost("notabot.example.com"); // avoid surprises
+        req.setAgent("Firefox"); // avoid surprises
+
+        String candidate;
+
+        // Test agent patterns
+        req.setAgent("msnboT Is WaTching you");
+        assertFalse("'msnbot' matched pattern", spiderDetectorService.isSpider(req));
+
+        req.setAgent("FirefOx");
+        assertFalse("'Firefox' matched a pattern", spiderDetectorService.isSpider(req));
+
+        // Test IP patterns
+        candidate = "192.168.2.1";
+        req.setAddress(candidate);
+        assertTrue(candidate + " did not match IP patterns", spiderDetectorService.isSpider(req));
+
+        req.setAddress(NOT_A_BOT_ADDRESS);
+        assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", spiderDetectorService.isSpider(req));
+
+        // Test DNS patterns
+        candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com";
+        req.setRemoteHost(candidate);
+        assertFalse(candidate + " did match DNS patterns", spiderDetectorService.isSpider(req));
+
+        candidate = "wIki.dsPace.oRg";
+        req.setRemoteHost(candidate);
+        assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req));
+
+    }
+
+    /**
+     * Test to see that lowercase will be matched but uppercase won't
+     */
+    @Test
+    public void testInsensitiveSensitiveDifference() {
+
+        DummyHttpServletRequest req = new DummyHttpServletRequest();
+        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
+        req.setRemoteHost("notabot.example.com"); // avoid surprises
+        req.setAgent("Firefox"); // avoid surprises
+
+        String candidate;
+
+        // Test agent patterns
+        req.setAgent("msnbot is WaTching you");
+        assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req));
+
+        req.setAgent("MSNBOT Is WaTching you");
+        assertFalse("'msnbot' matched pattern", spiderDetectorService.isSpider(req));
+
+        // Test DNS patterns
+        candidate = "baiduspider-dspace-test.crawl.baidu.com";
+        req.setRemoteHost(candidate);
+        assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req));
+
+        candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com";
+        req.setRemoteHost(candidate);
+        assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req));
+    }
+
+    /**
+     * Test to check if the same agent gets caught with and without upper-casing
+     */
+    @Test
+    public void testBothLowerAndUpperCaseGetMatched() {
+
+        configurationService.setProperty("usage-statistics.bots.case-insensitive", true);
+        spiderDetectorService = new SpiderDetectorServiceImpl(configurationService);
+
+        DummyHttpServletRequest req = new DummyHttpServletRequest();
+        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
+        req.setRemoteHost("notabot.example.com"); // avoid surprises
+        req.setAgent("Firefox"); // avoid surprises
+
+        String candidate;
+
+        // Test agent patterns
+        req.setAgent("msnbot is WaTching you");
+        assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req));
+
+        req.setAgent("MSNBOT Is WaTching you");
+        assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req));
+
+        // Test DNS patterns
+        candidate = "baiduspider-dspace-test.crawl.baidu.com";
+        req.setRemoteHost(candidate);
+        assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req));
+
+        candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com";
+        req.setRemoteHost(candidate);
+        assertTrue(candidate + " didn't match DNS patterns", spiderDetectorService.isSpider(req));
+    }
+
+    /**
+     * Test if wrong value is used for property
+     */
+    @Test
+    public void testNonBooleanConfig() {
+        configurationService.setProperty("usage-statistics.bots.case-insensitive", "RandomNonBooleanString");
+        spiderDetectorService = new SpiderDetectorServiceImpl(configurationService);
+
+        DummyHttpServletRequest req = new DummyHttpServletRequest();
+        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
+        req.setRemoteHost("notabot.example.com"); // avoid surprises
+        req.setAgent("Firefox"); // avoid surprises
+
+        String candidate;
+
+        // Test agent patterns
+        req.setAgent("msnbot is WaTching you");
+        assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req));
+
+        req.setAgent("MSNBOT Is WaTching you");
+        assertFalse("'msnbot' matched pattern", spiderDetectorService.isSpider(req));
+
+        // Test DNS patterns
+        candidate = "baiduspider-dspace-test.crawl.baidu.com";
+        req.setRemoteHost(candidate);
+        assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req));
+
+        candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com";
+        req.setRemoteHost(candidate);
+        assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req));
+
+
+    }
+
+
+
+    /**
+     * Method to make sure the SpiderDetector is using CaseSensitive matching again after each test
+     * @throws Exception
+     */
+    @After
+    public void cleanup() throws Exception {
+        spiderDetectorService = null;
+        configurationService.setProperty("usage-statistics.bots.case-insensitive", false);;
+    }
+
+
+
+
+    /**
+     * Dummy SolrLogger for testing.
+     * @author mwood
+     */
+    static public class MockSolrLogger
+            extends MockUp<SolrLoggerServiceImpl>
+    {
+        @Mock
+        public void $init() {}
+
+        @Mock
+        public void $clinit() {}
+
+        @Mock
+        public boolean isUseProxies()
+        {
+            return false;
+        }
+
+    }
+
+}

dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorTest.java

@@ -1,155 +0,0 @@
-/**
- * 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.statistics.util;
-
-import mockit.Mock;
-import mockit.MockUp;
-import org.dspace.AbstractDSpaceTest;
-import org.dspace.statistics.SolrLoggerServiceImpl;
-import org.junit.Test;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-/**
- * @author mwood
- */
-public class SpiderDetectorTest extends AbstractDSpaceTest
-{
-    private static final String NOT_A_BOT_ADDRESS = "192.168.0.1";
-
-    /**
-     * Test method for {@link org.dspace.statistics.util.SpiderDetector#readPatterns(java.io.File)}.
-     */
-    @Test
-    public void testReadPatterns()
-    {
-// FIXME        fail("Not yet implemented");
-    }
-
-    /**
-     * Test method for {@link org.dspace.statistics.util.SpiderDetector#getSpiderIpAddresses()}.
-     */
-    @Test
-    public void testGetSpiderIpAddresses()
-    {
-// FIXME        fail("Not yet implemented");
-    }
-
-    /**
-     * Test method for {@link org.dspace.statistics.util.SpiderDetector#isSpider(javax.servlet.http.HttpServletRequest)}.
-     */
-    @Test
-    public void testIsSpiderHttpServletRequest()
-    {
-        DummyHttpServletRequest req = new DummyHttpServletRequest();
-        req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises
-        req.setRemoteHost("notabot.example.com"); // avoid surprises
-        req.setAgent("Firefox"); // avoid surprises
-
-        String candidate;
-
-        // Test agent patterns
-        req.setAgent("msnbot is watching you");
-        assertTrue("'msnbot' did not match any pattern", SpiderDetector.isSpider(req));
-
-        req.setAgent("Firefox");
-        assertFalse("'Firefox' matched a pattern", SpiderDetector.isSpider(req));
-
-        // Test IP patterns
-        candidate = "192.168.2.1";
-        req.setAddress(candidate);
-        assertTrue(candidate + " did not match IP patterns", SpiderDetector.isSpider(req));
-
-        req.setAddress(NOT_A_BOT_ADDRESS);
-        assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", SpiderDetector.isSpider(req));
-
-        // Test DNS patterns
-        candidate = "baiduspider-dspace-test.crawl.baidu.com";
-        req.setRemoteHost(candidate);
-        assertTrue(candidate + " did not match DNS patterns", SpiderDetector.isSpider(req));
-
-        candidate = "wiki.dspace.org";
-        req.setRemoteHost(candidate);
-        assertFalse(candidate + " matched DNS patterns", SpiderDetector.isSpider(req));
-    }
-
-    /**
-     * Test method for {@link org.dspace.statistics.util.SpiderDetector#isSpider(java.lang.String, java.lang.String, java.lang.String, java.lang.String)}.
-     */
-    @Test
-    public void testIsSpiderStringStringStringString()
-    {
-        String candidate;
-
-        // Test IP patterns
-        candidate = "192.168.2.1";
-        assertTrue(candidate + " did not match IP patterns",
-                SpiderDetector.isSpider(candidate, null, null, null));
-
-        candidate = NOT_A_BOT_ADDRESS;
-        assertFalse(candidate + " matched IP patterns",
-                SpiderDetector.isSpider(candidate, null, null, null));
-
-        // Test DNS patterns
-        candidate = "baiduspider-dspace-test.crawl.baidu.com";
-        assertTrue(candidate + " did not match DNS patterns",
-                SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null));
-
-        candidate = "wiki.dspace.org";
-        assertFalse(candidate + " matched DNS patterns",
-                SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null));
-
-        // Test agent patterns
-        candidate = "msnbot is watching you";
-        assertTrue("'" + candidate + "' did not match agent patterns",
-                SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate));
-
-        candidate = "Firefox";
-        assertFalse("'" + candidate + "' matched agent patterns",
-                SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate));
-    }
-
-    /**
-     * Test method for {@link org.dspace.statistics.util.SpiderDetector#isSpider(java.lang.String)}.
-     */
-    @Test
-    public void testIsSpiderString()
-    {
-        String candidate;
-
-        candidate = "192.168.2.1";
-        assertTrue(candidate + " did not match IP patterns",
-                SpiderDetector.isSpider(candidate, null, null, null));
-
-        candidate = NOT_A_BOT_ADDRESS;
-        assertFalse(candidate + " matched IP patterns",
-                SpiderDetector.isSpider(candidate, null, null, null));
-
-    }
-
-    /**
-     * Dummy SolrLogger for testing.
-     * @author mwood
-     */
-    static public class MockSolrLogger
-            extends MockUp<SolrLoggerServiceImpl>
-    {
-        @Mock
-        public void $init() {}
-
-        @Mock
-        public void $clinit() {}
-
-        @Mock
-        public boolean isUseProxies()
-        {
-            return false;
-        }
-    }
-}

dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/ItemTag.java

@@ -1027,8 +1027,7 @@ public class ItemTag extends TagSupport
                                                 context,
                                                 b,
                                                 groupService.findByName(context, Group.ANONYMOUS),
-                                                Constants.READ,
+                                                Constants.READ);
-                                                -1);
                                 ResourcePolicy rp = null;
                                 for (ResourcePolicy policy : policies)
                                 {

dspace-jspui/src/main/java/org/dspace/app/webui/servlet/DSpaceServlet.java

@@ -160,6 +160,15 @@ public class DSpaceServlet extends HttpServlet
             }
             abortContext(context);
         }
+        catch (IOException ioe)
+        {
+            /*
+             * If a an IOException occurs (e.g. if the client interrupted a download),
+             * just log a simple warning without stacktrace.
+             */
+            log.warn(LogManager.getHeader(context, "io_error", ioe.toString()));
+            abortContext(context);
+        }
         catch (Exception e)
         {
             log.warn(LogManager.getHeader(context, "general_jspui_error", e

dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/EditItemServlet.java

@@ -923,11 +923,11 @@ public class EditItemServlet extends DSpaceServlet
                     //Retrieve the button key
                     String inputKey = button.replace("submit_order_", "") + "_value";
                     if(inputKey.startsWith(bundle.getID() + "_")){
-                        List<UUID> vals = Util.getUUIDParameters(request, inputKey);
+                        // Field contains comma-separated, ordered list of Bitstream UUIDs
-                        int idx = 0;
+                        String[] vals = request.getParameter(inputKey).split(",");
-                        for (UUID v : vals) {
+                        for (int i = 0; i < vals.length; i++) {
-                            newBitstreamOrder[idx] = v;
+                            String val = vals[i];
-                            idx++;
+                            newBitstreamOrder[i] = UUID.fromString(val);
                         }
                     }else{
                         newBitstreamOrder = null;

dspace-jspui/src/main/java/org/dspace/app/webui/util/JSPManager.java

@@ -57,7 +57,16 @@ public class JSPManager
         }
         try {
             // For the moment, a simple forward
+            // First test if the response is already committed (could happen by broken downloads),
+            // if that is the case, forward won't work.
+            if (!response.isCommitted())
+            {
                 request.getRequestDispatcher(jsp).forward(request, response);
+            }
+            else
+            {
+                log.warn("Couldn't show jsp, response is already commited.");
+            }
         } catch (Exception e) {
              throw new ServletException(e);
         }

dspace-jspui/src/main/webapp/dspace-admin/index.jsp

@@ -77,6 +77,12 @@
         UIUtil.sendAlert(request, se);
 
         JSPManager.showInternalError(request, response);
+    } finally {
+        // we need to close the database connection and free the resources
+        if(context != null && context.isValid())
+        {
+            context.abort();
+        }
     }
 %>
 

dspace-jspui/src/main/webapp/help/formats.jsp

@@ -62,6 +62,12 @@
         UIUtil.sendAlert(request, se);
 
         JSPManager.showInternalError(request, response);
+    } finally {
+        // we need to close the database connection and free the resources
+        if(context != null && context.isValid())
+        {
+            context.abort();
+        }
     }
 %>
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

dspace-jspui/src/main/webapp/index.jsp

@@ -81,5 +81,11 @@
         UIUtil.sendAlert(request, se);
 
         JSPManager.showInternalError(request, response);
+    } finally {
+        // we need to close the database connection and free the resources
+        if(context != null && context.isValid())
+        {
+            context.abort();
+        }
     }
 %>

dspace-jspui/src/main/webapp/login/chooser.jsp

@@ -89,6 +89,12 @@
         // Also email an alert
         UIUtil.sendAlert(request, se);
         JSPManager.showInternalError(request, response);
+    } finally {
+        // we need to close the database connection and free the resources
+        if(context != null && context.isValid())
+        {
+            context.abort();
+        }
     }
 
 %>

dspace-rest/src/main/java/org/dspace/rest/CollectionsResource.java

@@ -40,11 +40,14 @@ import org.dspace.content.service.InstallItemService;
 import org.dspace.content.service.ItemService;
 import org.dspace.content.service.WorkspaceItemService;
 import org.dspace.core.Constants;
+import org.dspace.core.LogManager;
 import org.dspace.rest.common.Collection;
 import org.dspace.rest.common.Item;
 import org.dspace.rest.common.MetadataEntry;
 import org.dspace.rest.exceptions.ContextException;
 import org.dspace.usage.UsageEvent;
+import org.dspace.workflow.WorkflowService;
+import org.dspace.workflow.factory.WorkflowServiceFactory;
 
 /**
  * This class provides all CRUD operation over collections.
@@ -58,7 +61,7 @@ public class CollectionsResource extends Resource
     protected ItemService itemService = ContentServiceFactory.getInstance().getItemService();
     protected AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService();
     protected WorkspaceItemService workspaceItemService = ContentServiceFactory.getInstance().getWorkspaceItemService();
-    protected InstallItemService installItemService = ContentServiceFactory.getInstance().getInstallItemService();
+    protected WorkflowService workflowService = WorkflowServiceFactory.getInstance().getWorkflowService();
 
     private static Logger log = Logger.getLogger(CollectionsResource.class);
 
@@ -365,11 +368,21 @@ public class CollectionsResource extends Resource
                 }
             }
 
-            log.trace("Installing item to collection(id=" + collectionId + ").");
-            dspaceItem = installItemService.installItem(context, workspaceItem);
             workspaceItemService.update(context, workspaceItem);
 
-            returnItem = new Item(dspaceItem, servletContext, "", context);
+            try
+            {
+                // Must insert the item into workflow
+                log.trace("Starting workflow for item(id=" + dspaceItem.getID() + ").");
+                workflowService.start(context, workspaceItem);
+            }
+            catch (Exception e)
+            {
+                log.error(LogManager.getHeader(context, "Error while starting workflow", "Item id: " + dspaceItem.getID()), e);
+                throw new ContextException("Error while starting workflow for item(id=" + dspaceItem.getID() + ")", e);
+            }
+
+            returnItem = new Item(workspaceItem.getItem(), servletContext, "",context);
 
             context.complete();
 

dspace-xmlui-mirage2/src/main/webapp/bower.json

@@ -7,7 +7,7 @@
         "jquery": "~1.10.2",
         "jquery-ui": "1.10.3",
         "jqueryuibootstrap": "4f3772cd37b898f456c0126c4b44178ce8d4aad7",
-        "handlebars": "2.0.0",
+        "handlebars": "4.0.0",
         "respond": "1.4.2",
         "html5shiv": "3.7.2",
         "holderjs": "2.4.1",

dspace-xmlui-mirage2/src/main/webapp/package.json

@@ -10,7 +10,7 @@
     "grunt-contrib-compass": "^1.1.1",
     "grunt-contrib-concat": "~1.0.1",
     "grunt-contrib-copy": "~1.0.0",
-    "grunt-contrib-handlebars": "0.9.3",
+    "grunt-contrib-handlebars": "1.0.0",
     "grunt-contrib-uglify": "~2.0.0",
     "grunt-usemin": "~3.1.1",
     "load-grunt-tasks": "~3.5.2",

dspace-xmlui-mirage2/src/main/webapp/scripts.xml

@@ -13,6 +13,10 @@
     <script src="vendor/jquery/jquery.min.js">&#160;</script>
     <script src="vendor/jquery-ui/ui/jquery.ui.core.js">&#160;</script>
     <script src="vendor/jquery-ui/ui/jquery.ui.datepicker.js">&#160;</script>
+    <script src="vendor/jquery-ui/ui/jquery.ui.widget.js">&#160;</script>
+    <script src="vendor/jquery-ui/ui/jquery.ui.position.js">&#160;</script>
+    <script src="vendor/jquery-ui/ui/jquery.ui.menu.js">&#160;</script>
+    <script src="vendor/jquery-ui/ui/jquery.ui.autocomplete.js">&#160;</script>
     <script src="vendor/handlebars/handlebars.js">&#160;</script>
     <script src="vendor/holderjs/holder.js">&#160;</script>
     <!--<script src="vendor/bootstrap-sass-official/assets/javascripts/bootstrap/affix.js">&#160;</script>-->

dspace-xmlui-mirage2/src/main/webapp/scripts/search-controls.js

@@ -144,7 +144,10 @@
         }
 
         template = getSimpleFiltersTemplate();
-        html = template(DSpace.discovery);
+        html = template({
+            filters: DSpace.discovery.orig_filters,
+            i18n: DSpace.i18n.discovery
+        });
 
         unAssignSimpleFilterEventHandlers();
         $('#filters-overview-wrapper').remove();

dspace-xmlui-mirage2/src/main/webapp/styles/classic_mirage_color_scheme/_jquery_ui.scss

@@ -16,7 +16,7 @@
 }
 
 input[type='text'].ui-autocomplete-loading{
-    background-image: url("../../images/authority_control/lookup-indicator.gif");
+    background-image: url("../images/authority_control/lookup-indicator.gif");
     background-repeat: no-repeat;
     background-position: 99% 50%;
     background-size: 25px 25px;

dspace-xmlui-mirage2/src/main/webapp/templates/discovery_advanced_filters.hbs

@@ -15,7 +15,7 @@
             <select id="aspect_discovery_SimpleSearch_field_filtertype_{{index}}" class="ds-select-field form-control"
                     name="filtertype_{{index}}">
                 {{#set_selected type}}
-                {{#each ../../i18n.filtertype}}
+                {{#each ../i18n.filtertype}}
                 <option value="{{@key}}">{{this}}</option>
                 {{/each}}
                 {{/set_selected}}
@@ -27,7 +27,7 @@
             <select id="aspect_discovery_SimpleSearch_field_filter_relational_operator_{{index}}"
                     class="ds-select-field form-control" name="filter_relational_operator_{{index}}">
                 {{#set_selected relational_operator}}
-                {{#each ../../i18n.filter_relational_operator}}
+                {{#each ../i18n.filter_relational_operator}}
                 <option value="{{@key}}">{{this}}</option>
                 {{/each}}
                 {{/set_selected}}

dspace-xmlui-mirage2/src/main/webapp/templates/discovery_simple_filters.hbs

@@ -7,6 +7,6 @@
     http://www.dspace.org/license/
 
 -->
-{{#each orig_filters}}
+{{#each filters}}
-    <label href="#" class="label label-primary" data-index="{{index}}">{{query}}&nbsp;&times;</label>
+    <label href="#" class="label label-primary" data-index="{{index}}">{{lookup ../i18n.filtertype type}}: {{query}}&nbsp;&times;</label>
 {{/each}}

dspace-xmlui-mirage2/src/main/webapp/xsl/aspect/general/choice-authority-control.xsl

@@ -74,7 +74,9 @@
           <!-- Collection ID for context -->
           <xsl:choose>
             <xsl:when test="/dri:document/dri:meta/dri:pageMeta/dri:metadata[@element='choice'][@qualifier='collection']">
+              <xsl:text>'</xsl:text>
               <xsl:value-of select="/dri:document/dri:meta/dri:pageMeta/dri:metadata[@element='choice'][@qualifier='collection']"/>
+              <xsl:text>'</xsl:text>
             </xsl:when>
             <xsl:otherwise>
               <xsl:text>-1</xsl:text>
@@ -322,9 +324,9 @@
                     <xsl:value-of select="$confidenceIndicatorID"/>
                     <xsl:text>', confidenceName: '</xsl:text>
                     <xsl:value-of select="$confidenceName"/>
-                    <xsl:text>', collection: </xsl:text>
+                    <xsl:text>', collection: '</xsl:text>
                     <xsl:value-of select="$collectionID"/>
-                    <xsl:text>, contextPath: '</xsl:text>
+                    <xsl:text>', contextPath: '</xsl:text>
                     <xsl:value-of select="/dri:document/dri:meta/dri:pageMeta/dri:metadata[@element='contextPath'][not(@qualifier)]"/>
                 <xsl:text>'});</xsl:text>
             <xsl:text>});</xsl:text>

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

@@ -339,8 +339,8 @@ public class FlowContainerUtils
 	 */
 	public static FlowResult processReimportCollection(Context context, UUID collectionID, Request request) throws SQLException, IOException, AuthorizeException, CrosswalkException, ParserConfigurationException, SAXException, TransformerException, BrowseException
 	{
-//		Context.Mode originalMode = context.getCurrentMode();
+		Context.Mode originalMode = context.getCurrentMode();
-//		context.setMode(Context.Mode.BATCH_EDIT);
+		context.setMode(Context.Mode.BATCH_EDIT);
 
 		Collection collection = collectionService.find(context, collectionID);
 		HarvestedCollection hc = harvestedCollectionService.find(context, collection);
@@ -362,7 +362,7 @@ public class FlowContainerUtils
         // update the context?
 		//context.dispatchEvent() // not sure if this is required yet.ts();
 
-//		context.setMode(originalMode);
+		context.setMode(originalMode);
 
 		return processRunCollectionHarvest(context, collectionID, request);
 	}

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/eperson/UnAuthenticateAction.java

@@ -23,6 +23,7 @@ import org.dspace.app.xmlui.utils.ContextUtil;
 import org.dspace.services.factory.DSpaceServicesFactory;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
+import org.dspace.services.ConfigurationService;
 
 /**
  * Unauthenticate the current user. There is no way this action will fail, 
@@ -80,15 +81,17 @@ public class UnAuthenticateAction extends AbstractAction
         context.setCurrentUser(eperson);
         
         // Forward the user to the home page.
-        if((DSpaceServicesFactory.getInstance().getConfigurationService().getBooleanProperty("xmlui.public.logout")) && (httpRequest.isSecure())) {
+        ConfigurationService configurationService
+                = DSpaceServicesFactory.getInstance().getConfigurationService();
+        if((configurationService.getBooleanProperty("xmlui.public.logout"))
+                && (httpRequest.isSecure())) {
 				StringBuffer location = new StringBuffer("http://");
-				location.append(DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.hostname")).append(
+				location.append(configurationService.getProperty("dspace.hostname"))
-						httpRequest.getContextPath());
+                        .append(httpRequest.getContextPath());
 				httpResponse.sendRedirect(location.toString());
-			
 		}
         else{
-        	httpResponse.sendRedirect(httpRequest.getContextPath());
+            httpResponse.sendRedirect(configurationService.getProperty("dspace.url"));
         }
         
         return new HashMap();

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

@@ -359,7 +359,7 @@ public class Submissions extends AbstractDSpaceTransformer
             Row row = table.addRow();
 
             // Add the title column
-        	if (title.length() > 0)
+            if (StringUtils.isNotBlank(title))
             {
                 String displayTitle = title;
     	        if (displayTitle.length() > 50)

dspace-xmlui/src/main/resources/aspects/Versioning/versioning.js

@@ -127,6 +127,7 @@ function doCreateNewVersion(itemID, result){
             result = VersionManager.processCreateNewVersion(getDSContext(),itemID, summary);
 
             var wsid = result.getParameter("wsid");
+            getDSContext().complete();
             cocoon.redirectTo(cocoon.request.getContextPath()+"/submit?workspaceID=" + wsid,true);
 	        cocoon.exit();
         }

dspace/config/dspace.cfg

@@ -96,8 +96,8 @@ db.maxconnections = 30
 db.maxwait = 5000
 
 # Maximum number of idle connections in pool (-1 = unlimited)
-# (default = -1, unlimited)
+# (default = 10)
-db.maxidle = -1
+db.maxidle = 10
 
 # Specify a configured database connection pool to be fetched from a
 # directory.  This overrides the pool and driver settings above.  If

dspace/config/hibernate-ehcache-config.xml

@@ -330,7 +330,7 @@
     <!-- The number of groups in a repository can be very big, but only a small percentage of them is used
          very frequently. So it makes sense to cache Group records because the cache hit rate is likely to be high -->
     <cache name="org.dspace.eperson.Group"
-           maxElementsInMemory="3000" eternal="false" timeToIdleSeconds="1800"
+           maxElementsInMemory="5000" eternal="false" timeToIdleSeconds="1800"
            timeToLiveSeconds="3600" overflowToDisk="false"
            memoryStoreEvictionPolicy="LRU"/>
 

dspace/config/local.cfg.EXAMPLE

@@ -104,8 +104,8 @@ db.schema = public
 #db.maxwait = 5000
 
 # Maximum number of idle connections in pool (-1 = unlimited)
-# (default = -1, unlimited)
+# (default = 10)
-#db.maxidle = -1
+#db.maxidle = 10
 
 
 #######################

dspace/config/modules/usage-statistics.cfg

@@ -35,3 +35,7 @@ usage-statistics.authorization.admin.workflow=true
 # (see query.filter.* for query filter options)
 # Default value is true.
 #usage-statistics.logBots = true
+
+# Enable/disable if a matching for a bot should be case sensitive
+# Setting this value to true will increase cpu usage, but bots will be found more accurately
+#usage-statistics.bots.case-insensitive = false

dspace/config/spring/api/core-services.xml

@@ -96,6 +96,8 @@
     <bean id="elasticSearchLoggerService" class="org.dspace.statistics.ElasticSearchLoggerServiceImpl" lazy-init="true"/>
     <bean id="solrLoggerService" class="org.dspace.statistics.SolrLoggerServiceImpl" lazy-init="true"/>
 
+    <bean id="spiderDetectorService" class="org.dspace.statistics.util.SpiderDetectorServiceImpl"/>
+
     <bean class="org.dspace.versioning.VersionHistoryServiceImpl"/>
 
     <!--Basic workflow services, comment or remove when switching to the configurable workflow -->