Merge pull request #1322 from tomdesair/feature/DS-3004-isAdmin-performance-problems

Feature/DS-3004 isAdmin performance problems

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

@@ -7,13 +7,6 @@
  */
 package org.dspace.app.util;
 
-import java.util.Map;
-import java.util.Set;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Enumeration;
-import java.sql.SQLException;
-
 import org.apache.log4j.Logger;
 import org.dspace.app.util.service.MetadataExposureService;
 import org.dspace.authorize.service.AuthorizeService;
@@ -21,6 +14,9 @@ import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import java.sql.SQLException;
+import java.util.*;
+
 /**
  * Static utility class to manage configuration for exposure (hiding) of
  * certain Item metadata fields.
@@ -78,11 +74,7 @@ public class MetadataExposureServiceImpl implements MetadataExposureService
     public boolean isHidden(Context context, String schema, String element, String qualifier)
         throws SQLException
     {
-        // the administrator's override
+        boolean hidden = false;
-        if (context != null && authorizeService.isAdmin(context))
-        {
-            return false;
-        }
 
         // for schema.element, just check schema->elementSet
         if (!isInitialized())
@@ -93,9 +85,8 @@ public class MetadataExposureServiceImpl implements MetadataExposureService
         if (qualifier == null)
         {
             Set<String> elts = hiddenElementSets.get(schema);
-            return elts == null ? false : elts.contains(element);
+            hidden = elts != null && elts.contains(element);
         }
-
         // for schema.element.qualifier, just schema->eltMap->qualSet
         else
         {
@@ -105,8 +96,15 @@ public class MetadataExposureServiceImpl implements MetadataExposureService
                 return false;
             }
             Set<String> quals = elts.get(element);
-            return quals == null ? false : quals.contains(qualifier);
+            hidden = quals != null && quals.contains(qualifier);
         }
+
+        if(hidden && context != null) {
+            // the administrator's override
+            hidden = !authorizeService.isAdmin(context);
+        }
+
+        return hidden;
     }
 
     /**

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

@@ -7,19 +7,12 @@
  */
 package org.dspace.authorize;
 
-import java.sql.SQLException;
-import java.util.*;
-
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.authorize.service.ResourcePolicyService;
-import org.dspace.content.Bitstream;
+import org.dspace.content.*;
-import org.dspace.content.Bundle;
 import org.dspace.content.Collection;
-import org.dspace.content.Community;
-import org.dspace.content.DSpaceObject;
-import org.dspace.content.Item;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.BitstreamService;
 import org.dspace.content.service.WorkspaceItemService;
@@ -31,6 +24,9 @@ import org.dspace.eperson.service.GroupService;
 import org.dspace.workflow.WorkflowItemService;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import java.sql.SQLException;
+import java.util.*;
+
 /**
  * AuthorizeManager handles all authorization checks for DSpace. For better
  * security, DSpace assumes that you do not have the right to do something
@@ -419,7 +415,7 @@ public class AuthorizeServiceImpl implements AuthorizeService
             return false; // anonymous users can't be admins....
         } else
         {
-            return groupService.isMember(c, groupService.findByName(c, Group.ADMIN));
+            return groupService.isMember(c, Group.ADMIN);
         }
     }
 

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

@@ -318,7 +318,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
             Group g = groupService.create(context);
             context.restoreAuthSystemState();
 
-            groupService.setName(context, g,
+            groupService.setName(g,
                     "COLLECTION_" + collection.getID() + "_WORKFLOW_STEP_" + step);
             groupService.update(context, g);
             setWorkflowGroup(collection, step, g);
@@ -396,7 +396,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
             submitters = groupService.create(context);
             context.restoreAuthSystemState();
 
-            groupService.setName(context, submitters,
+            groupService.setName(submitters,
                     "COLLECTION_" + collection.getID() + "_SUBMIT");
             groupService.update(context, submitters);
         }
@@ -437,7 +437,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
             admins = groupService.create(context);
             context.restoreAuthSystemState();
 
-            groupService.setName(context, admins, "COLLECTION_" + collection.getID() + "_ADMIN");
+            groupService.setName(admins, "COLLECTION_" + collection.getID() + "_ADMIN");
             groupService.update(context, admins);
         }
 
@@ -689,6 +689,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
         Group g = collection.getWorkflowStep1();
         if (g != null)
         {
+            collection.setWorkflowStep1(null);
             groupService.delete(context, g);
         }
 
@@ -696,6 +697,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
 
         if (g != null)
         {
+            collection.setWorkflowStep2(null);
             groupService.delete(context, g);
         }
 
@@ -703,6 +705,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
 
         if (g != null)
         {
+            collection.setWorkflowStep3(null);
             groupService.delete(context, g);
         }
 
@@ -711,6 +714,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
 
         if (g != null)
         {
+            collection.setAdmins(null);
             groupService.delete(context, g);
         }
 
@@ -719,6 +723,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl<Collection> i
 
         if (g != null)
         {
+            collection.setSubmitters(null);
             groupService.delete(context, g);
         }
 

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

@@ -270,7 +270,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
             admins = groupService.create(context);
             context.restoreAuthSystemState();
 
-            groupService.setName(context, admins, "COMMUNITY_" + community.getID() + "_ADMIN");
+            groupService.setName(admins, "COMMUNITY_" + community.getID() + "_ADMIN");
             groupService.update(context, admins);
         }
 

dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataFieldDAOImpl.java

@@ -40,11 +40,13 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO<MetadataField> im
         criteria.add(
                 Restrictions.and(
                         Restrictions.not(Restrictions.eq("id", metadataFieldId)),
-                        Restrictions.eq("metadataSchema", metadataSchema),
+                        Restrictions.eq("metadataSchema.id", metadataSchema.getSchemaID()),
                         Restrictions.eq("element", element),
                         Restrictions.eqOrIsNull("qualifier", qualifier)
                 )
         );
+        criteria.setCacheable(true);
+
         return singleResult(criteria);
     }
 
@@ -54,11 +56,13 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO<MetadataField> im
         Criteria criteria = createCriteria(context, MetadataField.class);
         criteria.add(
                 Restrictions.and(
-                        Restrictions.eq("metadataSchema", metadataSchema),
+                        Restrictions.eq("metadataSchema.id", metadataSchema.getSchemaID()),
                         Restrictions.eq("element", element),
                         Restrictions.eqOrIsNull("qualifier", qualifier)
                 )
         );
+        criteria.setCacheable(true);
+
         return singleResult(criteria);
     }
 
@@ -66,6 +70,7 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO<MetadataField> im
     public List<MetadataField> findAll(Context context, Class<MetadataField> clazz) throws SQLException {
         Criteria criteria = createCriteria(context, MetadataField.class);
         criteria.createAlias("metadataSchema", "s").addOrder(Order.asc("s.name")).addOrder(Order.asc("element")).addOrder(Order.asc("qualifier"));
+        criteria.setCacheable(true);
         return list(criteria);
     }
 
@@ -80,6 +85,8 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO<MetadataField> im
                         Restrictions.eqOrIsNull("qualifier", qualifier)
                 )
         );
+        criteria.setCacheable(true);
+
         return singleResult(criteria);
     }
 
@@ -93,6 +100,8 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO<MetadataField> im
                         Restrictions.eq("element", element)
                 )
         );
+        criteria.setCacheable(true);
+
         return list(criteria);
     }
 
@@ -102,8 +111,10 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO<MetadataField> im
         // Get all the metadatafieldregistry rows
         Criteria criteria = createCriteria(context, MetadataField.class);
         criteria.createAlias("metadataSchema", "s");
-        criteria.add(Restrictions.eq("metadataSchema", metadataSchema));
+        criteria.add(Restrictions.eq("s.id", metadataSchema.getSchemaID()));
         criteria.addOrder(Order.asc("s.name")).addOrder(Order.asc("element")).addOrder(Order.asc("qualifier"));
+
+        criteria.setCacheable(true);
         return list(criteria);
     }
 }

dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataSchemaDAOImpl.java

@@ -46,6 +46,8 @@ public class MetadataSchemaDAOImpl extends AbstractHibernateDAO<MetadataSchema>
         // Grab rows from DB
         Criteria criteria = createCriteria(context, MetadataSchema.class);
         criteria.add(Restrictions.eq("namespace", namespace));
+        criteria.setCacheable(true);
+
         return uniqueResult(criteria);
     }
 
@@ -54,6 +56,8 @@ public class MetadataSchemaDAOImpl extends AbstractHibernateDAO<MetadataSchema>
         // Get all the metadataschema rows
         Criteria criteria = createCriteria(context, MetadataSchema.class);
         criteria.addOrder(Order.asc("id"));
+        criteria.setCacheable(true);
+
         return list(criteria);
     }
 
@@ -74,6 +78,8 @@ public class MetadataSchemaDAOImpl extends AbstractHibernateDAO<MetadataSchema>
                 Restrictions.not(Restrictions.eq("id", metadataSchemaId)),
                 Restrictions.eq("namespace", namespace)
         ));
+        criteria.setCacheable(true);
+
         return uniqueResult(criteria) == null;
     }
 
@@ -93,6 +99,7 @@ public class MetadataSchemaDAOImpl extends AbstractHibernateDAO<MetadataSchema>
                 Restrictions.not(Restrictions.eq("id", metadataSchemaId)),
                 Restrictions.eq("name", name)
         ));
+        criteria.setCacheable(true);
 
         return uniqueResult(criteria) == null;
     }
@@ -114,6 +121,7 @@ public class MetadataSchemaDAOImpl extends AbstractHibernateDAO<MetadataSchema>
         criteria.add(
                 Restrictions.eq("name", shortName)
         );
+        criteria.setCacheable(true);
 
         return uniqueResult(criteria);
     }

dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java

@@ -7,22 +7,7 @@
  */
 package org.dspace.content.packager;
 
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.io.PipedInputStream;
-import java.io.PipedOutputStream;
-import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.List;
-
-import javax.xml.stream.XMLOutputFactory;
-import javax.xml.stream.XMLStreamException;
-import javax.xml.stream.XMLStreamWriter;
 import org.apache.log4j.Logger;
