DS-3004: Check group membership with one database call + cache the result in Hibernate. Also fixed a problem with the group2groupCache

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/eperson/Group.java

@@ -49,7 +49,7 @@ public class Group extends DSpaceObject implements DSpaceObjectLegacySupport
     @Column
     private Boolean permanent = false;
 
-    @Column
+    @Column(length = 250, unique = true)
     private String name;
 
     /** lists of epeople and groups in the group */

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

@@ -7,7 +7,9 @@
  */
 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;
@@ -159,15 +161,38 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements
 
     @Override
     public boolean isMember(Context context, Group group) throws SQLException {
+        return isMember(context, group.getName());
+    }
+
+    @Override
+    public boolean isMember(final Context context, final String groupName) throws SQLException {
         // special, everyone is member of group 0 (anonymous)
-        if (group.getName().equals(Group.ANONYMOUS))
+        if (StringUtils.equals(groupName, Group.ANONYMOUS))
         {
             return true;
+        } else if(context.getCurrentUser() != null) {
+            EPerson currentUser = context.getCurrentUser();
+
+            //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;
         }
-
-        EPerson currentuser = context.getCurrentUser();
-
-        return epersonInGroup(context, group, currentuser);
     }
 
     @Override
@@ -432,12 +457,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);
     }
 
 
@@ -450,11 +473,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)) {

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,21 +28,23 @@ import java.util.List;
  */
 public interface GroupDAO extends DSpaceObjectDAO<Group>, DSpaceObjectLegacySupportDAO<Group> {
 
-    public Group findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException;
+    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;
+    List<Group> search(Context context, String query, List<MetadataField> queryFields, int offset, int limit) throws SQLException;
 
-    public int searchResultCount(Context context, String query, List<MetadataField> queryFields) throws SQLException;
+    int searchResultCount(Context context, String query, List<MetadataField> queryFields) throws SQLException;
 
-    public List<Group> findAll(Context context, List<MetadataField> metadataFields, String sortColumn) throws SQLException;
+    List<Group> findAll(Context context, List<MetadataField> metadataFields, String sortColumn) throws SQLException;
 
-    public List<Group> findByEPerson(Context context, EPerson ePerson) throws SQLException;
+    List<Group> findByEPerson(Context context, EPerson ePerson) throws SQLException;
 
-    public List getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException;
+    List<Pair<UUID, UUID>> getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException;
 
     List<Group> getEmptyGroups(Context context) throws SQLException;
 
     int countRows(Context context) throws SQLException;
 
     Group findByName(Context context, String name) throws SQLException;
+
+    Group findByNameAndEPerson(Context context, String groupName, EPerson ePerson) throws SQLException;
 }

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

@@ -10,16 +10,14 @@ package org.dspace.eperson.dao.impl;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.ListUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.Pair;
 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.*;
@@ -76,19 +74,50 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO<Group> implements Grou
     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 Group findByName(final Context context, final String name) throws SQLException {
         Query query = createQuery(context,
-                "select g from Group g " +
+                "SELECT g from Group g " +
                 "where g.name = :name ");
 
         query.setParameter("name", name);
+        query.setCacheable(true);
 
         return uniqueResult(query);
     }
 
+    @Override
+    public Group findByNameAndEPerson(Context context, String groupName, EPerson ePerson) throws SQLException {
+        if(groupName == null || ePerson == null) {
+            return 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 " +
+                                ") " +
+                            ")");
+
+            query.setParameter("name", groupName);
+            query.setParameter("eperson_id", ePerson.getID());
+            query.setCacheable(true);
+
+            return uniqueResult(query);
+        }
+    }
+
     @Override
     public List<Group> search(Context context, String query, List<MetadataField> queryFields, int offset, int limit) throws SQLException {
         String groupTableName = "g";
@@ -157,15 +186,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

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

@@ -105,6 +105,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.
      *