Skip to content

CMR-10528: As a CMR Search API user I only want to see active collections in my search results. #2385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daniel-zamora merged 1 commit into from
Feb 24, 2026
Merged

CMR-10528: As a CMR Search API user I only want to see active collections in my search results. #2385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions indexer-app/src/cmr/indexer/data/concepts/collection.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@
"LOW_LATENCY" "3 to 24 hours"
"EXPEDITED" "1 to 4 days"
nil)
collection-progress (:CollectionProgress collection)
collection-data-type (if (= "NEAR_REAL_TIME" collection-data-type)
;; add in all the aliases for NEAR_REAL_TIME
(concat [collection-data-type] keyword-util/nrt-aliases)
Expand Down Expand Up @@ -397,8 +398,10 @@
(if (sequential? collection-data-type)
(map string/lower-case collection-data-type)
(string/lower-case collection-data-type)))
:collection-progress (:CollectionProgress collection)
:collection-progress-lowercase (util/safe-lowercase (:CollectionProgress collection))
:collection-progress collection-progress
:collection-progress-lowercase (util/safe-lowercase collection-progress)
:collection-progress-active (not (contains? #{"PLANNED" "DEPRECATED" "PREPRINT" "INREVIEW"}
collection-progress))
:platform-sn platform-short-names
:platform-sn-lowercase (map string/lower-case platform-short-names)
;; hierarchical fields
Expand Down
1 change: 1 addition & 0 deletions indexer-app/src/cmr/indexer/data/index_set.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@
:collection-data-type-lowercase m/string-field-mapping
:collection-progress m/string-field-mapping
:collection-progress-lowercase m/string-field-mapping
:collection-progress-active m/bool-field-mapping

;; Temporal date range
:start-date m/date-field-mapping
Expand Down
10 changes: 10 additions & 0 deletions search-app/src/cmr/search/config.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(ns cmr.search.config
(:require [cmr.common.config :refer [defconfig]]))

(defconfig enable-non-operational-collection-filter
"When true, collection searches will by default exclude non-operational
collections (those with CollectionProgress of PLANNED, DEPRECATED, PREPRINT,
or INREVIEW) unless the caller explicitly provides a collection-progress
parameter or passes include-non-operational=true."
{:default false
:type Boolean})
20 changes: 18 additions & 2 deletions search-app/src/cmr/search/services/parameters/conversion.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
[cmr.common.util :as util]
[cmr.metadata-db.services.concept-service :as cs]
[cmr.search.models.query :as qm]
[cmr.search.services.parameters.legacy-parameters :as lp]))
[cmr.search.services.parameters.legacy-parameters :as lp]
[cmr.search.config :as config]))

;; Note: the suffix "-h" on parameters denotes the humanized version of a parameter

Expand Down Expand Up @@ -51,6 +52,7 @@
:has-opendap-url :boolean
:cloud-hosted :boolean
:standard-product :boolean
:include-non-operational :include-non-operational
:instrument :string
:instrument-h :humanizer
:keyword :keyword
Expand Down Expand Up @@ -268,6 +270,15 @@
[_ _ _ value _]
(cqm/text-condition :keyword (string/lower-case value)))

(defmethod common-params/parameter->condition :include-non-operational
[_context _concept-type _param value _options]
;; When include-non-operational=false, restrict results to active collections only.
;; When true or unset, return all collections.
(let [v (when value (string/lower-case value))]
(if (= "false" v)
(cqm/boolean-condition :collection-progress-active true)
cqm/match-all)))

