Merge branch 'master' of https://github.com/DSpace/DSpace into DS-3851_workflow

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

@@ -33,7 +33,6 @@ import org.jdom.JDOMException;
 import org.jdom.Namespace;
 import org.jdom.output.DOMOutputter;
 import org.jdom.output.XMLOutputter;
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.w3c.dom.Document;
 
@@ -53,20 +52,14 @@ import org.w3c.dom.Document;
  *
  * @author Richard Rodgers
  */
-public class OpenSearchServiceImpl implements OpenSearchService, InitializingBean {
+public class OpenSearchServiceImpl implements OpenSearchService {
     private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(OpenSearchServiceImpl.class);
 
-    // are open search queries enabled?
-    protected boolean enabled = false;
-    // supported results formats
-    protected List<String> formats = null;
     // Namespaces used
     protected final String osNs = "http://a9.com/-/spec/opensearch/1.1/";
 
-    // base search UI URL
+    @Autowired(required = true)
-    protected String uiUrl = null;
+    protected ConfigurationService configurationService;
-    // base search service URL
-    protected String svcUrl = null;
 
     @Autowired(required = true)
     protected HandleService handleService;
@@ -75,30 +68,35 @@ public class OpenSearchServiceImpl implements OpenSearchService, InitializingBea
     }
 
     @Override
-    public void afterPropertiesSet() throws Exception {
+    public List<String> getFormats() {
-        ConfigurationService config = DSpaceServicesFactory.getInstance().getConfigurationService();
+        List<String> formats = new ArrayList<>();
-        enabled = config.getBooleanProperty("websvc.opensearch.enable");
+        // read formats only if enabled
-        svcUrl = config.getProperty("dspace.url") + "/" +
+        if (isEnabled()) {
-            config.getProperty("websvc.opensearch.svccontext");
+            String[] fmts = configurationService.getArrayProperty("websvc.opensearch.formats");
-        uiUrl = config.getProperty("dspace.url") + "/" +
-            config.getProperty("websvc.opensearch.uicontext");
-
-        // read rest of config info if enabled
-        formats = new ArrayList<String>();
-        if (enabled) {
-            String[] fmts = config.getArrayProperty("websvc.opensearch.formats");
             formats = Arrays.asList(fmts);
         }
-    }
-
-    @Override
-    public List<String> getFormats() {
         return formats;
     }
 
     @Override
     public boolean isEnabled() {
-        return enabled;
+        return configurationService.getBooleanProperty("websvc.opensearch.enable");
+    }
+
+    /**
+     * Get base search service URL (websvc.opensearch.svccontext)
+     */
+    protected String getBaseSearchServiceURL() {
+        return configurationService.getProperty("dspace.url") + "/" +
+            configurationService.getProperty("websvc.opensearch.svccontext");
+    }
+
+    /**
+     * Get base search UI URL (websvc.opensearch.uicontext)
+     */
+    protected String getBaseSearchUIURL() {
+        return configurationService.getProperty("dspace.url") + "/" +
+            configurationService.getProperty("websvc.opensearch.uicontext");
     }
 
     @Override
@@ -226,13 +224,13 @@ public class OpenSearchServiceImpl implements OpenSearchService, InitializingBea
             root.addContent(fav);
         }
         // service URLs
-        for (String format : formats) {
+        for (String format : getFormats()) {
             Element url = new Element("Url", ns).setAttribute("type", getContentType(format));
             StringBuilder template = new StringBuilder();
             if ("html".equals(format)) {
-                template.append(uiUrl);
+                template.append(getBaseSearchUIURL());
             } else {
-                template.append(svcUrl);
+                template.append(getBaseSearchServiceURL());
             }
             template.append("?query={searchTerms}");
             if (!"html".equals(format)) {

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

@@ -160,7 +160,7 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
             try {
                 server = new HttpSolrServer(configurationService.getProperty("solr-statistics.server"));
             } catch (Exception e) {
-                log.error(e.getMessage(), e);
+                log.error("Error accessing Solr server configured in 'solr-statistics.server'", e);
             }
         }
         solr = server;
@@ -231,7 +231,8 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
         } catch (RuntimeException re) {
             throw re;
         } catch (Exception e) {
-            log.error(e.getMessage(), e);
+            log.error("Error saving VIEW event to Solr for DSpaceObject {} by EPerson {}",
+                      dspaceObject.getID(), currentUser.getEmail(), e);
         }
     }
 
@@ -266,7 +267,8 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
         } catch (RuntimeException re) {
             throw re;
         } catch (Exception e) {
-            log.error(e.getMessage(), e);
+            log.error("Error saving VIEW event to Solr for DSpaceObject {} by EPerson {}",
+                      dspaceObject.getID(), currentUser.getEmail(), e);
         }
     }
 
@@ -338,7 +340,7 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
                         doc1.addField("longitude", longitude);
                     }
                 } catch (IOException | GeoIp2Exception e) {
-                    log.error("Unable to get location of request:  {}", e.getMessage());
+                    log.error("Unable to get location of request: {}", e.getMessage());
                 }
             }
         }
