[maven-release-plugin] prepare release dspace-4.9

fixes for DS-3866 (for 4.x branch)

.travis.yml

@@ -1,2 +1,29 @@
 language: java
-script: "mvn clean package license:check -Dmaven.test.skip=false"
+sudo: false
+
+env: 
+  # Give Maven 1GB of memory to work with
+  - MAVEN_OPTS=-Xmx1024M
+
+before_install:
+  # Remove outdated settings.xml from Travis builds. Workaround for https://github.com/travis-ci/travis-ci/issues/4629
+  - rm ~/.m2/settings.xml
+
+# Skip install stage, as we'll do it below
+install: "echo 'Skipping install stage, dependencies will be downloaded during build and test stages.'"
+
+# Two stage Build and Test
+# 1. Install & Unit Test APIs
+# 2. Assemble DSpace
+script:
+  # 1. [Install & Unit Test] Check source code licenses and run source code Unit Tests
+  #    (This explicitly skips building the 'dspace' assembly module, since we only want to do that ONCE.)
+  #        license:check => Validate all source code license headers
+  #        -Dmaven.test.skip=false => Enable DSpace Unit Tests
+  #        -P !dspace => SKIP full DSpace assembly (will do below)
+  #        -B => Maven batch/non-interactive mode (recommended for CI)
+  #        -V => Display Maven version info before build
+  - "mvn clean install license:check -Dmaven.test.skip=false -P !dspace -B -V"
+  # 2. [Assemble DSpace] Ensure overlay & assembly process works (from [src]/dspace/)
+  #        -P !assembly => SKIP the actual building of [src]/dspace/dspace-installer (as it can be memory intensive)
+  - "cd dspace && mvn package -P !assembly -B -V"

LICENSE

@@ -1,7 +1,7 @@
 DSpace source code license:
 
 
-Copyright (c) 2002-2013, DuraSpace.  All rights reserved.
+Copyright (c) 2002-2017, DuraSpace.  All rights reserved.
 
 Redistribution and use in source and binary forms, with or without
 modification, are permitted provided that the following conditions are

README

@@ -1,6 +1,9 @@
-Installation instructions are included in this release package under
+Installation instructions may be found in DSpace-Manual.pdf, offered 
+with the source archives in the SourceForge project:
 
- - dspace/docs/DSpace-Manual.pdf
+  http://sourceforge.net/projects/dspace/files/DSpace%20Stable/VERSION/
+
+(Replace "VERSION" with the version you are installing, such as "4.0".)
 
 DSpace version information can be viewed online at
  - https://wiki.duraspace.org/display/DSDOC/

dspace-api/pom.xml

@@ -12,7 +12,7 @@
     <parent>
         <groupId>org.dspace</groupId>
         <artifactId>dspace-parent</artifactId>
-        <version>4.0</version>
+        <version>4.9</version>
         <relativePath>..</relativePath>
     </parent>
 
@@ -436,12 +436,12 @@
         <dependency>
             <groupId>gr.ekt.bte</groupId>
             <artifactId>bte-core</artifactId>
-            <version>0.9.3.3</version>
+            <version>0.9.2.4</version>
         </dependency>
         <dependency>
             <groupId>gr.ekt.bte</groupId>
             <artifactId>bte-io</artifactId>
-            <version>0.9.3.3</version>
+            <version>0.9.2.4</version>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
@@ -463,6 +463,21 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>org.apache.solr</groupId>
+            <artifactId>solr-cell</artifactId>
+            <version>${solr.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>org.slf4j</groupId>
+                    <artifactId>jcl-over-slf4j</artifactId>
+                </exclusion>
+                <exclusion>
+                    <groupId>org.slf4j</groupId>
+                    <artifactId>slf4j-api</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
         <dependency>
             <groupId>commons-configuration</groupId>
             <artifactId>commons-configuration</artifactId>

dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImportInvalidHeadingException.java

@@ -89,16 +89,16 @@ public class MetadataImportInvalidHeadingException extends Exception
     {
         if (type == SCHEMA)
         {
-            return "Unknown metadata schema in row " + column + ": " + badHeading;
+            return "Unknown metadata schema in column " + column + ": " + badHeading;
         } else if (type == ELEMENT)
         {
-            return "Unknown metadata element in row " + column + ": " + badHeading;
+            return "Unknown metadata element in column " + column + ": " + badHeading;
         } else if (type == MISSING)
         {
-            return "Row with missing header: Row " + column;
+            return "Row with missing header: column " + column;
         } else
         {
-            return "Bad metadata declaration in row " + column + ": " + badHeading;
+            return "Bad metadata declaration in column" + column + ": " + badHeading;
         }
     }
-}
+}

dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java

@@ -7,6 +7,7 @@
  */
 package org.dspace.app.statistics;
 
+import org.dspace.content.MetadataSchema;
 import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
 import org.dspace.core.LogManager;
@@ -153,10 +154,10 @@ public class LogAnalyser
    private static Pattern logRegex = null;
    
    /** pattern to match commented out lines from the config file */
-   private static Pattern comment = Pattern.compile("^#");
+   private static final Pattern comment = Pattern.compile("^#");
         
    /** pattern to match genuine lines from the config file */
-   private static Pattern real = Pattern.compile("^(.+)=(.+)");
+   private static final Pattern real = Pattern.compile("^(.+)=(.+)");
    
    /** pattern to match all search types */
    private static Pattern typeRX = null;
@@ -1165,8 +1166,12 @@ public class LogAnalyser
                         "AND metadata_field_id = (" +
                         " SELECT metadata_field_id " +
                         " FROM metadatafieldregistry " +
-                        " WHERE element = 'type' " +
-                        " AND qualifier IS NULL) ";
+                        " WHERE metadata_schema_id = (" +
+                        "  SELECT metadata_schema_id" +
+                        "   FROM MetadataSchemaRegistry" +
+                        "   WHERE short_id = '" + MetadataSchema.DC_SCHEMA + "')" +
+                        "  AND element = 'type' " +
+                        "  AND qualifier IS NULL) ";
         }
         
         // start the date constraint query buffer
@@ -1184,8 +1189,12 @@ public class LogAnalyser
                           "WHERE metadata_field_id = (" +
                           " SELECT metadata_field_id " +
                           " FROM metadatafieldregistry " +
-                          " WHERE element = 'date' " +
-                          " AND qualifier = 'accessioned') ");
+                          " WHERE metadata_schema_id = (" +
+                          "  SELECT metadata_schema_id" +
+                          "   FROM MetadataSchemaRegistry" +
+                          "   WHERE short_id = '" + MetadataSchema.DC_SCHEMA + "')" +
+                          "  AND element = 'date' " +
+                          "  AND qualifier = 'accessioned') ");
 
         // Verifies that the metadata contains a valid date, otherwise the
         // postgres queries blow up when doing the ::timestamp cast.

dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java

@@ -7,11 +7,7 @@
  */
 package org.dspace.authenticate;
 
