Skip to content

Commit

Permalink
Merge pull request #658 from CSCfi/put-is-idempotent
Browse files Browse the repository at this point in the history 
change /applications/{save,judge,review_request} to use POST
  • Loading branch information
@luontola
luontola authored Aug 28, 2018
2 parents 5db5e94 + 653d6ee commit 4b68901
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 37 deletions.
4 changes: 2 additions & 2 deletions docs/usingtheapi.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Judging from the output of the previous command, in order to apply for access we
Form item "Duration of the project" seems to be optional so we will leave that empty. Let's send in the application.

```sh
curl -X PUT \
curl -X POST \
-H 'Content-Type: application/json' \
-H 'Accept: application/json' \
-H 'x-rems-api-key: 42' \
Expand All @@ -92,7 +92,7 @@ You should get the following output as a response:
This tells us that the request succeeded, the system assigned id 12 to our application, the form was properly filled and that the application has progressed to an applied state. Now we can proceed to approving the request. Both users Developer and Bob have been assigned as approvers for the current workflow but only one of them needs to grant the permission. Let's provide an answer as Developer:

```sh
curl -X PUT \
curl -X POST \
-H 'Content-Type: application/json' \
-H 'Accept: application/json' \
-H 'x-rems-api-key: 42' \
Expand Down
6 changes: 3 additions & 3 deletions src/clj/rems/api/applications.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,21 @@
(content-type "application/pdf"))
(not-found! "not found"))))

