diff --git a/dspace-api/src/main/java/org/dspace/app/util/MetadataExposureServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/util/MetadataExposureServiceImpl.java index 9e5f9ef848..bdf2c35965 100644 --- a/dspace-api/src/main/java/org/dspace/app/util/MetadataExposureServiceImpl.java +++ b/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 - if (context != null && authorizeService.isAdmin(context)) - { - return false; - } + boolean hidden = false; // for schema.element, just check schema->elementSet if (!isInitialized()) @@ -93,9 +85,8 @@ public class MetadataExposureServiceImpl implements MetadataExposureService if (qualifier == null) { Set 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 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; } /** diff --git a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java index 95e660826a..a33f5e9033 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java +++ b/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.Bundle; +import org.dspace.content.*; 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); } } diff --git a/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java index 276bbca6bd..7c7f287b4c 100644 --- a/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java @@ -318,7 +318,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl 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 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 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 i Group g = collection.getWorkflowStep1(); if (g != null) { + collection.setWorkflowStep1(null); groupService.delete(context, g); } @@ -696,6 +697,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i if (g != null) { + collection.setWorkflowStep2(null); groupService.delete(context, g); } @@ -703,6 +705,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i if (g != null) { + collection.setWorkflowStep3(null); groupService.delete(context, g); } @@ -711,6 +714,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i if (g != null) { + collection.setAdmins(null); groupService.delete(context, g); } @@ -719,6 +723,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i if (g != null) { + collection.setSubmitters(null); groupService.delete(context, g); } diff --git a/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java index 9775ba9d92..160d9b1d8b 100644 --- a/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java @@ -270,7 +270,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl 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); } diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataFieldDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataFieldDAOImpl.java index 6013daa689..c0de3200a6 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataFieldDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataFieldDAOImpl.java @@ -40,11 +40,13 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO 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 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 im public List findAll(Context context, Class 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 im Restrictions.eqOrIsNull("qualifier", qualifier) ) ); + criteria.setCacheable(true); + return singleResult(criteria); } @@ -93,6 +100,8 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO im Restrictions.eq("element", element) ) ); + criteria.setCacheable(true); + return list(criteria); } @@ -102,8 +111,10 @@ public class MetadataFieldDAOImpl extends AbstractHibernateDAO 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); } } diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataSchemaDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataSchemaDAOImpl.java index 214b405bf3..5747bbd4dc 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataSchemaDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataSchemaDAOImpl.java @@ -46,6 +46,8 @@ public class MetadataSchemaDAOImpl extends AbstractHibernateDAO // 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 // 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 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 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 criteria.add( Restrictions.eq("name", shortName) ); + criteria.setCacheable(true); return uniqueResult(criteria); } diff --git a/dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java b/dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java index cf4ddcdf55..695efccd7e 100644 --- a/dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java +++ b/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) { diff --git a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java b/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java index 7cfed39246..4714ebdce4 100644 --- a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java +++ b/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()); } diff --git a/dspace-api/src/main/java/org/dspace/eperson/Group.java b/dspace-api/src/main/java/org/dspace/eperson/Group.java index d61046f6d9..1e7babdaf6 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/Group.java +++ b/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, - MetadataSchema.DC_SCHEMA, "title", null, null, name); + if(!StringUtils.equals(this.name, name) && !isPermanent()) { + 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.) diff --git a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java index 1bed461071..2513ef63f3 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java +++ b/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 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 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 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 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 specialGroups = context.getSpecialGroups(); + if(CollectionUtils.isNotEmpty(specialGroups)) { + Iterator 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 @@ -247,59 +268,72 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl 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 metadataSortFields) instead */ @Override - public List findAll(Context context, int sortField) throws SQLException - { - List metadataFieldsSort = new ArrayList<>(); - switch (sortField) - { - case NAME: - metadataFieldsSort.add(metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null)); - - break; - - default: - metadataFieldsSort.add(metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null)); + @Deprecated + public List 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"); } - - return groupDAO.findAll(context, metadataFieldsSort, null); } @Override - public List search(Context context, String query) throws SQLException { - return search(context, query, -1, -1); - } - - @Override - public List search(Context context, String query, int offset, int limit) throws SQLException + public List findAll(Context context, List metadataSortFields) throws SQLException { - try { - List groups = new ArrayList<>(); - Group group = find(context, UUID.fromString(query)); + if(CollectionUtils.isEmpty(metadataSortFields)) { + return groupDAO.findAll(context); + } else { + return groupDAO.findAll(context, metadataSortFields); + } + } + + @Override + public List search(Context context, String groupIdentifier) throws SQLException { + return search(context, groupIdentifier, -1, -1); + } + + @Override + public List search(Context context, String groupIdentifier, int offset, int limit) throws SQLException + { + List groups = new ArrayList<>(); + 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 { - try { - return find(context, UUID.fromString(query)) != null ? 1 : 0; - } catch(IllegalArgumentException e) { - MetadataField nameField = metadataFieldService.findByElement(context, MetadataSchema.DC_SCHEMA, "title", null); - if (StringUtils.isBlank(query)) query = null; - return groupDAO.searchResultCount(context, query, Arrays.asList(nameField)); + public int searchResultCount(Context context, String groupIdentifier) throws SQLException { + int result = 0; + UUID uuid = UUIDUtils.fromString(groupIdentifier); + if(uuid == null && StringUtils.isNotBlank(groupIdentifier)) { + //Search by group name + result = groupDAO.countByNameLike(context, groupIdentifier); + } else { + //Search by group id + Group group = find(context, uuid); + if(group != null) + { + result = 1; + } } + + return result; } @Override @@ -381,7 +415,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl 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 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 implements - protected boolean epersonInGroup(Context c, Group group, EPerson e) + protected boolean epersonInGroup(Context context, String groupName, EPerson ePerson) throws SQLException { - List groups = allMemberGroups(c, e); - - return groups.contains(group); + return groupDAO.findByNameAndEPerson(context, groupName, ePerson) != null; } @@ -451,11 +483,10 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements Map> parents = new HashMap<>(); - List group2groupResults = groupDAO.getGroup2GroupResults(context, flushQueries); - for (Object[] group2groupResult : group2groupResults) { - //We cannot use UUID.nameUUIDFromBytes(), because it only generated "type 3 UUIDs" - UUID parent = (UUID) group2groupResult[0]; - UUID child = (UUID) group2groupResult[1]; + List> group2groupResults = groupDAO.getGroup2GroupResults(context, flushQueries); + for (Pair group2groupResult : group2groupResults) { + UUID parent = group2groupResult.getLeft(); + UUID child = group2groupResult.getRight(); // if parent doesn't have an entry, create one if (!parents.containsKey(parent)) { @@ -627,7 +658,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements } else { - return find(context, UUID.fromString(id)); + return find(context, UUIDUtils.fromString(id)); } } @@ -640,4 +671,9 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements public int countTotal(Context context) throws SQLException { return groupDAO.countRows(context); } + + @Override + public List findByMetadataField(final Context context, final String searchValue, final MetadataField metadataField) throws SQLException { + return groupDAO.findByMetadataField(context, searchValue, metadataField); + } } diff --git a/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java b/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java index 5dd7161ced..f2af343482 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java +++ b/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, DSpaceObjectLegacySupportDAO { - 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 findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException; - public List search(Context context, String query, List 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 findAll(Context context, List sortMetadataFields) throws SQLException; - public int searchResultCount(Context context, String query, List 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 findAll(Context context) throws SQLException; - public List findAll(Context context, List 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 findByEPerson(Context context, EPerson ePerson) throws SQLException; - public List findByEPerson(Context context, EPerson ePerson) throws SQLException; - - public List getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException; + /** + * Get a list of all direct parent - child group relations in the database + * @param context The DSpace context + * @param flushQueries Flush all pending queries + * @return A list of pairs indicating parent - child + * @throws SQLException + */ + List> getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException; + /** + * Return all empty groups + * @param context The DSpace context + * @return All empty groups + * @throws SQLException + */ List 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 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; + } diff --git a/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java b/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java index dfb17143b5..a27689d75e 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/dao/impl/Group2GroupCacheDAOImpl.java @@ -37,7 +37,9 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO 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 implements Grou } @Override - public Group findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException + public List 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 implements Grou query.setParameter(metadataField.toString(), metadataField.getFieldID()); query.setParameter("queryParam", searchValue); - return uniqueResult(query); + return list(query); } @Override - public List findAll(Context context, List sortFields, String sortColumn) throws SQLException + public List findAll(Context context, List 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); - addMetadataSortQuery(queryBuilder, sortFields, Collections.singletonList(sortColumn)); + addMetadataLeftJoin(queryBuilder, groupTableName, sortMetadataFields); + 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 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 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 search(Context context, String query, List queryFields, int offset, int limit) throws SQLException { - String groupTableName = "g"; - String queryString = "SELECT " + groupTableName + " FROM Group as " + groupTableName; - Query hibernateQuery = getSearchQuery(context, queryString, query, queryFields, ListUtils.EMPTY_LIST, null); + public Group findByName(final Context context, final String name) throws SQLException { + Query query = createQuery(context, + "SELECT g from Group g " + + "where g.name = :name "); - if(0 <= offset) - { - hibernateQuery.setFirstResult(offset); - } - if(0 <= limit) - { - hibernateQuery.setMaxResults(limit); - } - return list(hibernateQuery); + query.setParameter("name", name); + query.setCacheable(true); + + return uniqueResult(query); } @Override - public int searchResultCount(Context context, String query, List queryFields) throws SQLException { - String groupTableName = "g"; - String queryString = "SELECT count(*) FROM Group as " + groupTableName; - Query hibernateQuery = getSearchQuery(context, queryString, query, queryFields, ListUtils.EMPTY_LIST, null); + 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 " + + ") " + + ")"); - 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 queryFields, List sortFields, String sortField) throws SQLException { + @Override + public List 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(); - queryBuilder.append(queryString); - Set metadataFieldsToJoin = new LinkedHashSet<>(); - metadataFieldsToJoin.addAll(queryFields); - if(CollectionUtils.isNotEmpty(sortFields)) + if(0 <= offset) { - metadataFieldsToJoin.addAll(sortFields); + query.setFirstResult(offset); + } + if(0 <= limit) + { + query.setMaxResults(limit); } - if(!CollectionUtils.isEmpty(metadataFieldsToJoin)) { - addMetadataLeftJoin(queryBuilder, "g", metadataFieldsToJoin); - } - if(queryParam != null) { - addMetadataValueWhereQuery(queryBuilder, queryFields, "like", null); - } - if(!CollectionUtils.isEmpty(sortFields)) { - addMetadataSortQuery(queryBuilder, sortFields, Collections.singletonList(sortField)); - } + return list(query); + } - Query query = createQuery(context, queryBuilder.toString()); - if(StringUtils.isNotBlank(queryParam)) { - query.setParameter("queryParam", "%"+queryParam+"%"); - } - for (MetadataField metadataField : metadataFieldsToJoin) { - query.setParameter(metadataField.toString(), metadataField.getFieldID()); - } + @Override + public int countByNameLike(final Context context, final String groupName) throws SQLException { + Query query = createQuery(context, + "SELECT count(*) FROM Group g WHERE lower(g.name) LIKE lower(:name)"); + query.setParameter("name", "%" + groupName + "%"); - return query; + return count(query); } @Override @@ -147,15 +164,15 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO implements Grou @Override - public List 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); - sqlQuery.addScalar("child_id", StandardBasicTypes.UUID_BINARY); - if(flushQueries){ - //Optional, flush queries when executing, could be usefull since you are using native queries & these sometimes require this option. - sqlQuery.setFlushMode(FlushMode.ALWAYS); - } - return sqlQuery.list(); + public List> getGroup2GroupResults(Context context, boolean flushQueries) throws SQLException { + + Query query = createQuery(context, "SELECT new org.apache.commons.lang3.tuple.ImmutablePair(g.id, c.id) " + + "FROM Group g " + + "JOIN g.groups c "); + + @SuppressWarnings("unchecked") + List> results = query.list(); + return results; } @Override @@ -167,4 +184,5 @@ public class GroupDAOImpl extends AbstractHibernateDSODAO implements Grou public int countRows(Context context) throws SQLException { return count(createQuery(context, "SELECT count(*) FROM Group")); } + } diff --git a/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java b/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java index c5186f8cef..ae0dacce4d 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java +++ b/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, 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, 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, DSpaceObjectLe * * @param context * DSpace context - * @param sortField - * field to sort by -- Group.ID or Group.NAME + * @param metadataSortFields + * metadata fields to sort by, leave empty to sort by Name * * @return array of all groups in the site */ - public List findAll(Context context, int sortField) throws SQLException; + public List findAll(Context context, List metadataSortFields) throws SQLException; + /** DEPRECATED: Please use findAll(Context context, List metadataFieldsSort) instead */ + @Deprecated + public List 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 - * The search string + * @param groupIdentifier + * The group name or group ID * * @return array of Group objects */ - public List search(Context context, String query) throws SQLException; + public List 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 - * The search string + * @param groupIdentifier + * The group name or group ID * @param offset * Inclusive offset * @param limit @@ -177,7 +193,7 @@ public interface GroupService extends DSpaceObjectService, DSpaceObjectLe * * @return array of Group objects */ - public List search(Context context, String query, int offset, int limit) throws SQLException; + public List 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, 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 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 findByMetadataField(Context context, String searchValue, MetadataField metadataField) throws SQLException; } diff --git a/dspace-api/src/main/java/org/dspace/util/UUIDUtils.java b/dspace-api/src/main/java/org/dspace/util/UUIDUtils.java new file mode 100644 index 0000000000..ac116f5917 --- /dev/null +++ b/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(); + } +} diff --git a/dspace-api/src/main/java/org/dspace/xmlworkflow/XmlWorkflowServiceImpl.java b/dspace-api/src/main/java/org/dspace/xmlworkflow/XmlWorkflowServiceImpl.java index ad1fd3c975..8dca275979 100644 --- a/dspace-api/src/main/java/org/dspace/xmlworkflow/XmlWorkflowServiceImpl.java +++ b/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); diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql new file mode 100644 index 0000000000..f4a87b11e7 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql new file mode 100644 index 0000000000..18cb4a5084 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V6.0_2016.02.25__DS-3004-slow-searching-as-admin.sql new file mode 100644 index 0000000000..2c4fb27619 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/dspace-api/src/test/java/org/dspace/eperson/GroupServiceImplIT.java b/dspace-api/src/test/java/org/dspace/eperson/GroupServiceImplIT.java index 5de98ed16c..9a05bdc653 100644 --- a/dspace-api/src/test/java/org/dspace/eperson/GroupServiceImplIT.java +++ b/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); } /** diff --git a/dspace-api/src/test/java/org/dspace/eperson/GroupTest.java b/dspace-api/src/test/java/org/dspace/eperson/GroupTest.java index 7c870cb4e5..ab08dc9a2c 100644 --- a/dspace-api/src/test/java/org/dspace/eperson/GroupTest.java +++ b/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 groups = groupService.findAll(context, GroupService.NAME); + List 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 groups = groupService.findAll(context, GroupService.NAME); + List 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; diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/AccessSettingTag.java b/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/AccessSettingTag.java index 2b3b50fa87..bc447a428f 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/AccessSettingTag.java +++ b/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; diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/AuthorizeAdminServlet.java b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/AuthorizeAdminServlet.java index 7dd9cd86fc..3daae4ceee 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/AuthorizeAdminServlet.java +++ b/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.Bundle; +import org.dspace.content.*; 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.BundleService; -import org.dspace.content.service.CollectionService; -import org.dspace.content.service.CommunityService; -import org.dspace.content.service.ItemService; +import org.dspace.content.service.*; 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 collections = collectionService.findAll(c); - List groups = groupService.findAll(c, GroupService.NAME); + List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List 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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); List epeople = personService.findAll(c, EPerson.EMAIL); // return to collection permission page diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/CollectionWizardServlet.java b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/CollectionWizardServlet.java index 387a05b520..9a791f6222 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/CollectionWizardServlet.java +++ b/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 diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/GroupEditServlet.java b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/GroupEditServlet.java index b995d09c4d..b8723360bc 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/GroupEditServlet.java +++ b/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 groups = groupService.findAll(c, GroupService.NAME); + List groups = groupService.findAll(c, null); // if( groups == null ) { System.out.println("groups are null"); } // else System.out.println("# of groups: " + groups.length); diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowContainerUtils.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowContainerUtils.java index 850857d1de..8ff0a0f117 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowContainerUtils.java +++ b/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); diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowGroupUtils.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowGroupUtils.java index 30ba0086ed..18da4e9381 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowGroupUtils.java +++ b/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 { diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/authorization/AdvacedAuthorizationsForm.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/authorization/AdvacedAuthorizationsForm.java index 621db13abb..0dd0b745ba 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/authorization/AdvacedAuthorizationsForm.java +++ b/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()); diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/authorization/EditPolicyForm.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/authorization/EditPolicyForm.java index 6d2bbd8a01..19d44c5cd6 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/authorization/EditPolicyForm.java +++ b/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) { diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/AccessStepUtil.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/AccessStepUtil.java index 30c2bac5c8..7732ddb741 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/submission/submit/AccessStepUtil.java +++ b/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 diff --git a/dspace-xmlui/src/main/resources/aspects/Administrative/administrative.js b/dspace-xmlui/src/main/resources/aspects/Administrative/administrative.js index 8f2a77eed1..bf1b794f6a 100644 --- a/dspace-xmlui/src/main/resources/aspects/Administrative/administrative.js +++ b/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;