-import java.sql.SQLException;
-
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
+import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.core.ConfigurationManager;
@@ -20,6 +16,10 @@ import org.dspace.core.LogManager;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.sql.SQLException;
+
 /**
  * A stackable authentication method
  * based on the DSpace internal "EPerson" database.
@@ -128,7 +128,7 @@ public class PasswordAuthentication
 		// ensures they are password users
 		try
 		{
-			if (!context.getCurrentUser().getMetadata("password").equals(""))
+		if (context.getCurrentUser() != null && context.getCurrentUser().getPasswordHash()!=null && StringUtils.isNotBlank(context.getCurrentUser().getPasswordHash().toString()))
 			{
 				String groupName = ConfigurationManager.getProperty("authentication-password", "login.specialgroup");
 				if ((groupName != null) && (!groupName.trim().equals("")))
@@ -149,7 +149,7 @@ public class PasswordAuthentication
 			}
 		}
 		catch (Exception e) {
-			// The user is not a password user, so we don't need to worry about them
+            log.error(LogManager.getHeader(context,"getSpecialGroups",""),e);
 		}
 		return new int[0];
     }

dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java

@@ -26,6 +26,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.logging.Level;
 
 import javax.servlet.http.HttpServletResponse;
 
@@ -1094,6 +1095,22 @@ public class ShibAuthentication implements AuthenticationMethod
 			value = request.getHeader(name.toLowerCase());
 		if (StringUtils.isEmpty(value))
 			value = request.getHeader(name.toUpperCase());
+                
+                boolean reconvertAttributes = 
+                        ConfigurationManager.getBooleanProperty(
+                            "authentication-shibboleth",
+                            "reconvert.attributes",
+                            false);
+                
+                if (!StringUtils.isEmpty(value) && reconvertAttributes)
+                {
+                    try {
+                        value = new String(value.getBytes("ISO-8859-1"), "UTF-8");
+                    } catch (UnsupportedEncodingException ex) {
+                        log.warn("Failed to reconvert shibboleth attribute ("
+                                + name + ").", ex);
+                    }
+                }
 		
 		return value;
 	}

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

@@ -375,7 +375,7 @@ public class AuthorizeManager
                 }
 
                 if ((rp.getGroupID() != -1)
-                        && (Group.isMember(c, rp.getGroupID())))
+                        && (Group.isMember(c, e, rp.getGroupID())))
                 {
                     // group was set, and eperson is a member
                     // of that group
@@ -480,7 +480,9 @@ public class AuthorizeManager
 
         rp.update();
 
+        c.turnOffAuthorisationSystem();
         o.updateLastModified();
+        c.restoreAuthSystemState();
     }
 
     /**
@@ -534,8 +536,10 @@ public class AuthorizeManager
         rp.setRpType(type);
 
         rp.update();
-
+        
+        c.turnOffAuthorisationSystem();
         o.updateLastModified();
+        c.restoreAuthSystemState();
     }
 
     /**
@@ -799,7 +803,9 @@ public class AuthorizeManager
             drp.update();
         }
 
+        c.turnOffAuthorisationSystem();
         dest.updateLastModified();
+        c.restoreAuthSystemState();
     }
 
     /**
@@ -815,12 +821,14 @@ public class AuthorizeManager
     public static void removeAllPolicies(Context c, DSpaceObject o)
             throws SQLException
     {
-        o.updateLastModified();
-
         // FIXME: authorization check?
         DatabaseManager.updateQuery(c, "DELETE FROM resourcepolicy WHERE "
                 + "resource_type_id= ? AND resource_id= ? ",
                 o.getType(), o.getID());
+        
+        c.turnOffAuthorisationSystem();
+        o.updateLastModified();
+        c.restoreAuthSystemState();
     }
 
     /**
@@ -879,7 +887,6 @@ public class AuthorizeManager
     public static void removePoliciesActionFilter(Context context,
                                                   DSpaceObject dso, int actionID) throws SQLException
     {
-        dso.updateLastModified();
         if (actionID == -1)
         {
             // remove all policies from object
@@ -891,6 +898,10 @@ public class AuthorizeManager
                             "resource_id= ? AND action_id= ? ",
                     dso.getType(), dso.getID(), actionID);
         }
+        
+        context.turnOffAuthorisationSystem();
+        dso.updateLastModified();
+        context.restoreAuthSystemState();
     }
 
     /**
@@ -927,13 +938,38 @@ public class AuthorizeManager
     public static void removeGroupPolicies(Context c, DSpaceObject o, Group g)
             throws SQLException
     {
-        o.updateLastModified();
-
         DatabaseManager.updateQuery(c, "DELETE FROM resourcepolicy WHERE "
                 + "resource_type_id= ? AND resource_id= ? AND epersongroup_id= ? ",
                 o.getType(), o.getID(), g.getID());
+        
+        c.turnOffAuthorisationSystem();
+        o.updateLastModified();
+        c.restoreAuthSystemState();
     }
 
+    /**
+    * Change all the policies related to the action (fromPolicy) of the
+    * specified object to the new action (toPolicy)
+    * 
+    * @param context
+    * @param dso
+    *            the dspace object
+    * @param fromAction
+    *            the action to change
+    * @param toAction
+    *            the new action to set
+    * @throws SQLException
+    * @throws AuthorizeException
+    */
+	public static void switchPoliciesAction(Context context, DSpaceObject dso, int fromAction, int toAction)
+			throws SQLException, AuthorizeException {
+		List<ResourcePolicy> rps = getPoliciesActionFilter(context, dso, fromAction);
+		for (ResourcePolicy rp : rps) {
+			rp.setAction(toAction);
+			rp.update();
+		}
+	}
+    
     /**
      * Removes all policies from an eperson for a particular object that belong to
      * an EPerson. FIXME doesn't check authorization
@@ -950,10 +986,13 @@ public class AuthorizeManager
     public static void removeEPersonPolicies(Context c, DSpaceObject o, EPerson e)
             throws SQLException
     {
-        o.updateLastModified();
         DatabaseManager.updateQuery(c, "DELETE FROM resourcepolicy WHERE "
                 + "resource_type_id= ? AND resource_id= ? AND eperson_id= ? ",
                 o.getType(), o.getID(), e.getID());
+        
+        c.turnOffAuthorisationSystem();
+        o.updateLastModified();
+        c.restoreAuthSystemState();
     }
 
     /**

dspace-api/src/main/java/org/dspace/browse/SolrBrowseDAO.java

@@ -31,7 +31,8 @@ import org.dspace.utils.DSpace;
 /**
  * 
  * @author Andrea Bollini (CILEA)
- *
+ * @author Adán Román Ruiz at arvo.es (bugfix)
+ * 
  */
 public class SolrBrowseDAO implements BrowseDAO
 {
@@ -337,11 +338,11 @@ public class SolrBrowseDAO implements BrowseDAO
         query.addFilterQueries("search.resourcetype:" + Constants.ITEM);
         if (isAscending)
         {
-            query.setQuery("bi_"+column + "_sort" + ": [* TO \"" + value + "\"]");
+            query.setQuery("bi_"+column + "_sort" + ": [* TO \"" + value + "\"}");
         }
         else
         {
-            query.setQuery("bi_" + column + "_sort" + ": [\"" + value + "\" TO *]");
+            query.setQuery("bi_" + column + "_sort" + ": {\"" + value + "\" TO *]");
         }
         DiscoverResult resp = null;
         try

dspace-api/src/main/java/org/dspace/checker/DailyReportEmailer.java

@@ -65,12 +65,16 @@ public class DailyReportEmailer
     public void sendReport(File attachment, int numberOfBitstreams) 
             throws IOException, javax.mail.MessagingException 
     { 
-        Email email = new Email(); 
-        email.setSubject("Checksum checker Report - " + numberOfBitstreams + " Bitstreams found with POSSIBLE issues"); 
-        email.setContent("report is attached ..."); 
-        email.addAttachment(attachment, "checksum_checker_report.txt"); 
-        email.addRecipient(ConfigurationManager.getProperty("mail.admin")); 
-        email.send(); 
+        if(numberOfBitstreams > 0)
+        {
+            String hostname = ConfigurationManager.getProperty("dspace.hostname");
+            Email email = new Email();
+            email.setSubject("Checksum checker Report - " + numberOfBitstreams + " Bitstreams found with POSSIBLE issues on " + hostname);
+            email.setContent("report is attached ...");
+            email.addAttachment(attachment, "checksum_checker_report.txt");
+            email.addRecipient(ConfigurationManager.getProperty("mail.admin"));
+            email.send();
+        }
     } 
 
     /**

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

@@ -28,6 +28,7 @@ import org.dspace.workflow.WorkflowItem;
 import org.dspace.xmlworkflow.storedcomponents.CollectionRole;
 import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem;
 
+import java.io.Serializable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.PreparedStatement;
@@ -50,7 +51,6 @@ import java.util.MissingResourceException;
  * effect.
  *
  * @author Robert Tansley
- * @version $Revision$
  */
 public class Collection extends DSpaceObject
 {
@@ -350,8 +350,14 @@ public class Collection extends DSpaceObject
      */
     public static Collection[] findAll(Context context, Integer limit, Integer offset) throws SQLException
     {
-        TableRowIterator tri = DatabaseManager.queryTable(context, "collection",
-                "SELECT * FROM collection ORDER BY name limit ? offset ?", limit, offset);
+        StringBuffer query = new StringBuffer("SELECT * FROM collection ORDER BY name");
+        List<Serializable> params = new ArrayList<Serializable>();
+
+        DatabaseManager.applyOffsetAndLimit(query, params, offset, limit);
+
+        TableRowIterator tri = DatabaseManager.query(
+          context, query.toString(), params.toArray()
+        );
 
         List<Collection> collections = new ArrayList<Collection>();
 
@@ -419,13 +425,20 @@ public class Collection extends DSpaceObject
      */
     public ItemIterator getItems(Integer limit, Integer offset) throws SQLException
     {
-        String myQuery = "SELECT item.* FROM item, collection2item WHERE "
-                + "item.item_id=collection2item.item_id AND "
-                + "collection2item.collection_id= ? "
-                + "AND item.in_archive='1' limit ? offset ?";
+        List<Serializable> params = new ArrayList<Serializable>();
+        StringBuffer myQuery = new StringBuffer(
+            "SELECT item.* " + 
+            "FROM item, collection2item " + 
+            "WHERE item.item_id = collection2item.item_id " +
+              "AND collection2item.collection_id = ? " +
+              "AND item.in_archive = '1'"
+        );
 
-        TableRowIterator rows = DatabaseManager.queryTable(ourContext, "item",
-                myQuery,getID(), limit, offset);
+        params.add(getID());
+        DatabaseManager.applyOffsetAndLimit(myQuery, params, offset, limit);
+
+        TableRowIterator rows = DatabaseManager.query(ourContext,
+                myQuery.toString(), params.toArray());
 
         return new ItemIterator(ourContext, rows);
     }
@@ -644,8 +657,6 @@ public class Collection extends DSpaceObject
             g.setName("COLLECTION_" + getID() + "_WORKFLOW_STEP_" + step);
             g.update();
             setWorkflowGroup(step, g);
-
-            AuthorizeManager.addPolicy(ourContext, this, Constants.ADD, g);
         }
 
         return workflowGroup[step - 1];
@@ -654,26 +665,82 @@ public class Collection extends DSpaceObject
     /**
      * Set the workflow group corresponding to a particular workflow step.
      * <code>null</code> can be passed in if there should be no associated
-     * group for that workflow step; any existing group is NOT deleted.
+     * group for that workflow step.  Any existing group is NOT deleted.
      *
      * @param step
      *            the workflow step (1-3)
-     * @param g
+     * @param newGroup
      *            the new workflow group, or <code>null</code>
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
-    public void setWorkflowGroup(int step, Group g)
+    public void setWorkflowGroup(int step, Group newGroup)
+            throws SQLException, AuthorizeException
     {
-        workflowGroup[step - 1] = g;
-
-        if (g == null)
+        Group oldGroup = getWorkflowGroup(step);
+        String stepColumn;
+        int action;
+        switch(step)
         {
-            collectionRow.setColumnNull("workflow_step_" + step);
+        case 1:
+            action = Constants.WORKFLOW_STEP_1;
+            stepColumn = "workflow_step_1";
+            break;
+        case 2:
+            action = Constants.WORKFLOW_STEP_2;
+            stepColumn = "workflow_step_2";
+            break;
+        case 3:
+            action = Constants.WORKFLOW_STEP_3;
+            stepColumn = "workflow_step_3";
+            break;
+        default:
+            throw new IllegalArgumentException("Illegal step count:  " + step);
         }
+        workflowGroup[step-1] = newGroup;
+        if (newGroup != null)
+            collectionRow.setColumn(stepColumn, newGroup.getID());
         else
-        {
-            collectionRow.setColumn("workflow_step_" + step, g.getID());
-        }
+            collectionRow.setColumnNull(stepColumn);
         modified = true;
+
+        // Deal with permissions.
+        try {
+            ourContext.turnOffAuthorisationSystem();
+            // remove the policies for the old group
+            if (oldGroup != null)
+            {
+                List<ResourcePolicy> oldPolicies = AuthorizeManager
+                        .getPoliciesActionFilter(ourContext, this, action);
+                int oldGroupID = oldGroup.getID();
+                for (ResourcePolicy rp : oldPolicies)
+                {
+                    if (rp.getGroupID() == oldGroupID)
+                        rp.delete();
+                }
+
+                oldPolicies = AuthorizeManager
+                        .getPoliciesActionFilter(ourContext, this, Constants.ADD);
+                for (ResourcePolicy rp : oldPolicies)
+                {
+                    if ((rp.getGroupID() == oldGroupID)
+                            && ResourcePolicy.TYPE_WORKFLOW.equals(rp.getRpType()))
+                        rp.delete();
+                }
+           }
+
+            // New group can be null to delete workflow step.
+            // We need to grant permissions if new group is not null.
+            if (newGroup != null)
+            {
+                AuthorizeManager.addPolicy(ourContext, this, action, newGroup,
+                        ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(ourContext, this, Constants.ADD, newGroup,
+                        ResourcePolicy.TYPE_WORKFLOW);
+            }
+        } finally {
+            ourContext.restoreAuthSystemState();
+        }
     }
 
     /**

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

@@ -28,6 +28,9 @@ import org.dspace.authorize.AuthorizeManager;
 import org.dspace.authorize.ResourcePolicy;
 import org.dspace.browse.BrowseException;
 import org.dspace.browse.IndexBrowse;
+import org.dspace.content.authority.ChoiceAuthorityManager;
+import org.dspace.content.authority.Choices;
+import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.core.LogManager;
@@ -37,6 +40,7 @@ import org.dspace.content.authority.MetadataAuthorityManager;
 import org.dspace.event.Event;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
+import org.dspace.event.Event;
 import org.dspace.handle.HandleManager;
 import org.dspace.identifier.IdentifierException;
 import org.dspace.identifier.IdentifierService;
@@ -45,6 +49,8 @@ import org.dspace.storage.rdbms.TableRow;
 import org.dspace.storage.rdbms.TableRowIterator;
 import org.dspace.utils.DSpace;
 import org.dspace.versioning.VersioningService;
+import org.dspace.workflow.WorkflowItem;
+import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem;
 
 /**
  * Class representing an item in DSpace.
@@ -1904,8 +1910,14 @@ public class Item extends DSpaceObject
 
         ourContext.addEvent(new Event(Event.MODIFY, Constants.ITEM, getID(), "WITHDRAW"));
 
-        // remove all authorization policies, saving the custom ones
-        AuthorizeManager.removeAllPoliciesByDSOAndTypeNotEqualsTo(ourContext, this, ResourcePolicy.TYPE_CUSTOM);
+        // switch all READ authorization policies to WITHDRAWN_READ
+		AuthorizeManager.switchPoliciesAction(ourContext, this, Constants.READ, Constants.WITHDRAWN_READ);
+		for (Bundle bnd : this.getBundles()) {
+			AuthorizeManager.switchPoliciesAction(ourContext, bnd, Constants.READ, Constants.WITHDRAWN_READ);
+			for (Bitstream bs : bnd.getBitstreams()) {
+				AuthorizeManager.switchPoliciesAction(ourContext, bs, Constants.READ, Constants.WITHDRAWN_READ);
+			}
+		}
 
         // Write log
         log.info(LogManager.getHeader(ourContext, "withdraw_item", "user="
@@ -1961,15 +1973,25 @@ public class Item extends DSpaceObject
 
         ourContext.addEvent(new Event(Event.MODIFY, Constants.ITEM, getID(), "REINSTATE"));
 
-        // authorization policies
-        if (colls.length > 0)
-        {
-            // FIXME: not multiple inclusion friendly - just apply access
-            // policies from first collection
-            // remove the item's policies and replace them with
-            // the defaults from the collection
-            inheritCollectionDefaultPolicies(colls[0]);
-        }
+        // restore all WITHDRAWN_READ authorization policies back to READ
+		for (Bundle bnd : this.getBundles()) {
+			AuthorizeManager.switchPoliciesAction(ourContext, bnd, Constants.WITHDRAWN_READ, Constants.READ);
+			for (Bitstream bs : bnd.getBitstreams()) {
+				AuthorizeManager.switchPoliciesAction(ourContext, bs, Constants.WITHDRAWN_READ, Constants.READ);
+			}
+		}
+
+		// check if the item was withdrawn before the fix DS-3097
+		if (AuthorizeManager.getPoliciesActionFilter(ourContext, this, Constants.WITHDRAWN_READ).size() != 0) {
+			AuthorizeManager.switchPoliciesAction(ourContext, this, Constants.WITHDRAWN_READ, Constants.READ);
+		} else {
+			// authorization policies
+			if (colls.length > 0) {
+				// remove the item's policies and replace them with
+				// the defaults from the collection
+				adjustItemPolicies(getOwningCollection());
+			}
+		}
 
         // Write log
         log.info(LogManager.getHeader(ourContext, "reinstate_item", "user="
@@ -2505,7 +2527,12 @@ public class Item extends DSpaceObject
         // is this collection not yet created, and an item template is created
         if (getOwningCollection() == null)
         {
-            return true;
+        	if (!isInProgressSubmission()) {
+        		return true;
+        	}
+        	else {
+        		return false;
+        	}
         }
 
         // is this person an COLLECTION_EDITOR for the owning collection?
@@ -2517,6 +2544,20 @@ public class Item extends DSpaceObject
         return false;
     }
     
+    /**
+     * Check if the item is an inprogress submission
+     * @param context
+     * @param item
+     * @return <code>true</code> if the item is an inprogress submission, i.e. a WorkspaceItem or WorkflowItem
+     * @throws SQLException
+     */
+    public boolean isInProgressSubmission() throws SQLException {
+		return WorkspaceItem.findByItem(ourContext, this) != null ||
+				((ConfigurationManager.getProperty("workflow", "workflow.framework").equals("xmlworkflow")
+						&& XmlWorkflowItem.findByItem(ourContext, this) != null)
+						|| WorkflowItem.findByItem(ourContext, this) != null);
+    }
+    
     public String getName()
     {
         DCValue t[] = getMetadata("dc", "title", null, Item.ANY);

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

@@ -81,6 +81,7 @@ public class WorkspaceItem implements InProgressSubmission
      *            ID of the workspace item
      * 
      * @return the workspace item, or null if the ID is invalid.
+     * @throws java.sql.SQLException passed through.
      */
     public static WorkspaceItem find(Context context, int id)
             throws SQLException
@@ -131,6 +132,9 @@ public class WorkspaceItem implements InProgressSubmission
      *            of the collection's template item
      * 
      * @return the newly created workspace item
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     * @throws java.sql.SQLException passed through.
+     * @throws java.io.IOException passed through.
      */
     public static WorkspaceItem create(Context c, Collection coll,
             boolean template) throws AuthorizeException, SQLException,
@@ -140,98 +144,49 @@ public class WorkspaceItem implements InProgressSubmission
         AuthorizeManager.authorizeAction(c, coll, Constants.ADD);
 
         // Create an item
-        Item i = Item.create(c);
-        i.setSubmitter(c.getCurrentUser());
+        Item item = Item.create(c);
+        item.setSubmitter(c.getCurrentUser());
 
-        // Now create the policies for the submitter and workflow
-        // users to modify item and contents
+        // Now create the policies for the submitter to modify item and contents.
         // contents = bitstreams, bundles
         // FIXME: icky hardcoded workflow steps
         Group step1group = coll.getWorkflowGroup(1);
         Group step2group = coll.getWorkflowGroup(2);
         Group step3group = coll.getWorkflowGroup(3);
 
-        EPerson e = c.getCurrentUser();
+        EPerson submitter = c.getCurrentUser();
 
-        // read permission
-        AuthorizeManager.addPolicy(c, i, Constants.READ, e, ResourcePolicy.TYPE_SUBMISSION);
+        // Add policies for the submitter
+        AuthorizeManager.addPolicy(c, item, Constants.READ, submitter, ResourcePolicy.TYPE_SUBMISSION);
+        AuthorizeManager.addPolicy(c, item, Constants.WRITE, submitter, ResourcePolicy.TYPE_SUBMISSION);
+        AuthorizeManager.addPolicy(c, item, Constants.ADD, submitter, ResourcePolicy.TYPE_SUBMISSION);
+        AuthorizeManager.addPolicy(c, item, Constants.REMOVE, submitter, ResourcePolicy.TYPE_SUBMISSION);
 
-
-        if (ConfigurationManager.getProperty("workflow", "workflow.framework").equals("originalworkflow")) {
+        if (ConfigurationManager.getProperty("workflow", "workflow.framework").equals("originalworkflow"))
+        {
+            // Add policies for the workflow step administrative groups
             if (step1group != null)
             {
-                AuthorizeManager.addPolicy(c, i, Constants.READ, step1group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.READ, step1group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.WRITE, step1group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.ADD, step1group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.REMOVE, step1group, ResourcePolicy.TYPE_WORKFLOW);
             }
 
             if (step2group != null)
             {
-                AuthorizeManager.addPolicy(c, i, Constants.READ, step2group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.READ, step2group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.WRITE, step2group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.ADD, step2group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.REMOVE, step2group, ResourcePolicy.TYPE_WORKFLOW);
             }
 
             if (step3group != null)
             {
-                AuthorizeManager.addPolicy(c, i, Constants.READ, step3group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-        }
-
-
-        // write permission
-        AuthorizeManager.addPolicy(c, i, Constants.WRITE, e, ResourcePolicy.TYPE_SUBMISSION);
-
-        if (ConfigurationManager.getProperty("workflow", "workflow.framework").equals("originalworkflow")) {
-            if (step1group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.WRITE, step1group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-
-            if (step2group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.WRITE, step2group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-
-            if (step3group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.WRITE, step3group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-        }
-
-        // add permission
-        AuthorizeManager.addPolicy(c, i, Constants.ADD, e, ResourcePolicy.TYPE_SUBMISSION);
-
-        if (ConfigurationManager.getProperty("workflow", "workflow.framework").equals("originalworkflow")) {
-            if (step1group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.ADD, step1group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-
-            if (step2group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.ADD, step2group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-
-            if (step3group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.ADD, step3group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-        }
-
-        // remove contents permission
-        AuthorizeManager.addPolicy(c, i, Constants.REMOVE, e, ResourcePolicy.TYPE_SUBMISSION);
-
-        if (ConfigurationManager.getProperty("workflow", "workflow.framework").equals("originalworkflow")) {
-            if (step1group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.REMOVE, step1group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-
-            if (step2group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.REMOVE, step2group, ResourcePolicy.TYPE_WORKFLOW);
-            }
-
-            if (step3group != null)
-            {
-                AuthorizeManager.addPolicy(c, i, Constants.REMOVE, step3group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.READ, step3group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.WRITE, step3group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.ADD, step3group, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(c, item, Constants.REMOVE, step3group, ResourcePolicy.TYPE_WORKFLOW);
             }
         }
 
@@ -244,22 +199,22 @@ public class WorkspaceItem implements InProgressSubmission
 
             for (int n = 0; n < md.length; n++)
             {
-                i.addMetadata(md[n].schema, md[n].element, md[n].qualifier, md[n].language,
+                item.addMetadata(md[n].schema, md[n].element, md[n].qualifier, md[n].language,
                         md[n].value);
             }
         }
 
-        i.update();
+        item.update();
 
         // Create the workspace item row
         TableRow row = DatabaseManager.row("workspaceitem");
 
-        row.setColumn("item_id", i.getID());
+        row.setColumn("item_id", item.getID());
         row.setColumn("collection_id", coll.getID());
 
         log.info(LogManager.getHeader(c, "create_workspace_item",
                 "workspace_item_id=" + row.getIntColumn("workspace_item_id")
-                        + "item_id=" + i.getID() + "collection_id="
+                        + "item_id=" + item.getID() + "collection_id="
                         + coll.getID()));
 
         DatabaseManager.insert(c, row);
@@ -280,6 +235,7 @@ public class WorkspaceItem implements InProgressSubmission
      *            the eperson
      * 
      * @return the corresponding workspace items
+     * @throws java.sql.SQLException passed through.
      */
     public static WorkspaceItem[] findByEPerson(Context context, EPerson ep)
             throws SQLException
@@ -332,6 +288,7 @@ public class WorkspaceItem implements InProgressSubmission
      *            the collection
      * 
      * @return the corresponding workspace items
+     * @throws java.sql.SQLException passed through.
      */
     public static WorkspaceItem[] findByCollection(Context context, Collection c)
             throws SQLException
@@ -384,6 +341,7 @@ public class WorkspaceItem implements InProgressSubmission
      *            the item
      *
      * @return workflow item corresponding to the item, or null
+     * @throws java.sql.SQLException passed through.
      */
     public static WorkspaceItem findByItem(Context context, Item i)
             throws SQLException
@@ -408,6 +366,7 @@ public class WorkspaceItem implements InProgressSubmission
      * @param   context     the context object
      *
      * @return      all workspace items
+     * @throws java.sql.SQLException passed through.
      */
     public static WorkspaceItem[] findAll(Context context)
         throws SQLException
@@ -505,6 +464,8 @@ public class WorkspaceItem implements InProgressSubmission
 
     /**
      * Update the workspace item, including the unarchived item.
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
     public void update() throws SQLException, AuthorizeException
     {
@@ -554,6 +515,10 @@ public class WorkspaceItem implements InProgressSubmission
      * Delete the workspace item. The entry in workspaceitem, the unarchived
      * item and its contents are all removed (multiple inclusion
      * notwithstanding.)
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException
+     *          if not original submitter or an administrator.
+     * @throws java.io.IOException passed through.
      */
     public void deleteAll() throws SQLException, AuthorizeException,
             IOException

dspace-api/src/main/java/org/dspace/content/dao/ItemDAOOracle.java

@@ -9,25 +9,107 @@ package org.dspace.content.dao;
 
 import org.dspace.core.Context;
 import org.dspace.content.Bitstream;
+import org.dspace.storage.rdbms.DatabaseManager;
+import org.dspace.storage.rdbms.TableRowIterator;
+import org.dspace.storage.rdbms.TableRow;
 
 import java.sql.SQLException;
 
 public class ItemDAOOracle extends ItemDAO
-{
+{		
+		private static final String selectPrimaryBitstreamID =
+            "SELECT bundle.primary_bitstream_id FROM item2bundle, bundle " +
+            "WHERE item2bundle.item_id=? AND item2bundle.bundle_id=bundle.bundle_id AND bundle.name=?";
+
+    private static final String selectFirstBitstreamID =
+        "SELECT bundle2bitstream.bitstream_id FROM item2bundle, bundle, bundle2bitstream " +
+        "WHERE item2bundle.item_id=? AND item2bundle.bundle_id=bundle.bundle_id AND bundle.name=? " +
+        "AND bundle.bundle_id=bundle2bitstream.bundle_id";
+    
+		private static final String selectNamedBitstreamID =
+        "SELECT bitstream.bitstream_id FROM item2bundle, bundle, bundle2bitstream, bitstream " +
+        "WHERE item2bundle.item_id=? AND item2bundle.bundle_id=bundle.bundle_id AND bundle.name=? " +
+        "AND bundle.bundle_id=bundle2bitstream.bundle_id AND bundle2bitstream.bitstream_id=bitstream.bitstream_id " +
+        "AND bitstream.name=?";
+    
     ItemDAOOracle(Context ctx)
     {
         super(ctx);
     }
 
-    public Bitstream getPrimaryBitstream(int itemId, String bundleName) throws SQLException {
-        return null;  //To change body of implemented methods use File | Settings | File Templates.
+    public Bitstream getPrimaryBitstream(int itemId, String bundleName) throws SQLException
+    {
+        TableRowIterator tri = null;
+
+        try
+        {
+            tri = DatabaseManager.query(context, selectPrimaryBitstreamID, itemId, bundleName);
+
+            if (tri.hasNext())
+            {
+                TableRow row = tri.next();
+                int bid = row.getIntColumn("primary_bitstream_id");
+                return Bitstream.find(context, bid);
+            }
+        }        
+        finally
+        {
+            if (tri != null)
+            {
+                tri.close();
+            }
+        }
+
+        return null;
     }
 
-    public Bitstream getFirstBitstream(int itemId, String bundleName) throws SQLException {
-        return null;  //To change body of implemented methods use File | Settings | File Templates.
+    public Bitstream getFirstBitstream(int itemId, String bundleName) throws SQLException
+    {
+        TableRowIterator tri = null;
+
+        try
+        {
+            tri = DatabaseManager.query(context, selectFirstBitstreamID, itemId, bundleName);
+            if (tri.hasNext())
+            {
+                TableRow row = tri.next();
+                int bid = row.getIntColumn("bitstream_id");
+                return Bitstream.find(context, bid);
+            }
+        }
+        finally
+        {
+            if (tri != null)
+            {
+                tri.close();
+            }
+        }
+
+        return null;
     }
 
-    public Bitstream getNamedBitstream(int itemId, String bundleName, String fileName) throws SQLException {
-        return null;  //To change body of implemented methods use File | Settings | File Templates.
+    public Bitstream getNamedBitstream(int itemId, String bundleName, String fileName) throws SQLException
+    {
+        TableRowIterator tri = null;
+
+        try
+        {
+            tri = DatabaseManager.query(context, selectNamedBitstreamID, itemId, bundleName, fileName);
+            if (tri.hasNext())
+            {
+                TableRow row = tri.next();
+                int bid = row.getIntColumn("bitstream_id");
+                return Bitstream.find(context, bid);
+            }
+        }
+        finally
+        {
+            if (tri != null)
+            {
+                tri.close();
+            }
+        }
+
+        return null;
     }
 }

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

@@ -127,6 +127,8 @@ public class Constants
      */
     public static final int ADMIN = 11;
     
+    public static final int WITHDRAWN_READ = 12;
+    
     /** Position of front page news item -- top box */
     public static final int NEWS_TOP = 0;
 
@@ -139,7 +141,7 @@ public class Constants
     public static final String[] actionText = { "READ", "WRITE",
             "OBSOLETE (DELETE)", "ADD", "REMOVE", "WORKFLOW_STEP_1",
             "WORKFLOW_STEP_2", "WORKFLOW_STEP_3", "WORKFLOW_ABORT",
-            "DEFAULT_BITSTREAM_READ", "DEFAULT_ITEM_READ", "ADMIN" };
+            "DEFAULT_BITSTREAM_READ", "DEFAULT_ITEM_READ", "ADMIN", "WITHDRAWN_READ" };
 
     /**
      * generating constants for the relevance array dynamically is simple: just
@@ -175,7 +177,8 @@ public class Constants
             0, // 8 - WORKFLOW_ABORT
             RCOLLECTION, // 9 - DEFAULT_BITSTREAM_READ
             RCOLLECTION, // 10 - DEFAULT_ITEM_READ
-            RITEM | RCOLLECTION | RCOMMUNITY // 11 - ADMIN
+            RITEM | RCOLLECTION | RCOMMUNITY, // 11 - ADMIN
+            RBITSTREAM | RBUNDLE | RITEM // 12 - WITHDRAWN_READ
     };
 
     public static final String DEFAULT_ENCODING = "UTF-8";

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

@@ -16,6 +16,9 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
+
+import org.dspace.core.service.NewsService;
+import org.dspace.utils.DSpace;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -39,6 +42,10 @@ public class NewsManager
      */
     public static String readNewsFile(String newsFile)
     {
+    	NewsService newsService = new DSpace().getSingletonService(NewsService.class);
+    	if (!newsService.validate(newsFile)) {
+    		throw new IllegalArgumentException("The file "+ newsFile + " is not a valid news file");
+    	}
         String fileName = getNewsFilePath();
 
         fileName += newsFile;
@@ -81,6 +88,10 @@ public class NewsManager
      */
     public static String writeNewsFile(String newsFile, String news)
     {
+    	NewsService newsService = new DSpace().getSingletonService(NewsService.class);
+    	if (!newsService.validate(newsFile)) {
+    		throw new IllegalArgumentException("The file "+ newsFile + " is not a valid news file");
+    	}
         String fileName = getNewsFilePath();
 
         fileName += newsFile;

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

@@ -0,0 +1,29 @@
+/**
+ * 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 java.util.List;
+
+import org.dspace.core.service.NewsService;
+
+public class NewsServiceImpl implements NewsService {
+	private List<String> acceptableFilenames;
+	
+	public void setAcceptableFilenames(List<String> acceptableFilenames) {
+		this.acceptableFilenames = acceptableFilenames;
+	}
+	
+	@Override
+	public boolean validate(String newsName) {
+		if (acceptableFilenames != null) {
+			return acceptableFilenames.contains(newsName);
+		}
+		return false;
+	}
+
+}

dspace-api/src/main/java/org/dspace/core/service/NewsService.java

@@ -0,0 +1,12 @@
+/**
+ * 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.service;
+
+public interface NewsService {
+	boolean validate(String newsName);
+}

dspace-api/src/main/java/org/dspace/curate/TaskResolver.java

@@ -29,36 +29,50 @@ import org.dspace.core.PluginManager;
 /**
  * TaskResolver takes a logical name of a curation task and attempts to deliver 
  * a suitable implementation object. Supported implementation types include:
- * (1) Classpath-local Java classes configured and loaded via PluginManager.
- * (2) Local script-based tasks, viz. coded in any scripting language whose
+ * <ol>
+ *  <li> Classpath-local Java classes configured and loaded via PluginManager</li>.
+ *  <li> Local script-based tasks, viz. coded in any scripting language whose
  * runtimes are accessible via the JSR-223 scripting API. This really amounts
- * to the family of dynamic JVM languages: JRuby, Jython, Groovy, Javascript, etc
- * Note that the requisite jars and other resources for these languages must be
- * installed in the DSpace instance for them to be used here.
+ * to the family of dynamic JVM languages: JRuby, Jython, Groovy, Javascript, etc.</li>
+ * </ol>
+ * <p>
+ * Note that the requisite jars and other resources for these languages must
+ * be installed in the DSpace instance for them to be used here.
  * Further work may involve remote URL-loadable code, etc. 
  * 
+ * <p>
  * Scripted tasks are managed in a directory configured with the
- * dspace/config/modules/curate.cfg property "script.dir". A catalog of
- * scripted tasks named 'task.catalog" is kept in this directory.
+ * {@code dspace/config/modules/curate.cfg} property "script.dir".
+ * A catalog of
+ * scripted tasks named "task.catalog" is kept in this directory.
  * Each task has a 'descriptor' property with value syntax:
- * <engine>|<relFilePath>|<implClassCtor>
+ * <br/>
+ * {@code <engine>|<relFilePath>|<implClassCtor>}
+ *
+ * <p>
  * An example property:
- * 
- * linkchecker = ruby|rubytask.rb|LinkChecker.new
- * 
+ * <br/>
+ * {@code linkchecker = ruby|rubytask.rb|LinkChecker.new}
+ *
+ * <p>
  * This descriptor means that a 'ruby' script engine will be created,
- * a script file named 'rubytask.rb' in the directory <script.dir> will be
- * loaded and the resolver will expect an evaluation of 'LinkChecker.new' will 
- * provide a correct implementation object.
- * 
+ * a script file named 'rubytask.rb' in the directory {@code <script.dir>}
+ * will be
+ * loaded and the resolver will expect an evaluation of 'LinkChecker.new'
+ * will provide a correct implementation object.
+ *
+ * <p>
  * Script files may embed their descriptors to facilitate deployment.
- * To accomplish this, a script must include the descriptor string with syntax:
- * $td=<descriptor> somewhere on a comment line. for example:
- * 
- * # My descriptor $td=ruby|rubytask.rb|LinkChecker.new
- * 
- * For portability, the <relFilePath> component may be omitted in this context.
- * Thus, $td=ruby||LinkChecker.new will be expanded to a descriptor
+ * To accomplish this, a script must include the descriptor string with
+ * syntax {@code $td=<descriptor>} somewhere on a comment line. For example:
+ *
+ * <p>
+ * {@code # My descriptor $td=ruby|rubytask.rb|LinkChecker.new}
+ *
+ * <p>
+ * For portability, the {@code <relFilePath>} component may be omitted in
+ * this context.
+ * Thus, {@code $td=ruby||LinkChecker.new} will be expanded to a descriptor
  * with the name of the embedding file.
  * 
  * @author richardrodgers

dspace-api/src/main/java/org/dspace/discovery/BitstreamContentStream.java

@@ -0,0 +1,73 @@
+/**
+ * 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.discovery;
+
+import org.apache.log4j.Logger;
+import org.apache.solr.common.util.ContentStreamBase;
+import org.dspace.content.Bitstream;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+
+/**
+ * Construct a <code>ContentStream</code> from a <code>File</code>
+ */
+public class BitstreamContentStream extends ContentStreamBase
+{
+    private static final Logger log = Logger.getLogger(BitstreamContentStream.class);
+    private final Bitstream file;
+
+    public BitstreamContentStream( Bitstream f ) {
+        file = f;
+
+        contentType = f.getFormat().getMIMEType();
+        name = file.getName();
+        size = file.getSize();
+        sourceInfo = file.getName();
+    }
+
+    @Override
+    public String getContentType() {
+        if(contentType==null) {
+            InputStream stream = null;
+            try {
+                stream = file.retrieve();
+                char first = (char)stream.read();
+                if(first == '<') {
+                    return "application/xml";
+                }
+                if(first == '{') {
+                    return "application/json";
+                }
+            } catch(Exception ex) {
+                log.error("Error determining content type for bitstream:" + file.getID(), ex);
+            } finally {
+                if (stream != null) try {
+                    stream.close();
+                } catch (IOException ioe) {
+                    log.error("Error closing stream:" + file.getID(), ioe);
+
+                }
+            }
+        }
+        return contentType;
+    }
+
+    @Override
+    public InputStream getStream() throws IOException {
+        try {
+            return file.retrieve();
+        } catch (Exception e) {
+            log.error(e.getMessage(),e);
+            return new ByteArrayInputStream(e.getMessage().getBytes(StandardCharsets.UTF_8));
+        }
+    }
+}
+

dspace-api/src/main/java/org/dspace/discovery/IndexClient.java

@@ -66,6 +66,10 @@ public class IndexClient {
                 "(re)build index, wiping out current one if it exists").create(
                 "b"));
 
+        options.addOption(OptionBuilder.isRequired(false).withDescription(
+                "Rebuild the spellchecker, can be combined with -b and -f.").create(
+                "s"));
+
         options
                 .addOption(OptionBuilder
                         .isRequired(false)
@@ -111,15 +115,32 @@ public class IndexClient {
         } else if (line.hasOption("b")) {
             log.info("(Re)building index from scratch.");
             indexer.createIndex(context);
+            checkRebuildSpellCheck(line, indexer);
         } else if (line.hasOption("o")) {
             log.info("Optimizing search core.");
             indexer.optimize();
+        } else if(line.hasOption('s')) {
+            checkRebuildSpellCheck(line, indexer);
         } else {
             log.info("Updating and Cleaning Index");
             indexer.cleanIndex(line.hasOption("f"));
             indexer.updateIndex(context, line.hasOption("f"));
+            checkRebuildSpellCheck(line, indexer);
         }
 
         log.info("Done with indexing");
 	}
+
+    /**
+     * Check the command line options and rebuild the spell check if active.
+     * @param line the command line options
+     * @param indexer the solr indexer
+     * @throws SearchServiceException in case of a solr exception
+     */
+    protected static void checkRebuildSpellCheck(CommandLine line, IndexingService indexer) throws SearchServiceException {
+        if (line.hasOption("s")) {
+            log.info("Rebuilding spell checker.");
+            indexer.buildSpellCheck();
+        }
+    }
 }

dspace-api/src/main/java/org/dspace/discovery/IndexingService.java

@@ -7,6 +7,7 @@
  */
 package org.dspace.discovery;
 
+import org.apache.solr.client.solrj.SolrServerException;
 import org.dspace.content.DSpaceObject;
 import org.dspace.core.Context;
 
@@ -58,4 +59,6 @@ public interface IndexingService {
     void commit() throws SearchServiceException;
 
     void optimize() throws SearchServiceException;
+
+    void buildSpellCheck() throws SearchServiceException;
 }

dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java

@@ -7,12 +7,7 @@
  */
 package org.dspace.discovery;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import java.net.MalformedURLException;
+import java.io.*;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.sql.SQLException;
@@ -31,7 +26,6 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.TimeZone;
-import java.util.TreeMap;
 import java.util.Vector;
 
 import org.apache.commons.collections.CollectionUtils;
@@ -39,7 +33,6 @@ import org.apache.commons.collections.MapUtils;
 import org.apache.commons.collections.Transformer;
 import org.apache.commons.httpclient.HttpClient;
 import org.apache.commons.httpclient.methods.GetMethod;
-import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.time.DateFormatUtils;
@@ -53,6 +46,8 @@ import org.apache.log4j.Logger;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.HttpSolrServer;
+import org.apache.solr.client.solrj.request.AbstractUpdateRequest;
+import org.apache.solr.client.solrj.request.ContentStreamUpdateRequest;
 import org.apache.solr.client.solrj.response.FacetField;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.client.solrj.util.ClientUtils;
@@ -61,6 +56,7 @@ import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.*;
 import org.apache.solr.common.util.NamedList;
+import org.apache.solr.handler.extraction.ExtractingParams;
 import org.dspace.content.Bitstream;
 import org.dspace.content.Bundle;
 import org.dspace.content.Collection;
@@ -148,7 +144,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                     solr = new HttpSolrServer(solrService);
 
                     solr.setBaseURL(solrService);
-
+                    solr.setUseMultiPartPost(true);
                     SolrQuery solrQuery = new SolrQuery()
                             .setQuery("search.resourcetype:2 AND search.resourceid:1");
 
@@ -504,11 +500,11 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                 return;
             }
             long start = System.currentTimeMillis();
-            System.out.println("SOLR Search Optimize -- Process Started:"+start);
+            System.out.println("SOLR Search Optimize -- Process Started:" + start);
             getSolr().optimize();
             long finish = System.currentTimeMillis();
-            System.out.println("SOLR Search Optimize -- Process Finished:"+finish);
-            System.out.println("SOLR Search Optimize -- Total time taken:"+(finish-start) + " (ms).");
+            System.out.println("SOLR Search Optimize -- Process Finished:" + finish);
+            System.out.println("SOLR Search Optimize -- Total time taken:" + (finish - start) + " (ms).");
         } catch (SolrServerException sse)
         {
             System.err.println(sse.getMessage());
@@ -518,6 +514,23 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         }
     }
 
+    public void buildSpellCheck() throws SearchServiceException {
+        try {
+            if (getSolr() == null) {
+                return;
+            }
+            SolrQuery solrQuery = new SolrQuery();
+            solrQuery.set("spellcheck", true);
+            solrQuery.set(SpellingParams.SPELLCHECK_BUILD, true);
+            getSolr().query(solrQuery);
+        }catch (SolrServerException e)
+        {
+            //Make sure to also log the exception since this command is usually run from a crontab.
+            log.error(e, e);
+            throw new SearchServiceException(e);
+        }
+    }
+
     // //////////////////////////////////
     // Private
     // //////////////////////////////////
@@ -666,15 +679,47 @@ public class SolrServiceImpl implements SearchService, IndexingService {
 
     /**
      * Write the document to the index under the appropriate handle.
+     *
      * @param doc the solr document to be written to the server
+     * @param streams
      * @throws IOException IO exception
      */
-    protected void writeDocument(SolrInputDocument doc) throws IOException {
+    protected void writeDocument(SolrInputDocument doc, List<BitstreamContentStream> streams) throws IOException {
 
         try {
             if(getSolr() != null)
             {
-                getSolr().add(doc);
+                if(CollectionUtils.isNotEmpty(streams))
+                {
+                    ContentStreamUpdateRequest req = new ContentStreamUpdateRequest("/update/extract");
+
+                    for(BitstreamContentStream bce : streams)
+                    {
+                        req.addContentStream(bce);
+                    }
+
+                    ModifiableSolrParams params = new ModifiableSolrParams();
+
+                    //req.setParam(ExtractingParams.EXTRACT_ONLY, "true");
+                    for(String name : doc.getFieldNames())
+                    {
+                        for(Object val : doc.getFieldValues(name))
+                        {
+                             params.add(ExtractingParams.LITERALS_PREFIX + name,val.toString());
+                        }
+                    }
+
+                    req.setParams(params);
+                    req.setParam(ExtractingParams.UNKNOWN_FIELD_PREFIX, "attr_");
+                    req.setParam(ExtractingParams.MAP_PREFIX + "content", "fulltext");
+                    req.setParam(ExtractingParams.EXTRACT_FORMAT, "text");
+                    req.setAction(AbstractUpdateRequest.ACTION.COMMIT, true, true);
+                    req.process(getSolr());
+                }
+                else
+                {
+                    getSolr().add(doc);
+                }
             }
         } catch (SolrServerException e)
         {
@@ -727,7 +772,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
             solrServiceIndexPlugin.additionalIndex(context, community, doc);
         }
 
-        writeDocument(doc);
+        writeDocument(doc, null);
     }
 
     /**
@@ -783,7 +828,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
             solrServiceIndexPlugin.additionalIndex(context, collection, doc);
         }
 
-        writeDocument(doc);
+        writeDocument(doc, null);
     }
 
     /**
@@ -1300,7 +1345,8 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         log.debug("  Added Grouping");
 
 
-        Vector<InputStreamReader> readers = new Vector<InputStreamReader>();
+
+        List<BitstreamContentStream> streams = new ArrayList<BitstreamContentStream>();
 
         try {
             // now get full text of any bitstreams in the TEXT bundle
@@ -1318,18 +1364,8 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                     for (Bitstream myBitstream : myBitstreams)
                     {
                         try {
-                            InputStreamReader is = new InputStreamReader(
-                                    myBitstream.retrieve()); // get input
-                            readers.add(is);
 
-                            // Add each InputStream to the Indexed Document
-                            String value = IOUtils.toString(is);
-                            doc.addField("fulltext", value);
-
-                            if(hitHighlightingFields.contains("*") || hitHighlightingFields.contains("fulltext"))
-                            {
-                                doc.addField("fulltext_hl", value);
-                            }
+                            streams.add(new BitstreamContentStream(myBitstream));
 
                             log.debug("  Added BitStream: "
                                     + myBitstream.getStoreNumber() + "	"
@@ -1349,16 +1385,6 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         {
             log.error(e.getMessage(), e);
         }
-        finally {
-            Iterator<InputStreamReader> itr = readers.iterator();
-            while (itr.hasNext()) {
-                InputStreamReader reader = itr.next();
-                if (reader != null) {
-                    reader.close();
-                }
-            }
-            log.debug("closed " + readers.size() + " readers");
-        }
 
         //Do any additional indexing, depends on the plugins
         List<SolrServiceIndexPlugin> solrServiceIndexPlugins = new DSpace().getServiceManager().getServicesByType(SolrServiceIndexPlugin.class);
@@ -1369,7 +1395,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
 
         // write the index and close the inputstreamreaders
         try {
-            writeDocument(doc);
+            writeDocument(doc, streams);
             log.info("Wrote Item: " + handle + " to Index");
         } catch (RuntimeException e)
         {

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

@@ -357,9 +357,9 @@ public class Group extends DSpaceObject
     }
 
     /**
-     * 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
+     * fast check to see if the current EPerson is a member of a Group.  Does
+     * database lookup without instantiating all of the EPerson objects and is
+     * thus a static method.
      * 
      * @param c
      *            context
@@ -379,6 +379,29 @@ public class Group extends DSpaceObject
         return epersonInGroup(c, groupid, currentuser);
     }
 
+    /**
+     * Fast check to see if a given EPerson is a member of a Group.
+     * Does database lookup without instantiating all of the EPerson objects and
+     * is thus a static method.
+     *
+     * @param c current DSpace context.
+     * @param eperson candidate to test for membership.
+     * @param groupid group whose membership is to be tested.
+     * @return true if {@link eperson} is a member of Group {@link groupid}.
+     * @throws SQLException passed through
+     */
+    public static boolean isMember(Context c, EPerson eperson, int groupid)
+            throws SQLException
+    {
+        // Every EPerson is a member of Anonymous
+        if (groupid == 0)
+        {
+            return true;
+        }
+
+        return epersonInGroup(c, groupid, eperson);
+    }
+
     /**
      * Get all of the groups that an eperson is a member of.
      * 

dspace-api/src/main/java/org/dspace/handle/UpdateHandlePrefix.java

@@ -9,89 +9,167 @@ package org.dspace.handle;
 
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
-
+import java.sql.SQLException;
+import org.apache.log4j.Logger;
 import org.dspace.core.Context;
 import org.dspace.storage.rdbms.DatabaseManager;
 import org.dspace.storage.rdbms.TableRow;
-import org.dspace.search.DSIndexer;
-import org.dspace.browse.IndexBrowse;
+import org.dspace.discovery.IndexClient;
 
 /**
  * A script to update the handle values in the database. This is typically used
- * when moving from a test machine (handle = 123456789) to a production service.
+ * when moving from a test machine (handle = 123456789) to a production service
+ * or when make a test clone from production service.
  *
  * @author Stuart Lewis
+ * @author Ivo Prajer (Czech Technical University in Prague)
  */
 public class UpdateHandlePrefix
 {
+
+    private static final Logger log = Logger.getLogger(UpdateHandlePrefix.class);
+
+    /**
+     * When invoked as a command-line tool, updates handle prefix
+     *
+     * @param args the command-line arguments, none used
+     * @throws java.lang.Exception
+     *
+     */
     public static void main(String[] args) throws Exception
     {
-        // There should be two paramters
+        // There should be two parameters
         if (args.length < 2)
         {
             System.out.println("\nUsage: update-handle-prefix <old handle> <new handle>\n");
+            System.exit(1);
         }
         else
         {
-            // Confirm with the user that this is what they want to do
             String oldH = args[0];
             String newH = args[1];
 
-            BufferedReader input = new BufferedReader(new InputStreamReader(System.in));
+            // Get info about changes
+            System.out.println("\nGetting information about handles from database...");
             Context context = new Context();
-            System.out.println("If you continue, all handles in your repository with prefix " +
-                                oldH + " will be updated to have handle prefix " + newH + "\n");
-            String sql = "SELECT count(*) as count FROM handle " +
+            String sql = "SELECT count(*) as count " +
+                         "FROM handle " +
                          "WHERE handle LIKE '" + oldH + "%'";
             TableRow row = DatabaseManager.querySingle(context, sql, new Object[] {});
             long count = row.getLongColumn("count");
-            System.out.println(count + " items will be updated.\n");
-            System.out.print("Have you taken a backup, and are you ready to continue? [y/n]: ");
-            String choiceString = input.readLine();
 
-            if (choiceString.equalsIgnoreCase("y"))
+            if (count > 0)
             {
-                // Make the changes
-                System.out.print("Updating handle table... ");
-                sql = "update handle set handle = '" + newH + "' || '/' || handle_id " +
-                      "where handle like '" + oldH + "/%'";
-                int updated = DatabaseManager.updateQuery(context, sql, new Object[] {});
-                System.out.println(updated + " items updated");
+                // Print info text about changes
+                System.out.println(
+                  "In your repository will be updated " + count + " handle" +
+                  ((count > 1) ? "s" : "") + " to new prefix " + newH +
+                  " from original " + oldH + "!\n"
+                );
 
-                System.out.print("Updating metadatavalues table... ");
-                sql = "UPDATE metadatavalue SET text_value= (SELECT 'http://hdl.handle.net/' || " +
-                      "handle FROM handle WHERE handle.resource_id=item_id AND " +
-                      "handle.resource_type_id=2) WHERE  text_value LIKE 'http://hdl.handle.net/%';";
-                updated = DatabaseManager.updateQuery(context, sql, new Object[] {});
-                System.out.println(updated + " metadata values updated");
+                // Confirm with the user that this is what they want to do
+                System.out.print(
+                  "Servlet container (e.g. Apache Tomcat, Jetty, Caucho Resin) must be running.\n" +
+                  "If it is necessary, please make a backup of the database.\n" +
+                  "Are you ready to continue? [y/n]: "
+                );
+                BufferedReader input = new BufferedReader(new InputStreamReader(System.in));
+                String choiceString = input.readLine();
 
-                // Commit the changes
-                context.complete();
-
-                System.out.print("Re-creating browse and search indexes... ");                
-
-                // Reinitialise the browse system
-                IndexBrowse.main(new String[] {"-i"});
-
-                // Reinitialise the browse system
-                try
+                if (choiceString.equalsIgnoreCase("y"))
                 {
-                    DSIndexer.main(new String[0]);
-                }
-                catch (Exception e)
-                {
-                    // Not a lot we can do
-                    System.out.println("Error re-indexing:");
-                    e.printStackTrace();
-                    System.out.println("\nPlease manually run [dspace]/bin/index-all");
-                }
+                    try {
+                        log.info("Updating handle prefix from " + oldH + " to " + newH);
 
-                // All done
-                System.out.println("\nHandles successfully updated.");
+                        // Make the changes
+                        System.out.print("\nUpdating handle table... ");
+                        sql = "UPDATE handle " +
+                              "SET handle = '" + newH + "' || '/' || handle_id " +
+                              "WHERE handle like '" + oldH + "/%'";
+                        int updHdl = DatabaseManager.updateQuery(context, sql, new Object[] {});
+                        System.out.println(
+                          updHdl + " item" + ((updHdl > 1) ? "s" : "") + " updated"
+                        );
+
+                        System.out.print("Updating metadatavalues table... ");
+                        sql = "UPDATE metadatavalue " +
+                              "SET text_value = " +
+                                "(" +
+                                  "SELECT 'http://hdl.handle.net/' || handle " +
+                                  "FROM handle " +
+                                  "WHERE handle.resource_id = metadatavalue.item_id " +
+                                    "AND handle.resource_type_id = 2" +
+                                ") " +
+                              "WHERE text_value LIKE 'http://hdl.handle.net/" + oldH + "/%'" +
+                                "AND EXISTS " +
+                                  "(" +
+                                    "SELECT 1 " +
+                                    "FROM handle " + 
+                                    "WHERE handle.resource_id = metadatavalue.item_id " +
+                                      "AND handle.resource_type_id = 2" +
+                                  ")";
+                        int updMeta = DatabaseManager.updateQuery(context, sql, new Object[] {});
+                        System.out.println(
+                          updMeta + " metadata value" + ((updMeta > 1) ? "s" : "") + " updated"
+                        );
+
+                        // Commit the changes
+                        context.complete();
+
+                        log.info(
+                          "Done with updating handle prefix. " +
+                          "It was changed " + updHdl + " handle" + ((updHdl > 1) ? "s" : "") +
+                          " and " + updMeta + " metadata record" + ((updMeta > 1) ? "s" : "")
+                        );
+
+                    }
+                    catch (SQLException sqle)
+                    {
+                        if ((context != null) && (context.isValid()))
+                        {
+                            context.abort();
+                            context = null;
+                        }
+                        System.out.println("\nError during SQL operations.");
+                        throw sqle;
+                    }
+
+                    System.out.println("Handles successfully updated in database.\n");
+                    System.out.println("Re-creating browse and search indexes...");
+
+                    try
+                    {
+                        // Reinitialise the search and browse system
+                        IndexClient.main(new String[] {"-b"});
+                        System.out.println("Browse and search indexes are ready now.");
+                        // All done
+                        System.out.println("\nAll done successfully. Please check the DSpace logs!\n");
+                    }
+                    catch (Exception e)
+                    {
+                        // Not a lot we can do
+                        System.out.println("Error during re-indexing.");
+                        System.out.println(
+                          "\n\nAutomatic re-indexing failed. Please perform it manually.\n" +
+                          "You should run one of the following commands:\n\n" +
+                          "  [dspace]/bin/dspace index-discovery -b\n\n" +
+                          "If you are using Solr for browse (this is the default setting).\n" +
+                          "When launching this command, your servlet container must be running.\n\n" +
+                          "  [dspace]/bin/dspace index-lucene-init\n\n" +
+                          "If you enabled Lucene for search.\n" +
+                          "When launching this command, your servlet container must be shutdown.\n"
+                        );
+                        throw e;
+                    }
+                }
+                else
+                {
+                    System.out.println("No changes have been made to your data.\n");
+                }
             }
             else
             {
-                System.out.println("No changes have been made to your data.");
+                System.out.println("Nothing to do! All handles are up-to-date.\n");
             }
         }
     }

dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java

@@ -29,6 +29,7 @@ import java.util.TimeZone;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.TransformerException;
 
+import ORG.oclc.oai.harvester2.verb.*;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
@@ -64,12 +65,6 @@ import org.jdom.input.DOMBuilder;
 import org.jdom.output.XMLOutputter;
 import org.xml.sax.SAXException;
 
-import ORG.oclc.oai.harvester2.verb.GetRecord;
-import ORG.oclc.oai.harvester2.verb.Identify;
-import ORG.oclc.oai.harvester2.verb.ListMetadataFormats;
-import ORG.oclc.oai.harvester2.verb.ListRecords;
-import ORG.oclc.oai.harvester2.verb.ListSets;
-
 
 /**
  * This class handles OAI harvesting of externally located records into this repository.
@@ -369,7 +364,13 @@ public class OAIHarvester {
 				else {
 					listRecords = new ListRecords(oaiSource, resumptionToken);
 				}
-				targetCollection.update();
+                ourContext.turnOffAuthorisationSystem();
+                try {
+                    targetCollection.update();
+                } finally {
+                    //In case of an exception, make sure to restore our authentication state to the previous state
+                    ourContext.restoreAuthSystemState();
+                }
 				ourContext.commit();
 			}
 		}
@@ -394,6 +395,7 @@ public class OAIHarvester {
 		}
 		finally {
 			harvestRow.update();
+            ourContext.turnOffAuthorisationSystem();
 			targetCollection.update();
 			ourContext.commit();
 			ourContext.restoreAuthSystemState();
@@ -954,29 +956,21 @@ public class OAIHarvester {
     	try {
             //If we do not want to harvest from one set, then skip this.
     		if(!"all".equals(oaiSetId)){
-                ListSets ls = new ListSets(oaiSource);
+                ListIdentifiers ls = new ListIdentifiers(oaiSource, null, null, oaiSetId, DMDOAIPrefix);
 
                 // The only error we can really get here is "noSetHierarchy"
                 if (ls.getErrors() != null && ls.getErrors().getLength() > 0) {
                     for (int i=0; i<ls.getErrors().getLength(); i++) {
                         String errorCode = ls.getErrors().item(i).getAttributes().getNamedItem("code").getTextContent();
-                        errorSet.add(errorCode);
+                        errorSet.add(OAI_SET_ERROR + ": The OAI server does not have a set with the specified setSpec (" + errorCode + ")");
                     }
                 }
                 else {
                     // Drilling down to /OAI-PMH/ListSets/set
                     Document reply = db.build(ls.getDocument());
                     Element root = reply.getRootElement();
-                    List<Element> sets= root.getChild("ListSets",OAI_NS).getChildren("set",OAI_NS);
-
-                    for (Element set : sets)
-                    {
-                        String setSpec = set.getChildText("setSpec", OAI_NS);
-                        if (setSpec.equals(oaiSetId)) {
-                            foundSet = true;
-                            break;
-                        }
-                    }
+                    //Check if we can find items, if so this indicates that we have children and our sets exist
+                    foundSet = 0 < root.getChild("ListIdentifiers",OAI_NS).getChildren().size();
 
                     if (!foundSet) {
                         errorSet.add(OAI_SET_ERROR + ": The OAI server does not have a set with the specified setSpec");

dspace-api/src/main/java/org/dspace/identifier/DOIIdentifierProvider.java

@@ -10,7 +10,6 @@ package org.dspace.identifier;
 
 import java.sql.SQLException;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.DCValue;
@@ -68,12 +67,12 @@ public class DOIIdentifierProvider
     public static final String DOI_QUALIFIER = "uri";
     
     public static final Integer TO_BE_REGISTERED = 1;
-    public static final Integer TO_BE_RESERVERED = 2;
+    public static final Integer TO_BE_RESERVED = 2;
     public static final Integer IS_REGISTERED = 3;
     public static final Integer IS_RESERVED = 4;
-    public static final Integer UPDATE_RESERVERED = 5;
+    public static final Integer UPDATE_RESERVED = 5;
     public static final Integer UPDATE_REGISTERED = 6;
-    public static final Integer UPDATE_BEFORE_REGISTERATION = 7;
+    public static final Integer UPDATE_BEFORE_REGISTRATION = 7;
     public static final Integer TO_BE_DELETED = 8;
     public static final Integer DELETED = 9;
     
@@ -251,7 +250,7 @@ public class DOIIdentifierProvider
             return;
         } 
                 
-        doiRow.setColumn("status", TO_BE_RESERVERED);
+        doiRow.setColumn("status", TO_BE_RESERVED);
         try
         {
             DatabaseManager.update(context, doiRow);
@@ -352,11 +351,11 @@ public class DOIIdentifierProvider
         }
         else if (TO_BE_REGISTERED == doiRow.getIntColumn("status")) 
         {
-            doiRow.setColumn("status", UPDATE_BEFORE_REGISTERATION);
+            doiRow.setColumn("status", UPDATE_BEFORE_REGISTRATION);
         }
         else if (IS_RESERVED == doiRow.getIntColumn("status")) 
         {
-            doiRow.setColumn("status", UPDATE_RESERVERED);
+            doiRow.setColumn("status", UPDATE_RESERVED);
         }
         else
         {
@@ -415,11 +414,11 @@ public class DOIIdentifierProvider
         {
             doiRow.setColumn("status", IS_REGISTERED);
         }
-        else if (UPDATE_BEFORE_REGISTERATION == doiRow.getIntColumn("status")) 
+        else if (UPDATE_BEFORE_REGISTRATION == doiRow.getIntColumn("status"))
         {
             doiRow.setColumn("status", TO_BE_REGISTERED);
         }
-        else if (UPDATE_RESERVERED == doiRow.getIntColumn("status")) 
+        else if (UPDATE_RESERVED == doiRow.getIntColumn("status"))
         {
             doiRow.setColumn("status", IS_RESERVED);
         }

dspace-api/src/main/java/org/dspace/identifier/VersionedHandleIdentifierProvider.java

@@ -545,7 +545,7 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
     protected String getCanonical(Item item)
     {
         String canonical = item.getHandle();
-        if( canonical.lastIndexOf(DOT)!=-1)
+        if( canonical.matches(".*/.*\\.\\d+") && canonical.lastIndexOf(DOT)!=-1)
         {
             canonical =  canonical.substring(0, canonical.lastIndexOf(DOT));
         }
@@ -556,7 +556,7 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
     protected String getCanonical(String identifier)
     {
         String canonical = identifier;
-        if( canonical.lastIndexOf(DOT)!=-1)
+        if( canonical.matches(".*/.*\\.\\d+") && canonical.lastIndexOf(DOT)!=-1)
         {
             canonical =  canonical.substring(0, canonical.lastIndexOf(DOT));
         }

dspace-api/src/main/java/org/dspace/identifier/doi/DOIOrganiser.java

@@ -179,19 +179,19 @@ public class DOIOrganiser {
         
         if (line.hasOption('l'))
         {
-            organiser.list("reservation", null, null, DOIIdentifierProvider.TO_BE_RESERVERED);
+            organiser.list("reservation", null, null, DOIIdentifierProvider.TO_BE_RESERVED);
             organiser.list("registration", null, null, DOIIdentifierProvider.TO_BE_REGISTERED);
             organiser.list("update", null, null,
-                    DOIIdentifierProvider.UPDATE_BEFORE_REGISTERATION,
+                    DOIIdentifierProvider.UPDATE_BEFORE_REGISTRATION,
                     DOIIdentifierProvider.UPDATE_REGISTERED,
-                    DOIIdentifierProvider.UPDATE_RESERVERED);
+                    DOIIdentifierProvider.UPDATE_RESERVED);
             organiser.list("deletion", null, null, DOIIdentifierProvider.TO_BE_DELETED);
         }
 
         if (line.hasOption('s'))
         {
             TableRowIterator it = organiser
-                                    .getDOIsByStatus(DOIIdentifierProvider.TO_BE_RESERVERED);
+                                    .getDOIsByStatus(DOIIdentifierProvider.TO_BE_RESERVED);
            
             try { 
                 if (!it.hasNext()) 
@@ -244,8 +244,8 @@ public class DOIOrganiser {
         if (line.hasOption('u'))
         {
             TableRowIterator it = organiser.getDOIsByStatus(
-                    DOIIdentifierProvider.UPDATE_BEFORE_REGISTERATION,
-                    DOIIdentifierProvider.UPDATE_RESERVERED,
+                    DOIIdentifierProvider.UPDATE_BEFORE_REGISTRATION,
+                    DOIIdentifierProvider.UPDATE_RESERVED,
                     DOIIdentifierProvider.UPDATE_REGISTERED);
            
             try { 

dspace-api/src/main/java/org/dspace/search/DSQuery.java

@@ -475,6 +475,9 @@ public class DSQuery
             lastModified = reader.getVersion();         
             searcher = new IndexSearcher(reader);
             
+        } else {
+            reader.close();
+            searchDir.close();
         }
 
         return searcher;

dspace-api/src/main/java/org/dspace/statistics/SolrLogger.java

@@ -7,7 +7,6 @@
  */
 package org.dspace.statistics;
 
-import au.com.bytecode.opencsv.CSVParser;
 import au.com.bytecode.opencsv.CSVReader;
 import au.com.bytecode.opencsv.CSVWriter;
 import com.maxmind.geoip.Location;
@@ -78,8 +77,6 @@ public class SolrLogger
 
     private static final LookupService locationService;
 
-    private static Map<String, String> metadataStorageInfo;
-
     private static final boolean useProxies;
 
     private static List<String> statisticYearCores = new ArrayList<String>();
@@ -331,6 +328,13 @@ public class SolrLogger
                 log.debug(e.getMessage(),e);
             }
 
+		    if(request.getHeader("User-Agent") != null)
+		    {
+		        doc1.addField("userAgent", request.getHeader("User-Agent"));
+		    }
+
+		    doc1.addField("isBot",isSpiderBot);
+
             // Save the location information if valid, save the event without
             // location information if not valid
             if(locationService != null)
@@ -354,12 +358,9 @@ public class SolrLogger
                     doc1.addField("city", location.city);
                     doc1.addField("latitude", location.latitude);
                     doc1.addField("longitude", location.longitude);
-                    doc1.addField("isBot",isSpiderBot);
+                    
+
 
-                    if(request.getHeader("User-Agent") != null)
-                    {
-                        doc1.addField("userAgent", request.getHeader("User-Agent"));
-                    }
                 }
             }
         }
@@ -416,6 +417,12 @@ public class SolrLogger
                 log.debug(e.getMessage(),e);
             }
 
+		    if(userAgent != null)
+		    {
+		        doc1.addField("userAgent", userAgent);
+		    }
+		    doc1.addField("isBot",isSpiderBot);
+
             // Save the location information if valid, save the event without
             // location information if not valid
             if(locationService != null)
@@ -439,12 +446,9 @@ public class SolrLogger
                     doc1.addField("city", location.city);
                     doc1.addField("latitude", location.latitude);
                     doc1.addField("longitude", location.longitude);
-                    doc1.addField("isBot",isSpiderBot);
+                    
+
 
-                    if(userAgent != null)
-                    {
-                        doc1.addField("userAgent", userAgent);
-                    }
                 }
             }
         }
@@ -563,11 +567,6 @@ public class SolrLogger
 
     }
 
-    public static Map<String, String> getMetadataStorageInfo()
-    {
-        return metadataStorageInfo;
-    }
-
     /**
      * Method just used to log the parents.
      * <ul>
@@ -676,18 +675,6 @@ public class SolrLogger
 
             // We have at least one document good
             SolrDocument document = response.getResults().get(0);
-            for (Object storedField : metadataStorageInfo.keySet())
-            {
-                // For each of these fields that are stored we are to create a
-                // list of the values it holds now
-                java.util.Collection collection = document
-                        .getFieldValues((String) storedField);
-                List<String> storedVals = new ArrayList<String>();
-                storedVals.addAll(collection);
-                // Now add it to our hashmap
-                currentValsStored.put((String) storedField, storedVals);
-            }
-
             // System.out.println("HERE");
             // Get the info we need
         }

dspace-api/src/main/java/org/dspace/statistics/StatisticsLoggingConsumer.java

@@ -37,6 +37,7 @@ public class StatisticsLoggingConsumer implements Consumer
 
     private Set<String> toRemoveQueries = null;
 
+    @Override
     public void initialize() throws Exception
     {
 
@@ -45,6 +46,7 @@ public class StatisticsLoggingConsumer implements Consumer
     // TODO: checkout whether moving of collections, communities and bitstreams works
     // TODO: use async threaded consumer as this might require some processing time
     // TODO: we might be able to improve the performance: changing the collection will trigger 4 update commands
+    @Override
     public void consume(Context ctx, Event event) throws Exception
     {
         if (toRemoveQueries == null)
@@ -78,45 +80,9 @@ public class StatisticsLoggingConsumer implements Consumer
                     updateQuery, null, null);
 
             // Get all the metadata
-            Map<String, String> metadataStorageInfo = SolrLogger.getMetadataStorageInfo();
             List<String> storageFieldList = new ArrayList<String>();
             List<List<Object>> storageValuesList = new ArrayList<List<Object>>();
 
-            for (Map.Entry<String, String> entry : metadataStorageInfo.entrySet())
-            {
-                String[] metadataFieldInfo = entry.getValue().split("\\.");
-
-                List<Object> values = new ArrayList<Object>();
-                List<Object> valuesLow = new ArrayList<Object>();
-                for (int i = 0; i < item.getMetadata(metadataFieldInfo[0],
-                        metadataFieldInfo[1], metadataFieldInfo[2], Item.ANY).length; i++)
-                {
-                    values.add(item.getMetadata(metadataFieldInfo[0],
-                            metadataFieldInfo[1], metadataFieldInfo[2],
-                            Item.ANY)[i].value);
-                    
-                    valuesLow.add(item.getMetadata(metadataFieldInfo[0],
-                            metadataFieldInfo[1], metadataFieldInfo[2],
-                            Item.ANY)[i].value.toLowerCase());
-                }
-
-                List<String> indexedVals = indexedValues.get(entry.getKey());
-
-                boolean update = true;
-                if (values.size() == indexedVals.size() && values.containsAll(indexedVals))
-                {
-                    update = false;
-                }
-
-                if (update)
-                {
-                    storageFieldList.add(entry.getKey());
-                    storageFieldList.add(entry.getKey() + "_search");
-                    storageValuesList.add(values);
-                    storageValuesList.add(valuesLow);
-                }
-            }
-
             SolrLogger.update(updateQuery, "replace", storageFieldList,
                     storageValuesList);
 
@@ -202,6 +168,7 @@ public class StatisticsLoggingConsumer implements Consumer
         findComms(comm.getParentCommunity(), parentComms);
     }
 
+    @Override
     public void end(Context ctx) throws Exception
     {
         if (toRemoveQueries != null)
@@ -215,6 +182,7 @@ public class StatisticsLoggingConsumer implements Consumer
         toRemoveQueries = null;
     }
 
+    @Override
     public void finish(Context ctx) throws Exception
     {
     }

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

@@ -33,10 +33,10 @@ import java.text.ParsePosition;
  * @author Stuart Lewis
  */
 public class ClassicDSpaceLogConverter {
-    private Logger log = Logger.getLogger(ClassicDSpaceLogConverter.class);
+    private final Logger log = Logger.getLogger(ClassicDSpaceLogConverter.class);
 
     /** A DSpace context */
-    private Context context;
+    private final Context context;
 
     /** Whether or not to provide verbose output */
     private boolean verbose = false;
@@ -45,19 +45,19 @@ public class ClassicDSpaceLogConverter {
     private boolean newEvents = false;
 
     /** A regular expression for extracting the IP address from a log line */
-    private Pattern ipaddrPattern = Pattern.compile("ip_addr=(\\d*\\.\\d*\\.\\d*\\.\\d*):");
+    private final Pattern ipaddrPattern = Pattern.compile("ip_addr=(\\d*\\.\\d*\\.\\d*\\.\\d*):");
 
     /** Date format (in) from the log line */
-    private SimpleDateFormat dateFormatIn = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
+    private final SimpleDateFormat dateFormatIn = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
 
     /** Date format out (for solr) */
-    private SimpleDateFormat dateFormatOut = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
+    private final SimpleDateFormat dateFormatOut = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
 
     /** Date format (in) from the log line for the UID */
-    private SimpleDateFormat dateFormatInUID = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss,SSS");
+    private final SimpleDateFormat dateFormatInUID = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss,SSS");
 
     /** Date format out (for uid) */
-    private SimpleDateFormat dateFormatOutUID = new SimpleDateFormat("yyyyMMddHHmmssSSS");
+    private final SimpleDateFormat dateFormatOutUID = new SimpleDateFormat("yyyyMMddHHmmssSSS");
 
 
     /**
@@ -349,24 +349,33 @@ public class ClassicDSpaceLogConverter {
         {
             // Convert all the files
             final File sample = new File(line.getOptionValue('i'));
-            File dir = sample.getParentFile();
+            File dir = sample.getAbsoluteFile().getParentFile();
             FilenameFilter filter = new FilenameFilter()
             {
+                @Override
                 public boolean accept(File dir, String name)
                 {
                     return name.startsWith(sample.getName());
                 }
             };
             String[] children = dir.list(filter);
-            for (String in : children)
+            if (null == children)
             {
-                System.err.println(in);
-                String out = line.getOptionValue('o') +
-                             (dir.getAbsolutePath() +
-                              System.getProperty("file.separator") + in).substring(line.getOptionValue('i').length());
-
-                converter.convert(dir.getAbsolutePath() + System.getProperty("file.separator") + in, out);
+                System.err.println(sample + " could not be used to find a directory of log files.");
+                System.exit(1);
             }
+            else if (children.length <= 0)
+                System.err.println(sample + " matched no files.");
+            else
+                for (String in : children)
+                {
+                    System.err.println(in);
+                    String out = line.getOptionValue('o') +
+                                 (dir.getAbsolutePath() +
+                                  System.getProperty("file.separator") + in).substring(line.getOptionValue('i').length());
+
+                    converter.convert(dir.getAbsolutePath() + System.getProperty("file.separator") + in, out);
+                }
         }
         else
         {

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

@@ -205,7 +205,8 @@ public class SpiderDetector {
 
             for (Pattern candidate : agents)
             {
-                if (candidate.matcher(agent).find())
+                // prevent matcher() invocation from a null Pattern object
+                if (null != candidate && candidate.matcher(agent).find())
                 {
                     return true;
                 }
@@ -236,7 +237,8 @@ public class SpiderDetector {
 
             for (Pattern candidate : domains)
             {
-                if (candidate.matcher(hostname).find())
+                // prevent matcher() invocation from a null Pattern object
+                if (null != candidate && candidate.matcher(hostname).find())
                 {
                     return true;
                 }

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

@@ -16,13 +16,10 @@ import org.dspace.core.Constants;
 import org.dspace.core.ConfigurationManager;
 import org.dspace.content.DSpaceObject;
 import org.dspace.content.Bitstream;
-import org.dspace.content.DCValue;
-import org.dspace.content.Item;
 import org.dspace.eperson.EPerson;
 import org.dspace.statistics.SolrLogger;
 
 import java.util.Date;
-import java.util.Map;
 import java.text.SimpleDateFormat;
 
 import com.maxmind.geoip.LookupService;
@@ -196,8 +193,6 @@ public class StatisticsDataGenerator {
 		solr.deleteByQuery("*:*");
 		solr.commit();
 
-		Map<String, String> metadataStorageInfo = SolrLogger.getMetadataStorageInfo();
-
 		String prevIp = null;
 		String dbfile = ConfigurationManager.getProperty("usage-statistics", "dbfile");
 		LookupService cl = new LookupService(dbfile,
@@ -366,24 +361,6 @@ public class StatisticsDataGenerator {
                 doc1.addField("dns", dns.toLowerCase());
             }
 
-			if (dso instanceof Item) {
-				Item item = (Item) dso;
-				// Store the metadata
-                for (Map.Entry<String, String> entry : metadataStorageInfo.entrySet())
-				{
-					String dcField = entry.getValue();
-
-					DCValue[] vals = item.getMetadata(dcField.split("\\.")[0],
-							dcField.split("\\.")[1], dcField.split("\\.")[2],
-							Item.ANY);
-					for (DCValue val1 : vals) {
-						String val = val1.value;
-						doc1.addField(entry.getKey(), val);
-						doc1.addField(entry.getKey() + "_search", val.toLowerCase());
-					}
-				}
-			}
-
 			SolrLogger.storeParents(doc1, dso);
 
 			solr.add(doc1);

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

@@ -29,7 +29,7 @@ import com.maxmind.geoip.LookupService;
 import com.maxmind.geoip.Location;
 
 /**
- * Class to load intermediate statistics files (produced from log files by <code>ClassicDSpaceLogConverter</code>) into Solr
+ * Class to load intermediate statistics files (produced from log files by {@link ClassicDSpaceLogConverter}) into Solr.
  *
  * @see ClassicDSpaceLogConverter
  *
@@ -40,7 +40,7 @@ public class StatisticsImporter
     private static final Logger log = Logger.getLogger(StatisticsImporter.class);
 
     /** Date format (for solr) */
-    private static SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
+    private static final SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
 
     /** Solr server connection */
     private static HttpSolrServer solr;
@@ -48,9 +48,6 @@ public class StatisticsImporter
     /** GEOIP lookup service */
     private static LookupService geoipLookup;
 
-    /** Metadata storage information */
-    private static Map<String, String> metadataStorageInfo;
-
     /** Whether to skip the DNS reverse lookup or not */
     private static boolean skipReverseDNS = false;
 
@@ -67,7 +64,7 @@ public class StatisticsImporter
     private List<Integer> localBitstreams;
 
     /** Whether or not to replace item IDs with local values (for testing) */
-    private boolean useLocal;
+    private final boolean useLocal;
 
     /**
      * Constructor. Optionally loads local data to replace foreign data
@@ -339,24 +336,6 @@ public class StatisticsImporter
                     sid.addField("dns", dns.toLowerCase());
                 }
 
-                if (dso instanceof Item) {
-                    Item item = (Item) dso;
-                    // Store the metadata
-                    for (String storedField : metadataStorageInfo.keySet()) {
-                        String dcField = metadataStorageInfo.get(storedField);
-
-                        DCValue[] vals = item.getMetadata(dcField.split("\\.")[0],
-                                dcField.split("\\.")[1], dcField.split("\\.")[2],
-                                Item.ANY);
-                        for (DCValue val1 : vals) {
-                            String val = val1.value;
-                            sid.addField(String.valueOf(storedField), val);
-                            sid.addField(String.valueOf(storedField + "_search"),
-                                    val.toLowerCase());
-                        }
-                    }
-                }
-
                 SolrLogger.storeParents(sid, dso);
                 solr.add(sid);
                 errors--;
@@ -471,7 +450,6 @@ public class StatisticsImporter
         }
 		solr = new HttpSolrServer(sserver);
 
-		metadataStorageInfo = SolrLogger.getMetadataStorageInfo();
         String dbfile = ConfigurationManager.getProperty("usage-statistics", "dbfile");
         try
         {
@@ -495,6 +473,7 @@ public class StatisticsImporter
             File dir = sample.getParentFile();
             FilenameFilter filter = new FilenameFilter()
             {
+                @Override
                 public boolean accept(File dir, String name)
                 {
                     return name.startsWith(sample.getName());
@@ -522,7 +501,7 @@ public class StatisticsImporter
      */
     static class DNSCache<K,V> extends LinkedHashMap<K,V>
     {
-        private int maxCapacity;
+        private final int maxCapacity;
 
         public DNSCache(int initialCapacity, float loadFactor, int maxCapacity)
         {

dspace-api/src/main/java/org/dspace/workflow/WorkflowManager.java

@@ -23,6 +23,9 @@ import org.apache.log4j.Logger;
 
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.AuthorizeManager;
+import org.dspace.authorize.ResourcePolicy;
+import org.dspace.content.Bitstream;
+import org.dspace.content.Bundle;
 import org.dspace.content.Collection;
 import org.dspace.content.DCDate;
 import org.dspace.content.DCValue;
@@ -30,6 +33,7 @@ import org.dspace.content.InstallItem;
 import org.dspace.content.Item;
 import org.dspace.content.WorkspaceItem;
 import org.dspace.core.ConfigurationManager;
+import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.core.Email;
 import org.dspace.core.I18nUtil;
@@ -38,6 +42,7 @@ import org.dspace.curate.WorkflowCurator;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 import org.dspace.handle.HandleManager;
+import org.dspace.services.ConfigurationService;
 import org.dspace.storage.rdbms.DatabaseManager;
 import org.dspace.storage.rdbms.TableRow;
 import org.dspace.storage.rdbms.TableRowIterator;
@@ -45,28 +50,31 @@ import org.dspace.usage.UsageWorkflowEvent;
 import org.dspace.utils.DSpace;
 
 /**
- * Workflow state machine
+ * Workflow state machine.
  *
- * Notes:
+ * <p>Notes:
  *
- * Determining item status from the database:
+ * <p>Determining item status from the database:
  *
- * When an item has not been submitted yet, it is in the user's personal
+ * <ul>
+ * <li>When an item has not been submitted yet, it is in the user's personal
  * workspace (there is a row in PersonalWorkspace pointing to it.)
  *
- * When an item is submitted and is somewhere in a workflow, it has a row in the
+ * <li>When an item is submitted and is somewhere in a workflow, it has a row in the
  * WorkflowItem table pointing to it. The state of the workflow can be
- * determined by looking at WorkflowItem.getState()
+ * determined by looking at {@link WorkflowItem#getState()}
  *
- * When a submission is complete, the WorkflowItem pointing to the item is
- * destroyed and the archive() method is called, which hooks the item up to the
- * archive.
+ * <li>When a submission is complete, the {@link WorkflowItem} pointing to the
+ * item is destroyed and the
+ * {@link #archive(org.dspace.core.Context, org.dspace.workflow.WorkflowItem)}
+ * method is called, which hooks the item up to the archive.
+ * </ul>
  *
- * Notification: When an item enters a state that requires notification,
+ * <p>Notification: When an item enters a state that requires notification,
  * (WFSTATE_STEP1POOL, WFSTATE_STEP2POOL, WFSTATE_STEP3POOL,) the workflow needs
  * to notify the appropriate groups that they have a pending task to claim.
  *
- * Revealing lists of approvers, editors, and reviewers. A method could be added
+ * <p>Revealing lists of approvers, editors, and reviewers. A method could be added
  * to do this, but it isn't strictly necessary. (say public List
  * getStateEPeople( WorkflowItem wi, int state ) could return people affected by
  * the item's current state.
@@ -110,7 +118,7 @@ public class WorkflowManager
     };
 
     /* support for 'no notification' */
-    private static Map<Integer, Boolean> noEMail = new HashMap<Integer, Boolean>();
+    private static final Map<Integer, Boolean> noEMail = new HashMap<Integer, Boolean>();
 
     /** log4j logger */
     private static Logger log = Logger.getLogger(WorkflowManager.class);
@@ -144,11 +152,13 @@ public class WorkflowManager
      * @param wsi
      *            The WorkspaceItem to convert to a workflow item
      * @return The resulting workflow item
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     * @throws java.io.IOException passed through.
      */
     public static WorkflowItem start(Context c, WorkspaceItem wsi)
             throws SQLException, AuthorizeException, IOException
     {
-        // FIXME Check auth
         Item myitem = wsi.getItem();
         Collection collection = wsi.getCollection();
 
@@ -272,10 +282,15 @@ public class WorkflowManager
     /**
      * claim() claims a workflow task for an EPerson
      *
+     * @param c
+     *            Current user context.
      * @param wi
      *            WorkflowItem to do the claim on
      * @param e
      *            The EPerson doing the claim
+     * @throws java.sql.SQLException passed through.
+     * @throws java.io.IOException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
     public static void claim(Context c, WorkflowItem wi, EPerson e)
             throws SQLException, IOException, AuthorizeException
@@ -286,27 +301,28 @@ public class WorkflowManager
         {
         case WFSTATE_STEP1POOL:
 
-            // authorize DSpaceActions.SUBMIT_REVIEW
+            // FIXME note:  authorizeAction ASSUMES that c.getCurrentUser() == e!
+            AuthorizeManager.authorizeAction(c, wi.getCollection(), Constants.WORKFLOW_STEP_1, true);
             doState(c, wi, WFSTATE_STEP1, e);
 
             break;
 
         case WFSTATE_STEP2POOL:
 
-            // authorize DSpaceActions.SUBMIT_STEP2
+            AuthorizeManager.authorizeAction(c, wi.getCollection(), Constants.WORKFLOW_STEP_2, true);
             doState(c, wi, WFSTATE_STEP2, e);
 
             break;
 
         case WFSTATE_STEP3POOL:
 
-            // authorize DSpaceActions.SUBMIT_STEP3
+            AuthorizeManager.authorizeAction(c, wi.getCollection(), Constants.WORKFLOW_STEP_3, true);
             doState(c, wi, WFSTATE_STEP3, e);
 
             break;
 
-        // if we got here, we weren't pooled... error?
-        // FIXME - log the error?
+        default:
+            throw new IllegalArgumentException("Workflow Step " + taskstate + " is out of range.");
         }
 
         log.info(LogManager.getHeader(c, "claim_task", "workflow_item_id="
@@ -329,6 +345,9 @@ public class WorkflowManager
      *            WorkflowItem do do the approval on
      * @param e
      *            EPerson doing the approval
+     * @throws java.sql.SQLException passed through.
+     * @throws java.io.IOException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
     public static void advance(Context c, WorkflowItem wi, EPerson e)
             throws SQLException, IOException, AuthorizeException
@@ -337,11 +356,12 @@ public class WorkflowManager
     }
 
     /**
-     * advance() sends an item forward in the workflow (reviewers,
-     * approvers, and editors all do an 'approve' to move the item forward) if
-     * the item arrives at the submit state, then remove the WorkflowItem and
-     * call the archive() method to put it in the archive, and email notify the
-     * submitter of a successful submission
+     * advance() sends an item forward in the workflow. Reviewers,
+     * approvers, and editors all do an 'approve' to move the item forward.
+     * If the item arrives at the submit state, then remove the WorkflowItem,
+     * call the {@link #archive(org.dspace.core.Context, org.dspace.workflow.WorkflowItem)}
+     * method to put it in the archive, and email notify the
+     * submitter of a successful submission.
      *
      * @param c
      *            Context
@@ -355,6 +375,10 @@ public class WorkflowManager
      *
      * @param record
      *            boolean indicating whether to record action
+     * @return true if the state was advanced.
+     * @throws java.sql.SQLException passed through.
+     * @throws java.io.IOException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
     public static boolean advance(Context c, WorkflowItem wi, EPerson e,
                                   boolean curate, boolean record)
@@ -385,8 +409,15 @@ public class WorkflowManager
             break;
 
         case WFSTATE_STEP1:
+            // advance(...) will call itself if no workflow step group exists
+            // so we need to check permissions only if a workflow step group is
+            // in place.
+            if (wi.getCollection().getWorkflowGroup(1) != null)
+            {
+                // FIXME note:  authorizeAction ASSUMES that c.getCurrentUser() == e!
+                AuthorizeManager.authorizeAction(c, wi.getCollection(), Constants.WORKFLOW_STEP_1, true);
+            }
 
-            // authorize DSpaceActions.SUBMIT_REVIEW
             // Record provenance
             if (record)
             {
@@ -397,8 +428,14 @@ public class WorkflowManager
             break;
 
         case WFSTATE_STEP2:
+            // advance(...) will call itself if no workflow step group exists
+            // so we need to check permissions only if a workflow step group is
+            // in place.
+            if (wi.getCollection().getWorkflowGroup(2) != null)
+            {
+                AuthorizeManager.authorizeAction(c, wi.getCollection(), Constants.WORKFLOW_STEP_2, true);
+            }
 
-            // authorize DSpaceActions.SUBMIT_STEP2
             // Record provenance
             if (record)
             {
@@ -409,8 +446,14 @@ public class WorkflowManager
             break;
 
         case WFSTATE_STEP3:
+            // advance(...) will call itself if no workflow step group exists
+            // so we need to check permissions only if a workflow step group is
+            // in place.
+            if (wi.getCollection().getWorkflowGroup(3) != null)
+            {
+                AuthorizeManager.authorizeAction(c, wi.getCollection(), Constants.WORKFLOW_STEP_3, true);
+            }
 
-            // authorize DSpaceActions.SUBMIT_STEP3
             // We don't record approval for editors, since they can't reject,
             // and thus didn't actually make a decision
             archived = doState(c, wi, WFSTATE_ARCHIVE, e);
@@ -429,7 +472,7 @@ public class WorkflowManager
     }
 
     /**
-     * unclaim() returns an owned task/item to the pool
+     * returns an owned task/item to the pool
      *
      * @param c
      *            Context
@@ -437,6 +480,9 @@ public class WorkflowManager
      *            WorkflowItem to operate on
      * @param e
      *            EPerson doing the operation
+     * @throws java.sql.SQLException passed through.
+     * @throws java.io.IOException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
     public static void unclaim(Context c, WorkflowItem wi, EPerson e)
             throws SQLException, IOException, AuthorizeException
@@ -447,27 +493,31 @@ public class WorkflowManager
         {
         case WFSTATE_STEP1:
 
-            // authorize DSpaceActions.STEP1
             doState(c, wi, WFSTATE_STEP1POOL, e);
 
             break;
 
         case WFSTATE_STEP2:
 
-            // authorize DSpaceActions.APPROVE
             doState(c, wi, WFSTATE_STEP2POOL, e);
 
             break;
 
         case WFSTATE_STEP3:
 
-            // authorize DSpaceActions.STEP3
             doState(c, wi, WFSTATE_STEP3POOL, e);
 
             break;
 
-        // error handling? shouldn't get here
-        // FIXME - what to do with error - log it?
+        default:
+            throw new IllegalStateException("WorkflowItem reached an unknown state.");
+        }
+
+        try {
+            c.turnOffAuthorisationSystem();
+            wi.update();
+        } finally {
+            c.restoreAuthSystemState();
         }
 
         log.info(LogManager.getHeader(c, "unclaim_workflow",
@@ -511,165 +561,298 @@ public class WorkflowManager
         returnToWorkspace(c, wi);
     }
 
-    // returns true if archived
+    /**
+     * Move a workflow item to a new state.  The item may be put in a pool,
+     * removed from a pool and assigned to a user, or archived.
+     *
+     * @param c current DSpace context.
+     * @param wi workflow item whose state should transition.
+     * @param newstate move {@link wi} to this state.
+     * @param newowner assign {@link wi} to this user.
+     * @return true if archived.
+     * @throws SQLException passed through.
+     * @throws IOException passed through.
+     * @throws AuthorizeException passed through.
+     */
     private static boolean doState(Context c, WorkflowItem wi, int newstate,
             EPerson newowner) throws SQLException, IOException,
             AuthorizeException
     {
         Collection mycollection = wi.getCollection();
-        Group mygroup = null;
-        boolean archived = false;
 
         //Gather our old data for launching the workflow event
         int oldState = wi.getState();
 
         wi.setState(newstate);
 
+        boolean archived;
         switch (newstate)
         {
         case WFSTATE_STEP1POOL:
-
-            // any reviewers?
-            // if so, add them to the tasklist
-            wi.setOwner(null);
-
-            // get reviewers (group 1 )
-            mygroup = mycollection.getWorkflowGroup(1);
-
-            if ((mygroup != null) && !(mygroup.isEmpty()))
-            {
-                // get a list of all epeople in group (or any subgroups)
-                EPerson[] epa = Group.allMembers(c, mygroup);
-
-                // there were reviewers, change the state
-                //  and add them to the list
-                createTasks(c, wi, epa);
-                wi.update();
-
-                // email notification
-                notifyGroupOfTask(c, wi, mygroup, epa);
-            }
-            else
-            {
-                // no reviewers, skip ahead
-                wi.setState(WFSTATE_STEP1);
-                archived = advance(c, wi, null, true, false);
-            }
-
+            archived = pool(c, wi, 1);
             break;
 
         case WFSTATE_STEP1:
-
-            // remove reviewers from tasklist
-            // assign owner
-            deleteTasks(c, wi);
-            wi.setOwner(newowner);
-
+            assignToReviewer(c, wi, 1, newowner);
+            archived = false;
             break;
 
         case WFSTATE_STEP2POOL:
-
-            // clear owner
-            // any approvers?
-            // if so, add them to tasklist
-            // if not, skip to next state
-            wi.setOwner(null);
-
-            // get approvers (group 2)
-            mygroup = mycollection.getWorkflowGroup(2);
-
-            if ((mygroup != null) && !(mygroup.isEmpty()))
-            {
-                //get a list of all epeople in group (or any subgroups)
-                EPerson[] epa = Group.allMembers(c, mygroup);
-
-                // there were approvers, change the state
-                //  timestamp, and add them to the list
-                createTasks(c, wi, epa);
-
-                // email notification
-                notifyGroupOfTask(c, wi, mygroup, epa);
-            }
-            else
-            {
-                // no reviewers, skip ahead
-                wi.setState(WFSTATE_STEP2);
-                archived = advance(c, wi, null, true, false);
-            }
-
+            archived = pool(c, wi, 2);
             break;
 
         case WFSTATE_STEP2:
-
-            // remove admins from tasklist
-            // assign owner
-            deleteTasks(c, wi);
-            wi.setOwner(newowner);
-
+            assignToReviewer(c, wi, 2, newowner);
+            archived = false;
             break;
 
         case WFSTATE_STEP3POOL:
-
-            // any editors?
-            // if so, add them to tasklist
-            wi.setOwner(null);
-            mygroup = mycollection.getWorkflowGroup(3);
-
-            if ((mygroup != null) && !(mygroup.isEmpty()))
-            {
-                // get a list of all epeople in group (or any subgroups)
-                EPerson[] epa = Group.allMembers(c, mygroup);
-
-                // there were editors, change the state
-                //  timestamp, and add them to the list
-                createTasks(c, wi, epa);
-
-                // email notification
-                notifyGroupOfTask(c, wi, mygroup, epa);
-            }
-            else
-            {
-                // no editors, skip ahead
-                wi.setState(WFSTATE_STEP3);
-                archived = advance(c, wi, null, true, false);
-            }
-
+            archived = pool(c, wi, 3);
             break;
 
         case WFSTATE_STEP3:
-
-            // remove editors from tasklist
-            // assign owner
-            deleteTasks(c, wi);
-            wi.setOwner(newowner);
-
+            assignToReviewer(c, wi, 3, newowner);
+            archived = false;
             break;
 
         case WFSTATE_ARCHIVE:
-
             // put in archive in one transaction
             // remove workflow tasks
             deleteTasks(c, wi);
-
             mycollection = wi.getCollection();
-
-            Item myitem = archive(c, wi);
+            Item myItem = archive(c, wi);
 
             // now email notification
-            notifyOfArchive(c, myitem, mycollection);
-            archived = true;
+            notifyOfArchive(c, myItem, mycollection);
 
-            break;
+            // remove any workflow policies left
+            try {
+                c.turnOffAuthorisationSystem();
+                revokeReviewerPolicies(c, myItem);
+            } finally {
+                c.restoreAuthSystemState();
+            }
+
+            logWorkflowEvent(c, wi.getItem(), wi, c.getCurrentUser(), newstate,
+                    newowner, mycollection, oldState, null);
+            return true;
+        default:
+            throw new IllegalArgumentException("WorkflowManager cannot handle workflowItemState " + newstate);
         }
 
-        logWorkflowEvent(c, wi.getItem(), wi, c.getCurrentUser(), newstate, newowner, mycollection, oldState, mygroup);
-
-        if (!archived)
-        {
+        try {
+            c.turnOffAuthorisationSystem();
             wi.update();
+        } finally {
+            c.restoreAuthSystemState();
+        }
+        return archived;
+    }
+
+    /**
+     * Assign this workflow item to a reviewer.
+     *
+     * @param context current DSpace context.
+     * @param workflowItem the item to be assigned.
+     * @param step review step.
+     * @param newowner the reviewer to be assigned.
+     * @throws AuthorizeException passed through.
+     * @throws SQLException passed through.
+     * @throws IllegalArgumentException if {@code step} is unknown.
+     */
+    protected static void assignToReviewer(Context context, WorkflowItem workflowItem,
+            int step, EPerson newowner)
+            throws AuthorizeException, SQLException
+    {
+        // shortcut to the collection
+        Collection collection = workflowItem.getCollection();
+        // from the step we can recognize the new state and the corresponding policy action.
+        int newState;
+        int correspondingAction;
+        switch (step)
+        {
+        case 1:
+            newState = WFSTATE_STEP1;
+            correspondingAction = Constants.WORKFLOW_STEP_1;
+            break;
+        case 2:
+            newState = WFSTATE_STEP2;
+            correspondingAction = Constants.WORKFLOW_STEP_2;
+            break;
+        case 3:
+            newState = WFSTATE_STEP3;
+            correspondingAction = Constants.WORKFLOW_STEP_3;
+            break;
+        default:
+            throw new IllegalArgumentException("Unknown workflow step " + step);
         }
 
-        return archived;
+        // Gather the old state for logging.
+        int oldState = workflowItem.getState();
+
+        // If there is a workflow state group and it contains any members,
+        // then we have to check the permissions first.
+        Group stateGroup = collection.getWorkflowGroup(step);
+        if ((stateGroup != null) && !(stateGroup.isEmpty()))
+        {
+            // FIXME note:  authorizeAction ASSUMES that c.getCurrentUser() == newowner!
+            AuthorizeManager.authorizeAction(context, collection, correspondingAction, true);
+        }
+
+        // Give the owner the appropriate permissions.
+        try {
+            context.turnOffAuthorisationSystem();
+            // maybe unnecessary, but revoke any perviously granted permissions.
+            revokeReviewerPolicies(context, workflowItem.getItem());
+            // Finally grant the new permissions.
+            grantReviewerPolicies(context, workflowItem, newowner);
+        } finally {
+            context.restoreAuthSystemState();
+        }
+
+        // Remove task from tasklist as someone is working on it now.
+        deleteTasks(context, workflowItem);
+        // Assign new owner.
+        workflowItem.setState(newState);
+        workflowItem.setOwner(newowner);
+
+        logWorkflowEvent(context, workflowItem.getItem(), workflowItem,
+                context.getCurrentUser(), newState, newowner, collection, oldState, null);
+    }
+
+    /**
+     * Helper method that manages state, policies, owner, notifies, task list items
+     * and so on whenever a WorkflowItem should be added to a workflow step pool.
+     * Don't use this method directly.  Either use
+     * {@link #unclaim(Context, WorkflowItem, EPerson)} if the item is claimed,
+     * {@link #start(Context, WorkspaceItem)} to start the workflow, or
+     * {@link #advance(Context, WorkflowItem, EPerson)} to move an item to the next state.
+     *
+     * @param context DSpace context object.
+     * @param workflowItem the item to be pooled.
+     * @param step the step (1-3) of the pool the item should be put into.
+     * @return true if the item was archived because no reviewers were assigned
+     *         to any of the following workflow steps, false otherwise.
+     * @throws SQLException passed through.
+     * @throws AuthorizeException passed through.
+     * @throws IOException passed through.
+     * @throws IllegalArgumentException if {@code step} has another value than
+     *         either 1, 2, or 3.
+     */
+    protected static boolean pool(Context context, WorkflowItem workflowItem, int step)
+            throws SQLException, AuthorizeException, IOException
+    {
+        // shortcut to the collection
+        Collection collection = workflowItem.getCollection();
+        
+        // From the step we can recognize the new state and the corresponding state.
+        // The new state is the pool of the step.
+        // The corresponding state is the state an item gets when it is claimed.
+        // That is important to recognize if we have to send notifications
+        // and if we have to skip a pool.
+        int newState;
+        int correspondingState;
+        switch (step)
+        {
+        case 1:
+            newState = WFSTATE_STEP1POOL;
+            correspondingState = WFSTATE_STEP1;
+            break;
+        case 2:
+            newState = WFSTATE_STEP2POOL;
+            correspondingState = WFSTATE_STEP2;
+            break;
+        case 3:
+            newState = WFSTATE_STEP3POOL;
+            correspondingState = WFSTATE_STEP3;
+            break;
+        default:
+            throw new IllegalArgumentException("Unknown workflow step " + step);
+        }
+        
+        // Gather our old owner and state, as we need those as well to determine
+        // whether we have to send notifications.
+        int oldState = workflowItem.getState();
+        EPerson oldOwner = workflowItem.getOwner();
+        // Clear owner.
+        workflowItem.setOwner(null);
+        // Don't revoke the reviewer policies yet.  They may be needed to advance the item.
+        
+        // Any approvers?  If so, add them to the tasklist; if not, skip to next state.
+        Group workflowStepGroup = collection.getWorkflowGroup(step);
+        if ((workflowStepGroup != null) && !(workflowStepGroup.isEmpty()))
+        {
+            // Set new item state.
+            workflowItem.setState(newState);
+            
+            // Revoke previously granted reviewer policies and grant read permissions.
+            try {
+                context.turnOffAuthorisationSystem();
+                // Revoke previously granted policies.
+                revokeReviewerPolicies(context, workflowItem.getItem());
+                
+                // JSPUI offers a preview to every task before a reviewer claims it.
+                // So we need to grant permissions in advance, so that all
+                // possible reviewers can read the item and all bitstreams in
+                // the bundle "ORIGINAL".
+                AuthorizeManager.addPolicy(context, workflowItem.getItem(),
+                        Constants.READ, workflowStepGroup,
+                        ResourcePolicy.TYPE_WORKFLOW);
+                Bundle originalBundle;
+                try {
+                    originalBundle = workflowItem.getItem().getBundles("ORIGINAL")[0];
+                } catch (IndexOutOfBoundsException ex) {
+                    originalBundle = null;
+                }
+                if (originalBundle != null)
+                {
+                    AuthorizeManager.addPolicy(context, originalBundle, Constants.READ,
+                            workflowStepGroup, ResourcePolicy.TYPE_WORKFLOW);
+                    for (Bitstream bitstream : originalBundle.getBitstreams())
+                    {
+                        AuthorizeManager.addPolicy(context, bitstream, Constants.READ,
+                                workflowStepGroup, ResourcePolicy.TYPE_WORKFLOW);
+                    }
+                }
+            } finally {
+                context.restoreAuthSystemState();
+            }
+            
+            // Get a list of all epeople in group (or any subgroups)
+            EPerson[] epa = Group.allMembers(context, workflowStepGroup);
+            
+            // There were reviewers.  Change the state and then add them to the list.
+            createTasks(context, workflowItem, epa);
+            ConfigurationService configurationService = new DSpace().getConfigurationService();
+            if (configurationService.getPropertyAsType("workflow.notify.returned.tasks", true)
+                    || oldState != correspondingState
+                    || oldOwner == null)
+            {
+                // Email notification
+                notifyGroupOfTask(context, workflowItem, workflowStepGroup, epa);
+            }
+            logWorkflowEvent(context, workflowItem.getItem(), workflowItem,
+                    context.getCurrentUser(), newState, null, collection,
+                    oldState, workflowStepGroup);
+            return false;
+        }
+        else
+        {
+            // No reviewers -- skip ahead.
+            workflowItem.setState(correspondingState);
+            boolean archived = advance(context, workflowItem, null, true, false);
+            if (archived)
+            {
+                // Remove any workflow policies that may be left over.
+                try {
+                    context.turnOffAuthorisationSystem();
+                    revokeReviewerPolicies(context, workflowItem.getItem());
+                } finally {
+                    context.restoreAuthSystemState();
+                }
+            }
+            return archived;
+        }
     }
 
     private static void logWorkflowEvent(Context c, Item item, WorkflowItem workflowItem, EPerson actor, int newstate, EPerson newOwner, Collection mycollection, int oldState, Group newOwnerGroup) {
@@ -793,6 +976,8 @@ public class WorkflowManager
         Item myitem = wfi.getItem();
         Collection mycollection = wfi.getCollection();
 
+        // Regarding auth:  this method s private.
+        // Authorization should be checked in all public methods calling this one.
         // FIXME: How should this interact with the workflow system?
         // FIXME: Remove license
         // FIXME: Provenance statement?
@@ -895,7 +1080,7 @@ public class WorkflowManager
         }
     }
 
-    // deletes all tasks associated with a workflowitem
+    /** Deletes all tasks associated with a workflowitem. */
     static void deleteTasks(Context c, WorkflowItem wi) throws SQLException
     {
         String myrequest = "DELETE FROM TaskListItem WHERE workflow_id= ? ";
@@ -1095,6 +1280,8 @@ public class WorkflowManager
      * get the name of the eperson who started this workflow
      *
      * @param wi  the workflow item
+     * @return "user name (email@address)"
+     * @throws java.sql.SQLException passed through.
      */
     public static String getSubmitterName(WorkflowItem wi) throws SQLException
     {
@@ -1105,6 +1292,10 @@ public class WorkflowManager
 
     private static String getEPersonName(EPerson e) throws SQLException
     {
+        if (e == null)
+        {
+            return "Unknown";
+        }
         String submitter = e.getFullName();
 
         submitter = submitter + " (" + e.getEmail() + ")";
@@ -1166,4 +1357,116 @@ public class WorkflowManager
         myitem.addDC("description", "provenance", "en", provmessage);
         myitem.update();
     }
+
+    /**
+     * This method grants the appropriate permissions to reviewers so that they
+     * can read and edit metadata and read files and edit files if allowed by
+     * configuration.
+     * <p>
+     * In most cases this method must be called within a try-finally-block that
+     * temporarily disables the authentication system. That is not done by this
+     * method as it should be done carefully and only in contexts in which
+     * granting the permissions is authorized by some previous checks.
+     *
+     * @param context
+     * @param wfi While all policies are granted on item, bundle or bitstream
+     *            level, this method takes a {@link WorkflowItem} for convenience and
+     *            uses wfi.getItem() to get the actual item.
+     * @param reviewer EPerson to grant the rights to.
+     * @throws SQLException
+     * @throws AuthorizeException
+     */
+    protected static void grantReviewerPolicies(Context context, WorkflowItem wfi, EPerson reviewer)
+            throws SQLException, AuthorizeException
+    {
+        // get item and bundle "ORIGINAL"
+        Item item = wfi.getItem();
+        Bundle originalBundle;
+        try {
+            originalBundle = item.getBundles("ORIGINAL")[0];
+        } catch (IndexOutOfBoundsException ex) {
+            originalBundle = null;
+        }
+
+        // grant item level policies
+        for (int action : new int[] {Constants.READ, Constants.WRITE, Constants.ADD, Constants.REMOVE, Constants.DELETE})
+        {
+            AuthorizeManager.addPolicy(context, item, action, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+        }
+
+        // set bitstream and bundle policies
+        if (originalBundle != null)
+        {
+            AuthorizeManager.addPolicy(context, originalBundle, Constants.READ, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+
+            // shall reviewers be able to edit files?
+            ConfigurationService configurationService = new DSpace().getConfigurationService();
+            boolean editFiles = Boolean.parseBoolean(configurationService.getProperty("workflow.reviewer.file-edit"));
+            // if a reviewer should be able to edit bitstreams, we need add
+            // permissions regarding the bundle "ORIGINAL" and its bitstreams
+            if (editFiles)
+            {
+                AuthorizeManager.addPolicy(context, originalBundle, Constants.ADD, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(context, originalBundle, Constants.REMOVE, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+                // Whenever a new bitstream is added, it inherit the policies of the bundle.
+                // So we need to add all policies newly created bitstreams should get.
+                AuthorizeManager.addPolicy(context, originalBundle, Constants.WRITE, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+                AuthorizeManager.addPolicy(context, originalBundle, Constants.DELETE, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+            }
+            for (Bitstream bitstream : originalBundle.getBitstreams())
+            {
+                AuthorizeManager.addPolicy(context, bitstream, Constants.READ, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+
+                // add further rights if reviewer should be able to edit bitstreams
+                if (editFiles)
+                {
+                    AuthorizeManager.addPolicy(context, bitstream, Constants.WRITE, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+                    AuthorizeManager.addPolicy(context, bitstream, Constants.DELETE, reviewer, ResourcePolicy.TYPE_WORKFLOW);
+                }
+            }
+        }
+    }
+
+    /**
+     * This method revokes any permission granted by the basic workflow system
+     * on the item specified as attribute. At time of writing this method these
+     * permissions will all be granted by
+     * {@link #grantReviewerPolicies(org.dspace.core.Context, org.dspace.workflowbasic.BasicWorkflowItem, org.dspace.eperson.EPerson)}.
+     * <p>
+     * In most cases this method must be called within a try-finally-block that
+     * temporarily disables the authentication system. That is not done by this
+     * method as it should be done carefully and only in contexts in which
+     * revoking the permissions is authorized by some previous checks.
+     *
+     * @param context
+     * @param item
+     * @throws SQLException passed through.
+     * @throws AuthorizeException passed through.
+     */
+    protected static void revokeReviewerPolicies(Context context, Item item)
+            throws SQLException, AuthorizeException
+    {
+        // get bundle "ORIGINAL"
+        Bundle originalBundle;
+        try {
+            originalBundle = item.getBundles("ORIGINAL")[0];
+        } catch (IndexOutOfBoundsException ex) {
+            originalBundle = null;
+        }
+
+        // remove bitstream and bundle level policies
+        if (originalBundle != null)
+        {
+            // We added policies for Bitstreams of the bundle "original" only
+            for (Bitstream bitstream : originalBundle.getBitstreams())
+            {
+                AuthorizeManager.removeAllPoliciesByDSOAndType(context, bitstream, ResourcePolicy.TYPE_WORKFLOW);
+            }
+
+            AuthorizeManager.removeAllPoliciesByDSOAndType(context, originalBundle, ResourcePolicy.TYPE_WORKFLOW);
+        }
+
+        // remove item level policies
+        AuthorizeManager.removeAllPoliciesByDSOAndType(context, item, ResourcePolicy.TYPE_WORKFLOW);
+     }
 }

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

@@ -92,6 +92,15 @@ public class XmlWorkflowManager {
      * startWithoutNotify() starts the workflow normally, but disables
      * notifications (useful for large imports,) for the first workflow step -
      * subsequent notifications happen normally
+     * @param c
+     * @param wsi
+     * @return a new workflow item wrapping the item removed from the workspace.
+     * @throws java.sql.SQLException passed through
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     * @throws java.io.IOException passed through.
+     * @throws org.dspace.xmlworkflow.WorkflowException passed through.
+     * @throws org.dspace.xmlworkflow.WorkflowConfigurationException passed through.
+     * @throws javax.mail.MessagingException passed through.
      */
     public static XmlWorkflowItem startWithoutNotify(Context c, WorkspaceItem wsi)
             throws SQLException, AuthorizeException, IOException, WorkflowException, WorkflowConfigurationException, MessagingException {
@@ -352,9 +361,14 @@ public class XmlWorkflowManager {
      * with the relevant collection, added to the search index, and any other
      * tasks such as assigning dates are performed.
      *
+     * @param c
+     * @param wfi
      * @return the fully archived item.
+     * @throws java.sql.SQLException passed through.
+     * @throws java.io.IOException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
      */
-    public static Item archive(Context c, XmlWorkflowItem wfi)
+    protected static Item archive(Context c, XmlWorkflowItem wfi)
             throws SQLException, IOException, AuthorizeException {
         // FIXME: Check auth
         Item item = wfi.getItem();

dspace-api/src/test/java/org/dspace/app/bulkedit/DSpaceCSVTest.java

@@ -120,7 +120,7 @@ public class DSpaceCSVTest extends AbstractUnitTest
                 fail("An exception should have been thrown due to bad CSV");
             } catch (Exception e)
             {
-                    assertThat("testDSpaceCSV Bad heading CSV", e.getMessage(), equalTo("Unknown metadata element in row 4: dc.contributor.foobar"));
+                assertThat("testDSpaceCSV Bad heading CSV", e.getMessage(), equalTo("Unknown metadata element in column 4: dc.contributor.foobar"));
             }
             lines = dcsv.getCSVLinesAsStringArray();
             assertThat("testDSpaceCSV Good CSV", lines.length, equalTo(7));
@@ -147,7 +147,7 @@ public class DSpaceCSVTest extends AbstractUnitTest
                 fail("An exception should have been thrown due to bad CSV");
             } catch (Exception e)
             {
-                assertThat("testDSpaceCSV Bad heading CSV", e.getMessage(), equalTo("Unknown metadata schema in row 3: dcdc.title"));
+                assertThat("testDSpaceCSV Bad heading CSV", e.getMessage(), equalTo("Unknown metadata schema in column 3: dcdc.title"));
             }
 
             // Delete the test file

dspace-api/src/test/java/org/dspace/content/CollectionHelper.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.content;
+
+import java.io.IOException;
+import java.sql.SQLException;
+import org.dspace.authorize.AuthorizeException;
+
+/**
+ * Give tests access to package-private operations on Collection.
+ * @author mwood
+ */
+public class CollectionHelper
+{
+    /**
+     * Delete the Collection by calling {@link org.dspace.content.Collection#delete()}.
+     *
+     * @param collection to be deleted.
+     * @throws SQLException passed through.
+     * @throws AuthorizeException passed through.
+     * @throws IOException passed through.
+     */
+    static public void delete(Collection collection)
+            throws SQLException, AuthorizeException, IOException
+    {
+        collection.delete();
+    }
+}

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

@@ -478,6 +478,29 @@ public class CollectionTest extends AbstractDSpaceObjectTest
         assertThat("testSetWorkflowGroup 1",c.getWorkflowGroup(step), equalTo(g));
     }
 
+    /**
+     * Test of setWorkflowGroup method, of class Collection.
+     * The setWorkflowGroup adjusts the policies for the basic Workflow. This test
+     * shall assure that no exception (e.g. ConcurrentModificationException) is
+     * thrown during these adjustments.
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     */
+    @Test
+    public void testChangeWorkflowGroup()
+            throws SQLException, AuthorizeException
+    {
+        context.turnOffAuthorisationSystem(); //must be an Admin to create a Group
+        int step = 1;
+        Group g1 = Group.create(context);
+        Group g2 = Group.create(context);
+        context.restoreAuthSystemState();
+        c.setWorkflowGroup(step, g1);
+        c.setWorkflowGroup(step, g2);
+        assertThat("testSetWorkflowGroup 0", c.getWorkflowGroup(step), notNullValue());
+        assertThat("testSetWorkflowGroup 1", c.getWorkflowGroup(step), equalTo(g2));
+    }
+
     /**
      * Test of getWorkflowGroup method, of class Collection.
      */
@@ -1733,23 +1756,19 @@ public class CollectionTest extends AbstractDSpaceObjectTest
 
      /**
      * Test of delete method, of class Collection.
+     * @throws java.lang.Exception
      */
     @Test(expected=AuthorizeException.class)
     public void testDeleteNoAuth2() throws Exception
     {
-        new NonStrictExpectations()
-        {
-            AuthorizeUtil authUtil;
-            AuthorizeManager authManager;
-            {
-                AuthorizeUtil.authorizeManageTemplateItem((Context) any, (Collection) any);
-                 result = null;
-                AuthorizeManager.authorizeAction((Context) any, (Collection) any,
-                        Constants.WRITE, anyBoolean); result = new AuthorizeException();
-            }
-        };
+        new NonStrictExpectations(AuthorizeUtil.class, AuthorizeManager.class)
+        {{
+            AuthorizeUtil.authorizeManageTemplateItem((Context) any, (Collection) any);
+             result = new AuthorizeException();
+            AuthorizeManager.authorizeAction((Context) any, (Collection) any,
+                    Constants.WRITE, anyBoolean); result = null;
+        }};
 
-        int id = c.getID();
         c.delete();
         fail("Exception expected");
     }
@@ -1880,4 +1899,4 @@ public class CollectionTest extends AbstractDSpaceObjectTest
         }
     }
 
-}
+}

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

@@ -9,7 +9,7 @@ package org.dspace.content;
 
 import java.io.File;
 import java.io.FileInputStream;
-import java.lang.reflect.Constructor;
+import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.sql.SQLException;
 
@@ -24,13 +24,14 @@ 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.AuthorizeManager;
 import org.dspace.authorize.ResourcePolicy;
-import org.dspace.content.authority.MetadataAuthorityManager;
 import org.dspace.core.Constants;
 
 /**
@@ -96,6 +97,7 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     @Override
     public void destroy()
     {
+        context.turnOffAuthorisationSystem();
         it = null;
         super.destroy();
     }
@@ -103,11 +105,12 @@ public class ItemTest  extends AbstractDSpaceObjectTest
 
     /**
      * Test of find method, of class Item.
+     * @throws java.lang.Exception passed through.
      */
     @Test
     public void testItemFind() throws Exception
     {
-        int id = 1;
+        int id = this.it.getID();
         Item found =  Item.find(context, id);
         assertThat("testItemFind 0", found, notNullValue());
         assertThat("testItemFind 1", found.getID(), equalTo(id));
@@ -1613,6 +1616,35 @@ public class ItemTest  extends AbstractDSpaceObjectTest
         assertTrue("testCanEditBooleanAuth3 0", it.canEdit());
     }
 
+	// testCanEditBooleanAuth5 was backported.
+    // testCanEditBooleanAuth4 is part of an later version of DSpace.
+
+	/**
+	 * Test of canEdit method, of class Item.
+     */
+    @Test
+    public void testCanEditBooleanAuth5() throws Exception
+    {
+        // Test Inheritance of permissions
+        new NonStrictExpectations()
+        {
+            AuthorizeManager authManager;
+            {
+                // Disallow Item WRITE perms
+                AuthorizeManager.authorizeAction((Context) any, (Item) any,
+                        Constants.WRITE); result = new AuthorizeException();
+                // Allow Collection WRITE perms
+                AuthorizeManager.authorizeAction((Context) any, (Collection) any,
+                        Constants.WRITE, anyBoolean); result = null;
+            }
+        };
+
+        Collection c = Collection.create(context);
+        c.createTemplateItem();
+        c.update();
+        assertTrue("testCanEditBooleanNoAuth5 0", c.getTemplateItem().canEdit());
+    }
+
     /**
      * Test of canEditBoolean method, of class Collection.
      */
@@ -1642,6 +1674,87 @@ public class ItemTest  extends AbstractDSpaceObjectTest
         assertFalse("testCanEditBooleanNoAuth 0", it.canEdit());
     }
 
+    /**
+     * Test of canEdit method, of class Item.
+     */
+    @Test
+    public void testCanEditBooleanNoAuth2() throws Exception
+    {
+        // Test Inheritance of permissions
+        new NonStrictExpectations(AuthorizeManager.class)
+        {{
+            // Disallow Item WRITE perms
+            AuthorizeManager.authorizeActionBoolean((Context) any, (Item) any,
+                    Constants.WRITE); result = false;                
+            // Disallow parent Community WRITE and ADD perms
+            AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
+                    Constants.WRITE,anyBoolean); result = false;
+            // Allow parent Collection ADD perms
+            AuthorizeManager.authorizeAction((Context) any, (Collection) any,
+                    Constants.ADD); result = null;
+            AuthorizeManager.authorizeActionBoolean((Context) any, (Collection) any,
+                    Constants.ADD); result = true;
+        }};
+
+        Collection c = Collection.create(context);
+        WorkspaceItem wi = WorkspaceItem.create(context, c, true);
+        assertFalse("testCanEditBooleanNoAuth2 0", wi.getItem().canEdit());
+    }
+
+    /**
+     * Test of isInProgressSubmission method, of class Item.
+     * @throws AuthorizeException 
+     * @throws SQLException 
+     * @throws IOException 
+     * 
+     */
+    @Test
+    public void testIsInProgressSubmission() throws SQLException, AuthorizeException, IOException
+    {
+    	context.turnOffAuthorisationSystem();
+    	Collection c = Collection.create(context);
+        WorkspaceItem wi = WorkspaceItem.create(context, c, true);
+    	context.restoreAuthSystemState();
+        assertTrue("testIsInProgressSubmission 0", wi.getItem().isInProgressSubmission());
+    }
+    
+    /**
+     * Test of isInProgressSubmission method, of class Item.
+     * @throws AuthorizeException 
+     * @throws SQLException 
+     * @throws IOException 
+     * 
+     */
+    @Test
+    public void testIsInProgressSubmissionFalse() throws SQLException, AuthorizeException, IOException
+    {
+    	context.turnOffAuthorisationSystem();
+    	Collection c = Collection.create(context);
+        WorkspaceItem wi = WorkspaceItem.create(context, c, true);
+        Item item = InstallItem.installItem(context, wi);
+    	context.restoreAuthSystemState();
+        assertFalse("testIsInProgressSubmissionFalse 0", item.isInProgressSubmission());
+    }
+
+    /**
+     * Test of isInProgressSubmission method, of class Item.
+     * @throws AuthorizeException 
+     * @throws SQLException 
+     * @throws IOException 
+     * 
+     */
+    @Test
+    public void testIsInProgressSubmissionFalse2() throws SQLException, AuthorizeException, IOException
+    {
+    	context.turnOffAuthorisationSystem();
+    	Collection c = Collection.create(context);
+        c.createTemplateItem();
+        c.update();
+        Item item = c.getTemplateItem();
+    	context.restoreAuthSystemState();
+        assertFalse("testIsInProgressSubmissionFalse2 0", item.isInProgressSubmission());
+    }
+
     /**
      * Test of getName method, of class Item.
      */
@@ -1742,4 +1855,4 @@ public class ItemTest  extends AbstractDSpaceObjectTest
         assertThat("testFindByAuthorityValue 5",result.next(),equalTo(it));
     }
 
-}
+}

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

@@ -112,11 +112,15 @@ public class MetadataSchemaTest extends AbstractUnitTest
     @Test
     public void testSetName() 
     {
+        String oldname = ms.getName();
         String name = "new name";
         ms.setName(name);
         assertThat("testSetName 0",ms.getName(),notNullValue());
         assertThat("testSetName 1",ms.getName(),not(equalTo("")));
         assertThat("testSetName 2",ms.getName(),equalTo(name));
+
+        //we restore the old name to avoid issues in other tests
+        ms.setName(oldname);
     }
 
     /**
@@ -392,4 +396,4 @@ public class MetadataSchemaTest extends AbstractUnitTest
         assertThat("testFind_Context_String 4",found, nullValue());
     }
 
-}
+}

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

@@ -274,13 +274,37 @@ public class WorkspaceItemTest extends AbstractUnitTest
      * Test of update method, of class WorkspaceItem.
      */
     @Test
-    public void testUpdate() throws Exception
+    public void testUpdateAuth() throws Exception
     {
-        //TODO: how can we verify it works?
-        wi.update();
-        System.out.println("update");
+		// no need to mockup the authorization as we are the same user that have
+		// created the wi
+        boolean pBefore = wi.isPublishedBefore();
+        wi.setPublishedBefore(!pBefore);
+    	wi.update();
+    	context.removeCached(wi, wi.getID());
+        wi = WorkspaceItem.find(context, wi.getID());
+    	assertTrue("testUpdate", pBefore != wi.isPublishedBefore());
     }
 
+    /**
+     * Test of update method, of class WorkspaceItem with no WRITE auth.
+     */
+    @Test(expected=AuthorizeException.class)
+    public void testUpdateNoAuth() throws Exception
+    {
+    	new NonStrictExpectations(AuthorizeManager.class)
+        {{
+             // Remove Item WRITE perms
+        	AuthorizeManager.authorizeActionBoolean((Context) any, (Item) any,
+                    Constants.WRITE); result = false; 
+        	AuthorizeManager.authorizeAction((Context) any, (Item) any,
+                     Constants.WRITE); result = new AuthorizeException();
+        }};
+        boolean pBefore = wi.isPublishedBefore();
+        wi.setPublishedBefore(!pBefore);
+    	wi.update();
+    	fail("Exception expected");
+    }
     /**
      * Test of deleteAll method, of class WorkspaceItem.
      */

dspace-api/src/test/java/org/dspace/identifier/DOIIdentifierProviderTest.java

@@ -551,7 +551,7 @@ public class DOIIdentifierProviderTest
         assumeNotNull(doiRow);
         
         assertTrue("Reservation of DOI did not set the corret DOI status.",
-                DOIIdentifierProvider.TO_BE_RESERVERED.intValue() == doiRow.getIntColumn("status"));
+                DOIIdentifierProvider.TO_BE_RESERVED.intValue() == doiRow.getIntColumn("status"));
     }
     
     @Test

dspace-api/src/test/java/org/dspace/identifier/EZIDIdentifierProviderTest.java

@@ -10,17 +10,14 @@ package org.dspace.identifier;
 
 import java.io.IOException;
 import java.sql.SQLException;
-import java.util.UUID;
 import org.dspace.AbstractUnitTest;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.*;
-import org.dspace.core.Context;
 import org.dspace.kernel.ServiceManager;
 import org.dspace.services.ConfigurationService;
 import org.dspace.workflow.WorkflowItem;
 import org.dspace.workflow.WorkflowManager;
 import org.junit.*;
-import static org.junit.Assert.*;
 
 /**
  *
@@ -66,14 +63,14 @@ public class EZIDIdentifierProviderTest
      * @throws AuthorizeException
      * @throws IOException
      */
-    private Item newItem(Context ctx)
+    private Item newItem()
             throws SQLException, AuthorizeException, IOException
     {
-        ctx.turnOffAuthorisationSystem();
-        ctx.setCurrentUser(eperson);
+        context.turnOffAuthorisationSystem();
+        context.setCurrentUser(eperson);
 
         // Create an Item to play with
-        WorkspaceItem wsItem = WorkspaceItem.create(ctx, collection, false);
+        WorkspaceItem wsItem = WorkspaceItem.create(context, collection, false);
 
         // Get it from the workspace and set some metadata
         Item item = wsItem.getItem();
@@ -85,14 +82,14 @@ public class EZIDIdentifierProviderTest
         item.update();
 
         // I think we have to do this?
-        WorkflowItem wfItem = WorkflowManager.startWithoutNotify(ctx, wsItem);
-        WorkflowManager.advance(ctx, wfItem, ctx.getCurrentUser());
+        WorkflowItem wfItem = WorkflowManager.startWithoutNotify(context, wsItem);
+        WorkflowManager.advance(context, wfItem, context.getCurrentUser());
         wfItem.update();
         wfItem.deleteWrapper();
 
         // Commit work, clean up
-        ctx.commit();
-        ctx.restoreAuthSystemState();
+        context.commit();
+        context.restoreAuthSystemState();
 
         return item;
     }

dspace-api/src/test/java/org/dspace/workflowbasic/BasicWorkflowAuthorizationIntegrationTest.java

@@ -0,0 +1,421 @@
+/**
+ * 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.workflowbasic;
+
+import org.apache.log4j.Logger;
+import org.dspace.AbstractIntegrationTest;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.content.Bitstream;
+import org.dspace.content.Bundle;
+import org.dspace.content.Collection;
+import org.dspace.content.Community;
+import org.dspace.content.Item;
+import org.dspace.content.WorkspaceItem;
+import org.dspace.core.Constants;
+import org.dspace.eperson.EPerson;
+import org.dspace.eperson.Group;
+import org.hamcrest.CoreMatchers;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.sql.SQLException;
+import org.dspace.authorize.AuthorizeManager;
+import org.dspace.content.CollectionHelper;
+import org.dspace.eperson.EPersonDeletionException;
+import org.dspace.services.ConfigurationService;
+import org.dspace.utils.DSpace;
+import org.dspace.workflow.WorkflowItem;
+import org.dspace.workflow.WorkflowManager;
+
+/**
+ * This is an integration test to ensure that the basic workflow system
+ * -including methods of the collection service dealing with it- works properly
+ * together with the authorization service.
+ * @author Pascal-Nicolas Becker
+ * @author Terry Brady
+ */
+public class BasicWorkflowAuthorizationIntegrationTest
+extends AbstractIntegrationTest
+{
+    /** log4j category */
+    private static final Logger log = Logger.getLogger(BasicWorkflowAuthorizationIntegrationTest.class);
+
+    protected ConfigurationService configurationService
+            = new DSpace().getConfigurationService();
+
+    protected Community owningCommunity;
+    protected Collection collection;
+    protected Group group;
+    protected EPerson member;
+
+    public BasicWorkflowAuthorizationIntegrationTest()
+    {
+        owningCommunity = null;
+        collection = null;
+        group = null;
+        member = null;
+    }
+
+    /**
+     * This method will be run before every test as per @Before. It will
+     * initialize resources required for the tests.
+     *
+     * Other methods can be annotated with @Before here or in subclasses
+     * but no execution order is guaranteed
+     */
+    @Before
+    @Override
+    public void init()
+    {
+        super.init();
+
+        try
+        {
+            //we have to create a new community in the database
+            context.turnOffAuthorisationSystem();
+
+            this.owningCommunity = Community.create(null, context);
+            this.collection = owningCommunity.createCollection();
+            this.member = EPerson.create(context);
+            this.member.setEmail("john.smith@example.com");
+            this.member.setFirstName("John");
+            this.member.setLastName("Smith");
+            this.group = Group.create(context);
+            group.addMember(member);
+            group.update();
+        }
+        catch (AuthorizeException ex)
+        {
+            log.error("Authorization Error in init", ex);
+            Assert.fail("Authorization Error in init: " + ex.getMessage());
+        }
+        catch (SQLException ex)
+        {
+            log.error("SQL Error in init", ex);
+            Assert.fail("SQL Error in init: " + ex.getMessage());
+        }
+        finally
+        {
+            // restore the authorization system as tests expect it to be in place
+            context.restoreAuthSystemState();
+        }
+    }
+
+    /**
+     * This method will be run after every test as per @After. It will
+     * clean resources initialized by the @Before methods.
+     *
+     * Other methods can be annotated with @After here or in subclasses
+     * but no execution order is guaranteed
+     */
+    @After
+    @Override
+    public void destroy() {
+        try {
+            context.turnOffAuthorisationSystem();
+
+            // reload collection, community, group and eperson
+            if (collection != null)
+            {
+                try {
+                    CollectionHelper.delete(collection);
+                } catch (IOException e) {
+                    log.error("deleting collection", e);
+                } catch (SQLException e)
+                {
+                    log.error("deleting collection", e);
+                } catch (AuthorizeException e)
+                {
+                    log.error("deleting collection", e);
+                }
+                collection = null;
+            }
+
+            if (owningCommunity != null)
+            {
+                try {
+                    owningCommunity.delete();
+                } catch (IOException e) {
+                    log.error("deleting community", e);
+                } catch (SQLException e)
+                {
+                    log.error("deleting community", e);
+                } catch (AuthorizeException e)
+                {
+                    log.error("deleting community", e);
+                }
+                owningCommunity = null;
+            }
+
+            if (member != null)
+            {
+                if (group != null)
+                {
+                    try {
+                        group.removeMember(member);
+                    } catch (Exception e) {
+                        log.error("detaching group relationship", e);
+                    }
+                    try {
+                        group.delete();
+                    } catch (SQLException e) {
+                        log.error("deleting group");
+                    }
+                    group = null;
+                }
+                try {
+                    member.delete();
+                } catch (SQLException e) {
+                    log.error("deleting user", e);
+                } catch (AuthorizeException e)
+                {
+                    log.error("deleting user", e);
+                } catch (EPersonDeletionException e)
+                {
+                    log.error("deleting user", e);
+                }
+            }
+        }
+        finally
+        {
+            // restore the authorization system
+            context.restoreAuthSystemState();
+        }
+        super.destroy();
+    }
+
+    private void setWorkflowGroup(Collection collection, int step, Group group)
+            throws SQLException, AuthorizeException
+    {
+        collection.setWorkflowGroup(step, group);
+    }
+
+    /**
+     * Test if setWorkflowGroup method sets the appropriate policies for the
+     * new workflow group.
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     */
+    @Test
+    public void testsetWorkflowGroupSetsPermission() throws SQLException, AuthorizeException
+    {
+        int step = 1;
+        try {
+            context.turnOffAuthorisationSystem();
+            setWorkflowGroup(collection, step, group);
+            collection.update();
+        } finally {
+            context.restoreAuthSystemState();
+        }
+        context.setCurrentUser(member);
+        Assert.assertThat("Workflow step " + step + " Group is not our test Group",
+                collection.getWorkflowGroup(step), CoreMatchers.equalTo(group));
+        Assert.assertTrue("Test EPerson is not member of test Group", group.isMember(member));
+        Assert.assertTrue("Test EPerson is not authorized for step " + step,
+                AuthorizeManager.authorizeActionBoolean(context, collection, Constants.WORKFLOW_STEP_1, true));
+    }
+
+    /**
+     * Test if setWorkflowGroup method revokes policies when a workflow group
+     * is removed.
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     */
+    @Test
+    public void testsetWorkflowGroupRevokesPermission()
+            throws SQLException, AuthorizeException
+    {
+        int step = 1;
+        try {
+            context.turnOffAuthorisationSystem();
+            setWorkflowGroup(collection, step, group);
+            collection.update();
+        } finally {
+            context.restoreAuthSystemState();
+        }
+        context.setCurrentUser(member);
+        Assert.assertThat("Test workflow group is not the group for step " + step,
+                collection.getWorkflowGroup(step),
+                CoreMatchers.equalTo(group));
+        Assert.assertTrue("Member of test workflow group not authorized for step " + step,
+                AuthorizeManager.authorizeActionBoolean(context, collection,
+                        Constants.WORKFLOW_STEP_1, true));
+        try {
+            context.turnOffAuthorisationSystem();
+            setWorkflowGroup(collection, step, null);
+            collection.update();
+        } finally {
+            context.restoreAuthSystemState();
+        }
+        Assert.assertThat("Workflow step " + step + "group is not null",
+                collection.getWorkflowGroup(step),
+                CoreMatchers.nullValue());
+        Assert.assertFalse("Member of test workflow group is still authorized for step " + step,
+                AuthorizeManager.authorizeActionBoolean(context, collection,
+                        Constants.WORKFLOW_STEP_1, true));
+    }
+
+    /**
+     * Test that a member of a workflow step group can claim a task and get the
+     * appropriate policies.
+     * @throws java.sql.SQLException
+     * @throws org.dspace.authorize.AuthorizeException
+     * @throws java.io.IOException
+     */
+    @Test
+    public void testReviewerPermissions()
+            throws SQLException, AuthorizeException, IOException
+    {
+        WorkflowItem wfi = null;
+        try {
+            // prepare a task to claim
+            // turn of the authorization system to be able to create the task
+            context.turnOffAuthorisationSystem();
+            setWorkflowGroup(collection, 1, group);
+            collection.update();
+            WorkspaceItem wsi = WorkspaceItem.create(context, collection, false);
+            Item item = wsi.getItem();
+            Bundle bundle = item.createBundle("ORIGINAL");
+            File f = new File(testProps.get("test.bitstream").toString());
+            Bitstream bs = bundle.createBitstream(new FileInputStream(f));
+            bundle.update();
+            item.update();
+            wsi.update();
+
+            wfi = WorkflowManager.startWithoutNotify(context, wsi);
+            wfi.update();
+        } finally {
+            // restore the authorization system to perform our tests
+            context.restoreAuthSystemState();
+        }
+
+        context.setCurrentUser(member);
+
+        wfi = WorkflowItem.find(context, wfi.getID());
+        WorkflowManager.claim(context, wfi, context.getCurrentUser());
+        Item item = wfi.getItem();
+
+        int i = 0;
+        // check item policies
+        for (int action : new int[] {Constants.READ, Constants.WRITE, Constants.ADD, Constants.REMOVE, Constants.DELETE})
+        {
+            Assert.assertTrue("testReviewerPermissions 1-" + i++,
+                    AuthorizeManager.authorizeActionBoolean(context, item, action, false));
+        }
+
+        // ensure we can read the original bundle and its bitstream
+        Bundle bundle = item.getBundles("ORIGINAL")[0];
+        Bitstream bitstream = bundle.getBitstreams()[0];
+        Assert.assertTrue("testReviewerPermissions 2-1",
+                    AuthorizeManager.authorizeActionBoolean(context, bundle, Constants.READ, false));
+        Assert.assertTrue("testReviewerPermissions 2-2" + i++,
+                    AuthorizeManager.authorizeActionBoolean(context, bitstream, Constants.READ, false));
+    }
+
+    /**
+     * Test that a eperson not a member of a workflow step group can't claim a task.
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     * @throws java.io.IOException passed through.
+     */
+    @Test(expected=AuthorizeException.class)
+    public void testNonWorkflowGroupMemberCannotClaimTask()
+            throws SQLException, AuthorizeException, IOException
+    {
+        WorkflowItem wfi = null;
+        EPerson someone = null;
+        try {
+            // prepare a task to claim
+            // turn of the authorization system to be able to create the task
+            context.turnOffAuthorisationSystem();
+            someone = EPerson.create(context);
+            someone.setEmail("jane.doe@example.com");
+            someone.setFirstName("Jane");
+            someone.setLastName("Doe");
+            setWorkflowGroup(collection, 1, group);
+            collection.update();
+            WorkspaceItem wsi = WorkspaceItem.create(context, collection, false);
+            Item item = wsi.getItem();
+            Bundle bundle = item.createBundle("ORIGINAL");
+            File f = new File(testProps.get("test.bitstream").toString());
+            Bitstream bs = bundle.createBitstream(new FileInputStream(f));
+            bundle.update();
+            item.update();
+            wsi.update();
+
+            wfi = WorkflowManager.startWithoutNotify(context, wsi);
+            wfi.update();
+        } finally {
+            // restore the authorization system to perform our tests
+            context.restoreAuthSystemState();
+        }
+
+        context.setCurrentUser(someone);
+
+        wfi = WorkflowItem.find(context, wfi.getID());
+        WorkflowManager.claim(context, wfi, context.getCurrentUser());
+        Assert.fail("Someone, not part of a workflow step group was able to claim a "
+                + "task without an AUthorizeException.");
+    }
+
+    /**
+     * Test that the submitter of an item who is not member of the appropriate
+     * workflow step group cannot claim the task of his/her own submission.
+     * Submitters have special permissions on Workflow and Workspace items, so we
+     * need to test that they are still not able to claim tasks for there own
+     * items.
+     * @throws java.sql.SQLException passed through.
+     * @throws org.dspace.authorize.AuthorizeException passed through.
+     * @throws java.io.IOException passed through.
+     */
+    @Test(expected=AuthorizeException.class)
+    public void testNonWorkflowGroupSubmitterCannotClaimTask()
+            throws SQLException, AuthorizeException, IOException
+    {
+        WorkflowItem wfi = null;
+        EPerson submitter = null;
+        try {
+            // prepare a task to claim
+            // turn of the authorization system to be able to create the task
+            context.turnOffAuthorisationSystem();
+            submitter = EPerson.create(context);
+            submitter.setEmail("richard.roe@example.com");
+            submitter.setFirstName("Richard");
+            submitter.setLastName("Roe");
+            setWorkflowGroup(collection, 1, group);
+            collection.update();
+            WorkspaceItem wsi = WorkspaceItem.create(context, collection, false);
+            Item item = wsi.getItem();
+            item.setSubmitter(submitter);
+            Bundle bundle = item.createBundle("ORIGINAL");
+            File f = new File(testProps.get("test.bitstream").toString());
+            Bitstream bs = bundle.createBitstream(new FileInputStream(f));
+            bundle.update();
+            item.update();
+            wsi.update();
+
+            wfi = WorkflowManager.startWithoutNotify(context, wsi);
+            wfi.update();
+        } finally {
+            // restore the authorization system to perform our tests
+            context.restoreAuthSystemState();
+        }
+
+        context.setCurrentUser(submitter);
+        wfi = WorkflowItem.find(context, wfi.getID());
+        WorkflowManager.claim(context, wfi, context.getCurrentUser());
+        Assert.fail("A submitter was able to claim a task without being a member of the "
+                + "appropriate workflow step group. Expected: AuthorizeException.");
+    }
+
+}

dspace-jspui/pom.xml

@@ -13,7 +13,7 @@
    <parent>
       <groupId>org.dspace</groupId>
       <artifactId>dspace-parent</artifactId>
-      <version>4.0</version>
+      <version>4.9</version>
       <relativePath>..</relativePath>
    </parent>
 

dspace-jspui/src/main/java/org/dspace/app/webui/discovery/DiscoverySearchRequestProcessor.java

@@ -418,12 +418,23 @@ public class DiscoverySearchRequestProcessor implements SearchRequestProcessor
             }
             catch (SQLException e)
             {
-                throw new SearchProcessorException(e.getMessage(), e);            }
 
-            if ("submit_export_metadata".equals(UIUtil.getSubmitButton(request,
-                    "submit")))
-            {
-                exportMetadata(context, response, resultsItems);
+            }
+
+            if ("submit_export_metadata".equals(UIUtil.getSubmitButton(request,"submit"))) {
+                try {
+                    if (AuthorizeManager.isAdmin(context)) {
+                        exportMetadata(context, response, resultsItems);
+                    }
+                    else {
+                        JSPManager.showJSP(request, response, "/error/authorize.jsp");
+                    }
+                }
+                catch (SQLException e)
+                {
+                    throw new SearchProcessorException(e.getMessage(), e);
+                }
+
             }
         }
         catch (SearchServiceException e)

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

@@ -17,6 +17,7 @@ import javax.servlet.jsp.tagext.TagSupport;
 
 import org.apache.commons.lang.ArrayUtils;
 import org.dspace.eperson.EPerson;
+import org.dspace.core.Utils;
 
 /**
  * <p>
@@ -122,7 +123,7 @@ public class SelectEPersonTag extends TagSupport
                 for (int i = 0; i < epeople.length; i++)
                 {
                     out.print("<option value=\"" + epeople[i].getID() + "\">");
-                    out.print(epeople[i].getFullName() + " ("
+                    out.print(Utils.addEntities(epeople[i].getFullName()) + " ("
                             + epeople[i].getEmail() + ")");
                     out.println("</option>");
                 }

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

@@ -17,6 +17,7 @@ import javax.servlet.jsp.tagext.TagSupport;
 
 import org.apache.commons.lang.ArrayUtils;
 import org.dspace.eperson.Group;
+import org.dspace.core.Utils;
 
 /**
  * <P>Tag for producing an e-person select widget in a form.  Somewhat
@@ -119,7 +120,7 @@ public class SelectGroupTag extends TagSupport
 				for (int i = 0; i < groups.length; i++)
 				{
 					out.print("<option value=\"" + groups[i].getID() + "\">");
-					out.print(groups[i].getName() + " (" + groups[i].getID() + ")");
+					out.print(Utils.addEntities(groups[i].getName()) + " (" + groups[i].getID() + ")");
 					out.println("</option>");
 				}
 			}

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

@@ -14,6 +14,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.dspace.app.webui.util.UIUtil;
 import org.dspace.authorize.AuthorizeException;
@@ -30,6 +31,7 @@ import org.dspace.content.Community;
 import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
 import org.dspace.core.LogManager;
+import org.dspace.core.Utils;
 
 /**
  * Servlet for browsing through indices, as they are defined in
@@ -86,6 +88,25 @@ public abstract class AbstractBrowserServlet extends DSpaceServlet
             String month = request.getParameter("month");
             String year = request.getParameter("year");
             String startsWith = request.getParameter("starts_with");
+            //validate input to avoid cross-site scripting
+            try {
+            	if (StringUtils.isNotBlank(month) && !"-1".equals(month)) {
+            		Integer.valueOf(month);
+            	}
+            	if (StringUtils.isNotBlank(year) && !"-1".equals(year)) {
+            		Integer.valueOf(year);
+            	}
+            	if(StringUtils.isNotBlank(startsWith)) {
+            		startsWith = Utils.addEntities(startsWith);
+            	}
+            }
+            catch(Exception ex) {
+                log.warn("We were unable to parse the browse request: maybe a cross-site scripting attach?");
+                return null;
+            }
+            
+            
+            
             String valueFocus = request.getParameter("vfocus");
             String valueFocusLang = request.getParameter("vfocus_lang");
             String authority = request.getParameter("authority");
@@ -108,7 +129,7 @@ public abstract class AbstractBrowserServlet extends DSpaceServlet
 
             // process the input, performing some inline validation
             BrowseIndex bi = null;
-            if (type != null && !"".equals(type))
+            if (StringUtils.isNotEmpty(type))
             {
                 bi = BrowseIndex.getBrowseIndex(type);
             }

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

@@ -65,7 +65,7 @@ public class BrowserServlet extends AbstractBrowserServlet
         // all browse requests currently come to GET.
         BrowserScope scope = getBrowserScopeForRequest(context, request, response);
 
-        if (scope.getBrowseIndex() == null)
+        if (scope == null || scope.getBrowseIndex() == null)
         {
             throw new ServletException("There is no browse index for the request");
         }

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

@@ -11,6 +11,7 @@ import java.io.IOException;
 import java.sql.SQLException;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.locks.*;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -33,46 +34,71 @@ import org.dspace.core.LogManager;
  */
 public class CommunityListServlet extends DSpaceServlet
 {
+    
+    // This will map community IDs to arrays of collections
+    private Map<Integer, Collection[]> colMap;
+
+    // This will map communityIDs to arrays of sub-communities
+    private Map<Integer, Community[]> commMap;
+    private static final Object staticLock = new Object();
+    
     /** log4j category */
     private static Logger log = Logger.getLogger(CommunityListServlet.class);
 
     protected void doDSGet(Context context, HttpServletRequest request,
             HttpServletResponse response) throws ServletException, IOException,
             SQLException, AuthorizeException
-    {
-        log.info(LogManager.getHeader(context, "view_community_list", ""));
+    { 
+           synchronized (staticLock) 
+           {
+            colMap = new HashMap<Integer, Collection[]>();
+            commMap = new HashMap<Integer, Community[]>();
 
-        // This will map community IDs to arrays of collections
-        Map<Integer, Collection[]> colMap = new HashMap<Integer, Collection[]>();
+            log.info(LogManager.getHeader(context, "view_community_list", ""));
 
-        // This will map communityIDs to arrays of sub-communities
-        Map<Integer, Community[]> commMap = new HashMap<Integer, Community[]>();
+            Community[] communities = Community.findAllTop(context);
 
-        Community[] communities = Community.findAllTop(context);
+            for (int com = 0; com < communities.length; com++) 
+            {
+                build(communities[com]);
+            }
 
-        for (int com = 0; com < communities.length; com++)
+            // can they admin communities?
+            if (AuthorizeManager.isAdmin(context)) 
+            {
+                // set a variable to create an edit button
+                request.setAttribute("admin_button", Boolean.TRUE);
+            }
+
+            request.setAttribute("communities", communities);
+            request.setAttribute("collections.map", colMap);
+            request.setAttribute("subcommunities.map", commMap);
+            JSPManager.showJSP(request, response, "/community-list.jsp");
+           }
+    }
+    /*
+     * Get all subcommunities and collections from a community
+     */
+    private void build(Community c) throws SQLException {
+
+        Integer comID = Integer.valueOf(c.getID());
+
+        // Find collections in community
+        Collection[] colls = c.getCollections();
+        colMap.put(comID, colls);
+
+        // Find subcommunties in community
+        Community[] comms = c.getSubcommunities();
+        
+        // Get all subcommunities for each communities if they have some
+        if (comms.length > 0) 
         {
-            Integer comID = Integer.valueOf(communities[com].getID());
-
-            // Find collections in community
-            Collection[] colls = communities[com].getCollections();
-            colMap.put(comID, colls);
-
-            // Find subcommunties in community
-            Community[] comms = communities[com].getSubcommunities();
             commMap.put(comID, comms);
+            
+            for (int sub = 0; sub < comms.length; sub++) {
+                
+                build(comms[sub]);
+            }
         }
-
-        // can they admin communities?
-        if (AuthorizeManager.isAdmin(context))
-        {
-            // set a variable to create an edit button
-            request.setAttribute("admin_button", Boolean.TRUE);
-        }
-
-        request.setAttribute("communities", communities);
-        request.setAttribute("collections.map", colMap);
-        request.setAttribute("subcommunities.map", commMap);
-        JSPManager.showJSP(request, response, "/community-list.jsp");
     }
 }

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

@@ -19,6 +19,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.dspace.app.util.GoogleMetadata;
 import org.dspace.app.webui.util.Authenticate;
@@ -129,8 +130,7 @@ public class HandleServlet extends DSpaceServlet
 
         if (dso == null)
         {
-            log.info(LogManager
-                    .getHeader(context, "invalid_id", "path=" + path));
+            log.info(LogManager.getHeader(context, "invalid_id", "path=" + path));
             JSPManager.showInvalidIDError(request, response, StringEscapeUtils.escapeHtml(path), -1);
 
             return;
@@ -152,8 +152,28 @@ public class HandleServlet extends DSpaceServlet
             // and firing a usage event for the DSO we're reporting for
             return;
 
+        } else if ("/display-statistics.jsp".equals(extraPathInfo))
+        {
+            request.getRequestDispatcher(extraPathInfo).forward(request, response);
+            // If we don't return here, we keep processing and end up
+            // throwing a NPE when checking community authorization
+            // and firing a usage event for the DSO we're reporting for
+            return;
+        } else if ("/browse".equals((extraPathInfo)) || StringUtils.startsWith(extraPathInfo, "/browse?")) {
+            request.getRequestDispatcher(extraPathInfo).forward(request, response);
+            // If we don't return here, we keep processing and end up
+            // throwing a NPE when checking community authorization
+            // and firing a usage event for the DSO we're reporting for
+            return;
+        } else if ("/simple-search".equals(extraPathInfo) || StringUtils.startsWith(extraPathInfo, "simple-search?")) {
+            request.getRequestDispatcher(extraPathInfo).forward(request, response);
+            // If we don't return here, we keep processing and end up
+            // throwing a NPE when checking community authorization
+            // and firing a usage event for the DSO we're reporting for
+            return;
         }
 
+
         // OK, we have a valid Handle. What is it?
         if (dso.getType() == Constants.ITEM)
         {
@@ -195,9 +215,9 @@ public class HandleServlet extends DSpaceServlet
             }
             else
             {
-                // Forward to another servlet
-                request.getRequestDispatcher(extraPathInfo).forward(request,
-                        response);
+                log.debug("Found Item with extraPathInfo => Error.");
+                JSPManager.showInvalidIDError(request, response, StringEscapeUtils.escapeHtml(path), -1);
+                return;
             }
 
         }
@@ -231,9 +251,9 @@ public class HandleServlet extends DSpaceServlet
             }
             else
             {
-                // Forward to another servlet
-                request.getRequestDispatcher(extraPathInfo).forward(request,
-                        response);
+                log.debug("Found Collection with extraPathInfo => Error.");
+                JSPManager.showInvalidIDError(request, response, StringEscapeUtils.escapeHtml(path), -1);
+                return;
             }
         }
         else if (dso.getType() == Constants.COMMUNITY)
@@ -255,9 +275,9 @@ public class HandleServlet extends DSpaceServlet
             }
             else
             {
-                // Forward to another servlet
-                request.getRequestDispatcher(extraPathInfo).forward(request,
-                        response);
+                log.debug("Found Community with extraPathInfo => Error.");
+                JSPManager.showInvalidIDError(request, response, StringEscapeUtils.escapeHtml(path), -1);
+                return;
             }
         }
         else

dspace-jspui/src/main/webapp/collection-home.jsp

@@ -38,6 +38,7 @@
 <%@ page import="org.dspace.content.*"%>
 <%@ page import="org.dspace.core.ConfigurationManager"%>
 <%@ page import="org.dspace.core.Context" %>
+<%@ page import="org.dspace.core.Utils" %>
 <%@ page import="org.dspace.eperson.Group"     %>
 <%@ page import="javax.servlet.jsp.jstl.fmt.LocaleSupport" %>
 <%@ page import="java.net.URLEncoder" %>
@@ -374,7 +375,7 @@
 			{
 				if (dcv.length > 0)
 				{
-					displayTitle = dcv[0].value;
+					displayTitle = Utils.addEntities(dcv[0].value);
 				}
 			}
 			%><p class="recentItem"><a href="<%= request.getContextPath() %>/handle/<%= items[i].getHandle() %>"><%= displayTitle %></a></p><%

dspace-jspui/src/main/webapp/community-home.jsp

@@ -32,6 +32,7 @@
 <%@ page import="org.dspace.browse.ItemCounter" %>
 <%@ page import="org.dspace.content.*" %>
 <%@ page import="org.dspace.core.ConfigurationManager" %>
+<%@ page import="org.dspace.core.Utils" %>
 <%@ page import="javax.servlet.jsp.jstl.fmt.LocaleSupport" %>
 
 
@@ -158,7 +159,7 @@
 			{
 				if (dcv.length > 0)
 				{
-					displayTitle = dcv[0].value;
+					displayTitle = Utils.addEntities(dcv[0].value);
 				}
 			}
 			%>
@@ -401,4 +402,4 @@
 		</div>
   </dspace:sidebar>
     <% } %>
-</dspace:layout>
+</dspace:layout>

dspace-jspui/src/main/webapp/controlledvocabulary/search.jsp

@@ -14,6 +14,7 @@
 	The add-on may be turn off in dspace.cfg
 --%>  
 
+<%@page import="org.dspace.core.Utils"%>
 <%@ page contentType="text/html;charset=UTF-8" %>
 <%@ page  import="javax.servlet.jsp.jstl.fmt.LocaleSupport" %>
 <%@ taglib uri="/WEB-INF/dspace-tags.tld" prefix="dspace" %>
@@ -52,7 +53,7 @@
 	<form name="filterVocabulary" method="post" action="<%= request.getContextPath() %>/subject-search">
 	  <input style="border-width:1px;border-style:solid;" 
 	  		 name="filter" type="text" id="filter" 
-	  		 size="15" value="<%= filter %>" 
+	  		 size="15" value="<%= Utils.addEntities(filter) %>" 
 	  		 title="<%= LocaleSupport.getLocalizedMessage(pageContext, "jsp.controlledvocabulary.search.trimmessage") %>"/>
 	  <input type="submit" name="submit" value="<%= LocaleSupport.getLocalizedMessage(pageContext, "jsp.controlledvocabulary.search.trimbutton") %>"/>
 	  <input type="hidden" name="action" value="filter"/>

dspace-jspui/src/main/webapp/dspace-admin/group-group-select.jsp

@@ -29,6 +29,7 @@
 <%@ taglib uri="http://java.sun.com/jsp/jstl/fmt" prefix="fmt" %>
 
 <%@ page import="org.dspace.eperson.Group"   %>
+<%@ page import="org.dspace.core.Utils" %>
 
 <%
     Group group = (Group) request.getAttribute("group");
@@ -56,7 +57,7 @@
                     <select class="form-control" size="15" name="groups_id" multiple="multiple">
                         <%  for (int i = 0; i < groups.length; i++) { %>
                             <option value="<%= groups[i].getID()%>">
-                                <%= groups[i].getName()%>
+                                <%= Utils.addEntities(groups[i].getName())%>
                             </option>
                         <%  } %>
                 </select>

dspace-jspui/src/main/webapp/dspace-admin/supervise-confirm-remove.jsp

@@ -81,7 +81,7 @@
 <div class="row">
 <label class="col-md-2"><fmt:message key="jsp.dspace-admin.supervise-confirm-remove.supervisorgroupheader"/>:</label>
 <span>
-<%= group.getName() %>
+<%= Utils.addEntities(group.getName()) %>
 </span>
 </div>
 <br/>

dspace-jspui/src/main/webapp/dspace-admin/supervise-link.jsp

@@ -62,7 +62,7 @@
     for (int i = 0; i < groups.length; i++)
     {
 %>
-                <option value="<%= groups[i].getID() %>"><%= groups[i].getName() %></option>
+                <option value="<%= groups[i].getID() %>"><%= Utils.addEntities(groups[i].getName()) %></option>
 <%
     }
 %>

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

@@ -15,6 +15,7 @@
   -    recent.submissions - RecetSubmissions
   --%>
 
+<%@page import="org.dspace.core.Utils"%>
 <%@page import="org.dspace.content.Bitstream"%>
 <%@ page contentType="text/html;charset=UTF-8" %>
 
@@ -133,13 +134,13 @@ if (submissions != null && submissions.count() > 0)
 		        String displayTitle = "Untitled";
 		        if (dcv != null & dcv.length > 0)
 		        {
-		            displayTitle = dcv[0].value;
+		            displayTitle = Utils.addEntities(dcv[0].value);
 		        }
 		        dcv = item.getMetadata("dc", "description", "abstract", Item.ANY);
 		        String displayAbstract = "";
 		        if (dcv != null & dcv.length > 0)
 		        {
-		            displayAbstract = dcv[0].value;
+		            displayAbstract = Utils.addEntities(dcv[0].value);
 		        }
 		%>
 		    <div style="padding-bottom: 50px; min-height: 200px;" class="item <%= first?"active":""%>">

dspace-jspui/src/main/webapp/search/advanced.jsp

@@ -106,7 +106,7 @@
 						{
 							String key = "jsp.search.advanced.type." + index;
 					%>
-							<option value="<%= index %>" <%= field1.equals(index) ? "selected=\"selected\"" : "" %>><fmt:message key="<%= key %>"/></option>
+							<option value="<%= StringEscapeUtils.escapeHtml(index) %>" <%= field1.equals(index) ? "selected=\"selected\"" : "" %>><fmt:message key="<%= key %>"/></option>
 					<%
 						}
 					%>
@@ -136,7 +136,7 @@
 						{
 							String key = "jsp.search.advanced.type." + index;
 					%>
-							<option value="<%= index %>" <%= field2.equals(index) ? "selected=\"selected\"" : "" %>><fmt:message key="<%= key %>"/></option>
+							<option value="<%= StringEscapeUtils.escapeHtml(index) %>" <%= field2.equals(index) ? "selected=\"selected\"" : "" %>><fmt:message key="<%= key %>"/></option>
 					<%
 						}
 					%>
@@ -162,7 +162,7 @@
 						{
 							String key = "jsp.search.advanced.type." + index;
 					%>
-							<option value="<%= index %>" <%= field3.equals(index) ? "selected=\"selected\"" : "" %>><fmt:message key="<%= key %>"/></option>
+							<option value="<%= StringEscapeUtils.escapeHtml(index) %>" <%= field3.equals(index) ? "selected=\"selected\"" : "" %>><fmt:message key="<%= key %>"/></option>
 					<%
 						}
 					%>

dspace-jspui/src/main/webapp/search/discovery.jsp

@@ -33,6 +33,8 @@
   -   admin_button     - If the user is an admin
   --%>
 
+<%@page import="org.dspace.core.Utils"%>
+<%@page import="com.coverity.security.Escape"%>
 <%@page import="org.dspace.discovery.configuration.DiscoverySearchFilterFacet"%>
 <%@page import="org.dspace.app.webui.util.UIUtil"%>
 <%@page import="java.util.HashMap"%>
@@ -55,7 +57,6 @@
     prefix="c" %>
 
 <%@ taglib uri="http://www.dspace.org/dspace-tags.tld" prefix="dspace" %>
-<%@ page import="org.apache.commons.lang.StringEscapeUtils" %>
 <%@ page import="java.net.URLEncoder"            %>
 <%@ page import="org.dspace.content.Community"   %>
 <%@ page import="org.dspace.content.Collection"  %>
@@ -149,6 +150,9 @@
 				}
 			});
 	});
+	function validateFilters() {
+		return document.getElementById("filterquery").value.length > 0;
+	}
 </script>		
 </c:set>
 
@@ -189,14 +193,14 @@
     }
 %>                                </select><br/>
                                 <label for="query"><fmt:message key="jsp.search.results.searchfor"/></label>
-                                <input type="text" size="50" id="query" name="query" value="<%= (query==null ? "" : StringEscapeUtils.escapeHtml(query)) %>"/>
+                                <input type="text" size="50" id="query" name="query" value="<%= (query==null ? "" : Utils.addEntities(query)) %>"/>
                                 <input type="submit" id="main-query-submit" class="btn btn-primary" value="<fmt:message key="jsp.general.go"/>" />
 <% if (StringUtils.isNotBlank(spellCheckQuery)) {%>
-	<p class="lead"><fmt:message key="jsp.search.didyoumean"><fmt:param><a id="spellCheckQuery" data-spell="<%= StringEscapeUtils.escapeHtml(spellCheckQuery) %>" href="#"><%= spellCheckQuery %></a></fmt:param></fmt:message></p>
+	<p class="lead"><fmt:message key="jsp.search.didyoumean"><fmt:param><a id="spellCheckQuery" data-spell="<%= Utils.addEntities(spellCheckQuery) %>" href="#"><%= spellCheckQuery %></a></fmt:param></fmt:message></p>
 <% } %>                  
                                 <input type="hidden" value="<%= rpp %>" name="rpp" />
-                                <input type="hidden" value="<%= sortedBy %>" name="sort_by" />
-                                <input type="hidden" value="<%= order %>" name="order" />
+                                <input type="hidden" value="<%= Utils.addEntities(sortedBy) %>" name="sort_by" />
+                                <input type="hidden" value="<%= Utils.addEntities(order) %>" name="order" />
 <% if (appliedFilters.size() > 0 ) { %>                                
 		<div class="discovery-search-appliedFilters">
 		<span><fmt:message key="jsp.search.filter.applied" /></span>
@@ -210,8 +214,8 @@
 				<%
 					for (DiscoverySearchFilter searchFilter : availableFilters)
 					{
-					    String fkey = "jsp.search.filter."+searchFilter.getIndexFieldName();
-					    %><option value="<%= searchFilter.getIndexFieldName() %>"<% 
+					    String fkey = "jsp.search.filter." + Escape.uriParam(searchFilter.getIndexFieldName());
+					    %><option value="<%= Utils.addEntities(searchFilter.getIndexFieldName()) %>"<% 
 					            if (filter[0].equals(searchFilter.getIndexFieldName()))
 					            {
 					                %> selected="selected"<%
@@ -221,8 +225,8 @@
 					}
 					if (!found)
 					{
-					    String fkey = "jsp.search.filter."+filter[0];
-					    %><option value="<%= filter[0] %>" selected="selected"><fmt:message key="<%= fkey %>"/></option><%
+					    String fkey = "jsp.search.filter." + Escape.uriParam(filter[0]);
+					    %><option value="<%= Utils.addEntities(filter[0]) %>" selected="selected"><fmt:message key="<%= fkey %>"/></option><%
 					}
 				%>
 				</select>
@@ -230,12 +234,12 @@
 				<%
 					for (String opt : options)
 					{
-					    String fkey = "jsp.search.filter.op."+opt;
-					    %><option value="<%= opt %>"<%= opt.equals(filter[1])?" selected=\"selected\"":"" %>><fmt:message key="<%= fkey %>"/></option><%
+					    String fkey = "jsp.search.filter.op." + Escape.uriParam(opt);
+					    %><option value="<%= Utils.addEntities(opt) %>"<%= opt.equals(filter[1])?" selected=\"selected\"":"" %>><fmt:message key="<%= fkey %>"/></option><%
 					}
 				%>
 				</select>
-				<input type="text" id="filter_value_<%=idx %>" name="filter_value_<%=idx %>" value="<%= StringEscapeUtils.escapeHtml(filter[2]) %>" size="45"/>
+				<input type="text" id="filter_value_<%=idx %>" name="filter_value_<%=idx %>" value="<%= Utils.addEntities(filter[2]) %>" size="45"/>
 				<input class="btn btn-default" type="submit" id="submit_filter_remove_<%=idx %>" name="submit_filter_remove_<%=idx %>" value="X" />
 				<br/>
 				<%
@@ -252,17 +256,17 @@
 		<h5><fmt:message key="jsp.search.filter.heading" /></h5>
 		<p class="discovery-search-filters-hint"><fmt:message key="jsp.search.filter.hint" /></p>
 		<form action="simple-search" method="get">
-		<input type="hidden" value="<%= StringEscapeUtils.escapeHtml(searchScope) %>" name="location" />
-		<input type="hidden" value="<%= StringEscapeUtils.escapeHtml(query) %>" name="query" />
+		<input type="hidden" value="<%= Utils.addEntities(searchScope) %>" name="location" />
+		<input type="hidden" value="<%= Utils.addEntities(query) %>" name="query" />
 		<% if (appliedFilterQueries.size() > 0 ) { 
 				int idx = 1;
 				for (String[] filter : appliedFilters)
 				{
 				    boolean found = false;
 				    %>
-				    <input type="hidden" id="filter_field_<%=idx %>" name="filter_field_<%=idx %>" value="<%= filter[0] %>" />
-					<input type="hidden" id="filter_type_<%=idx %>" name="filter_type_<%=idx %>" value="<%= filter[1] %>" />
-					<input type="hidden" id="filter_value_<%=idx %>" name="filter_value_<%=idx %>" value="<%= StringEscapeUtils.escapeHtml(filter[2]) %>" />
+				    <input type="hidden" id="filter_field_<%=idx %>" name="filter_field_<%=idx %>" value="<%= Utils.addEntities(filter[0]) %>" />
+					<input type="hidden" id="filter_type_<%=idx %>" name="filter_type_<%=idx %>" value="<%= Utils.addEntities(filter[1]) %>" />
+					<input type="hidden" id="filter_value_<%=idx %>" name="filter_value_<%=idx %>" value="<%= Utils.addEntities(filter[2]) %>" />
 					<%
 					idx++;
 				}
@@ -271,8 +275,8 @@
 		<%
 			for (DiscoverySearchFilter searchFilter : availableFilters)
 			{
-			    String fkey = "jsp.search.filter."+searchFilter.getIndexFieldName();
-			    %><option value="<%= searchFilter.getIndexFieldName() %>"><fmt:message key="<%= fkey %>"/></option><%
+			    String fkey = "jsp.search.filter." + Escape.uriParam(searchFilter.getIndexFieldName());
+			    %><option value="<%= Utils.addEntities(searchFilter.getIndexFieldName()) %>"><fmt:message key="<%= fkey %>"/></option><%
 			}
 		%>
 		</select>
@@ -280,33 +284,33 @@
 		<%
 			for (String opt : options)
 			{
-			    String fkey = "jsp.search.filter.op."+opt;
-			    %><option value="<%= opt %>"><fmt:message key="<%= fkey %>"/></option><%
+			    String fkey = "jsp.search.filter.op." + Escape.uriParam(opt);
+			    %><option value="<%= Utils.addEntities(opt) %>"><fmt:message key="<%= fkey %>"/></option><%
 			}
 		%>
 		</select>
-		<input type="text" id="filterquery" name="filterquery" size="45"/>
-        <input type="hidden" value="<%= rpp %>" name="rpp" />
-        <input type="hidden" value="<%= sortedBy %>" name="sort_by" />
-        <input type="hidden" value="<%= order %>" name="order" />
-		<input class="btn btn-default" type="submit" value="<fmt:message key="jsp.search.filter.add"/>" />
+		<input type="text" id="filterquery" name="filterquery" size="45" required="required" />
+		<input type="hidden" value="<%= rpp %>" name="rpp" />
+		<input type="hidden" value="<%= Utils.addEntities(sortedBy) %>" name="sort_by" />
+		<input type="hidden" value="<%= Utils.addEntities(order) %>" name="order" />
+		<input class="btn btn-default" type="submit" value="<fmt:message key="jsp.search.filter.add"/>" onclick="return validateFilters()" />
 		</form>
 		</div>        
 <% } %>
         <%-- Include a component for modifying sort by, order, results per page, and et-al limit --%>
    <div class="discovery-pagination-controls panel-footer">
    <form action="simple-search" method="get">
-   <input type="hidden" value="<%= StringEscapeUtils.escapeHtml(searchScope) %>" name="location" />
-   <input type="hidden" value="<%= StringEscapeUtils.escapeHtml(query) %>" name="query" />
+   <input type="hidden" value="<%= Utils.addEntities(searchScope) %>" name="location" />
+   <input type="hidden" value="<%= Utils.addEntities(query) %>" name="query" />
 	<% if (appliedFilterQueries.size() > 0 ) { 
 				int idx = 1;
 				for (String[] filter : appliedFilters)
 				{
 				    boolean found = false;
 				    %>
-				    <input type="hidden" id="filter_field_<%=idx %>" name="filter_field_<%=idx %>" value="<%= filter[0] %>" />
-					<input type="hidden" id="filter_type_<%=idx %>" name="filter_type_<%=idx %>" value="<%= filter[1] %>" />
-					<input type="hidden" id="filter_value_<%=idx %>" name="filter_value_<%=idx %>" value="<%= StringEscapeUtils.escapeHtml(filter[2]) %>" />
+				    <input type="hidden" id="filter_field_<%=idx %>" name="filter_field_<%=idx %>" value="<%= Utils.addEntities(filter[0]) %>" />
+					<input type="hidden" id="filter_type_<%=idx %>" name="filter_type_<%=idx %>" value="<%= Utils.addEntities(filter[1]) %>" />
+					<input type="hidden" id="filter_value_<%=idx %>" name="filter_value_<%=idx %>" value="<%= Utils.addEntities(filter[2]) %>" />
 					<%
 					idx++;
 				}
@@ -335,8 +339,8 @@
                for (String sortBy : sortOptions)
                {
                    String selected = (sortBy.equals(sortedBy) ? "selected=\"selected\"" : "");
-                   String mKey = "search.sort-by." + sortBy;
-                   %> <option value="<%= sortBy %>" <%= selected %>><fmt:message key="<%= mKey %>"/></option><%
+                   String mKey = "search.sort-by." + Utils.addEntities(sortBy);
+                   %> <option value="<%= Utils.addEntities(sortBy) %>" <%= selected %>><fmt:message key="<%= mKey %>"/></option><%
                }
 %>
                </select>
@@ -432,7 +436,7 @@ else if( qResults != null)
     
     // create the URLs accessing the previous and next search result pages
     String baseURL =  request.getContextPath()
-                    + searchScope
+                    + (searchScope != "" ? "/handle/" + searchScope : "")
                     + "/simple-search?query="
                     + URLEncoder.encode(query,"UTF-8")
                     + httpFilters
@@ -628,28 +632,30 @@ else
 		
 	for (DiscoverySearchFilterFacet facetConf : facetsConf)
 	{
-	    String f = facetConf.getIndexFieldName();
-	    List<FacetResult> facet = qResults.getFacetResult(f);
-	    if (facet.size() == 0)
-	    {
-	        facet = qResults.getFacetResult(f+".year");
+		if(qResults!=null) {
+		    String f = facetConf.getIndexFieldName();
+		    List<FacetResult> facet = qResults.getFacetResult(f);
 		    if (facet.size() == 0)
 		    {
-		        showFacets.put(f, false);
-		        continue;
+		        facet = qResults.getFacetResult(f+".year");
+			    if (facet.size() == 0)
+			    {
+			        showFacets.put(f, false);
+			        continue;
+			    }
 		    }
-	    }
-	    boolean showFacet = false;
-	    for (FacetResult fvalue : facet)
-	    { 
-			if(!appliedFilterQueries.contains(f+"::"+fvalue.getFilterType()+"::"+fvalue.getAsFilterQuery()))
-		    {
-		        showFacet = true;
-		        break;
+		    boolean showFacet = false;
+		    for (FacetResult fvalue : facet)
+		    { 
+				if(!appliedFilterQueries.contains(f+"::"+fvalue.getFilterType()+"::"+fvalue.getAsFilterQuery()))
+			    {
+			        showFacet = true;
+			        break;
+			    }
 		    }
-	    }
-	    showFacets.put(f, showFacet);
-	    brefine = brefine || showFacet;
+		    showFacets.put(f, showFacet);
+		    brefine = brefine || showFacet;
+		}
 	}
 	if (brefine) {
 %>
@@ -743,5 +749,4 @@ else
 </div>
 <% } %>
 </dspace:sidebar>
-</dspace:layout>
-
+</dspace:layout>

dspace-jspui/src/main/webapp/search/results.jsp

@@ -396,7 +396,7 @@ if (pageTotal > pageCurrent)
 </p>
 
 <form id="dso-display" action="<%=request.getContextPath()%>/dso-display" method="post">
-    <input type="hidden" name="query"   value="<%=query%>"/>
+    <input type="hidden" name="query"   value="<%=StringEscapeUtils.escapeHtml(query)%>"/>
     <input type="hidden" name="rpp"     value="<%=rpp%>"/>
     <input type="hidden" name="page"   value="<%=pageCurrent%>"/>
     <input type="hidden" name="sort_by" value="<%=(so != null ? so.getNumber() : 0)%>"/>

dspace-jspui/src/main/webapp/tools/eperson-list.jsp

@@ -130,7 +130,16 @@
         <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
         <link rel="stylesheet" href="<%= request.getContextPath() %>/styles.css" type="text/css"/>
         <link rel="shortcut icon" href="<%= request.getContextPath() %>/favicon.ico" type="image/x-icon"/>
-
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/jquery-ui-1.10.3.custom/redmond/jquery-ui-1.10.3.custom.css" type="text/css" />
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/bootstrap/bootstrap.min.css" type="text/css" />
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/bootstrap/bootstrap-theme.min.css" type="text/css" />
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/bootstrap/dspace-theme.css" type="text/css" />
+        <script type='text/javascript' src="<%= request.getContextPath() %>/static/js/jquery/jquery-1.10.2.min.js"></script>
+        <script type='text/javascript' src='<%= request.getContextPath() %>/static/js/jquery/jquery-ui-1.10.3.custom.min.js'></script>
+        <script type='text/javascript' src='<%= request.getContextPath() %>/static/js/bootstrap/bootstrap.min.js'></script>
+        <script type='text/javascript' src='<%= request.getContextPath() %>/static/js/holder.js'></script>
+        <script type="text/javascript" src="<%= request.getContextPath() %>/utils.js"></script>
+							
 <script type="text/javascript">
 <!-- Begin
 
@@ -169,73 +178,65 @@ function clearEPeople()
 	<p class="submitFormHelp"><fmt:message key="jsp.tools.eperson-list.info1"/></p>
 <%  } %>
 <center>
-<form method="get">
-    <input type="hidden" name="first" value="<%= first %>" />
-    <input type="hidden" name="sortby" value="<%= sortBy %>" />
-    <input type="hidden" name="multiple" value="<%= multiple %>" />    
-    <label for="search"><fmt:message key="jsp.tools.eperson-list.search.query" /></label><input type="text" name="search" value="<%= search %>"/>
-    <input type="submit" value="<fmt:message key="jsp.tools.eperson-list.search.submit" />" />
-<%
-    if (search != null && !search.equals(""))
-    {   %>
-    <br/>
-    <a href="<%= request.getContextPath() + "/tools/eperson-list?multiple=" + multiple + "&sortby=" + sortByParam + "&first="+first %>"><fmt:message key="jsp.tools.eperson-list.search.return-browse" /></a>	
-<%
-    }    
-%>
-</form>
+	<form method="get">
+	    <input type="hidden" name="first" value="<%= first %>" />
+	    <input type="hidden" name="sortby" value="<%= sortBy %>" />
+	    <input type="hidden" name="multiple" value="<%= multiple %>" />    
+	    <label for="search"><fmt:message key="jsp.tools.eperson-list.search.query"/></label>
+	    <input class="form-control" style="width:200px;"type="text" name="search" value="<%= search %>"/>
+	    <input class="btn btn-success" type="submit" value="<fmt:message key="jsp.tools.eperson-list.search.submit" />" />
+	<%
+	    if (search != null && !search.equals("")){   %>
+	    <a class="btn btn-warning" href="<%= request.getContextPath() + "/tools/eperson-list?multiple=" + multiple + "&sortby=" + sortByParam + "&first="+first %>"><fmt:message key="jsp.tools.eperson-list.search.return-browse" /></a>	
+		<%}%>
+		
+	</form>
 </center>
+
 <%-- Controls for jumping around list--%>
-	<table width="99%">
-		<tr>
-		 <%--   <td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0">First</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>">&lt; 5 Pages</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>">&lt; 1 Page</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>">1 Page &gt;</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>">5 Pages &gt;</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>">Last</a></strong></small></td> --%>
-			
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.eperson-list.jump.first"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.eperson-list.jump.five-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.eperson-list.jump.one-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.eperson-list.jump.one-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.eperson-list.jump.five-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.eperson-list.jump.last"/></a></strong></small></td>
-		</tr>
-	</table>
+<div class="span12" style="text-align:center">
+	<ul class="pagination">			
+			<li><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.eperson-list.jump.first"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.eperson-list.jump.five-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.eperson-list.jump.one-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.eperson-list.jump.one-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.eperson-list.jump.five-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.eperson-list.jump.last"/></a></li>
+	</ul>
+</div
 <br/>
 
 	<form method="get" action=""> <%-- Will never actually be posted, it's just so buttons will appear --%>
 
-    <table class="miscTable" align="center" summary="Epeople list">
+    <table class="table table-striped" align="center" summary="Epeople list">
 <% if (search != null && !search.equals(""))
    {  %>
        <tr>
-            <th class="oddRowOddCol">&nbsp;</th>
-            <th class="oddRowEvenCol"><fmt:message key="jsp.tools.eperson-list.th.id" /></th>
-            <th class="oddRowOddCol"><fmt:message key="jsp.tools.eperson-list.th.email" /></th>
-            <th class="oddRowEvenCol"><fmt:message key="jsp.tools.eperson-list.th.lastname" /></th>
-            <th class="oddRowOddCol"><fmt:message key="jsp.tools.eperson-list.th.lastname" /></th>
+            <th>&nbsp;</th>
+            <th><fmt:message key="jsp.tools.eperson-list.th.id" /></th>
+            <th><fmt:message key="jsp.tools.eperson-list.th.email" /></th>
+            <th><fmt:message key="jsp.tools.eperson-list.th.lastname" /></th>
+            <th><fmt:message key="jsp.tools.eperson-list.th.lastname" /></th>
         </tr>
 <% }
    else 
    {  %>
         <tr>
-            <th id="t1" class="oddRowOddCol">&nbsp;</th>
-            <th id="t2" class="oddRowEvenCol"><%
+            <th id="t1">&nbsp;</th>
+            <th id="t2"><%
                 if (sortBy == EPerson.ID)
                 {
-                    %><strong><fmt:message key="jsp.tools.eperson-list.th.id.sortedby" /></strong><%
+                    %><fmt:message key="jsp.tools.eperson-list.th.id"/><span class="glyphicon glyphicon-arrow-down"><%
                 }
                 else
                 {
                     %><a href="<%= sortLink %>id"><fmt:message key="jsp.tools.eperson-list.th.id" /></a><%
                 }
             %></th>
-            <th id="t3" class="oddRowOddCol"><%
+            <th id="t3"><%
                 if (sortBy == EPerson.EMAIL)
                 {
-                    %><strong><fmt:message key="jsp.tools.eperson-list.th.email.sortedby" /></strong><%
+                    %><fmt:message key="jsp.tools.eperson-list.th.email"/><span class="glyphicon glyphicon-arrow-down"><%
                 }
                 else
                 {
@@ -243,10 +244,10 @@ function clearEPeople()
                 }
             %></th>
             <%-- <th class="oddRowEvenCol"><%= sortBy == EPerson.LASTNAME ? "<strong>Last Name &uarr;</strong>" : "<a href=\"" + sortLink + "lastname\">Last Name</a>" %></th> --%>
-            <th id="t4" class="oddRowEvenCol"><%
+            <th id="t4"><%
                 if (sortBy == EPerson.LASTNAME)
                 {
-                    %><fmt:message key="jsp.tools.eperson-list.th.lastname.sortedby" /><%
+                    %><fmt:message key="jsp.tools.eperson-list.th.lastname"/><span class="glyphicon glyphicon-arrow-down"><%
                 }
                 else
                 {
@@ -254,12 +255,12 @@ function clearEPeople()
                 }
             %></th>
 
-            <th id="t5" class="oddRowOddCol"><fmt:message key="jsp.tools.eperson-list.th.firstname"/></th>
+            <th id="t5"><fmt:message key="jsp.tools.eperson-list.th.firstname"/></th>
  
-             <th id="t6" class="oddRowEvenCol"><%
+             <th id="t6"><%
                 if (sortBy == EPerson.LANGUAGE)
                 {
-                    %><fmt:message key="jsp.tools.eperson-list.th.language.sortedby" /><%
+                    %><fmt:message key="jsp.tools.eperson-list.th.language"/><span class="glyphicon glyphicon-arrow-down"></span><%
                 }
                 else
                 {
@@ -285,20 +286,20 @@ function clearEPeople()
         String fullname = StringEscapeUtils.escapeXml(StringEscapeUtils.escapeJavaScript(e.getFullName()));
         String email = StringEscapeUtils.escapeXml(StringEscapeUtils.escapeJavaScript(e.getEmail()));
 %>
-        <tr>
-			<td headers="t1" class="<%= row %>RowOddCol">
-			    <input type="button" value="<%
-	if (multiple) { %><fmt:message key="jsp.tools.general.add"/><% }
-	else {          %><fmt:message key="jsp.tools.general.select"/><% } %>" onclick="javascript:<%= clearList %>addEPerson(<%= e.getID() %>, '<%= email %>', '<%= fullname %>');<%= closeWindow %>"/></td>
-			<td headers="t2" class="<%= row %>RowEvenCol"><%= e.getID() %></td>
-			<td headers="t3" class="<%= row %>RowOddCol"><%= (e.getEmail() == null ? "" : Utils.addEntities(e.getEmail())) %></td>
-            <td headers="t4" class="<%= row %>RowEvenCol">
+  <tr>
+			<td headers="t1">
+			    <input class="btn btn-success" type="button" value="<%
+			if (multiple) { %><fmt:message key="jsp.tools.general.add"/><% }
+			else {          %><fmt:message key="jsp.tools.general.select"/><% } %>" onclick="javascript:<%= clearList %>addEPerson(<%= e.getID() %>, '<%= email %>', '<%= Utils.addEntities(fullname) %>');<%= closeWindow %>"/></td>
+			<td headers="t2"><%= e.getID() %></td>
+			<td headers="t3"><%= (e.getEmail() == null ? "" : Utils.addEntities(e.getEmail())) %></td>
+            <td headers="t4">
                 <%= (e.getLastName() == null ? "" : Utils.addEntities(e.getLastName())) %>
             </td>
-            <td headers="t5" class="<%= row %>RowOddCol">
+            <td headers="t5">
                 <%= (e.getFirstName() == null ? "" : Utils.addEntities(e.getFirstName())) %>
             </td>
-            <td headers="t6" class="<%= row %>RowOddCol">
+            <td headers="t6">
                 <%= (e.getLanguage() == null ? "" : Utils.addEntities(e.getLanguage())) %>
             </td>
         </tr>
@@ -311,28 +312,21 @@ function clearEPeople()
 <br/>
 
 <%-- Controls for jumping around list--%>
-	<table width="99%">
-		<tr>
-			<%--
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0">First</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>">&lt; 5 Pages</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>">&lt; 1 Page</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>">1 Page &gt;</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>">5 Pages &gt;</a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>">Last</a></strong></small></td> 
-			--%>
-			
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.eperson-list.jump.first"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.eperson-list.jump.five-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.eperson-list.jump.one-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.eperson-list.jump.one-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.eperson-list.jump.five-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.eperson-list.jump.last"/></a></strong></small></td>
-		</tr>
-	</table>
+<div class="span12" style="text-align:center">
+	<ul class="pagination">
+			<li><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.eperson-list.jump.first"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.eperson-list.jump.five-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.eperson-list.jump.one-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.eperson-list.jump.one-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.eperson-list.jump.five-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.eperson-list.jump.last"/></a></li>
+	</ul>
+</div>
 
 	<%-- <p align="center"><input type="button" value="Close" onClick="window.close();"/></p> --%>
-	<p align="center"><input type="button" value="<fmt:message key="jsp.tools.eperson-list.close.button"/>" onclick="window.close();"/></p>
+	<p align="center">
+		<input type="button" class="btn btn-danger" value="<fmt:message key="jsp.tools.eperson-list.close.button"/>" onclick="window.close();"/>
+			</p>
 
 	</form>
 

dspace-jspui/src/main/webapp/tools/group-edit.jsp

@@ -49,7 +49,7 @@
                parentlink="/dspace-admin"
                nocache="true">
 
-	<h1><fmt:message key="jsp.tools.group-edit.title"/> : <%=group.getName()%> (id: <%=group.getID()%>)
+	<h1><fmt:message key="jsp.tools.group-edit.title"/> : <%=Utils.addEntities(group.getName())%> (id: <%=group.getID()%>)
 	<dspace:popup page="<%= LocaleSupport.getLocalizedMessage(pageContext, \"help.collection-admin\") +\"#groupeditor\"%>"><fmt:message key="jsp.help"/></dspace:popup>
 	</h1>
     <form name="epersongroup" method="post" action="">

dspace-jspui/src/main/webapp/tools/group-list.jsp

@@ -27,6 +27,7 @@
 
 <%@ page import="org.dspace.eperson.EPerson" %>
 <%@ page import="org.dspace.eperson.Group" %>
+<%@ page import="org.dspace.core.Utils" %>
 
 <%
     Group[] groups =
@@ -72,7 +73,7 @@
             <tr>
                 <td class="<%= row %>RowOddCol"><%= groups[i].getID() %></td>
                 <td class="<%= row %>RowEvenCol">
-                    <%= groups[i].getName() %>
+                    <%= Utils.addEntities(groups[i].getName()) %>
                 </td>
                 <td class="<%= row %>RowOddCol">
 <%

dspace-jspui/src/main/webapp/tools/group-select-list.jsp

@@ -75,7 +75,11 @@
         <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
         <link rel="stylesheet" href="<%= request.getContextPath() %>/styles.css" type="text/css"/>
         <link rel="shortcut icon" href="<%= request.getContextPath() %>/favicon.ico" type="image/x-icon"/>
-
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/bootstrap/bootstrap.min.css" type="text/css" />
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/bootstrap/bootstrap-theme.min.css" type="text/css" />
+        <link rel="stylesheet" href="<%= request.getContextPath() %>/static/css/bootstrap/dspace-theme.css" type="text/css" />
+        <script type='text/javascript' src='<%= request.getContextPath() %>/static/js/bootstrap/bootstrap.min.js'></script>
+		
 <script type="text/javascript">
 <!-- Begin
 
@@ -116,35 +120,27 @@ function clearGroups()
 <%  } %>
     
 <%-- Controls for jumping around list--%>
-	<table width="99%">
-		<tr>
-			<%-- <td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0">First</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>">&lt; 5 Pages</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>">&lt; 1 Page</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>">1 Page &gt;</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>">5 Pages &gt;</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>">Last</A></strong></small></td>
-			--%>
-		
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.group-select-list.jump.first"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.group-select-list.jump.five-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.group-select-list.jump.one-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.group-select-list.jump.one-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.group-select-list.jump.five-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.group-select-list.jump.last"/></a></strong></small></td>
-		</tr>
-	</table>
+<div class="span12" style="text-align:center">
+	<ul class="pagination">			
+			<li><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.group-select-list.jump.first"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.group-select-list.jump.five-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.group-select-list.jump.one-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.group-select-list.jump.one-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.group-select-list.jump.five-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.group-select-list.jump.last"/></a></li>
+	</ul>
+</div>
 <br/>
 
 	<form method="get" action=""> <%-- Will never actually be posted, it's just so buttons will appear --%>
 
-    <table class="miscTable" align="center" summary="Group list">
+    <table class="table table-striped" align="center" summary="Group list">
         <tr>
             <th id="t1" class="oddRowOddCol">&nbsp;</th>
 			<th id="t2" class="oddRowEvenCol"><%
                 if (sortBy == Group.ID)
                 {
-                    %><strong><fmt:message key="jsp.tools.group-select-list.th.id.sortedby" /></strong><%
+                    %><fmt:message key="jsp.tools.group-select-list.th.id"/><span class="glyphicon glyphicon-arrow-down"><%
                 }
                 else
                 {
@@ -154,7 +150,7 @@ function clearGroups()
             <th id="t3" class="oddRowOddCol"><%
                 if (sortBy == Group.NAME)
                 {
-                    %><strong><fmt:message key="jsp.tools.group-select-list.th.name.sortedby" /></strong><%
+                    %><fmt:message key="jsp.tools.group-select-list.th.name" /><span class="glyphicon glyphicon-arrow-down"><%
                 }
                 else
                 {
@@ -180,12 +176,12 @@ function clearGroups()
         String fullname = g.getName().replace('\'', ' ');
 %>
         <tr>
-			<td headers="t1" class="<%= row %>RowOddCol">
-				<input type="button" value="<%
+			<td headers="t1" class="">
+				<input type="button" class="btn btn-success" value="<%
 	if (multiple) { %><fmt:message key="jsp.tools.general.add"/><% }
 	else {          %><fmt:message key="jsp.tools.general.select"/><% } %>" onclick="javascript:<%= clearList %>addGroup('<%= g.getID() %>', '<%= Utils.addEntities(fullname) %>');<%= closeWindow %>"/></td>
-			<td headers="t2" class="<%= row %>RowEvenCol"><%= g.getID() %></td>
-			<td headers="t3" class="<%= row %>RowOddCol"> <%= g.getName()%></td>
+			<td headers="t2" class=""><%= g.getID() %></td>
+			<td headers="t3" class=""> <%= Utils.addEntities(g.getName())%></td>
         </tr>
 <%
         row = (row.equals("odd") ? "even" : "odd");
@@ -196,27 +192,19 @@ function clearGroups()
 <br/>
 
 <%-- Controls for jumping around list--%>
-	<table width="99%">
-		<tr>
-			<%-- <td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0">First</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>">&lt; 5 Pages</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>">&lt; 1 Page</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>">1 Page &gt;</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>">5 Pages &gt;</A></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>">Last</A></strong></small></td>
-			--%>
-		
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.group-select-list.jump.first"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.group-select-list.jump.five-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.group-select-list.jump.one-back"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.group-select-list.jump.one-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.group-select-list.jump.five-forward"/></a></strong></small></td>
-			<td width="17%" align="center"><small><strong><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.group-select-list.jump.last"/></a></strong></small></td>
-		</tr>
-	</table>
+<div class="span12" style="text-align:center">
+	<ul class="pagination">	
+			<li><a href="<%= jumpLink %>0"><fmt:message key="jsp.tools.group-select-list.jump.first"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveBack %>"><fmt:message key="jsp.tools.group-select-list.jump.five-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneBack %>"><fmt:message key="jsp.tools.group-select-list.jump.one-back"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpOneForward %>"><fmt:message key="jsp.tools.group-select-list.jump.one-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpFiveForward %>"><fmt:message key="jsp.tools.group-select-list.jump.five-forward"/></a></li>
+			<li><a href="<%= jumpLink %><%= jumpEnd %>"><fmt:message key="jsp.tools.group-select-list.jump.last"/></a></li>
+	</ul>
+</div>
 
 	<%-- <p align="center"><input type="button" value="Close" onClick="window.close();"></p> --%>
-	<p align="center"><input type="button" value="<fmt:message key="jsp.tools.group-select-list.close.button"/>" onclick="window.close();"/></p>
+	<p align="center"><input type="button" class="btn btn-danger" value="<fmt:message key="jsp.tools.group-select-list.close.button"/>" onclick="window.close();"/></p>
 
 	</form>
 	

dspace-lni/dspace-lni-client/pom.xml

@@ -11,7 +11,7 @@
    <parent>
       <groupId>org.dspace</groupId>
       <artifactId>dspace-parent</artifactId>
-      <version>4.0</version>
+      <version>4.9</version>
       <relativePath>../..</relativePath>
    </parent>
    

dspace-lni/pom.xml

@@ -11,7 +11,7 @@
    <parent>
       <groupId>org.dspace</groupId>
       <artifactId>dspace-parent</artifactId>
-      <version>4.0</version>
+      <version>4.9</version>
       <relativePath>..</relativePath>
    </parent>
 

dspace-oai/pom.xml

@@ -8,7 +8,7 @@
     <parent>
         <artifactId>dspace-parent</artifactId>
         <groupId>org.dspace</groupId>
-        <version>4.0</version>
+        <version>4.9</version>
         <relativePath>..</relativePath>
     </parent>
 

dspace-oai/src/main/java/org/dspace/xoai/DSpaceOAIDataProvider.java

@@ -158,6 +158,11 @@ public class DSpaceOAIDataProvider extends HttpServlet
                     "Requested OAI context \""
                     + request.getPathInfo().replace("/", "")
                     + "\" does not exist");
+        } finally {
+            if(context != null && context.isValid())
+            {
+                context.abort();
+            }
         }
 
     }

dspace-oai/src/main/java/org/dspace/xoai/util/DateUtils.java

@@ -7,13 +7,14 @@
  */
 package org.dspace.xoai.util;
 
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.Locale;
-
-import org.apache.log4j.LogManager;
-import org.apache.log4j.Logger;
+import java.util.TimeZone;
 
 /**
  * 
@@ -33,8 +34,8 @@ public class DateUtils
         // 2008-01-01T00:00:00Z
     	SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.'000Z'");
     	if (!init) sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.'999Z'");
+        sdf.setTimeZone(TimeZone.getTimeZone("UTC"));
         String ret = sdf.format(date);
-        System.out.println(ret);
         return ret;
     }
 

dspace-oai/src/main/webapp/static/style.xsl

@@ -728,14 +728,20 @@
 									</td>
 									<td class="clear"></td>
 								</tr>
-								<tr class="info">
-									<td class="name">Description</td>
-									<td class="value">
-										<xsl:value-of
-											select="oai:OAI-PMH/oai:Identify/oai:description/node()/text()" />
-									</td>
-									<td class="clear"></td>
-								</tr>
+                                                                <tr class="info">
+                                                                        <td class="name">Repository identifier</td>
+                                                                        <td class="value">
+                                                                                <xsl:value-of select="oai:OAI-PMH/oai:Identify/oai:description//*[local-name() = 'repositoryIdentifier']/text()" />
+                                                                        </td>
+                                                                        <td class="clear"></td>
+                                                                </tr>
+                                                                <tr class="info">
+                                                                        <td class="name">Sample identifier</td>
+                                                                        <td class="value">
+                                                                                <xsl:value-of select="oai:OAI-PMH/oai:Identify/oai:description//*[local-name() = 'sampleIdentifier']/text()" />
+                                                                        </td>
+                                                                        <td class="clear"></td>
+                                                                </tr>
 								<tr>
 									<td class="separator"></td>
 								</tr>

dspace-rest/pom.xml

@@ -3,14 +3,14 @@
     <groupId>org.dspace</groupId>
     <artifactId>dspace-rest</artifactId>
     <packaging>war</packaging>
-    <version>4.0</version>
+    <version>4.9</version>
     <name>DSpace RESTful web services API</name>
     <url>http://demo.dspace.org</url>
 
     <parent>
         <groupId>org.dspace</groupId>
         <artifactId>dspace-parent</artifactId>
-        <version>4.0</version>
+        <version>4.9</version>
         <relativePath>..</relativePath>
     </parent>
 

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

@@ -36,7 +36,6 @@ import java.sql.SQLException;
 @Path("/bitstreams")
 public class BitstreamResource {
     Logger log = Logger.getLogger(BitstreamResource.class);
-    private static org.dspace.core.Context context;
     
     private static final boolean writeStatistics;
 	
@@ -50,12 +49,9 @@ public class BitstreamResource {
     @Path("/{bitstream_id}")
     @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
     public Bitstream getBitstream(@PathParam("bitstream_id") Integer bitstream_id, @QueryParam("expand") String expand) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid()) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Bitstream bitstream = org.dspace.content.Bitstream.find(context, bitstream_id);
 
@@ -67,6 +63,14 @@ public class BitstreamResource {
         } catch(SQLException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
         }
     }
 
@@ -75,17 +79,14 @@ public class BitstreamResource {
     public javax.ws.rs.core.Response getFile(@PathParam("bitstream_id") final Integer bitstream_id,
     		@QueryParam("userIP") String user_ip, @QueryParam("userAgent") String user_agent, @QueryParam("xforwarderfor") String xforwarderfor,
     		@Context HttpHeaders headers, @Context HttpServletRequest request) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Bitstream bitstream = org.dspace.content.Bitstream.find(context, bitstream_id);
             if(AuthorizeManager.authorizeActionBoolean(context, bitstream, org.dspace.core.Constants.READ)) {
             	if(writeStatistics){
-    				writeStats(bitstream_id, user_ip, user_agent, xforwarderfor, headers, request);
+    				writeStats(context, bitstream_id, user_ip, user_agent, xforwarderfor, headers, request);
     			}
             	
                 return Response.ok(bitstream.retrieve()).type(bitstream.getFormat().getMIMEType()).build();
@@ -102,10 +103,18 @@ public class BitstreamResource {
         } catch (AuthorizeException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.UNAUTHORIZED);
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
         }
     }
     
-	private void writeStats(Integer bitstream_id, String user_ip, String user_agent,
+	private void writeStats(org.dspace.core.Context context, Integer bitstream_id, String user_ip, String user_agent,
 			String xforwarderfor, HttpHeaders headers,
 			HttpServletRequest request) {
 		

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

@@ -37,11 +37,8 @@ http://localhost:8080/<webapp>/collections
 @Path("/collections")
 public class CollectionsResource {
     private static Logger log = Logger.getLogger(CollectionsResource.class);
-    
 
     @javax.ws.rs.core.Context ServletContext servletContext;
-
-    private static org.dspace.core.Context context;
     
     private static final boolean writeStatistics;
 	
@@ -49,46 +46,13 @@ public class CollectionsResource {
 		writeStatistics=ConfigurationManager.getBooleanProperty("rest","stats",false);
 	}
 
-    /*
-    The "GET" annotation indicates this method will respond to HTTP Get requests.
-    The "Produces" annotation indicates the MIME response the method will return.
-     */
-    @GET
-    @Path("/")
-    @Produces(MediaType.TEXT_HTML)
-    public String listHTML() {
-        StringBuilder everything = new StringBuilder();
-        try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
-
-            org.dspace.content.Collection[] collections = org.dspace.content.Collection.findAll(context);
-            for(org.dspace.content.Collection collection : collections) {
-                //TODO check auth...
-                everything.append("<li><a href='" + servletContext.getContextPath() + "/collections/" + collection.getID() + "'>" + collection.getID() + " - " + collection.getName() + "</a></li>\n");
-            }
-
-            return "<html><title>Hello!</title><body>Collections<br/><ul>" + everything.toString() + "</ul>.</body></html> ";
-
-        } catch (SQLException e) {
-            log.error(e.getMessage());
-            throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
-        }
-    }
-
     @GET
     @Path("/")
     @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
     public org.dspace.rest.common.Collection[] list(@QueryParam("expand") String expand, @QueryParam("limit") @DefaultValue("100") Integer limit, @QueryParam("offset") @DefaultValue("0") Integer offset) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Collection[] collections;
 
@@ -112,6 +76,14 @@ public class CollectionsResource {
         } catch (SQLException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
         }
     }
 
@@ -122,17 +94,14 @@ public class CollectionsResource {
     		@QueryParam("limit") @DefaultValue("100") Integer limit, @QueryParam("offset") @DefaultValue("0") Integer offset,
     		@QueryParam("userIP") String user_ip, @QueryParam("userAgent") String user_agent, @QueryParam("xforwarderfor") String xforwarderfor,
     		@Context HttpHeaders headers, @Context HttpServletRequest request) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Collection collection = org.dspace.content.Collection.find(context, collection_id);
             if(AuthorizeManager.authorizeActionBoolean(context, collection, org.dspace.core.Constants.READ)) {
             	if(writeStatistics){
-    				writeStats(collection_id, user_ip, user_agent, xforwarderfor, headers, request);
+    				writeStats(context, collection_id, user_ip, user_agent, xforwarderfor, headers, request);
     			}
                 return new org.dspace.rest.common.Collection(collection, expand, context, limit, offset);
             } else {
@@ -141,10 +110,18 @@ public class CollectionsResource {
         } catch (SQLException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
         }
     }
     
-    private void writeStats(Integer collection_id, String user_ip, String user_agent,
+    private void writeStats(org.dspace.core.Context context, Integer collection_id, String user_ip, String user_agent,
 			String xforwarderfor, HttpHeaders headers,
 			HttpServletRequest request) {
 		

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

@@ -36,52 +36,19 @@ http://localhost:8080/<webapp>/communities
 public class CommunitiesResource {
     private static Logger log = Logger.getLogger(CommunitiesResource.class);
 
-    private static org.dspace.core.Context context;
-
     private static final boolean writeStatistics;
 	
 	static{
 		writeStatistics=ConfigurationManager.getBooleanProperty("rest","stats",false);
 	}
-    
-    /*
-    The "GET" annotation indicates this method will respond to HTTP Get requests.
-    The "Produces" annotation indicates the MIME response the method will return.
-     */
-    @GET
-    @Produces(MediaType.TEXT_HTML)
-    public String list() {
-        StringBuilder everything = new StringBuilder();
-        try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
-            org.dspace.content.Community[] communities = org.dspace.content.Community.findAllTop(context);
-            for(org.dspace.content.Community community : communities) {
-                everything.append(community.getName() + "<br/>\n");
-            }
-            return "<html><title>Hello!</title><body>Communities:<br/>" + everything.toString() + ".</body></html> ";
-
-        } catch (SQLException e) {
-            log.error(e.getMessage());
-            throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
-        }
-    }
-
-    //TODO Respond to html for communities/:id
 
     @GET
     @Path("/")
     @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
     public org.dspace.rest.common.Community[] list(@QueryParam("expand") String expand) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Community[] topCommunities = org.dspace.content.Community.findAllTop(context);
             ArrayList<org.dspace.rest.common.Community> communityArrayList = new ArrayList<org.dspace.rest.common.Community>();
@@ -98,7 +65,15 @@ public class CommunitiesResource {
         } catch (SQLException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
-        } //finally?
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
+        }
     }
 
     @GET
@@ -107,17 +82,14 @@ public class CommunitiesResource {
     public org.dspace.rest.common.Community getCommunity(@PathParam("community_id") Integer community_id, @QueryParam("expand") String expand,
     		@QueryParam("userIP") String user_ip, @QueryParam("userAgent") String user_agent, @QueryParam("xforwarderfor") String xforwarderfor,
     		@Context HttpHeaders headers, @Context HttpServletRequest request) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid() ) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Community community = org.dspace.content.Community.find(context, community_id);
             if(AuthorizeManager.authorizeActionBoolean(context, community, org.dspace.core.Constants.READ)) {
             	if(writeStatistics){
-    				writeStats(community_id, user_ip, user_agent, xforwarderfor, headers, request);
+    				writeStats(context, community_id, user_ip, user_agent, xforwarderfor, headers, request);
     			}
                 return new org.dspace.rest.common.Community(community, expand, context);
             } else {
@@ -126,10 +98,18 @@ public class CommunitiesResource {
         } catch (SQLException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
-        } //finally?
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
+        }
     }
     
-    private void writeStats(Integer community_id, String user_ip, String user_agent,
+    private void writeStats(org.dspace.core.Context context, Integer community_id, String user_ip, String user_agent,
  			String xforwarderfor, HttpHeaders headers,
  			HttpServletRequest request) {
  		

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

@@ -32,18 +32,14 @@ import java.sql.SQLException;
 @Path("/handle")
 public class HandleResource {
     private static Logger log = Logger.getLogger(HandleResource.class);
-    private static org.dspace.core.Context context;
 
     @GET
     @Path("/{prefix}/{suffix}")
     @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
     public org.dspace.rest.common.DSpaceObject getObject(@PathParam("prefix") String prefix, @PathParam("suffix") String suffix, @QueryParam("expand") String expand) {
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid() ) {
-                context = new Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.DSpaceObject dso = HandleManager.resolveToObject(context, prefix + "/" + suffix);
             if(dso == null) {
@@ -68,6 +64,14 @@ public class HandleResource {
         } catch (SQLException e) {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
         }
     }
 }

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

@@ -44,28 +44,22 @@ public class ItemsResource {
     private static final Logger log = Logger.getLogger(ItemsResource.class);
     //ItemList - Not Implemented
 
-    private static org.dspace.core.Context context;
-
     @GET
     @Path("/{item_id}")
     @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
     public org.dspace.rest.common.Item getItem(@PathParam("item_id") Integer item_id, @QueryParam("expand") String expand,
     		@QueryParam("userIP") String user_ip, @QueryParam("userAgent") String user_agent, @QueryParam("xforwarderfor") String xforwarderfor,
     		@Context HttpHeaders headers, @Context HttpServletRequest request) throws WebApplicationException {
-    	
-    	
+
+        org.dspace.core.Context context = null;
         try {
-            if(context == null || !context.isValid()) {
-                context = new org.dspace.core.Context();
-                //Failed SQL is ignored as a failed SQL statement, prevent: current transaction is aborted, commands ignored until end of transaction block
-                context.getDBConnection().setAutoCommit(true);
-            }
+            context = new org.dspace.core.Context();
 
             org.dspace.content.Item item = org.dspace.content.Item.find(context, item_id);
 
             if(AuthorizeManager.authorizeActionBoolean(context, item, org.dspace.core.Constants.READ)) {
             	if(writeStatistics){
-    				writeStats(item_id, user_ip, user_agent, xforwarderfor, headers, request);
+    				writeStats(context, item_id, user_ip, user_agent, xforwarderfor, headers, request);
     			}
                 return new org.dspace.rest.common.Item(item, expand, context);
             } else {
@@ -75,11 +69,19 @@ public class ItemsResource {
         } catch (SQLException e)  {
             log.error(e.getMessage());
             throw new WebApplicationException(Response.Status.INTERNAL_SERVER_ERROR);
+        } finally {
+            if(context != null) {
+                try {
+                    context.complete();
+                } catch (SQLException e) {
+                    log.error(e.getMessage() + " occurred while trying to close");
+                }
+            }
         }
     }
     
     
-    private void writeStats(Integer item_id, String user_ip, String user_agent,
+    private void writeStats(org.dspace.core.Context context, Integer item_id, String user_ip, String user_agent,
 			String xforwarderfor, HttpHeaders headers,
 			HttpServletRequest request) {
 		

dspace-services/pom.xml

@@ -9,7 +9,7 @@
     <parent>
         <groupId>org.dspace</groupId>
         <artifactId>dspace-parent</artifactId>
-        <version>4.0</version>
+        <version>4.9</version>
     </parent>
 
     <properties>

dspace-services/src/main/java/org/dspace/services/email/EmailServiceImpl.java

@@ -96,17 +96,6 @@ public class EmailServiceImpl
             {
                 props.put("mail.smtp.port", port);
             }
-
-            if (null == cfg.getProperty("mail.server.username"))
-            {
-                session = Session.getInstance(props);
-            }
-            else
-            {
-                props.put("mail.smtp.auth", "true");
-                session = Session.getInstance(props, this);
-            }
-
             // Set extra configuration properties
             String extras = cfg.getProperty("mail.extraproperties");
             if ((extras != null) && (!"".equals(extras.trim())))
@@ -120,6 +109,17 @@ public class EmailServiceImpl
                     props.put(key, value);
                 }
             }
+            if (null == cfg.getProperty("mail.server.username"))
+            {
+                session = Session.getInstance(props);
+            }
+            else
+            {
+                props.put("mail.smtp.auth", "true");
+                session = Session.getInstance(props, this);
+            }
+
+
         }
     }
 

dspace-services/src/test/java/org/dspace/services/caching/CachingServiceTest.java

@@ -44,6 +44,7 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
     @After
     public void tearDown() {
         cachingService = null;
+        requestService = null;
     }
 
     /**
@@ -79,6 +80,9 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
         EhcacheCache cache2 = cachingService.instantiateEhCache("aaronz-eh", null);
         assertNotNull(cache2);
         assertEquals(cache2, cache);
+        
+        //trash the references
+        cache = cache2 = null;
     }
 
     /**
@@ -101,6 +105,9 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
         assertEquals(cache2, cache);
 
         requestService.endRequest(null);
+        
+        //trash the references
+        cache = cache2 = null;
     }
 
     /**
@@ -140,6 +147,9 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
         assertNotNull(c2);
         assertEquals(c1, c2);
 
+        //trash the references
+        cache = sampleCache = c1 = rc1 = c2 = null;
+
     }
 
     /**
@@ -156,13 +166,18 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
         assertNotNull(memCache);
         assertTrue(caches.contains(memCache));
 
-        Cache c1 = cachingService.getCache("org.dspace.aztest.new", null);
+        // This should create a new cache (as cache name is unique)
+        Cache c1 = cachingService.getCache("org.dspace.timtest.newcache", null);
         assertNotNull(c1);
 
+        // Test that new cache was created and total caches increases by one 
         caches = cachingService.getCaches();
         assertNotNull(caches);
         assertEquals(curSize+1, caches.size());
         assertTrue(caches.contains(c1));
+        
+        //trash the references
+        memCache = c1 = null;
     }
 
     /**
@@ -198,6 +213,8 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
 
         assertEquals(null, c1.get("AZ"));
         assertEquals(0, c1.size());
+        
+        c1 = null;
     }
 
     /**
@@ -227,6 +244,9 @@ public class CachingServiceTest extends DSpaceAbstractKernelTest {
         Cache cb = cachingService.getCache("org.dspace.aztest", null);
         assertNotNull(cb);
         assertNotSame(ca, cb);
+        
+        //trash the references
+        cache = c2 = ca = cb = null;
     }
 
 }

dspace-solr/pom.xml

@@ -20,7 +20,7 @@
     <parent>
         <groupId>org.dspace</groupId>
         <artifactId>dspace-parent</artifactId>
-        <version>4.0</version>
+        <version>4.9</version>
         <relativePath>..</relativePath>
     </parent>
 
@@ -126,6 +126,11 @@
             <artifactId>solr-solrj</artifactId>
             <version>${solr.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.apache.solr</groupId>
+            <artifactId>solr-cell</artifactId>
+            <version>${solr.version}</version>
+        </dependency>
         <dependency>
             <groupId>org.apache.lucene</groupId>
             <artifactId>lucene-core</artifactId>

dspace-sword/pom.xml

@@ -15,7 +15,7 @@
    <parent>
       <groupId>org.dspace</groupId>
       <artifactId>dspace-parent</artifactId>
-      <version>4.0</version>
+      <version>4.9</version>
       <relativePath>..</relativePath>
    </parent>
 

dspace-swordv2/pom.xml

@@ -13,7 +13,7 @@
     <parent>
         <groupId>org.dspace</groupId>
         <artifactId>dspace-parent</artifactId>
-        <version>4.0</version>
+        <version>4.9</version>
         <relativePath>..</relativePath>
     </parent>
 

dspace-xmlui/pom.xml

@@ -11,7 +11,7 @@
    <parent>
       <groupId>org.dspace</groupId>
       <artifactId>dspace-parent</artifactId>
-      <version>4.0</version>
+      <version>4.9</version>
       <relativePath>..</relativePath>
    </parent>
 

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

@@ -182,7 +182,7 @@ public class FlowEPersonUtils {
         		{
         			personModified.setEmail(email);
         		} 
-        		else if (potentialDupicate.equals(personModified)) 
+        		else
         		{	       
         			// set a special field in error so that the transformer can display a pretty error.
         			result.addError("eperson_email_key");

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/artifactbrowser/ConfigurableBrowse.java

@@ -707,7 +707,7 @@ public class ConfigurableBrowse extends AbstractDSpaceTransformer implements
                     // This is so that we can then highlight the correct option in the navigation
                     SortOption bso = bi.getSortOption();
                     SortOption so = SortOption.getSortOption(sortBy);
-                    if ( bso != null && bso.equals(so))
+                    if ( bso != null && !bso.equals(so))
                     {
                         BrowseIndex newBi = BrowseIndex.getBrowseIndex(so);
                         if (newBi != null)

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/discovery/AbstractRecentSubmissionTransformer.java

@@ -42,6 +42,11 @@ public abstract class AbstractRecentSubmissionTransformer extends AbstractDSpace
      */
     protected DiscoverResult queryResults;
 
+    /**
+     * The maximum number of recent submissions read from configuration.
+     */
+    protected int maxRecentSubmissions;
+
     /** Cached validity object */
     private SourceValidity validity;
 
@@ -132,7 +137,8 @@ public abstract class AbstractRecentSubmissionTransformer extends AbstractDSpace
 
             DiscoveryRecentSubmissionsConfiguration recentSubmissionConfiguration = discoveryConfiguration.getRecentSubmissionConfiguration();
             if(recentSubmissionConfiguration != null){
-                queryArgs.setMaxResults(recentSubmissionConfiguration.getMax());
+                maxRecentSubmissions = recentSubmissionConfiguration.getMax();
+                queryArgs.setMaxResults(maxRecentSubmissions);
                 String sortField = SearchUtils.getSearchService().toSortFieldIndex(recentSubmissionConfiguration.getMetadataSortField(), recentSubmissionConfiguration.getType());
                 if(sortField != null){
                     queryArgs.setSortField(
@@ -171,6 +177,7 @@ public abstract class AbstractRecentSubmissionTransformer extends AbstractDSpace
     public void recycle() {
         queryResults = null;
         validity = null;
+        maxRecentSubmissions = 0;
         super.recycle();
     }
 

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/discovery/CollectionRecentSubmissions.java

@@ -75,8 +75,9 @@ public class CollectionRecentSubmissions extends AbstractRecentSubmissionTransfo
                     lastSubmitted.addReference(resultObj);
                 }
             }
-            addViewMoreLink(lastSubmittedDiv, collection);
 
+            if (collection.countItems() > maxRecentSubmissions)
+                addViewMoreLink(lastSubmittedDiv, collection);
         }
     }
 }

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/discovery/CommunityRecentSubmissions.java

@@ -73,7 +73,11 @@ public class CommunityRecentSubmissions extends AbstractRecentSubmissionTransfor
                     lastSubmitted.addReference(resultObject);
                 }
             }
-            addViewMoreLink(lastSubmittedDiv, dso);
+
+            Community community = (Community) dso;
+
+            if (community.countItems() > maxRecentSubmissions)
+                addViewMoreLink(lastSubmittedDiv, dso);
         }
     }
 }

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/discovery/SidebarFacetsTransformer.java

