Skip to content

Commit

Permalink
Revert "CMR-6745: fix errors thrown with ORed granule counts (#1308)"
Browse files Browse the repository at this point in the history 
This reverts commit 5016e6d.
  • Loading branch information
@ygliuvt @jwteague
ygliuvt authored and jwteague committed Sep 9, 2021
1 parent e8bb056 commit 176c3d5
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 114 deletions.
2 changes: 0 additions & 2 deletions dev-system/.gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,3 @@ build.xml
/resources/elasticsearch/es_libs
dev/no_commit.clj
profiles.clj
.rebl
.calva
31 changes: 14 additions & 17 deletions search-app/src/cmr/search/services/query_execution/granule_counts_results_feature.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,16 @@

(defn- extract-spatial-operator
"Determine whether spatial conditions should be ORed or ANDed together"
[context]
(if (empty? (:query-string context))
; If there's no query string, we can safely assume that there is no
; options[spatial][or] parameter being passed
:and
(let [query-string (:query-string context)
parameters (string/split query-string #"\?|&")
spatial-param-regex #"(options%5Bspatial%5D%5Bor%5D=true|options\[spatial\]\[or\]=true)"
spatial-or-option? (some #(re-matches spatial-param-regex %) parameters)]
(if spatial-or-option?
:or
:and))))
[query]
(let [conditions (get-in query [:condition :conditions])
condition-is-shape? (->> conditions
(map keys)
flatten
(filter #(= % :shape))
seq)
parent-group-operator (get-in query [:condition :operation])]
(when condition-is-shape?
parent-group-operator)))

(defn- combine-spatial-and-non-spatial-conditions
"Helper function. If spatial conitions are ORed, combine them with the rest of
Expand All @@ -113,9 +111,9 @@
"Extracts a query to find the number of granules per collection in the results from a collection query
coll-query - The collection query
results - the results of the collection query"
[context coll-query results]
[coll-query results]
(let [collection-ids (query-results->concept-ids results)
spatial-operator (extract-spatial-operator context)
spatial-operator (extract-spatial-operator coll-query)
spatial-temp-conds (->> coll-query
extract-spatial-and-temporal-conditions
(map convert-spatial-or-temporal-condition))
Expand All @@ -134,8 +132,7 @@
;; The results were empty so the granule count query doesn't need to find anything.
q/match-none)]
(q/query {:concept-type :granule
:condition (if (and (= :or spatial-operator)
(seq combined-conditions))
:condition (if (= :or spatial-operator)
combined-conditions
condition)
;; We don't need any results
Expand All @@ -160,7 +157,7 @@
(if (zero? (count (query-results->concept-ids query-results)))
query-results
(->> query-results
(extract-granule-count-query context query)
(extract-granule-count-query query)
(query-execution/execute-query context)
search-results->granule-counts
(assoc query-results :granule-counts-map))))
15 changes: 6 additions & 9 deletions search-app/test/cmr/search/test/services/query_execution/granule_counts_results_feature.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,15 @@
(and-conds (spatial-cond 2)
(and-conds (temporal-cond 2)
(other))))})
results (results-with-items "C1-PROV1" "C2-PROV1")
context {:query-string "?include_granule_counts=true&concept_id=C1-PROV1"}]
results (results-with-items "C1-PROV1" "C2-PROV1")]
(is (= (expected-query-with-condition
2 (and-conds
(cqm/string-conditions :collection-concept-id ["C1-PROV1" "C2-PROV1"] true)
(spatial-cond 1)
(not-limit-to-granules (temporal-cond 1))
(spatial-cond 2)
(not-limit-to-granules (temporal-cond 2))))
(gcrf/extract-granule-count-query context coll-query results)))))
(gcrf/extract-granule-count-query coll-query results)))))
(testing "spatial and temporal query with no results"
(let [coll-query (cqm/query {:condition
(and-conds (other)
Expand All @@ -68,14 +67,12 @@
(and-conds (spatial-cond 2)
(and-conds (temporal-cond 2)
(other))))})
results (results-with-items)
context {:query-string "?include_granule_counts=true&concept_id=C1-PROV1"}]
results (results-with-items)]
(is (= (expected-query-with-condition 0 cqm/match-none)
(gcrf/extract-granule-count-query context coll-query results)))))
(gcrf/extract-granule-count-query coll-query results)))))
(testing "non-spatial non-temporal query"
(let [coll-query (cqm/query {:condition (and-conds (other) (other))})
results (results-with-items "C1-PROV1" "C2-PROV1")
context {:query-string "?include_granule_counts=true&concept_id=C1-PROV1"}]
results (results-with-items "C1-PROV1" "C2-PROV1")]
(is (= (expected-query-with-condition
2 (cqm/string-conditions :collection-concept-id ["C1-PROV1" "C2-PROV1"] true))
(gcrf/extract-granule-count-query context coll-query results))))))
(gcrf/extract-granule-count-query coll-query results))))))
87 changes: 1 addition & 86 deletions system-int-test/test/cmr/system_int_test/search/granule_counts_search_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
[cmr.spatial.codec :as codec]
[cmr.spatial.mbr :as m]
[cmr.spatial.point :as p]
[cmr.spatial.polygon :as poly]
[cmr.system-int-test.data2.collection :as dc]
[cmr.mock-echo.client.echo-util :as e]
[cmr.system-int-test.system :as s]
Expand All @@ -18,8 +17,7 @@
[cmr.system-int-test.utils.index-util :as index]
[cmr.system-int-test.utils.ingest-util :as ingest]
[cmr.system-int-test.utils.search-util :as search]
[cmr.system-int-test.utils.tag-util :as tags]
[cmr.umm.umm-spatial :as umm-spatial]))
[cmr.system-int-test.utils.tag-util :as tags]))

