97248: Implement feedback

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

@@ -62,12 +62,24 @@ public class SearchUtils {
         return searchService;
     }
 
+    /**
+     * Retrieves the Discovery Configuration for a null context, prefix and DSpace object.
+     * This will result in returning the default configuration
+     * @return the default configuration
+     */
     public static DiscoveryConfiguration getDiscoveryConfiguration() {
         return getDiscoveryConfiguration(null, null, null);
     }
 
-    public static DiscoveryConfiguration getDiscoveryConfiguration(final Context context,
+    /**
-                                                                   DSpaceObject dso) {
+     * Retrieves the Discovery Configuration with a null prefix for a DSpace object.
+     * @param context
+     *              the dabase context
+     * @param dso
+     *              the DSpace object
+     * @return the Discovery Configuration for the specified DSpace object
+     */
+    public static DiscoveryConfiguration getDiscoveryConfiguration(Context context, DSpaceObject dso) {
         return getDiscoveryConfiguration(context, null, dso);
     }
 
@@ -84,7 +96,7 @@ public class SearchUtils {
      *            the DSpaceObject
      * @return the discovery configuration for the specified scope
      */
-    public static DiscoveryConfiguration getDiscoveryConfiguration(final Context context, String prefix,
+    public static DiscoveryConfiguration getDiscoveryConfiguration(Context context, String prefix,
                                                                    DSpaceObject dso) {
         if (prefix != null) {
             return getDiscoveryConfigurationByName(dso != null ? prefix + "." + dso.getHandle() : prefix);
@@ -161,12 +173,12 @@ public class SearchUtils {
      * Method that retrieves a list of all the configuration objects from the given item
      * A configuration object can be returned for each parent community/collection
      *
-     * @param item the DSpace item
      * @param context   the database context
+     * @param item the DSpace item
      * @return a list of configuration objects
      * @throws SQLException An exception that provides information on a database access error or other errors.
      */
-    public static List<DiscoveryConfiguration> getAllDiscoveryConfigurations(Item item, Context context)
+    public static List<DiscoveryConfiguration> getAllDiscoveryConfigurations(Context context, Item item)
             throws SQLException {
         List<Collection> collections = item.getCollections();
         return getAllDiscoveryConfigurations(context, null, collections, item);

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

@@ -53,16 +53,20 @@ public class SolrServiceFileInfoPlugin implements SolrServiceIndexPlugin {
                         if (bitstreams != null) {
                             for (Bitstream bitstream : bitstreams) {
                                 document.addField(SOLR_FIELD_NAME_FOR_FILENAMES, bitstream.getName());
+                                // Add _keyword and _filter fields which are necessary to support filtering and faceting
+                                // for the file names
                                 document.addField(SOLR_FIELD_NAME_FOR_FILENAMES + "_keyword", bitstream.getName());
                                 document.addField(SOLR_FIELD_NAME_FOR_FILENAMES + "_filter", bitstream.getName());
 
                                 String description = bitstream.getDescription();
                                 if ((description != null) && !description.isEmpty()) {
                                     document.addField(SOLR_FIELD_NAME_FOR_DESCRIPTIONS, description);
+                                    // Add _keyword and _filter fields which are necessary to support filtering and
+                                    // faceting for the descriptions
                                     document.addField(SOLR_FIELD_NAME_FOR_DESCRIPTIONS + "_keyword",
-                                                      bitstream.getName());
+                                                      description);
                                     document.addField(SOLR_FIELD_NAME_FOR_DESCRIPTIONS + "_filter",
-                                                      bitstream.getName());
+                                                      description);
                                 }
                             }
                         }

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

@@ -17,6 +17,8 @@ import java.util.UUID;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.dspace.content.Collection;
+import org.dspace.content.Community;
 import org.dspace.content.DSpaceObject;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.DSpaceObjectService;
@@ -36,10 +38,11 @@ public class DiscoveryConfigurationService {
     private Map<Integer, List<String>> toIgnoreMetadataFields = new HashMap<>();
 
     /**
-     * Discovery configurations, cached by DSO UUID. When a DSO doesn't have its own configuration, we take the one of
+     * Discovery configurations, cached by Community/Collection UUID. When a  Community or Collection does not have its
-     * the first parent that does. This cache ensures we don't have to go up the hierarchy every time.
+     * own configuration, we take the one of the first parent that does.
+     * This cache ensures we do not have to go up the hierarchy every time.
      */
-    private final Map<UUID, DiscoveryConfiguration> uuidMap = new HashMap<>();
+    private final Map<UUID, DiscoveryConfiguration> comColToDiscoveryConfigurationMap = new HashMap<>();
 
     public Map<String, DiscoveryConfiguration> getMap() {
         return map;
@@ -57,15 +60,26 @@ public class DiscoveryConfigurationService {
         this.toIgnoreMetadataFields = toIgnoreMetadataFields;
     }
 
-    public DiscoveryConfiguration getDiscoveryConfiguration(final Context context,
+    /**
-                                                            IndexableObject dso) {
+     * Retrieve the discovery configuration for the provided IndexableObject. When a DSpace Object can be retrieved from
+     * the IndexableObject, the discovery configuration will be returned for the DSpace Object. Otherwise, a check will
+     * be done to look for the unique index ID of the IndexableObject. When the IndexableObject is null, the default
+     * configuration will be retrieved
+     *
+     * When no direct match is found, the parent object will
+     * be checked until there is no parent left, in which case the "default" configuration will be returned.
+     * @param context   - The database context
+     * @param indexableObject       - The IndexableObject to retrieve the configuration for
+     * @return the discovery configuration for the provided IndexableObject.
+     */
+    public DiscoveryConfiguration getDiscoveryConfiguration(Context context, IndexableObject indexableObject) {
         String name;
-        if (dso == null) {
+        if (indexableObject == null) {
-            name = "default";
+            return getDiscoveryConfiguration(null);
-        } else if (dso instanceof IndexableDSpaceObject) {
+        } else if (indexableObject instanceof IndexableDSpaceObject) {
-            return getDiscoveryDSOConfiguration(context, ((IndexableDSpaceObject) dso).getIndexedObject());
+            return getDiscoveryDSOConfiguration(context, ((IndexableDSpaceObject) indexableObject).getIndexedObject());
         } else {
-            name = dso.getUniqueIndexID();
+            name = indexableObject.getUniqueIndexID();
         }
         return getDiscoveryConfiguration(name);
     }
@@ -77,16 +91,15 @@ public class DiscoveryConfigurationService {
      * @param dso       - The DSpace object to retrieve the configuration for
      * @return the discovery configuration for the provided DSO.
      */
-    public DiscoveryConfiguration getDiscoveryDSOConfiguration(final Context context,
+    public DiscoveryConfiguration getDiscoveryDSOConfiguration(final Context context, DSpaceObject dso) {
-                                                               DSpaceObject dso) {
         // Fall back to default configuration
         if (dso == null) {
-            return getDiscoveryConfiguration("default", false);
+            return getDiscoveryConfiguration(null, true);
         }
 
         // Attempt to retrieve cached configuration by UUID
-        if (uuidMap.containsKey(dso.getID())) {
+        if (comColToDiscoveryConfigurationMap.containsKey(dso.getID())) {
-            return uuidMap.get(dso.getID());
+            return comColToDiscoveryConfigurationMap.get(dso.getID());
         }
 
         DiscoveryConfiguration configuration;
@@ -107,13 +120,21 @@ public class DiscoveryConfigurationService {
             configuration = getDiscoveryDSOConfiguration(context, parentObject);
         }
 
-        // Cache the resulting configuration
+        // Cache the resulting configuration when the DSO is a Community or Collection
-        uuidMap.put(dso.getID(), configuration);
+        if (dso instanceof Community || dso instanceof Collection) {
+            comColToDiscoveryConfigurationMap.put(dso.getID(), configuration);
+        }
 
         return configuration;
     }
 
-    public DiscoveryConfiguration getDiscoveryConfiguration(final String name) {
+    /**
+     * Retrieve the Discovery Configuration for the provided name. When no configuration can be found for the name, the
+     * default configuration will be returned.
+     * @param name  - The name of the configuration to be retrieved
+     * @return the Discovery Configuration for the provided name, or default when none was found.
+     */
+    public DiscoveryConfiguration getDiscoveryConfiguration(String name) {
         return getDiscoveryConfiguration(name, true);
     }
 
@@ -138,13 +159,23 @@ public class DiscoveryConfigurationService {
         return result;
     }
 
-    public DiscoveryConfiguration getDiscoveryConfigurationByNameOrDso(final Context context,
+    /**
-                                                                       final String configurationName,
+     * Retrieve the Discovery configuration for the provided name or IndexableObject. The configuration will first be
-                                                                       final IndexableObject dso) {
+     * checked for the provided name. When no match is found for the name, the configuration will be retrieved for the
+     * IndexableObject
+     *
+     * @param context           - The database context
+     * @param configurationName - The name of the configuration to be retrieved
+     * @param indexableObject   - The indexable object to retrieve the configuration for
+     * @return the Discovery configuration for the provided name, or when not found for the provided IndexableObject
+     */
+    public DiscoveryConfiguration getDiscoveryConfigurationByNameOrIndexableObject(Context context,
+                                                                                   String configurationName,
+                                                                                   IndexableObject indexableObject) {
         if (StringUtils.isNotBlank(configurationName) && getMap().containsKey(configurationName)) {
             return getMap().get(configurationName);
         } else {
-            return getDiscoveryConfiguration(context, dso);
+            return getDiscoveryConfiguration(context, indexableObject);
         }
     }
 

dspace-api/src/main/java/org/dspace/discovery/indexobject/InprogressSubmissionIndexFactoryImpl.java

@@ -79,7 +79,7 @@ public abstract class InprogressSubmissionIndexFactoryImpl
             discoveryConfigurations = SearchUtils.getAllDiscoveryConfigurations(context,
                                                                                 (WorkspaceItem) inProgressSubmission);
         } else {
-            discoveryConfigurations = SearchUtils.getAllDiscoveryConfigurations(item, context);
+            discoveryConfigurations = SearchUtils.getAllDiscoveryConfigurations(context, item);
         }
         indexableItemService.addDiscoveryFields(doc, context, item, discoveryConfigurations);
         indexableCollectionService.storeCommunityCollectionLocations(doc, locations);

dspace-api/src/main/java/org/dspace/discovery/indexobject/ItemIndexFactoryImpl.java

@@ -147,7 +147,7 @@ public class ItemIndexFactoryImpl extends DSpaceObjectIndexFactoryImpl<Indexable
         }
 
         // Add the item metadata
-        List<DiscoveryConfiguration> discoveryConfigurations = SearchUtils.getAllDiscoveryConfigurations(item, context);
+        List<DiscoveryConfiguration> discoveryConfigurations = SearchUtils.getAllDiscoveryConfigurations(context, item);
         addDiscoveryFields(doc, context, indexableItem.getIndexedObject(), discoveryConfigurations);
 
         //mandatory facet to show status on mydspace

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

@@ -84,7 +84,7 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
 
         IndexableObject scopeObject = scopeResolver.resolveScope(context, dsoScope);
         DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
-            .getDiscoveryConfigurationByNameOrDso(context, configuration, scopeObject);
+            .getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject);
 
         return discoverConfigurationConverter.convert(discoveryConfiguration, utils.obtainProjection());
     }
@@ -96,7 +96,7 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
         Context context = obtainContext();
         IndexableObject scopeObject = scopeResolver.resolveScope(context, dsoScope);
         DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
-            .getDiscoveryConfigurationByNameOrDso(context, configuration, scopeObject);
+            .getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject);
 
         DiscoverResult searchResult = null;
         DiscoverQuery discoverQuery = null;
@@ -121,7 +121,7 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
 
         IndexableObject scopeObject = scopeResolver.resolveScope(context, dsoScope);
         DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
-            .getDiscoveryConfigurationByNameOrDso(context, configuration, scopeObject);
+            .getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject);
 
         return discoverFacetConfigurationConverter.convert(configuration, dsoScope, discoveryConfiguration);
     }
@@ -138,7 +138,7 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
 
         IndexableObject scopeObject = scopeResolver.resolveScope(context, dsoScope);
         DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
-            .getDiscoveryConfigurationByNameOrDso(context, configuration, scopeObject);
+            .getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject);
 
         DiscoverQuery discoverQuery = queryBuilder.buildFacetQuery(context, scopeObject, discoveryConfiguration, prefix,
                 query, searchFilters, dsoTypes, page, facetName);
@@ -157,7 +157,7 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
         Pageable page = PageRequest.of(1, 1);
         IndexableObject scopeObject = scopeResolver.resolveScope(context, dsoScope);
         DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
-            .getDiscoveryConfigurationByNameOrDso(context, configuration, scopeObject);
+            .getDiscoveryConfigurationByNameOrIndexableObject(context, configuration, scopeObject);
 
         DiscoverResult searchResult = null;
         DiscoverQuery discoverQuery = null;

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

@@ -272,7 +272,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("parentcommunity1field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "parentcommunity1field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/parentcommunity1field")
@@ -310,7 +310,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("subcommunity11field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "subcommunity11field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/subcommunity11field")
@@ -339,7 +339,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("collection111field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "collection111field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/collection111field")
@@ -366,7 +366,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("subcommunity11field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "subcommunity11field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/subcommunity11field")
@@ -391,7 +391,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("parentcommunity1field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "parentcommunity1field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/parentcommunity1field")
@@ -420,7 +420,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("collection121field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "collection121field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/collection121field")
@@ -445,7 +445,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("parentcommunity1field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "parentcommunity1field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/parentcommunity1field")
@@ -490,7 +490,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("subcommunity21field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "subcommunity21field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/subcommunity21field")
@@ -519,7 +519,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("collection211field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "collection211field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/collection211field")
@@ -546,7 +546,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("subcommunity21field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "subcommunity21field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/subcommunity21field")
@@ -588,7 +588,7 @@ public class DiscoveryScopeBasedRestControllerIT extends AbstractControllerInteg
                    .andExpect(jsonPath("$._links.self.href", containsString("api/discover/facets")))
                    .andExpect(jsonPath("$._embedded.facets", containsInAnyOrder(
                            FacetEntryMatcher.authorFacet(false),
-                           FacetEntryMatcher.matchFacet("collection221field", "text", false)))
+                           FacetEntryMatcher.matchFacet(false, "collection221field", "text")))
                    );
 
         getClient().perform(get("/api/discover/facets/collection221field")

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

@@ -99,7 +99,7 @@ public class FacetEntryMatcher {
         );
     }
 
-    public static Matcher<? super Object> matchFacet(String name, String facetType, boolean hasNext) {
+    public static Matcher<? super Object> matchFacet(boolean hasNext, String name, String facetType) {
         return allOf(
                 hasJsonPath("$.name", is(name)),
                 hasJsonPath("$.facetType", is(facetType)),