CMR-10886: Granule counts are different for certain shapefiles when comparing include_granule_counts with the collections endpoint vs the granules endpoint (#2348)

* CMR-10886: adds tests for granule count of multipoly

* CMR-10886: refactors multipolygon es cond handling

* CMR-10886: adds unit tests for extract-spatial-conditions and helper

* CMR-10886: handles duplicate poly edge case in spatial extraction function

* CMR-10886: addresses some PR comments

Author: daniel-zamora

search-app/src/cmr/search/services/query_execution/granule_counts_results_feature.clj

@@ -63,10 +63,39 @@
         (validate-path-to-condition query condition-path)
         true))))
 
+(defn- is-spatial-or-group?
+  "Returns true if the condition is an OR ConditionGroup containing only SpatialConditions.
+  This identifies groups created by MultiPolygon/multi-feature shapefiles that should preserve
+  their OR semantics."
+  [condition]
+  (and (instance? ConditionGroup condition)
+       (= :or (:operation condition))
+       (seq (:conditions condition))
+       (every? #(instance? SpatialCondition %) (:conditions condition))))
+
 (defn extract-spatial-conditions
-  "Returns a list of the spatial conditions in the query."
+  "Returns a list of the spatial conditions in the query. If the conditions are wrapped
+  in an OR group (e.g., from a MultiPolygon shapefile), preserves that structure by
+  returning the entire group. Individual SpatialConditions not in such groups are also extracted."
   [query]
-  (extract-conditions query SpatialCondition))
+  ;; Step 1: Extract OR groups containing only SpatialConditions
+  (let [spatial-or-groups (condition-extractor/extract-conditions
+                           query
+                           (fn [condition-path condition]
+                             (when (is-spatial-or-group? condition)
+                               (validate-path-to-condition query condition-path)
+                               true)))
+
+        ;; Step 2: Extract individual SpatialConditions whose parent is NOT a spatial OR group
+        ungrouped-spatial-conds (condition-extractor/extract-conditions
+                                 query
+                                 (fn [condition-path condition]
+                                   (when (and (instance? SpatialCondition condition)
+                                              (not (is-spatial-or-group? (last condition-path))))
+                                     (validate-path-to-condition query condition-path)
+                                     true)))]
+
+    (concat spatial-or-groups ungrouped-spatial-conds)))
 
 (defn- sanitize-temporal-condition
   "Returns the temporal condition with collection related info cleared out."

search-app/test/cmr/search/test/unit/services/query_execution/granule_counts_results_feature_test.clj

@@ -0,0 +1,180 @@
+(ns cmr.search.test.unit.services.query-execution.granule-counts-results-feature-test
+  (:require
+   [clojure.test :refer :all]
+   [cmr.common.services.search.query-model :as q]
+   [cmr.search.models.query :as qm]
+   [cmr.search.services.query-execution.granule-counts-results-feature :as gcrf]
+   [cmr.spatial.point :as p]
+   [cmr.spatial.polygon :as poly]
+   [cmr.spatial.geodetic-ring :as gr]
+   [cmr.elastic-utils.search.es-group-query-conditions :as gc]))
+
+(defn- make-spatial-condition
+  "Creates a SpatialCondition with a simple polygon"
+  [lon lat]
+  (let [points [(p/point lon lat)
+                (p/point (+ lon 10) lat)
+                (p/point (+ lon 10) (+ lat 10))
+                (p/point lon (+ lat 10))
+                (p/point lon lat)]
+        ring (gr/ring points)
+        polygon (poly/polygon :geodetic [ring])]
+    (qm/->SpatialCondition polygon)))
+
+(deftest is-spatial-or-group-test
+  (testing "Returns true for OR group of SpatialConditions"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          or-group (gc/or-conds [cond1 cond2])]
+      (is (true? (#'gcrf/is-spatial-or-group? or-group)))))
+
+  (testing "Returns false for AND group of SpatialConditions"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          and-group (gc/and-conds [cond1 cond2])]
+      (is (false? (#'gcrf/is-spatial-or-group? and-group)))))
+
+  (testing "Returns false for OR group with non-SpatialConditions"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (q/string-condition :entry-title "test")
+          or-group (gc/or-conds [cond1 cond2])]
+      (is (false? (#'gcrf/is-spatial-or-group? or-group)))))
+
+  (testing "Returns false for single SpatialCondition"
+    (let [cond1 (make-spatial-condition 10 0)]
+      (is (false? (#'gcrf/is-spatial-or-group? cond1))))))
+
+(deftest extract-spatial-conditions-preserves-or-groups-test
+  (testing "Extracts OR group of SpatialConditions preserving structure"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          or-group (gc/or-conds [cond1 cond2])
+          query (q/query {:concept-type :collection
+                          :condition or-group})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [or-group] result))))
+
+  (testing "Extracts OR group nested with non-spatial conditions"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          or-group (gc/or-conds [cond1 cond2])
+          title-cond (q/string-condition :entry-title "test")
+          provider-cond (q/string-condition :provider "PROV1")
+          combined (gc/and-conds [title-cond or-group provider-cond])
+          query (q/query {:concept-type :collection
+                          :condition combined})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [or-group] result)))))
+
+(deftest extract-spatial-conditions-single-condition-test
+  (testing "Extracts single SpatialCondition"
+    (let [cond1 (make-spatial-condition 10 0)
+          query (q/query {:concept-type :collection
+                          :condition cond1})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [cond1] result))))
+
+  (testing "Extracts single SpatialCondition nested with non-spatial conditions"
+    (let [spatial-cond (make-spatial-condition 10 0)
+          title-cond (q/string-condition :entry-title "test")
+          provider-cond (q/string-condition :provider "PROV1")
+          combined (gc/and-conds [title-cond spatial-cond provider-cond])
+          query (q/query {:concept-type :collection
+                          :condition combined})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [spatial-cond] result)))))
+
+(deftest extract-spatial-conditions-mixed-test
+  (testing "Extracts both OR groups and ungrouped SpatialConditions"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          cond3 (make-spatial-condition 100 50)
+          or-group (gc/or-conds [cond1 cond2])
+          combined (gc/and-conds [or-group cond3])
+          query (q/query {:concept-type :collection
+                          :condition combined})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [or-group cond3] result))))
+
+  (testing "Extracts OR groups and ungrouped conditions with non-spatial conditions mixed in"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          cond3 (make-spatial-condition 100 50)
+          or-group (gc/or-conds [cond1 cond2])
+          title-cond (q/string-condition :entry-title "test")
+          provider-cond (q/string-condition :provider "PROV1")
+          combined (gc/and-conds [title-cond or-group provider-cond cond3])
+          query (q/query {:concept-type :collection
+                          :condition combined})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [or-group cond3] result)))))
+
+(deftest extract-spatial-conditions-empty-test
+  (testing "Returns empty list when no spatial conditions"
+    (let [cond1 (q/string-condition :entry-title "test")
+          query (q/query {:concept-type :collection
+                          :condition cond1})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [] result))))
+
+  (testing "Returns empty list when only non-spatial conditions"
+    (let [title-cond (q/string-condition :entry-title "test")
+          provider-cond (q/string-condition :provider "PROV1")
+          combined (gc/and-conds [title-cond provider-cond])
+          query (q/query {:concept-type :collection
+                          :condition combined})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [] result)))))
+
+(deftest extract-spatial-conditions-with-duplicate-polygon-test
+  (testing "Extracts both OR group and standalone duplicate SpatialCondition"
+    (let [cond1 (make-spatial-condition 10 0)
+          cond2 (make-spatial-condition 44 -25)
+          cond3 (make-spatial-condition 10 0)
+          or-group (gc/or-conds [cond1 cond2])
+          combined (gc/and-conds [or-group cond3])
+          query (q/query {:concept-type :collection
+                          :condition combined})
+          result (gcrf/extract-spatial-conditions query)]
+      (is (= [or-group cond3] result)))))
+
+(deftest extract-spatial-conditions-validates-parent-path-test
+  (testing "Rejects SpatialCondition under NegatedCondition"
+    (let [spatial-cond (make-spatial-condition 10 0)
+          negated (q/negated-condition spatial-cond)
+          query (q/query {:concept-type :collection
+                          :condition negated})]
+      (is (thrown? Exception
+                   (gcrf/extract-spatial-conditions query)))))
+
+  (testing "Rejects SpatialCondition nested under NegatedCondition in AND group"
+    (let [spatial-cond (make-spatial-condition 10 0)
+          title-cond (q/string-condition :entry-title "test")
+          negated (q/negated-condition spatial-cond)
+          combined (gc/and-conds [title-cond negated])
+          query (q/query {:concept-type :collection
+                          :condition combined})]
+      (is (thrown? Exception
+                   (gcrf/extract-spatial-conditions query)))))
+
+  (testing "Accepts valid SpatialCondition in AND group"
+    (let [spatial-cond (make-spatial-condition 10 0)
+          title-cond (q/string-condition :entry-title "test")
+          combined (gc/and-conds [title-cond spatial-cond])
+          query (q/query {:concept-type :collection
+                          :condition combined})]
+      (is (seq (gcrf/extract-spatial-conditions query)))))
+
+  (testing "Accepts valid SpatialCondition at top level"
+    (let [spatial-cond (make-spatial-condition 10 0)
+          query (q/query {:concept-type :collection
+                          :condition spatial-cond})]
+      (is (seq (gcrf/extract-spatial-conditions query)))))
+
+  (testing "Preserves MultiPolygon OR groups with validation"
+    (let [spatial-cond-1 (make-spatial-condition 10 0)
+          spatial-cond-2 (make-spatial-condition 20 10)
+          or-group (gc/or-conds [spatial-cond-1 spatial-cond-2])
+          query (q/query {:concept-type :collection
+                          :condition or-group})]
+      (is (= [or-group] (gcrf/extract-spatial-conditions query))))))

system-int-test/resources/shapefiles/multipolygon_test.geojson

@@ -0,0 +1,32 @@
+{
+  "type": "FeatureCollection",
+  "features": [
+    {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "MultiPolygon",
+        "coordinates": [
+          [
+            [
+              [10.0, 0.0],
+              [30.0, 0.0],
+              [30.0, 15.0],
+              [10.0, 15.0],
+              [10.0, 0.0]
+            ]
+          ],
+          [
+            [
+              [44.0, -25.0],
+              [50.0, -25.0],
+              [50.0, -15.0],
+              [44.0, -15.0],
+              [44.0, -25.0]
+            ]
+          ]
+        ]
+      }
+    }
+  ]
+}

system-int-test/test/cmr/system_int_test/search/granule/granule_counts_search_test.clj

@@ -1,8 +1,12 @@
 (ns cmr.system-int-test.search.granule.granule-counts-search-test
   "This tests the granule counts search feature which allows retrieving counts of granules per collection."
   (:require
+   [clojure.java.io :as io]
    [clojure.test :refer [are deftest is testing use-fixtures]]
+   [cmr.common.mime-types :as mt]
    [cmr.common.util :as util :refer [are3]]
+   [cmr.common-app.test.side-api :as side]
+   [cmr.search.services.parameters.converters.shapefile :as shapefile]
    [cmr.spatial.codec :as codec]
    [cmr.spatial.mbr :as m]
    [cmr.spatial.point :as p]
@@ -749,3 +753,42 @@
 
               "granule count in json format"
               :atom (search/find-concepts-json :collection {:include-granule-counts false})))))
