CMR-10801: refactors longname validation to handle not provided

daniel-zamora · daniel-zamora · commit dc860e98e366 · 2025-12-01T11:24:45.000-05:00
diff --git a/common-app-lib/src/cmr/common_app/services/kms_lookup.clj b/common-app-lib/src/cmr/common_app/services/kms_lookup.clj
@@ -267,6 +267,15 @@
         (error "lookup-by-location-string found redis carmine exception. Will return nil result." e)
         (throw e)))))
 
+(defn- skip-long-name-validation?
+  "Returns true if the long name should be skipped in validation.
+   Long name is skipped if it's nil, empty string, or 'Not provided' (case-insensitive)."
+  [long-name]
+  (or (nil? long-name)
+      (string/blank? long-name)
+      (when (string? long-name)
+        (= "not provided" (string/lower-case (string/trim long-name))))))
+
 (defn- remove-long-name-from-kms-index
   "Removes long-name from the umm-c-index keys in order to prevent validation when
    long-name is not present in the umm-c-keyword.  We only want to validate long-name if it is not nil."
@@ -296,30 +305,56 @@
         (error "lookup-by-umm-c-keyword-data-format found redis carmine exception. Will return nil result." e)
         (throw e)))))
 
+(defn- lookup-by-umm-c-keyword-with-long-name
+  "Generic lookup function for keyword types that have long-name field (platforms, instruments, projects).
+  Takes a keyword as represented in the UMM concepts as a map and returns the KMS keyword map
+  as its stored in the cache. Returns nil if a keyword is not found. Comparison is made case insensitively.
+  The transform-fn parameter is an optional function to apply additional transformations to the keyword
+  before lookup (e.g., renaming :type to :category for platforms)."
+  ([context keyword-scheme umm-c-keyword]
+   (lookup-by-umm-c-keyword-with-long-name context keyword-scheme umm-c-keyword identity))
+  ([context keyword-scheme umm-c-keyword transform-fn]
+   (try
+     (let [umm-c-keyword (csk-extras/transform-keys csk/->kebab-case umm-c-keyword)
+           umm-c-keyword (transform-fn umm-c-keyword)
+           skip-long-name? (skip-long-name-validation? (get umm-c-keyword :long-name))
+           ;; If long-name should be skipped (nil, empty, or "Not provided"), remove it before normalization
+           keyword-for-comparison (if skip-long-name? (dissoc umm-c-keyword :long-name) umm-c-keyword)
+           comparison-map (normalize-for-lookup keyword-for-comparison (kms-scheme->fields-for-umm-c-lookup keyword-scheme))
+           umm-c-cache (hash-cache/context->cache context kms-umm-c-cache-key)
+           [tm value] (util/time-execution (hash-cache/get-value umm-c-cache kms-umm-c-cache-key keyword-scheme))
+           _ (rl-util/log-redis-read-complete (str "lookup-by-umm-c-keyword-" keyword-scheme) kms-umm-c-cache-key tm)
+           ;; When skipping long-name, remove it from KMS index keys to enable matching on remaining fields
+           value-for-lookup (if skip-long-name? (remove-long-name-from-kms-index value) value)]
+       (get value-for-lookup comparison-map))
+     (catch Exception e
+       (if (clojure.string/includes? (ex-message e) "Carmine connection error")
+         (error (str "lookup-by-umm-c-keyword-" keyword-scheme " found redis carmine exception. Will return nil result.") e)
+         (throw e))))))
+
 (defn lookup-by-umm-c-keyword-platforms
   "Takes a keyword as represented in the UMM concepts as a map and returns the KMS keyword map
   as its stored in the cache. Returns nil if a keyword is not found. Comparison is made case insensitively."
   [context keyword-scheme umm-c-keyword]
-  (try
-    (let [umm-c-keyword (csk-extras/transform-keys csk/->kebab-case umm-c-keyword)
-          ;; CMR-3696 This is needed to compare the keyword category, which is mapped
-          ;; to the UMM Platform Type field.  This will avoid complications with facets.
-          umm-c-keyword (set/rename-keys umm-c-keyword {:type :category})
-          comparison-map (normalize-for-lookup umm-c-keyword (kms-scheme->fields-for-umm-c-lookup keyword-scheme))
-          umm-c-cache (hash-cache/context->cache context kms-umm-c-cache-key)
-          [tm value] (util/time-execution (hash-cache/get-value umm-c-cache kms-umm-c-cache-key keyword-scheme))
-          _ (rl-util/log-redis-read-complete "lookup-by-umm-c-keyword-platforms" kms-umm-c-cache-key tm)]
-      (if (get umm-c-keyword :long-name)
-        ;; Check both longname and shortname
-        (get-in value [comparison-map])
-        ;; Check just shortname
-        (-> value
-            (remove-long-name-from-kms-index)
-            (get comparison-map))))
-    (catch Exception e
-      (if (clojure.string/includes? (ex-message e) "Carmine connection error")
-        (error "lookup-by-umm-c-keyword-platforms found redis carmine exception. Will return nil result." e)
-        (throw e)))))
+  ;; CMR-3696 This is needed to compare the keyword category, which is mapped
+  ;; to the UMM Platform Type field.  This will avoid complications with facets.
+  (lookup-by-umm-c-keyword-with-long-name
+   context
+   keyword-scheme
+   umm-c-keyword
+   #(set/rename-keys % {:type :category})))
+
+(defn lookup-by-umm-c-keyword-instruments
+  "Takes a keyword as represented in the UMM concepts as a map and returns the KMS keyword map
+  as its stored in the cache. Returns nil if a keyword is not found. Comparison is made case insensitively."
+  [context keyword-scheme umm-c-keyword]
+  (lookup-by-umm-c-keyword-with-long-name context keyword-scheme umm-c-keyword))
+
+(defn lookup-by-umm-c-keyword-projects
+  "Takes a keyword as represented in the UMM concepts as a map and returns the KMS keyword map
+  as its stored in the cache. Returns nil if a keyword is not found. Comparison is made case insensitively."
+  [context keyword-scheme umm-c-keyword]
+  (lookup-by-umm-c-keyword-with-long-name context keyword-scheme umm-c-keyword))
 
 (defn lookup-by-umm-c-keyword
   "Takes a keyword as represented in UMM concepts as a map and returns the KMS keyword as it exists
@@ -329,6 +364,8 @@
     (when-not (:ignore-kms-keywords context)
       (case keyword-scheme
         :platforms (lookup-by-umm-c-keyword-platforms context keyword-scheme umm-c-keyword)
+        :instruments (lookup-by-umm-c-keyword-instruments context keyword-scheme umm-c-keyword)
+        :projects (lookup-by-umm-c-keyword-projects context keyword-scheme umm-c-keyword)
         :granule-data-format (lookup-by-umm-c-keyword-data-format context keyword-scheme umm-c-keyword)
         ;; default
         (let [umm-c-keyword (csk-extras/transform-keys csk/->kebab-case umm-c-keyword)
diff --git a/common-app-lib/test/cmr/common_app/test/services/kms_lookup.clj b/common-app-lib/test/cmr/common_app/test/services/kms_lookup.clj
@@ -11,9 +11,12 @@
   {:providers [{:level-0 "ACADEMIC" :level-1 "OR-STATE/EOARC" :short-name "PROV1"
                 :long-name "Eastern Oregon Agriculture Research Center, Oregon State University"
                 :uuid "prov1-uuid"}]
-   :platforms [{:short-name "PLAT1" :long-name "Platform 1" :category "Aircraft" :other-random-key 7 :uuid "plat1-uuid"}]
-   :instruments [{:short-name "INST1" :long-name "Instrument 1" :uuid "inst1-uuid"}]
-   :projects [{:short-name "PROJ1" :long-name "Project 1" :uuid "proj1-uuid"}]
+   :platforms [{:short-name "PLAT1" :long-name "Platform 1" :category "Aircraft" :other-random-key 7 :uuid "plat1-uuid"}
+               {:short-name "PLAT2" :category "Spacecraft" :uuid "plat2-uuid"}]
+   :instruments [{:short-name "INST1" :long-name "Instrument 1" :uuid "inst1-uuid"}
+                 {:short-name "INST2" :uuid "inst2-uuid"}]
+   :projects [{:short-name "PROJ1" :long-name "Project 1" :uuid "proj1-uuid"}
+              {:short-name "PROJ2" :uuid "proj2-uuid"}]
    :spatial-keywords [{:category "CONTINENT" :type "AFRICA" :subregion-1 "CENTRAL AFRICA"
                        :subregion-2 "CHAD" :subregion-3 "AOUZOU" :uuid "location1-uuid"}
                       {:category "CONTINENT" :type "AFRICA" :uuid "location2-uuid"}
@@ -236,3 +239,72 @@
    "Checking invalid measurement names with quantities. Getting back the error map."
    '({:context-medium "atmosphere-at_cloud_base" :object "air_bad" :quantity "pressure"})
    {:MeasurementContextMedium "atmosphere-at_cloud_base", :MeasurementContextMediumURI nil, :MeasurementObject "air_bad", :MeasurementObjectURI nil, :MeasurementQuantities [{:Value "pressure", :MeasurementQuantityURI nil}]}))
+
+(deftest blank-and-not-provided-long-name-validation-test
+  (testing "Platform long name validation with blank and 'Not provided' values"
+    (are3
+     [long-name expected-uuid]
+     (is (= expected-uuid
+            (:uuid (kms-lookup/lookup-by-umm-c-keyword create-context :platforms
+                                                       {:short-name "PLAT2" :long-name long-name :type "Spacecraft"}))))
+
+     "Nil long name should match platform without long name in KMS"
+     nil "plat2-uuid"
+
+     "Empty string long name should match platform without long name in KMS"
+     "" "plat2-uuid"
+
+     "Case insensitive 'Not provided' should match"
+     "nOt PrOvIdEd" "plat2-uuid"
+
+     "Invalid long name should not match even with valid short name"
+     "Invalid Long Name" nil))
+
+  (testing "Instrument long name validation with blank and 'Not provided' values"
+    (are3
+     [long-name expected-uuid]
+     (is (= expected-uuid
+            (:uuid (kms-lookup/lookup-by-umm-c-keyword create-context :instruments
+                                                       {:short-name "INST2" :long-name long-name}))))
+
+     "Nil long name should match instrument without long name in KMS"
+     nil "inst2-uuid"
+
+     "Empty string long name should match instrument without long name in KMS"
+     "" "inst2-uuid"
+
+     "Case insensitive 'Not provided' should match"
+     "nOt PrOvIdEd" "inst2-uuid"
+
+     "Invalid long name should not match even with valid short name"
+     "Invalid Long Name" nil))
+
+  (testing "Project long name validation with blank and 'Not provided' values"
+    (are3
+     [long-name expected-uuid]
+     (is (= expected-uuid
+            (:uuid (kms-lookup/lookup-by-umm-c-keyword create-context :projects
+                                                       {:short-name "PROJ2" :long-name long-name}))))
+
+     "Nil long name should match project without long name in KMS"
+     nil "proj2-uuid"
+
+     "Empty string long name should match project without long name in KMS"
+     "" "proj2-uuid"
+
+     "Case insensitive 'Not provided' should match"
+     "nOt PrOvIdEd" "proj2-uuid"
+
+     "Invalid long name should not match even with valid short name"
+     "Invalid Long Name" nil))
+
+  (testing "Platforms with long names in KMS can be matched with or without long name"
+    (is (= "plat1-uuid"
+           (:uuid (kms-lookup/lookup-by-umm-c-keyword create-context :platforms
+                                                      {:short-name "PLAT1" :long-name nil :type "Aircraft"})))
+        "Platform lookup with nil long name when KMS has long name should match (CMR-4400 behavior)")
+
+    (is (= "plat1-uuid"
+           (:uuid (kms-lookup/lookup-by-umm-c-keyword create-context :platforms
+                                                      {:short-name "PLAT1" :long-name "Platform 1" :type "Aircraft"})))
+        "Platform lookup with exact long name match should also match")))
diff --git a/system-int-test/test/cmr/system_int_test/ingest/collection/collection_keyword_validation_test.clj b/system-int-test/test/cmr/system_int_test/ingest/collection/collection_keyword_validation_test.clj
@@ -474,6 +474,60 @@
           "Case Insensitive"
           "Atm" "aIRBORNE Topographic Mapper"))
 
+  (testing "Blank and 'Not provided' long names should not cause validation errors"
+    (testing "Platform with 'Not provided' long name should be valid"
+      (assert-valid-keywords
+        {:Platforms [(data-umm-cmn/platform {:ShortName "CESSNA 188"
+                                             :LongName "Not provided"
+                                             :Type "Propeller"})]}))
+
+    (testing "Instrument with 'Not provided' long name should be valid"
+      (assert-valid-keywords
+        {:Platforms
+         [(data-umm-cmn/platform
+            {:ShortName "A340-600"
+             :LongName "Airbus A340-600"
+             :Type "Jet"
+             :Instruments [(data-umm-cmn/instrument {:ShortName "ACOUSTIC SOUNDERS"
+                                                     :LongName "Not provided"})]})]}))
+
+    (testing "Project with 'Not provided' long name should be valid"
+      (assert-valid-keywords
+        {:Projects [(assoc (data-umm-cmn/project "EUCREX-94" "") :LongName "Not provided")]}))
+
+    (testing "Invalid long names with valid short names should still fail validation"
+      (testing "Platform with invalid long name should fail"
+        (assert-invalid-keywords
+          {:Platforms [(data-umm-cmn/platform {:ShortName "CESSNA 188"
+                                               :LongName "Invalid Long Name"
+                                               :Type "Propeller"})]}
+          ["Platforms" 0]
+          [(format (str "Platform short name [%s], long name [%s], and type [%s]"
+                        " was not a valid keyword combination.")
+                   "CESSNA 188" "Invalid Long Name" "Propeller")]))
+
+      (testing "Instrument with invalid long name should fail"
+        (assert-invalid-keywords
+          {:Platforms
+           [(data-umm-cmn/platform
+              {:ShortName "A340-600"
+               :LongName "Airbus A340-600"
+               :Type "Jet"
+               :Instruments [(data-umm-cmn/instrument {:ShortName "ACOUSTIC SOUNDERS"
+                                                       :LongName "Invalid Long Name"})]})]}
+          ["Platforms" 0 "Instruments" 0]
+          [(format (str "Instrument short name [%s] and long name [%s]"
+                        " was not a valid keyword combination.")
+                   "ACOUSTIC SOUNDERS" "Invalid Long Name")]))
+
+      (testing "Project with invalid long name should fail"
+        (assert-invalid-keywords
+          {:Projects [(assoc (data-umm-cmn/project "EUCREX-94" "") :LongName "Invalid Long Name")]}
+          ["Projects" 0]
+          [(format (str "Project short name [%s] and long name [%s]"
+                        " was not a valid keyword combination.")
+                   "EUCREX-94" "Invalid Long Name")]))))
+
   (testing "Science Keyword validation"
     (are [attribs]
          (let [sk (data-umm-cmn/science-keyword attribs)]
