Skip to content

Commit

Permalink
Merge pull request #2108 from CSCfi/event-push-2095
Browse files Browse the repository at this point in the history
event notifications
  • Loading branch information
opqdonut authored Apr 20, 2020
2 parents 0628b49 + c5df8dd commit 27a89fe
Show file tree
Hide file tree
Showing 37 changed files with 606 additions and 116 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Changes since v2.11

### Additions
- `api/applications/:id/attachments` api for downloading all attachments as a zip file (#2075)
- Event notifications over HTTP. See [docs/event-notification.md](docs/event-notification.md) for details. (#2095)

## v2.11 "Kotitontuntie" 2020-04-07

Expand Down
33 changes: 33 additions & 0 deletions docs/event-notification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Event notification

REMS can be configured to send events notifications over HTTP.

Event notifications are performed _one at a time_, waiting for a HTTP
200 response from the notification endpoint. Everything except a HTTP
200 response counts as a failure. Failed notifications are retried
with exponential backoff for up to 12 hours.

Due to retries, the event notification endpoint _should be_ idempotent.

Event notifications are _not guaranteed to be ordered_ by event
creation order, especially when retries occur.

Event notification failures are logged. You can also inspect the
`outbox` db table for the retry state of notifications.

## Configuration

See `:event-notification-targets` in [config-defaults.edn](../resources/config-defaults.edn).

## Schema

The body of the HTTP PUT request will be a JSON object that contains:

- `"event/type"`: the type of the event, a string
- `"event/actor"`: who caused this event
- `"event/time"`: when the event occured
- `"application/id"`: the id of the application
- `"event/application"`: the state of the application, in the same format as the `/api/applications/:id/raw` endpoint returns (see Swagger docs)

Other keys may also be present depending on the event type.

2 changes: 1 addition & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
[figwheel-sidecar "0.5.19" :exclusions [org.clojure/tools.nrepl com.fasterxml.jackson.core/jackson-core]]
[re-frisk "0.5.4.1"]
[ring/ring-mock "0.4.0" :exclusions [cheshire]]
[se.haleby/stub-http "0.2.7"]]
[se.haleby/stub-http "0.2.8"]]

:plugins [[lein-ancient "0.6.15"]
[lein-doo "0.1.11"]
Expand Down
18 changes: 18 additions & 0 deletions resources/config-defaults.edn
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,24 @@
:remove nil
:ga4gh nil} ;; Url where entitlements are pushed in ga4gh format, see https://github.com/ga4gh-duri/ga4gh-duri.github.io/

;; URLs to notify about new events. An array of targets. Targets can have keys:
;; :url (mandatory) - the url to send HTTP PUT requests to
;; :event-types (optional) - an array of event types to send. A missing value means "send everything".
;; :timeout (optional) - timeout for the PUT in seconds. Defaults to 60s.
;; :headers (optional) - a map of additional HTTP headers to send.
;;
;; See also: docs/event-notification.md
;;
;; Example:
;;
;; :event-notification-targets [{:url "http://events/everything"}
;; {:url "http://events/filtered"
;; :event-types [:application.event/created :application.event/submitted]
;; :timeout 120
;; :headers {"Authorization" "abc123"
;; "X-Header" "value"}}]
:event-notification-targets []

;; Which database column to show as the application id.
;; Options: :id, :external-id
:application-id-column :external-id
Expand Down
2 changes: 1 addition & 1 deletion resources/sql/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ WHERE 1 = 1
/*~ (when (:ids params) */
AND id IN (:v*:ids)
/*~ ) ~*/
;
ORDER BY id ASC;

-- :name update-outbox! :!
UPDATE outbox
Expand Down
20 changes: 16 additions & 4 deletions src/clj/rems/api/applications.clj
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,24 @@