@@ -473,7 +475,8 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
         } catch (RuntimeException re) {
             throw re;
         } catch (Exception e) {
-            log.error(e.getMessage(), e);
+            log.error("Error saving SEARCH event to Solr by EPerson {}",
+                      currentUser.getEmail(), e);
         }
     }
 
@@ -520,7 +523,7 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
             solr.add(solrDoc);
         } catch (Exception e) {
             //Log the exception, no need to send it through, the workflow shouldn't crash because of this !
-            log.error(e.getMessage(), e);
+            log.error("Error saving WORKFLOW event to Solr", e);
         }
 
     }
@@ -1033,7 +1036,7 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
             // solr.set
             response = solr.query(solrQuery);
         } catch (SolrServerException e) {
-            System.err.println("Error using query " + query);
+            log.error("Error searching Solr usage events using query {}", query, e);
             throw e;
         }
         return response;

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

@@ -69,7 +69,7 @@ public class SolrLoggerUsageEventListener extends AbstractUsageEventListener {
                 }
 
             } catch (Exception e) {
-                log.error(e.getMessage());
+                log.error("Error processing/logging UsageEvent {}", event.getName(), e);
             }
         }
 

dspace-api/src/test/data/dspaceFolder/config/local.cfg

@@ -105,7 +105,3 @@ plugin.sequence.java.util.Collection = \
     java.util.LinkedList, \
     java.util.Stack, \
     java.util.TreeSet
-
-#### OpenSearch Settings ####
-# enable open search
-websvc.opensearch.enable = true

dspace-spring-rest/src/main/java/org/dspace/app/rest/Application.java

@@ -7,22 +7,15 @@
  */
 package org.dspace.app.rest;
 
-import java.io.File;
 import java.util.List;
-import javax.naming.Context;
-import javax.naming.InitialContext;
 import javax.servlet.Filter;
 
 import org.dspace.app.rest.filter.DSpaceRequestContextFilter;
 import org.dspace.app.rest.model.hateoas.DSpaceRelProvider;
 import org.dspace.app.rest.parameter.resolver.SearchFilterResolver;
 import org.dspace.app.rest.utils.ApplicationConfig;
+import org.dspace.app.rest.utils.DSpaceKernelInitializer;
 import org.dspace.app.util.DSpaceContextListener;
-import org.dspace.kernel.DSpaceKernel;
-import org.dspace.kernel.DSpaceKernelManager;
-import org.dspace.servicemanager.DSpaceKernelImpl;
-import org.dspace.servicemanager.DSpaceKernelInit;
-import org.dspace.servicemanager.config.DSpaceConfigurationService;
 import org.dspace.utils.servlet.DSpaceWebappServletFilter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,11 +24,7 @@ import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.boot.web.servlet.ServletContextInitializer;
 import org.springframework.boot.web.support.SpringBootServletInitializer;
-import org.springframework.context.ApplicationContextInitializer;
-import org.springframework.context.ApplicationListener;
-import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.event.ContextClosedEvent;
 import org.springframework.core.annotation.Order;
 import org.springframework.hateoas.RelProvider;
 import org.springframework.web.context.request.RequestContextListener;
@@ -159,96 +148,4 @@ public class Application extends SpringBootServletInitializer {
             }
         };
     }
-
+}
-    /**
-     * Utility class that will destroy the DSpace Kernel on Spring Boot shutdown
-     */
-    private class DSpaceKernelDestroyer implements ApplicationListener<ContextClosedEvent> {
-        private DSpaceKernel kernel;
-
-        public DSpaceKernelDestroyer(DSpaceKernel kernel) {
-            this.kernel = kernel;
-        }
-
-        public void onApplicationEvent(final ContextClosedEvent event) {
-            if (this.kernel != null) {
-                this.kernel.destroy();
-                this.kernel = null;
-            }
-        }
-    }
-
-    /**
-     * Utility class that will initialize the DSpace Kernel on Spring Boot startup
-     */
-    private class DSpaceKernelInitializer implements ApplicationContextInitializer<ConfigurableApplicationContext> {
-
-        private transient DSpaceKernel dspaceKernel;
-
-        public void initialize(final ConfigurableApplicationContext applicationContext) {
-
-            String dspaceHome = applicationContext.getEnvironment().getProperty("dspace.dir");
-
-            this.dspaceKernel = DSpaceKernelManager.getDefaultKernel();
-            if (this.dspaceKernel == null) {
-                DSpaceKernelImpl kernelImpl = null;
-                try {
-                    kernelImpl = DSpaceKernelInit.getKernel(null);
-                    if (!kernelImpl.isRunning()) {
-                        kernelImpl.start(getProvidedHome(dspaceHome)); // init the kernel
-                    }
-                    this.dspaceKernel = kernelImpl;
-
-                } catch (Exception e) {
-                    // failed to start so destroy it and log and throw an exception
-                    try {
-                        if (kernelImpl != null) {
-                            kernelImpl.destroy();
-                        }
-                        this.dspaceKernel = null;
-                    } catch (Exception e1) {
-                        // nothing
-                    }
-                    String message = "Failure during ServletContext initialisation: " + e.getMessage();
-                    log.error(message, e);
-                    throw new RuntimeException(message, e);
-                }
-            }
-
-            if (applicationContext.getParent() == null) {
-                //Set the DSpace Kernel Application context as a parent of the Spring Boot context so that
-                //we can auto-wire all DSpace Kernel services
-                applicationContext.setParent(dspaceKernel.getServiceManager().getApplicationContext());
-
-                //Add a listener for Spring Boot application shutdown so that we can nicely cleanup the DSpace kernel.
-                applicationContext.addApplicationListener(new DSpaceKernelDestroyer(dspaceKernel));
-            }
-        }
-
-        /**
-         * Find DSpace's "home" directory.
-         * Initially look for JNDI Resource called "java:/comp/env/dspace.dir".
-         * If not found, look for "dspace.dir" initial context parameter.
-         */
-        private String getProvidedHome(String dspaceHome) {
-            String providedHome = null;
-            try {
-                Context ctx = new InitialContext();
-                providedHome = (String) ctx.lookup("java:/comp/env/" + DSpaceConfigurationService.DSPACE_HOME);
-            } catch (Exception e) {
-                // do nothing
-            }
-
-            if (providedHome == null) {
-                if (dspaceHome != null && !dspaceHome.equals("") &&
-                    !dspaceHome.equals("${" + DSpaceConfigurationService.DSPACE_HOME + "}")) {
-                    File test = new File(dspaceHome);
-                    if (test.exists() && new File(test, DSpaceConfigurationService.DSPACE_CONFIG_PATH).exists()) {
-                        providedHome = dspaceHome;
-                    }
-                }
-            }
-            return providedHome;
-        }
-    }
-}