(def collection-only-params
"List of parameters that are valid in collection query models, but not in granule query models."
(let [granule-param-mappings (common-params/param-mappings :granule)
Expand Down Expand Up @@ -519,7 +530,12 @@

(defmethod common-params/parse-query-level-params :collection
[_concept-type params]
(let [[params query-attribs] (common-params/default-parse-query-level-params
(let [params (if (and (config/enable-non-operational-collection-filter)
(not (contains? params :collection-progress))
(not (contains? params :include-non-operational)))
(assoc params :include-non-operational "false")
params)
[params query-attribs] (common-params/default-parse-query-level-params
:collection params lp/param-aliases)
query-attribs (reverse-has-granules-sort query-attribs)
{:keys [begin-tag end-tag snippet-length num-snippets]} (get-in params [:options :highlights])
Expand Down
3 changes: 2 additions & 1 deletion search-app/src/cmr/search/services/parameters/parameter_validation.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,8 @@
(let [bool-params (select-keys params [:downloadable :browsable :include-granule-counts
:include-has-granules :has-granules :hierarchical-facets
:include-highlights :all-revisions :has-opendap-url
:simplify-shapefile :cloud-hosted :standard-product])]
:simplify-shapefile :cloud-hosted :standard-product
:include-non-operational])]
(mapcat
(fn [[param value]]
(when-not (contains? #{"true" "false" "unset"} (when value (string/lower-case value)))
Expand Down
43 changes: 43 additions & 0 deletions ...m-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
(:require
[clojure.test :refer :all]
[cmr.common.util :as util :refer [are3]]
[cmr.common-app.test.side-api :as side]
[cmr.system-int-test.data2.core :as d]
[cmr.system-int-test.data2.umm-spec-collection :as data-umm-c]
[cmr.system-int-test.utils.index-util :as index]
Expand Down Expand Up @@ -46,3 +47,45 @@

"invalid value returns empty"
[] "INVALID" nil))))

(deftest search-collection-progress-active-filter
"Tests the non-operational collection filter feature flag behavior."
Comment on lines +51 to +52
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

deftest does not support docstrings — this string is silently discarded.

Unlike defn, Clojure's deftest macro doesn't attach a string literal as metadata. Line 52 is evaluated and thrown away. Use a comment instead.

Suggested fix 
-(deftest search-collection-progress-active-filter
-  "Tests the non-operational collection filter feature flag behavior."
+(deftest search-collection-progress-active-filter
+  ;; Tests the non-operational collection filter feature flag behavior.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(deftest search-collection-progress-active-filter
"Tests the non-operational collection filter feature flag behavior."
(deftest search-collection-progress-active-filter
;; Tests the non-operational collection filter feature flag behavior.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj`
around lines 51 - 52, The test named search-collection-progress-active-filter
currently has a string literal after deftest which is ignored; remove that
docstring and replace it with a comment or an inline comment above the deftest
so the description isn’t silently discarded — locate the deftest
search-collection-progress-active-filter and convert the quoted string into a
comment (or move the text into an ;; comment) immediately above the deftest
declaration.

(let [coll1 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 1 {:CollectionProgress "ACTIVE"}))
coll2 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 2 {:CollectionProgress "PLANNED"}))
coll3 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 3 {:CollectionProgress "COMPLETE"}))
coll4 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 4 {:CollectionProgress "DEPRECATED"}))
coll5 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 5 {:CollectionProgress "NOT PROVIDED"}))
coll6 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 6 {:CollectionProgress "PREPRINT"}))
coll7 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 7 {:CollectionProgress "INREVIEW"}))
coll8 (d/ingest-umm-spec-collection "PROV1" (data-umm-c/collection 8 {:CollectionProgress "SUPERSEDED"}))]

(index/wait-until-indexed)

(testing "flag OFF (default) - all 8 collections returned"
(d/refs-match? [coll1 coll2 coll3 coll4 coll5 coll6 coll7 coll8]
(search/find-refs :collection {})))

(side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! true))

(testing "flag ON, no params - only active collections returned (excludes PLANNED, DEPRECATED, PREPRINT, INREVIEW)"
(d/refs-match? [coll1 coll3 coll5 coll8]
(search/find-refs :collection {})))

(testing "flag ON + include-non-operational=true - all 8 collections returned"
(d/refs-match? [coll1 coll2 coll3 coll4 coll5 coll6 coll7 coll8]
(search/find-refs :collection {:include-non-operational "true"})))

(testing "flag ON + include-non-operational=false - only active collections returned"
(d/refs-match? [coll1 coll3 coll5 coll8]
(search/find-refs :collection {:include-non-operational "false"})))

(testing "flag ON + explicit collection-progress=PLANNED - PLANNED returned (no synthetic filter injected)"
(d/refs-match? [coll2]
(search/find-refs :collection {:collection-progress "PLANNED"})))

(testing "flag ON + collection-progress=PLANNED + include-non-operational=false - empty (PLANNED is non-operational)"
(d/refs-match? []
(search/find-refs :collection {:collection-progress "PLANNED"
:include-non-operational "false"})))

(side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! false))))
Comment on lines +68 to +91
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Feature flag not reset on test failure — can poison subsequent tests.

If any assertion between set-...-filter! true (line 68) and set-...-filter! false (line 91) throws an exception, the flag stays enabled and can cause cascading failures in later tests. Wrap in try/finally:

🛡️ Proposed fix 
     (side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! true))
 
-    (testing "flag ON, no params - only active collections returned (excludes PLANNED, DEPRECATED, PREPRINT, INREVIEW)"
-      (d/refs-match? [coll1 coll3 coll5 coll8]
-                     (search/find-refs :collection {})))
-
-    (testing "flag ON + include-non-operational=true - all 8 collections returned"
-      (d/refs-match? [coll1 coll2 coll3 coll4 coll5 coll6 coll7 coll8]
-                     (search/find-refs :collection {:include-non-operational "true"})))
-
-    (testing "flag ON + include-non-operational=false - only active collections returned"
-      (d/refs-match? [coll1 coll3 coll5 coll8]
-                     (search/find-refs :collection {:include-non-operational "false"})))
-
-    (testing "flag ON + explicit collection-progress=PLANNED - PLANNED returned (no synthetic filter injected)"
-      (d/refs-match? [coll2]
-                     (search/find-refs :collection {:collection-progress "PLANNED"})))
-
-    (testing "flag ON + collection-progress=PLANNED + include-non-operational=false - empty (PLANNED is non-operational)"
-      (d/refs-match? []
-                     (search/find-refs :collection {:collection-progress "PLANNED"
-                                                    :include-non-operational "false"})))
-
-    (side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! false))))
+    (try
+      (testing "flag ON, no params - only active collections returned (excludes PLANNED, DEPRECATED, PREPRINT, INREVIEW)"
+        (d/refs-match? [coll1 coll3 coll5 coll8]
+                       (search/find-refs :collection {})))
+
+      (testing "flag ON + include-non-operational=true - all 8 collections returned"
+        (d/refs-match? [coll1 coll2 coll3 coll4 coll5 coll6 coll7 coll8]
+                       (search/find-refs :collection {:include-non-operational "true"})))
+
+      (testing "flag ON + include-non-operational=false - only active collections returned"
+        (d/refs-match? [coll1 coll3 coll5 coll8]
+                       (search/find-refs :collection {:include-non-operational "false"})))
+
+      (testing "flag ON + explicit collection-progress=PLANNED - PLANNED returned (no synthetic filter injected)"
+        (d/refs-match? [coll2]
+                       (search/find-refs :collection {:collection-progress "PLANNED"})))
+
+      (testing "flag ON + collection-progress=PLANNED + include-non-operational=false - empty (PLANNED is non-operational)"
+        (d/refs-match? []
+                       (search/find-refs :collection {:collection-progress "PLANNED"
+                                                      :include-non-operational "false"})))
+      (finally
+        (side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! false))))))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! true))
(testing "flag ON, no params - only active collections returned (excludes PLANNED, DEPRECATED, PREPRINT, INREVIEW)"
(d/refs-match? [coll1 coll3 coll5 coll8]
(search/find-refs :collection {})))
(testing "flag ON + include-non-operational=true - all 8 collections returned"
(d/refs-match? [coll1 coll2 coll3 coll4 coll5 coll6 coll7 coll8]
(search/find-refs :collection {:include-non-operational "true"})))
(testing "flag ON + include-non-operational=false - only active collections returned"
(d/refs-match? [coll1 coll3 coll5 coll8]
(search/find-refs :collection {:include-non-operational "false"})))
(testing "flag ON + explicit collection-progress=PLANNED - PLANNED returned (no synthetic filter injected)"
(d/refs-match? [coll2]
(search/find-refs :collection {:collection-progress "PLANNED"})))
(testing "flag ON + collection-progress=PLANNED + include-non-operational=false - empty (PLANNED is non-operational)"
(d/refs-match? []
(search/find-refs :collection {:collection-progress "PLANNED"
:include-non-operational "false"})))
(side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! false))))
(side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! true))
(try
(testing "flag ON, no params - only active collections returned (excludes PLANNED, DEPRECATED, PREPRINT, INREVIEW)"
(d/refs-match? [coll1 coll3 coll5 coll8]
(search/find-refs :collection {})))
(testing "flag ON + include-non-operational=true - all 8 collections returned"
(d/refs-match? [coll1 coll2 coll3 coll4 coll5 coll6 coll7 coll8]
(search/find-refs :collection {:include-non-operational "true"})))
(testing "flag ON + include-non-operational=false - only active collections returned"
(d/refs-match? [coll1 coll3 coll5 coll8]
(search/find-refs :collection {:include-non-operational "false"})))
(testing "flag ON + explicit collection-progress=PLANNED - PLANNED returned (no synthetic filter injected)"
(d/refs-match? [coll2]
(search/find-refs :collection {:collection-progress "PLANNED"})))
(testing "flag ON + collection-progress=PLANNED + include-non-operational=false - empty (PLANNED is non-operational)"
(d/refs-match? []
(search/find-refs :collection {:collection-progress "PLANNED"
:include-non-operational "false"})))
(finally
(side/eval-form `(cmr.search.config/set-enable-non-operational-collection-filter! false))))))
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj`
around lines 68 - 91, The test sets the feature flag via
(cmr.search.config/set-enable-non-operational-collection-filter! true) but only
resets it at the end, so a failing assertion will leave the flag enabled; wrap
the block of assertions between the enable and disable calls in a try/finally so
the flag is always reset: call
(cmr.search.config/set-enable-non-operational-collection-filter! true), run the
tests (the testing forms that call search/find-refs), and in a finally call
(cmr.search.config/set-enable-non-operational-collection-filter! false) to
ensure the flag is cleared even on failure.

Copy link
Copy Markdown
Contributor Author

@daniel-zamora daniel-zamora Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other tests use this pattern, and if any exception or error is thrown by the apps its handled gracefull by the apps and the dev system itself remains running

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for similar patterns where side/eval-form is used to toggle feature flags
# Look for patterns where flags are set and then reset at the end without try/finally
rg -n -C5 'side/eval-form.*set-enable' --type=clj

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Also search for any try/finally patterns in test files to see if they're used
rg -n -C3 '\(try\b' system-int-test/test/ --type=clj | head -100

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Find other tests that toggle configuration or state without cleanup protection
rg -n -A10 'side/eval-form' system-int-test/test/ --type=clj | head -150

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Search for similar patterns where side/eval-form is used to toggle feature flags
rg -n -C5 'side/eval-form' -g '*.clj' system-int-test/test/ | head -200

Length of output: 28251

🏁 Script executed:

#!/bin/bash
# Check if try/finally is commonly used in test files
rg -n -C3 '\(try\b' -g '*.clj' system-int-test/test/ | head -100

Length of output: 8313

@daniel-zamora, thank you for the clarification! I can confirm that pattern exists throughout the test suite. I found numerous examples where side/eval-form is used to toggle configuration without try/finally blocks, including tests like multipolygon-shapefile-granule-counts-test, subscription-count-exceeds-limit-test, and others.

Given that this is an established pattern in the codebase and you've confirmed the exception handling works gracefully in your environment, I understand the current approach is intentional and appropriate for your testing infrastructure. I appreciate you taking the time to explain the context!

🧠 Learnings used 
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:11.311Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.