+
+(deftest multipolygon-shapefile-granule-counts-test
+  (side/eval-form `(shapefile/set-enable-shapefile-parameter-flag! true))
+
+  (let [coll (make-coll 1 m/whole-world nil)]
+    ;; Granule in first polygon of MultiPolygon
+    (make-gran coll (p/point 20.0 5.0) nil)
+    ;; Granule in second polygon of MultiPolygon
+    (make-gran coll (p/point 47.0 -20.0) nil)
+    ;; Granule outside MultiPolygon
+    (make-gran coll (p/point -100.0 40.0) nil)
+
+    (index/wait-until-indexed)
+    (index/full-refresh-collection-granule-aggregate-cache)
+    (ingest/reindex-all-collections)
+    (index/wait-until-indexed)
+
+    (testing "MultiPolygon shapefile granule counts match direct granule search"
+      (let [granule-results (search/find-refs-with-multi-part-form-post
+                              :granule
+                              [{:name "shapefile"
+                                :content (io/file (io/resource "shapefiles/multipolygon_test.geojson"))
+                                :mime-type mt/geojson}
+                               {:name "collection_concept_id"
+                                :content (:concept-id coll)}])
+            granule-hits (:hits granule-results)]
+        (is (= 2 granule-hits) "Granule search should find 2 granules in MultiPolygon")
+
+        (let [collection-response (search/find-refs-with-multi-part-form-post
+                                    :collection
+                                    [{:name "shapefile"
+                                      :content (io/file (io/resource "shapefiles/multipolygon_test.geojson"))
+                                      :mime-type mt/geojson}
+                                     {:name "concept_id"
+                                      :content (:concept-id coll)}
+                                     {:name "include_granule_counts"
+                                      :content "true"}])]
+          (is (gran-counts/granule-counts-match? :xml {coll granule-hits} collection-response)
+              "MultiPolygon shapefile: collection granule counts should match granule search hits"))))))