dspace-spring-rest/src/main/java/org/dspace/app/rest/utils/DSpaceKernelInitializer.java

@@ -0,0 +1,122 @@
+/**
+ * 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.rest.utils;
+
+import java.io.File;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+import org.dspace.kernel.DSpaceKernel;
+import org.dspace.kernel.DSpaceKernelManager;
+import org.dspace.servicemanager.DSpaceKernelImpl;
+import org.dspace.servicemanager.DSpaceKernelInit;
+import org.dspace.servicemanager.config.DSpaceConfigurationService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.context.ApplicationContextInitializer;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.event.ContextClosedEvent;
+
+/**
+ * Utility class that will initialize the DSpace Kernel on Spring Boot startup.
+ * Used by org.dspace.app.rest.Application
+ */
+public class DSpaceKernelInitializer implements ApplicationContextInitializer<ConfigurableApplicationContext> {
+
+    private static final Logger log = LoggerFactory.getLogger(DSpaceKernelInitializer.class);
+
+    private transient DSpaceKernel dspaceKernel;
+
+    @Override
+    public void initialize(final ConfigurableApplicationContext applicationContext) {
+
+        String dspaceHome = applicationContext.getEnvironment().getProperty("dspace.dir");
+
+        this.dspaceKernel = DSpaceKernelManager.getDefaultKernel();
+        if (this.dspaceKernel == null) {
+            DSpaceKernelImpl kernelImpl = null;
+            try {
+                kernelImpl = DSpaceKernelInit.getKernel(null);
+                if (!kernelImpl.isRunning()) {
+                    kernelImpl.start(getProvidedHome(dspaceHome)); // init the kernel
+                }
+                this.dspaceKernel = kernelImpl;
+
+            } catch (Exception e) {
+                // failed to start so destroy it and log and throw an exception
+                try {
+                    if (kernelImpl != null) {
+                        kernelImpl.destroy();
+                    }
+                    this.dspaceKernel = null;
+                } catch (Exception e1) {
+                    // nothing
+                }
+                String message = "Failure during ServletContext initialisation: " + e.getMessage();
+                log.error(message, e);
+                throw new RuntimeException(message, e);
+            }
+        }
+
+        if (applicationContext.getParent() == null) {
+            //Set the DSpace Kernel Application context as a parent of the Spring Boot context so that
+            //we can auto-wire all DSpace Kernel services
+            applicationContext.setParent(dspaceKernel.getServiceManager().getApplicationContext());
+
+            //Add a listener for Spring Boot application shutdown so that we can nicely cleanup the DSpace kernel.
+            applicationContext.addApplicationListener(new DSpaceKernelDestroyer(dspaceKernel));
+        }
+    }
+
+    /**
+     * Find DSpace's "home" directory.
+     * Initially look for JNDI Resource called "java:/comp/env/dspace.dir".
+     * If not found, look for "dspace.dir" initial context parameter.
+     */
+    private String getProvidedHome(String dspaceHome) {
+        String providedHome = null;
+        try {
+            Context ctx = new InitialContext();
+            providedHome = (String) ctx.lookup("java:/comp/env/" + DSpaceConfigurationService.DSPACE_HOME);
+        } catch (Exception e) {
+            // do nothing
+        }
+
+        if (providedHome == null) {
+            if (dspaceHome != null && !dspaceHome.equals("") &&
+                !dspaceHome.equals("${" + DSpaceConfigurationService.DSPACE_HOME + "}")) {
+                File test = new File(dspaceHome);
+                if (test.exists() && new File(test, DSpaceConfigurationService.DSPACE_CONFIG_PATH).exists()) {
+                    providedHome = dspaceHome;
+                }
+            }
+        }
+        return providedHome;
+    }
+
+
+    /**
+     * Utility class that will destroy the DSpace Kernel on Spring Boot shutdown
+     */
+    private class DSpaceKernelDestroyer implements ApplicationListener<ContextClosedEvent> {
+        private DSpaceKernel kernel;
+
+        public DSpaceKernelDestroyer(DSpaceKernel kernel) {
+            this.kernel = kernel;
+        }
+
+        public void onApplicationEvent(final ContextClosedEvent event) {
+            if (this.kernel != null) {
+                this.kernel.destroy();
+                this.kernel = null;
+            }
+        }
+    }
+}
+