(use-fixtures :each (join-fixtures
[(ingest/reset-fixture {"provguid1" "PROV1"})
Expand Down Expand Up @@ -53,12 +51,6 @@
other-attribs)]
(d/ingest "PROV1" (dc/collection coll-attribs)))))

(defn- polygon
"Creates a single ring polygon with the given ordinates. Points must be in counter clockwise order.
The polygon will be closed automatically."
[& ords]
(poly/polygon [(apply umm-spatial/ords->ring ords)]))

(defn make-gran
"Creates a granule using a shape for the spatial metadata."
[coll shape temporal-attribs]
Expand Down Expand Up @@ -319,83 +311,6 @@
(is (= (set (map :concept-id [coll1 coll3 coll4 coll5 coll6 orbit-coll]))
(set (map :concept-id (:items results))))))))))

(deftest CMR-6745-ORed-spatial-search-granule-count
(let [no-match-temporal {:beginning-date-time "1990-01-01T00:00:00"
:ending-date-time "1991-01-01T00:00:00"}
coll6745 (make-coll 6745 m/whole-world no-match-temporal
{:science-keywords
[(dc/science-keyword {:category "Tornado"
:topic "Wind"
:term "Speed"})]})
search-polygon ["-21.33069,80.92296,-24.68258,80.58223,-26.80316,80.36716,-29.06056,79.60629,-24.34055,78.86559,-17.56837,79.10088,-14.62691,79.83818,-14.83213,80.79254,-21.33069,80.92296"
"136.75804,-34.82121,135.13466,-34.7865,134.43289,-35.0361,133.97631,-35.53991,134.6189,-36.7958,136.56358,-36.90405,137.07088,-36.15672,136.75804,-34.82121"]]
(make-gran
coll6745
(polygon 136.11192342 -36.17110948
136.18665788 -35.90588352
136.37132055 -35.23530856
136.09904081 -35.23808754
136.11192342 -36.17110948)
nil)
(make-gran
coll6745
(polygon 134.99977994 -35.3312641
136.20781537 -35.32523074
136.19344454 -34.33530605
134.99978255 -34.34112235
134.99977994 -35.3312641)
nil)
(make-gran
coll6745
(polygon 136.10023295 -35.32625771
136.34706996 -35.32375924
136.61794555 -34.33043224
136.08714151 -34.33629608
136.10023295 -35.32625771)
nil)

;; This granule should not be found
(make-gran
coll6745
(polygon 134.9997868 -32.62558044
136.1701524 -32.6201286
136.15757909 -31.62975815
134.99978909 -31.63500577
134.9997868 -32.62558044)
nil)

(index/wait-until-indexed)

;; Refresh the aggregate cache so that it includes all the granules that were added.
(index/full-refresh-collection-granule-aggregate-cache)
;; Reindex all the collections to get the latest information.
(ingest/reindex-all-collections)
(index/wait-until-indexed)

(testing "ORed granule counts special case"
(let [coll6745-id (:concept-id coll6745)
refs (search/find-refs :collection {:include-granule-counts true
:concept-id coll6745-id
:polygon search-polygon
"options[spatial][or]" "true"})]
(is (gran-counts/granule-counts-match? :xml {coll6745 3} refs))))

(testing "ANDed granule counts special case"
(let [coll6745-id (:concept-id coll6745)
refs (search/find-refs :collection {:include-granule-counts true
:concept-id coll6745-id
:polygon search-polygon
"options[spatial][or]" "false"})]
(is (gran-counts/granule-counts-match? :xml {coll6745 0} refs))))

(testing "EDSC Pageload error case"
(let [refs (search/find-refs :collection {:include-granule-counts true
:has-granules-or-cwic true
"options[science_keywords_h][or]" "true"
"options[spatial][or]" "true"
"options[temporal][limit_to_granules]" "true"})]
(is (gran-counts/granule-counts-match? :xml {coll6745 4} refs))))))

(deftest collection-has-granules-caching-test
(let [;; Create collections
;; whole world, no temporal, and science keywords
Expand Down

0 comments on commit 176c3d5

Please sign in to comment.