-
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
 import org.dspace.content.Community;
@@ -33,12 +18,19 @@ import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 import org.dspace.eperson.PasswordHash;
-
 import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.eperson.service.GroupService;
 import org.jdom.Namespace;
 
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamWriter;
+import java.io.*;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Plugin to export all Group and EPerson objects in XML, perhaps for reloading.
  * 
@@ -549,7 +541,7 @@ public class RoleDisseminator implements PackageDisseminator
         {
             // @TODO FIXME -- if there was a way to ONLY export Groups which are NOT
             // associated with a Community or Collection, we should be doing that instead!
-            return groupService.findAll(context, GroupService.NAME);
+            return groupService.findAll(context, null);
         }
         else if(object.getType()==Constants.COMMUNITY)
         {

dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java

@@ -7,19 +7,7 @@
  */
 package org.dspace.content.packager;
 
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
 import org.apache.commons.codec.DecoderException;
-
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
 import org.dspace.content.Community;
@@ -42,6 +30,17 @@ import org.slf4j.LoggerFactory;
 import org.w3c.dom.*;
 import org.xml.sax.SAXException;
 
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
 /**
  * Create EPersons and Groups from a file of external representations.
  * 
@@ -356,7 +355,7 @@ public class RoleIngester implements PackageIngester
                 }
 
                 // Always set the name:  parent.createBlop() is guessing
-                groupService.setName(context, groupObj, name);
+                groupService.setName(groupObj, name);
 
                 log.info("Created Group {}.", groupObj.getName());
             }

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

@@ -7,6 +7,7 @@
  */
 package org.dspace.eperson;
 
+import org.apache.commons.lang3.StringUtils;
 import org.dspace.content.DSpaceObject;
 import org.dspace.content.DSpaceObjectLegacySupport;
 import org.dspace.content.MetadataSchema;
@@ -48,6 +49,9 @@ public class Group extends DSpaceObject implements DSpaceObjectLegacySupport
     @Column
     private Boolean permanent = false;
 
+    @Column(length = 250, unique = true)
+    private String name;
+
     /** lists of epeople and groups in the group */
     @ManyToMany(fetch = FetchType.LAZY)
     @JoinTable(
@@ -74,9 +78,6 @@ public class Group extends DSpaceObject implements DSpaceObjectLegacySupport
     @Transient
     private boolean groupsChanged;
 
-    @Transient
-    private transient GroupService groupService;
-
     /**
      * Protected constructor, create object using:
      * {@link org.dspace.eperson.service.GroupService#create(Context)}
@@ -197,14 +198,16 @@ public class Group extends DSpaceObject implements DSpaceObjectLegacySupport
     @Override
     public String getName()
     {
-        return getGroupService().getName(this);
+        return name;
     }
 
     /** Change the name of this Group. */
-    void setName(Context context, String name) throws SQLException
+    void setName(String name) throws SQLException
     {
-        getGroupService().setMetadataSingleValue(context, this,
+        if(!StringUtils.equals(this.name, name) && !isPermanent()) {
-                MetadataSchema.DC_SCHEMA, "title", null, null, name);
+            this.name = name;
+            groupsChanged = true;
+        }
     }
 
     public boolean isGroupsChanged() {
@@ -224,14 +227,6 @@ public class Group extends DSpaceObject implements DSpaceObjectLegacySupport
         return supervisedItems;
     }
 
-    private GroupService getGroupService() {
-        if(groupService == null)
-        {
-            groupService = EPersonServiceFactory.getInstance().getGroupService();
-        }
-        return groupService;
-    }
-
     /**
      * May this Group be renamed or deleted?  (The content of any group may be
      * changed.)

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

@@ -7,14 +7,15 @@
  */
 package org.dspace.eperson;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.Pair;
 import org.dspace.authorize.AuthorizeConfiguration;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.content.DSpaceObject;
 import org.dspace.content.DSpaceObjectServiceImpl;
 import org.dspace.content.MetadataField;
-import org.dspace.content.MetadataSchema;
 import org.dspace.content.service.CollectionService;
 import org.dspace.content.service.CommunityService;
 import org.dspace.core.Constants;
@@ -26,14 +27,14 @@ import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.eperson.service.GroupService;
 import org.dspace.event.Event;
+import org.dspace.util.UUIDUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.sql.SQLException;
 import java.util.*;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 /**
  * Service implementation for the Group object.
  * This class is responsible for all business logic calls for the Group object and is autowired by spring.
@@ -48,9 +49,6 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
     @Autowired(required = true)
     protected GroupDAO groupDAO;
 
-//    @Autowired(required = true)
-//    protected Group2GroupDAO group2GroupDAO;
-
     @Autowired(required = true)
     protected Group2GroupCacheDAO group2GroupCacheDAO;
 
@@ -93,7 +91,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
     }
 
     @Override
-    public void setName(Context context, Group group, String name) throws SQLException {
+    public void setName(Group group, String name) throws SQLException {
         if (group.isPermanent())
         {
             log.error("Attempt to rename permanent Group {} to {}.",
@@ -101,7 +99,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
             throw new SQLException("Attempt to rename a permanent Group");
         }
         else
-            group.setName(context, name);
+            group.setName(name);
     }
 
     @Override
@@ -160,15 +158,38 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
 
     @Override
     public boolean isMember(Context context, Group group) throws SQLException {
-        // special, everyone is member of group 0 (anonymous)
+        return isMember(context, group.getName());
-        if (group.getName().equals(Group.ANONYMOUS))
-        {
-            return true;
     }
 
-        EPerson currentuser = context.getCurrentUser();
+    @Override
+    public boolean isMember(final Context context, final String groupName) throws SQLException {
+        // special, everyone is member of group 0 (anonymous)
+        if (StringUtils.equals(groupName, Group.ANONYMOUS))
+        {
+            return true;
+        } else if(context.getCurrentUser() != null) {
+            EPerson currentUser = context.getCurrentUser();
 
-        return epersonInGroup(context, group, currentuser);
+            //First check the special groups
+            boolean found = false;
+            List<Group> specialGroups = context.getSpecialGroups();
+            if(CollectionUtils.isNotEmpty(specialGroups)) {
+                Iterator<Group> it = specialGroups.iterator();
+                while (it.hasNext() && !found) {
+                    Group next = it.next();
+                    found = StringUtils.equals(next.getName(), groupName);
+                }
+            }
+
+            if(found) {
+                return true;
+            } else {
+                //lookup eperson in normal groups and subgroups
+                return epersonInGroup(context, groupName, currentUser);
+            }
+        } else {
+            return false;
+        }
     }
 
     @Override
@@ -247,61 +268,74 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
             return null;
         }
 
-        return groupDAO.findByMetadataField(context, name, metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null));
+        return groupDAO.findByName(context, name);
+    }
+
+    /** DEPRECATED: Please use findAll(Context context, List<MetadataField> metadataSortFields) instead */
+    @Override
+    @Deprecated
+    public List<Group> findAll(Context context, int sortField) throws SQLException {
+        if(sortField == GroupService.NAME) {
+            return findAll(context, null);
+        } else {
+            throw new UnsupportedOperationException("You can only find all groups sorted by name with this method");
+        }
     }
 
     @Override
-    public List<Group> findAll(Context context, int sortField) throws SQLException
+    public List<Group> findAll(Context context, List<MetadataField> metadataSortFields) throws SQLException
     {
-        List<MetadataField> metadataFieldsSort = new ArrayList<>();
+        if(CollectionUtils.isEmpty(metadataSortFields)) {
-        switch (sortField)
+            return groupDAO.findAll(context);
-        {
+        } else {
-            case NAME:
+            return groupDAO.findAll(context, metadataSortFields);
-                metadataFieldsSort.add(metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null));
-
-                break;
-
-            default:
-                metadataFieldsSort.add(metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null));
         }
-
-        return groupDAO.findAll(context, metadataFieldsSort, null);
     }
 
     @Override
-    public List<Group> search(Context context, String query) throws SQLException {
+    public List<Group> search(Context context, String groupIdentifier) throws SQLException {
-        return search(context, query, -1, -1);
+        return search(context, groupIdentifier, -1, -1);
     }
 
     @Override
-    public List<Group> search(Context context, String query, int offset, int limit) throws SQLException
+    public List<Group> search(Context context, String groupIdentifier, int offset, int limit) throws SQLException
     {
-        try {
         List<Group> groups = new ArrayList<>();
-            Group group = find(context, UUID.fromString(query));
+        UUID uuid = UUIDUtils.fromString(groupIdentifier);
+        if(uuid == null) {
+            //Search by group name
+            groups = groupDAO.findByNameLike(context, groupIdentifier, offset, limit);
+        } else {
+            //Search by group id
+            Group group = find(context, uuid);
             if(group != null)
             {
                 groups.add(group);
             }
-            return groups;
-        } catch(IllegalArgumentException e) {
-            MetadataField nameField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
-            if(StringUtils.isBlank(query)) query = null;
-            return groupDAO.search(context, query, Arrays.asList(nameField), offset, limit);
         }
+
+        return groups;
     }
 
     @Override
-    public int searchResultCount(Context context, String query) throws SQLException {
+    public int searchResultCount(Context context, String groupIdentifier) throws SQLException {
-        try {
+        int result = 0;
-            return find(context, UUID.fromString(query)) != null ? 1 : 0;
+        UUID uuid = UUIDUtils.fromString(groupIdentifier);
-        } catch(IllegalArgumentException e) {
+        if(uuid == null && StringUtils.isNotBlank(groupIdentifier)) {
-            MetadataField nameField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null);
+            //Search by group name
-            if (StringUtils.isBlank(query)) query = null;
+            result = groupDAO.countByNameLike(context, groupIdentifier);
-            return groupDAO.searchResultCount(context, query, Arrays.asList(nameField));
+        } else {
+            //Search by group id
+            Group group = find(context, uuid);
+            if(group != null)
+            {
+                result = 1;
             }
         }
 
+        return result;
+    }
+
     @Override
     public void delete(Context context, Group group) throws SQLException {
         if (group.isPermanent())
@@ -381,7 +415,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
         if(anonymousGroup==null)
         {
             anonymousGroup = groupService.create(context);
-            anonymousGroup.setName(context, Group.ANONYMOUS);
+            anonymousGroup.setName(Group.ANONYMOUS);
             anonymousGroup.setPermanent(true);
             groupService.update(context, anonymousGroup);
         }
@@ -392,7 +426,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
         if(adminGroup == null)
         {
             adminGroup = groupService.create(context);
-            adminGroup.setName(context, Group.ADMIN);
+            adminGroup.setName(Group.ADMIN);
             adminGroup.setPermanent(true);
             groupService.update(context, adminGroup);
         }
@@ -433,12 +467,10 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
 
 
 
-    protected boolean epersonInGroup(Context c, Group group, EPerson e)
+    protected boolean epersonInGroup(Context context, String groupName, EPerson ePerson)
             throws SQLException
     {
-        List<Group> groups = allMemberGroups(c, e);
+        return groupDAO.findByNameAndEPerson(context, groupName, ePerson) != null;
-
-        return groups.contains(group);
     }
 
 
@@ -451,11 +483,10 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
 
         Map<UUID, Set<UUID>> parents = new HashMap<>();
 
-        List<Object[]> group2groupResults = groupDAO.getGroup2GroupResults(context, flushQueries);
+        List<Pair<UUID, UUID>> group2groupResults = groupDAO.getGroup2GroupResults(context, flushQueries);
-        for (Object[] group2groupResult : group2groupResults) {
+        for (Pair<UUID, UUID> group2groupResult : group2groupResults) {
-            //We cannot use UUID.nameUUIDFromBytes(), because it only generated "type 3 UUIDs"
+            UUID parent = group2groupResult.getLeft();
-            UUID parent = (UUID) group2groupResult[0];
+            UUID child = group2groupResult.getRight();
-            UUID child = (UUID) group2groupResult[1];
 
             // if parent doesn't have an entry, create one
             if (!parents.containsKey(parent)) {
@@ -627,7 +658,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
         }
         else
         {
-            return find(context, UUID.fromString(id));
+            return find(context, UUIDUtils.fromString(id));
         }
     }
 
@@ -640,4 +671,9 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
     public int countTotal(Context context) throws SQLException {
         return groupDAO.countRows(context);
     }
+
+    @Override
+    public List<Group> findByMetadataField(final Context context, final String searchValue, final MetadataField metadataField) throws SQLException {
+        return groupDAO.findByMetadataField(context, searchValue, metadataField);
+    }
 }

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

@@ -7,6 +7,7 @@
  */
 package org.dspace.eperson.dao;
 
+import org.apache.commons.lang3.tuple.Pair;
 import org.dspace.content.MetadataField;
 import org.dspace.content.dao.DSpaceObjectDAO;
 import org.dspace.content.dao.DSpaceObjectLegacySupportDAO;
@@ -16,6 +17,7 @@ import org.dspace.eperson.Group;
 
 import java.sql.SQLException;
 import java.util.List;
+import java.util.UUID;
 
 /**
  * Database Access Object interface class for the Group object.
@@ -26,19 +28,104 @@ import java.util.List;
  */
 public interface GroupDAO extends DSpaceObjectDAO<Group>, DSpaceObjectLegacySupportDAO<Group> {
 
-    public Group findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException;
+    /**
+     * Look up groups based on their value for a certain metadata field (NOTE: name is not stored as metadata)
+     * @param context The DSpace context
+     * @param searchValue The value to match
+     * @param metadataField The metadata field to search in
+     * @return The groups that have a matching value for specified metadata field
+     * @throws SQLException
+     */
+    List<Group> findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException;
 
-    public List<Group> search(Context context, String query, List<MetadataField> queryFields, int offset, int limit) throws SQLException;
+    /**
+     * Find all groups ordered by the specified metadata fields ascending
+     * @param context The DSpace context
+     * @param sortMetadataFields The metadata fields to sort on
+     * @return A list of all groups, ordered by metadata fields
+     * @throws SQLException
+     */
+    List<Group> findAll(Context context, List<MetadataField> sortMetadataFields) throws SQLException;
 
-    public int searchResultCount(Context context, String query, List<MetadataField> queryFields) throws SQLException;
+    /**
+     * Find all groups ordered by name ascending
+     * @param context The DSpace context
+     * @return A list of all groups, ordered by name
+     * @throws SQLException
+     */
+    List<Group> findAll(Context context) throws SQLException;
 
-    public List<Group> findAll(Context context, List<MetadataField> metadataFields, String sortColumn) throws SQLException;
+    /**
+     * Find all groups that the given ePerson belongs to
+     * @param context The DSpace context
+     * @param ePerson The EPerson to match
+     * @return A list of all groups to which the given EPerson belongs
+     * @throws SQLException
+     */
+    List<Group> findByEPerson(Context context, EPerson ePerson) throws SQLException;
 
-    public List<Group> findByEPerson(Context context, EPerson ePerson) throws SQLException;
+    /**
-
+     * Get a list of all direct parent - child group relations in the database
-    public List getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException;
+     * @param context The DSpace context
+     * @param flushQueries Flush all pending queries
+     * @return A list of pairs indicating parent - child
+     * @throws SQLException
+     */
+    List<Pair<UUID, UUID>> getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException;
 
+    /**
+     * Return all empty groups
+     * @param context The DSpace context
+     * @return All empty groups
+     * @throws SQLException
+     */
     List<Group> getEmptyGroups(Context context) throws SQLException;
 
+    /**
+     * Count the number of groups in DSpace
+     * @param context The DSpace context
+     * @return The number of groups
+     * @throws SQLException
+     */
     int countRows(Context context) throws SQLException;
+
+    /**
+     * Find a group by its name (exact match)
+     * @param context The DSpace context
+     * @param name The name of the group to look for
+     * @return The group with the specified name
+     * @throws SQLException
+     */
+    Group findByName(Context context, String name) throws SQLException;
+
+    /**
+     * Find a group by its name (fuzzy match)
+     * @param context The DSpace context
+     * @param groupName Part of the group's name to search for
+     * @param offset Offset to use for pagination (-1 to disable)
+     * @param limit The maximum number of results to return (-1 to disable)
+     * @return Groups matching the query
+     * @throws SQLException
+     */
+    List<Group> findByNameLike(Context context, String groupName, int offset, int limit) throws SQLException;
+
+    /**
+     * Count the number of groups that have a name that contains the given string
+     * @param context The DSpace context
+     * @param groupName Part of the group's name to search for
+     * @return The number of matching groups
+     * @throws SQLException
+     */
+    int countByNameLike(Context context, String groupName) throws SQLException;
+
+    /**
+     * Find a group by its name and the membership of the given EPerson
+     * @param context The DSpace context
+     * @param groupName The name of the group to look for
+     * @param ePerson The EPerson which has to be a member
+     * @return The group with the specified name
+     * @throws SQLException
+     */
+    Group findByNameAndEPerson(Context context, String groupName, EPerson ePerson) throws SQLException;
+
 }

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

@@ -37,7 +37,9 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO<Group2GroupCac
     @Override
     public List<Group2GroupCache> findByParent(Context context, Group group) throws SQLException {
         Criteria criteria = createCriteria(context, Group2GroupCache.class);
-        criteria.add(Restrictions.eq("parent", group));
+        criteria.add(Restrictions.eq("parent.id", group.getID()));
+        criteria.setCacheable(true);
+
         return list(criteria);
     }
 
@@ -48,18 +50,21 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO<Group2GroupCac
         Disjunction orDisjunction = Restrictions.or();
         for(Group group : groups)
         {
-            orDisjunction.add(Restrictions.eq("child", group));
+            orDisjunction.add(Restrictions.eq("child.id", group.getID()));
         }
 
         criteria.add(orDisjunction);
+        criteria.setCacheable(true);
+
         return list(criteria);
     }
 
     @Override
     public Group2GroupCache find(Context context, Group parent, Group child) throws SQLException {
         Criteria criteria = createCriteria(context, Group2GroupCache.class);
-        criteria.add(Restrictions.eq("parent", parent));
+        criteria.add(Restrictions.eq("parent.id", parent.getID()));
-        criteria.add(Restrictions.eq("child", child));
+        criteria.add(Restrictions.eq("child.id", child.getID()));
+        criteria.setCacheable(true);
         return uniqueResult(criteria);
     }
 

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

@@ -7,22 +7,20 @@
  */
 package org.dspace.eperson.dao.impl;
 
-import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.StringUtils;
-import org.apache.commons.collections.ListUtils;
+import org.apache.commons.lang3.tuple.Pair;
-import org.apache.commons.lang3.StringUtils;
 import org.dspace.content.MetadataField;
-import org.dspace.core.Context;
 import org.dspace.core.AbstractHibernateDSODAO;
+import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 import org.dspace.eperson.dao.GroupDAO;
-import org.hibernate.FlushMode;
 import org.hibernate.Query;
-import org.hibernate.SQLQuery;
-import org.hibernate.type.StandardBasicTypes;
 
 import java.sql.SQLException;
-import java.util.*;
+import java.util.Collections;
+import java.util.List;
+import java.util.UUID;
 
 /**
  * Hibernate implementation of the Database Access Object interface class for the Group object.
@@ -39,7 +37,7 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO<Group> implements Grou
     }
 
     @Override
-    public Group findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException
+    public List<Group> findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException
     {
         StringBuilder queryBuilder = new StringBuilder();
         String groupTableName = "g";
@@ -52,89 +50,108 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO<Group> implements Grou
         query.setParameter(metadataField.toString(), metadataField.getFieldID());
         query.setParameter("queryParam", searchValue);
 
-        return uniqueResult(query);
+        return list(query);
     }
 
     @Override
-    public List<Group> findAll(Context context, List<MetadataField> sortFields, String sortColumn) throws SQLException
+    public List<Group> findAll(Context context, List<MetadataField> sortMetadataFields) throws SQLException
     {
         StringBuilder queryBuilder = new StringBuilder();
         String groupTableName = "g";
         queryBuilder.append("SELECT ").append(groupTableName).append(" FROM Group as ").append(groupTableName);
 
-        addMetadataLeftJoin(queryBuilder, groupTableName, sortFields);
+        addMetadataLeftJoin(queryBuilder, groupTableName, sortMetadataFields);
-        addMetadataSortQuery(queryBuilder, sortFields, Collections.singletonList(sortColumn));
+        addMetadataSortQuery(queryBuilder, sortMetadataFields, null);
 
         Query query = createQuery(context, queryBuilder.toString());
-        for (MetadataField sortField : sortFields) {
+        for (MetadataField sortField : sortMetadataFields) {
             query.setParameter(sortField.toString(), sortField.getFieldID());
         }
         return list(query);
     }
 
+    @Override
+    public List<Group> findAll(Context context) throws SQLException {
+        Query query = createQuery(context,
+                "SELECT g FROM Group g ORDER BY g.name ASC");
+        query.setCacheable(true);
+
+        return list(query);
+    }
+
     @Override
     public List<Group> findByEPerson(Context context, EPerson ePerson) throws SQLException {
         Query query = createQuery(context, "from Group where (from EPerson e where e.id = :eperson_id) in elements(epeople)");
         query.setParameter("eperson_id", ePerson.getID());
+        query.setCacheable(true);
+
         return list(query);
     }
 
     @Override
-    public List<Group> search(Context context, String query, List<MetadataField> queryFields, int offset, int limit) throws SQLException {
+    public Group findByName(final Context context, final String name) throws SQLException {
-        String groupTableName = "g";
+        Query query = createQuery(context,
-        String queryString = "SELECT " + groupTableName + " FROM Group as " + groupTableName;
+                "SELECT g from Group g " +
-        Query hibernateQuery = getSearchQuery(context, queryString, query, queryFields, ListUtils.EMPTY_LIST, null);
+                "where g.name = :name ");
 
-        if(0 <= offset)
+        query.setParameter("name", name);
-        {
+        query.setCacheable(true);
-            hibernateQuery.setFirstResult(offset);
+
-        }
+        return uniqueResult(query);
-        if(0 <= limit)
-        {
-            hibernateQuery.setMaxResults(limit);
-        }
-        return list(hibernateQuery);
     }
 
     @Override
-    public int searchResultCount(Context context, String query, List<MetadataField> queryFields) throws SQLException {
+    public Group findByNameAndEPerson(Context context, String groupName, EPerson ePerson) throws SQLException {
-        String groupTableName = "g";
+        if(groupName == null || ePerson == null) {
-        String queryString = "SELECT count(*) FROM Group as " + groupTableName;
+            return null;
-        Query hibernateQuery = getSearchQuery(context, queryString, query, queryFields, ListUtils.EMPTY_LIST, null);
+        } else {
+            Query query = createQuery(context,
+                    "SELECT DISTINCT g FROM Group g " +
+                            "LEFT JOIN g.epeople p " +
+                            "WHERE g.name = :name AND " +
+                            "(p.id = :eperson_id OR " +
+                            "EXISTS ( " +
+                                "SELECT 1 FROM Group2GroupCache gc " +
+                                "JOIN gc.parent p " +
+                                "JOIN gc.child c " +
+                                "JOIN c.epeople cp " +
+                                "WHERE p.id = g.id AND cp.id = :eperson_id " +
+                                ") " +
+                            ")");
 
-        return count(hibernateQuery);
+            query.setParameter("name", groupName);
+            query.setParameter("eperson_id", ePerson.getID());
+            query.setCacheable(true);
+
+            return uniqueResult(query);
+        }
     }
 
-    protected Query getSearchQuery(Context context, String queryString, String queryParam, List<MetadataField> queryFields, List<MetadataField> sortFields, String sortField) throws SQLException {
+    @Override
+    public List<Group> findByNameLike(final Context context, final String groupName, final int offset, final int limit) throws SQLException {
+        Query query = createQuery(context,
+                "SELECT g FROM Group g WHERE lower(g.name) LIKE lower(:name)");
+        query.setParameter("name", "%" + StringUtils.trimToEmpty(groupName) + "%");
 
-        StringBuilder queryBuilder = new StringBuilder();
+        if(0 <= offset)
-        queryBuilder.append(queryString);
-        Set<MetadataField> metadataFieldsToJoin = new LinkedHashSet<>();
-        metadataFieldsToJoin.addAll(queryFields);
-        if(CollectionUtils.isNotEmpty(sortFields))
         {
-            metadataFieldsToJoin.addAll(sortFields);
+            query.setFirstResult(offset);
+        }
+        if(0 <= limit)
+        {
+            query.setMaxResults(limit);
         }
 
-        if(!CollectionUtils.isEmpty(metadataFieldsToJoin)) {
+        return list(query);
-            addMetadataLeftJoin(queryBuilder, "g", metadataFieldsToJoin);
-        }
-        if(queryParam != null) {
-            addMetadataValueWhereQuery(queryBuilder, queryFields, "like", null);
-        }
-        if(!CollectionUtils.isEmpty(sortFields)) {
-            addMetadataSortQuery(queryBuilder, sortFields, Collections.singletonList(sortField));
     }
 
-        Query query = createQuery(context, queryBuilder.toString());
+    @Override
-        if(StringUtils.isNotBlank(queryParam)) {
+    public int countByNameLike(final Context context, final String groupName) throws SQLException {
-            query.setParameter("queryParam", "%"+queryParam+"%");
+        Query query = createQuery(context,
-        }
+                "SELECT count(*) FROM Group g WHERE lower(g.name) LIKE lower(:name)");
-        for (MetadataField metadataField : metadataFieldsToJoin) {
+        query.setParameter("name", "%" + groupName + "%");
-            query.setParameter(metadataField.toString(), metadataField.getFieldID());
-        }
 
-        return query;
+        return count(query);
     }
 
     @Override
@@ -147,15 +164,15 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO<Group> implements Grou
 
 
     @Override
-    public List getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException {
+    public List<Pair<UUID, UUID>> getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException {
-        SQLQuery sqlQuery = getHibernateSession(context).createSQLQuery("SELECT parent_id, child_id FROM Group2Group");
+
-        sqlQuery.addScalar("parent_id", StandardBasicTypes.UUID_BINARY);
+        Query query = createQuery(context, "SELECT new org.apache.commons.lang3.tuple.ImmutablePair(g.id, c.id) " +
-        sqlQuery.addScalar("child_id", StandardBasicTypes.UUID_BINARY);
+                "FROM Group g " +
-        if(flushQueries){
+                "JOIN g.groups c ");
-            //Optional, flush queries when executing, could be usefull since you are using native queries & these sometimes require this option.
+
-            sqlQuery.setFlushMode(FlushMode.ALWAYS);
+        @SuppressWarnings("unchecked")
-        }
+        List<Pair<UUID, UUID>> results = query.list();
-        return sqlQuery.list();
+        return results;
     }
 
     @Override
@@ -167,4 +184,5 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO<Group> implements Grou
     public int countRows(Context context) throws SQLException {
         return count(createQuery(context, "SELECT count(*) FROM Group"));
     }
+
 }

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

@@ -11,6 +11,7 @@ import java.sql.SQLException;
 import java.util.List;
 
 import org.dspace.authorize.AuthorizeException;
+import org.dspace.content.MetadataField;
 import org.dspace.content.service.DSpaceObjectLegacySupportService;
 import org.dspace.content.service.DSpaceObjectService;
 import org.dspace.core.Context;
@@ -42,7 +43,7 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      * @param name
      *            new group name
      */
-    public void setName(Context context, Group group, String name) throws SQLException;
+    public void setName(Group group, String name) throws SQLException;
 
     /**
      * add an eperson member
@@ -105,6 +106,18 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      */
     public boolean isMember(Context context, Group group) 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
+     *            context
+     * @param groupName
+     *            the name of the group to check
+     */
+    public boolean isMember(Context context, String groupName) throws SQLException;
+
     /**
      * Get all of the groups that an eperson is a member of.
      *
@@ -143,33 +156,36 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      *
      * @param context
      *            DSpace context
-     * @param sortField
+     * @param metadataSortFields
-     *            field to sort by -- Group.ID or Group.NAME
+     *            metadata fields to sort by, leave empty to sort by Name
      *
      * @return array of all groups in the site
      */
-    public List<Group> findAll(Context context, int sortField) throws SQLException;
+    public List<Group> findAll(Context context, List<MetadataField> metadataSortFields) throws SQLException;
 
+    /** DEPRECATED: Please use findAll(Context context, List<MetadataField> metadataFieldsSort) instead */
+    @Deprecated
+    public List<Group> findAll(Context context, int sortField) throws SQLException;
 
     /**
      * Find the groups that match the search query across eperson_group_id or name
      *
      * @param context
      *            DSpace context
-     * @param query
+     * @param groupIdentifier
-     *            The search string
+     *            The group name or group ID
      *
      * @return array of Group objects
      */
-    public List<Group> search(Context context, String query) throws SQLException;
+    public List<Group> search(Context context, String groupIdentifier) throws SQLException;
 
     /**
      * Find the groups that match the search query across eperson_group_id or name
      *
      * @param context
      *            DSpace context
-     * @param query
+     * @param groupIdentifier
-     *            The search string
+     *            The group name or group ID
      * @param offset
      *            Inclusive offset
      * @param limit
@@ -177,7 +193,7 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      *
      * @return array of Group objects
      */
-    public List<Group> search(Context context, String query, int offset, int limit) throws SQLException;
+    public List<Group> search(Context context, String groupIdentifier, int offset, int limit) throws SQLException;
 
     /**
      * Returns the total number of groups returned by a specific query, without the overhead
@@ -207,7 +223,29 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
      */
     public void initDefaultGroupNames(Context context) throws SQLException, AuthorizeException;
 
+    /**
+     * Find all empty groups in DSpace
+     * @param context The DSpace context
+     * @return All empty groups
+     * @throws SQLException
+     */
     List<Group> getEmptyGroups(Context context) throws SQLException;
 
+    /**
+     * Count the total number of groups in DSpace
+     * @param context The DSpace context
+     * @return The total number of groups
+     * @throws SQLException
+     */
     int countTotal(Context context) throws SQLException;
+
+    /**
+     * Look up groups based on their value for a certain metadata field (NOTE: name is not stored as metadata)
+     * @param context The DSpace context
+     * @param searchValue The value to match
+     * @param metadataField The metadata field to search in
+     * @return The groups that have a matching value for specified metadata field
+     * @throws SQLException
+     */
+    List<Group> findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException;
 }

dspace-api/src/main/java/org/dspace/util/UUIDUtils.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.util;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.UUID;
+
+/**
+ * Utility class to read UUIDs
+ */
+public class UUIDUtils {
+
+    public static UUID fromString(final String identifier) {
+        UUID output = null;
+        if(StringUtils.isNotBlank(identifier)) {
+            try {
+                output = UUID.fromString(identifier.trim());
+            } catch(IllegalArgumentException e) {
+                output = null;
+            }
+        }
+        return output;
+    }
+
+    public static String toString(final UUID identifier) {
+        return identifier == null ? null : identifier.toString();
+    }
+}

dspace-api/src/main/java/org/dspace/xmlworkflow/XmlWorkflowServiceImpl.java

@@ -131,11 +131,11 @@ public class XmlWorkflowServiceImpl implements XmlWorkflowService {
                     authorizeService.authorizeAction(context, collection, Constants.WRITE);
                     roleGroup = groupService.create(context);
                     if(role.getScope() == Role.Scope.COLLECTION){
-                        groupService.setName(context, roleGroup,
+                        groupService.setName(roleGroup,
                                 "COLLECTION_" + collection.getID().toString()
                                         + "_WORKFLOW_ROLE_" + roleName);
                     }else{
-                        groupService.setName(context, roleGroup,  role.getName());
+                        groupService.setName(roleGroup,  role.getName());
                     }
                     groupService.update(context, roleGroup);
                     authorizeService.addPolicy(context, collection, Constants.ADD, roleGroup);

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql

@@ -0,0 +1,33 @@
+--
+-- 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-3024 extremely slow searching when logged in as admin
+---------------------------------------------------------------
+-- This script will put the group name on the epersongroup
+-- record itself for performance reasons. It will also make
+-- sure that a group name is unique (so that for example no two
+-- Administrator groups can be created).
+---------------------------------------------------------------
+
+ALTER TABLE epersongroup
+DROP COLUMN IF EXISTS name;
+
+ALTER TABLE epersongroup
+ADD (name VARCHAR(250));
+
+CREATE UNIQUE INDEX epersongroup_unique_idx_name on epersongroup(name);
+
+UPDATE epersongroup
+SET name =
+(SELECT text_value
+ FROM metadatavalue v
+   JOIN metadatafieldregistry field on v.metadata_field_id = field.metadata_field_id
+   JOIN metadataschemaregistry s ON field.metadata_schema_id = s.metadata_schema_id
+ WHERE s.short_id = 'dc' AND element = 'title' AND qualifier IS NULL
+ AND v.dspace_object_id = epersongroup.uuid LIMIT 1);

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql

@@ -0,0 +1,30 @@
+--
+-- 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-3024 extremely slow searching when logged in as admin
+---------------------------------------------------------------
+-- This script will put the group name on the epersongroup
+-- record itself for performance reasons. It will also make
+-- sure that a group name is unique (so that for example no two
+-- Administrator groups can be created).
+---------------------------------------------------------------
+
+ALTER TABLE epersongroup
+ADD name VARCHAR2(250);
+
+CREATE UNIQUE INDEX epersongroup_unique_idx_name on epersongroup(name);
+
+UPDATE epersongroup
+SET name =
+(SELECT text_value
+ FROM metadatavalue v
+   JOIN metadatafieldregistry field on v.metadata_field_id = field.metadata_field_id
+   JOIN metadataschemaregistry s ON field.metadata_schema_id = s.metadata_schema_id
+ WHERE s.short_id = 'dc' AND element = 'title' AND qualifier IS NULL
+ AND v.dspace_object_id = epersongroup.uuid);

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql

@@ -0,0 +1,33 @@
+--
+-- 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-3024 extremely slow searching when logged in as admin
+---------------------------------------------------------------
+-- This script will put the group name on the epersongroup
+-- record itself for performance reasons. It will also make
+-- sure that a group name is unique (so that for example no two
+-- Administrator groups can be created).
+---------------------------------------------------------------
+
+ALTER TABLE epersongroup
+DROP COLUMN IF EXISTS name;
+
+ALTER TABLE epersongroup
+ADD COLUMN name VARCHAR(250);
+
+CREATE UNIQUE INDEX epersongroup_unique_idx_name on epersongroup(name);
+
+UPDATE epersongroup
+SET name =
+(SELECT text_value
+ FROM metadatavalue v
+   JOIN metadatafieldregistry field on v.metadata_field_id = field.metadata_field_id
+   JOIN metadataschemaregistry s ON field.metadata_schema_id = s.metadata_schema_id
+ WHERE s.short_id = 'dc' AND element = 'title' AND qualifier IS NULL
+ AND v.dspace_object_id = epersongroup.uuid LIMIT 1);

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

@@ -7,7 +7,6 @@
  */
 package org.dspace.eperson;
 
-import java.sql.SQLException;
 import org.dspace.AbstractUnitTest;
 import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.GroupService;
@@ -15,6 +14,8 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.sql.SQLException;
+
 /**
  * Test integration of GroupServiceImpl.
  *
@@ -105,7 +106,7 @@ public class GroupServiceImplIT
         String name = "NOTANONYMOUS";
         GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
         Group group = groupService.findByName(context, Group.ANONYMOUS);
-        groupService.setName(context, group, name);
+        groupService.setName(group, name);
     }
 
     /**

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

@@ -141,12 +141,25 @@ public class GroupTest extends AbstractUnitTest {
 
     @Test
     public void setGroupName() throws SQLException, AuthorizeException {
-        topGroup.setName(context, "new name");
+        topGroup.setName("new name");
         groupService.update(context, topGroup);
         assertThat("setGroupName 1", topGroup.getName(), notNullValue());
         assertEquals("setGroupName 2", topGroup.getName(), "new name");
     }
 
+    @Test
+    public void setGroupNameOnPermanentGroup() throws SQLException, AuthorizeException {
+        topGroup.setPermanent(true);
+
+        topGroup.setName("new name");
+        groupService.update(context, topGroup);
+        assertThat("setGroupName 1", topGroup.getName(), notNullValue());
+        assertEquals("setGroupName 2", topGroup.getName(), "topGroup");
+
+        topGroup.setPermanent(false);
+        groupService.update(context, topGroup);
+    }
+
     @Test
     public void findByName() throws SQLException {
         Group group = groupService.findByName(context, "topGroup");
@@ -157,7 +170,7 @@ public class GroupTest extends AbstractUnitTest {
 
     @Test
     public void findAll() throws SQLException {
-        List<Group> groups = groupService.findAll(context, GroupService.NAME);
+        List<Group> groups = groupService.findAll(context, null);
         assertThat("findAll 1", groups, notNullValue());
         System.out.println("TEST GROUP OUTPUT " + groups);
         assertTrue("findAll 2", 0 < groups.size());
@@ -184,7 +197,7 @@ public class GroupTest extends AbstractUnitTest {
     @Test
     public void findAllNameSort() throws SQLException {
         // Retrieve groups sorted by name
-        List<Group> groups = groupService.findAll(context, GroupService.NAME);
+        List<Group> groups = groupService.findAll(context, null);
 
         assertThat("findAllNameSort 1", groups, notNullValue());
 
@@ -491,7 +504,7 @@ public class GroupTest extends AbstractUnitTest {
     protected Group createGroup(String name) throws SQLException, AuthorizeException {
         context.turnOffAuthorisationSystem();
         Group group = groupService.create(context);
-        group.setName(context, name);
+        group.setName(name);
         groupService.update(context, group);
         context.restoreAuthSystemState();
         return group;

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

@@ -394,7 +394,7 @@ public class AccessSettingTag extends TagSupport
         }
 
         if (groups == null || groups.size() == 0){
-            groups = groupService.findAll(context, GroupService.NAME);
+            groups = groupService.findAll(context, null);
         }
 
         return groups;

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

@@ -7,18 +7,6 @@
  */
 package org.dspace.app.webui.servlet.admin;
 
-import java.io.IOException;
-import java.sql.SQLException;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
 import org.apache.commons.lang.time.DateUtils;
 import org.dspace.app.util.AuthorizeUtil;
 import org.dspace.app.webui.servlet.DSpaceServlet;
@@ -29,18 +17,10 @@ import org.dspace.authorize.PolicySet;
 import org.dspace.authorize.ResourcePolicy;
 import org.dspace.authorize.factory.AuthorizeServiceFactory;
 import org.dspace.authorize.service.ResourcePolicyService;
-import org.dspace.content.Bitstream;
+import org.dspace.content.*;
-import org.dspace.content.Bundle;
 import org.dspace.content.Collection;
-import org.dspace.content.Community;
-import org.dspace.content.DSpaceObject;
-import org.dspace.content.Item;
 import org.dspace.content.factory.ContentServiceFactory;
-import org.dspace.content.service.BitstreamService;
+import org.dspace.content.service.*;
-import org.dspace.content.service.BundleService;
-import org.dspace.content.service.CollectionService;
-import org.dspace.content.service.CommunityService;
-import org.dspace.content.service.ItemService;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
@@ -51,6 +31,13 @@ import org.dspace.eperson.service.GroupService;
 import org.dspace.handle.factory.HandleServiceFactory;
 import org.dspace.handle.service.HandleService;
 
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.*;
+
 /**
  * Servlet for editing permissions
  * 
@@ -123,7 +110,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
         {
             // select a collections to work on
             List<Collection> collections = collectionService.findAll(c);
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
 
             request.setAttribute("collections", collections);
             request.setAttribute("groups", groups);
@@ -188,7 +175,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 			ResourcePolicy policy = authorizeService.createResourcePolicy(c, item,
 					groupService.findByName(c, Group.ANONYMOUS), null, -1, null);
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to item permission page
@@ -215,7 +202,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 
             policy = resourcePolicyService.find(c, policyId);
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to collection permission page
@@ -240,7 +227,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 			ResourcePolicy policy = authorizeService.createResourcePolicy(c, bundle,
 					groupService.findByName(c, Group.ANONYMOUS), null, -1, null);
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to item permission page
@@ -268,7 +255,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 			ResourcePolicy policy = authorizeService.createResourcePolicy(c, bitstream,
 					groupService.findByName(c, Group.ANONYMOUS), null, -1, null);
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to item permission page
@@ -317,7 +304,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 					groupService.findByName(c, Group.ANONYMOUS), null, -1, null);
 
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to collection permission page
@@ -410,7 +397,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
                 policy = resourcePolicyService.find(c, policyId);
             }
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to collection permission page
@@ -447,7 +434,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
                 policy = resourcePolicyService.find(c, policyId);
             }
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to collection permission page
@@ -473,7 +460,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 					groupService.findByName(c, Group.ANONYMOUS), null, -1, null);
 
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to collection permission page
@@ -500,7 +487,7 @@ public class AuthorizeAdminServlet extends DSpaceServlet
 					groupService.findByName(c, Group.ANONYMOUS), null, -1, null);
 
 
-            List<Group> groups = groupService.findAll(c, GroupService.NAME);
+            List<Group> groups = groupService.findAll(c, null);
             List<EPerson> epeople = personService.findAll(c, EPerson.EMAIL);
 
             // return to collection permission page

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

@@ -400,7 +400,7 @@ public class CollectionWizardServlet extends DSpaceServlet
             g = groupService.create(context);
 
             // Name it according to our conventions
-            groupService.setName(context, g,
+            groupService.setName(g,
                     "COLLECTION_" + collection.getID() + "_DEFAULT_ITEM_READ");
 
             // Give it the needed permission

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

@@ -7,17 +7,6 @@
  */
 package org.dspace.app.webui.servlet.admin;
 
-import java.io.IOException;
-import java.sql.SQLException;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-import java.util.UUID;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
 import org.dspace.app.webui.servlet.DSpaceServlet;
 import org.dspace.app.webui.util.JSPManager;
 import org.dspace.app.webui.util.UIUtil;
@@ -30,6 +19,16 @@ import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.eperson.service.GroupService;
 
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+
 /**
  * Servlet for editing groups
  * 
@@ -97,7 +96,7 @@ public class GroupEditServlet extends DSpaceServlet
 
                 if (!newName.equals(group.getName()))
                 {
-                    groupService.setName(c, group, newName);
+                    groupService.setName(group, newName);
                     groupService.update(c, group);
                 }
 
@@ -257,7 +256,7 @@ public class GroupEditServlet extends DSpaceServlet
             {
                 group = groupService.create(c);
 
-                groupService.setName(c, group, "new group" + group.getID());
+                groupService.setName(group, "new group" + group.getID());
                 groupService.update(c, group);
 
                 request.setAttribute("group", group);
@@ -279,7 +278,7 @@ public class GroupEditServlet extends DSpaceServlet
             HttpServletResponse response) throws ServletException, IOException,
             SQLException, AuthorizeException
     {
-        List<Group> groups = groupService.findAll(c, GroupService.NAME);
+        List<Group> groups = groupService.findAll(c, null);
 
         // if( groups == null ) { System.out.println("groups are null"); }
         // else System.out.println("# of groups: " + groups.length);

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

@@ -9,6 +9,7 @@ package org.dspace.app.xmlui.aspect.administrative;
 
 import org.apache.cocoon.environment.Request;
 import org.apache.cocoon.servlet.multipart.Part;
+import org.apache.commons.lang.StringUtils;
 import org.dspace.app.xmlui.utils.UIException;
 import org.dspace.app.xmlui.wing.Message;
 import org.dspace.authorize.AuthorizeException;
@@ -25,7 +26,6 @@ import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.CollectionService;
 import org.dspace.content.service.CommunityService;
 import org.dspace.content.service.ItemService;
-import org.dspace.services.factory.DSpaceServicesFactory;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.curate.Curator;
@@ -37,6 +37,7 @@ import org.dspace.harvest.HarvestedCollection;
 import org.dspace.harvest.OAIHarvester;
 import org.dspace.harvest.factory.HarvestServiceFactory;
 import org.dspace.harvest.service.HarvestedCollectionService;
+import org.dspace.services.factory.DSpaceServicesFactory;
 import org.dspace.workflow.WorkflowException;
 import org.dspace.workflow.WorkflowService;
 import org.dspace.workflow.factory.WorkflowServiceFactory;
@@ -57,7 +58,6 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.UUID;
-import org.apache.commons.lang.StringUtils;
 
 /**
  * Utility methods to processes actions on Communities and Collections.
@@ -604,7 +604,7 @@ public class FlowContainerUtils
         }
 		
 		Group role = groupService.create(context);
-        groupService.setName(context, role, "COLLECTION_"+collection.getID().toString() +"_DEFAULT_READ");
+        groupService.setName(role, "COLLECTION_"+collection.getID().toString() +"_DEFAULT_READ");
 		
 		// Remove existing privileges from the anonymous group.
 		authorizeService.removePoliciesActionFilter(context, collection, Constants.DEFAULT_ITEM_READ);

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

@@ -217,7 +217,7 @@ public class FlowGroupUtils {
 	    	{
 				// All good, create the new group.
 				group = groupService.create(context);
-				groupService.setName(context, group, newName);
+				groupService.setName(group, newName);
 	    	}
 			else
 			{
@@ -245,7 +245,7 @@ public class FlowGroupUtils {
 				if (potentialDuplicate == null)
 		    	{
 					// All good, update the name
-					groupService.setName(context, group, newName);
+					groupService.setName(group, newName);
 		    	}
 				else
 				{

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

@@ -7,10 +7,6 @@
  */
 package org.dspace.app.xmlui.aspect.administrative.authorization;
 
-import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Collections;
-
 import org.apache.cocoon.environment.ObjectModelHelper;
 import org.apache.cocoon.environment.Request;
 import org.dspace.app.xmlui.cocoon.AbstractDSpaceTransformer;
@@ -25,6 +21,10 @@ import org.dspace.eperson.Group;
 import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.GroupService;
 
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collections;
+
 /**
  * @author Alexey Maslov
  */
@@ -145,7 +145,7 @@ public class AdvacedAuthorizationsForm extends AbstractDSpaceTransformer
         if (errors.contains("groupIDs")){
             groupSelect.addError(T_error_groupIds);
         }
-        for (Group group : groupService.findAll(context, GroupService.NAME))
+        for (Group group : groupService.findAll(context, null))
         {
             if(wasElementSelected(group.getID().toString(), groupIDs)){
                 groupSelect.addOption(true, group.getID().toString(), group.getName());

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

@@ -268,7 +268,7 @@ public class EditPolicyForm extends AbstractDSpaceTransformer
         // currently set group
         actionsList.addLabel(T_policy_currentGroup);
         Select groupSelect = actionsList.addItem().addSelect("group_id");
-        for (Group group : groupService.findAll(context, GroupService.NAME))
+        for (Group group : groupService.findAll(context, null))
         {
             if (group == currentGroup)
             {

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

@@ -129,7 +129,7 @@ public class AccessStepUtil extends AbstractDSpaceTransformer {
                     loadedGroups= uiGroup.getMemberGroups();
             }
             if(loadedGroups==null || loadedGroups.size() ==0){
-                loadedGroups = groupService.findAll(context, GroupService.NAME);
+                loadedGroups = groupService.findAll(context, null);
             }
 
             // if no group selected for default set anonymous

dspace-xmlui/src/main/resources/aspects/Administrative/administrative.js

@@ -2356,7 +2356,7 @@ function doEditPolicy(objectType,objectID,policyID)
     var result;
     var query= "-1";
     var groupID;
-    var actionID;
+    var actionID = -1;
     var page = 0;
     var name;
     var description;