dspace-spring-rest/src/test/java/org/dspace/app/opensearch/OpenSearchControllerDisabledIT.java

@@ -24,7 +24,7 @@ import org.junit.Test;
  *
  * @author Oliver Goldschmidt (o dot goldschmidt at tuhh dot de)
  */
-public class OpenSearchControllerDisabledTest extends AbstractControllerIntegrationTest {
+public class OpenSearchControllerDisabledIT extends AbstractControllerIntegrationTest {
 
     private ConfigurationService configurationService;
 
@@ -58,4 +58,4 @@ public class OpenSearchControllerDisabledTest extends AbstractControllerIntegrat
                    .andExpect(content().string("OpenSearch Service is disabled"))
         ;
     }
-}
+}

dspace-spring-rest/src/test/java/org/dspace/app/opensearch/OpenSearchControllerIT.java

@@ -22,6 +22,9 @@ import org.dspace.content.Collection;
 import org.dspace.content.Community;
 import org.dspace.content.Item;
 
+import org.dspace.services.ConfigurationService;
+import org.dspace.services.factory.DSpaceServicesFactory;
+import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -31,7 +34,16 @@ import org.junit.Test;
  *
  * @author Oliver Goldschmidt (o dot goldschmidt at tuhh dot de)
  */
-public class OpenSearchControllerTest extends AbstractControllerIntegrationTest {
+public class OpenSearchControllerIT extends AbstractControllerIntegrationTest {
+
+    private ConfigurationService configurationService;
+
+    @Before
+    public void init() throws Exception {
+        //enable OpenSearch by configuration
+        configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
+        configurationService.setProperty("websvc.opensearch.enable", true);
+    }
 
     @Test
     public void searchAtomTest() throws Exception {
@@ -221,4 +233,4 @@ public class OpenSearchControllerTest extends AbstractControllerIntegrationTest
             </OpenSearchDescription>
         */
     }
-}
+}

dspace-spring-rest/src/test/java/org/dspace/app/rest/BitstreamContentRestControllerIT.java

@@ -29,11 +29,11 @@ import java.util.UUID;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.CharEncoding;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.pdfbox.pdmodel.PDDocument;
 import org.apache.pdfbox.text.PDFTextStripper;
-import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.response.QueryResponse;
 import org.dspace.app.rest.builder.BitstreamBuilder;
 import org.dspace.app.rest.builder.CollectionBuilder;
 import org.dspace.app.rest.builder.CommunityBuilder;
@@ -43,12 +43,16 @@ import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
 import org.dspace.content.Bitstream;
 import org.dspace.content.Collection;
 import org.dspace.content.Item;
+import org.dspace.core.Constants;
 import org.dspace.disseminate.CitationDocumentServiceImpl;
 import org.dspace.eperson.Group;
 import org.dspace.services.ConfigurationService;
-import org.dspace.solr.MockSolrServer;
+import org.dspace.statistics.ObjectCount;
-import org.junit.After;
+import org.dspace.statistics.SolrLoggerServiceImpl;
+import org.dspace.statistics.factory.StatisticsServiceFactory;
+import org.dspace.statistics.service.SolrLoggerService;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -60,7 +64,10 @@ import org.springframework.beans.factory.annotation.Autowired;
  */
 public class BitstreamContentRestControllerIT extends AbstractControllerIntegrationTest {
 
-    private MockSolrServer mockSolrServer;
+    protected SolrLoggerService solrLoggerService = StatisticsServiceFactory.getInstance().getSolrLoggerService();
+
+    private static final Logger log = LogManager
+        .getLogger(BitstreamContentRestControllerIT.class);
 
     @Autowired
     private ConfigurationService configurationService;
@@ -68,19 +75,18 @@ public class BitstreamContentRestControllerIT extends AbstractControllerIntegrat
     @Autowired
     private CitationDocumentServiceImpl citationDocumentService;
 
+    @BeforeClass
+    public static void clearStatistics() throws Exception {
+        // To ensure these tests start "fresh", clear out any existing statistics data.
+        // NOTE: this is committed immediately in removeIndex()
+        StatisticsServiceFactory.getInstance().getSolrLoggerService().removeIndex("*:*");
+    }
+
     @Before
     public void setup() throws Exception {
         super.setUp();
-        mockSolrServer = new MockSolrServer("statistics");
-        mockSolrServer.getSolrServer().deleteByQuery("*:*");
-        mockSolrServer.getSolrServer().commit();
-        configurationService.setProperty("citation-page.enable_globally", false);
-    }
 
-    @After
+        configurationService.setProperty("citation-page.enable_globally", false);
-    public void destroy() throws Exception {
-        super.destroy();
-        mockSolrServer.destroy();
     }
 
     @Test
@@ -314,15 +320,18 @@ public class BitstreamContentRestControllerIT extends AbstractControllerIntegrat
         }
     }
 
