Skip to content

Commit

Permalink
CMR-10140-1: Fixing email subscription timing and notification time e…
Browse files Browse the repository at this point in the history 
…ntry into the database.
  • Loading branch information
@eereiter
eereiter committed Nov 1, 2024
1 parent 3b0ec17 commit ff172e3
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 38 deletions.
16 changes: 11 additions & 5 deletions ingest-app/src/cmr/ingest/services/subscriptions_helper.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,11 @@
#_{:clj-kondo/ignore [:unresolved-var]}
(defn- send-update-subscription-notification-time!
"Fires off an http call to update the time which the subscription last was processed"
[context sub-id]
(debug "send-update-subscription-notification-time with" sub-id)
(search/save-subscription-notification-time context sub-id))
[context sub-id last-notified-time]
(debug "send-update-subscription-notification-time with" sub-id )
(search/save-subscription-notification-time context sub-id last-notified-time))

#_{:clj-kondo/ignore [:unresolved-var]}
(defn- filter-concept-refs-by-subscriber-id
"Takes a list of concept references and a subscriber id and removes any concept that the user does
not have read access to."
Expand Down Expand Up @@ -266,12 +267,17 @@
Sent subscription email to [" email-address "].
\nSubscription email contents: [" email-content "]."))
(when update-notification-time?
(send-update-subscription-notification-time! context sub-id))
(send-update-subscription-notification-time! context sub-id (:end-time subscription)))
(catch Exception e
(error "Exception caught in email subscription: " sub-id "\n\n"
(.getMessage e) "\n\n" e))))))
subscriber-filtered-concept-refs-list))

(defn remove-ingest-subscriptions
"Remove ingest subscriptions since emails are not sent out for those."
[concept]
(:EndPoint (json/decode (:metadata concept) true)))

(defn email-subscription-processing
"Process email subscriptions and send email when found granules matching the collection and queries
in the subscription and were created/updated during the last processing interval."
Expand All @@ -280,7 +286,7 @@
([context revision-date-range]
(let [subscriptions (->> (mdb/find-concepts context {:latest true} :subscription)
(remove :deleted)
(remove #(:endpoint (:extra-fields %)))
(remove remove-ingest-subscriptions)
(map #(select-keys % [:concept-id :extra-fields :metadata])))]
(send-subscription-emails context (process-subscriptions context subscriptions revision-date-range) (nil? revision-date-range)))))

Expand Down
13 changes: 10 additions & 3 deletions metadata-db-app/src/cmr/metadata_db/api/subscriptions.clj
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
(ns cmr.metadata-db.api.subscriptions
"Defines the HTTP URL routes for the application as related to subscriptions."
(:require
[cheshire.core :as json]
[clojure.string :as string]
[cmr.metadata-db.services.sub-notifications :as sub-note]
[cmr.metadata-db.services.subscriptions :as subscriptions]
[compojure.core :refer [PUT POST context]]))

(defn- update-subscription-notification-time
"Update a subscription notification time"
[context params]
[context params body]
(let [sub-id (:subscription-concept-id params)
_ (sub-note/update-subscription-notification context sub-id)]
last-notified-time (-> (slurp body)
(string/trim)
(json/decode true)
(get :last-notified-time))]
(sub-note/update-subscription-notification context sub-id last-notified-time)
{:status 204}))

(def subscription-api-routes
(context "/subscription" []
;; receive notification to update subscription time
(PUT "/:subscription-concept-id/notification-time"
{params :params
body :body
request-context :request-context}
(update-subscription-notification-time request-context params))
(update-subscription-notification-time request-context params body))
(POST "/refresh-subscription-cache"
{request-context :request-context}
(subscriptions/refresh-subscription-cache request-context))))
14 changes: 6 additions & 8 deletions metadata-db-app/src/cmr/metadata_db/data/oracle/sub_notifications.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
(:require
[clj-time.coerce :as cr]
[clojure.java.jdbc :as j]
[cmr.common.time-keeper :as t] ;; don't use clj-time
[cmr.oracle.connection :as oracle]))

; A note about prepared statments, with j/query using ? and [] is the same as
Expand Down Expand Up @@ -50,18 +49,17 @@
"Create subscription notification record in Oracle."
[db subscription-id]
(let [sql (str "INSERT INTO cmr_sub_notifications"
"(id, subscription_concept_id)"
"VALUES (cmr_sub_notifications_seq.nextval, ?)")]
"(id, subscription_concept_id, last_notified_at)"
"VALUES (cmr_sub_notifications_seq.nextval, ?, NULL)")]
(j/db-do-prepared db sql [subscription-id])))

