Skip to content

Commit

Permalink
CMR-9261: When delete a draft, remove it from database, not tombstone…
Browse files Browse the repository at this point in the history
… it… (#1955)

* CMR-9261: When delete a draft, remove it from database, not tombstone it.

* CMR-9261: Updated a doc string

---------

Co-authored-by: siwei xu <[email protected]>
  • Loading branch information
sxu123 and siwei xu authored Aug 17, 2023
1 parent 95ad1f0 commit af6e37b
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 4 deletions.
20 changes: 20 additions & 0 deletions ingest-app/src/cmr/ingest/api/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,23 @@
(ingest/delete-concept
request-context
concept-attribs)))))

(defn delete-draft
"Delete the given draft by its concept type, provider id and native id."
[concept-type provider-id native-id request]
(let [{:keys [request-context params headers]} request
concept-attribs (-> {:provider-id provider-id
:native-id native-id
:concept-type concept-type}
(set-revision-id headers)
(set-user-id request-context headers))]
(lt-validation/validate-launchpad-token request-context)
(common-enabled/validate-write-enabled request-context "ingest")
(verify-provider-exists request-context provider-id)
(info (format "Deleting %s %s from client %s"
(name concept-type) (pr-str concept-attribs) (:client-id request-context)))
(generate-ingest-response headers
(format-and-contextualize-warnings-existing-errors
(ingest/delete-draft
request-context
concept-attribs)))))
10 changes: 7 additions & 3 deletions ingest-app/src/cmr/ingest/api/generic_documents.clj
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,10 @@
(ingest-document request-context concept headers)))

(defn delete-generic-document
"Deletes a generic document in elasticsearch and creates a tombstone in the database. Returns a 201
if successful with the concept id and revision number. A 404 status is returned if the concept has
"Deletes a generic document in elasticsearch. Creates a tombstone in the database for non-draft
document. Delete all revisions from database for draft document. Returns a 201
if successful with the concept id and revision number for non-draft document. Returns a 200
if successful with the concept id for draft document. A 404 status is returned if the concept has
already been deleted."
[request]
(let [{:keys [route-params request-context params]} request
Expand All @@ -230,7 +232,9 @@
request-context :update :provider-object provider-id)
(acl/verify-provider-context-permission
request-context :read :provider-object provider-id))]
(api-core/delete-concept concept-type provider-id native-id request)))
(if (is-draft-concept? request)
(api-core/delete-draft concept-type provider-id native-id request)
(api-core/delete-concept concept-type provider-id native-id request))))

(defn crud-generic-document
"This function checks for required parameters. If they don't exist then throw an error, otherwise send the request
Expand Down
1 change: 1 addition & 0 deletions ingest-app/src/cmr/ingest/services/ingest_service.clj
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
[cmr.ingest.services.ingest-service.util
;; Public utility functions
delete-concept
delete-draft
fix-ingest-concept-format
health
reset]
Expand Down
20 changes: 20 additions & 0 deletions ingest-app/src/cmr/ingest/services/ingest_service/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,26 @@
{:keys [revision-id]} (mdb/save-concept context concept)]
{:concept-id concept-id, :revision-id revision-id})))

(defn-timed delete-draft
"Delete a draft from mdb and indexer. Throws a 404 error if the draft does not exist."
[context concept-attribs]
(let [{:keys [concept-type provider-id native-id]} concept-attribs
existing-concept (first (mdb/find-concepts context
{:provider-id provider-id
:native-id native-id
:exclude-metadata true
:latest true}
concept-type))
concept-id (:concept-id existing-concept)]
(when-not concept-id
(errors/throw-service-error
:not-found (cmsg/invalid-native-id-msg concept-type provider-id native-id)))
(let [concept (-> concept-attribs
(dissoc :provider-id :native-id)
(assoc :concept-id concept-id :deleted true))
{:keys [concept-id]} (mdb/delete-draft context concept)]
{:concept-id concept-id})))

(defn concept-json->concept
"Returns the concept for the given concept JSON string.
This is a temporary function and will be replaced by the UMM parse-metadata function once
Expand Down
13 changes: 13 additions & 0 deletions metadata-db-app/src/cmr/metadata_db/api/concepts.clj
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@
:body (json/generate-string {:revision-id revision-id})
:headers rh/json-header}))

(defn- force-delete-draft
"Permanently remove a draft concept from the database."
[context params concept-id revision-id]
(let [{:keys [concept-id]} (concept-service/force-delete-draft
context concept-id revision-id)]
{:status 200
:body (json/generate-string {:concept-id concept-id})
:headers rh/json-header}))

(defn- get-concept-id
"Get the concept id for a given concept."
[context params concept-type provider-id native-id]
Expand Down Expand Up @@ -153,6 +162,10 @@
(POST "/" {:keys [request-context params body]}
(save-concept-revision request-context params body))
;; mark a concept as deleted (add a tombstone) specifying the revision the tombstone should have
;; remove a draft from the database
(DELETE "/force-delete-draft/:concept-id" {{:keys [concept-id] :as params} :params
request-context :request-context}
(force-delete-draft request-context params concept-id nil))
(DELETE "/:concept-id/:revision-id" {{:keys [concept-id revision-id] :as params} :params
request-context :request-context}
(delete-concept request-context params concept-id revision-id))
Expand Down
4 changes: 4 additions & 0 deletions metadata-db-app/src/cmr/metadata_db/data/concepts.clj
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@
[db concept-type provider concept-id revision-id]
"Remove a revision of a concept from the database completely.")

(force-delete-draft
[db concept-type provider concept-id]
"Remove a draft concept from the database completely.")