(PUT "/save" []
(POST "/save" []
:summary "Create a new application, change an existing one or submit an application"
:body [request SaveApplicationCommand]
:return SaveApplicationResponse
(check-user)
(ok (form/api-save (fix-keys request))))

(PUT "/judge" []
(POST "/judge" []
:summary "Judge an application"
:body [request JudgeApplicationCommand]
:return SuccessResponse
(check-user)
(ok (api-judge request)))

(PUT "/review_request" []
(POST "/review_request" []
:summary "Request a review"
:body [request ReviewRequestCommand]
:return SuccessResponse
Expand Down
8 changes: 4 additions & 4 deletions src/cljs/rems/application.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
[rems.phase :refer [phases get-application-phases]]
[rems.spinner :as spinner]
[rems.text :refer [text text-format localize-state localize-event localize-time localize-item]]
[rems.util :refer [dispatch! fetch index-by put!]]
[rems.util :refer [dispatch! fetch index-by post!]]
[secretary.core :as secretary])
(:require-macros [rems.guide-macros :refer [component-info example]]))

Expand Down Expand Up @@ -131,7 +131,7 @@
(if application-id
{:application-id application-id}
{:catalogue-items catalogue-items}))]
(put! "/api/applications/save"
(post! "/api/applications/save"
{:handler (fn [resp]
(if (:success resp)
(do (rf/dispatch [::set-status :saved])
Expand Down Expand Up @@ -164,7 +164,7 @@
{}))

(defn- judge-application [command application-id round comment]
(put! "/api/applications/judge"
(post! "/api/applications/judge"
{:params {:command command
:application-id application-id
:round round
Expand Down Expand Up @@ -555,7 +555,7 @@
(::review-comment db)))

(defn- send-third-party-review-request [reviewers user application-id round comment]
(put! "/api/applications/review_request"
(post! "/api/applications/review_request"
{:params {:application-id application-id
:round round
:comment comment
Expand Down
56 changes: 28 additions & 28 deletions test/clj/rems/test/api/applications.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
user-id "alice"
another-user "alice_smith"
catid 2]
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "save"
:catalogue-items [catid]
Expand Down Expand Up @@ -86,15 +86,15 @@
application (read-body response)]
(is (= 401 (:status response)))))
(testing "saving as other user"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key another-user)
(json-body {:command "save"
:application-id application-id
:items {1 "REST-Test"}})
app)]
(is (= 401 (:status response)))))
(testing "submitting"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "submit"
:application-id application-id
Expand All @@ -111,7 +111,7 @@
(is (:valid cmd-response))
(is (empty? (:validation cmd-response)))))
(testing "approving"
(let [response (-> (request :put (str "/api/applications/judge"))
(let [response (-> (request :post (str "/api/applications/judge"))
(authenticate api-key "developer")
(json-body {:command "approve"
:application-id application-id
Expand All @@ -131,7 +131,7 @@
(let [api-key "42"
user-id "alice"
catid 2]
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "save"
:catalogue-items [catid]
Expand All @@ -150,7 +150,7 @@
(is (some #(.contains (:text %) "non-localized link license") validations))
(is (some #(.contains (:text %) "non-localized text license") validations)))
(testing "add one field"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "save"
:application-id application-id
Expand All @@ -162,7 +162,7 @@
(is (not (:valid cmd-response)))
(is (= 3 (count validations)))))
(testing "add one license"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "save"
:application-id application-id
Expand All @@ -175,7 +175,7 @@
(is (not (:valid cmd-response)))
(is (= 2 (count validations)))))
(testing "submit partial form"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "submit"
:application-id application-id
Expand All @@ -188,7 +188,7 @@
(is (not (:valid cmd-response)))
(is (= 2 (count validations)))))
(testing "save full form"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "save"
:application-id application-id
Expand All @@ -201,7 +201,7 @@
(is (:valid cmd-response))
(is (empty? validations))))
(testing "submit full form"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "submit"
:application-id application-id
Expand All @@ -219,7 +219,7 @@
user-id "developer"
catid 6]
(testing "save draft for disabled item"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:command "save"
:catalogue-items [catid]
Expand All @@ -229,7 +229,7 @@
;; TODO should we actually return a nice error message here?
(is (= 400 (:status response)) "should not be able to save draft with disbled item")))
(testing "submit for application with disabled item"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key user-id)
(json-body {:application-id 6 ;; application-id 6 is already created, but catalogue-item was disabled later
:command "submit"
Expand All @@ -245,7 +245,7 @@
applicant "alice"
approver "developer"
catid 2]
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(authenticate api-key applicant)
(json-body {:command "submit"
:catalogue-items [catid]
Expand Down Expand Up @@ -275,7 +275,7 @@
(is (:can-approve? application))))
;; TODO tests for :review-type
(testing "approve application"
(is (= 200 (-> (request :put (str "/api/applications/judge"))
(is (= 200 (-> (request :post (str "/api/applications/judge"))
(authenticate api-key approver)
(json-body {:command "approve"
:application-id app-id
Expand Down Expand Up @@ -305,7 +305,7 @@
(let [api-key "42"
user "developer"
catid 2
app-id (-> (request :put (str "/api/applications/save"))
app-id (-> (request :post (str "/api/applications/save"))
(authenticate api-key user)
(json-body {:command "save"
:catalogue-items [catid]
Expand All @@ -316,7 +316,7 @@
:id)
submit (fn []
(is (= 200
(-> (request :put (str "/api/applications/save"))
(-> (request :post (str "/api/applications/save"))
(authenticate api-key user)
(json-body {:command "submit"
:application-id app-id
Expand All @@ -326,7 +326,7 @@
:status))))
action (fn [body]
(is (= 200
(-> (request :put (str "/api/applications/judge"))
(-> (request :post (str "/api/applications/judge"))
(authenticate api-key user)
(json-body (merge {:application-id app-id
:round 0}
Expand Down Expand Up @@ -367,7 +367,7 @@
approver "developer"
reviewer "carl"
catid 2
app-id (-> (request :put (str "/api/applications/save"))
app-id (-> (request :post (str "/api/applications/save"))
(authenticate api-key applicant)
(json-body {:command "submit"
:catalogue-items [catid]
Expand All @@ -385,7 +385,7 @@
(is (not (contains? (set (map :userid reviewers)) "invalid")))))
(testing "send review request"
(is (= 200
(-> (request :put (str "/api/applications/review_request"))
(-> (request :post (str "/api/applications/review_request"))
(authenticate api-key approver)
(json-body {:application-id app-id
:round 0
Expand All @@ -405,7 +405,7 @@
(map #(select-keys % [:userid :comment :event]) events)))))
(testing "send review"
(is (= 200
(-> (request :put (str "/api/applications/judge"))
(-> (request :post (str "/api/applications/judge"))
(authenticate api-key reviewer)
(json-body {:command "third-party-review"
:application-id app-id
Expand All @@ -415,7 +415,7 @@
:status))))
(testing "approve"
(is (= 200
(-> (request :put (str "/api/applications/judge"))
(-> (request :post (str "/api/applications/judge"))
(authenticate api-key approver)
(json-body {:command "approve"
:application-id app-id
Expand Down Expand Up @@ -487,7 +487,7 @@
(is cookie)
(is csrf)
(testing "submit with session"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(header "Cookie" cookie)
(header "x-csrf-token" csrf)
(json-body {:command "submit"
Expand All @@ -499,7 +499,7 @@
(is (= 200 (:status response)))
(is (:success body))))
(testing "submit with session but without csrf"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(header "Cookie" cookie)
(json-body {:command "submit"
:catalogue-items [2]
Expand All @@ -508,7 +508,7 @@
app)]
(is (= 403 (:status response)))))
(testing "submit with session and csrf and wrong api-key"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(header "Cookie" cookie)
(header "x-csrf-token" csrf)
(header "x-rems-api-key" "WRONG")
Expand Down Expand Up @@ -539,15 +539,15 @@
body (read-body response)]
(is (= body "unauthorized"))))
(testing "save without authentication"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(json-body {:command "save"
:catalogue-items [2]
:items {1 "REST-Test"}})
app)
body (read-body response)]
(is (str/includes? body "Invalid anti-forgery token"))))
(testing "save with wrong API-Key"
(let [response (-> (request :put (str "/api/applications/save"))
(let [response (-> (request :post (str "/api/applications/save"))
(assoc-in [:headers "x-rems-api-key"] "invalid-api-key")
(json-body {:command "save"
:catalogue-items [2]
Expand All @@ -556,7 +556,7 @@
body (read-body response)]
(is (= "invalid api key" body))))
(testing "judge without authentication"
(let [body (-> (request :put (str "/api/applications/judge"))
(let [body (-> (request :post (str "/api/applications/judge"))
(json-body {:command "approve"
:application-id 2
:round 0
Expand All @@ -565,7 +565,7 @@
read-body)]
(is (str/includes? body "Invalid anti-forgery token"))))
(testing "judge with wrong API-Key"
(let [body (-> (request :put (str "/api/applications/judge"))
(let [body (-> (request :post (str "/api/applications/judge"))
(authenticate "invalid-api-key" "developer")
(json-body {:command "approve"
:application-id 2
Expand Down

0 comments on commit 4b68901

Please sign in to comment.