(defn update-sub-notification
"Update a subscription notification in Oracle."
[db subscription-id]
[db subscription-id last-notified-time]
(let [sql (str "UPDATE cmr_sub_notifications "
"SET last_notified_at = ? "
"WHERE subscription_concept_id = ?")
now (t/now)]
(j/db-do-prepared db sql [(cr/to-sql-time now) subscription-id])))
"WHERE subscription_concept_id = ?")]
(j/db-do-prepared db sql [(cr/to-sql-time last-notified-time) subscription-id])))

(defn update-sub-not-with-aws-arn
"Updates the subscription notification with the subscription arn.
Expand Down Expand Up @@ -91,6 +89,6 @@
(println (save-sub-notification db "SUB1234-test"))
(println (sub-notification-exists? db "SUB1234-test"))
(println (get-sub-notification db "SUB1234-test"))
(println (update-sub-notification db "SUB1234-test"))
(println (update-sub-notification db "SUB1234-test" "2024-11-01T02:17:09.749Z"))
(println (update-sub-not-with-aws-arn db "SUB1234-test" "arn:aws:sns:us-east-1:1234455667:SometestSubscription"))
(println (delete-sub-notification db "SUB1234-test")) )
8 changes: 4 additions & 4 deletions metadata-db-app/src/cmr/metadata_db/services/sub_notifications.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@
"Do the work for updating the subscription notificitation time in the database.
The record is lazly created, if a subscription exists, but not notification
record then create the notification, otherwise update the existing one."
[db subscription-id]
[db subscription-id last-notified-time]
(if (sub-note/subscription-exists? db subscription-id)
(if (sub-note/sub-notification-exists? db subscription-id)
(sub-note/update-sub-notification db subscription-id)
(sub-note/update-sub-notification db subscription-id last-notified-time)
(sub-note/save-sub-notification db subscription-id))
(errors/throw-service-error :not-found (msg/subscription-not-found subscription-id))))

(defn update-subscription-notification
"update a subscription notification record, creating one if needed, complain
if subscription id is not valid or not found"
[context subscription-id]
[context subscription-id last-notified-time]
(let [errors (common-concepts/concept-id-validation subscription-id)
db (mdb-util/context->db context)]
(if (nil? errors)
(update-subscription-notification-time-in-database db subscription-id)
(update-subscription-notification-time-in-database db subscription-id last-notified-time)
(errors/throw-service-error
:not-found
(msg/subscription-not-found subscription-id)))))
Expand Down
28 changes: 13 additions & 15 deletions ...em-int-test/test/cmr/system_int_test/ingest/subscription/subscription_processing_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"CMR subscription processing tests."
(:require
[clj-time.core :as t]
[clojure.test :refer :all]
[clojure.test :refer [deftest is join-fixtures testing use-fixtures]]
[cmr.access-control.test.util :as ac-util]
[cmr.ingest.services.subscriptions-helper :as jobs]
[cmr.mock-echo.client.echo-util :as echo-util]
Expand Down Expand Up @@ -52,7 +52,7 @@
integration tests. If send-subscription-emails is called in tests without send-email being mocked,
errors will be returned when attempting to connect to the mail server in
postal-core/send-message."
[email-settings email-content])
[_email-settings _email-content])

(deftest ^:oracle subscription-job-manual-time-constraint-test
"This test is used to validate that email-subscription-processing will use a
Expand Down Expand Up @@ -161,9 +161,9 @@
(data-umm-c/collection {:ShortName "coll1"
:EntryTitle "entry-title1"})
{:token "mock-echo-system-token"})
gran1 (create-granule-and-index "PROV1" coll1 "Granule1")
_ (create-granule-and-index "PROV1" coll1 "Granule1")
;; Setup subscriptions
sub1 (subscription-util/create-subscription-and-index coll1 "test_sub_prov1" "user2" "provider=PROV1")]
_ (subscription-util/create-subscription-and-index coll1 "test_sub_prov1" "user2" "provider=PROV1")]

(testing "Using the manual endpoint does not update last-notified-at for subscriptions"
(let [system-context (system/context)
Expand Down Expand Up @@ -207,13 +207,13 @@
:EntryTitle "entry-title1"})
{:token "mock-echo-system-token"})

