DS-4166 community feedback: code cleanup

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

@@ -379,10 +379,21 @@ public class DiscoverQuery {
         return (int) (Math.ceil((float) (newestYear) / gap) * gap);
     }
 
+    /**
+     * Return the name of discovery configuration used by this query
+     * 
+     * @return the discovery configuration name used
+     */
     public String getDiscoveryConfigurationName() {
         return discoveryConfigurationName;
     }
 
+    /**
+     * Set the name of discovery configuration to use to run this query
+     * 
+     * @param discoveryConfigurationName
+     *            the name of the discovery configuration to use to run this query
+     */
     public void setDiscoveryConfigurationName(String discoveryConfigurationName) {
         this.discoveryConfigurationName = discoveryConfigurationName;
     }

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

@@ -179,6 +179,10 @@ public class DiscoverResult {
         this.spellCheckQuery = spellCheckQuery;
     }
 
+    /**
+     * An utility class to represent the highlighting section of a Discovery Search
+     *
+     */
     public static final class IndexableObjectHighlightResult {
         private IndexableObject indexableObject;
         private Map<String, List<String>> highlightResults;
@@ -191,18 +195,42 @@ public class DiscoverResult {
             this.highlightResultsWithAuthority = highlightResultsWithAuthority;
         }
 
+        /**
+         * Return the indexable object that the highlighting snippets refer to
+         * 
+         * @return the indexable object
+         */
         public IndexableObject getIndexableObject() {
             return indexableObject;
         }
 
+        /**
+         * The matching snippets for a specific metadata ignoring any authority value
+         * 
+         * @param metadataKey
+         *            the metadata where the snippets have been found
+         * @return the matching snippets
+         */
         public List<String> getHighlightResults(String metadataKey) {
             return highlightResults.get(metadataKey);
         }
 
+        /**
+         * The matching snippets for a specific metadata including the authority value if any
+         * 
+         * @param metadataKey
+         *            the metadata where the snippets have been found
+         * @return the matching snippets
+         */
         public List<String[]> getHighlightResultsWithAuthority(String metadataKey) {
             return highlightResultsWithAuthority.get(metadataKey);
         }
 
+        /**
+         * All the matching snippets in whatever metadata ignoring any authority value
+         * 
+         * @return All the matching snippets
+         */
         public Map<String, List<String>> getHighlightResults() {
             return highlightResults;
         }

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

@@ -159,7 +159,7 @@ public class IndexEventConsumer implements Consumer {
                  * allow the search indexer to make
                  * decisions on indexing and/or removal
                  */
-                // iu = ctx.reloadEntity(o);
+                iu = ctx.reloadEntity(iu);
                 String uniqueIndexID = iu.getUniqueIndexID();
                 if (uniqueIndexID != null && !uniqueIdsToDelete.contains(uniqueIndexID)) {
                     try {

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

@@ -10,6 +10,7 @@ package org.dspace.discovery;
 import java.io.Serializable;
 
 import org.dspace.core.Constants;
+import org.dspace.core.ReloadableEntity;
 
 /**
  * This is the basic interface that a data model entity need to implement to be indexable in Discover
@@ -19,7 +20,7 @@ import org.dspace.core.Constants;
  * @param <PK>
  *            the Class of the primary key
  */
-public interface IndexableObject<PK extends Serializable> {
+public interface IndexableObject<PK extends Serializable> extends ReloadableEntity<PK> {
 
     /**
      * 
@@ -27,12 +28,6 @@ public interface IndexableObject<PK extends Serializable> {
      */
     public int getType();
 
-    /**
-     * 
-     * @return the primary key of the Entity instance
-     */
-    public PK getID();
-
     /**
      * 
      * @return an unique id to index

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

@@ -58,6 +58,16 @@ public class SearchUtils {
         return getDiscoveryConfiguration(null, dso);
     }
 
+    /**
+     * Return the discovery configuration to use in a specific scope for the king of search identified by the prefix. A
+     * null prefix mean the normal query, other predefined values are workspace or workflow
+     * 
+     * @param prefix
+     *            the namespace of the configuration to lookup if any
+     * @param dso
+     *            the DSpaceObject
+     * @return the discovery configuration for the specified scope
+     */
     public static DiscoveryConfiguration getDiscoveryConfiguration(String prefix, DSpaceObject dso) {
         if (prefix != null) {
             return getDiscoveryConfigurationByName(dso != null ? prefix + "." + dso.getHandle() : prefix);
@@ -96,12 +106,24 @@ public class SearchUtils {
         return getAllDiscoveryConfigurations(null, collections, item);
     }
 
+    /**
+     * Return all the discovery configuration applicable to the provided workspace item
+     * @param witem a workspace item
+     * @return a list of discovery configuration
+     * @throws SQLException
+     */
     public static List<DiscoveryConfiguration> getAllDiscoveryConfigurations(WorkspaceItem witem) throws SQLException {
         List<Collection> collections = new ArrayList<Collection>();
         collections.add(witem.getCollection());
         return getAllDiscoveryConfigurations("workspace", collections, witem.getItem());
     }
 
+    /**
+     * Return all the discovery configuration applicable to the provided workflow item
+     * @param witem a workflow item
+     * @return a list of discovery configuration
+     * @throws SQLException
+     */
     public static List<DiscoveryConfiguration> getAllDiscoveryConfigurations(WorkflowItem witem) throws SQLException {
         List<Collection> collections = new ArrayList<Collection>();
         collections.add(witem.getCollection());

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

@@ -33,8 +33,6 @@ import java.util.Set;
 import java.util.TimeZone;
 import java.util.UUID;
 import java.util.Vector;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import com.google.common.collect.ImmutableList;
 
@@ -170,8 +168,6 @@ public class SolrServiceImpl implements SearchService, IndexingService {
 
     public static final String VARIANTS_STORE_SEPARATOR = "###";
 
-    private static final String REGEX = "\\S+(?:\\s*\\|\\|\\|(\\s*\\S+))+";
-
     @Autowired(required = true)
     protected ContentServiceFactory contentServiceFactory;
     @Autowired(required = true)
@@ -478,12 +474,18 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                     Iterator<Item> items = itemService.findAllUnfiltered(context);
                     for (Item item : ImmutableList.copyOf(items)) {
                         indexContent(context, item, force);
+                        //To prevent memory issues, discard an object from the cache after processing
+                        context.uncacheEntity(item);
                     }
                     for (WorkspaceItem wsi : workspaceItemService.findAll(context)) {
                         indexContent(context, wsi.getItem(), force);
+                        //To prevent memory issues, discard an object from the cache after processing
+                        context.uncacheEntity(wsi);
                     }
                     for (WorkflowItem wfi : workflowItemService.findAll(context)) {
                         indexContent(context, wfi.getItem(), force);
+                        //To prevent memory issues, discard an object from the cache after processing
+                        context.uncacheEntity(wfi);
                     }
                     break;
                 case Constants.COLLECTION:
@@ -525,7 +527,9 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         if (force) {
             try {
                 getSolr().deleteByQuery(
-                    "search.resourcetype:[" + Constants.ITEM + " TO " + Constants.CLAIMEDTASK + "]");
+                    "search.resourcetype:[" + Constants.ITEM + " TO " + Constants.COMMUNITY + "]" +
+                    " AND " +
+                    "search.resourcetype:[" + Constants.WORKSPACEITEM + " TO " + Constants.CLAIMEDTASK + "]");
             } catch (Exception e) {
                 throw new SearchServiceException(e.getMessage(), e);
             }
@@ -1066,16 +1070,16 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                 "discovery.facet.namedtype." + typeText,
                 typeText + SolrServiceImpl.AUTHORITY_SEPARATOR + typeText);
         if (StringUtils.isNotBlank(acvalue)) {
-            String fvalue = acvalue;
+            addNamedResourceTypeIndex(doc, acvalue);
-            addNamedResourceTypeIndex(doc, acvalue, fvalue);
         }
 
         // write the index and close the inputstreamreaders
         try {
             writeDocument(doc, new FullTextContentStreams(context, item));
-            log.info("Wrote Item: " + handle + " to Index");
+            log.info("Wrote Item: " + item.getUniqueIndexID() + " to Index");
         } catch (RuntimeException e) {
-            log.error("Error while writing item to discovery index: " + handle + " message:" + e.getMessage(), e);
+            log.error("Error while writing item to discovery index: " + item.getUniqueIndexID() + " message:"
+                    + e.getMessage(), e);
         }
     }
 
@@ -1484,7 +1488,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                 if (hitHighlightingFields.contains(field) || hitHighlightingFields
                     .contains("*") || hitHighlightingFields.contains(unqualifiedField + "." + Item.ANY)) {
                     if (authority != null) {
-                        doc.addField(field + "_hl", value + "###" + authority);
+                        doc.addField(field + "_hl", value + AUTHORITY_SEPARATOR + authority);
                     } else {
                         doc.addField(field + "_hl", value);
                     }
@@ -1606,8 +1610,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
             if (StringUtils.isBlank(acvalue)) {
                 acvalue = workspaceItem.getTypeText();
             }
-            String fvalue = acvalue;
+            addNamedResourceTypeIndex(doc, acvalue);
-            addNamedResourceTypeIndex(doc, acvalue, fvalue);
             doc.addField("inprogress.item", item.getUniqueIndexID());
 
             getSolr().add(doc);
@@ -1647,8 +1650,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                     if (StringUtils.isBlank(acvalue)) {
                         acvalue = claimedTask.getTypeText();
                     }
-                    String fvalue = acvalue;
+                    addNamedResourceTypeIndex(claimDoc, acvalue);
-                    addNamedResourceTypeIndex(claimDoc, acvalue, fvalue);
 
                     docs.add(claimDoc);
                 }
@@ -1673,8 +1675,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
                     if (StringUtils.isBlank(acvalue)) {
                         acvalue = poolTask.getTypeText();
                     }
-                    String fvalue = acvalue;
+                    addNamedResourceTypeIndex(claimDoc, acvalue);
-                    addNamedResourceTypeIndex(claimDoc, acvalue, fvalue);
                     docs.add(claimDoc);
                 }
             }
@@ -1684,8 +1685,7 @@ public class SolrServiceImpl implements SearchService, IndexingService {
             if (StringUtils.isBlank(acvalue)) {
                 acvalue = workflowItem.getTypeText();
             }
-            String fvalue = acvalue;
+            addNamedResourceTypeIndex(doc, acvalue);
-            addNamedResourceTypeIndex(doc, acvalue, fvalue);
 
             addBasicInfoToDocument(doc, Constants.WORKFLOWITEM, workflowItem.getID(), null, locations);
             if (workflowItem.getSubmitter() != null) {
@@ -2006,13 +2006,6 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         boolean isWorkflow = StringUtils.startsWith(discoveryQuery.getDiscoveryConfigurationName(),
                 DISCOVER_WORKFLOW_CONFIGURATION_NAME);
         EPerson currentUser = context.getCurrentUser();
-        // Retrieve all the groups the current user is a member of !
-        Set<Group> groups;
-        try {
-            groups = groupService.allMemberGroupsSet(context, currentUser);
-        } catch (SQLException e) {
-            throw new org.dspace.discovery.SearchServiceException(e.getMessage(), e);
-        }
 
         // extra security check to avoid the possibility that an anonymous user
         // get access to workspace or workflow
@@ -2024,6 +2017,14 @@ public class SolrServiceImpl implements SearchService, IndexingService {
             solrQuery
                 .addFilterQuery("submitter:(" + currentUser.getID() + ")");
         } else if (isWorkflow) {
+            // Retrieve all the groups the current user is a member of !
+            Set<Group> groups;
+            try {
+                groups = groupService.allMemberGroupsSet(context, currentUser);
+            } catch (SQLException e) {
+                throw new org.dspace.discovery.SearchServiceException(e.getMessage(), e);
+            }
+
             // insert filter by controllers
             StringBuilder controllerQuery = new StringBuilder();
             controllerQuery.append("taskfor:(e" + currentUser.getID());
@@ -2187,20 +2188,6 @@ public class SolrServiceImpl implements SearchService, IndexingService {
         return result;
     }
 
-    public DiscoverResult.FacetResult getDiscoveryFacet(Context context, FacetField facetField,
-            FacetField.Count facetValue, String facetType) throws SQLException {
-        String displayedValue = transformDisplayedValue(context, facetField.getName(), facetValue.getName());
-        String authorityValue = transformAuthorityValue(context, facetField.getName(), facetValue.getName());
-        String sortValue = transformSortValue(context, facetField.getName(), facetValue.getName());
-        String filterValue = displayedValue;
-        if (StringUtils.isNotBlank(authorityValue)) {
-            filterValue = authorityValue;
-        }
-        DiscoverResult.FacetResult facetResult = new DiscoverResult.FacetResult(filterValue, displayedValue,
-                authorityValue, sortValue, facetValue.getCount(), facetType);
-        return facetResult;
-    }
-
     /**
      * Find the indexable object by type and UUID
      *
@@ -2609,30 +2596,38 @@ public class SolrServiceImpl implements SearchService, IndexingService {
      *
      * @param document
      *            the solr document
-     * @param acvalue
+     * @param filterValue
-     *            the authority value
+     *            the filter value (i.e. <sort_value>\n|||\n<display_value>###<authority_value>
-     * @param fvalue
-     *            the human readable value
      */
-    private void addNamedResourceTypeIndex(SolrInputDocument document, String acvalue, String fvalue) {
+    private void addNamedResourceTypeIndex(SolrInputDocument document, String filterValue) {
 
-        document.addField("namedresourcetype_filter", acvalue);
+        document.addField("namedresourcetype_filter", filterValue);
 
-        String[] avalues = acvalue.split(SolrServiceImpl.AUTHORITY_SEPARATOR);
+        // the separator for the filter can be eventually configured
-        acvalue = avalues[0];
+        String separator = DSpaceServicesFactory.getInstance().getConfigurationService()
-
+                .getProperty("discovery.solr.facets.split.char");
-        String avalue = avalues[1];
+        if (separator == null) {
-        document.addField("namedresourcetype_authority", avalue);
+            separator = FILTER_SEPARATOR;
-        document.addField("namedresourcetype_group", avalue);
-        document.addField("namedresourcetype_ac", acvalue);
-
-        Pattern pattern = Pattern.compile(REGEX);
-        Matcher matcher = pattern.matcher(acvalue);
-        if (matcher.matches()) {
-            fvalue = matcher.group(1);
         }
 
-        document.addField("namedresourcetype_keyword", fvalue);
+        // split the authority part from the sort/display
+        String[] avalues = filterValue.split(SolrServiceImpl.AUTHORITY_SEPARATOR);
+
+        String sortDisplayValues = avalues[0];
+        String authorityValue = avalues.length == 2 ? avalues[1] : filterValue;
+
+        // get the display value
+        int idxSeparator = sortDisplayValues.indexOf(separator);
+        String displayValue = idxSeparator != -1 ? sortDisplayValues.substring(idxSeparator + separator.length())
+                : sortDisplayValues;
+
+        document.addField("namedresourcetype_authority", authorityValue);
+        // build the solr fields used for the autocomplete
+        document.addField("namedresourcetype_ac", displayValue.toLowerCase() + separator + displayValue);
+        document.addField("namedresourcetype_acid", displayValue.toLowerCase() + separator + displayValue
+                + SolrServiceImpl.AUTHORITY_SEPARATOR + authorityValue);
+        // build the solr field used for the keyword search
+        document.addField("namedresourcetype_keyword", displayValue);
     }
 
 }

dspace-spring-rest/src/main/java/org/dspace/app/rest/converter/SearchFilterToAppliedFilterConverter.java

@@ -34,6 +34,7 @@ public class SearchFilterToAppliedFilterConverter {
             // Moreover, it is not possible to discover which authority is responsible for the value selected in the
             // facet as the authority is bind at the metadata level and so a facet could contains values from multiple
             // authorities
+            // https://jira.duraspace.org/browse/DS-4209
             authorityValue = authorityValueService.findByUID(context, searchFilter.getValue());
         }
 

dspace/config/modules/discovery.cfg

@@ -18,6 +18,12 @@ discovery.search.server = ${solr.server}/search
 # discovery.index.ignore-authority = false
 discovery.index.projection=dc.title,dc.contributor.*,dc.date.issued
 
+# Value used for the namedresourcetype facet used by the mydspace
+# <sort-value>\n|||\n<display-value>###<authority-value>
+# the separator between the sort-value and the display-value \n|||\n must 
+# match the value of the discovery.solr.facets.split.char defined above
+# the sort-value can be used to force a fixed order for the facet if it is 
+# configured in the discovery.xml to be sorted by value
 discovery.facet.namedtype.item = 000item\n|||\nArchived###item
 discovery.facet.namedtype.workspace = 001workspace\n|||\nWorkspace###workspace
 discovery.facet.namedtype.workflow.item = 002workflow\n|||\nWorkflow###workflow

dspace/config/spring/api/discovery.xml

@@ -301,7 +301,6 @@
         <property name="spellCheckEnabled" value="true"/>
     </bean>
 
-
     <!--The workspace configuration settings for discovery -->
     <bean id="workspaceConfiguration"
         class="org.dspace.discovery.configuration.DiscoveryConfiguration"