Removes browse for private and withdrawn items, removes the includeUnDiscoverable boolean from SearchService methods, adds solr plugin to prevent non-admin discovery of private items.

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

@@ -128,7 +128,7 @@ public class ItemCountDAOSolr implements ItemCountDAO {
 
         DiscoverResult sResponse = null;
         try {
-            sResponse = searcher.search(context, query, false);
+            sResponse = searcher.search(context, query);
             List<FacetResult> commCount = sResponse.getFacetResult("location.comm");
             List<FacetResult> collCount = sResponse.getFacetResult("location.coll");
             for (FacetResult c : commCount) {

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

@@ -169,9 +169,6 @@ public class SolrBrowseDAO implements BrowseDAO {
 
     private DiscoverResult sResponse = null;
 
-    private boolean itemsWithdrawn = false;
-    private boolean itemsDiscoverable = true;
-
     private boolean showFrequencies;
 
     private DiscoverResult getSolrResponse() throws BrowseException {
@@ -217,8 +214,7 @@ public class SolrBrowseDAO implements BrowseDAO {
                 }
             }
             try {
-                sResponse = searcher.search(context, query, itemsWithdrawn
+                sResponse = searcher.search(context, query);
-                    || !itemsDiscoverable);
             } catch (SearchServiceException e) {
                 throw new BrowseException(e);
             }
@@ -227,21 +223,14 @@ public class SolrBrowseDAO implements BrowseDAO {
     }
 
     private void addStatusFilter(DiscoverQuery query) {
-        if (itemsWithdrawn) {
+        try {
-            query.addFilterQueries("withdrawn:true");
+            if (!authorizeService.isAdmin(context)
-        } else if (!itemsDiscoverable) {
+                && (authorizeService.isCommunityAdmin(context)
-            query.addFilterQueries("discoverable:false");
+                || authorizeService.isCollectionAdmin(context))) {
-            // TODO
+                query.addFilterQueries(searcher.createLocationQueryForAdministrableItems(context));
-
-            try {
-                if (!authorizeService.isAdmin(context)
-                    && (authorizeService.isCommunityAdmin(context)
-                    || authorizeService.isCollectionAdmin(context))) {
-                    query.addFilterQueries(searcher.createLocationQueryForAdministrableItems(context));
-                }
-            } catch (SQLException ex) {
-                log.error(ex);
             }
+        } catch (SQLException ex) {
+            log.error(ex);
         }
     }
 
@@ -363,10 +352,9 @@ public class SolrBrowseDAO implements BrowseDAO {
             query.setQuery("bi_" + column + "_sort" + ": {\"" + value + "\" TO *]");
             query.addFilterQueries("-(bi_" + column + "_sort" + ":" + value + "*)");
         }
-        boolean includeUnDiscoverable = itemsWithdrawn || !itemsDiscoverable;
         DiscoverResult resp = null;
         try {
-            resp = searcher.search(context, query, includeUnDiscoverable);
+            resp = searcher.search(context, query);
         } catch (SearchServiceException e) {
             throw new BrowseException(e);
         }
@@ -693,11 +681,6 @@ public class SolrBrowseDAO implements BrowseDAO {
      */
     @Override
     public void setTable(String table) {
-        if (table.equals(BrowseIndex.getWithdrawnBrowseIndex().getTableName())) {
-            itemsWithdrawn = true;
-        } else if (table.equals(BrowseIndex.getPrivateBrowseIndex().getTableName())) {
-            itemsDiscoverable = false;
-        }
         facetField = table;
     }
 

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

@@ -51,29 +51,6 @@ public interface SearchService {
     DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery query)
         throws SearchServiceException;
 
-    /**
-     * @param context          DSpace Context object.
-     * @param query            the discovery query object.
-     * @param includeWithdrawn use <code>true</code> to include in the results also withdrawn
-     *                         items that match the query.
-     * @return discovery search result object
-     * @throws SearchServiceException if search error
-     */
-    DiscoverResult search(Context context, DiscoverQuery query,
-                          boolean includeWithdrawn) throws SearchServiceException;
-
-    /**
-     * @param context          DSpace Context object
-     * @param dso              a DSpace Object to use as scope of the search (only results
-     *                         within this object)
-     * @param query            the discovery query object
-     * @param includeWithdrawn use <code>true</code> to include in the results also withdrawn
-     *                         items that match the query
-     * @return discovery search result object
-     * @throws SearchServiceException if search error
-     */
-    DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery query, boolean includeWithdrawn)
-        throws SearchServiceException;
 
     List<IndexableObject> search(Context context, String query, String orderfield, boolean ascending, int offset,
             int max, String... filterquery);

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

@@ -1855,21 +1855,10 @@ public class SolrServiceImpl implements SearchService, IndexingService {
     }
 
     //========== SearchService implementation
-    @Override
-    public DiscoverResult search(Context context, DiscoverQuery query) throws SearchServiceException {
-        return search(context, query, false);
-    }
 
     @Override
-    public DiscoverResult search(Context context, IndexableObject dso,
+    public DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery discoveryQuery)
-                                 DiscoverQuery query)
         throws SearchServiceException {
-        return search(context, dso, query, false);
-    }
-
-    @Override
-    public DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery discoveryQuery,
-                                 boolean includeUnDiscoverable) throws SearchServiceException {
         if (dso != null) {
             if (dso instanceof Community) {
                 discoveryQuery.addFilterQueries("location:m" + dso.getID());
@@ -1879,19 +1868,19 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                 discoveryQuery.addFilterQueries(HANDLE_FIELD + ":" + ((Item) dso).getHandle());
             }
         }
-        return search(context, discoveryQuery, includeUnDiscoverable);
+        return search(context, discoveryQuery);
 
     }
 
 
     @Override
-    public DiscoverResult search(Context context, DiscoverQuery discoveryQuery, boolean includeUnDiscoverable)
+    public DiscoverResult search(Context context, DiscoverQuery discoveryQuery )
         throws SearchServiceException {
         try {
             if (getSolr() == null) {
                 return new DiscoverResult();
             }
-            SolrQuery solrQuery = resolveToSolrQuery(context, discoveryQuery, includeUnDiscoverable);
+            SolrQuery solrQuery = resolveToSolrQuery(context, discoveryQuery);
 
 
             QueryResponse queryResponse = getSolr().query(solrQuery, SolrRequest.METHOD.POST);
@@ -1902,8 +1891,8 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         }
     }
 
-    protected SolrQuery resolveToSolrQuery(Context context, DiscoverQuery discoveryQuery,
+    protected SolrQuery resolveToSolrQuery(Context context, DiscoverQuery discoveryQuery)
-                                           boolean includeUnDiscoverable) throws SearchServiceException {
+        throws SearchServiceException {
         SolrQuery solrQuery = new SolrQuery();
 
         String query = "*:*";
@@ -1929,11 +1918,6 @@ public class SolrServiceImpl implements SearchService, IndexingService {
             solrQuery.setParam("spellcheck", Boolean.TRUE);
         }
 
-        if (!includeUnDiscoverable) {
-            solrQuery.addFilterQuery("NOT(withdrawn:true)");
-            solrQuery.addFilterQuery("NOT(discoverable:false)");
-        }
-
         for (int i = 0; i < discoveryQuery.getFilterQueries().size(); i++) {
             String filterQuery = discoveryQuery.getFilterQueries().get(i);
             solrQuery.addFilterQuery(filterQuery);
@@ -2401,8 +2385,8 @@ public class SolrServiceImpl implements SearchService, IndexingService {
 
         } catch (Exception e) {
             log.error(
-                LogManager.getHeader(context, "Error while retrieving related items", "Handle: " + item.getHandle()),
+                LogManager.getHeader(context, "Error while retrieving related items", "Handle: "
-                e);
+                    + item.getHandle()), e);
         }
         return results;
     }

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

@@ -18,9 +18,12 @@ import org.dspace.core.Context;
 import org.dspace.core.LogManager;
 import org.springframework.beans.factory.annotation.Autowired;
 
-public class SolrServiceUnDiscoverableItemsPlugin implements SolrServiceSearchPlugin {
+/**
+ * This plugin prevents discovery of private items by non-administrators.
+ */
+public class SolrServicePrivateItemPlugin implements SolrServiceSearchPlugin {
 
-    private static final Logger log = getLogger(SolrServiceUnDiscoverableItemsPlugin.class.getSimpleName());
+    private static final Logger log = getLogger(SolrServicePrivateItemPlugin.class.getSimpleName());
 
     @Autowired(required = true)
     protected AuthorizeService authorizeService;
@@ -29,11 +32,10 @@ public class SolrServiceUnDiscoverableItemsPlugin implements SolrServiceSearchPl
     public void additionalSearchParameters(Context context, DiscoverQuery discoveryQuery, SolrQuery solrQuery) {
         try {
             if (!authorizeService.isAdmin(context)) {
-                solrQuery.addFilterQuery("NOT(withdrawn:true");
                 solrQuery.addFilterQuery("NOT(discoverable:false)");
             }
         } catch (SQLException e) {
-            log.error(LogManager.getHeader(context, "Error while adding non-administrator filter to query", ""), e);
+            log.error(LogManager.getHeader(context, "Error while adding non-administrator filters to query", ""), e);
         }
     }
-}
+}

dspace-api/src/main/java/org/dspace/discovery/configuration/DiscoveryConfigurationService.java

@@ -13,7 +13,6 @@ import java.util.List;
 import java.util.Map;
 
 import org.apache.commons.lang3.StringUtils;
-import org.dspace.content.Community;
 import org.dspace.content.DSpaceObject;
 import org.dspace.discovery.IndexableObject;
 import org.dspace.services.factory.DSpaceServicesFactory;
@@ -70,15 +69,7 @@ public class DiscoveryConfigurationService {
 
     public DiscoveryConfiguration getDiscoveryConfigurationByNameOrDso(final String configurationName,
                                                                        final IndexableObject dso) {
-
+        if (StringUtils.isNotBlank(configurationName) && getMap().containsKey(configurationName)) {
-        if (dso instanceof Community && configurationName.contentEquals("showUnDiscoverableItems")) {
-            // Use community-level configuration if it is defined. If not, fall back to configuration name.
-            if (getMap().containsKey(configurationName + "." + ((Community) dso).getHandle())) {
-                return getMap().get(configurationName +  "." + ((Community) dso).getHandle());
-            } else {
-                return getMap().get(configurationName);
-            }
-        } else if (StringUtils.isNotBlank(configurationName) && getMap().containsKey(configurationName)) {
             return getMap().get(configurationName);
         } else {
             return getDiscoveryConfiguration(dso);

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java

@@ -102,13 +102,7 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
         try {
             discoverQuery = queryBuilder
                 .buildQuery(context, scopeObject, discoveryConfiguration, query, searchFilters, dsoType, page);
-
+            searchResult = searchService.search(context, scopeObject, discoverQuery);
-            if (configuration != null && configuration.contentEquals("showUnDiscoverableItems")) {
-                // Prevents the exclusion of withdrawn and private items for this configuration.
-                searchResult = searchService.search(context, scopeObject, discoverQuery, true);
-            } else {
-                searchResult = searchService.search(context, scopeObject, discoverQuery);
-            }
 
         } catch (SearchServiceException e) {
             log.error("Error while searching with Discovery", e);

dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java

@@ -3582,4 +3582,138 @@ public class DiscoveryRestControllerIT extends AbstractControllerIntegrationTest
 
     }
 
+    @Test
+    public void discoverSearchObjectsTestForWithdrawnOrPrivateItemsNonAdmin() throws Exception {
+        //We turn off the authorization system in order to create the structure as defined below
+        context.turnOffAuthorisationSystem();
+
+        //** GIVEN **
+        //1. A community-collection structure with one parent community with sub-community and two collections.
+        parentCommunity = CommunityBuilder.createCommunity(context)
+                                          .withName("Parent Community")
+                                          .build();
+        Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity)
+                                           .withName("Sub Community")
+                                           .build();
+        Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build();
+        Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build();
+        //2. Two public items that are readable by Anonymous with different subjects and one private item
+        Item publicItem1 = ItemBuilder.createItem(context, col1)
+                                      .withTitle("Test")
+                                      .withIssueDate("2010-10-17")
+                                      .withAuthor("Smith, Donald")
+                                      .withSubject("ExtraEntry")
+                                      .build();
+
+        Item publicItem2 = ItemBuilder.createItem(context, col2)
+                                      .withTitle("WithdrawnTest 2")
+                                      .withIssueDate("1990-02-13")
+                                      .withAuthor("Smith, Maria").withAuthor("Doe, Jane")
+                                      .withSubject("ExtraEntry")
+                                      .withdrawn()
+                                      .build();
+
+        Item publicItem3 = ItemBuilder.createItem(context, col2)
+                                      .withTitle("Private Test item 2")
+                                      .withIssueDate("2010-02-13")
+                                      .withAuthor("Smith, Maria").withAuthor("Doe, Jane")
+                                      .withSubject("AnotherTest").withSubject("ExtraEntry")
+                                      .makeUnDiscoverable()
+                                      .build();
+
+
+        String query = "Test";
+        //** WHEN **
+        //An anonymous user browses this endpoint to find the the withdrawn or private objects in the system
+        //With a query stating 'Test'
+        getClient().perform(get("/api/discover/search/objects")
+            .param("configuration", "showUnDiscoverableItems")
+            .param("query", query))
+                   //** THEN **
+                   //The status has to be 200 OK
+                   .andExpect(status().isOk())
+                   //The type has to be 'discover'
+                   .andExpect(jsonPath("$.type", is("discover")))
+                   //The page object needs to look like this
+                   .andExpect(jsonPath("$._embedded.searchResult.page", is(
+                       PageMatcher.pageEntry(0, 20)
+                   )))
+                   //The search results should be an empty list.
+                   .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.empty()))
+                   //There always needs to be a self link available
+                   .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
+
+        ;
+
+    }
+
+    @Test
+    public void discoverSearchObjectsTestForWithdrawnOrPrivateItemsByAdminUser() throws Exception {
+        //We turn off the authorization system in order to create the structure as defined below
+        context.turnOffAuthorisationSystem();
+
+        //** GIVEN **
+        //1. A community-collection structure with one parent community with sub-community and two collections.
+        parentCommunity = CommunityBuilder.createCommunity(context)
+                                          .withName("Parent Community")
+                                          .build();
+        Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity)
+                                           .withName("Sub Community")
+                                           .build();
+        Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build();
+        Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build();
+        //2. Two public items that are readable by Anonymous with different subjects and one private item
+        Item publicItem1 = ItemBuilder.createItem(context, col1)
+                                      .withTitle("Test")
+                                      .withIssueDate("2010-10-17")
+                                      .withAuthor("Smith, Donald")
+                                      .withSubject("ExtraEntry")
+                                      .build();
+
+        Item publicItem2 = ItemBuilder.createItem(context, col2)
+                                      .withTitle("Withdrawn Test 2")
+                                      .withIssueDate("1990-02-13")
+                                      .withAuthor("Smith, Maria").withAuthor("Doe, Jane")
+                                      .withSubject("ExtraEntry")
+                                      .withdrawn()
+                                      .build();
+
+        Item publicItem3 = ItemBuilder.createItem(context, col2)
+                                      .withTitle("Private Test item 2")
+                                      .withIssueDate("2010-02-13")
+                                      .withAuthor("Smith, Maria").withAuthor("Doe, Jane")
+                                      .withSubject("AnotherTest").withSubject("ExtraEntry")
+                                      .makeUnDiscoverable()
+                                      .build();
+        context.restoreAuthSystemState();
+
+        String query = "Test";
+        String adminToken = getAuthToken(admin.getEmail(), password);
+        //** WHEN **
+        //A system admin user browses this endpoint to find the withdrawn or private objects in the system
+        //With a query stating 'Test'
+        getClient(adminToken).perform(get("/api/discover/search/objects")
+            .param("configuration", "showUnDiscoverableItems")
+            .param("query", query))
+                   //** THEN **
+                   //The status has to be 200 OK
+                   .andExpect(status().isOk())
+                   //The type has to be 'discover'
+                   .andExpect(jsonPath("$.type", is("discover")))
+                   //The page object needs to look like this
+                   .andExpect(jsonPath("$._embedded.searchResult.page", is(
+                       PageMatcher.pageEntry(0, 20)
+                   )))
+                   //The search results should be an empty list.
+                             .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.containsInAnyOrder(
+                                 SearchResultMatcher.matchOnItemName("item", "items", "Private Test item 2"),
+                                 SearchResultMatcher.matchOnItemName("item", "items", "Withdrawn Test 2")
+                             )))
+                   //There always needs to be a self link available
+                   .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
+
+        ;
+
+    }
+
 }

dspace/config/spring/api/discovery.xml

@@ -27,6 +27,7 @@
     <bean id="solrServiceResourceIndexPlugin" class="org.dspace.discovery.SolrServiceResourceRestrictionPlugin" scope="prototype"/>
     <bean id="SolrServiceSpellIndexingPlugin" class="org.dspace.discovery.SolrServiceSpellIndexingPlugin" scope="prototype"/>
     <bean id="solrServiceMetadataBrowseIndexingPlugin" class="org.dspace.discovery.SolrServiceMetadataBrowseIndexingPlugin" scope="prototype"/>
+    <bean id="solrServicePrivateItemPlugin" class="org.dspace.discovery.SolrServicePrivateItemPlugin" scope="prototype"/>
 
     <alias name="solrServiceResourceIndexPlugin" alias="org.dspace.discovery.SolrServiceResourceRestrictionPlugin"/>
 
@@ -157,6 +158,8 @@
             <list>
                 <!--Only find items, communities and collections-->
                 <value>search.resourcetype:2 OR search.resourcetype:3 OR search.resourcetype:4</value>
+                <value>NOT(withdrawn:true)</value>
+                <value>NOT(discoverable:false)</value>
             </list>
         </property>
         <!--The configuration for the recent submissions-->
@@ -244,7 +247,7 @@
         <property name="spellCheckEnabled" value="true"/>
     </bean>
 
-    <!--The default configuration settings for discovery-->
+    <!--The configuration settings for discovery of withdrawn and indiscoverable items (admin only)-->
     <bean id="unDiscoverableItemsConfiguration" class="org.dspace.discovery.configuration.DiscoveryConfiguration" scope="prototype">
         <!--Which sidebar facets are to be displayed-->
         <property name="sidebarFacets">
@@ -295,8 +298,9 @@
             queries done by discovery for this configuration -->
         <property name="defaultFilterQueries">
             <list>
-                <!--Only find items, communities and collections-->
+                <!--Only find items-->
                 <value>search.resourcetype:2</value>
+                <!-- Only find withdrawn or undiscoverable-->
                 <value>withdrawn:true OR discoverable:false</value>
             </list>
         </property>
@@ -545,6 +549,8 @@
             <list>
                 <!--Only find items, workspace and accepted for workflow -->
                 <value>search.resourcetype:2 OR search.resourcetype:[8 TO 9]</value>
+                <value>NOT(withdrawn:true)</value>
+                <value>NOT(discoverable:false)</value>
             </list>
         </property>
         <!--Default result per page  -->
@@ -622,6 +628,8 @@
             <list>
                 <!--Only find PoolTask and ClaimedTask -->
                 <value>search.resourcetype:[10 TO 11]</value>
+                <value>NOT(withdrawn:true)</value>
+                <value>NOT(discoverable:false)</value>
             </list>
         </property>
         <!--Default result per page  -->
@@ -775,6 +783,8 @@
             <list>
                 <!--Only find items, communities and collections-->
                 <value>search.resourcetype:2 AND entityType_keyword:Person</value>
+                <value>NOT(withdrawn:true)</value>
+                <value>NOT(discoverable:false)</value>
             </list>
         </property>
         <!--Default result per page  -->