Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

event notifications #2108

Merged
merged 37 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e4571fc
feat: document :event/actor-attributes in schema
opqdonut Apr 1, 2020
6a2a945
feat: first version of event notifications
opqdonut Apr 1, 2020
a3a1a6f
feat: outbox for event notifications
opqdonut Apr 2, 2020
9ddbb68
feat: allow filtering event notifications by type
opqdonut Apr 6, 2020
268a23d
feat: make outbox processing order more stable
opqdonut Apr 6, 2020
cff3f83
feat: docs and validation for :event-notification-targets config option
opqdonut Apr 6, 2020
8dc6c4a
fix: outbox expects string errors, not map
opqdonut Apr 6, 2020
5136b33
feat: send application state along with event notification
opqdonut Apr 6, 2020
b23b660
deps: upgrade stub-http
opqdonut Apr 7, 2020
998d934
feat: use PUT for event notifications
opqdonut Apr 7, 2020
80e7b65
feat: initial version of /api/application/:id/raw endpoint
opqdonut Apr 7, 2020
ae6c6b6
doc: some TODO comments
opqdonut Apr 7, 2020
496d999
feat: make ApplicationRaw closer to Application
opqdonut Apr 7, 2020
f854b79
feat: rename :rems.permissions/* to :application/*
opqdonut Apr 7, 2020
af7fe9d
refactor: get-application-raw instead ofget-unrestricted-application
opqdonut Apr 7, 2020
a58100e
refactor: rename get-application -> get-application-for-user
opqdonut Apr 7, 2020
bb5f8a9
refactor: rename get-unrestricted-application -> get-application-inte…
opqdonut Apr 7, 2020
fa5d232
refactor: rename get-application-raw -> get-application
opqdonut Apr 7, 2020
6aae187
refactor: do less work in queue-notification!
opqdonut Apr 7, 2020
c826770
test: tests for rems.event-notification
opqdonut Apr 7, 2020
766baf4
doc: docs/event-notification.md
opqdonut Apr 7, 2020
5c04f11
doc: update CHANGELOG.md
opqdonut Apr 7, 2020
61c11a3
feat: schema for :application/{role-permissions,user-roles}
opqdonut Apr 7, 2020
50c95c1
feat: configurable headers & timeout for event notifications
opqdonut Apr 9, 2020
1574085
test: test that mimics an external id service
opqdonut Apr 9, 2020
8ba4d12
doc: fix CHANGELOG.md
opqdonut Apr 14, 2020
8dcc380
Merge branch 'master' of github.com:CSCfi/rems into event-push-2095
opqdonut Apr 14, 2020
963a4ba
fix: typo
opqdonut Apr 20, 2020
0c22656
doc: better docs for /api/applications/:application-id/raw
opqdonut Apr 20, 2020
2b5d1cb
refactor: golf rems.event-notifications
opqdonut Apr 20, 2020
68102a5
test: minor improvements in resms.test-event-notification
opqdonut Apr 20, 2020
6139666
refactor: create rems.db.outbox/get-due-entries
opqdonut Apr 20, 2020
425e04b
doc: talk more about ordering, retried etc. in event-notification.md
opqdonut Apr 20, 2020
9604f5d
feat: log more details when sending event notifications
opqdonut Apr 20, 2020
f200bbf
Merge branch 'master' of github.com:CSCfi/rems into event-push-2095
opqdonut Apr 20, 2020
b7fca1a
feat: fix application raw api, gold standard test
opqdonut Apr 20, 2020
c5df8dd
test: test event notification body against application raw api
opqdonut Apr 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ have notable changes.

Changes since v2.11

### Additions
- Event notifications over HTTP. See [docs/event-notification.md](docs/event-notification.md) for details. (#2095)

### Fixes
- Long attachment filenames are now truncated in the UI (#2118)

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

REMS can be configured to send events notifications over HTTP.
Macroz marked this conversation as resolved.
Show resolved Hide resolved

## 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.

## Error handling

Event notifications are retried with exponential backoff for up to 12
hours. Everything except a HTTP 200 status counts as a failure.
Failures are logged. You can also inspect the `outbox` db table for
the retry state of notifications.
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
16 changes: 13 additions & 3 deletions src/clj/rems/api/applications.clj
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,22 @@

;; 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 "Get application by `application-id`. Application is not customized."
opqdonut marked this conversation as resolved.
Show resolved Hide resolved
: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 @@ -272,7 +282,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 @@ -27,15 +27,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
16 changes: 11 additions & 5 deletions src/clj/rems/db/applications.clj
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,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 @@ -100,10 +100,16 @@
(model/build-application-view events fetcher-injections))))

(defn get-application
"Full application state with internal information hidden. No personalized for any users. Suitable for public APIs"
opqdonut marked this conversation as resolved.
Show resolved Hide resolved
[application-id]
(when-let [application (get-application-internal application-id)]
(model/hide-non-public-information application)))

(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 @@ -143,7 +149,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 @@ -165,7 +171,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 @@ -182,7 +188,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
2 changes: 1 addition & 1 deletion src/clj/rems/db/entitlements.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)))))
5 changes: 3 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
4 changes: 2 additions & 2 deletions src/clj/rems/db/test_data.clj
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@
:time (or time (time/now))})

(defn fill-form! [{:keys [application-id actor field-value optional-fields] :as command}]
(let [app (applications/get-application actor application-id)]
(let [app (applications/get-application-for-user actor application-id)]
(command! (assoc (base-command command)
:type :application.command/save-draft
:field-values (for [form (:application/forms app)
Expand All @@ -263,7 +263,7 @@
(or field-value "x"))})))))

(defn accept-licenses! [{:keys [application-id actor] :as command}]
(let [app (applications/get-application actor application-id)]
(let [app (applications/get-application-for-user actor application-id)]
(command! (assoc (base-command command)
:type :application.command/accept-licenses
:accepted-licenses (map :license/id (:application/licenses app))))))
Expand Down
2 changes: 1 addition & 1 deletion src/clj/rems/email/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
(defn- event-to-emails [event]
(when-let [app-id (:application/id event)]
(template/event-to-emails (rems.application.model/enrich-event event users/get-user (constantly nil))
(applications/get-unrestricted-application app-id))))
(applications/get-application app-id))))

(defn- enqueue-email! [email]
(outbox/put! {:outbox/type :email
Expand Down
Loading