;; the path parameter matches also non-numeric paths, so this route must be after all overlapping routes
(GET "/:application-id" []
:summary "Get application by `application-id`"
:summary "Get application by `application-id`. Application is customized for the requesting user (e.g. event visibility, permissions, etc)."
:roles #{:logged-in}
:path-params [application-id :- (describe s/Int "application id")]
:responses {200 {:schema Application}
404 {:schema s/Str :description "Not found"}}
(if-let [app (applications/get-application (getx-user-id) application-id)]
(if-let [app (applications/get-application-for-user (getx-user-id) application-id)]
(ok app)
(api-util/not-found-json-response)))

(GET "/:application-id/raw" []
:summary (str "Get application by `application-id`. Unlike the /api/applicaitons/:application-id endpoint, "
"the data here isn't customized for the requesting user (see schema for details). Suitable "
"for integrations and exporting applications.")
:roles #{:reporter :owner}
:path-params [application-id :- (describe s/Int "application id")]
:responses {200 {:schema ApplicationRaw}
404 {:schema s/Str :description "Not found"}}
(if-let [app (applications/get-application application-id)]
(ok app)
(api-util/not-found-json-response)))

Expand All @@ -260,7 +272,7 @@
:path-params [application-id :- (describe s/Int "application id")]
:responses {200 {}
404 {:schema s/Str :description "Not found"}}
(if-let [app (applications/get-application (getx-user-id) application-id)]
(if-let [app (applications/get-application-for-user (getx-user-id) application-id)]
(attachment/zip-attachments app)
(api-util/not-found-json-response)))

Expand All @@ -282,7 +294,7 @@
:roles #{:logged-in}
:path-params [application-id :- (describe s/Int "application id")]
:produces ["application/pdf"]
(if-let [app (applications/get-application (getx-user-id) application-id)]
(if-let [app (applications/get-application-for-user (getx-user-id) application-id)]
(with-language context/*lang*
#(-> app
(pdf/application-to-pdf-bytes)
Expand Down
8 changes: 8 additions & 0 deletions src/clj/rems/api/schema.clj
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@

(s/defschema Event
(assoc events/EventBase
:event/actor-attributes UserWithAttributes
s/Keyword s/Any))

(s/defschema Entitlement
Expand Down Expand Up @@ -257,6 +258,13 @@
:application/permissions Permissions
:application/attachments [ApplicationAttachment]})

(s/defschema ApplicationRaw
(-> Application
(dissoc :application/permissions
:application/roles)
(assoc :application/role-permissions {s/Keyword #{s/Keyword}}
:application/user-roles {s/Str #{s/Keyword}})))

(s/defschema ApplicationOverview
(dissoc Application
:application/events
Expand Down
4 changes: 2 additions & 2 deletions src/clj/rems/api/services/attachment.clj
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
(= user-id (:attachment/user attachment))
attachment

(contains-attachment? (applications/get-application user-id (:application/id attachment))
(contains-attachment? (applications/get-application-for-user user-id (:application/id attachment))
attachment-id)
attachment

:else
(throw-forbidden))))

(defn add-application-attachment [user-id application-id file]
(let [application (applications/get-application user-id application-id)]
(let [application (applications/get-application-for-user user-id application-id)]
(when-not (some (set/union commands/commands-with-comments
#{:application.command/save-draft})
(:application/permissions application))
Expand Down
8 changes: 5 additions & 3 deletions src/clj/rems/api/services/command.clj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
[rems.db.users :as users]
[rems.db.workflow :as workflow]
[rems.email.core :as email]
[rems.event-notification :as event-notification]
[rems.form-validation :as form-validation]
[rems.util :refer [secure-token]])
(:import rems.TryAgainException))
Expand All @@ -27,7 +28,7 @@
(defn- revokes-to-blacklist [new-events]
(doseq [event new-events]
(when (= :application.event/revoked (:event/type event))
(let [application (applications/get-unrestricted-application (:application/id event))]
(let [application (applications/get-application-internal (:application/id event))]
(doseq [resource (:application/resources application)]
(blacklist/add-users-to-blacklist! {:users (application-util/applicant-and-members application)
:resource/ext-id (:resource/ext-id resource)
Expand All @@ -41,7 +42,8 @@
(email/generate-event-emails! new-events)
(run-entitlements new-events)
(rejecter-bot/run-rejecter-bot new-events)
(approver-bot/run-approver-bot new-events)))
(approver-bot/run-approver-bot new-events)
(event-notification/queue-notifications! new-events)))

(def ^:private command-injections
{:valid-user? users/user-exists?
Expand Down Expand Up @@ -75,7 +77,7 @@
(throw (TryAgainException. e))
(throw e))))
(let [app (when-let [app-id (:application-id cmd)]
(applications/get-unrestricted-application app-id))
(applications/get-application-internal app-id))
result (commands/handle-command cmd app command-injections)]
(when-not (:errors result)
(doseq [event (:events result)]
Expand Down
2 changes: 1 addition & 1 deletion src/clj/rems/api/services/licenses.clj
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
:attachment/type (:type attachment)}))

(defn get-application-license-attachment [user-id application-id license-id language]
(when-let [app (applications/get-application user-id application-id)]
(when-let [app (applications/get-application-for-user user-id application-id)]
(when-let [license (some #(when (= license-id (:license/id %)) %)
(:application/licenses app))]
(when-let [attachment-id (get-in license [:license/attachment-id language])]
Expand Down
2 changes: 1 addition & 1 deletion src/clj/rems/application/approver_bot.clj
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@
:comment ""}]))

(defn run-approver-bot [new-events]
(doall (mapcat #(generate-commands % (applications/get-unrestricted-application (:application/id %)))
(doall (mapcat #(generate-commands % (applications/get-application (:application/id %)))
new-events)))
3 changes: 3 additions & 0 deletions src/clj/rems/application/events.clj
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@
:application.event/revoked RevokedEvent
:application.event/submitted SubmittedEvent})

(def event-types
(keys event-schemas))

(s/defschema Event
(apply r/dispatch-on :event/type (flatten (seq event-schemas))))

Expand Down
2 changes: 1 addition & 1 deletion src/clj/rems/application/model.clj
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@
(defn apply-privacy [application roles]
(transform [:application/forms ALL :form/fields ALL] #(apply-field-privacy % roles) application))

(defn- hide-non-public-information [application]
(defn hide-non-public-information [application]
(-> application
hide-invitation-tokens
;; these are not used by the UI, so no need to expose them (especially the user IDs)
Expand Down
2 changes: 1 addition & 1 deletion src/clj/rems/application/rejecter_bot.clj
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@
:actor bot-userid}]))

(defn run-rejecter-bot [new-events]
(doall (mapcat #(generate-commands % (applications/get-unrestricted-application (:application/id %)))
(doall (mapcat #(generate-commands % (applications/get-application (:application/id %)))
new-events)))
2 changes: 1 addition & 1 deletion src/clj/rems/application/search.clj
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
(let [app-ids (distinct (map :application/id events))]
(log/info "Start indexing" (count app-ids) "applications...")
(doseq [app-id app-ids]
(index-application! writer (applications/get-unrestricted-application app-id)))
(index-application! writer (applications/get-application app-id)))
(log/info "Finished indexing" (count app-ids) "applications")))
(.maybeRefresh searcher-manager)
(swap! search-index assoc ::last-processed-event-id (:event/id (last events)))))))
Expand Down
12 changes: 10 additions & 2 deletions src/clj/rems/config.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
[cprop.tools :refer [merge-maps]]
[mount.core :refer [defstate]]
[rems.application.commands :as commands]
[rems.application.events :as events]
[rems.json :as json])
(:import [java.io FileNotFoundException]
[org.joda.time Period]))
Expand Down Expand Up @@ -52,8 +53,15 @@
(assert (.endsWith url "/")
(str ":public-url should end with /:" (pr-str url))))
(when-let [invalid-commands (seq (remove (set commands/command-names) (:disable-commands config)))]
(log/warn "Unrecognized values in :disable-commands : " (pr-str invalid-commands))
(log/warn "Supported-values: " (pr-str commands/command-names)))
(log/warn "Unrecognized values in :disable-commands :" (pr-str invalid-commands))
(log/warn "Supported-values:" (pr-str commands/command-names)))
(doseq [target (:event-notification-targets config)]
(when-let [invalid-events (seq (remove (set events/event-types) (:event-types target)))]
(log/warn "Unrecognized event types in event notification target"
(pr-str target)
":"
(pr-str invalid-events))
(log/warn "Supported event types:" (pr-str events/event-types))))
(assert (not (empty? (:organizations config)))
":organizations can not be empty")
(when-let [invalid-keys (seq (remove known-config-keys (keys config)))]
Expand Down
18 changes: 13 additions & 5 deletions src/clj/rems/db/applications.clj
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
:blacklisted? #(cache/lookup-or-miss blacklist-cache [%1 %2] (fn [[userid resource]]
(blacklist/blacklisted? userid resource)))})

(defn get-unrestricted-application
(defn get-application-internal
"Returns the full application state without any user permission
checks and filtering of sensitive information. Don't expose via APIs."
[application-id]
Expand All @@ -101,10 +101,18 @@
(model/build-application-view events fetcher-injections))))

(defn get-application
"Full application state with internal information hidden. Not personalized for any users. Suitable for public APIs."
[application-id]
(when-let [application (get-application-internal application-id)]
(-> application
(model/hide-non-public-information)
(model/apply-privacy #{:reporter})))) ;; to populate required :field/private attributes

(defn get-application-for-user
"Returns the part of application state which the specified user
is allowed to see. Suitable for returning from public APIs as-is."
[user-id application-id]
(when-let [application (get-unrestricted-application application-id)]
(when-let [application (get-application-internal application-id)]
(or (model/apply-user-permissions application user-id)
(throw-forbidden))))

Expand Down Expand Up @@ -144,7 +152,7 @@
(defn- group-apps-by-user [apps]
(->> apps
(mapcat (fn [app]
(for [user (keys (:rems.permissions/user-roles app))]
(for [user (keys (:application/user-roles app))]
(when-let [app (model/apply-user-permissions app user)]
[user app]))))
(reduce (fn [apps-by-user [user app]]
Expand All @@ -166,7 +174,7 @@

(defn- group-roles-by-user [apps]
(->> apps
(mapcat (fn [app] (:rems.permissions/user-roles app)))
(mapcat (fn [app] (:application/user-roles app)))
(reduce (fn [roles-by-user [user roles]]
(update roles-by-user user set/union roles))
{})))
Expand All @@ -183,7 +191,7 @@
(defn- group-users-by-role [apps]
(->> apps
(mapcat (fn [app]
(for [[user roles] (:rems.permissions/user-roles app)
(for [[user roles] (:application/user-roles app)
role roles]
[user role])))
(reduce (fn [users-by-role [user role]]
Expand Down
6 changes: 3 additions & 3 deletions src/clj/rems/db/entitlements.clj
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@

(defn process-outbox! []
(doseq [entry (mapv fix-entry-from-db
(outbox/get-entries {:type :entitlement-post :due-now? true}))]
(outbox/get-due-entries :entitlement-post))]
;; TODO could send multiple entitlements at once instead of one outbox entry at a time
(if-let [error (post-entitlements! (:outbox/entitlement-post entry))]
(let [entry (outbox/attempt-failed! entry error)]
Expand Down Expand Up @@ -177,7 +177,7 @@
(when (seq members-to-update)
(log/info "updating entitlements on application" application-id)
(doseq [[userid resource-ids] entitlements-to-add]
(grant-entitlements! application-id userid resource-ids actor))
(grant-entitlements! application-id userid resource-ids actor))
(doseq [[userid resource-ids] entitlements-to-remove]
(revoke-entitlements! application-id userid resource-ids actor)))))

Expand All @@ -190,5 +190,5 @@
:application.event/resources-changed
:application.event/revoked}
(:event/type event))
(let [application (applications/get-unrestricted-application (:application/id event))]
(let [application (applications/get-application-internal (:application/id event))]
(update-entitlements-for-application application (:event/actor event)))))
8 changes: 6 additions & 2 deletions src/clj/rems/db/outbox.clj
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@

(def OutboxData
{(s/optional-key :outbox/id) s/Int
:outbox/type (s/enum :email :entitlement-post)
:outbox/type (s/enum :email :entitlement-post :event-notification)
:outbox/backoff Duration
:outbox/created DateTime
:outbox/deadline DateTime
:outbox/next-attempt (s/maybe DateTime)
:outbox/latest-attempt (s/maybe DateTime)
:outbox/latest-error (s/maybe s/Str)
(s/optional-key :outbox/email) s/Any
(s/optional-key :outbox/entitlement-post) s/Any})
(s/optional-key :outbox/entitlement-post) s/Any
(s/optional-key :outbox/event-notification) s/Any})

(def ^Duration initial-backoff (Duration/standardSeconds 10))
(def ^Duration max-backoff (Duration/standardHours 12))
Expand Down Expand Up @@ -73,6 +74,9 @@
due-now? (filter (partial next-attempt-now? (DateTime/now))) ;; TODO move to db?
type (filter #(= type (:outbox/type %))))))

(defn get-due-entries [type]
(get-entries {:type type :due-now? true}))

(defn get-entry-by-id [id]
(first (get-entries {:ids [id]})))

Expand Down
Loading

0 comments on commit 27a89fe

Please sign in to comment.