diff --git a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportCLITool.java b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportCLITool.java index 4c941253a3..d4339e11a2 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportCLITool.java +++ b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportCLITool.java @@ -189,7 +189,7 @@ public class ItemImportCLITool { String zipfilename = ""; if (line.hasOption('z')) { zip = true; - zipfilename = sourcedir + System.getProperty("file.separator") + line.getOptionValue('z'); + zipfilename = line.getOptionValue('z'); } //By default assume collections will be given on the command line diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickPdfThumbnailFilter.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickPdfThumbnailFilter.java index c67c848705..ff00e29c09 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickPdfThumbnailFilter.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickPdfThumbnailFilter.java @@ -46,11 +46,4 @@ public class ImageMagickPdfThumbnailFilter extends ImageMagickThumbnailFilter { } } - public static final String[] PDF = {"Adobe PDF"}; - @Override - public String[] getInputMIMETypes() - { - return PDF; - } - } diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickThumbnailFilter.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickThumbnailFilter.java index 81f4086d7f..a8ecd4dca6 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickThumbnailFilter.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/ImageMagickThumbnailFilter.java @@ -34,7 +34,7 @@ import org.dspace.core.ConfigurationManager; * thumbnail.maxwidth, thumbnail.maxheight, the size we want our thumbnail to be * no bigger than. Creates only JPEGs. */ -public abstract class ImageMagickThumbnailFilter extends MediaFilter implements SelfRegisterInputFormats +public abstract class ImageMagickThumbnailFilter extends MediaFilter { protected static int width = 180; protected static int height = 120; @@ -188,21 +188,4 @@ public abstract class ImageMagickThumbnailFilter extends MediaFilter implements return true; //assume that the thumbnail is a custom one } - @Override - public String[] getInputMIMETypes() - { - return ImageIO.getReaderMIMETypes(); - } - - @Override - public String[] getInputDescriptions() - { - return null; - } - - @Override - public String[] getInputExtensions() - { - return ImageIO.getReaderFileSuffixes(); - } } 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 1adff9324d..6029c6d272 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java @@ -260,7 +260,7 @@ public class AuthorizeServiceImpl implements AuthorizeService // if user is an Admin on this object DSpaceObject adminObject = useInheritance ? serviceFactory.getDSpaceObjectService(o).getAdminObject(c, o, action) : null; - if (isAdmin(c, adminObject)) + if (isAdmin(c, e, adminObject)) { c.cacheAuthorizedAction(o, action, e, true, null); return true; @@ -322,7 +322,7 @@ public class AuthorizeServiceImpl implements AuthorizeService } if ((rp.getGroup() != null) - && (groupService.isMember(c, rp.getGroup()))) + && (groupService.isMember(c, e, rp.getGroup()))) { // group was set, and eperson is a member // of that group @@ -370,9 +370,14 @@ public class AuthorizeServiceImpl implements AuthorizeService @Override public boolean isAdmin(Context c, DSpaceObject o) throws SQLException { + return this.isAdmin(c, c.getCurrentUser(), o); + } + @Override + public boolean isAdmin(Context c, EPerson e, DSpaceObject o) throws SQLException + { // return true if user is an Administrator - if (isAdmin(c)) + if (isAdmin(c, e)) { return true; } @@ -382,7 +387,7 @@ public class AuthorizeServiceImpl implements AuthorizeService return false; } - Boolean cachedResult = c.getCachedAuthorizationResult(o, Constants.ADMIN, c.getCurrentUser()); + Boolean cachedResult = c.getCachedAuthorizationResult(o, Constants.ADMIN, e); if (cachedResult != null) { return cachedResult.booleanValue(); } @@ -397,18 +402,18 @@ public class AuthorizeServiceImpl implements AuthorizeService // check policies for date validity if (resourcePolicyService.isDateValid(rp)) { - if (rp.getEPerson() != null && rp.getEPerson().equals(c.getCurrentUser())) + if (rp.getEPerson() != null && rp.getEPerson().equals(e)) { - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), true, rp); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, true, rp); return true; // match } if ((rp.getGroup() != null) - && (groupService.isMember(c, rp.getGroup()))) + && (groupService.isMember(c, e, rp.getGroup()))) { // group was set, and eperson is a member // of that group - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), true, rp); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, true, rp); return true; } } @@ -427,12 +432,12 @@ public class AuthorizeServiceImpl implements AuthorizeService DSpaceObject parent = serviceFactory.getDSpaceObjectService(o).getParentObject(c, o); if (parent != null) { - boolean admin = isAdmin(c, parent); - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), admin, null); + boolean admin = isAdmin(c, e, parent); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, admin, null); return admin; } - c.cacheAuthorizedAction(o, Constants.ADMIN, c.getCurrentUser(), false, null); + c.cacheAuthorizedAction(o, Constants.ADMIN, e, false, null); return false; } @@ -455,7 +460,25 @@ public class AuthorizeServiceImpl implements AuthorizeService return groupService.isMember(c, Group.ADMIN); } } - + + @Override + public boolean isAdmin(Context c, EPerson e) throws SQLException + { + // if we're ignoring authorization, user is member of admin + if (c.ignoreAuthorization()) + { + return true; + } + + if (e == null) + { + return false; // anonymous users can't be admins.... + } else + { + return groupService.isMember(c, e, Group.ADMIN); + } + } + public boolean isCommunityAdmin(Context c) throws SQLException { EPerson e = c.getCurrentUser(); @@ -679,13 +702,14 @@ public class AuthorizeServiceImpl implements AuthorizeService @Override public boolean isAnIdenticalPolicyAlreadyInPlace(Context c, DSpaceObject dso, Group group, int action, int policyID) throws SQLException { - return findByTypeIdGroupAction(c, dso, group, action, policyID) != null; + return !resourcePolicyService.findByTypeGroupActionExceptId(c, dso, group, action, policyID).isEmpty(); } @Override - public ResourcePolicy findByTypeIdGroupAction(Context c, DSpaceObject dso, Group group, int action, int policyID) throws SQLException + public ResourcePolicy findByTypeGroupAction(Context c, DSpaceObject dso, Group group, int action) + throws SQLException { - List policies = resourcePolicyService.find(c, dso, group, action, policyID); + List policies = resourcePolicyService.find(c, dso, group, action); if (CollectionUtils.isNotEmpty(policies)) { @@ -695,7 +719,6 @@ public class AuthorizeServiceImpl implements AuthorizeService } } - /** * Generate Policies policies READ for the date in input adding reason. New policies are assigned automatically at the groups that * have right on the collection. E.g., if the anonymous can access the collection policies are assigned to anonymous. @@ -771,12 +794,19 @@ public class AuthorizeServiceImpl implements AuthorizeService public ResourcePolicy createOrModifyPolicy(ResourcePolicy policy, Context context, String name, Group group, EPerson ePerson, Date embargoDate, int action, String reason, DSpaceObject dso) throws AuthorizeException, SQLException { + ResourcePolicy policyTemp = null; + if (policy != null) + { + List duplicates = resourcePolicyService.findByTypeGroupActionExceptId(context, dso, group, action, policy.getID()); + if (!duplicates.isEmpty()) + { + policy = duplicates.get(0); + } + } else { + // if an identical policy (same Action and same Group) is already in place modify it... + policyTemp = findByTypeGroupAction(context, dso, group, action); + } - int policyID = -1; - if (policy != null) policyID = policy.getID(); - - // if an identical policy (same Action and same Group) is already in place modify it... - ResourcePolicy policyTemp = findByTypeIdGroupAction(context, dso, group, action, policyID); if (policyTemp != null) { policy = policyTemp; diff --git a/dspace-api/src/main/java/org/dspace/authorize/ResourcePolicyServiceImpl.java b/dspace-api/src/main/java/org/dspace/authorize/ResourcePolicyServiceImpl.java index 387bf1e55c..615a237b72 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/ResourcePolicyServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authorize/ResourcePolicyServiceImpl.java @@ -104,13 +104,22 @@ public class ResourcePolicyServiceImpl implements ResourcePolicyService } @Override - public List find(Context c, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException { - return resourcePolicyDAO.findByTypeIdGroupAction(c, dso, group, action, notPolicyID); + public List find(Context c, DSpaceObject dso, Group group, int action) throws SQLException { + return resourcePolicyDAO.findByTypeGroupAction(c, dso, group, action); } + @Override public List find(Context c, EPerson e, List groups, int action, int type_id) throws SQLException{ return resourcePolicyDAO.findByEPersonGroupTypeIdAction(c, e, groups, action, type_id); } + + @Override + public List findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) + throws SQLException + { + return resourcePolicyDAO.findByTypeGroupActionExceptId(context, dso, group, action, notPolicyID); + } + /** * Delete an ResourcePolicy diff --git a/dspace-api/src/main/java/org/dspace/authorize/dao/ResourcePolicyDAO.java b/dspace-api/src/main/java/org/dspace/authorize/dao/ResourcePolicyDAO.java index 16103f95c8..05b13c8ba8 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/dao/ResourcePolicyDAO.java +++ b/dspace-api/src/main/java/org/dspace/authorize/dao/ResourcePolicyDAO.java @@ -34,7 +34,16 @@ public interface ResourcePolicyDAO extends GenericDAO { public List findByDSoAndAction(Context context, DSpaceObject dso, int actionId) throws SQLException; - public List findByTypeIdGroupAction(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException; + public List findByTypeGroupAction(Context context, DSpaceObject dso, Group group, int action) throws SQLException; + + /** + * Look for ResourcePolicies by DSpaceObject, Group, and action, ignoring IDs with a specific PolicyID. + * This method can be used to detect duplicate ResourcePolicies. + * @param notPolicyID ResourcePolicies with this ID will be ignored while looking out for equal ResourcePolicies. + * @return List of resource policies for the same DSpaceObject, group and action but other policyID. + * @throws SQLException + */ + public List findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException; public List findByEPersonGroupTypeIdAction(Context context, EPerson e, List groups, int action, int type_id) throws SQLException; diff --git a/dspace-api/src/main/java/org/dspace/authorize/dao/impl/ResourcePolicyDAOImpl.java b/dspace-api/src/main/java/org/dspace/authorize/dao/impl/ResourcePolicyDAOImpl.java index 1123e74a4e..a876c77819 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/dao/impl/ResourcePolicyDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/authorize/dao/impl/ResourcePolicyDAOImpl.java @@ -75,7 +75,7 @@ public class ResourcePolicyDAOImpl extends AbstractHibernateDAO } @Override - public List findByTypeIdGroupAction(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException { + public List findByTypeGroupAction(Context context, DSpaceObject dso, Group group, int action) throws SQLException { Criteria criteria = createCriteria(context, ResourcePolicy.class); criteria.add(Restrictions.and( Restrictions.eq("dSpaceObject", dso), @@ -83,15 +83,21 @@ public class ResourcePolicyDAOImpl extends AbstractHibernateDAO Restrictions.eq("actionId", action) )); criteria.setMaxResults(1); - - List results; - if (notPolicyID != -1) - { - criteria.add(Restrictions.and(Restrictions.not(Restrictions.eq("id", notPolicyID)))); - } - return list(criteria); } + + @Override + public List findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException { + Criteria criteria = createCriteria(context, ResourcePolicy.class); + criteria.add(Restrictions.and( + Restrictions.eq("dSpaceObject", dso), + Restrictions.eq("epersonGroup", group), + Restrictions.eq("actionId", action) + )); + criteria.add(Restrictions.and(Restrictions.not(Restrictions.eq("id", notPolicyID)))); + return list(criteria); + } + public List findByEPersonGroupTypeIdAction(Context context, EPerson e, List groups, int action, int type_id) throws SQLException { Criteria criteria = createCriteria(context, ResourcePolicy.class); diff --git a/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java b/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java index 6e4be3acb7..07178d766d 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java +++ b/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java @@ -167,11 +167,28 @@ public interface AuthorizeService { */ public boolean isAdmin(Context c, DSpaceObject o) throws SQLException; + /** + * Check to see if a specific user is an Administrator of a given object + * within DSpace. Always return {@code true} if the user is a System + * Admin + * + * @param c current context + * @param e the user to check + * @param o current DSpace Object, if null the call will be + * equivalent to a call to the isAdmin(Context c) + * method + * @return {@code true} if the user has administrative privileges on the + * given DSpace object + * @throws SQLException if database error + */ + public boolean isAdmin(Context c, EPerson e, DSpaceObject o) throws SQLException; + /** * Check to see if the current user is a System Admin. Always return - * {@code true} if c.ignoreAuthorization is set. Anonymous users - * can't be Admins (EPerson set to NULL) + * {@code true} if c.ignoreAuthorization is set. If no EPerson is + * logged in and context.getCurrentUser() returns null, this method + * returns false as anonymous users can never be administrators. * * @param c current context * @return {@code true} if user is an admin or ignore authorization @@ -179,6 +196,17 @@ public interface AuthorizeService { * @throws SQLException if database error */ public boolean isAdmin(Context c) throws SQLException; + + /** + * Check to see if a specific user is system admin. Always return + * {@code true} if c.ignoreAuthorization is set. + * + * @param c current context + * @return {@code true} if user is an admin or ignore authorization + * flag set + * @throws SQLException if database error + */ + public boolean isAdmin(Context c, EPerson e) throws SQLException; public boolean isCommunityAdmin(Context c) throws SQLException; @@ -410,8 +438,8 @@ public interface AuthorizeService { * @throws SQLException if there's a database problem */ public boolean isAnIdenticalPolicyAlreadyInPlace(Context c, DSpaceObject o, Group group, int actionID, int policyID) throws SQLException; - - public ResourcePolicy findByTypeIdGroupAction(Context c, DSpaceObject dso, Group group, int action, int policyID) throws SQLException; + + public ResourcePolicy findByTypeGroupAction(Context c, DSpaceObject dso, Group group, int action) throws SQLException; /** diff --git a/dspace-api/src/main/java/org/dspace/authorize/service/ResourcePolicyService.java b/dspace-api/src/main/java/org/dspace/authorize/service/ResourcePolicyService.java index 240f4f4113..8dc934d69e 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/service/ResourcePolicyService.java +++ b/dspace-api/src/main/java/org/dspace/authorize/service/ResourcePolicyService.java @@ -33,11 +33,21 @@ public interface ResourcePolicyService extends DSpaceCRUDService public List find(Context c, DSpaceObject o, int actionId) throws SQLException; - public List find(Context c, DSpaceObject dso, Group group, int action, int notPolicyID) throws SQLException; + public List find(Context c, DSpaceObject dso, Group group, int action) throws SQLException; public List find(Context context, Group group) throws SQLException; public List find(Context c, EPerson e, List groups, int action, int type_id) throws SQLException; + + /** + * Look for ResourcePolicies by DSpaceObject, Group, and action, ignoring IDs with a specific PolicyID. + * This method can be used to detect duplicate ResourcePolicies. + * @param notPolicyID ResourcePolicies with this ID will be ignored while looking out for equal ResourcePolicies. + * @return List of resource policies for the same DSpaceObject, group and action but other policyID. + * @throws SQLException + */ + public List findByTypeGroupActionExceptId(Context context, DSpaceObject dso, Group group, int action, int notPolicyID) + throws SQLException; public String getActionText(ResourcePolicy resourcePolicy); diff --git a/dspace-api/src/main/java/org/dspace/content/BundleServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/BundleServiceImpl.java index 7ef6d0b9f1..ca6d034e1b 100644 --- a/dspace-api/src/main/java/org/dspace/content/BundleServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/BundleServiceImpl.java @@ -269,29 +269,60 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl implement public void setOrder(Context context, Bundle bundle, UUID[] bitstreamIds) throws AuthorizeException, SQLException { authorizeService.authorizeAction(context, bundle, Constants.WRITE); - bundle.getBitstreams().clear(); + List currentBitstreams = bundle.getBitstreams(); + List updatedBitstreams = new ArrayList(); + + // Loop through and ensure these Bitstream IDs are all valid. Add them to list of updatedBitstreams. for (int i = 0; i < bitstreamIds.length; i++) { UUID bitstreamId = bitstreamIds[i]; Bitstream bitstream = bitstreamService.find(context, bitstreamId); - if(bitstream == null){ + + // If we have an invalid Bitstream ID, just ignore it, but log a warning + if(bitstream == null) { //This should never occur but just in case log.warn(LogManager.getHeader(context, "Invalid bitstream id while changing bitstream order", "Bundle: " + bundle.getID() + ", bitstream id: " + bitstreamId)); continue; } - bitstream.getBundles().remove(bundle); - bundle.getBitstreams().add(bitstream); - bitstream.getBundles().add(bundle); - bitstreamService.update(context, bitstream); + // If we have a Bitstream not in the current list, log a warning & exit immediately + if(!currentBitstreams.contains(bitstream)) + { + log.warn(LogManager.getHeader(context, "Encountered a bitstream not in this bundle while changing bitstream order. Bitstream order will not be changed.", "Bundle: " + bundle.getID() + ", bitstream id: " + bitstreamId)); + return; + } + updatedBitstreams.add(bitstream); } - //The order of the bitstreams has changed, ensure that we update the last modified of our item - Item owningItem = (Item) getParentObject(context, bundle); - if(owningItem != null) + // If our lists are different sizes, exit immediately + if(updatedBitstreams.size()!=currentBitstreams.size()) { - itemService.updateLastModified(context, owningItem); - itemService.update(context, owningItem); + log.warn(LogManager.getHeader(context, "Size of old list and new list do not match. Bitstream order will not be changed.", "Bundle: " + bundle.getID())); + return; + } + // As long as the order has changed, update it + if(CollectionUtils.isNotEmpty(updatedBitstreams) && !updatedBitstreams.equals(currentBitstreams)) + { + //First clear out the existing list of bitstreams + bundle.getBitstreams().clear(); + + // Now add them back in the proper order + for (Bitstream bitstream : updatedBitstreams) + { + bitstream.getBundles().remove(bundle); + bundle.getBitstreams().add(bitstream); + bitstream.getBundles().add(bundle); + bitstreamService.update(context, bitstream); + } + + //The order of the bitstreams has changed, ensure that we update the last modified of our item + Item owningItem = (Item) getParentObject(context, bundle); + if(owningItem != null) + { + itemService.updateLastModified(context, owningItem); + itemService.update(context, owningItem); + + } } } diff --git a/dspace-api/src/main/java/org/dspace/content/Collection.java b/dspace-api/src/main/java/org/dspace/content/Collection.java index f3d5edbe16..9c94298de0 100644 --- a/dspace-api/src/main/java/org/dspace/content/Collection.java +++ b/dspace-api/src/main/java/org/dspace/content/Collection.java @@ -7,6 +7,7 @@ */ package org.dspace.content; +import org.dspace.content.comparator.NameAscendingComparator; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.CollectionService; import org.dspace.core.*; @@ -16,8 +17,7 @@ import org.hibernate.proxy.HibernateProxyHelper; import javax.persistence.*; import java.sql.SQLException; -import java.util.ArrayList; -import java.util.List; +import java.util.*; /** * Class representing a collection. @@ -86,7 +86,7 @@ public class Collection extends DSpaceObject implements DSpaceObjectLegacySuppor joinColumns = {@JoinColumn(name = "collection_id") }, inverseJoinColumns = {@JoinColumn(name = "community_id") } ) - private final List communities = new ArrayList<>(); + private Set communities = new HashSet<>(); @Transient private transient CollectionService collectionService; @@ -266,7 +266,11 @@ public class Collection extends DSpaceObject implements DSpaceObjectLegacySuppor */ public List getCommunities() throws SQLException { - return communities; + // We return a copy because we do not want people to add elements to this collection directly. + // We return a list to maintain backwards compatibility + Community[] output = communities.toArray(new Community[]{}); + Arrays.sort(output, new NameAscendingComparator()); + return Arrays.asList(output); } void addCommunity(Community community) { @@ -274,7 +278,7 @@ public class Collection extends DSpaceObject implements DSpaceObjectLegacySuppor setModified(); } - void removeCommunity(Community community){ + void removeCommunity(Community community) { this.communities.remove(community); setModified(); } 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 4c3c20d798..097d4901cb 100644 --- a/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java @@ -749,8 +749,8 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i while (owningCommunities.hasNext()) { Community owningCommunity = owningCommunities.next(); - owningCommunities.remove(); - owningCommunity.getCollections().remove(collection); + collection.removeCommunity(owningCommunity); + owningCommunity.removeCollection(collection); } collectionDAO.delete(context, collection); diff --git a/dspace-api/src/main/java/org/dspace/content/Community.java b/dspace-api/src/main/java/org/dspace/content/Community.java index 011d6e281c..da61aa59e3 100644 --- a/dspace-api/src/main/java/org/dspace/content/Community.java +++ b/dspace-api/src/main/java/org/dspace/content/Community.java @@ -9,6 +9,7 @@ package org.dspace.content; import org.apache.commons.lang.builder.HashCodeBuilder; import org.apache.log4j.Logger; +import org.dspace.content.comparator.NameAscendingComparator; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.CommunityService; import org.dspace.core.*; @@ -47,13 +48,13 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport joinColumns = {@JoinColumn(name = "parent_comm_id") }, inverseJoinColumns = {@JoinColumn(name = "child_comm_id") } ) - private final List subCommunities = new ArrayList<>(); + private Set subCommunities = new HashSet<>(); @ManyToMany(fetch = FetchType.LAZY, mappedBy = "subCommunities") - private List parentCommunities = new ArrayList<>(); + private Set parentCommunities = new HashSet<>(); @ManyToMany(fetch = FetchType.LAZY, mappedBy = "communities", cascade = {CascadeType.PERSIST}) - private final List collections = new ArrayList<>(); + private Set collections = new HashSet<>(); @OneToOne @JoinColumn(name = "admin") @@ -88,13 +89,13 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport void addSubCommunity(Community subCommunity) { - getSubcommunities().add(subCommunity); + subCommunities.add(subCommunity); setModified(); } void removeSubCommunity(Community subCommunity) { - getSubcommunities().remove(subCommunity); + subCommunities.remove(subCommunity); setModified(); } @@ -143,17 +144,21 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport */ public List getCollections() { - return collections; + // We return a copy because we do not want people to add elements to this collection directly. + // We return a list to maintain backwards compatibility + Collection[] output = collections.toArray(new Collection[]{}); + Arrays.sort(output, new NameAscendingComparator()); + return Arrays.asList(output); } void addCollection(Collection collection) { - getCollections().add(collection); + collections.add(collection); } void removeCollection(Collection collection) { - getCollections().remove(collection); + collections.remove(collection); } /** @@ -165,7 +170,11 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport */ public List getSubcommunities() { - return subCommunities; + // We return a copy because we do not want people to add elements to this collection directly. + // We return a list to maintain backwards compatibility + Community[] output = subCommunities.toArray(new Community[]{}); + Arrays.sort(output, new NameAscendingComparator()); + return Arrays.asList(output); } /** @@ -176,16 +185,25 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport */ public List getParentCommunities() { - return parentCommunities; + // We return a copy because we do not want people to add elements to this collection directly. + // We return a list to maintain backwards compatibility + Community[] output = parentCommunities.toArray(new Community[]{}); + Arrays.sort(output, new NameAscendingComparator()); + return Arrays.asList(output); } void addParentCommunity(Community parentCommunity) { - getParentCommunities().add(parentCommunity); + parentCommunities.add(parentCommunity); } void clearParentCommunities(){ - this.parentCommunities.clear(); - this.parentCommunities = null; + parentCommunities.clear(); + } + + public void removeParentCommunity(Community parentCommunity) + { + parentCommunities.remove(parentCommunity); + setModified(); } /** 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 0002871a0a..f93e6d441c 100644 --- a/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java @@ -455,7 +455,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl imp rawDelete(context, childCommunity); - childCommunity.getParentCommunities().remove(parentCommunity); + childCommunity.removeParentCommunity(parentCommunity); parentCommunity.removeSubCommunity(childCommunity); log.info(LogManager.getHeader(context, "remove_subcommunity", @@ -492,7 +492,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl imp Iterator subcommunities = community.getSubcommunities().iterator(); while (subcommunities.hasNext()) { Community subCommunity = subcommunities.next(); - subcommunities.remove(); + community.removeSubCommunity(subCommunity); delete(context, subCommunity); } // now let the parent remove the community @@ -535,7 +535,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl imp while (collections.hasNext()) { Collection collection = collections.next(); - collections.remove(); + community.removeCollection(collection); removeCollection(context, community, collection); } // delete subcommunities @@ -544,7 +544,7 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl imp while (subCommunities.hasNext()) { Community subComm = subCommunities.next(); - subCommunities.remove(); + community.removeSubCommunity(subComm); delete(context, subComm); } diff --git a/dspace-api/src/main/java/org/dspace/content/Item.java b/dspace-api/src/main/java/org/dspace/content/Item.java index 3322ac5627..49ab4b8caa 100644 --- a/dspace-api/src/main/java/org/dspace/content/Item.java +++ b/dspace-api/src/main/java/org/dspace/content/Item.java @@ -7,17 +7,18 @@ */ package org.dspace.content; +import org.dspace.content.comparator.NameAscendingComparator; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.ItemService; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.eperson.EPerson; +import org.hibernate.annotations.Sort; +import org.hibernate.annotations.SortType; import org.hibernate.proxy.HibernateProxyHelper; import javax.persistence.*; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; +import java.util.*; /** * Class representing an item in DSpace. @@ -78,7 +79,7 @@ public class Item extends DSpaceObject implements DSpaceObjectLegacySupport joinColumns = {@JoinColumn(name = "item_id") }, inverseJoinColumns = {@JoinColumn(name = "collection_id") } ) - private final List collections = new ArrayList<>(); + private final Set collections = new HashSet<>(); @ManyToMany(fetch = FetchType.LAZY, mappedBy = "items") private final List bundles = new ArrayList<>(); @@ -224,23 +225,31 @@ public class Item extends DSpaceObject implements DSpaceObjectLegacySupport } /** - * Get the collections this item is in. The order is indeterminate. + * Get the collections this item is in. The order is sorted ascending by collection name. * * @return the collections this item is in, if any. */ public List getCollections() { - return collections; + // We return a copy because we do not want people to add elements to this collection directly. + // We return a list to maintain backwards compatibility + Collection[] output = collections.toArray(new Collection[]{}); + Arrays.sort(output, new NameAscendingComparator()); + return Arrays.asList(output); } void addCollection(Collection collection) { - getCollections().add(collection); + collections.add(collection); } void removeCollection(Collection collection) { - getCollections().remove(collection); + collections.remove(collection); + } + + public void clearCollections(){ + collections.clear(); } public Collection getTemplateItemOf() { diff --git a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java index e19b4083e1..72e63744c9 100644 --- a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java @@ -651,7 +651,7 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl implements It } //Only clear collections after we have removed everything else from the item - item.getCollections().clear(); + item.clearCollections(); item.setOwningCollection(null); // Finally remove item row diff --git a/dspace-api/src/main/java/org/dspace/content/comparator/NameAscendingComparator.java b/dspace-api/src/main/java/org/dspace/content/comparator/NameAscendingComparator.java new file mode 100644 index 0000000000..bb78300e55 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/content/comparator/NameAscendingComparator.java @@ -0,0 +1,39 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.content.comparator; + +import org.apache.commons.lang.ObjectUtils; +import org.apache.commons.lang.StringUtils; +import org.dspace.content.DSpaceObject; + +import java.util.Comparator; + +public class NameAscendingComparator implements Comparator{ + + @Override + public int compare(DSpaceObject dso1, DSpaceObject dso2) { + if (dso1 == dso2){ + return 0; + }else if (dso1 == null){ + return -1; + }else if (dso2 == null){ + return 1; + }else { + String name1 = StringUtils.trimToEmpty(dso1.getName()); + String name2 = StringUtils.trimToEmpty(dso2.getName()); + + //When two DSO's have the same name, use their UUID to put them in an order + if(name1.equals(name2)) { + return ObjectUtils.compare(dso1.getID(), dso2.getID()); + } else { + return name1.compareToIgnoreCase(name2); + } + } + } + +} \ No newline at end of file diff --git a/dspace-api/src/main/java/org/dspace/core/Context.java b/dspace-api/src/main/java/org/dspace/core/Context.java index d8d395f536..bdb29ceb93 100644 --- a/dspace-api/src/main/java/org/dspace/core/Context.java +++ b/dspace-api/src/main/java/org/dspace/core/Context.java @@ -9,8 +9,7 @@ package org.dspace.core; import org.apache.log4j.Logger; import org.dspace.authorize.ResourcePolicy; -import org.dspace.content.*; -import org.dspace.content.Collection; +import org.dspace.content.DSpaceObject; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; @@ -692,8 +691,14 @@ public class Context log.warn("Unable to set database connection mode", ex); } + //Always clear the cache, except when going from READ_ONLY to READ_ONLY + if(mode != Mode.READ_ONLY || newMode != Mode.READ_ONLY) { + //clear our read-only cache to prevent any inconsistencies + readOnlyCache.clear(); + } + //save the new mode - this.mode = newMode; + mode = newMode; } /** diff --git a/dspace-api/src/main/java/org/dspace/core/ContextReadOnlyCache.java b/dspace-api/src/main/java/org/dspace/core/ContextReadOnlyCache.java index f3c2617da6..4eba1b1b1f 100644 --- a/dspace-api/src/main/java/org/dspace/core/ContextReadOnlyCache.java +++ b/dspace-api/src/main/java/org/dspace/core/ContextReadOnlyCache.java @@ -79,6 +79,12 @@ public class ContextReadOnlyCache { return allMemberGroupsCache.get(buildAllMembersGroupKey(ePerson)); } + public void clear() { + authorizedActionsCache.clear(); + groupMembershipCache.clear(); + allMemberGroupsCache.clear(); + } + private String buildAllMembersGroupKey(EPerson ePerson) { return ePerson == null ? "" : ePerson.getID().toString(); } @@ -93,4 +99,5 @@ public class ContextReadOnlyCache { return new ImmutablePair<>(group == null ? "" : group.getName(), eperson == null ? "" : eperson.getID().toString()); } + } 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 47248960e8..87108fb49e 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java @@ -8,6 +8,7 @@ package org.dspace.eperson; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.dspace.authorize.AuthorizeConfiguration; @@ -157,9 +158,19 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements } @Override - public boolean isMember(Context context, Group group) throws SQLException { - EPerson currentUser = context.getCurrentUser(); + public boolean isParentOf(Context context, Group parentGroup, Group childGroup) throws SQLException { + return group2GroupCacheDAO.findByParentAndChild(context, parentGroup, childGroup) != null; + } + @Override + public boolean isMember(Context context, Group group) throws SQLException { + return isMember(context, context.getCurrentUser(), group); + } + + @Override + public boolean isMember(Context context, EPerson ePerson, Group group) + throws SQLException + { if(group == null) { return false; @@ -167,34 +178,59 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements } else if (StringUtils.equals(group.getName(), Group.ANONYMOUS)) { return true; - } else if(currentUser != null) { + } else { + Boolean cachedGroupMembership = context.getCachedGroupMembership(group, ePerson); - Boolean cachedGroupMembership = context.getCachedGroupMembership(group, currentUser); if(cachedGroupMembership != null) { return cachedGroupMembership.booleanValue(); - } else if(CollectionUtils.isNotEmpty(context.getSpecialGroups())) { - Set allMemberGroups = allMemberGroupsSet(context, currentUser); - boolean result = allMemberGroups.contains(group); - - context.cacheGroupMembership(group, currentUser, result); - return result; } else { - //lookup eperson in normal groups and subgroups - boolean result = epersonInGroup(context, group.getName(), currentUser); - context.cacheGroupMembership(group, currentUser, result); - return result; + boolean isMember = false; + + //If we have an ePerson, check we can find membership in the database + if(ePerson != null) { + //lookup eperson in normal groups and subgroups with 1 query + isMember = isEPersonInGroup(context, group.getName(), ePerson); + } + + //If we did not find the group membership in the database, check the special groups. + //If there are special groups we need to check direct membership or check if the + //special group is a subgroup of the provided group. + //Note that special groups should only be checked if the current user == the ePerson. + //This also works for anonymous users (ePerson == null) if IP authentication used + if(!isMember && CollectionUtils.isNotEmpty(context.getSpecialGroups()) && isAuthenticatedUser(context, ePerson)) { + Iterator it = context.getSpecialGroups().iterator(); + + while (it.hasNext() && !isMember) { + Group specialGroup = it.next(); + //Check if the special group matches the given group or if it is a subgroup (with 1 query) + if (specialGroup.equals(group) || isParentOf(context, group, specialGroup)) { + isMember = true; + } + } + } + + context.cacheGroupMembership(group, ePerson, isMember); + return isMember; + } - } else { - return false; } } + private boolean isAuthenticatedUser(final Context context, final EPerson ePerson) { + return ObjectUtils.equals(context.getCurrentUser(), ePerson); + } + @Override public boolean isMember(final Context context, final String groupName) throws SQLException { return isMember(context, findByName(context, groupName)); } + @Override + public boolean isMember(final Context context, EPerson eperson, final String groupName) throws SQLException { + return isMember(context, eperson, findByName(context, groupName)); + } + @Override public List allMemberGroups(Context context, EPerson ePerson) throws SQLException { return new ArrayList<>(allMemberGroupsSet(context, ePerson)); @@ -478,7 +514,7 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements - protected boolean epersonInGroup(Context context, String groupName, EPerson ePerson) + protected boolean isEPersonInGroup(Context context, String groupName, EPerson ePerson) throws SQLException { return groupDAO.findByNameAndMembership(context, groupName, ePerson) != null; diff --git a/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java b/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java index 9fc0422dca..8532d67c0c 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java +++ b/dspace-api/src/main/java/org/dspace/eperson/dao/Group2GroupCacheDAO.java @@ -14,7 +14,6 @@ import org.dspace.eperson.Group2GroupCache; import java.sql.SQLException; import java.util.List; -import java.util.Set; /** * Database Access Object interface class for the Group2GroupCache object. @@ -27,7 +26,9 @@ public interface Group2GroupCacheDAO extends GenericDAO { public List findByParent(Context context, Group group) throws SQLException; - public List findByChildren(Context context, Set groups) throws SQLException; + public List findByChildren(Context context, Iterable groups) throws SQLException; + + public Group2GroupCache findByParentAndChild(Context context, Group parent, Group child) throws SQLException; public Group2GroupCache find(Context context, Group parent, Group child) throws SQLException; 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 a27689d75e..7c724819a3 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 @@ -13,6 +13,7 @@ import org.dspace.eperson.Group; import org.dspace.eperson.Group2GroupCache; import org.dspace.eperson.dao.Group2GroupCacheDAO; import org.hibernate.Criteria; +import org.hibernate.Query; import org.hibernate.criterion.Disjunction; import org.hibernate.criterion.Restrictions; @@ -44,7 +45,7 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO findByChildren(Context context, Set groups) throws SQLException { + public List findByChildren(Context context, Iterable groups) throws SQLException { Criteria criteria = createCriteria(context, Group2GroupCache.class); Disjunction orDisjunction = Restrictions.or(); @@ -59,6 +60,19 @@ public class Group2GroupCacheDAOImpl extends AbstractHibernateDAO implements Grou "(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 " + + "JOIN gc.parent parent " + + "JOIN gc.child child " + + "JOIN child.epeople cp " + + "WHERE parent.id = g.id AND cp.id = :eperson_id " + ") " + ")"); 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 567a5d26cf..0a4b2c58a1 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 @@ -7,10 +7,6 @@ */ package org.dspace.eperson.service; -import java.sql.SQLException; -import java.util.List; -import java.util.Set; - import org.dspace.authorize.AuthorizeException; import org.dspace.content.MetadataField; import org.dspace.content.service.DSpaceObjectLegacySupportService; @@ -19,6 +15,10 @@ import org.dspace.core.Context; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; +import java.sql.SQLException; +import java.util.List; +import java.util.Set; + /** * Service interface class for the Group object. * The implementation of this class is responsible for all business logic calls for the Group object and is autowired by spring @@ -117,6 +117,15 @@ public interface GroupService extends DSpaceObjectService, DSpaceObjectLe */ public boolean isMember(Group owningGroup, Group childGroup); + /** + * Check to see if parentGroup is a direct or in-direct parent of a childGroup. + * + * @param parentGroup parent group + * @param childGroup child group + * @return true or false + */ + public boolean isParentOf(Context context, Group parentGroup, Group childGroup) throws SQLException; + /** * fast check to see if an eperson is a member called with eperson id, does * database lookup without instantiating all of the epeople objects and is @@ -134,7 +143,8 @@ public interface GroupService extends DSpaceObjectService, DSpaceObjectLe /** * fast check to see if an eperson is a member called with eperson id, does * database lookup without instantiating all of the epeople objects and is - * thus a static method + * thus a static method. This method uses context.getCurrentUser() as + * eperson whos membership should be checked. * * @param context * context @@ -145,6 +155,34 @@ public interface GroupService extends DSpaceObjectService, DSpaceObjectLe */ public boolean isMember(Context context, String groupName) throws SQLException; + /** + * fast check to see if an eperson is a member called with eperson id, does + * database lookup without instantiating all of the epeople objects and is + * thus a static method. The eperson whos membership should be checked must + * be defined as method attribute. + * + * @param context + * context + * @param groupName + * the name of the group to check + * @return true or false + * @throws SQLException if database error + */ + public boolean isMember(Context context, EPerson epersonToCheck, String groupName) throws SQLException; + + /** + * fast check to see if an eperson is a member called with eperson id, does + * database lookup without instantiating all of the epeople objects and is + * thus a static method + * + * @param context DSpace context object. + * @param eperson EPerson whos membership should be checked. + * @param group The group to check against. + * @return true or false + * @throws SQLException if database error + */ + public boolean isMember(Context context, EPerson eperson, Group group) throws SQLException; + /** * Get all of the groups that an eperson is a member of. * diff --git a/dspace-api/src/main/java/org/dspace/statistics/content/StatisticsDataVisits.java b/dspace-api/src/main/java/org/dspace/statistics/content/StatisticsDataVisits.java index 20db4abb45..3be57e83c4 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/content/StatisticsDataVisits.java +++ b/dspace-api/src/main/java/org/dspace/statistics/content/StatisticsDataVisits.java @@ -511,7 +511,7 @@ public class StatisticsDataVisits extends StatisticsData { break; } - return value; + return bit.getName(); case Constants.ITEM: Item item = itemService.findByIdOrLegacyId(context, dsoId); if(item == null) diff --git a/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactory.java b/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactory.java index 8742de480a..2591d59097 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactory.java +++ b/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactory.java @@ -10,6 +10,8 @@ package org.dspace.statistics.factory; import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.statistics.service.ElasticSearchLoggerService; import org.dspace.statistics.service.SolrLoggerService; +import org.dspace.statistics.util.SpiderDetector; +import org.dspace.statistics.util.SpiderDetectorService; /** * Abstract factory to get services for the statistics package, use StatisticsServiceFactory.getInstance() to retrieve an implementation @@ -22,6 +24,8 @@ public abstract class StatisticsServiceFactory { public abstract ElasticSearchLoggerService getElasticSearchLoggerService(); + public abstract SpiderDetectorService getSpiderDetectorService(); + public static StatisticsServiceFactory getInstance() { return DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName("statisticsServiceFactory", StatisticsServiceFactory.class); diff --git a/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactoryImpl.java b/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactoryImpl.java index 1ff7163193..d4e80bfb44 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactoryImpl.java +++ b/dspace-api/src/main/java/org/dspace/statistics/factory/StatisticsServiceFactoryImpl.java @@ -10,6 +10,8 @@ package org.dspace.statistics.factory; import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.statistics.service.ElasticSearchLoggerService; import org.dspace.statistics.service.SolrLoggerService; +import org.dspace.statistics.util.SpiderDetector; +import org.dspace.statistics.util.SpiderDetectorService; /** * Factory implementation to get services for the statistics package, use StatisticsServiceFactory.getInstance() to retrieve an implementation @@ -29,4 +31,9 @@ public class StatisticsServiceFactoryImpl extends StatisticsServiceFactory { // In order to lazy load, we cannot autowire it and instead load it by name return DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName("elasticSearchLoggerService", ElasticSearchLoggerService.class); } + + @Override + public SpiderDetectorService getSpiderDetectorService() { + return DSpaceServicesFactory.getInstance().getServiceManager().getServiceByName("spiderDetectorService", SpiderDetectorService.class); + } } diff --git a/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetector.java b/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetector.java index e508d03618..b2cc12a453 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetector.java +++ b/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetector.java @@ -7,48 +7,40 @@ */ package org.dspace.statistics.util; -import java.io.BufferedReader; import java.io.File; -import java.io.FileReader; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; import java.util.Set; -import java.util.Collections; -import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; -import org.dspace.services.factory.DSpaceServicesFactory; + +import org.dspace.statistics.factory.StatisticsServiceFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * SpiderDetector is used to find IP's that are spiders... - * In future someone may add Host Domains - * to the detection criteria here. + * SpiderDetector delegates static methods to SpiderDetectorService, which is used to find IP's that are spiders... * * @author kevinvandevelde at atmire.com * @author ben at atmire.com * @author Mark Diggory (mdiggory at atmire.com) + * @author frederic at atmire.com */ public class SpiderDetector { private static final Logger log = LoggerFactory.getLogger(SpiderDetector.class); - private static Boolean useProxies; + //Service where all methods get delegated to, this is instantiated by a spring-bean defined in core-services.xml + private static SpiderDetectorService spiderDetectorService = StatisticsServiceFactory.getInstance().getSpiderDetectorService(); /** - * Sparse HashTable structure to hold IP address ranges. + * Get an immutable Set representing all the Spider Addresses here + * + * @return a set of IP addresses as strings */ - private static IPTable table = null; + public static Set getSpiderIpAddresses() { - /** Collection of regular expressions to match known spiders' agents. */ - private static final List agents - = Collections.synchronizedList(new ArrayList()); - - /** Collection of regular expressions to match known spiders' domain names. */ - private static final List domains - = Collections.synchronizedList(new ArrayList()); + spiderDetectorService.loadSpiderIpAddresses(); + return spiderDetectorService.getTable().toSet(); + } /** * Utility method which reads lines from a file & returns them in a Set. @@ -60,130 +52,7 @@ public class SpiderDetector { public static Set readPatterns(File patternFile) throws IOException { - Set patterns = new HashSet<>(); - - if (!patternFile.exists() || !patternFile.isFile()) - { - return patterns; - } - - //Read our file & get all them patterns. - try (BufferedReader in = new BufferedReader(new FileReader(patternFile))) - { - String line; - while ((line = in.readLine()) != null) { - if (!line.startsWith("#")) { - line = line.trim(); - - if (!line.equals("")) { - patterns.add(line); - } - } else { - // ua.add(line.replaceFirst("#","").replaceFirst("UA","").trim()); - // ... add this functionality later - } - } - } - return patterns; - } - - /** - * Get an immutable Set representing all the Spider Addresses here - * - * @return a set of IP addresses as strings - */ - public static Set getSpiderIpAddresses() { - - loadSpiderIpAddresses(); - return table.toSet(); - } - - /* - * private loader to populate the table from files. - */ - - private synchronized static void loadSpiderIpAddresses() { - - if (table == null) { - table = new IPTable(); - - String filePath = DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.dir"); - - try { - File spidersDir = new File(filePath, "config/spiders"); - - if (spidersDir.exists() && spidersDir.isDirectory()) { - for (File file : spidersDir.listFiles()) { - if (file.isFile()) - { - for (String ip : readPatterns(file)) { - log.debug("Loading {}", ip); - if (!Character.isDigit(ip.charAt(0))) - { - try { - ip = DnsLookup.forward(ip); - log.debug("Resolved to {}", ip); - } catch (IOException e) { - log.warn("Not loading {}: {}", ip, e.getMessage()); - continue; - } - } - table.add(ip); - } - log.info("Loaded Spider IP file: " + file); - } - } - } else { - log.info("No spider file loaded"); - } - } - catch (IOException | IPTable.IPFormatException e) { - log.error("Error Loading Spiders:" + e.getMessage(), e); - } - - } - - } - - /** - * Load agent name patterns from all files in a single subdirectory of config/spiders. - * - * @param directory simple directory name (e.g. "agents"). - * "${dspace.dir}/config/spiders" will be prepended to yield the path to - * the directory of pattern files. - * @param patternList patterns read from the files in {@code directory} will - * be added to this List. - */ - private static void loadPatterns(String directory, List patternList) - { - String dspaceHome = DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.dir"); - File spidersDir = new File(dspaceHome, "config/spiders"); - File patternsDir = new File(spidersDir, directory); - if (patternsDir.exists() && patternsDir.isDirectory()) - { - for (File file : patternsDir.listFiles()) - { - Set patterns; - try - { - patterns = readPatterns(file); - } catch (IOException ex) - { - log.error("Patterns not read from {}: {}", - file.getPath(), ex.getMessage()); - continue; - } - for (String pattern : patterns) - { - patternList.add(Pattern.compile(pattern,Pattern.CASE_INSENSITIVE)); - } - log.info("Loaded pattern file: {}", file.getPath()); - } - } - else - { - log.info("No patterns loaded from {}", patternsDir.getPath()); - } + return spiderDetectorService.readPatterns(patternFile); } /** @@ -199,60 +68,9 @@ public class SpiderDetector { * @return true if the client matches any spider characteristics list. */ public static boolean isSpider(String clientIP, String proxyIPs, - String hostname, String agent) + String hostname, String agent) { - // See if any agent patterns match - if (null != agent) - { - synchronized(agents) - { - if (agents.isEmpty()) - loadPatterns("agents", agents); - } - for (Pattern candidate : agents) - { - // prevent matcher() invocation from a null Pattern object - if (null != candidate && candidate.matcher(agent).find()) - { - return true; - } - } - } - - // No. See if any IP addresses match - if (isUseProxies() && proxyIPs != null) { - /* This header is a comma delimited list */ - for (String xfip : proxyIPs.split(",")) { - if (isSpider(xfip)) - { - return true; - } - } - } - - if (isSpider(clientIP)) - return true; - - // No. See if any DNS names match - if (null != hostname) - { - synchronized(domains) - { - if (domains.isEmpty()) - loadPatterns("domains", domains); - } - for (Pattern candidate : domains) - { - // prevent matcher() invocation from a null Pattern object - if (null != candidate && candidate.matcher(hostname).find()) - { - return true; - } - } - } - - // Not a known spider. - return false; + return spiderDetectorService.isSpider(clientIP, proxyIPs, hostname, agent); } /** @@ -263,10 +81,7 @@ public class SpiderDetector { */ public static boolean isSpider(HttpServletRequest request) { - return isSpider(request.getRemoteAddr(), - request.getHeader("X-Forwarded-For"), - request.getRemoteHost(), - request.getHeader("User-Agent")); + return spiderDetectorService.isSpider(request); } /** @@ -276,30 +91,7 @@ public class SpiderDetector { * @return if is spider IP */ public static boolean isSpider(String ip) { - - if (table == null) { - SpiderDetector.loadSpiderIpAddresses(); - } - - try { - if (table.contains(ip)) { - return true; - } - } catch (Exception e) { - return false; - } - - return false; - - - } - - private static boolean isUseProxies() { - if(useProxies == null) { - useProxies = DSpaceServicesFactory.getInstance().getConfigurationService().getBooleanProperty("useProxies"); - } - - return useProxies; + return spiderDetectorService.isSpider(ip); } } diff --git a/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorService.java b/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorService.java new file mode 100644 index 0000000000..8b8f7dffee --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorService.java @@ -0,0 +1,34 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.statistics.util; + +import javax.servlet.http.HttpServletRequest; +import java.io.File; +import java.io.IOException; +import java.util.Set; + +/** + * Interface to implement a SpiderDetectorService + * @author frederic at atmire.com + */ +public interface SpiderDetectorService { + + public boolean isSpider(String clientIP, String proxyIPs, String hostname, String agent); + + public boolean isSpider(HttpServletRequest request); + + public boolean isSpider(String ip); + + public void loadSpiderIpAddresses(); + + public Set readPatterns(File patternFile) + throws IOException; + + public IPTable getTable(); + +} diff --git a/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorServiceImpl.java b/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorServiceImpl.java new file mode 100644 index 0000000000..184b7d1890 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/statistics/util/SpiderDetectorServiceImpl.java @@ -0,0 +1,329 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.statistics.util; + +import org.apache.commons.configuration.ConversionException; +import org.apache.commons.lang.StringUtils; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; + +import javax.servlet.http.HttpServletRequest; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.util.*; +import java.util.regex.Pattern; + +/** + * SpiderDetectorServiceImpl is used to find IP's that are spiders... + * In future someone may add Host Domains + * to the detection criteria here. + * + * @author kevinvandevelde at atmire.com + * @author ben at atmire.com + * @author Mark Diggory (mdiggory at atmire.com) + * @author frederic at atmire.com + */ +public class SpiderDetectorServiceImpl implements SpiderDetectorService { + + private static final Logger log = LoggerFactory.getLogger(SpiderDetectorServiceImpl.class); + + private Boolean useProxies; + + private Boolean useCaseInsensitiveMatching; + + private final List agents + = Collections.synchronizedList(new ArrayList()); + + private final List domains + = Collections.synchronizedList(new ArrayList()); + + private ConfigurationService configurationService; + + /** + * Sparse HashTable structure to hold IP address ranges. + */ + private IPTable table = null; + + @Autowired(required = true) + public SpiderDetectorServiceImpl(ConfigurationService configurationService) { + this.configurationService = configurationService; + } + + public IPTable getTable() { + return table; + } + + /** + * Service Method for testing spiders against existing spider files. + *

+ * In future spiders HashSet may be optimized as byte offset array to + * improve performance and memory footprint further. + * + * @param clientIP address of the client. + * @param proxyIPs comma-list of X-Forwarded-For addresses, or null. + * @param hostname domain name of host, or null. + * @param agent User-Agent header value, or null. + * @return true if the client matches any spider characteristics list. + */ + public boolean isSpider(String clientIP, String proxyIPs, String hostname, String agent) { + // See if any agent patterns match + if (null != agent) + { + synchronized(agents) + { + if (agents.isEmpty()) + loadPatterns("agents", agents); + } + + if(isUseCaseInsensitiveMatching()) { + agent = StringUtils.lowerCase(agent); + hostname = StringUtils.lowerCase(hostname); + } + + for (Pattern candidate : agents) { + + // prevent matcher() invocation from a null Pattern object + if (null != candidate && candidate.matcher(agent).find()) { + return true; + } + + + } + } + + // No. See if any IP addresses match + if (isUseProxies() && proxyIPs != null) { + /* This header is a comma delimited list */ + for (String xfip : proxyIPs.split(",")) { + if (isSpider(xfip)) + { + return true; + } + } + } + + if (isSpider(clientIP)) + return true; + + // No. See if any DNS names match + if (null != hostname) + { + synchronized(domains) + { + if (domains.isEmpty()) + loadPatterns("domains", domains); + } + for (Pattern candidate : domains) + { + // prevent matcher() invocation from a null Pattern object + if (null != candidate && candidate.matcher(hostname).find()) + { + return true; + } + } + } + + // Not a known spider. + return false; + } + + /** + * Utility method which reads lines from a file & returns them in a Set. + * + * @param patternFile the location of our spider file + * @return a vector full of patterns + * @throws IOException could not happen since we check the file be4 we use it + */ + public Set readPatterns(File patternFile) + throws IOException + { + Set patterns = new HashSet<>(); + + if (!patternFile.exists() || !patternFile.isFile()) + { + return patterns; + } + + //Read our file & get all them patterns. + try (BufferedReader in = new BufferedReader(new FileReader(patternFile))) + { + String line; + while ((line = in.readLine()) != null) { + if (!line.startsWith("#")) { + line = line.trim(); + + if (!line.equals("")) { + patterns.add(line); + } + } else { + // ua.add(line.replaceFirst("#","").replaceFirst("UA","").trim()); + // ... add this functionality later + } + } + } + return patterns; + } + + /** + * Load agent name patterns from all files in a single subdirectory of config/spiders. + * + * @param directory simple directory name (e.g. "agents"). + * "${dspace.dir}/config/spiders" will be prepended to yield the path to + * the directory of pattern files. + * @param patternList patterns read from the files in {@code directory} will + * be added to this List. + */ + private void loadPatterns(String directory, List patternList) + { + String dspaceHome = configurationService.getProperty("dspace.dir"); + File spidersDir = new File(dspaceHome, "config/spiders"); + File patternsDir = new File(spidersDir, directory); + if (patternsDir.exists() && patternsDir.isDirectory()) + { + for (File file : patternsDir.listFiles()) + { + Set patterns; + try + { + patterns = readPatterns(file); + } catch (IOException ex) + { + log.error("Patterns not read from {}: {}", + file.getPath(), ex.getMessage()); + continue; + } + //If case insensitive matching is enabled, lowercase the patterns so they can be lowercase matched + for (String pattern : patterns) { + if(isUseCaseInsensitiveMatching()) { + pattern = StringUtils.lowerCase(pattern); + } + patternList.add(Pattern.compile(pattern)); + } + + + log.info("Loaded pattern file: {}", file.getPath()); + } + } + else + { + log.info("No patterns loaded from {}", patternsDir.getPath()); + } + } + + /** + * Service Method for testing spiders against existing spider files. + * + * @param request + * @return true|false if the request was detected to be from a spider. + */ + public boolean isSpider(HttpServletRequest request) { + return isSpider(request.getRemoteAddr(), + request.getHeader("X-Forwarded-For"), + request.getRemoteHost(), + request.getHeader("User-Agent")); + } + + /** + * Check individual IP is a spider. + * + * @param ip + * @return if is spider IP + */ + public boolean isSpider(String ip) { + if (table == null) { + loadSpiderIpAddresses(); + } + + try { + if (table.contains(ip)) { + return true; + } + } catch (Exception e) { + return false; + } + + return false; + } + + /* + * loader to populate the table from files. + */ + public synchronized void loadSpiderIpAddresses() { + + if (table == null) { + table = new IPTable(); + + String filePath = configurationService.getProperty("dspace.dir"); + + try { + File spidersDir = new File(filePath, "config/spiders"); + + if (spidersDir.exists() && spidersDir.isDirectory()) { + for (File file : spidersDir.listFiles()) { + if (file.isFile()) + { + for (String ip : readPatterns(file)) { + log.debug("Loading {}", ip); + if (!Character.isDigit(ip.charAt(0))) + { + try { + ip = DnsLookup.forward(ip); + log.debug("Resolved to {}", ip); + } catch (IOException e) { + log.warn("Not loading {}: {}", ip, e.getMessage()); + continue; + } + } + table.add(ip); + } + log.info("Loaded Spider IP file: " + file); + } + } + } else { + log.info("No spider file loaded"); + } + } + catch (IOException | IPTable.IPFormatException e) { + log.error("Error Loading Spiders:" + e.getMessage(), e); + } + + } + + } + + /** + * checks if case insensitive matching is enabled + * @return true if it's enabled, false if not + */ + private boolean isUseCaseInsensitiveMatching() { + if (useCaseInsensitiveMatching == null) { + try { + useCaseInsensitiveMatching = configurationService.getBooleanProperty("usage-statistics.bots.case-insensitive"); + } catch (ConversionException e) { + useCaseInsensitiveMatching = false; + log.warn("Please use a boolean value for usage-statistics.bots.case-insensitive"); + } + } + + return useCaseInsensitiveMatching; + } + + private boolean isUseProxies() { + if(useProxies == null) { + useProxies = configurationService.getBooleanProperty("useProxies"); + } + + return useProxies; + } + +} diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql new file mode 100644 index 0000000000..57d6e7a55f --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql @@ -0,0 +1,16 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +------------------------------------------------------ +-- DS-3563 Missing database index on metadatavalue.resource_type_id +------------------------------------------------------ +-- Create an index on the metadata value resource_type_id column so that it can be searched efficiently. + +DROP INDEX IF EXISTS metadatavalue_resource_type_id_idx; + +CREATE INDEX metadatavalue_resource_type_id_idx ON metadatavalue (resource_type_id); diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql new file mode 100644 index 0000000000..6279154838 --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql @@ -0,0 +1,23 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +------------------------------------------------------ +-- DS-3563 Missing database index on metadatavalue.resource_type_id +------------------------------------------------------ +-- Create an index on the metadata value resource_type_id column so that it can be searched efficiently. +declare + index_not_exists EXCEPTION; + PRAGMA EXCEPTION_INIT(index_not_exists, -1418); +begin + + execute immediate 'DROP INDEX metadatavalue_type_id_idx'; + exception + when index_not_exists then null; +end; + +CREATE INDEX metadatavalue_type_id_idx ON metadatavalue (resource_type_id); \ No newline at end of file diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql new file mode 100644 index 0000000000..57d6e7a55f --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V5.7_2017.04.11__DS-3563_Index_metadatavalue_resource_type_id_column.sql @@ -0,0 +1,16 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +------------------------------------------------------ +-- DS-3563 Missing database index on metadatavalue.resource_type_id +------------------------------------------------------ +-- Create an index on the metadata value resource_type_id column so that it can be searched efficiently. + +DROP INDEX IF EXISTS metadatavalue_resource_type_id_idx; + +CREATE INDEX metadatavalue_resource_type_id_idx ON metadatavalue (resource_type_id); diff --git a/dspace-api/src/test/java/org/dspace/authorize/AuthorizeServiceTest.java b/dspace-api/src/test/java/org/dspace/authorize/AuthorizeServiceTest.java new file mode 100644 index 0000000000..527207b1a5 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/authorize/AuthorizeServiceTest.java @@ -0,0 +1,145 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ + +package org.dspace.authorize; + +import org.dspace.AbstractUnitTest; +import org.dspace.authorize.factory.AuthorizeServiceFactory; +import org.dspace.authorize.service.ResourcePolicyService; +import org.dspace.content.Community; +import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.CommunityService; +import org.dspace.core.Constants; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.dspace.eperson.factory.EPersonServiceFactory; +import org.dspace.eperson.service.EPersonService; +import org.dspace.eperson.service.GroupService; +import org.junit.Assert; +import org.junit.Test; + +import java.sql.SQLException; + +/** + * Created by pbecker as he wanted to write a test against DS-3572. + * This definitely needs to be extended, but it's at least a start. + */ +public class AuthorizeServiceTest extends AbstractUnitTest +{ + + protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); + protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + protected ResourcePolicyService resourcePolicyService = AuthorizeServiceFactory.getInstance().getResourcePolicyService(); + protected CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); + + public AuthorizeServiceTest() + {} + + @Test + public void testauthorizeMethodDoesNotConfuseEPersonWithCurrentUser() + { + Community dso; + EPerson eperson1; + EPerson eperson2; + Group group; + + try + { + context.turnOffAuthorisationSystem(); + + // create two epersons: one to test a permission the other one to be used as currentUser + eperson1 = ePersonService.create(context); + eperson2 = ePersonService.create(context); + // create a group as the bug described in DS-3572 contains a wrong group membership check + group = groupService.create(context); + // A group has to have a name, otherwise there are queries that break + groupService.setName(group, "My test group"); + // add eperson1 to the group. + groupService.addMember(context, group, eperson1); + groupService.update(context, group); + + // Use a top level community as DSpaceObject to test permissions + dso = communityService.create(null, context); + + // grant write permission to the eperson1 by its group membership + authorizeService.addPolicy(context, dso, Constants.WRITE, group); + context.commit(); + + // set the other eperson as the current user + // Notice that it is not a member of the group, and does not have write permission + context.setCurrentUser(eperson2); + } + catch (SQLException | AuthorizeException ex) + { + throw new RuntimeException(ex); + } + finally + { + context.restoreAuthSystemState(); + } + + try { + // eperson1 should be able to write as he is member of a group that has write permissions + Assert.assertTrue(authorizeService.authorizeActionBoolean(context, eperson1, dso, Constants.WRITE, true)); + // person2 shouldn't have write access + Assert.assertFalse(authorizeService.authorizeActionBoolean(context, eperson2, dso, Constants.WRITE, true)); + } + catch (SQLException ex) + { + throw new RuntimeException(ex); + } + } + + @Test + public void testauthorizeMethodRespectSpecialGroups() + { + + EPerson eperson1; + EPerson eperson2; + Group group1; + + Community dso; + try + { + context.turnOffAuthorisationSystem(); + + // create an eperson and a group + eperson1 = ePersonService.create(context); + group1 = groupService.create(context); + // A group has to have a name, otherwise there are queries that break + groupService.setName(group1, "My test group 2"); + + // Use a top level community as DSpaceObject to test permissions + dso = communityService.create(null, context); + + // allow the group some action on a DSpaceObject and set it as + // special group to the user. Then test if the action on the DSO + // is allowed for the user + authorizeService.addPolicy(context, dso, Constants.ADD, group1); + context.setCurrentUser(eperson1); + context.setSpecialGroup(group1.getID()); + context.commit(); + } + catch (SQLException | AuthorizeException ex) + { + throw new RuntimeException(ex); + } + finally + { + context.restoreAuthSystemState(); + } + + try { + Assert.assertTrue(authorizeService.authorizeActionBoolean(context, eperson1, dso, Constants.ADD, true)); + } + catch (SQLException ex) + { + throw new RuntimeException(ex); + } + } +} diff --git a/dspace-api/src/test/java/org/dspace/checker/dao/impl/ChecksumHistoryDAOImplTest.java b/dspace-api/src/test/java/org/dspace/checker/dao/impl/ChecksumHistoryDAOImplTest.java index 63187fb3a8..10b824239a 100644 --- a/dspace-api/src/test/java/org/dspace/checker/dao/impl/ChecksumHistoryDAOImplTest.java +++ b/dspace-api/src/test/java/org/dspace/checker/dao/impl/ChecksumHistoryDAOImplTest.java @@ -97,7 +97,7 @@ public class ChecksumHistoryDAOImplTest qry.setInteger("id", checkId); qry.setDate("date", matchDate); qry.setString("result", ChecksumResultCode.CHECKSUM_MATCH.name()); - qry.setString("bitstream", bs.getID().toString()); // FIXME identifier not being set??? + qry.setParameter("bitstream", bs.getID()); // FIXME identifier not being set??? qry.executeUpdate(); // Row with nonmatching result code @@ -107,7 +107,7 @@ public class ChecksumHistoryDAOImplTest qry.setInteger("id", checkId); qry.setDate("date", noMatchDate); qry.setString("result", ChecksumResultCode.CHECKSUM_NO_MATCH.name()); - qry.setString("bitstream", bs.getID().toString()); + qry.setParameter("bitstream", bs.getID()); // FIXME identifier not being set??? qry.executeUpdate(); // Create one newer row @@ -117,7 +117,7 @@ public class ChecksumHistoryDAOImplTest qry.setInteger("id", checkId); qry.setDate("date", new java.sql.Date(futureDate.getTime())); qry.setString("result", ChecksumResultCode.CHECKSUM_MATCH.name()); - qry.setString("bitstream", bs.getID().toString()); + qry.setParameter("bitstream", bs.getID()); // FIXME identifier not being set??? qry.executeUpdate(); // Test! diff --git a/dspace-api/src/test/java/org/dspace/content/BundleTest.java b/dspace-api/src/test/java/org/dspace/content/BundleTest.java index 880f9ce4de..aebff76661 100644 --- a/dspace-api/src/test/java/org/dspace/content/BundleTest.java +++ b/dspace-api/src/test/java/org/dspace/content/BundleTest.java @@ -40,7 +40,7 @@ public class BundleTest extends AbstractDSpaceObjectTest { /** log4j category */ private static final Logger log = Logger.getLogger(BundleTest.class); - + /** * Bundle instance for the tests */ @@ -214,7 +214,7 @@ public class BundleTest extends AbstractDSpaceObjectTest * Test of getPrimaryBitstreamID method, of class Bundle. */ @Test - public void testGetPrimaryBitstreamID() + public void testGetPrimaryBitstreamID() { //is -1 when not set assertThat("testGetPrimaryBitstreamID 0", b.getPrimaryBitstream(), equalTo(null)); @@ -275,7 +275,7 @@ public class BundleTest extends AbstractDSpaceObjectTest */ @Override @Test - public void testGetHandle() + public void testGetHandle() { //no handle for bundles assertThat("testGetHandle 0", b.getHandle(), nullValue()); @@ -300,7 +300,7 @@ public class BundleTest extends AbstractDSpaceObjectTest String name = "name"; //by default there is no bitstream assertThat("testGetHandle 0", bundleService.getBitstreamByName(b, name), nullValue()); - + //let's add a bitstream File f = new File(testProps.get("test.bitstream").toString()); Bitstream bs = bitstreamService.create(context, new FileInputStream(f)); @@ -397,12 +397,12 @@ public class BundleTest extends AbstractDSpaceObjectTest assertThat("testCreateBitstreamAuth 1", bundleService.getBitstreamByName(b, name), equalTo(bs)); assertThat("testCreateBitstreamAuth 2", bundleService.getBitstreamByName(b, name).getName(), equalTo(name)); } - + /** * Test of registerBitstream method, of class Bundle. */ @Test(expected=AuthorizeException.class) - public void testRegisterBitstreamNoAuth() throws AuthorizeException, IOException, SQLException + public void testRegisterBitstreamNoAuth() throws AuthorizeException, IOException, SQLException { new NonStrictExpectations(authorizeService.getClass()) {{ @@ -422,7 +422,7 @@ public class BundleTest extends AbstractDSpaceObjectTest * Test of registerBitstream method, of class Bundle. */ @Test - public void testRegisterBitstreamAuth() throws AuthorizeException, IOException, SQLException + public void testRegisterBitstreamAuth() throws AuthorizeException, IOException, SQLException { new NonStrictExpectations(authorizeService.getClass()) {{ @@ -436,7 +436,7 @@ public class BundleTest extends AbstractDSpaceObjectTest int assetstore = 0; //default assetstore String name = "name bitstream"; - File f = new File(testProps.get("test.bitstream").toString()); + File f = new File(testProps.get("test.bitstream").toString()); Bitstream bs = bitstreamService.register(context, b, assetstore, f.getName()); bs.setName(context, name); assertThat("testRegisterBitstream 0", bundleService.getBitstreamByName(b, name), notNullValue()); @@ -481,7 +481,7 @@ public class BundleTest extends AbstractDSpaceObjectTest Constants.WRITE); result = null; }}; - + File f = new File(testProps.get("test.bitstream").toString()); Bitstream bs = bitstreamService.create(context, new FileInputStream(f)); bs.setName(context, "name"); @@ -546,7 +546,7 @@ public class BundleTest extends AbstractDSpaceObjectTest * Test of update method, of class Bundle. */ @Test - public void testUpdate() throws SQLException, AuthorizeException + public void testUpdate() throws SQLException, AuthorizeException { //TODO: we only check for sql errors //TODO: note that update can't throw authorize exception!! @@ -632,7 +632,7 @@ public class BundleTest extends AbstractDSpaceObjectTest } } assertTrue("testInheritCollectionDefaultPolicies 2", exists); - + } /** @@ -646,7 +646,7 @@ public class BundleTest extends AbstractDSpaceObjectTest newpolicies.add(resourcePolicyService.create(context)); newpolicies.add(resourcePolicyService.create(context)); bundleService.replaceAllBitstreamPolicies(context, b, newpolicies); - + List bspolicies = bundleService.getBundlePolicies(context, b); assertTrue("testReplaceAllBitstreamPolicies 0", newpolicies.size() == bspolicies.size()); @@ -694,6 +694,69 @@ public class BundleTest extends AbstractDSpaceObjectTest assertTrue("testGetBitstreamPolicies 0", bspolicies.isEmpty()); } + @Test + public void testSetOrder() throws SQLException, AuthorizeException, FileNotFoundException, IOException + { + new NonStrictExpectations(authorizeService.getClass()) + {{ + // Allow Bundle ADD perms + authorizeService.authorizeAction((Context) any, (Bundle) any, + Constants.ADD); result = null; + authorizeService.authorizeAction((Context) any, (Bitstream) any, + Constants.WRITE); result = null; + }}; + + // Create three Bitstreams to test ordering with. Give them different names + File f = new File(testProps.get("test.bitstream").toString()); + Bitstream bs = bitstreamService.create(context, new FileInputStream(f)); + bs.setName(context, "bitstream1"); + bundleService.addBitstream(context, b, bs); + Bitstream bs2 = bitstreamService.create(context, new FileInputStream(f)); + bs2.setName(context, "bitstream2"); + bundleService.addBitstream(context, b, bs2); + Bitstream bs3 = bitstreamService.create(context, new FileInputStream(f)); + bs3.setName(context, "bitstream3"); + bundleService.addBitstream(context, b, bs3); + + // Assert Bitstreams are in the order added + Bitstream[] bitstreams = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]); + assertTrue("testSetOrder: starting count correct", bitstreams.length == 3); + assertThat("testSetOrder: Bitstream 1 is first", bitstreams[0].getName(), equalTo(bs.getName())); + assertThat("testSetOrder: Bitstream 2 is second", bitstreams[1].getName(), equalTo(bs2.getName())); + assertThat("testSetOrder: Bitstream 3 is third", bitstreams[2].getName(), equalTo(bs3.getName())); + + UUID bsID1 = bs.getID(); + UUID bsID2 = bs2.getID(); + UUID bsID3 = bs3.getID(); + + // Now define a new order and call setOrder() + UUID[] newBitstreamOrder = new UUID[] { bsID3, bsID1, bsID2 }; + bundleService.setOrder(context, b, newBitstreamOrder); + + // Assert Bitstreams are in the new order + bitstreams = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]); + assertTrue("testSetOrder: new count correct", bitstreams.length == 3); + assertThat("testSetOrder: Bitstream 3 is now first", bitstreams[0].getName(), equalTo(bs3.getName())); + assertThat("testSetOrder: Bitstream 1 is now second", bitstreams[1].getName(), equalTo(bs.getName())); + assertThat("testSetOrder: Bitstream 2 is now third", bitstreams[2].getName(), equalTo(bs2.getName())); + + // Now give only a partial list of bitstreams + newBitstreamOrder = new UUID[] { bsID1, bsID2 }; + bundleService.setOrder(context, b, newBitstreamOrder); + + // Assert Bitstream order is unchanged + Bitstream[] bitstreamsAfterPartialData = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]); + assertThat("testSetOrder: Partial data doesn't change order", bitstreamsAfterPartialData, equalTo(bitstreams)); + + // Now give bad data in the list of bitstreams + newBitstreamOrder = new UUID[] { bsID1, null, bsID2 }; + bundleService.setOrder(context, b, newBitstreamOrder); + + // Assert Bitstream order is unchanged + Bitstream[] bitstreamsAfterBadData = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]); + assertThat("testSetOrder: Partial data doesn't change order", bitstreamsAfterBadData, equalTo(bitstreams)); + } + /** * Test of getAdminObject method, of class Bundle. */ diff --git a/dspace-api/src/test/java/org/dspace/content/CollectionTest.java b/dspace-api/src/test/java/org/dspace/content/CollectionTest.java index 6f9e705482..e6dfee2b95 100644 --- a/dspace-api/src/test/java/org/dspace/content/CollectionTest.java +++ b/dspace-api/src/test/java/org/dspace/content/CollectionTest.java @@ -7,6 +7,20 @@ */ package org.dspace.content; +import mockit.NonStrictExpectations; +import org.apache.log4j.Logger; +import org.dspace.app.util.AuthorizeUtil; +import org.dspace.authorize.AuthorizeException; +import org.dspace.core.Constants; +import org.dspace.core.Context; +import org.dspace.core.factory.CoreServiceFactory; +import org.dspace.core.service.LicenseService; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -15,20 +29,8 @@ import java.util.Iterator; import java.util.List; import java.util.UUID; - -import org.dspace.authorize.AuthorizeException; -import org.apache.log4j.Logger; -import org.dspace.core.Context; -import org.dspace.core.factory.CoreServiceFactory; -import org.dspace.core.service.LicenseService; -import org.dspace.eperson.EPerson; -import org.dspace.eperson.Group; -import org.junit.*; -import static org.junit.Assert.* ; import static org.hamcrest.CoreMatchers.*; -import mockit.NonStrictExpectations; -import org.dspace.app.util.AuthorizeUtil; -import org.dspace.core.Constants; +import static org.junit.Assert.*; /** * Unit Tests for class Collection @@ -1823,8 +1825,22 @@ public class CollectionTest extends AbstractDSpaceObjectTest @Test public void testGetCommunities() throws Exception { - assertThat("testGetCommunities 0",collection.getCommunities(), notNullValue()); - assertTrue("testGetCommunities 1",collection.getCommunities().size() == 1); + context.turnOffAuthorisationSystem(); + Community community = communityService.create(null, context); + communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "community 3"); + this.collection.addCommunity(community); + community = communityService.create(null, context); + communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "community 1"); + this.collection.addCommunity(community); + community = communityService.create(null, context); + communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "community 2"); + this.collection.addCommunity(community); + context.restoreAuthSystemState(); + assertTrue("testGetCommunities 0",collection.getCommunities().size() == 4); + //Communities should be sorted by name + assertTrue("testGetCommunities 1",collection.getCommunities().get(1).getName().equals("community 1")); + assertTrue("testGetCommunities 1",collection.getCommunities().get(2).getName().equals("community 2")); + assertTrue("testGetCommunities 1",collection.getCommunities().get(3).getName().equals("community 3")); } /** diff --git a/dspace-api/src/test/java/org/dspace/content/CommunityTest.java b/dspace-api/src/test/java/org/dspace/content/CommunityTest.java index dfe29c7ac9..0a52ae63b5 100644 --- a/dspace-api/src/test/java/org/dspace/content/CommunityTest.java +++ b/dspace-api/src/test/java/org/dspace/content/CommunityTest.java @@ -8,22 +8,25 @@ package org.dspace.content; -import java.io.File; -import java.io.FileInputStream; -import java.sql.SQLException; -import java.util.List; -import java.util.UUID; - +import mockit.NonStrictExpectations; import org.apache.log4j.Logger; import org.dspace.app.util.AuthorizeUtil; import org.dspace.authorize.AuthorizeException; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.eperson.Group; -import org.junit.*; -import static org.junit.Assert.* ; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.io.FileInputStream; +import java.sql.SQLException; +import java.util.List; +import java.util.UUID; + import static org.hamcrest.CoreMatchers.*; -import mockit.NonStrictExpectations; +import static org.junit.Assert.*; /** * Unit Tests for class Community @@ -681,10 +684,20 @@ public class CommunityTest extends AbstractDSpaceObjectTest assertThat("testGetCollections 0",c.getCollections(), notNullValue()); assertTrue("testGetCollections 1", c.getCollections().size() == 0); - Collection result = collectionService.create(context, c); - assertThat("testGetCollections 2",c.getCollections(), notNullValue()); - assertTrue("testGetCollections 3", c.getCollections().size() == 1); - assertThat("testGetCollections 4",c.getCollections().get(0), equalTo(result)); + context.turnOffAuthorisationSystem(); + Collection collection = collectionService.create(context, c); + collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection B"); + collection = collectionService.create(context, c); + collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection C"); + collection = collectionService.create(context, c); + collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection A"); + //we need to commit the changes so we don't block the table for testing + context.restoreAuthSystemState(); + + //sorted + assertTrue("testGetCollections 2",c.getCollections().get(0).getName().equals("collection A")); + assertTrue("testGetCollections 3",c.getCollections().get(1).getName().equals("collection B")); + assertTrue("testGetCollections 4",c.getCollections().get(2).getName().equals("collection C")); } /** @@ -707,11 +720,20 @@ public class CommunityTest extends AbstractDSpaceObjectTest assertThat("testGetSubcommunities 0",c.getSubcommunities(), notNullValue()); assertTrue("testGetSubcommunities 1", c.getSubcommunities().size() == 0); - //community with parent - Community son = communityService.create(c, context); - assertThat("testGetSubcommunities 2",c.getSubcommunities(), notNullValue()); - assertTrue("testGetSubcommunities 3", c.getSubcommunities().size() == 1); - assertThat("testGetSubcommunities 4", c.getSubcommunities().get(0), equalTo(son)); + context.turnOffAuthorisationSystem(); + Community community = communityService.create(c, context); + communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "subcommunity B"); + community = communityService.create(c, context); + communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "subcommunity A"); + community = communityService.create(c, context); + communityService.setMetadataSingleValue(context, community, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "subcommunity C"); + //we need to commit the changes so we don't block the table for testing + context.restoreAuthSystemState(); + + //get Subcommunities sorted + assertTrue("testGetCollections 2",c.getSubcommunities().get(0).getName().equals("subcommunity A")); + assertTrue("testGetCollections 3",c.getSubcommunities().get(1).getName().equals("subcommunity B")); + assertTrue("testGetCollections 4",c.getSubcommunities().get(2).getName().equals("subcommunity C")); } /** diff --git a/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java b/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java index e757cfac08..2ff28bc599 100644 --- a/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java +++ b/dspace-api/src/test/java/org/dspace/content/InstallItemTest.java @@ -15,6 +15,7 @@ import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.*; import org.dspace.core.Constants; import org.dspace.core.Context; +import org.dspace.eperson.EPerson; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -122,7 +123,7 @@ public class InstallItemTest extends AbstractUnitTest public void testInstallItem_validHandle() throws Exception { context.turnOffAuthorisationSystem(); - String handle = "123456789/567"; + String handle = "123456789/56789"; WorkspaceItem is = workspaceItemService.create(context, collection, false); //Test assigning a specified handle to an item @@ -147,9 +148,10 @@ public class InstallItemTest extends AbstractUnitTest Constants.ADD); result = false; // Allow full Admin perms authorizeService.isAdmin((Context) any); result = true; + authorizeService.isAdmin((Context) any, (EPerson) any); result = true; }}; - String handle = "123456789/567"; + String handle = "123456789/56789"; WorkspaceItem is = workspaceItemService.create(context, collection, false); WorkspaceItem is2 = workspaceItemService.create(context, collection, false); @@ -170,7 +172,7 @@ public class InstallItemTest extends AbstractUnitTest public void testRestoreItem() throws Exception { context.turnOffAuthorisationSystem(); - String handle = "123456789/567"; + String handle = "123456789/56789"; WorkspaceItem is = workspaceItemService.create(context, collection, false); //get current date @@ -240,7 +242,7 @@ public class InstallItemTest extends AbstractUnitTest { //create a dummy WorkspaceItem context.turnOffAuthorisationSystem(); - String handle = "123456789/567"; + String handle = "123456789/56789"; WorkspaceItem is = workspaceItemService.create(context, collection, false); // Set "today" as "dc.date.issued" @@ -274,7 +276,7 @@ public class InstallItemTest extends AbstractUnitTest { //create a dummy WorkspaceItem with no dc.date.issued context.turnOffAuthorisationSystem(); - String handle = "123456789/567"; + String handle = "123456789/56789"; WorkspaceItem is = workspaceItemService.create(context, collection, false); Item result = installItemService.installItem(context, is, handle); @@ -293,7 +295,7 @@ public class InstallItemTest extends AbstractUnitTest { //create a dummy WorkspaceItem context.turnOffAuthorisationSystem(); - String handle = "123456789/567"; + String handle = "123456789/56789"; WorkspaceItem is = workspaceItemService.create(context, collection, false); // Set "today" as "dc.date.issued" diff --git a/dspace-api/src/test/java/org/dspace/content/ItemTest.java b/dspace-api/src/test/java/org/dspace/content/ItemTest.java index 30ac385599..48b63dc404 100644 --- a/dspace-api/src/test/java/org/dspace/content/ItemTest.java +++ b/dspace-api/src/test/java/org/dspace/content/ItemTest.java @@ -7,38 +7,39 @@ */ package org.dspace.content; +import mockit.NonStrictExpectations; +import org.apache.commons.lang.time.DateUtils; +import org.apache.log4j.Logger; +import org.dspace.app.util.AuthorizeUtil; +import org.dspace.authorize.AuthorizeException; +import org.dspace.authorize.ResourcePolicy; +import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.BitstreamFormatService; +import org.dspace.content.service.MetadataFieldService; +import org.dspace.content.service.MetadataSchemaService; +import org.dspace.core.Constants; +import org.dspace.core.Context; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.sql.SQLException; - -import org.apache.commons.lang.time.DateUtils; -import org.dspace.authorize.AuthorizeException; -import org.apache.log4j.Logger; - import java.util.*; -import org.dspace.content.factory.ContentServiceFactory; -import org.dspace.content.service.BitstreamFormatService; -import org.dspace.content.service.MetadataFieldService; -import org.dspace.content.service.MetadataSchemaService; -import org.dspace.core.Context; -import org.dspace.eperson.EPerson; -import org.dspace.eperson.Group; -import org.junit.*; -import static org.junit.Assert.* ; import static org.hamcrest.CoreMatchers.*; -import mockit.*; -import org.dspace.app.util.AuthorizeUtil; -import org.dspace.authorize.ResourcePolicy; -import org.dspace.core.Constants; +import static org.junit.Assert.*; /** * Unit Tests for class Item * @author pvillega */ -public class ItemTest extends AbstractDSpaceObjectTest +public class ItemTest extends AbstractDSpaceObjectTest { /** log4j category */ @@ -107,28 +108,30 @@ public class ItemTest extends AbstractDSpaceObjectTest @Override public void destroy() { -// try { - context.turnOffAuthorisationSystem(); - //Get new instances, god knows what happended before -// it = itemService.find(context, it.getID()); -// collection = collectionService.find(context, collection.getID()); -// owningCommunity = communityService.find(context, owningCommunity.getID()); -// -// communityService.delete(context, owningCommunity); -// context.commit(); -// context.restoreAuthSystemState(); - it = null; - collection = null; - owningCommunity = null; + context.turnOffAuthorisationSystem(); + try { + itemService.delete(context, it); + } catch(Exception e){ + } + + try { + collectionService.delete(context, collection); + } catch(Exception e){ + } + + try { + communityService.delete(context, owningCommunity); + } catch(Exception e){ + } + + context.restoreAuthSystemState(); + it = null; + collection = null; + owningCommunity = null; + try { super.destroy(); -// } catch (SQLException | AuthorizeException | IOException ex) { -// if(context.isValid()) -// { -// context.abort(); -// } -// log.error("Error in destroy", ex); -// fail("Error in destroy: " + ex.getMessage()); -// } + } catch(Exception e){ + } } @@ -612,8 +615,18 @@ public class ItemTest extends AbstractDSpaceObjectTest @Test public void testGetCollections() throws Exception { + context.turnOffAuthorisationSystem(); + Collection collection = collectionService.create(context, owningCommunity); + collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection B"); + it.addCollection(collection); + collection = collectionService.create(context, owningCommunity); + collectionService.setMetadataSingleValue(context, collection, MetadataSchema.DC_SCHEMA, "title", null, Item.ANY, "collection A"); + it.addCollection(collection); + context.restoreAuthSystemState(); assertThat("testGetCollections 0", it.getCollections(), notNullValue()); - assertTrue("testGetCollections 1", it.getCollections().size() == 1); + assertTrue("testGetCollections 1", it.getCollections().size() == 3); + assertTrue("testGetCollections 2", it.getCollections().get(1).getName().equals("collection A")); + assertTrue("testGetCollections 3", it.getCollections().get(2).getName().equals("collection B")); } /** @@ -1478,6 +1491,7 @@ public class ItemTest extends AbstractDSpaceObjectTest context.turnOffAuthorisationSystem(); Collection from = createCollection(); Collection to = createCollection(); + it.addCollection(from); it.setOwningCollection(from); itemService.move(context, it, from, to); diff --git a/dspace-api/src/test/java/org/dspace/content/comparator/NameAscendingComparatorTest.java b/dspace-api/src/test/java/org/dspace/content/comparator/NameAscendingComparatorTest.java new file mode 100644 index 0000000000..86953be1ba --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/content/comparator/NameAscendingComparatorTest.java @@ -0,0 +1,97 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.content.comparator; + +import org.dspace.content.DSpaceObject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class NameAscendingComparatorTest { + + private NameAscendingComparator comparator = new NameAscendingComparator(); + + @Mock + private DSpaceObject dso1; + + @Mock + private DSpaceObject dso2; + + + @Test + public void testCompareLessThan() throws Exception { + when(dso1.getName()).thenReturn("a"); + when(dso2.getName()).thenReturn("b"); + + assertTrue(comparator.compare(dso1, dso2) < 0); + } + + @Test + public void testCompareGreaterThan() throws Exception { + when(dso1.getName()).thenReturn("b"); + when(dso2.getName()).thenReturn("a"); + + assertTrue(comparator.compare(dso1, dso2) > 0); + } + + @Test + public void testCompareEqual() throws Exception { + when(dso1.getName()).thenReturn("b"); + when(dso2.getName()).thenReturn("b"); + + assertTrue(comparator.compare(dso1, dso2) == 0); + } + + @Test + public void testCompareFirstNull() throws Exception { + when(dso2.getName()).thenReturn("b"); + + assertTrue(comparator.compare(null, dso2) < 0); + } + + @Test + public void testCompareSecondNull() throws Exception { + when(dso1.getName()).thenReturn("a"); + + assertTrue(comparator.compare(dso1, null) > 0); + } + + @Test + public void testCompareBothNull() throws Exception { + assertTrue(comparator.compare(null, null) == 0); + } + + @Test + public void testCompareNameNull() throws Exception { + when(dso1.getName()).thenReturn(null); + when(dso2.getName()).thenReturn("b"); + + assertTrue(comparator.compare(dso1, dso2) < 0); + } + + @Test + public void testCompareCaseInsensitive() throws Exception { + when(dso1.getName()).thenReturn("a"); + when(dso2.getName()).thenReturn("B"); + + assertTrue(comparator.compare(dso1, dso2) < 0); + } + + @Test + public void testCompareCaseTrimmed() throws Exception { + when(dso1.getName()).thenReturn("a"); + when(dso2.getName()).thenReturn(" b "); + + assertTrue(comparator.compare(dso1, dso2) < 0); + } +} \ No newline at end of file diff --git a/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java b/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java index 6067cb1a8d..66fdab1005 100644 --- a/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java +++ b/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java @@ -167,6 +167,11 @@ public class ITDSpaceAIP extends AbstractUnitTest ePersonService.update(context, submitter); context.setCurrentUser(submitter); + //Make our test ePerson an admin so he can perform deletes and restores + GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + Group adminGroup = groupService.findByName(context, Group.ADMIN); + groupService.addMember(context, adminGroup, submitter); + // Create our primary Test Item WorkspaceItem wsItem = workspaceItemService.create(context, grandchildCol, false); Item item = installItemService.installItem(context, wsItem); diff --git a/dspace-api/src/test/java/org/dspace/core/ContextReadOnlyCacheTest.java b/dspace-api/src/test/java/org/dspace/core/ContextReadOnlyCacheTest.java new file mode 100644 index 0000000000..fe11b9e33c --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/core/ContextReadOnlyCacheTest.java @@ -0,0 +1,121 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.core; + +import org.dspace.content.Item; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.UUID; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.when; + +/** + * Class to test the read-only Context cache + */ +@RunWith(MockitoJUnitRunner.class) +public class ContextReadOnlyCacheTest { + + private ContextReadOnlyCache readOnlyCache; + + @Mock + private EPerson ePerson; + + @Before + public void init() { + readOnlyCache = new ContextReadOnlyCache(); + when(ePerson.getID()).thenReturn(UUID.randomUUID()); + } + + @Test + public void cacheAuthorizedAction() throws Exception { + Item item = Mockito.mock(Item.class); + when(item.getID()).thenReturn(UUID.randomUUID()); + + readOnlyCache.cacheAuthorizedAction(item, Constants.READ, ePerson, true); + readOnlyCache.cacheAuthorizedAction(item, Constants.WRITE, ePerson, false); + + assertTrue(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, ePerson)); + assertFalse(readOnlyCache.getCachedAuthorizationResult(item, Constants.WRITE, ePerson)); + + assertNull(readOnlyCache.getCachedAuthorizationResult(item, Constants.ADMIN, ePerson)); + assertNull(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, null)); + assertNull(readOnlyCache.getCachedAuthorizationResult(null, Constants.READ, ePerson)); + } + + @Test + public void cacheGroupMembership() throws Exception { + Group group1 = buildGroupMock("Test Group 1"); + Group group2 = buildGroupMock("Test Group 2"); + Group group3 = buildGroupMock("Test Group 3"); + + readOnlyCache.cacheGroupMembership(group1, ePerson, true); + readOnlyCache.cacheGroupMembership(group2, ePerson, false); + + assertTrue(readOnlyCache.getCachedGroupMembership(group1, ePerson)); + assertFalse(readOnlyCache.getCachedGroupMembership(group2, ePerson)); + + assertNull(readOnlyCache.getCachedGroupMembership(group3, ePerson)); + assertNull(readOnlyCache.getCachedGroupMembership(null, ePerson)); + assertNull(readOnlyCache.getCachedGroupMembership(group2, null)); + } + + @Test + public void cacheAllMemberGroupsSet() throws Exception { + Group group1 = buildGroupMock("Test Group 1"); + Group group2 = buildGroupMock("Test Group 2"); + Group group3 = buildGroupMock("Test Group 3"); + + readOnlyCache.cacheAllMemberGroupsSet(ePerson, new HashSet<>(Arrays.asList(group1, group2))); + + assertTrue(readOnlyCache.getCachedGroupMembership(group1, ePerson)); + assertTrue(readOnlyCache.getCachedGroupMembership(group2, ePerson)); + assertFalse(readOnlyCache.getCachedGroupMembership(group3, ePerson)); + assertFalse(readOnlyCache.getCachedGroupMembership(null, ePerson)); + + assertNull(readOnlyCache.getCachedGroupMembership(group2, null)); + } + + @Test + public void clear() throws Exception { + Item item = Mockito.mock(Item.class); + when(item.getID()).thenReturn(UUID.randomUUID()); + Group group1 = buildGroupMock("Test Group 1"); + + //load data into the cache + readOnlyCache.cacheAuthorizedAction(item, Constants.READ, ePerson, true); + readOnlyCache.cacheGroupMembership(group1, ePerson, true); + + //double check the data is there + assertTrue(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, ePerson)); + assertTrue(readOnlyCache.getCachedGroupMembership(group1, ePerson)); + + //clear the cache + readOnlyCache.clear(); + + //check that the data is not present anymore + assertNull(readOnlyCache.getCachedAuthorizationResult(item, Constants.READ, ePerson)); + assertNull(readOnlyCache.getCachedGroupMembership(group1, ePerson)); + } + + private Group buildGroupMock(final String name) { + Group group = Mockito.mock(Group.class); + when(group.getName()).thenReturn(name); + return group; + } + +} \ No newline at end of file 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 ab08dc9a2c..827b444043 100644 --- a/dspace-api/src/test/java/org/dspace/eperson/GroupTest.java +++ b/dspace-api/src/test/java/org/dspace/eperson/GroupTest.java @@ -206,6 +206,10 @@ public class GroupTest extends AbstractUnitTest { List names = new ArrayList<>(); List sortedNames = new ArrayList<>(); for (Group group : groups) { + // Ignore any unnamed groups. This is only necessary when running unit tests via a persistent database (e.g. Postgres) as unnamed groups may be created by other tests. + if (group.getName() == null) { + continue; + } names.add(group.getName()); sortedNames.add(group.getName()); } @@ -331,6 +335,18 @@ public class GroupTest extends AbstractUnitTest { assertFalse("isMemberGroup 4", groupService.isMember(level2Group, level1Group)); } + @Test + public void isSubgroupOf() throws SQLException { + assertTrue("isMemberGroup 1", groupService.isParentOf(context, topGroup, level1Group)); + assertTrue("isMemberGroup 2", groupService.isParentOf(context, level1Group, level2Group)); + assertFalse("isMemberGroup 3", groupService.isParentOf(context, level1Group, topGroup)); + assertFalse("isMemberGroup 4", groupService.isParentOf(context, level2Group, level1Group)); + + //Also check ancestor relations + assertTrue("isMemberGroup 5", groupService.isParentOf(context, topGroup, level2Group)); + assertFalse("isMemberGroup 6", groupService.isParentOf(context, level2Group, topGroup)); + } + @Test public void isMemberEPerson() throws SQLException, AuthorizeException, EPersonDeletionException, IOException { EPerson ePerson = null; @@ -355,9 +371,9 @@ public class GroupTest extends AbstractUnitTest { ePerson = createEPersonAndAddToGroup("isMemberContext@dspace.org", level2Group); context.setCurrentUser(ePerson); - assertTrue(groupService.isMember(context, topGroup)); - assertTrue(groupService.isMember(context, level1Group)); - assertTrue(groupService.isMember(context, level2Group)); + assertTrue(groupService.isMember(context, ePerson, topGroup)); + assertTrue(groupService.isMember(context, ePerson, level1Group)); + assertTrue(groupService.isMember(context, ePerson, level2Group)); } finally { if(ePerson != null) { @@ -373,10 +389,9 @@ public class GroupTest extends AbstractUnitTest { try { ePerson = createEPersonAndAddToGroup("isMemberContextGroupId@dspace.org", level2Group); - context.setCurrentUser(ePerson); - assertTrue(groupService.isMember(context, topGroup)); - assertTrue(groupService.isMember(context, level1Group)); - assertTrue(groupService.isMember(context, level2Group)); + assertTrue(groupService.isMember(context, ePerson, topGroup.getName())); + assertTrue(groupService.isMember(context, ePerson, level1Group.getName())); + assertTrue(groupService.isMember(context, ePerson, level2Group.getName())); } finally { if(ePerson != null) { @@ -386,6 +401,117 @@ public class GroupTest extends AbstractUnitTest { } } + @Test + public void isMemberContextSpecialGroup() throws SQLException, AuthorizeException, EPersonDeletionException, IOException { + EPerson ePerson = null; + Group specialGroup = null; + try { + specialGroup = createGroup("specialGroup"); + groupService.addMember(context, level1Group, specialGroup); + groupService.update(context, level1Group); + + ePerson = createEPerson("isMemberContextGroupSpecial@dspace.org"); + + context.setCurrentUser(ePerson); + context.setSpecialGroup(specialGroup.getID()); + + assertTrue(groupService.isMember(context, topGroup)); + assertTrue(groupService.isMember(context, level1Group)); + assertFalse(groupService.isMember(context, level2Group)); + assertTrue(groupService.isMember(context, specialGroup)); + + } finally { + if(ePerson != null) + { + context.turnOffAuthorisationSystem(); + ePersonService.delete(context, ePerson); + } + if(specialGroup != null) + { + context.turnOffAuthorisationSystem(); + groupService.delete(context, specialGroup); + } + } + } + + @Test + public void isMemberContextSpecialGroupOtherUser() throws SQLException, AuthorizeException, EPersonDeletionException, IOException { + EPerson ePerson1 = null; + EPerson ePerson2 = null; + Group specialGroup = null; + try { + specialGroup = createGroup("specialGroup"); + groupService.addMember(context, level2Group, specialGroup); + groupService.update(context, level2Group); + + //The authenticated user has a special group + ePerson1 = createEPerson("isMemberContextGroupSpecial@dspace.org"); + context.setCurrentUser(ePerson1); + context.setSpecialGroup(specialGroup.getID()); + + //Or second user is member of the level 1 group + ePerson2 = createEPersonAndAddToGroup("isMemberContextSpecialGroupOtherUser@dspace.org", level1Group); + + assertTrue(groupService.isMember(context, ePerson2, topGroup)); + assertTrue(groupService.isMember(context, ePerson2, level1Group)); + assertFalse(groupService.isMember(context, ePerson2, level2Group)); + assertFalse(groupService.isMember(context, ePerson2, specialGroup)); + + assertTrue(groupService.isMember(context, ePerson1, level2Group)); + assertTrue(groupService.isMember(context, ePerson1, specialGroup)); + + } finally { + if(ePerson1 != null) + { + context.turnOffAuthorisationSystem(); + ePersonService.delete(context, ePerson1); + } + if(ePerson2 != null) + { + context.turnOffAuthorisationSystem(); + ePersonService.delete(context, ePerson2); + } + if(specialGroup != null) + { + context.turnOffAuthorisationSystem(); + groupService.delete(context, specialGroup); + } + } + } + + @Test + public void isMemberContextSpecialGroupDbMembership() throws SQLException, AuthorizeException, EPersonDeletionException, IOException { + EPerson ePerson = null; + Group specialGroup = null; + try { + specialGroup = createGroup("specialGroup"); + groupService.addMember(context, level1Group, specialGroup); + groupService.update(context, level1Group); + + ePerson = createEPersonAndAddToGroup("isMemberContextGroupSpecialDbMembership@dspace.org", level2Group); + + context.setCurrentUser(ePerson); + context.setSpecialGroup(specialGroup.getID()); + + assertTrue(groupService.isMember(context, topGroup)); + assertTrue(groupService.isMember(context, level1Group)); + assertTrue(groupService.isMember(context, level2Group)); + assertTrue(groupService.isMember(context, specialGroup)); + + } finally { + if(ePerson != null) + { + context.turnOffAuthorisationSystem(); + ePersonService.delete(context, ePerson); + } + if(specialGroup != null) + { + context.turnOffAuthorisationSystem(); + groupService.delete(context, specialGroup); + } + } + } + @Test public void isPermanent() throws SQLException @@ -443,10 +569,15 @@ public class GroupTest extends AbstractUnitTest { } @Test - public void removeMemberGroup() throws SQLException { + public void removeMemberGroup() throws SQLException, AuthorizeException { assertTrue(groupService.isMember(topGroup, level1Group)); + assertTrue(groupService.isParentOf(context, topGroup, level1Group)); + groupService.removeMember(context, topGroup, level1Group); + groupService.update(context, topGroup); + assertFalse(groupService.isMember(topGroup, level1Group)); + assertFalse(groupService.isParentOf(context, topGroup, level1Group)); } @Test diff --git a/dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorServiceImplTest.java b/dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorServiceImplTest.java new file mode 100644 index 0000000000..f98ce6d040 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorServiceImplTest.java @@ -0,0 +1,372 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.statistics.util; + + +import mockit.Mock; +import mockit.MockUp; +import org.dspace.AbstractDSpaceTest; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; +import org.dspace.statistics.SolrLoggerServiceImpl; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.mockito.runners.MockitoJUnitRunner; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import static org.mockito.Mockito.mock; + +/** + * @author mwood + * @author frederic at atmire.com + */ +@RunWith(MockitoJUnitRunner.class) +public class SpiderDetectorServiceImplTest extends AbstractDSpaceTest +{ + private static final String NOT_A_BOT_ADDRESS = "192.168.0.1"; + + private ConfigurationService configurationService; + + + private SpiderDetectorService spiderDetectorService; + + @Before + public void init() { + configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + spiderDetectorService = new SpiderDetectorServiceImpl(configurationService); + + } + + @Test + public void testReadPatterns() + { +// FIXME fail("Not yet implemented"); + } + + @Test + public void testGetSpiderIpAddresses() + { +// FIXME fail("Not yet implemented"); + } + + /** + * Test if Case Insitive matching option works + * @throws Exception + */ + @Test + public void testCaseInsensitiveMatching() throws Exception + { + configurationService.setProperty("usage-statistics.bots.case-insensitive", true); + spiderDetectorService = new SpiderDetectorServiceImpl(configurationService); + + DummyHttpServletRequest req = new DummyHttpServletRequest(); + req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises + req.setRemoteHost("notabot.example.com"); // avoid surprises + req.setAgent("Firefox"); // avoid surprises + + String candidate; + + // Test agent patterns + req.setAgent("msnboT Is WaTching you"); + assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req)); + + req.setAgent("FirefOx"); + assertFalse("'Firefox' matched a pattern", spiderDetectorService.isSpider(req)); + + // Test IP patterns + candidate = "192.168.2.1"; + req.setAddress(candidate); + assertTrue(candidate + " did not match IP patterns", spiderDetectorService.isSpider(req)); + + req.setAddress(NOT_A_BOT_ADDRESS); + assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", spiderDetectorService.isSpider(req)); + + // Test DNS patterns + candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com"; + req.setRemoteHost(candidate); + assertTrue(candidate + " did match DNS patterns", spiderDetectorService.isSpider(req)); + + candidate = "wIki.dsPace.oRg"; + req.setRemoteHost(candidate); + assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req)); + + } + + /** + * Test method for {@link org.dspace.statistics.util.SpiderDetectorService#isSpider(javax.servlet.http.HttpServletRequest)}. + */ + @Test + public void testIsSpiderHttpServletRequest() + { + DummyHttpServletRequest req = new DummyHttpServletRequest(); + req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises + req.setRemoteHost("notabot.example.com"); // avoid surprises + req.setAgent("Firefox"); // avoid surprises + + String candidate; + + // Test agent patterns + req.setAgent("msnbot is watching you"); + assertTrue("'msnbot' did not match any pattern", spiderDetectorService.isSpider(req)); + + req.setAgent("Firefox"); + assertFalse("'Firefox' matched a pattern", spiderDetectorService.isSpider(req)); + + // Test IP patterns + candidate = "192.168.2.1"; + req.setAddress(candidate); + assertTrue(candidate + " did not match IP patterns", spiderDetectorService.isSpider(req)); + + req.setAddress(NOT_A_BOT_ADDRESS); + assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", spiderDetectorService.isSpider(req)); + + // Test DNS patterns + candidate = "baiduspider-dspace-test.crawl.baidu.com"; + req.setRemoteHost(candidate); + assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req)); + + candidate = "wiki.dspace.org"; + req.setRemoteHost(candidate); + assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req)); + } + + /** + * Test method for {@link org.dspace.statistics.util.SpiderDetectorService#isSpider(java.lang.String, java.lang.String, java.lang.String, java.lang.String)}. + */ + @Test + public void testIsSpiderStringStringStringString() + { + String candidate; + + // Test IP patterns + candidate = "192.168.2.1"; + assertTrue(candidate + " did not match IP patterns", + spiderDetectorService.isSpider(candidate, null, null, null)); + + candidate = NOT_A_BOT_ADDRESS; + assertFalse(candidate + " matched IP patterns", + spiderDetectorService.isSpider(candidate, null, null, null)); + + // Test DNS patterns + candidate = "baiduspider-dspace-test.crawl.baidu.com"; + assertTrue(candidate + " did not match DNS patterns", + spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null)); + + candidate = "wiki.dspace.org"; + assertFalse(candidate + " matched DNS patterns", + spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null)); + + // Test agent patterns + candidate = "msnbot is watching you"; + assertTrue("'" + candidate + "' did not match agent patterns", + spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate)); + + candidate = "Firefox"; + assertFalse("'" + candidate + "' matched agent patterns", + spiderDetectorService.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate)); + } + + /** + * Test method for {@link org.dspace.statistics.util.SpiderDetectorService#isSpider(java.lang.String)}. + */ + @Test + public void testIsSpiderString() + { + String candidate; + + candidate = "192.168.2.1"; + assertTrue(candidate + " did not match IP patterns", + spiderDetectorService.isSpider(candidate, null, null, null)); + + candidate = NOT_A_BOT_ADDRESS; + assertFalse(candidate + " matched IP patterns", + spiderDetectorService.isSpider(candidate, null, null, null)); + + } + + + /** + * Test if Case Sensitive matching still works after adding the option + * @throws Exception + */ + @Test + public void testCaseSensitiveMatching() throws Exception + { + + DummyHttpServletRequest req = new DummyHttpServletRequest(); + req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises + req.setRemoteHost("notabot.example.com"); // avoid surprises + req.setAgent("Firefox"); // avoid surprises + + String candidate; + + // Test agent patterns + req.setAgent("msnboT Is WaTching you"); + assertFalse("'msnbot' matched pattern", spiderDetectorService.isSpider(req)); + + req.setAgent("FirefOx"); + assertFalse("'Firefox' matched a pattern", spiderDetectorService.isSpider(req)); + + // Test IP patterns + candidate = "192.168.2.1"; + req.setAddress(candidate); + assertTrue(candidate + " did not match IP patterns", spiderDetectorService.isSpider(req)); + + req.setAddress(NOT_A_BOT_ADDRESS); + assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", spiderDetectorService.isSpider(req)); + + // Test DNS patterns + candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com"; + req.setRemoteHost(candidate); + assertFalse(candidate + " did match DNS patterns", spiderDetectorService.isSpider(req)); + + candidate = "wIki.dsPace.oRg"; + req.setRemoteHost(candidate); + assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req)); + + } + + /** + * Test to see that lowercase will be matched but uppercase won't + */ + @Test + public void testInsensitiveSensitiveDifference() { + + DummyHttpServletRequest req = new DummyHttpServletRequest(); + req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises + req.setRemoteHost("notabot.example.com"); // avoid surprises + req.setAgent("Firefox"); // avoid surprises + + String candidate; + + // Test agent patterns + req.setAgent("msnbot is WaTching you"); + assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req)); + + req.setAgent("MSNBOT Is WaTching you"); + assertFalse("'msnbot' matched pattern", spiderDetectorService.isSpider(req)); + + // Test DNS patterns + candidate = "baiduspider-dspace-test.crawl.baidu.com"; + req.setRemoteHost(candidate); + assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req)); + + candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com"; + req.setRemoteHost(candidate); + assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req)); + } + + /** + * Test to check if the same agent gets caught with and without upper-casing + */ + @Test + public void testBothLowerAndUpperCaseGetMatched() { + + configurationService.setProperty("usage-statistics.bots.case-insensitive", true); + spiderDetectorService = new SpiderDetectorServiceImpl(configurationService); + + DummyHttpServletRequest req = new DummyHttpServletRequest(); + req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises + req.setRemoteHost("notabot.example.com"); // avoid surprises + req.setAgent("Firefox"); // avoid surprises + + String candidate; + + // Test agent patterns + req.setAgent("msnbot is WaTching you"); + assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req)); + + req.setAgent("MSNBOT Is WaTching you"); + assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req)); + + // Test DNS patterns + candidate = "baiduspider-dspace-test.crawl.baidu.com"; + req.setRemoteHost(candidate); + assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req)); + + candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com"; + req.setRemoteHost(candidate); + assertTrue(candidate + " didn't match DNS patterns", spiderDetectorService.isSpider(req)); + } + + /** + * Test if wrong value is used for property + */ + @Test + public void testNonBooleanConfig() { + configurationService.setProperty("usage-statistics.bots.case-insensitive", "RandomNonBooleanString"); + spiderDetectorService = new SpiderDetectorServiceImpl(configurationService); + + DummyHttpServletRequest req = new DummyHttpServletRequest(); + req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises + req.setRemoteHost("notabot.example.com"); // avoid surprises + req.setAgent("Firefox"); // avoid surprises + + String candidate; + + // Test agent patterns + req.setAgent("msnbot is WaTching you"); + assertTrue("'msnbot' didn't match pattern", spiderDetectorService.isSpider(req)); + + req.setAgent("MSNBOT Is WaTching you"); + assertFalse("'msnbot' matched pattern", spiderDetectorService.isSpider(req)); + + // Test DNS patterns + candidate = "baiduspider-dspace-test.crawl.baidu.com"; + req.setRemoteHost(candidate); + assertTrue(candidate + " did not match DNS patterns", spiderDetectorService.isSpider(req)); + + candidate = "baiduspiDer-dSPace-test.crawl.baIDu.com"; + req.setRemoteHost(candidate); + assertFalse(candidate + " matched DNS patterns", spiderDetectorService.isSpider(req)); + + + } + + + + /** + * Method to make sure the SpiderDetector is using CaseSensitive matching again after each test + * @throws Exception + */ + @After + public void cleanup() throws Exception { + spiderDetectorService = null; + configurationService.setProperty("usage-statistics.bots.case-insensitive", false);; + } + + + + + /** + * Dummy SolrLogger for testing. + * @author mwood + */ + static public class MockSolrLogger + extends MockUp + { + @Mock + public void $init() {} + + @Mock + public void $clinit() {} + + @Mock + public boolean isUseProxies() + { + return false; + } + + } + +} diff --git a/dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorTest.java b/dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorTest.java deleted file mode 100644 index 5a8dab3d5c..0000000000 --- a/dspace-api/src/test/java/org/dspace/statistics/util/SpiderDetectorTest.java +++ /dev/null @@ -1,155 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.statistics.util; - -import mockit.Mock; -import mockit.MockUp; -import org.dspace.AbstractDSpaceTest; -import org.dspace.statistics.SolrLoggerServiceImpl; -import org.junit.Test; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -/** - * @author mwood - */ -public class SpiderDetectorTest extends AbstractDSpaceTest -{ - private static final String NOT_A_BOT_ADDRESS = "192.168.0.1"; - - /** - * Test method for {@link org.dspace.statistics.util.SpiderDetector#readPatterns(java.io.File)}. - */ - @Test - public void testReadPatterns() - { -// FIXME fail("Not yet implemented"); - } - - /** - * Test method for {@link org.dspace.statistics.util.SpiderDetector#getSpiderIpAddresses()}. - */ - @Test - public void testGetSpiderIpAddresses() - { -// FIXME fail("Not yet implemented"); - } - - /** - * Test method for {@link org.dspace.statistics.util.SpiderDetector#isSpider(javax.servlet.http.HttpServletRequest)}. - */ - @Test - public void testIsSpiderHttpServletRequest() - { - DummyHttpServletRequest req = new DummyHttpServletRequest(); - req.setAddress(NOT_A_BOT_ADDRESS); // avoid surprises - req.setRemoteHost("notabot.example.com"); // avoid surprises - req.setAgent("Firefox"); // avoid surprises - - String candidate; - - // Test agent patterns - req.setAgent("msnbot is watching you"); - assertTrue("'msnbot' did not match any pattern", SpiderDetector.isSpider(req)); - - req.setAgent("Firefox"); - assertFalse("'Firefox' matched a pattern", SpiderDetector.isSpider(req)); - - // Test IP patterns - candidate = "192.168.2.1"; - req.setAddress(candidate); - assertTrue(candidate + " did not match IP patterns", SpiderDetector.isSpider(req)); - - req.setAddress(NOT_A_BOT_ADDRESS); - assertFalse(NOT_A_BOT_ADDRESS + " matched IP patterns", SpiderDetector.isSpider(req)); - - // Test DNS patterns - candidate = "baiduspider-dspace-test.crawl.baidu.com"; - req.setRemoteHost(candidate); - assertTrue(candidate + " did not match DNS patterns", SpiderDetector.isSpider(req)); - - candidate = "wiki.dspace.org"; - req.setRemoteHost(candidate); - assertFalse(candidate + " matched DNS patterns", SpiderDetector.isSpider(req)); - } - - /** - * Test method for {@link org.dspace.statistics.util.SpiderDetector#isSpider(java.lang.String, java.lang.String, java.lang.String, java.lang.String)}. - */ - @Test - public void testIsSpiderStringStringStringString() - { - String candidate; - - // Test IP patterns - candidate = "192.168.2.1"; - assertTrue(candidate + " did not match IP patterns", - SpiderDetector.isSpider(candidate, null, null, null)); - - candidate = NOT_A_BOT_ADDRESS; - assertFalse(candidate + " matched IP patterns", - SpiderDetector.isSpider(candidate, null, null, null)); - - // Test DNS patterns - candidate = "baiduspider-dspace-test.crawl.baidu.com"; - assertTrue(candidate + " did not match DNS patterns", - SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null)); - - candidate = "wiki.dspace.org"; - assertFalse(candidate + " matched DNS patterns", - SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, candidate, null)); - - // Test agent patterns - candidate = "msnbot is watching you"; - assertTrue("'" + candidate + "' did not match agent patterns", - SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate)); - - candidate = "Firefox"; - assertFalse("'" + candidate + "' matched agent patterns", - SpiderDetector.isSpider(NOT_A_BOT_ADDRESS, null, null, candidate)); - } - - /** - * Test method for {@link org.dspace.statistics.util.SpiderDetector#isSpider(java.lang.String)}. - */ - @Test - public void testIsSpiderString() - { - String candidate; - - candidate = "192.168.2.1"; - assertTrue(candidate + " did not match IP patterns", - SpiderDetector.isSpider(candidate, null, null, null)); - - candidate = NOT_A_BOT_ADDRESS; - assertFalse(candidate + " matched IP patterns", - SpiderDetector.isSpider(candidate, null, null, null)); - - } - - /** - * Dummy SolrLogger for testing. - * @author mwood - */ - static public class MockSolrLogger - extends MockUp - { - @Mock - public void $init() {} - - @Mock - public void $clinit() {} - - @Mock - public boolean isUseProxies() - { - return false; - } - } -} diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/ItemTag.java b/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/ItemTag.java index c28352fc22..a8d728c725 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/ItemTag.java +++ b/dspace-jspui/src/main/java/org/dspace/app/webui/jsptag/ItemTag.java @@ -1027,8 +1027,7 @@ public class ItemTag extends TagSupport context, b, groupService.findByName(context, Group.ANONYMOUS), - Constants.READ, - -1); + Constants.READ); ResourcePolicy rp = null; for (ResourcePolicy policy : policies) { diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/DSpaceServlet.java b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/DSpaceServlet.java index 9eef1ae66a..f06ed95939 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/DSpaceServlet.java +++ b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/DSpaceServlet.java @@ -160,6 +160,15 @@ public class DSpaceServlet extends HttpServlet } abortContext(context); } + catch (IOException ioe) + { + /* + * If a an IOException occurs (e.g. if the client interrupted a download), + * just log a simple warning without stacktrace. + */ + log.warn(LogManager.getHeader(context, "io_error", ioe.toString())); + abortContext(context); + } catch (Exception e) { log.warn(LogManager.getHeader(context, "general_jspui_error", e diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/EditItemServlet.java b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/EditItemServlet.java index 1ae9a77e4f..f5c36688d9 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/EditItemServlet.java +++ b/dspace-jspui/src/main/java/org/dspace/app/webui/servlet/admin/EditItemServlet.java @@ -110,7 +110,7 @@ public class EditItemServlet extends DSpaceServlet /** User updates Creative Commons License */ public static final int UPDATE_CC = 12; - + /** JSP to upload bitstream */ protected static final String UPLOAD_BITSTREAM_JSP = "/tools/upload-bitstream.jsp"; @@ -119,33 +119,33 @@ public class EditItemServlet extends DSpaceServlet private final transient CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService(); - + private final transient ItemService itemService = ContentServiceFactory.getInstance().getItemService(); - + private final transient BitstreamFormatService bitstreamFormatService = ContentServiceFactory.getInstance().getBitstreamFormatService(); - + private final transient BitstreamService bitstreamService = ContentServiceFactory.getInstance().getBitstreamService(); - + private final transient BundleService bundleService = ContentServiceFactory.getInstance().getBundleService(); - + private final transient HandleService handleService = HandleServiceFactory.getInstance().getHandleService(); - + private final transient MetadataFieldService metadataFieldService = ContentServiceFactory.getInstance().getMetadataFieldService(); - + private final transient MetadataSchemaService metadataSchemaService = ContentServiceFactory.getInstance().getMetadataSchemaService(); - + private final transient CreativeCommonsService creativeCommonsService = LicenseServiceFactory.getInstance().getCreativeCommonsService(); - + protected ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); - + @Override protected void doDSGet(Context context, HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException, @@ -242,14 +242,14 @@ public class EditItemServlet extends DSpaceServlet Item item = itemService.find(context, UIUtil.getUUIDParameter(request, "item_id")); - + if (request.getParameter("submit_cancel_cc") != null) { showEditForm(context, request, response, item); return; } - + String handle = handleService.findHandle(context, item); // now check to see if person can edit item @@ -271,9 +271,9 @@ public class EditItemServlet extends DSpaceServlet case CONFIRM_DELETE: // Delete the item - if "cancel" was pressed this would be - // picked up above + // picked up above itemService.delete(context, item); - + JSPManager.showJSP(request, response, "/tools/get-item-id.jsp"); context.complete(); @@ -314,7 +314,7 @@ public class EditItemServlet extends DSpaceServlet // Display move collection page with fields of collections and communities List allNotLinkedCollections = itemService.getCollectionsNotLinked(context, item); List allLinkedCollections = item.getCollections(); - + // get only the collection where the current user has the right permission List authNotLinkedCollections = new ArrayList<>(); for (Collection c : allNotLinkedCollections) @@ -333,18 +333,18 @@ public class EditItemServlet extends DSpaceServlet authLinkedCollections.add(c); } } - + request.setAttribute("linkedCollections", authLinkedCollections); request.setAttribute("notLinkedCollections", authNotLinkedCollections); - + JSPManager.showJSP(request, response, "/tools/move-item.jsp"); } else { throw new ServletException("You must be an administrator to move an item"); } - + break; - + case CONFIRM_MOVE_ITEM: if (authorizeService.isAdmin(context, item)) { @@ -361,17 +361,17 @@ public class EditItemServlet extends DSpaceServlet { throw new ServletException("Missing or incorrect collection IDs for moving item"); } - + itemService.move(context, item, fromCollection, toCollection, inheritPolicies); - + showEditForm(context, request, response, item); - + context.complete(); } else { throw new ServletException("You must be an administrator to move an item"); } - + break; case START_PRIVATING: @@ -399,7 +399,7 @@ public class EditItemServlet extends DSpaceServlet context.complete(); break; - + case UPDATE_CC: Map map = new HashMap(); @@ -412,15 +412,15 @@ public class EditItemServlet extends DSpaceServlet map.put("sampling", request.getParameter("sampling_chooser")); } map.put("jurisdiction", jurisdiction); - + LicenseMetadataValue uriField = creativeCommonsService.getCCField("uri"); LicenseMetadataValue nameField = creativeCommonsService.getCCField("name"); - + boolean exit = false; - if (licenseclass.equals("webui.Submission.submit.CCLicenseStep.no_license")) + if (licenseclass.equals("webui.Submission.submit.CCLicenseStep.no_license")) { creativeCommonsService.removeLicense(context, uriField, nameField, item); - + itemService.update(context, item); context.dispatchEvents(); exit = true; @@ -429,7 +429,7 @@ public class EditItemServlet extends DSpaceServlet //none exit = true; } - + if (!exit) { CCLookup ccLookup = new CCLookup(); ccLookup.issue(licenseclass, map, configurationService.getProperty("cc.license.locale")); @@ -504,7 +504,7 @@ public class EditItemServlet extends DSpaceServlet HttpServletResponse response, Item item) throws ServletException, IOException, SQLException, AuthorizeException { - + // Get the handle, if any String handle = handleService.findHandle(context, item); @@ -513,10 +513,10 @@ public class EditItemServlet extends DSpaceServlet // All DC types in the registry List types = metadataFieldService.findAll(context); - + // Get a HashMap of metadata field ids and a field name to display Map metadataFields = new HashMap<>(); - + // Get all existing Schemas List schemas = metadataSchemaService.findAll(context); for (MetadataSchema s : schemas) @@ -542,7 +542,7 @@ public class EditItemServlet extends DSpaceServlet { request.setAttribute("policy_button", Boolean.FALSE); } - + if (authorizeService.authorizeActionBoolean(context, itemService .getParentObject(context, item), Constants.REMOVE)) { @@ -552,7 +552,7 @@ public class EditItemServlet extends DSpaceServlet { request.setAttribute("delete_button", Boolean.FALSE); } - + try { authorizeService.authorizeAction(context, item, Constants.ADD); @@ -562,7 +562,7 @@ public class EditItemServlet extends DSpaceServlet { request.setAttribute("create_bitstream_button", Boolean.FALSE); } - + try { authorizeService.authorizeAction(context, item, Constants.REMOVE); @@ -572,7 +572,7 @@ public class EditItemServlet extends DSpaceServlet { request.setAttribute("remove_bitstream_button", Boolean.FALSE); } - + try { AuthorizeUtil.authorizeManageCCLicense(context, item); @@ -582,7 +582,7 @@ public class EditItemServlet extends DSpaceServlet { request.setAttribute("cclicense_button", Boolean.FALSE); } - + try { if( 0 < itemService.getBundles(item, "ORIGINAL").size()){ @@ -620,17 +620,17 @@ public class EditItemServlet extends DSpaceServlet } } - if (item.isDiscoverable()) + if (item.isDiscoverable()) { request.setAttribute("privating_button", authorizeService .authorizeActionBoolean(context, item, Constants.WRITE)); - } - else + } + else { request.setAttribute("publicize_button", authorizeService .authorizeActionBoolean(context, item, Constants.WRITE)); } - + request.setAttribute("item", item); request.setAttribute("handle", handle); request.setAttribute("collections", collections); @@ -711,7 +711,7 @@ public class EditItemServlet extends DSpaceServlet // Get a string with "element" for unqualified or // "element_qualifier" - String key = metadataFieldService.findByElement(context, + String key = metadataFieldService.findByElement(context, schema,element,qualifier).toString(); // Get the language @@ -878,7 +878,7 @@ public class EditItemServlet extends DSpaceServlet { // Show cc-edit page request.setAttribute("item", item); - + boolean exists = creativeCommonsService.hasLicense(context, item); request.setAttribute("cclicense.exists", Boolean.valueOf(exists)); @@ -886,15 +886,15 @@ public class EditItemServlet extends DSpaceServlet /** Default locale to 'en' */ ccLocale = (StringUtils.isNotBlank(ccLocale)) ? ccLocale : "en"; request.setAttribute("cclicense.locale", ccLocale); - + CCLookup cclookup = new CCLookup(); java.util.Collection collectionLicenses = cclookup.getLicenses(ccLocale); request.setAttribute("cclicense.licenses", collectionLicenses); - + JSPManager .showJSP(request, response, "/tools/creative-commons-edit.jsp"); } - + if (button.equals("submit_addbitstream")) { // Show upload bitstream page @@ -923,11 +923,11 @@ public class EditItemServlet extends DSpaceServlet //Retrieve the button key String inputKey = button.replace("submit_order_", "") + "_value"; if(inputKey.startsWith(bundle.getID() + "_")){ - List vals = Util.getUUIDParameters(request, inputKey); - int idx = 0; - for (UUID v : vals) { - newBitstreamOrder[idx] = v; - idx++; + // Field contains comma-separated, ordered list of Bitstream UUIDs + String[] vals = request.getParameter(inputKey).split(","); + for (int i = 0; i < vals.length; i++) { + String val = vals[i]; + newBitstreamOrder[i] = UUID.fromString(val); } }else{ newBitstreamOrder = null; @@ -950,7 +950,7 @@ public class EditItemServlet extends DSpaceServlet // Show edit page again showEditForm(context, request, response, item); } - + // Complete transaction context.complete(); } @@ -976,11 +976,11 @@ public class EditItemServlet extends DSpaceServlet Bitstream b = null; Item item = itemService.find(context, UIUtil.getUUIDParameter(wrapper, "item_id")); File temp = wrapper.getFile("file"); - + if(temp == null) { boolean noFileSelected = true; - + // Show upload bitstream page request.setAttribute("noFileSelected", noFileSelected); request.setAttribute("item", item); @@ -1010,7 +1010,7 @@ public class EditItemServlet extends DSpaceServlet bundleService.inheritCollectionDefaultPolicies(context, bnd, owningCollection); } - } + } else { // we have a bundle already, just add bitstream diff --git a/dspace-jspui/src/main/java/org/dspace/app/webui/util/JSPManager.java b/dspace-jspui/src/main/java/org/dspace/app/webui/util/JSPManager.java index 8838da68c4..7075012179 100644 --- a/dspace-jspui/src/main/java/org/dspace/app/webui/util/JSPManager.java +++ b/dspace-jspui/src/main/java/org/dspace/app/webui/util/JSPManager.java @@ -57,7 +57,16 @@ public class JSPManager } try { // For the moment, a simple forward - request.getRequestDispatcher(jsp).forward(request, response); + // First test if the response is already committed (could happen by broken downloads), + // if that is the case, forward won't work. + if (!response.isCommitted()) + { + request.getRequestDispatcher(jsp).forward(request, response); + } + else + { + log.warn("Couldn't show jsp, response is already commited."); + } } catch (Exception e) { throw new ServletException(e); } diff --git a/dspace-jspui/src/main/webapp/dspace-admin/index.jsp b/dspace-jspui/src/main/webapp/dspace-admin/index.jsp index 140d215716..4d203cde1a 100644 --- a/dspace-jspui/src/main/webapp/dspace-admin/index.jsp +++ b/dspace-jspui/src/main/webapp/dspace-admin/index.jsp @@ -77,6 +77,12 @@ UIUtil.sendAlert(request, se); JSPManager.showInternalError(request, response); + } finally { + // we need to close the database connection and free the resources + if(context != null && context.isValid()) + { + context.abort(); + } } %> diff --git a/dspace-jspui/src/main/webapp/help/formats.jsp b/dspace-jspui/src/main/webapp/help/formats.jsp index 2f84247870..9b7a823116 100644 --- a/dspace-jspui/src/main/webapp/help/formats.jsp +++ b/dspace-jspui/src/main/webapp/help/formats.jsp @@ -62,6 +62,12 @@ UIUtil.sendAlert(request, se); JSPManager.showInternalError(request, response); + } finally { + // we need to close the database connection and free the resources + if(context != null && context.isValid()) + { + context.abort(); + } } %> diff --git a/dspace-jspui/src/main/webapp/index.jsp b/dspace-jspui/src/main/webapp/index.jsp index d00826c7ed..4d5e3a6a8c 100644 --- a/dspace-jspui/src/main/webapp/index.jsp +++ b/dspace-jspui/src/main/webapp/index.jsp @@ -81,5 +81,11 @@ UIUtil.sendAlert(request, se); JSPManager.showInternalError(request, response); + } finally { + // we need to close the database connection and free the resources + if(context != null && context.isValid()) + { + context.abort(); + } } %> diff --git a/dspace-jspui/src/main/webapp/login/chooser.jsp b/dspace-jspui/src/main/webapp/login/chooser.jsp index 3a9aa3be36..fc35cfb30d 100644 --- a/dspace-jspui/src/main/webapp/login/chooser.jsp +++ b/dspace-jspui/src/main/webapp/login/chooser.jsp @@ -89,6 +89,12 @@ // Also email an alert UIUtil.sendAlert(request, se); JSPManager.showInternalError(request, response); + } finally { + // we need to close the database connection and free the resources + if(context != null && context.isValid()) + { + context.abort(); + } } %> diff --git a/dspace-rest/src/main/java/org/dspace/rest/CollectionsResource.java b/dspace-rest/src/main/java/org/dspace/rest/CollectionsResource.java index 4fd2965cfe..8b897467cb 100644 --- a/dspace-rest/src/main/java/org/dspace/rest/CollectionsResource.java +++ b/dspace-rest/src/main/java/org/dspace/rest/CollectionsResource.java @@ -40,11 +40,14 @@ import org.dspace.content.service.InstallItemService; import org.dspace.content.service.ItemService; import org.dspace.content.service.WorkspaceItemService; import org.dspace.core.Constants; +import org.dspace.core.LogManager; import org.dspace.rest.common.Collection; import org.dspace.rest.common.Item; import org.dspace.rest.common.MetadataEntry; import org.dspace.rest.exceptions.ContextException; import org.dspace.usage.UsageEvent; +import org.dspace.workflow.WorkflowService; +import org.dspace.workflow.factory.WorkflowServiceFactory; /** * This class provides all CRUD operation over collections. @@ -58,7 +61,7 @@ public class CollectionsResource extends Resource protected ItemService itemService = ContentServiceFactory.getInstance().getItemService(); protected AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService(); protected WorkspaceItemService workspaceItemService = ContentServiceFactory.getInstance().getWorkspaceItemService(); - protected InstallItemService installItemService = ContentServiceFactory.getInstance().getInstallItemService(); + protected WorkflowService workflowService = WorkflowServiceFactory.getInstance().getWorkflowService(); private static Logger log = Logger.getLogger(CollectionsResource.class); @@ -365,11 +368,21 @@ public class CollectionsResource extends Resource } } - log.trace("Installing item to collection(id=" + collectionId + ")."); - dspaceItem = installItemService.installItem(context, workspaceItem); workspaceItemService.update(context, workspaceItem); - returnItem = new Item(dspaceItem, servletContext, "", context); + try + { + // Must insert the item into workflow + log.trace("Starting workflow for item(id=" + dspaceItem.getID() + ")."); + workflowService.start(context, workspaceItem); + } + catch (Exception e) + { + log.error(LogManager.getHeader(context, "Error while starting workflow", "Item id: " + dspaceItem.getID()), e); + throw new ContextException("Error while starting workflow for item(id=" + dspaceItem.getID() + ")", e); + } + + returnItem = new Item(workspaceItem.getItem(), servletContext, "",context); context.complete(); diff --git a/dspace-xmlui-mirage2/src/main/webapp/bower.json b/dspace-xmlui-mirage2/src/main/webapp/bower.json index aee656cd4b..2af7363a43 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/bower.json +++ b/dspace-xmlui-mirage2/src/main/webapp/bower.json @@ -7,7 +7,7 @@ "jquery": "~1.10.2", "jquery-ui": "1.10.3", "jqueryuibootstrap": "4f3772cd37b898f456c0126c4b44178ce8d4aad7", - "handlebars": "2.0.0", + "handlebars": "4.0.0", "respond": "1.4.2", "html5shiv": "3.7.2", "holderjs": "2.4.1", diff --git a/dspace-xmlui-mirage2/src/main/webapp/package.json b/dspace-xmlui-mirage2/src/main/webapp/package.json index c93b999c70..9e4d034f12 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/package.json +++ b/dspace-xmlui-mirage2/src/main/webapp/package.json @@ -10,7 +10,7 @@ "grunt-contrib-compass": "^1.1.1", "grunt-contrib-concat": "~1.0.1", "grunt-contrib-copy": "~1.0.0", - "grunt-contrib-handlebars": "0.9.3", + "grunt-contrib-handlebars": "1.0.0", "grunt-contrib-uglify": "~2.0.0", "grunt-usemin": "~3.1.1", "load-grunt-tasks": "~3.5.2", diff --git a/dspace-xmlui-mirage2/src/main/webapp/scripts.xml b/dspace-xmlui-mirage2/src/main/webapp/scripts.xml index 5651a71876..7a7a8fe49f 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/scripts.xml +++ b/dspace-xmlui-mirage2/src/main/webapp/scripts.xml @@ -13,6 +13,10 @@ + + + + diff --git a/dspace-xmlui-mirage2/src/main/webapp/scripts/search-controls.js b/dspace-xmlui-mirage2/src/main/webapp/scripts/search-controls.js index d25641b044..2425a314e5 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/scripts/search-controls.js +++ b/dspace-xmlui-mirage2/src/main/webapp/scripts/search-controls.js @@ -144,7 +144,10 @@ } template = getSimpleFiltersTemplate(); - html = template(DSpace.discovery); + html = template({ + filters: DSpace.discovery.orig_filters, + i18n: DSpace.i18n.discovery + }); unAssignSimpleFilterEventHandlers(); $('#filters-overview-wrapper').remove(); diff --git a/dspace-xmlui-mirage2/src/main/webapp/styles/classic_mirage_color_scheme/_jquery_ui.scss b/dspace-xmlui-mirage2/src/main/webapp/styles/classic_mirage_color_scheme/_jquery_ui.scss index 4a8c513ed7..92fdde30de 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/styles/classic_mirage_color_scheme/_jquery_ui.scss +++ b/dspace-xmlui-mirage2/src/main/webapp/styles/classic_mirage_color_scheme/_jquery_ui.scss @@ -16,7 +16,7 @@ } input[type='text'].ui-autocomplete-loading{ - background-image: url("../../images/authority_control/lookup-indicator.gif"); + background-image: url("../images/authority_control/lookup-indicator.gif"); background-repeat: no-repeat; background-position: 99% 50%; background-size: 25px 25px; diff --git a/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_advanced_filters.hbs b/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_advanced_filters.hbs index b5622179c4..061277173a 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_advanced_filters.hbs +++ b/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_advanced_filters.hbs @@ -15,7 +15,7 @@ {{#set_selected relational_operator}} - {{#each ../../i18n.filter_relational_operator}} + {{#each ../i18n.filter_relational_operator}} {{/each}} {{/set_selected}} diff --git a/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_simple_filters.hbs b/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_simple_filters.hbs index 7600137734..b309fad4ab 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_simple_filters.hbs +++ b/dspace-xmlui-mirage2/src/main/webapp/templates/discovery_simple_filters.hbs @@ -7,6 +7,6 @@ http://www.dspace.org/license/ --> -{{#each orig_filters}} - +{{#each filters}} + {{/each}} \ No newline at end of file diff --git a/dspace-xmlui-mirage2/src/main/webapp/xsl/aspect/general/choice-authority-control.xsl b/dspace-xmlui-mirage2/src/main/webapp/xsl/aspect/general/choice-authority-control.xsl index b3bf0610b4..ac243e0cff 100644 --- a/dspace-xmlui-mirage2/src/main/webapp/xsl/aspect/general/choice-authority-control.xsl +++ b/dspace-xmlui-mirage2/src/main/webapp/xsl/aspect/general/choice-authority-control.xsl @@ -74,7 +74,9 @@ + ' + ' -1 @@ -322,9 +324,9 @@ ', confidenceName: ' - ', collection: + ', collection: ' - , contextPath: ' + ', contextPath: ' '}); }); 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 3a17a9b0ef..e68fc3343a 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 @@ -339,8 +339,8 @@ public class FlowContainerUtils */ public static FlowResult processReimportCollection(Context context, UUID collectionID, Request request) throws SQLException, IOException, AuthorizeException, CrosswalkException, ParserConfigurationException, SAXException, TransformerException, BrowseException { -// Context.Mode originalMode = context.getCurrentMode(); -// context.setMode(Context.Mode.BATCH_EDIT); + Context.Mode originalMode = context.getCurrentMode(); + context.setMode(Context.Mode.BATCH_EDIT); Collection collection = collectionService.find(context, collectionID); HarvestedCollection hc = harvestedCollectionService.find(context, collection); @@ -362,7 +362,7 @@ public class FlowContainerUtils // update the context? //context.dispatchEvent() // not sure if this is required yet.ts(); -// context.setMode(originalMode); + context.setMode(originalMode); return processRunCollectionHarvest(context, collectionID, request); } diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/eperson/UnAuthenticateAction.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/eperson/UnAuthenticateAction.java index 25896e85b3..8e02154462 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/eperson/UnAuthenticateAction.java +++ b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/eperson/UnAuthenticateAction.java @@ -23,6 +23,7 @@ import org.dspace.app.xmlui.utils.ContextUtil; import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.core.Context; import org.dspace.eperson.EPerson; +import org.dspace.services.ConfigurationService; /** * Unauthenticate the current user. There is no way this action will fail, @@ -80,15 +81,17 @@ public class UnAuthenticateAction extends AbstractAction context.setCurrentUser(eperson); // Forward the user to the home page. - if((DSpaceServicesFactory.getInstance().getConfigurationService().getBooleanProperty("xmlui.public.logout")) && (httpRequest.isSecure())) { + ConfigurationService configurationService + = DSpaceServicesFactory.getInstance().getConfigurationService(); + if((configurationService.getBooleanProperty("xmlui.public.logout")) + && (httpRequest.isSecure())) { StringBuffer location = new StringBuffer("http://"); - location.append(DSpaceServicesFactory.getInstance().getConfigurationService().getProperty("dspace.hostname")).append( - httpRequest.getContextPath()); + location.append(configurationService.getProperty("dspace.hostname")) + .append(httpRequest.getContextPath()); httpResponse.sendRedirect(location.toString()); - } else{ - httpResponse.sendRedirect(httpRequest.getContextPath()); + httpResponse.sendRedirect(configurationService.getProperty("dspace.url")); } return new HashMap(); diff --git a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/workflow/Submissions.java b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/workflow/Submissions.java index 9a72f48705..707ded9758 100644 --- a/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/workflow/Submissions.java +++ b/dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/workflow/Submissions.java @@ -353,31 +353,31 @@ public class Submissions extends AbstractDSpaceTransformer { String title = workflowItem.getItem().getName(); String collectionName = workflowItem.getCollection().getName(); - Message state = getWorkflowStateMessage(workflowItem); + Message state = getWorkflowStateMessage(workflowItem); - Row row = table.addRow(); + Row row = table.addRow(); - // Add the title column - if (title.length() > 0) - { - String displayTitle = title; - if (displayTitle.length() > 50) + // Add the title column + if (StringUtils.isNotBlank(title)) + { + String displayTitle = title; + if (displayTitle.length() > 50) { displayTitle = displayTitle.substring(0, 50) + " ..."; } - row.addCellContent(displayTitle); - } - else + row.addCellContent(displayTitle); + } + else { row.addCellContent(T_untitled); } - // Collection name column - row.addCellContent(collectionName); + // Collection name column + row.addCellContent(collectionName); - // Status column - row.addCellContent(state); + // Status column + row.addCellContent(state); } } diff --git a/dspace-xmlui/src/main/resources/aspects/Versioning/versioning.js b/dspace-xmlui/src/main/resources/aspects/Versioning/versioning.js index 91ac43126e..5580e9e8f6 100644 --- a/dspace-xmlui/src/main/resources/aspects/Versioning/versioning.js +++ b/dspace-xmlui/src/main/resources/aspects/Versioning/versioning.js @@ -127,6 +127,7 @@ function doCreateNewVersion(itemID, result){ result = VersionManager.processCreateNewVersion(getDSContext(),itemID, summary); var wsid = result.getParameter("wsid"); + getDSContext().complete(); cocoon.redirectTo(cocoon.request.getContextPath()+"/submit?workspaceID=" + wsid,true); cocoon.exit(); } diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index 1823efe2ea..86930ecd4d 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -96,8 +96,8 @@ db.maxconnections = 30 db.maxwait = 5000 # Maximum number of idle connections in pool (-1 = unlimited) -# (default = -1, unlimited) -db.maxidle = -1 +# (default = 10) +db.maxidle = 10 # Specify a configured database connection pool to be fetched from a # directory. This overrides the pool and driver settings above. If diff --git a/dspace/config/hibernate-ehcache-config.xml b/dspace/config/hibernate-ehcache-config.xml index d0caf41c0d..45d75fe1a5 100644 --- a/dspace/config/hibernate-ehcache-config.xml +++ b/dspace/config/hibernate-ehcache-config.xml @@ -330,7 +330,7 @@ diff --git a/dspace/config/local.cfg.EXAMPLE b/dspace/config/local.cfg.EXAMPLE index 140bfe5d74..6efb4e3ada 100644 --- a/dspace/config/local.cfg.EXAMPLE +++ b/dspace/config/local.cfg.EXAMPLE @@ -104,8 +104,8 @@ db.schema = public #db.maxwait = 5000 # Maximum number of idle connections in pool (-1 = unlimited) -# (default = -1, unlimited) -#db.maxidle = -1 +# (default = 10) +#db.maxidle = 10 ####################### diff --git a/dspace/config/modules/usage-statistics.cfg b/dspace/config/modules/usage-statistics.cfg index 2bc7e24b2b..335355a3c7 100644 --- a/dspace/config/modules/usage-statistics.cfg +++ b/dspace/config/modules/usage-statistics.cfg @@ -35,3 +35,7 @@ usage-statistics.authorization.admin.workflow=true # (see query.filter.* for query filter options) # Default value is true. #usage-statistics.logBots = true + +# Enable/disable if a matching for a bot should be case sensitive +# Setting this value to true will increase cpu usage, but bots will be found more accurately +#usage-statistics.bots.case-insensitive = false \ No newline at end of file diff --git a/dspace/config/spring/api/core-services.xml b/dspace/config/spring/api/core-services.xml index 795d2da4ed..2c6160f55f 100644 --- a/dspace/config/spring/api/core-services.xml +++ b/dspace/config/spring/api/core-services.xml @@ -96,6 +96,8 @@ + +