+    // Verify number of hits/views of Bitstream is as expected
     private void checkNumberOfStatsRecords(Bitstream bitstream, int expectedNumberOfStatsRecords)
         throws SolrServerException, IOException {
-        mockSolrServer.getSolrServer().commit();
+        // Use the SolrLoggerServiceImpl.ResultProcessor inner class to force a Solr commit to occur.
+        // This is required because statistics hits will not be committed to Solr until autoCommit next runs.
+        SolrLoggerServiceImpl.ResultProcessor rs = ((SolrLoggerServiceImpl) solrLoggerService).new ResultProcessor();
+        rs.commit();
 
-        SolrQuery query = new SolrQuery("id:\"" + bitstream.getID() + "\"")
+        // Find all hits/views of bitstream
-            .setRows(0)
+        ObjectCount objectCount = solrLoggerService.queryTotal("type:" + Constants.BITSTREAM +
-            .setStart(0);
+                                                               " AND id:" + bitstream.getID(), null);
-        QueryResponse queryResponse = mockSolrServer.getSolrServer().query(query);
+        assertEquals(expectedNumberOfStatsRecords, objectCount.getCount());
-        assertEquals(expectedNumberOfStatsRecords, queryResponse.getResults().getNumFound());
     }
 
     @Test

dspace-spring-rest/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java

@@ -115,27 +115,15 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
 
     @Test
     public void findAllUnauthorizedTest() throws Exception {
-        context.turnOffAuthorisationSystem();
+        // Access endpoint without being authenticated
-
-        EPerson newUser = EPersonBuilder.createEPerson(context)
-                                        .withNameInMetadata("John", "Doe")
-                                        .withEmail("Johndoe@fake-email.com")
-                                        .build();
-
         getClient().perform(get("/api/eperson/eperson"))
                    .andExpect(status().isUnauthorized());
     }
 
     @Test
     public void findAllForbiddenTest() throws Exception {
-        context.turnOffAuthorisationSystem();
-
-        EPerson newUser = EPersonBuilder.createEPerson(context)
-                                        .withNameInMetadata("John", "Doe")
-                                        .withEmail("Johndoe@fake-email.com")
-                                        .build();
-
         String authToken = getAuthToken(eperson.getEmail(), password);
+        // Access endpoint logged in as an unprivileged user
         getClient(authToken).perform(get("/api/eperson/eperson"))
                             .andExpect(status().isForbidden());
     }
@@ -150,32 +138,33 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
                                         .build();
 
         String authToken = getAuthToken(admin.getEmail(), password);
-        // using size = 2 the first page will contains our test user and admin
+        // NOTE: /eperson/epersons endpoint returns users sorted by email
+        // using size = 2 the first page will contain our new test user and default 'admin' ONLY
         getClient(authToken).perform(get("/api/eperson/epersons")
                                 .param("size", "2"))
                    .andExpect(status().isOk())
                    .andExpect(content().contentType(contentType))
                    .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder(
-                           EPersonMatcher.matchEPersonEntry(admin),
+                           EPersonMatcher.matchEPersonEntry(testEPerson),
-                           EPersonMatcher.matchEPersonEntry(testEPerson)
+                           EPersonMatcher.matchEPersonOnEmail(admin.getEmail())
                    )))
                    .andExpect(jsonPath("$._embedded.epersons", Matchers.not(
                        Matchers.contains(
-                           EPersonMatcher.matchEPersonEntry(admin)
+                           EPersonMatcher.matchEPersonOnEmail(eperson.getEmail())
                        )
                    )))
                    .andExpect(jsonPath("$.page.size", is(2)))
                    .andExpect(jsonPath("$.page.totalElements", is(3)))
         ;
 
-        // using size = 2 the first page will contains our test user and admin
+        // using size = 2 the *second* page will contains our default 'eperson' ONLY
         getClient(authToken).perform(get("/api/eperson/epersons")
                                 .param("size", "2")
                                 .param("page", "1"))
                    .andExpect(status().isOk())
                    .andExpect(content().contentType(contentType))
                    .andExpect(jsonPath("$._embedded.epersons", Matchers.contains(
-                       EPersonMatcher.matchEPersonEntry(eperson)
+                       EPersonMatcher.matchEPersonOnEmail(eperson.getEmail())
                    )))
                    .andExpect(jsonPath("$._embedded.epersons", Matchers.hasSize(1)))
                    .andExpect(jsonPath("$.page.size", is(2)))
