diff --git a/src/dienstplan/commands.clj b/src/dienstplan/commands.clj index 6af39a6..c99ed1c 100644 --- a/src/dienstplan/commands.clj +++ b/src/dienstplan/commands.clj @@ -225,18 +225,18 @@ Example: where is one of: [create, delete, list] \"\" is a command for a bot to run on schedule - is a crontab file line, e.g. `9 0 * * Mon-Fri` + is a crontab file line, e.g. `0 9 * * Mon-Fri` Example: ``` -@dienstplan schedule create \"rotate my-rota\" 7 0 * * Mon-Fri +@dienstplan schedule create \"rotate my-rota\" 0 7 * * Mon-Fri @dienstplan schedule delete \"rotate my-rota\" @dienstplan schedule list ``` Caveats: -\"\" must be enclosed into double quotation marks +\"\" must be enclosed in the double quotation marks ") (def help-cmd-help @@ -633,7 +633,7 @@ Caveats: name rotation channel-formatted))] text)) -(defn schedule-args-valid? +(defn schedule-args-validation [command-map] (let [{:keys [subcommand executable crontab]} (get command-map :args) subcommand-ok? (contains? #{"create" "delete" "list"} subcommand) @@ -647,9 +647,11 @@ Caveats: (-> crontab kairos/parse-cron some?))] - (and subcommand-ok? - executable-ok? - crontab-ok?))) + (cond + (not subcommand-ok?) :subcommand + (not executable-ok?) :executable + (not crontab-ok?) :crontab + :else :valid))) (defn get-run-at "Return java.sql.Timestamp for the next run for a given crontab string" @@ -661,21 +663,28 @@ Caveats: java.sql.Timestamp/from) (catch Exception _ nil))) +(defn fmt-schedule-invalid-arg + [invalid-arg] + (format "Invalid <%s> argument for `schedule` command\n\n%s" + (name invalid-arg) + help-cmd-schedule)) + (defmethod command-exec! :schedule [command-map] - (if (schedule-args-valid? command-map) - (let [crontab (get-in command-map [:args :crontab]) - query-params {:channel (get-in command-map [:context :channel]) - :executable (get-in command-map [:args :executable]) - :crontab crontab - :run_at (get-run-at crontab)} - {:keys [result error]} - (case (keyword (get-in command-map [:args :subcommand])) - :create (db/schedule-insert! query-params) - :delete (db/schedule-delete! query-params) - :list (db/schedule-list query-params)) - message (or result (:message error))] - message) - (format "Invalid arguments for `schedule` command: %s" (:args command-map)))) + (let [args-validation (schedule-args-validation command-map)] + (if (= args-validation :valid) + (let [crontab (get-in command-map [:args :crontab]) + query-params {:channel (get-in command-map [:context :channel]) + :executable (get-in command-map [:args :executable]) + :crontab crontab + :run_at (get-run-at crontab)} + {:keys [result error]} + (case (keyword (get-in command-map [:args :subcommand])) + :create (db/schedule-insert! query-params) + :delete (db/schedule-delete! query-params) + :list (db/schedule-list query-params)) + message (or result (:message error))] + message) + (fmt-schedule-invalid-arg args-validation)))) (defn get-help-message [] (let [version (get-in config [:application :version])] diff --git a/src/dienstplan/schedule.clj b/src/dienstplan/schedule.clj index 7e230d0..e444eb0 100644 --- a/src/dienstplan/schedule.clj +++ b/src/dienstplan/schedule.clj @@ -50,7 +50,8 @@ :channel (:schedule/channel schedule-row)}}}) (defn process-rows - "Iterate over rows from `schedule` table, process them, return number of processed rows" + "Iterate over rows from `schedule` table, process them, return number + of processed rows" [rows fn-process-command fn-update-schedule] (loop [events (seq rows) processed 0] diff --git a/test/dienstplan/api_test.clj b/test/dienstplan/api_test.clj index fe5346c..a3861e0 100644 --- a/test/dienstplan/api_test.clj +++ b/test/dienstplan/api_test.clj @@ -41,6 +41,9 @@ [inst] (.atZone inst (ZoneId/of "UTC"))) +(defn boom! [& _] + (throw (ex-info "Boom!" {:data nil}))) + (def events-request-base {:method :post :url "http://localhost:8080/api/events" @@ -586,8 +589,7 @@ (deftest ^:integration test-api-unhandled-exception (testing "API unhandled exception" (with-redefs - [helpers/text-trim (fn [& _] - (throw (ex-info "Boom!" {:data nil})))] + [helpers/text-trim boom!] (let [request (merge events-request-base @@ -816,3 +818,136 @@ .getMonth .getValue))) (is (= 2020 (.getYear zoned-run-at)))))))) + +(deftest ^:integration test-schedule-runner-exception + (testing "Schedule processing failed" + (with-redefs + [db/schedule-update! boom! + org.pilosus.kairos/get-current-dt (constantly (zoned-dt dt-before)) + helpers/now-ts-sql (constantly (clojure.instant/read-instant-timestamp dt-now))] + (let [command "<@U001> schedule create \"rotate my-rota\" 59 23 31 12 *" + response (http/request + (merge + events-request-base + {:body + (json/generate-string + {:event + {:text command + :ts "1640250011.000100" + :team "T123" + :channel "C123"}})})) + events-before-processing + (jdbc/with-transaction [conn db/db] + (db/schedules-get + conn + (-> dt-after + clojure.instant/read-instant-timestamp))) + processed-events (schedule/run) + events-after-processing + (jdbc/with-transaction [conn db/db] + (db/schedules-get + conn + (-> dt-after + clojure.instant/read-instant-timestamp)))] + (is (= 1 (count events-before-processing))) + (is (= 1 processed-events)) + (is (= 1 (count events-after-processing))) + (let [event-before (first events-before-processing) + event-after (first events-after-processing)] + (is (= event-before event-after))))))) + +(def schedule-invalid-args "Invalid arguments for `schedule` command: %s") + +(def params-schedule-command-invalid-args + [["<@U001> schedule create who my rota 0 9 * * Mon-Fri" + :executable + "Invalid executable, double quotes omitted"] + ["<@U001> schedule create \"who my rota\" Mon-Fri" + :crontab + "Invalid crontab"]]) + +(deftest ^:integration test-schedule-command-invalid-args + (testing "Invalid `schedule` command arguments: " + (doseq [[command invalid-arg description] params-schedule-command-invalid-args] + (testing description + (let [response (http/request + (merge + events-request-base + {:body + (json/generate-string + {:event + {:text command + :ts "1640250011.000100" + :team "T123" + :channel "C123"}})})) + actual (-> response + :body + (json/parse-string true) + :text) + expected (cmd/fmt-schedule-invalid-arg invalid-arg)] + (is (= expected actual))))))) + +(deftest ^:integration test-schedule-command-duplicate + (testing "Duplicate schedule" + (let [executable "rotate my-rota" + crontab "0 9 * * Mon-Fri" + command (format "<@U001> schedule create \"%s\" %s" + executable + crontab) + request (merge + events-request-base + {:body + (json/generate-string + {:event + {:text command + :ts "1640250011.000100" + :team "T123" + :channel "C123"}})}) + create-response (http/request request) + duplicate-response (http/request request) + created (-> create-response + :body + (json/parse-string true) + :text) + duplicate (-> duplicate-response + :body + (json/parse-string true) + :text)] + (is (= created (format "Executable `%s` successfully scheduled with `%s`" + executable crontab))) + (is (= duplicate (format "Duplicate schedule for `%s` in the channel" + executable)))))) + +(def params-schedule-command-db-exceptions + [["<@U001> schedule create \"rotate my-rota\" 0 9 * * Mon-Fri" + "Boom!" + "Error on schedule insert"] + ["<@U001> schedule delete \"rotate my-rota\"" + "Boom!" + "Error on schedule delete"] + ["<@U001> schedule list" + "Boom!" + "Error on listing schedules"]]) + +(deftest ^:integration test-schedule-command-db-exceptions + (testing "DB exception for schedule subcommand: " + (with-redefs [next.jdbc.sql/insert! boom! + next.jdbc.sql/delete! boom! + next.jdbc/execute! boom!] + (doseq [[command expected description] params-schedule-command-db-exceptions] + (testing description + (let [request (merge + events-request-base + {:body + (json/generate-string + {:event + {:text command + :ts "1640250011.000100" + :team "T123" + :channel "C123"}})}) + response (http/request request) + actual (-> response + :body + (json/parse-string true) + :text)] + (is (= expected actual))))))))