CMR-4753 Unable to update GES_DISC ACL due to unknown concept id (#461)

* CMR-4753 removes uneeded concept-id validation.
nasa · Feb 26, 2018 · 02f4b5b · 02f4b5b
1 parent ae71443
commit 02f4b5b
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 22 deletions.
diff --git a/access-control-app/int_test/cmr/access_control/int_test/acl_crud_test.clj b/access-control-app/int_test/cmr/access_control/int_test/acl_crud_test.clj
@@ -420,22 +420,13 @@
                                    :collection_identifier {:entry_titles ["notreal"]}}}
 
           "An error is returned if specifying a collection identifier with collection concept-ids that do not exist."
-          ["[INVALID ID] is not a valid collection concept-id."
-           "collection with concept-id [INVALID ID] does not exist in provider [PROV1]"]
+          ["[INVALID ID] is not a valid collection concept-id."]
           {:group_permissions [{:user_type "guest" :permissions ["read"]}]
            :catalog_item_identity {:name "A Catalog Item ACL"
                                    :provider_id "PROV1"
                                    :collection_applicable true
                                    :collection_identifier {:concept_ids ["INVALID ID"]}}}
 
-          "An error is returned if specifying a collection identifier with collection concept-ids that do not exist."
-          ["collection with concept-id [C999999999-PROV1] does not exist in provider [PROV1]"]
-          {:group_permissions [{:user_type "guest" :permissions ["read"]}]
-           :catalog_item_identity {:name "A Catalog Item ACL"
-                                   :provider_id "PROV1"
-                                   :collection_applicable true
-                                   :collection_identifier {:concept_ids ["C999999999-PROV1"]}}}
-
           "At least one of a range (min and/or max) or include_undefined value must be specified (collection_identifier)"
           ["either include_undefined_value or the combination of min_value and max_value must be specified"]
           {:group_permissions [{:user_type "guest" :permissions ["read"]}]
@@ -537,6 +528,32 @@
                                                                                      :entry_titles ["coll1 v1"]}}})
                 [:revision_id :status])))))
 
+    (testing "collection concept id and entry title check passes when one collection doesn't exist."
+      (let [concept-id (u/save-collection {:entry-title "coll5 v1"
+                                           :native-id "coll5"
+                                           :entry-id "coll5"
+                                           :short-name "coll5"
+                                           :version "v1"
+                                           :provider-id "PROV1"})
+            ;; To test that validly formated collection concept-ids that don't exist in the provider
+            ;; are not added into the collection-identifier on creation.
+            non-existent-coll-id "C999999-PROV1"
+            acl (u/create-acl token {:group_permissions [{:user_type "guest" :permissions ["read"]}]
+                                     :catalog_item_identity {:name "A real live catalog item ACL2"
+                                                             :provider_id "PROV1"
+                                                             :collection_applicable true
+                                                             :collection_identifier {:concept_ids
+                                                                                     [concept-id non-existent-coll-id]
+                                                                                     :entry_titles ["coll5 v1"]}}})
+            resp (ac/get-acl (u/conn-context) (get acl :concept_id) {:token token :raw? true
+                                                                     :include_full_acl true})]
+        (is (= 1 (get acl :revision_id)))
+        (is (= 200 (get acl :status)))
+        (is (= (get-in resp [:body :catalog_item_identity :collection_identifier :concept_ids])
+               [concept-id]))
+        (is (= (get-in resp [:body :catalog_item_identity :collection_identifier :entry_titles])
+               ["coll5 v1"]))))
+
     (testing "long entry titles"
       (u/save-collection {:entry-title "coll2 xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
                           :native-id "coll2"

diff --git a/access-control-app/src/cmr/access_control/services/acl_util.clj b/access-control-app/src/cmr/access_control/services/acl_util.clj
@@ -3,20 +3,21 @@
    group-service and acl-service."
   (:require
     [clojure.edn :as edn]
+    [clojure.set :as set]
     [clojure.string :as str]
+    [cmr.access-control.config :as config]
     [cmr.access-control.data.access-control-index :as index]
     [cmr.access-control.data.acls :as acls]
-    [cmr.access-control.config :as config]
-    [cmr.common.log :refer [info debug]]
+    [cmr.common-app.services.search.group-query-conditions :as gc]
+    [cmr.common-app.services.search.query-execution :as qe]
+    [cmr.common-app.services.search.query-model :as qm]
+    [cmr.common.log :refer [info debug warn]]
     [cmr.common.mime-types :as mt]
     [cmr.common.services.errors :as errors]
     [cmr.common.util :refer [defn-timed] :as util]
-    [cmr.common-app.services.search.query-execution :as qe]
-    [cmr.common-app.services.search.query-model :as qm]
     [cmr.transmit.echo.tokens :as tokens]
-    [cmr.transmit.metadata-db2 :as mdb]
     [cmr.transmit.metadata-db :as mdb1]
-    [cmr.common-app.services.search.group-query-conditions :as gc]))
+    [cmr.transmit.metadata-db2 :as mdb]))
 
 (def acl-provider-id
   "The provider ID for all ACLs. Since ACLs are not owned by individual
@@ -131,11 +132,16 @@
           colls-from-concept-ids (when (seq concept-ids)
                                    (get-collections-chunked context concept-ids :concept-id provider-id))
           collections (distinct (concat colls-from-entry-titles colls-from-concept-ids))
-          concept-ids (map :concept-id collections)
+          synced-concept-ids (map :concept-id collections)
+          dropped-concept-ids (set/difference (set concept-ids)
+                                              (set synced-concept-ids))
           entry-titles (map #(get-in % [:extra-fields :entry-title]) collections)
           collection-identifier (-> collection-identifier
                                     (assoc :entry-titles entry-titles)
-                                    (assoc :concept-ids concept-ids)
+                                    (assoc :concept-ids synced-concept-ids)
                                     util/remove-nil-keys)]
+      (when (seq dropped-concept-ids)
+        (warn (format "Dropping non existent collection concept-ids from collection identifier: %s"
+                      (vec dropped-concept-ids))))
       (assoc-in acl [:catalog-item-identity :collection-identifier] collection-identifier))
     acl))
diff --git a/access-control-app/src/cmr/access_control/services/acl_validation.clj b/access-control-app/src/cmr/access_control/services/acl_validation.clj
@@ -101,10 +101,7 @@
     [(v/every (fn [key-path concept-id]
                 (let [regex #"C\d+-\S+"]
                   (when-not (re-matches regex concept-id)
-                    {key-path [(format "[%s] is not a valid collection concept-id." concept-id)]}))))
-     (v/every (fn [key-path concept-id]
-                (when-not (seq (mdb1/find-concepts context {:provider-id provider-id :concept-id concept-id} :collection))
-                  {key-path [(format "collection with concept-id [%s] does not exist in provider [%s]" concept-id provider-id)]})))]))
+                    {key-path [(format "[%s] is not a valid collection concept-id." concept-id)]}))))]))
 
 (defn- access-value-validation
   "Validates the access_value part of a collection or granule identifier."