@@ -317,7 +317,7 @@ public class SidebarFacetsTransformer extends AbstractDSpaceTransformer implemen
         DiscoveryConfiguration discoveryConfiguration = SearchUtils.getDiscoveryConfiguration(scope);
         java.util.List<DiscoverySearchFilterFacet> facets = discoveryConfiguration.getSidebarFacets();
 
-        log.info("facets for scope, " + scope + ": " + (facets != null ? facets.size() : null));
+        log.debug("facets for scope, " + scope + ": " + (facets != null ? facets.size() : null));
 
 
 

dspace-xmlui/src/main/java/org/dspace/app/xmlui/cocoon/ConcatenationReader.java

@@ -12,6 +12,7 @@ import com.yahoo.platform.yui.compressor.JavaScriptCompressor;
 import org.apache.avalon.framework.parameters.ParameterException;
 import org.apache.avalon.framework.parameters.Parameters;
 import org.apache.cocoon.ProcessingException;
+import org.apache.cocoon.ResourceNotFoundException;
 import org.apache.cocoon.environment.*;
 import org.apache.cocoon.reading.ResourceReader;
 import org.apache.excalibur.source.Source;
@@ -69,10 +70,19 @@ public class ConcatenationReader extends ResourceReader {
 
         // setup list of sources, get relevant parts of path
         this.inputSources = new ArrayList<Source>();
-        String path = src.substring(0, src.lastIndexOf('/'));
+        
+        // Check for an empty path
+        String path = "";
+        if(src.contains("/"))
+        {
+            path = src.substring(0, src.lastIndexOf('/'));
+        }
         String file = src.substring(src.lastIndexOf('/')+1);
 
-        // now build own list of inputsources
+        // Now build own list of inputsources
+        // Several files may be passed in at once, e.g.
+        // "themes/Mirage/lib/css/reset,base,helper,style,print.css"
+        // So, we need to build the fullPath to *each* file individually
         String[] files = file.split(",");
         for (String f : files) {
             if (file.endsWith(".json") && !f.endsWith(".json")) {
@@ -85,8 +95,21 @@ public class ConcatenationReader extends ResourceReader {
                 f += ".css";
             }
 
-            String fullPath = path + "/" + f;
-            this.inputSources.add(resolver.resolveURI(fullPath));
+            // Build full path to this individual file
+            String fullPath;
+            if(!path.isEmpty())
+                fullPath = path + "/" + f;
+            else
+                fullPath = f;
+
+            // Add to list of inputsources if this file exists
+            Source inSource = resolver.resolveURI(fullPath);
+            if(inSource.exists())
+            {
+                this.inputSources.add(inSource);
+            }
+            else // else throw a ResourceNotFound (which triggers a 404)
+                throw new ResourceNotFoundException("Resource not found (" + fullPath + ")");
         }
 
         // do super stuff

dspace-xmlui/src/main/java/org/dspace/app/xmlui/cocoon/SafeResourceReader.java

@@ -0,0 +1,71 @@
+package org.dspace.app.xmlui.cocoon;
+
+import java.io.IOException;
+import java.util.Map;
+
+import org.apache.avalon.framework.configuration.Configurable;
+import org.apache.avalon.framework.parameters.Parameters;
+import org.apache.cocoon.ResourceNotFoundException;
+import org.apache.cocoon.reading.ResourceReader;
+
+import org.apache.cocoon.ProcessingException;
+import org.apache.cocoon.caching.CacheableProcessingComponent;
+import org.apache.cocoon.environment.SourceResolver;
+import org.apache.excalibur.source.Source;
+
+import org.xml.sax.SAXException;
+
+/**
+ * The SafeResourceReader blocks access to specific paths which we do NOT want
+ * to be readable on the web. By default, the Cocoon ResourceReader just loads
+ * whatever local file you request, which is not very secure and exposes XMLUI 
+ * to possible directory traversal vulnerabilities (when encoded dots or slashes
+ * are passed on the URL).
+ */
+public class SafeResourceReader extends ResourceReader
+        implements CacheableProcessingComponent, Configurable {
+
+    /**
+     * Setup the reader.
+     * The resource is opened to get an <code>InputStream</code>,
+     * the length and the last modification date
+     */
+    @Override
+    public void setup(SourceResolver resolver, Map objectModel, String src, Parameters par)
+            throws ProcessingException, SAXException, IOException {
+
+        // If the requested path includes any of the following strings/characters
+        // then block access and return "Resource Not Found"
+        if(src != null && (
+            src.toLowerCase().contains(":") ||
+            // %3a = encoded colon (:)
+            src.toLowerCase().contains("%3a") ||
+            // %252e = double encoded dot (.)
+            src.toLowerCase().contains("%252e") ||
+            // %2e = encoded dot (.)
+            src.toLowerCase().contains("%2e") ||
+            // %2f = encoded slash (/)
+            src.toLowerCase().contains("%2f") ||
+            // block public access to all Cocoon Sitemaps (*.xmap)
+            src.toLowerCase().contains(".xmap") ||
+            // block public access to all Theme XSLs
+            src.toLowerCase().contains(".xsl")
+        ))
+        {
+            // Block access by throwing a ResourceNotFound (triggers a 404)
+            throw new ResourceNotFoundException("Resource not found (" + src + ")");
+        }
+
+        // See if this resource actually exists by attempting to resolve it
+        // If not, throw a ResourceNotFound (triggers a 404)
+        Source resource = resolver.resolveURI(src);
+        if(!resource.exists())
+        {
+            throw new ResourceNotFoundException("Resource not found (" + src + ")");
+        }    
+        
+        // Otherwise, simply load the requested resource via ResourceReader
+        super.setup(resolver, objectModel, src, par);
+    }
+
+}

dspace-xmlui/src/main/java/org/dspace/app/xmlui/cocoon/ThemeResourceReader.java

@@ -0,0 +1,74 @@
+/**
+ * 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.app.xmlui.cocoon;
+
+import java.io.IOException;
+import java.util.Map;
+import org.apache.avalon.framework.configuration.Configurable;
+import org.apache.avalon.framework.parameters.Parameters;
+import org.apache.cocoon.ProcessingException;
+import org.apache.cocoon.ResourceNotFoundException;
+import org.apache.cocoon.caching.CacheableProcessingComponent;
+import org.apache.cocoon.environment.SourceResolver;
+import org.apache.commons.lang.StringUtils;
+import org.dspace.core.ConfigurationManager;
+import org.xml.sax.SAXException;
+
+/**
+ * An XMLUI Theme Resource Reader, which ONLY allows for certain types of files
+ * to be included in a themes.
+ *
+ * @author Tim Donohue
+ * @author Andrea Schweer
+ */
+public class ThemeResourceReader extends SafeResourceReader
+        implements CacheableProcessingComponent, Configurable
+{
+    // Default whitelist of file extensions that are allowed in an XMLUI theme
+    protected String[] DEFAULT_WHITELIST = new String[]{"css", "js", "json", "gif", "jpg", "jpeg", "png", "ico", "bmp", "htm", "html", "svg", "ttf", "woff"};
+
+    @Override
+    public void setup(SourceResolver resolver, Map objectModel, String src, Parameters par)
+            throws ProcessingException, SAXException, IOException
+    {
+        // If our XMLUI sitemap has specified to bypass the whitelist for this resource,
+        // then just let this through to the SafeResourceReader
+        if (par.getParameterAsBoolean("bypass-whitelist", false))
+        {
+             super.setup(resolver, objectModel, src, par);
+             return;
+        }
+
+        // Otherwise, we'll load our configured file-extension whitelist
+        String whitelistProp = ConfigurationManager.getProperty("xmlui.theme.whitelist");
+        String[] whitelist;
+
+        if(StringUtils.isEmpty(whitelistProp))
+        {
+            whitelist = DEFAULT_WHITELIST;
+        }
+        else
+        {
+            whitelist = whitelistProp.split(",");
+        }
+
+        // Check resource suffix against our whitelist
+        for(String suffix : whitelist)
+        {
+            // If it is in our whitelist, let it through to the SafeResourceReader
+            if(src != null && src.toLowerCase().endsWith("." + suffix.trim()))
+            {
+                super.setup(resolver, objectModel, src, par);
+                return;
+            }
+        }
+
+        // Finally, if the resource has a suffix that is NOT in our whitelist, block it
+        throw new ResourceNotFoundException("Resource not found (" + src + ")");
+    }
+}

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

@@ -2900,10 +2900,10 @@ function doCreateCommunity(parentCommunityID)
 /**
  * Edit a community.
  */
-function doEditCommunity(itemID)
+function doEditCommunity(communityID)
 {
 	// Always go to the status page first
-	doEditCommunityMetadata(itemID);
+	doEditCommunityMetadata(communityID);
 
 	do {
 	    if (cocoon.request.get("submit_return"))
@@ -2911,29 +2911,29 @@ function doEditCommunity(itemID)
 			return null;
 		}
                 else if (cocoon.request.get("submit_metadata")) {
-                    doEditCommunityMetadata(itemID);
+                    doEditCommunityMetadata(communityID);
                 }
 		else if (cocoon.request.get("submit_status"))
 		{
-			doEditItemStatus(itemID);
+			doEditItemStatus(communityID);
 		}
 		else if (cocoon.request.get("submit_bitstreams"))
 		{
-			doEditItemBitstreams(itemID);
+			doEditItemBitstreams(communityID);
 		}
 		else if (cocoon.request.get("submit_save") || cocoon.request.get("submit_delete") || cocoon.request.get("submit_delete_logo"))
 		{
-			doEditCommunityMetadata(itemID, -1);
+			doEditCommunityMetadata(communityID, -1);
 		}
                 else if (cocoon.request.get("submit_authorizations")) {
 			result = doAuthorizeCommunity(communityID);
 		}
 		else if (cocoon.request.get("submit_roles"))
 		{
-			doAssignCommunityRoles(itemID);
+			doAssignCommunityRoles(communityID);
 		}
 	        else if (cocoon.request.get("submit_curate"))  {
-                        doCurateCommunity(itemID, cocoon.request.get("curate_task"));
+                        doCurateCommunity(communityID, cocoon.request.get("curate_task"));
                 }
                 else
 		{