Merge pull request #1721 from UNC-Libraries/bxc-4524-facet

BXC-4524 - Cleanup warnings when invalid facet key provided
UNC-Libraries · Apr 9, 2024 · 20fbf70 · 20fbf70
2 parents 354c78b + 225fc51
commit 20fbf70
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 5 deletions.
diff --git a/search-solr/pom.xml b/search-solr/pom.xml
@@ -117,5 +117,11 @@
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-inline</artifactId>
+            <version>${mockito.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
diff --git a/search-solr/src/main/java/edu/unc/lib/boxc/search/solr/services/SearchStateFactory.java b/search-solr/src/main/java/edu/unc/lib/boxc/search/solr/services/SearchStateFactory.java
@@ -308,11 +308,16 @@ private void populateSearchState(SearchState searchState, Map<String,String[]> r
                 String parameterPairArray[] = parameterPair.split(":", 2);
                 if (parameterPairArray.length > 1) {
                     try {
-                        facetFieldUtil.setFacetLimit(searchSettings.searchFieldKey(parameterPairArray[0]),
-                                Integer.parseInt(parameterPairArray[1]), searchState);
-                    } catch (Exception e) {
-                        log.warn("Failed to add facet limit {} to field {}", new Object[] { parameterPairArray[0],
-                                parameterPairArray[1] }, e);
+                        var fieldKey = searchSettings.searchFieldKey(parameterPairArray[0]);
+                        if (fieldKey == null) {
+                            log.warn("Unknown facet limit field key: {}", parameterPairArray[0]);
+                            continue;
+                        }
+                        facetFieldUtil.setFacetLimit(fieldKey, Integer.parseInt(parameterPairArray[1]), searchState);
+                    } catch (IllegalArgumentException | InvalidFacetException e) {
+                        log.warn("Failed to add facet limit {} to field {}: {}", parameterPairArray[0],
+                                parameterPairArray[1], e.getMessage());
+                        log.debug("Exception from invalid facet limit", e);
                     }
                 }
             }
@@ -493,4 +498,8 @@ public void setSearchSettings(SearchSettings searchSettings) {
     public void setFacetFieldFactory(FacetFieldFactory facetFieldFactory) {
         this.facetFieldFactory = facetFieldFactory;
     }
+
+    public void setFacetFieldUtil(FacetFieldUtil facetFieldUtil) {
+        this.facetFieldUtil = facetFieldUtil;
+    }
 }
diff --git a/search-solr/src/main/java/edu/unc/lib/boxc/search/solr/utils/FacetFieldUtil.java b/search-solr/src/main/java/edu/unc/lib/boxc/search/solr/utils/FacetFieldUtil.java
@@ -171,4 +171,8 @@ public void setSearchSettings(SearchSettings searchSettings) {
     public void setSolrSettings(SolrSettings solrSettings) {
         this.solrSettings = solrSettings;
     }
+
+    public void setFacetFieldFactory(FacetFieldFactory facetFieldFactory) {
+        this.facetFieldFactory = facetFieldFactory;
+    }
 }
diff --git a/search-solr/src/test/java/edu/unc/lib/boxc/search/solr/services/SearchStateFactoryTest.java b/search-solr/src/test/java/edu/unc/lib/boxc/search/solr/services/SearchStateFactoryTest.java
@@ -2,15 +2,20 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mockStatic;
 
 import java.io.FileInputStream;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
+import edu.unc.lib.boxc.search.solr.config.SolrSettings;
+import edu.unc.lib.boxc.search.solr.utils.FacetFieldUtil;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -19,13 +24,15 @@
 import edu.unc.lib.boxc.search.api.facets.SearchFacet;
 import edu.unc.lib.boxc.search.api.requests.SearchState;
 import edu.unc.lib.boxc.search.solr.config.SearchSettings;
+import org.mockito.MockedStatic;
 
 /**
  * @author bbpennel
  */
 public class SearchStateFactoryTest {
 
     private SearchStateFactory searchStateFactory;
+    private FacetFieldUtil facetFieldUtil;
 
     @BeforeEach
     public void setup() throws Exception {
@@ -37,10 +44,15 @@ public void setup() throws Exception {
 
         FacetFieldFactory fff = new FacetFieldFactory();
         fff.setSearchSettings(searchSettings);
+        facetFieldUtil = new FacetFieldUtil();
+        facetFieldUtil.setSearchSettings(searchSettings);
+        facetFieldUtil.setSolrSettings(new SolrSettings());
+        facetFieldUtil.setFacetFieldFactory(fff);
 
         searchStateFactory = new SearchStateFactory();
         searchStateFactory.setSearchSettings(searchSettings);
         searchStateFactory.setFacetFieldFactory(fff);
+        searchStateFactory.setFacetFieldUtil(facetFieldUtil);
     }
 
     @Test
@@ -265,4 +277,111 @@ public void extractAdvancedSearchCollectionIdRight() {
         assertEquals("40000", searchState.getSearchFields()
                 .get(SearchFieldKey.COLLECTION_ID.name()));
     }
+
+    @Test
+    public void extractFacetLimitsValid() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("facetLimits", new String[]{"language:20"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        var limits = searchState.getFacetLimits();
+        assertEquals(1, limits.size());
+        assertEquals(20, limits.get("LANGUAGE"));
+    }
+
+    @Test
+    public void extractFacetLimitsUnknownField() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("facetLimits", new String[]{"boxyfield:20"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertTrue(searchState.getFacetLimits().isEmpty());
+    }
+
+    @Test
+    public void extractFacetLimitsInvalidFacetClass() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("facetLimits", new String[]{"language:20"});
+
+        try (MockedStatic<SearchSettings> mockedStatic = mockStatic(SearchSettings.class)) {
+            mockedStatic.when(() -> SearchSettings.getFacetClass(anyString())).thenReturn(Object.class);
+            SearchState searchState = searchStateFactory.createSearchState(parameters);
+            assertTrue(searchState.getFacetLimits().isEmpty());
+        }
+    }
+
+    @Test
+    public void extractFacetLimitBase() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("facetLimit", new String[]{"40"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertEquals(40, searchState.getBaseFacetLimit());
+    }
+
+    @Test
+    public void extractFacetLimitBaseNotANumber() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("facetLimit", new String[]{"boxy"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        // Should have the default base facet limit
+        assertEquals(6, searchState.getBaseFacetLimit());
+    }
+
+    @Test
+    public void extractSort() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("sort", new String[]{"title,normal"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertEquals("title", searchState.getSortType());
+        assertTrue(searchState.getSortNormalOrder());
+    }
+
+    @Test
+    public void extractFacetSelect() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("facetSelect", new String[]{"collection,format"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertIterableEquals(List.of("PARENT_COLLECTION", "FILE_FORMAT_CATEGORY"), searchState.getFacetsToRetrieve());
+    }
+
+    @Test
+    public void extractResourceTypes() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("types", new String[]{"work,file"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertIterableEquals(List.of("work", "file"), searchState.getResourceTypes());
+    }
+
+    @Test
+    public void extractRollup() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("rollup", new String[]{"true"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertTrue(searchState.getRollup());
+    }
+
+    @Test
+    public void extractOperator() {
+        Map<String, String[]> parameters = new LinkedHashMap<>();
+        parameters.put("anywhere", new String[]{""});
+        parameters.put("operator", new String[]{"OR"});
+
+        SearchState searchState = searchStateFactory.createSearchState(parameters);
+        assertEquals("OR", searchState.getSearchTermOperator());
+    }
 }