@@ -221,7 +210,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
     public void readEpersonAuthorizationTest() throws Exception {
         context.turnOffAuthorisationSystem();
 
-        EPerson ePerson = EPersonBuilder.createEPerson(context)
+        EPerson ePerson1 = EPersonBuilder.createEPerson(context)
                                         .withNameInMetadata("John", "Doe")
                                         .withEmail("Johndoe@fake-email.com")
                                         .build();
@@ -231,6 +220,8 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
                                          .withEmail("janesmith@fake-email.com")
                                          .build();
 
+
+        // Verify admin can access information about any user (and only one user is included in response)
         String authToken = getAuthToken(admin.getEmail(), password);
         getClient(authToken).perform(get("/api/eperson/epersons/" + ePerson2.getID()))
                    .andExpect(status().isOk())
@@ -240,20 +231,19 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
                    )))
                    .andExpect(jsonPath("$", Matchers.not(
                        is(
-                           EPersonMatcher.matchEPersonEntry(eperson)
+                           EPersonMatcher.matchEPersonEntry(ePerson1)
                        )
                    )))
                    .andExpect(jsonPath("$._links.self.href",
                                        Matchers.containsString("/api/eperson/epersons/" + ePerson2.getID())));
 
 
-        //EPerson can only access himself
+        // Verify an unprivileged user cannot access information about a *different* user
         String epersonToken = getAuthToken(eperson.getEmail(), password);
-
         getClient(epersonToken).perform(get("/api/eperson/epersons/" + ePerson2.getID()))
                                .andExpect(status().isForbidden());
 
-
+        // Verify an unprivilegd user can access information about himself/herself
         getClient(epersonToken).perform(get("/api/eperson/epersons/" + eperson.getID()))
                 .andExpect(status().isOk())
                 .andExpect(content().contentType(contentType))

dspace-spring-rest/src/test/java/org/dspace/app/rest/test/AbstractControllerIntegrationTest.java

@@ -22,9 +22,7 @@ import org.apache.commons.io.Charsets;
 import org.apache.commons.lang3.StringUtils;
 import org.dspace.app.rest.Application;
 import org.dspace.app.rest.model.patch.Operation;
-import org.dspace.app.rest.security.MethodSecurityConfig;
+import org.dspace.app.rest.utils.DSpaceKernelInitializer;
-import org.dspace.app.rest.security.WebSecurityConfiguration;
-import org.dspace.app.rest.utils.ApplicationConfig;
 import org.junit.Assert;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -35,12 +33,8 @@ import org.springframework.http.MediaType;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
 import org.springframework.mock.web.MockHttpServletResponse;
-import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.ContextConfiguration;
-import org.springframework.test.context.TestExecutionListeners;
+import org.springframework.test.context.junit4.SpringRunner;
-import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
-import org.springframework.test.context.support.DependencyInjectionTestExecutionListener;
-import org.springframework.test.context.support.DirtiesContextTestExecutionListener;
-import org.springframework.test.context.transaction.TransactionalTestExecutionListener;
 import org.springframework.test.context.web.WebAppConfiguration;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.request.RequestPostProcessor;
@@ -50,16 +44,20 @@ import org.springframework.web.context.WebApplicationContext;
 
 /**
  * Abstract controller integration test class that will take care of setting up the
- * environment to run the integration test
+ * Spring Boot environment to run the integration test
  *
- * @author Tom Desair (tom dot desair at atmire dot com)
+ * @author Tom Desair
+ * @author Tim Donohue
  */
