From 37b400ff18ba515f4c5bf0a5167039e47c12b833 Mon Sep 17 00:00:00 2001 From: Peter Taoussanis Date: Thu, 11 Apr 2024 09:50:56 +0200 Subject: [PATCH] [new] Prevent common signal opts errors --- src/taoensso/telemere/impl.cljc | 43 +++++++++++++++++++++---------- test/taoensso/telemere_tests.cljc | 17 +++++++----- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/taoensso/telemere/impl.cljc b/src/taoensso/telemere/impl.cljc index 3f17376..dba5590 100644 --- a/src/taoensso/telemere/impl.cljc +++ b/src/taoensso/telemere/impl.cljc @@ -470,27 +470,44 @@ [context defaults main-key extra-key arg-order args] (enc/cond - :let [num-args (count args)] + :let [context-name (str "`" (name context) "`") + num-args (count args) + bad-args! + (fn [msg data] + (throw + (ex-info (str "Invalid " context-name " args: " msg) + (conj + {:context context + :args args} + data))))] (not (#{1 2} num-args)) - (throw - (ex-info (str "Wrong number of args (" num-args ") passed to: " context) - {:context context - :num-args {:actual num-args, :expected #{1 2}} - :args args})) + (bad-args! (str "wrong number of args (" num-args ")") + {:actual num-args, :expected #{1 2}}) - :let [[main-val extra-val-or-opts] + :let [[main-arg extra-arg] (case arg-order - :dsc args, ; [main ...] + :dsc args ; [main ...] :asc (reverse args) ; [... main] (enc/unexpected-arg! - arg-order))] + arg-order)) - (if (or (= num-args 1) (map? extra-val-or-opts)) - (let [opts extra-val-or-opts] (merge defaults {main-key main-val} opts)) - (let [extra-val extra-val-or-opts] (merge defaults {main-key main-val, extra-key extra-val})))))) + extra-arg? (= num-args 2) + extra-opts? (and extra-arg? (map? extra-arg))] -(comment (signal-opts `signal! {:level :info} :id :level :dsc [::my-id {:level :warn}])) + :do + (cond + (map? main-arg) + (bad-args! "single map arg is USUALLY a mistake, so isn't allowed. Please use 2 arg call if this is intentional." {}) + + (and extra-opts? (contains? extra-arg main-key)) + (bad-args! (str "given opts should not contain `" main-key "`.") {})) + + extra-opts? (merge defaults {main-key main-arg} extra-arg) + extra-arg? (merge defaults {main-key main-arg, extra-key extra-arg}) + :else (merge defaults {main-key main-arg})))) + +(comment (signal-opts `foo! {:level :info} :id :level :dsc [::my-id {:level :warn}])) #?(:clj (defn signal-catch-opts diff --git a/test/taoensso/telemere_tests.cljc b/test/taoensso/telemere_tests.cljc index 910ade3..24b31ba 100644 --- a/test/taoensso/telemere_tests.cljc +++ b/test/taoensso/telemere_tests.cljc @@ -438,17 +438,20 @@ (deftest _common-signals [#?(:clj (testing "signal-opts" - [(is (= (impl/signal-opts `signal! {:level :info} :id :level :dsc [::my-id ]) {:level :info, :id ::my-id})) - (is (= (impl/signal-opts `signal! {:level :info} :id :level :dsc [::my-id :warn ]) {:level :warn, :id ::my-id})) - (is (= (impl/signal-opts `signal! {:level :info} :id :level :dsc [::my-id {:level :warn}]) {:level :warn, :id ::my-id})) + [(is (= (impl/signal-opts `foo! {:level :info} :id :level :dsc [::my-id ]) {:level :info, :id ::my-id})) + (is (= (impl/signal-opts `foo! {:level :info} :id :level :dsc [::my-id :warn ]) {:level :warn, :id ::my-id})) + (is (= (impl/signal-opts `foo! {:level :info} :id :level :dsc [::my-id {:level :warn}]) {:level :warn, :id ::my-id})) - (is (= (impl/signal-opts `signal! {:level :info} :id :level :asc [ ::my-id]) {:level :info, :id ::my-id})) - (is (= (impl/signal-opts `signal! {:level :info} :id :level :asc [:warn ::my-id]) {:level :warn, :id ::my-id})) - (is (= (impl/signal-opts `signal! {:level :info} :id :level :asc [{:level :warn} ::my-id]) {:level :warn, :id ::my-id})) + (is (= (impl/signal-opts `foo! {:level :info} :id :level :asc [ ::my-id]) {:level :info, :id ::my-id})) + (is (= (impl/signal-opts `foo! {:level :info} :id :level :asc [:warn ::my-id]) {:level :warn, :id ::my-id})) + (is (= (impl/signal-opts `foo! {:level :info} :id :level :asc [{:level :warn} ::my-id]) {:level :warn, :id ::my-id})) (is (= (impl/signal-catch-opts {:id :main-id, :location {:ns "ns"}, :catch->error true}) [{:id :main-id, :location {:ns "ns"}} {:location {:ns "ns"}, :id :main-id}])) (is (= (impl/signal-catch-opts {:id :main-id, :location {:ns "ns"}, :catch->error :error-id}) [{:id :main-id, :location {:ns "ns"}} {:location {:ns "ns"}, :id :error-id}])) - (is (= (impl/signal-catch-opts {:id :main-id, :location {:ns "ns"}, :catch->error {:id :error-id}}) [{:id :main-id, :location {:ns "ns"}} {:location {:ns "ns"}, :id :error-id}]))])) + (is (= (impl/signal-catch-opts {:id :main-id, :location {:ns "ns"}, :catch->error {:id :error-id}}) [{:id :main-id, :location {:ns "ns"}} {:location {:ns "ns"}, :id :error-id}])) + + (is (throws? :ex-info "Invalid `foo!` args: single map arg is USUALLY a mistake" (impl/signal-opts `foo! {:level :info} :id :level :dsc [{:msg "msg"}]))) + (is (throws? :ex-info "Invalid `foo!` args: given opts should not contain `:id`" (impl/signal-opts `foo! {:level :info} :id :level :dsc [:my-id1 {:id ::my-id2}])))])) (testing "event!" ; id + ?level => allowed? [(let [[[rv] [sv]] (with-sigs (tel/event! :id1 ))] [(is (= rv true)) (is (sm? sv {:kind :event, :line :submap/ex, :level :info, :id :id1}))])