From a2880d7dc9e62a8e28a96c426b599cd04e51f8a2 Mon Sep 17 00:00:00 2001 From: Nick dos Remedios Date: Thu, 23 Jan 2025 17:31:26 +1100 Subject: [PATCH 1/6] #940 Fix for qid query error --- .../ala/biocache/util/QueryFormatUtils.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java index 85e35dbfa..d16e6a272 100644 --- a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java +++ b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java @@ -136,10 +136,11 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for Map activeFacetMap = new HashMap(); Map> activeFacetObj = new HashMap<>(); Map[] fqMaps = {activeFacetMap, activeFacetObj}; + Boolean isIncludeUnfilteredFacetValues = searchParams.getIncludeUnfilteredFacetValues(); //Only format the query if it doesn't already supply a formattedQuery. if (forceQueryFormat || StringUtils.isEmpty(searchParams.getFormattedQuery())) { - String[] originalFqs = Arrays.copyOf(searchParams.getFq(), searchParams.getFq().length); // copy by value + String[] originalFqs = searchParams.getFq(); String [] formatted = formatQueryTerm(searchParams.getQ(), searchParams); searchParams.setDisplayString(formatted[0]); @@ -149,7 +150,7 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for searchParams.setFormattedFq(null); // Apply filter tagging and excluding filters if flag is set - if (searchParams.getIncludeUnfilteredFacetValues()) { + if (isIncludeUnfilteredFacetValues) { applyFilterTagging(searchParams); } @@ -157,7 +158,7 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for if (searchParams.getFq() != null) { for (int i = 0; i < searchParams.getFq().length; i++) { String fq = searchParams.getFq()[i]; - String fqOriginal = originalFqs[i]; // not altered by `applyFilterTagging()` + String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : null; // not altered by `applyFilterTagging()` if (fq != null && !fq.isEmpty()) { formatted = formatQueryTerm(fq, searchParams); @@ -167,11 +168,11 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for addFormattedFq(new String[]{formatted[1]}, searchParams); } - //add to activeFacetMap fqs that are not inserted by a qid, and the q of qids in fqs. - //do not add spatial fields - if (originalFqs != null && i < originalFqs.length && !formatted[1].contains(spatialField + ":")) { + // add to activeFacetMap fqs that are not inserted by a qid, and the q of qids in fqs. + // do not add spatial fields + if (originalFqs != null && originalFqs.length > i && !formatted[1].contains(spatialField + ":")) { Facet facet = new Facet(); - facet.setDisplayName(formattedOriginal[0]); + facet.setDisplayName(isIncludeUnfilteredFacetValues ? formattedOriginal[0] : formatted[0]); String[] fv = fqOriginal.split(":"); if (fv.length >= 2) { facet.setName(fv[0]); @@ -184,7 +185,8 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for // activeFacetObj which is [StringKey: List] String fqKey = parseFQ(fqOriginal); if (fqKey != null) { - Facet fct = new Facet(fqKey, formattedOriginal[0]); // display name is the formatted name, for example '11' to 'November' + String formattedFacet = (isIncludeUnfilteredFacetValues) ? formattedOriginal[0] : formatted[0]; + Facet fct = new Facet(fqKey, formattedFacet); // display name is the formatted name, for example '11' to 'November' fct.setValue(fqOriginal); // value in activeFacetMap is the part with key replaced by '', but here is the original fq because front end will need it List valList = activeFacetObj.getOrDefault(fqKey, new ArrayList<>()); valList.add(fct); From d768db8ff6c19b143b43f9a8f6b0e2c06d525991 Mon Sep 17 00:00:00 2001 From: Nick dos Remedios Date: Fri, 24 Jan 2025 09:17:15 +1100 Subject: [PATCH 2/6] #941 Revert change not needed Check not required with other changes in PR --- src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java index d16e6a272..7014d370d 100644 --- a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java +++ b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java @@ -158,7 +158,7 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for if (searchParams.getFq() != null) { for (int i = 0; i < searchParams.getFq().length; i++) { String fq = searchParams.getFq()[i]; - String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : null; // not altered by `applyFilterTagging()` + String fqOriginal = originalFqs[i]; // not altered by `applyFilterTagging()` if (fq != null && !fq.isEmpty()) { formatted = formatQueryTerm(fq, searchParams); From 880138b9546988337bd8cc28bc8e230c6131480b Mon Sep 17 00:00:00 2001 From: Nick dos Remedios Date: Fri, 24 Jan 2025 09:20:55 +1100 Subject: [PATCH 3/6] Revert "#941 Revert change not needed" This reverts commit d768db8ff6c19b143b43f9a8f6b0e2c06d525991. --- src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java index 7014d370d..d16e6a272 100644 --- a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java +++ b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java @@ -158,7 +158,7 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for if (searchParams.getFq() != null) { for (int i = 0; i < searchParams.getFq().length; i++) { String fq = searchParams.getFq()[i]; - String fqOriginal = originalFqs[i]; // not altered by `applyFilterTagging()` + String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : null; // not altered by `applyFilterTagging()` if (fq != null && !fq.isEmpty()) { formatted = formatQueryTerm(fq, searchParams); From 6f1d0cc631e6480463a60a370e33af0a3df2271d Mon Sep 17 00:00:00 2001 From: Nick dos Remedios Date: Fri, 24 Jan 2025 10:00:19 +1100 Subject: [PATCH 4/6] #941 replace null with empty string Based on SearchRequestDto stating `fq` must not be null `protected String[] fq = {}; // must not be null`. Probably overkill but seems slightly safer. --- src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java index d16e6a272..e0353c507 100644 --- a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java +++ b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java @@ -158,7 +158,7 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for if (searchParams.getFq() != null) { for (int i = 0; i < searchParams.getFq().length; i++) { String fq = searchParams.getFq()[i]; - String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : null; // not altered by `applyFilterTagging()` + String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : ""; // not altered by `applyFilterTagging()` if (fq != null && !fq.isEmpty()) { formatted = formatQueryTerm(fq, searchParams); From 7da48521ef36e81763a88efe1e5d46259c0c4e8c Mon Sep 17 00:00:00 2001 From: Nick dos Remedios Date: Fri, 24 Jan 2025 12:12:02 +1100 Subject: [PATCH 5/6] #941 Code review fix and unit tests --- .../ala/biocache/util/QueryFormatUtils.java | 2 +- .../biocache/util/QueryFormatUtilsSpec.groovy | 38 ++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java index e0353c507..35bde6431 100644 --- a/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java +++ b/src/main/java/au/org/ala/biocache/util/QueryFormatUtils.java @@ -158,7 +158,7 @@ public Map[] formatSearchQuery(SpatialSearchRequestDTO searchParams, boolean for if (searchParams.getFq() != null) { for (int i = 0; i < searchParams.getFq().length; i++) { String fq = searchParams.getFq()[i]; - String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : ""; // not altered by `applyFilterTagging()` + String fqOriginal = (originalFqs.length > i) ? originalFqs[i] : fq; // not altered by `applyFilterTagging()` if (fq != null && !fq.isEmpty()) { formatted = formatQueryTerm(fq, searchParams); diff --git a/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy b/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy index c7ea46be3..0fc7536bc 100644 --- a/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy +++ b/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy @@ -35,6 +35,9 @@ class QueryFormatUtilsSpec extends Specification { queryFormatUtils.fieldMappingUtil = Mock(FieldMappingUtil) { translateQueryFields(_ as String) >> { String query -> return query } } + queryFormatUtils.qidCacheDao = Mock(QidCacheDAO) { + get(_ as String) >> { String qid -> new Qid(q: "scientificName:TestTaxon", fqs: new String[0]) } + } queryFormatUtils.authService = authService } @@ -157,8 +160,6 @@ class QueryFormatUtilsSpec extends Specification { searchParams.getPivotFacets() == new String[]{} } - - def "test formatSearchQuery with tagging and excluded facets"() { given: SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() @@ -182,6 +183,39 @@ class QueryFormatUtilsSpec extends Specification { searchParams.getPivotFacets() == new String[]{"{!ex=month}month", "{!ex=year}year"} } + def "test formatSearchQuery with qid query"() { + given: + SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() + searchParams.setQ("qid:123456") + searchParams.setFacet(true) + when: + def result = queryFormatUtils.formatSearchQuery(searchParams, false) + + then: + result[0].size() == 0 + result[1].size() == 0 + searchParams.getFormattedQuery() == "scientificName:TestTaxon" + searchParams.getDisplayString() == "scientificName:TestTaxon" + } + + def "test formatSearchQuery with qid fq"() { + given: + SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() + searchParams.setQ("*:*") + searchParams.setFq(new String[]{"qid:123456"}) + searchParams.setFacet(true) + when: + def result = queryFormatUtils.formatSearchQuery(searchParams, false) + + then: + result[0].size() == 1 + result[1].size() == 1 + searchParams.getFormattedQuery() == "*:*" + searchParams.getDisplayString() == "[all records]" + searchParams.getFq() == new String[]{"qid:123456"} + searchParams.getFormattedFq() == new String[]{"scientificName:TestTaxon"} + } + private static ObjectMapper om = new ObjectMapper() private static String getResultQuery(String uid) { From a68fbd8ee3f22bb419526b44af5f16245df55d02 Mon Sep 17 00:00:00 2001 From: Nick dos Remedios Date: Fri, 24 Jan 2025 12:43:19 +1100 Subject: [PATCH 6/6] #941 Fix for broken unit test --- .../biocache/util/QueryFormatUtilsSpec.groovy | 55 +++++++------------ 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy b/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy index 0fc7536bc..2b89a2a23 100644 --- a/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy +++ b/src/test/java/au/org/ala/biocache/util/QueryFormatUtilsSpec.groovy @@ -35,9 +35,6 @@ class QueryFormatUtilsSpec extends Specification { queryFormatUtils.fieldMappingUtil = Mock(FieldMappingUtil) { translateQueryFields(_ as String) >> { String query -> return query } } - queryFormatUtils.qidCacheDao = Mock(QidCacheDAO) { - get(_ as String) >> { String qid -> new Qid(q: "scientificName:TestTaxon", fqs: new String[0]) } - } queryFormatUtils.authService = authService } @@ -137,6 +134,25 @@ class QueryFormatUtilsSpec extends Specification { searchParams.getDisplayString() == "scientificName:Test" } + def "test formatSearchQuery with qid fq"() { + given: + SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() + searchParams.setQ("*:*") + searchParams.setFq(new String[]{"qid:123456"}) + qidCacheDao.get(_) >> new Qid(q: "scientificName:TestTaxon", fqs: new String[0]) + searchParams.setFacet(true) + when: + def result = queryFormatUtils.formatSearchQuery(searchParams, false) + + then: + result[0].size() == 1 + result[1].size() == 1 + searchParams.getFormattedQuery() == "*:*" + searchParams.getDisplayString() == "[all records]" + searchParams.getFq() == new String[]{"qid:123456"} + searchParams.getFormattedFq() == new String[]{"scientificName:TestTaxon"} + } + def "test formatSearchQuery with facets"() { given: SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() @@ -183,39 +199,6 @@ class QueryFormatUtilsSpec extends Specification { searchParams.getPivotFacets() == new String[]{"{!ex=month}month", "{!ex=year}year"} } - def "test formatSearchQuery with qid query"() { - given: - SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() - searchParams.setQ("qid:123456") - searchParams.setFacet(true) - when: - def result = queryFormatUtils.formatSearchQuery(searchParams, false) - - then: - result[0].size() == 0 - result[1].size() == 0 - searchParams.getFormattedQuery() == "scientificName:TestTaxon" - searchParams.getDisplayString() == "scientificName:TestTaxon" - } - - def "test formatSearchQuery with qid fq"() { - given: - SpatialSearchRequestDTO searchParams = new SpatialSearchRequestDTO() - searchParams.setQ("*:*") - searchParams.setFq(new String[]{"qid:123456"}) - searchParams.setFacet(true) - when: - def result = queryFormatUtils.formatSearchQuery(searchParams, false) - - then: - result[0].size() == 1 - result[1].size() == 1 - searchParams.getFormattedQuery() == "*:*" - searchParams.getDisplayString() == "[all records]" - searchParams.getFq() == new String[]{"qid:123456"} - searchParams.getFormattedFq() == new String[]{"scientificName:TestTaxon"} - } - private static ObjectMapper om = new ObjectMapper() private static String getResultQuery(String uid) {