-@RunWith(SpringJUnit4ClassRunner.class)
+// Run tests with JUnit 4 and Spring TestContext Framework
-@SpringBootTest(classes = {Application.class, ApplicationConfig.class, WebSecurityConfiguration.class,
+@RunWith(SpringRunner.class)
-        MethodSecurityConfig.class})
+// Specify main class to use to load Spring ApplicationContext
-@TestExecutionListeners( {DependencyInjectionTestExecutionListener.class, DirtiesContextTestExecutionListener.class,
+// NOTE: By default, Spring caches and reuses ApplicationContext for each integration test (to speed up tests)
-    TransactionalTestExecutionListener.class})
+// See: https://docs.spring.io/spring/docs/current/spring-framework-reference/testing.html#integration-testing
-@DirtiesContext
+@SpringBootTest(classes = Application.class)
+// Load DSpaceKernelInitializer in Spring ApplicationContext (to initialize DSpace Kernel)
+@ContextConfiguration(initializers = DSpaceKernelInitializer.class)
+// Tell Spring to make ApplicationContext an instance of WebApplicationContext (for web-based tests)
 @WebAppConfiguration
 public class AbstractControllerIntegrationTest extends AbstractIntegrationTestWithDatabase {
 

dspace-spring-rest/src/test/java/org/dspace/app/rest/test/AbstractDSpaceIntegrationTest.java

@@ -54,11 +54,10 @@ public class AbstractDSpaceIntegrationTest {
      * This method will be run before the first test as per @BeforeClass. It will
      * initialize shared resources required for all tests of this class.
      *
-     * This method loads our test properties to initialize our test environment,
+     * This method loads our test properties for usage in test environment.
-     * and then starts the DSpace Kernel (which allows access to services).
      */
     @BeforeClass
-    public static void initKernel() {
+    public static void initTestEnvironment() {
         try {
             //Stops System.exit(0) throws exception instead of exitting
             System.setSecurityManager(new NoExitSecurityManager());
@@ -72,12 +71,17 @@ public class AbstractDSpaceIntegrationTest {
                                                                 .getResource("test-config.properties");
             testProps.load(properties.openStream());
 
-            // Initialise the service manager kernel
+            // Get a reference to current Kernel
             kernelImpl = DSpaceKernelInit.getKernel(null);
+            // If somehow the kernel is NOT initialized, initialize it.
+            // NOTE: This is likely never going to occur, as Spring Boot initializes it
+            // See AbstractControllerIntegrationTest (where @SpringBootTest is defined)
             if (!kernelImpl.isRunning()) {
                 // NOTE: the "dspace.dir" system property MUST be specified via Maven
                 kernelImpl.start(getDspaceDir()); // init the kernel
             }
+
+            // Initialize our builder (by loading all DSpace services)
             AbstractBuilder.init();
         } catch (IOException ex) {
             log.error("Error initializing tests", ex);
@@ -90,20 +94,20 @@ public class AbstractDSpaceIntegrationTest {
      * will clean resources initialized by the @BeforeClass methods.
      */
     @AfterClass
-    public static void destroyKernel() throws SQLException {
+    public static void destroyTestEnvironment() throws SQLException {
         System.setSecurityManager(null);
 
-        //we clear the properties
+        // Clear our test properties
         testProps.clear();
         testProps = null;
 
+        // Unload DSpace services
         AbstractBuilder.destroy();
 
-        //Also clear out the kernel & nullify (so JUnit will clean it up)
+        // NOTE: We explicitly do NOT stop/destroy the kernel, as it is cached
-        if (kernelImpl != null) {
+        // in the Spring ApplicationContext. By default, to speed up tests,
-            kernelImpl.destroy();
+        // Spring caches & reuses its ApplicationContext for all tests. So,
-        }
+        // similarly, our kernel is being cached & reused for all tests.
-        kernelImpl = null;
     }
 
     public static String getDspaceDir() {

dspace-spring-rest/src/test/java/org/dspace/app/rest/test/AbstractIntegrationTestWithDatabase.java

@@ -170,21 +170,11 @@ public class AbstractIntegrationTestWithDatabase extends AbstractDSpaceIntegrati
         // Cleanup our global context object
         try {
             AbstractBuilder.cleanupObjects();
-            if (context == null || !context.isValid()) {
-                context = new Context();
-            }
-            eperson = context.reloadEntity(eperson);
-            admin = context.reloadEntity(admin);
-
-            context.turnOffAuthorisationSystem();
-            if (eperson != null) {
-                EPersonServiceFactory.getInstance().getEPersonService().delete(context, eperson);
-            }
-            if (admin != null) {
-                EPersonServiceFactory.getInstance().getEPersonService().delete(context, admin);
-            }
             parentCommunity = null;
             cleanupContext();
+
+            // NOTE: we explicitly do NOT destroy our default eperson & admin as they
+            // are cached and reused for all tests. This speeds up all tests.
         } catch (Exception e) {
             throw new RuntimeException(e);
         }

dspace-spring-rest/src/test/java/org/dspace/app/rest/test/DSpaceKernelContextCustomizerFactory.java

@@ -1,32 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.app.rest.test;
-
-import java.util.List;
-
-import org.dspace.kernel.DSpaceKernelManager;
-import org.springframework.test.context.ContextConfigurationAttributes;
-import org.springframework.test.context.ContextCustomizer;
-import org.springframework.test.context.ContextCustomizerFactory;
-
-/**
- * Context customizer factory to set the parent context of our Spring Boot application in TEST mode
- *
- * @author Tom Desair (tom dot desair at atmire dot com)
- */
-public class DSpaceKernelContextCustomizerFactory implements ContextCustomizerFactory {
-
-    @Override
-    public ContextCustomizer createContextCustomizer(Class<?> testClass,
-                                                     List<ContextConfigurationAttributes> configAttributes) {
-        return (context, mergedConfig) -> {
-            context.setParent(DSpaceKernelManager.getDefaultKernel().getServiceManager().getApplicationContext());
-        };
-    }
-
-}

dspace-spring-rest/src/test/java/org/dspace/statistics/MockSolrLoggerServiceImpl.java

@@ -8,21 +8,34 @@
 package org.dspace.statistics;
 
 import java.io.File;
+import java.sql.SQLException;
+import java.util.List;
+
+import javax.servlet.http.HttpServletRequest;
 
 import com.maxmind.geoip2.DatabaseReader;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.dspace.content.DSpaceObject;
+import org.dspace.eperson.EPerson;
 import org.dspace.services.ConfigurationService;
 import org.dspace.solr.MockSolrServer;
+import org.dspace.usage.UsageWorkflowEvent;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
 
 /**
  * Mock service that uses an embedded SOLR server for the statistics core.
  */
+@Service
 public class MockSolrLoggerServiceImpl
         extends SolrLoggerServiceImpl
         implements InitializingBean, DisposableBean {
 
+    private static final Logger log = LogManager.getLogger();
+
     private MockSolrServer mockSolrServer;
 
     @Autowired(required = true)
@@ -33,15 +46,44 @@ public class MockSolrLoggerServiceImpl
 
     @Override
     public void afterPropertiesSet() throws Exception {
+        // Initialize our service with a Mock Solr statistics core
         mockSolrServer = new MockSolrServer("statistics");
         solr = mockSolrServer.getSolrServer();
+    }
 
-        new FakeDatabaseReader(); // Activate fake
+    @Override
-        new FakeDatabaseReader.Builder(); // Activate fake
+    public void postView(DSpaceObject dspaceObject, HttpServletRequest request,
-        String locationDbPath = configurationService.getProperty("usage-statistics.dbfile");
+                         EPerson currentUser) {
-        File locationDb = new File(locationDbPath);
+        // Load our FakeDatabaseReader before each view event is logged
-        locationDb.createNewFile();
+        loadFakeDatabaseReader();
-        locationService = new DatabaseReader.Builder(locationDb).build();
+        // Call overridden method
+        super.postView(dspaceObject, request, currentUser);
+    }
+
+    @Override
+    public void postView(DSpaceObject dspaceObject,
+                         String ip, String userAgent, String xforwardedfor, EPerson currentUser) {
+        // Load our FakeDatabaseReader before each view event is logged
+        loadFakeDatabaseReader();
+        // Call overridden method
+        super.postView(dspaceObject, ip, userAgent, xforwardedfor, currentUser);
+    }
+
+    @Override
+    public void postSearch(DSpaceObject resultObject, HttpServletRequest request, EPerson currentUser,
+                           List<String> queries, int rpp, String sortBy, String order, int page, DSpaceObject scope) {
+        // Load our FakeDatabaseReader before each search event is logged
+        loadFakeDatabaseReader();
+        // Call overridden method
+        super.postSearch(resultObject, request, currentUser, queries, rpp, sortBy, order, page, scope);
+    }
+
+    @Override
+    public void postWorkflow(UsageWorkflowEvent usageWorkflowEvent) throws SQLException {
+        // Load our FakeDatabaseReader before each workflow event is logged
+        loadFakeDatabaseReader();
+        // Call overridden method
+        super.postWorkflow(usageWorkflowEvent);
     }
 
     @Override
@@ -49,4 +91,22 @@ public class MockSolrLoggerServiceImpl
         mockSolrServer.destroy();
     }
 
+    /**
+     * Load / activate our FakeDatabaseReader which uses JMockit to replace specific
+     * methods of the DatabaseReader. This MUST be called for each method that uses
+     * DatabaseReader as JMockit fake classes are only in effect for tests in which
+     * they are defined: http://jmockit.github.io/tutorial/Faking.html
+     */
+    private void loadFakeDatabaseReader() {
+        try {
+            new FakeDatabaseReader(); // Activate fake
+            new FakeDatabaseReader.Builder(); // Activate fake
+            String locationDbPath = configurationService.getProperty("usage-statistics.dbfile");
+            File locationDb = new File(locationDbPath);
+            locationDb.createNewFile();
+            locationService = new DatabaseReader.Builder(locationDb).build();
+        } catch (Exception e) {
+            log.error("Unable to load FakeDatabaseReader", e);
+        }
+    }
 }

dspace-spring-rest/src/test/resources/META-INF/spring.factories

@@ -1,2 +0,0 @@
-org.springframework.test.context.ContextCustomizerFactory= \
-    org.dspace.app.rest.test.DSpaceKernelContextCustomizerFactory

pom.xml

@@ -149,7 +149,7 @@
                         <excludes>
                             <exclude>**/Abstract*</exclude>
                         </excludes>
-                        <!-- Detailed logs in reportsDirectory/testName-output.txt instead of stdout -->
+                        <!-- Detailed logs in surefire-reports/testName-output.txt instead of stdout -->
                         <redirectTestOutputToFile>true</redirectTestOutputToFile>
                         <!-- Ensure full stacktrace is logged (when errors occur) -->
                         <trimStackTrace>false</trimStackTrace>
@@ -170,7 +170,10 @@
                         <excludes>
                             <exclude>**/Abstract*</exclude>
                         </excludes>
+                         <!-- Detailed logs in failsafe-reports/testName-output.txt instead of stdout -->
                         <redirectTestOutputToFile>true</redirectTestOutputToFile>
+                        <!-- Ensure full stacktrace is logged (when errors occur) -->
+                        <trimStackTrace>false</trimStackTrace>
                     </configuration>
                     <executions>
                         <execution>