(force-delete-concepts
[db provider concept-type concept-id-revision-id-tuples]
"Remove concept revisions given by concept-id/revision-id tuples.")
Expand Down
11 changes: 11 additions & 0 deletions metadata-db-app/src/cmr/metadata_db/data/memory_db.clj
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,16 @@
(= revision-id (:revision-id c)))))
%)))

(defn force-delete-draft
[db concept-type provider concept-id]
(swap! (:concepts-atom db)
#(filter
(fn [c]
(not (and (= concept-type (:concept-type c))
(= (:provider-id provider) (:provider-id c))
(= concept-id (:concept-id c)))))
%)))

(defn force-delete-concepts
[db provider concept-type concept-id-revision-id-tuples]
(doseq [[concept-id revision-id] concept-id-revision-id-tuples]
Expand Down Expand Up @@ -561,6 +571,7 @@
:get-transactions-for-concept get-transactions-for-concept
:save-concept save-concept
:force-delete force-delete
:force-delete-draft force-delete-draft
:force-delete-concepts force-delete-concepts
:get-concept-type-counts-by-collection get-concept-type-counts-by-collection
:reset reset
Expand Down
8 changes: 8 additions & 0 deletions metadata-db-app/src/cmr/metadata_db/data/oracle/concepts.clj
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,13 @@
(= :revision-id ~revision-id)))))]
(j/execute! this stmt)))

(defn force-delete-draft
[this concept-type provider concept-id]
(let [table (tables/get-table-name provider concept-type)
stmt (su/build (delete table
(where `(= :concept-id ~concept-id))))]
(j/execute! this stmt)))

(defn force-delete-by-params
[db provider params]
(sh/force-delete-concept-by-params db provider params))
Expand Down Expand Up @@ -664,6 +671,7 @@
:get-transactions-for-concept get-transactions-for-concept
:save-concept save-concept
:force-delete force-delete
:force-delete-draft force-delete-draft
:force-delete-by-params force-delete-by-params
:force-delete-concepts force-delete-concepts
:get-concept-type-counts-by-collection get-concept-type-counts-by-collection
Expand Down
16 changes: 16 additions & 0 deletions metadata-db-app/src/cmr/metadata_db/services/concept_service.clj
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,22 @@
{:concept-id concept-id
:revision-id revision-id}))

(defn force-delete-draft
"Remove a draft concept from the database completely."
[context concept-id revision-id]
(let [db (util/context->db context)
{:keys [concept-type provider-id]} (cu/parse-concept-id concept-id)
provider (provider-service/get-provider-by-id context provider-id true)
concept (c/get-concept db concept-type provider concept-id revision-id)]
(if concept
(do
(c/force-delete-draft db concept-type provider concept-id))
(cmsg/data-error :not-found
msg/concept-with-concept-id-and-rev-id-does-not-exist
concept-id
revision-id))
{:concept-id concept-id}))

(defn reset
"Delete all concepts from the concept store and all providers."
[context]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,18 @@
nil "PROV2" native-id :collection-draft gen-util/collection-draft :post)]
(is (= (:errors coll-draft) nil))
(is (= (:errors coll-draft-np) ["You do not have PROVIDER_CONTEXT permission to perform that action."]))))


;; Test that collection-draft is removed from database when deleted.
(deftest test-collection-draft-removed-from-database-on-delete
;; Drafts have permissions to ingest on PROV1, but not on PROV2.
(let [native-id "NativeId"
coll-draft (gen-util/ingest-generic-document
nil "PROV1" native-id :collection-draft gen-util/collection-draft :post)
result1 (gen-util/ingest-generic-document
nil "PROV1" native-id :collection-draft gen-util/collection-draft :delete)
result2 (gen-util/ingest-generic-document
nil "PROV1" native-id :collection-draft gen-util/collection-draft :delete)]
;;The first delete removes the draft from database completely and returns concept-id.
;;The second delete will not say the draft is already deleted because it no longer exists in the database
(is (= result1 {:concept-id "CD1200000011-PROV1", :warnings nil, :existing-errors nil}))
(is (= result2 {:errors ["CollectionDraft with native id [NativeId] in provider [PROV1] does not exist."]}))))
42 changes: 42 additions & 0 deletions transmit-lib/src/cmr/transmit/metadata_db.clj
Original file line number Diff line number Diff line change
Expand Up @@ -468,3 +468,45 @@
(errors/internal-error!
(format "Save concept failed. MetadataDb app response status code: %s response: %s"
status response)))))

(defn-timed delete-draft
"delete a draft in metadata db"
[context concept]
(let [conn (config/context->app-connection context :metadata-db)
concept-json-str (json/generate-string concept)
response (client/delete (str (conn/root-url conn)
"/concepts/force-delete-draft/"
(:concept-id concept))
(merge
(config/conn-params conn)
{:body concept-json-str
:content-type :json
:accept :json
:throw-exceptions false
:headers (ch/context->http-headers context)}))
status (int (:status response))
;; For CMR-4841 - log the first 255 characters of the response body if
;; the parsing of the html throws exception.
response-body (:body response)
body (try
(json/decode response-body)
(catch Exception e
(warn "Exception occurred while parsing the response body: "
(util/trunc response-body 255))
(throw e)))
{:strs [concept-id]} body]
(case status
422
(errors/throw-service-errors :invalid-data (get body "errors"))

200
{:concept-id concept-id}

409
;; Post commit constraint violation occurred
(errors/throw-service-errors :conflict (get body "errors"))

;; default
(errors/internal-error!
(format "Delete draft failed. MetadataDb app response status code: %s response: %s"
status response)))))

0 comments on commit af6e37b

Please sign in to comment.