Merge pull request #8044 from 4Science/CST-4882

[CST-4882] The search loads indefinitely if the written query is invalid

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

@@ -17,6 +17,7 @@ import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.dspace.app.rest.converter.ConverterService;
+import org.dspace.app.rest.exception.UnprocessableEntityException;
 import org.dspace.app.rest.link.HalLinkService;
 import org.dspace.app.rest.model.FacetConfigurationRest;
 import org.dspace.app.rest.model.FacetResultsRest;
@@ -52,6 +53,8 @@ public class DiscoveryRestController implements InitializingBean {
 
     private static final Logger log = LogManager.getLogger();
 
+    private static final String SOLR_PARSE_ERROR_CLASS = "org.apache.solr.search.SyntaxError";
+
     @Autowired
     protected Utils utils;
 
@@ -149,13 +152,22 @@ public class DiscoveryRestController implements InitializingBean {
         }
 
         //Get the Search results in JSON format
-        SearchResultsRest searchResultsRest = discoveryRestRepository
+        try {
-            .getSearchObjects(query, dsoTypes, dsoScope, configuration, searchFilters, page, utils.obtainProjection());
+            SearchResultsRest searchResultsRest = discoveryRestRepository.getSearchObjects(query, dsoTypes, dsoScope,
+                configuration, searchFilters, page, utils.obtainProjection());
 
-        //Convert the Search JSON results to paginated HAL resources
+            //Convert the Search JSON results to paginated HAL resources
-        SearchResultsResource searchResultsResource = new SearchResultsResource(searchResultsRest, utils, page);
+            SearchResultsResource searchResultsResource = new SearchResultsResource(searchResultsRest, utils, page);
-        halLinkService.addLinks(searchResultsResource, page);
+            halLinkService.addLinks(searchResultsResource, page);
-        return searchResultsResource;
+            return searchResultsResource;
+        } catch (IllegalArgumentException e) {
+            boolean isParsingException = e.getMessage().contains(SOLR_PARSE_ERROR_CLASS);
+            if (isParsingException) {
+                throw new UnprocessableEntityException(e.getMessage());
+            } else {
+                throw e;
+            }
+        }
     }
 
     @RequestMapping(method = RequestMethod.GET, value = "/facets")
@@ -198,13 +210,27 @@ public class DiscoveryRestController implements InitializingBean {
                           + ", page: " + Objects.toString(page));
         }
 
-        FacetResultsRest facetResultsRest = discoveryRestRepository
+        try {
-            .getFacetObjects(facetName, prefix, query, dsoTypes, dsoScope, configuration, searchFilters, page);
+            FacetResultsRest facetResultsRest = discoveryRestRepository
+                .getFacetObjects(facetName, prefix, query, dsoTypes, dsoScope, configuration, searchFilters, page);
 
-        FacetResultsResource facetResultsResource = converter.toResource(facetResultsRest);
+            FacetResultsResource facetResultsResource = converter.toResource(facetResultsRest);
 
-        halLinkService.addLinks(facetResultsResource, page);
+            halLinkService.addLinks(facetResultsResource, page);
-        return facetResultsResource;
+            return facetResultsResource;
+        } catch (Exception e) {
+            boolean isParsingException = e.getMessage().contains(SOLR_PARSE_ERROR_CLASS);
+            /*
+             * We unfortunately have to do a string comparison to locate the source of the error, as Solr only sends
+             * back a generic exception, and the org.apache.solr.search.SyntaxError is only available as plain text
+             * in the error message.
+             */
+            if (isParsingException) {
+                throw new UnprocessableEntityException(e.getMessage());
+            } else {
+                throw e;
+            }
+        }
     }
 
 }

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

@@ -131,7 +131,8 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
     }
 
     public FacetResultsRest getFacetObjects(String facetName, String prefix, String query, List<String> dsoTypes,
-            String dsoScope, final String configuration, List<SearchFilter> searchFilters, Pageable page) {
+            String dsoScope, final String configuration, List<SearchFilter> searchFilters, Pageable page)
+                    throws SearchServiceException {
 
         Context context = obtainContext();
 
@@ -139,17 +140,9 @@ public class DiscoveryRestRepository extends AbstractDSpaceRestRepository {
         DiscoveryConfiguration discoveryConfiguration = searchConfigurationService
             .getDiscoveryConfigurationByNameOrDso(configuration, scopeObject);
 
-        DiscoverResult searchResult = null;
+        DiscoverQuery discoverQuery = queryBuilder.buildFacetQuery(context, scopeObject, discoveryConfiguration, prefix,
-        DiscoverQuery discoverQuery = null;
+                query, searchFilters, dsoTypes, page, facetName);
-        try {
+        DiscoverResult searchResult = searchService.search(context, scopeObject, discoverQuery);
-            discoverQuery = queryBuilder.buildFacetQuery(context, scopeObject, discoveryConfiguration, prefix, query,
-                    searchFilters, dsoTypes, page, facetName);
-            searchResult = searchService.search(context, scopeObject, discoverQuery);
-
-        } catch (SearchServiceException e) {
-            log.error("Error while searching with Discovery", e);
-            //TODO TOM handle search exception
-        }
 
         FacetResultsRest facetResultsRest = discoverFacetResultsConverter.convert(context, facetName, prefix, query,
                 dsoTypes, dsoScope, searchFilters, searchResult, discoveryConfiguration, page,

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

@@ -1442,6 +1442,29 @@ public class DiscoveryRestControllerIT extends AbstractControllerIntegrationTest
                 //There always needs to be a self link available
                 .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
         ;
+
+        getClient().perform(get("/api/discover/search/objects")
+                .param("query", "test"))
+                .andExpect(status().isOk());
+
+        getClient().perform(get("/api/discover/search/objects")
+                .param("query", "test:"))
+                .andExpect(status().isUnprocessableEntity());
+
+    }
+
+
+    @Test
+    public void discoverSearchObjectsTestWithInvalidSolrQuery() throws Exception {
+
+        getClient().perform(get("/api/discover/search/objects")
+                .param("query", "test"))
+                .andExpect(status().isOk());
+
+        getClient().perform(get("/api/discover/search/objects")
+                .param("query", "test:"))
+                .andExpect(status().isUnprocessableEntity());
+
     }
 
 
@@ -3910,7 +3933,7 @@ public class DiscoveryRestControllerIT extends AbstractControllerIntegrationTest
         getClient().perform(get("/api/discover/search/objects")
                                 .param("query", "OR"))
 
-                   .andExpect(status().isBadRequest())
+                   .andExpect(status().isUnprocessableEntity())
         ;
 
     }