coll2 (data-core/ingest-umm-spec-collection "PROV1"
(data-umm-c/collection {:ShortName "coll2"
:EntryTitle "entry-title2"})
{:token "mock-echo-system-token"})
_ (data-core/ingest-umm-spec-collection "PROV1"
(data-umm-c/collection {:ShortName "coll2"
:EntryTitle "entry-title2"})
{:token "mock-echo-system-token"})
_ (index/wait-until-indexed)
;; Setup subscriptions
sub1 (subscription-util/create-subscription-and-index coll1 "test_sub_prov1" "user2" "provider=PROV1")]
_ (subscription-util/create-subscription-and-index coll1 "test_sub_prov1" "user2" "provider=PROV1")]

(testing "First query executed does not have a last-notified-at and looks back 24 hours"
(let [gran1 (create-granule-and-index "PROV1" coll1 "Granule1")
Expand All @@ -224,8 +224,6 @@
(map :concept-id))]
(is (= (:concept-id gran1) (first results)))))

(dev-system/advance-time! 10)

(testing "Second run finds only granules created since the last notification"
(let [gran2 (create-granule-and-index "PROV1" coll1 "Granule2")
response (->> (system/context)
Expand Down Expand Up @@ -256,7 +254,7 @@
(testing "Tests subscriber-id filtering in subscription email processing job"
(let [user1-group-id (echo-util/get-or-create-group (system/context) "group1")
;; User 1 is in group1
user1-token (echo-util/login (system/context) "user1" [user1-group-id])
_ (echo-util/login (system/context) "user1" [user1-group-id])
_ (echo-util/ungrant (system/context)
(-> (access-control/search-for-acls (system/context)
{:provider "PROV1"
Expand Down Expand Up @@ -317,14 +315,14 @@
(data-umm-c/collection {:ShortName "coll2"
:EntryTitle "entry-title2"})
{:token "mock-echo-system-token"})
coll3 (data-core/ingest-umm-spec-collection "PROV1"
_ (data-core/ingest-umm-spec-collection "PROV1"
(data-umm-c/collection
{:ShortName "coll3"
:EntryTitle "entry-title3"
:AccessConstraints (data-umm-c/access-constraints
{:Value 51 :Description "Those files are for British eyes only."})})
{:token "mock-echo-system-token"})
coll4 (data-core/ingest-umm-spec-collection "PROV1"
_ (data-core/ingest-umm-spec-collection "PROV1"
(data-umm-c/collection
{:ShortName "coll4"
:EntryTitle "entry-title4"
Expand Down Expand Up @@ -358,7 +356,7 @@
{:granule-ur "Granule1"
:access-value 33})
{:token "mock-echo-system-token"})
gran2 (data-core/ingest "PROV1"
_ (data-core/ingest "PROV1"
(data-granule/granule-with-umm-spec-collection coll1
(:concept-id coll1)
{:granule-ur "Granule2"
Expand Down
9 changes: 6 additions & 3 deletions transmit-lib/src/cmr/transmit/search.clj
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns cmr.transmit.search
"Provide functions to invoke search app"
(:require
[cheshire.core :as json]
[clj-http.client :as client]
[clojure.data.xml :as xml]
[cmr.common.api.context :as ch]
Expand All @@ -21,16 +22,18 @@
#_{:clj-kondo/ignore [:unresolved-symbol]}
(defn-timed save-subscription-notification-time
"make an http call to the database application"
[context sub-id]
[context sub-id last-notified-time]
(let [conn (config/context->app-connection context :metadata-db)
request-url (str (conn/root-url conn) (format "/subscription/%s/notification-time" sub-id))
request-body {:last-notified-time last-notified-time}
params (merge
(config/conn-params conn)
{:accept :xml
:headers (merge
(token-header context)
{:client-id config/cmr-client-id})
:throw-exceptions false})
:throw-exceptions false
:body (json/generate-string request-body)})
response (client/put request-url params)
{:keys [body]} response
status (int (:status response))]
Expand Down Expand Up @@ -106,7 +109,7 @@
(errors/internal-error!
(format "Granule search failed. status: %s body: %s" status body)))))

(declare validate-search-params)
(declare validate-search-params context params concept-type)
(defn-timed validate-search-params
"Attempts to search granules using given params via a POST request. If the response contains a
non-200 http code, returns the response body."
Expand Down

0 comments on commit ff172e3

Please sign in to comment.