From 93090ba4eb4d55f79f2da994948e71f70d93c409 Mon Sep 17 00:00:00 2001 From: Mateus Dubiela Oliveira Date: Fri, 12 May 2023 17:06:50 -0300 Subject: [PATCH 1/5] raise-interceptor: Improve tests --- .../interceptors/raise_test.clj | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/test/kubernetes_api/interceptors/raise_test.clj b/test/kubernetes_api/interceptors/raise_test.clj index 76aa9b3..d7dee8d 100644 --- a/test/kubernetes_api/interceptors/raise_test.clj +++ b/test/kubernetes_api/interceptors/raise_test.clj @@ -1,25 +1,28 @@ (ns kubernetes-api.interceptors.raise-test - (:require [clojure.test :refer :all] + (:require [clojure.test :refer [deftest is testing]] [kubernetes-api.interceptors.raise :as interceptors.raise] - [matcher-combinators.test])) + [matcher-combinators.test :refer [match? thrown-match?]] + [tripod.context :as tc])) + +(defn- run-interceptor [interceptor input] + (tc/execute + (tc/enqueue input interceptor))) (deftest raise-test - (let [{:keys [leave]} (interceptors.raise/new {})] + (let [raise-interceptor (interceptors.raise/new {})] (testing "should raise the body to be the response on 2xx status" - (is (match? - {:response {:my :body}} - (leave {:response {:status 200 - :body {:my :body}}})))) + (is (match? {:response {:my :body}} + (run-interceptor raise-interceptor {:response {:status 200 :body {:my :body}}})))) + (testing "should have the request/response on metadata" - (is (match? - {:request {:my :request} - :response {:status 200 - :body {:my :body}}} - (meta (leave {:request {:my :request} - :response {:status 200 - :body {:my :body}}}))))) + (is (match? {:request {:my :request} + :response {:status 200 + :body {:my :body}}} + (meta + (run-interceptor raise-interceptor {:request {:my :request} + :response {:status 200 + :body {:my :body}}}))))) (testing "should raise exception on 4XX responses" - (is (thrown-match? - {:type :unauthorized} - (leave {:response {:status 401}})))))) + (is (thrown-match? {:type :unauthorized} + (run-interceptor raise-interceptor {:response {:status 401}})))))) From 94397fb5a9feb902955126d97b81775431b175af Mon Sep 17 00:00:00 2001 From: Mateus Dubiela Oliveira Date: Fri, 12 May 2023 19:06:47 -0300 Subject: [PATCH 2/5] raise-interceptor: Make raise interceptor send exception as object --- project.clj | 4 +-- src/kubernetes_api/interceptors/raise.clj | 27 ++++++++++++------- src/kubernetes_api/swagger.clj | 4 +-- .../interceptors/raise_test.clj | 16 ++++++----- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/project.clj b/project.clj index aee127d..a5f0f7d 100644 --- a/project.clj +++ b/project.clj @@ -18,5 +18,5 @@ :aliases {"lint" ["do" ["cljfmt" "check"] ["nsorg"] ["kibit"]] "lint-fix" ["do" ["cljfmt" "fix"] ["nsorg" "--replace"] ["kibit" "--replace"]]} :profiles {:uberjar {:aot :all} - :dev {:dependencies [[nubank/matcher-combinators "1.2.7"] - [mockfn "0.5.0"]]}}) + :dev {:dependencies [[nubank/matcher-combinators "3.8.5"] + [nubank/mockfn "0.7.0"]]}}) diff --git a/src/kubernetes_api/interceptors/raise.clj b/src/kubernetes_api/interceptors/raise.clj index 02633b3..b03e9c5 100644 --- a/src/kubernetes_api/interceptors/raise.clj +++ b/src/kubernetes_api/interceptors/raise.clj @@ -1,6 +1,6 @@ (ns kubernetes-api.interceptors.raise) -(defn status-error? [status] +(defn- status-error? [status] (or (nil? status) (>= status 400))) (def ^:private error-type->status-code @@ -34,23 +34,30 @@ :gateway-timeout 504 :http-version-not-supported 505}) -(def status-code->error-type (zipmap (vals error-type->status-code) (keys error-type->status-code))) +(def ^:private status-code->error-type (zipmap (vals error-type->status-code) (keys error-type->status-code))) -(defn raise-exception [{:keys [status] :as response}] - (throw (ex-info (str "APIServer error: " status) - {:type (status-code->error-type status) - :response response}))) +(defn- make-exception [{:keys [status] :as response}] + (ex-info (str "APIServer error: " status) + {:type (status-code->error-type status) + :response response})) (defn check-response "Checks the status code. If 400+, raises an exception, returns body otherwise" [response] (cond (:error response) (throw (:error response)) - (status-error? (:status response)) (raise-exception response) + (status-error? (:status response)) (throw (make-exception response)) :else (:body response))) +(defn- maybe-assoc-error [{{:keys [error status body] :as response} :response :as context}] + (cond + error (assoc context :kubernetes-api.core/error error) + (status-error? status) (assoc context :kubernetes-api.core/error (make-exception response)) + :else {:response body})) + (defn new [_] {:name ::raise - :leave (fn [{:keys [request response] :as _context}] - (with-meta {:response (check-response response)} - {:request request :response response}))}) + :leave (fn [context] + (with-meta + (maybe-assoc-error context) + (select-keys context [:request :response])))}) diff --git a/src/kubernetes_api/swagger.clj b/src/kubernetes_api/swagger.clj index 51ab14d..ee41b68 100644 --- a/src/kubernetes_api/swagger.clj +++ b/src/kubernetes_api/swagger.clj @@ -1,12 +1,12 @@ (ns kubernetes-api.swagger (:refer-clojure :exclude [read]) (:require [cheshire.core :as json] + [clojure.java.io :as io] [clojure.string :as string] [clojure.walk :as walk] [kubernetes-api.interceptors.auth :as interceptors.auth] [kubernetes-api.interceptors.raise :as interceptors.raise] - [org.httpkit.client :as http] - [clojure.java.io :as io])) + [org.httpkit.client :as http])) (defn remove-watch-endpoints "Watch endpoints doesn't follow the http1.1 specification, so it will not work diff --git a/test/kubernetes_api/interceptors/raise_test.clj b/test/kubernetes_api/interceptors/raise_test.clj index d7dee8d..c3039f1 100644 --- a/test/kubernetes_api/interceptors/raise_test.clj +++ b/test/kubernetes_api/interceptors/raise_test.clj @@ -1,19 +1,19 @@ (ns kubernetes-api.interceptors.raise-test (:require [clojure.test :refer [deftest is testing]] [kubernetes-api.interceptors.raise :as interceptors.raise] - [matcher-combinators.test :refer [match? thrown-match?]] + [matcher-combinators.matchers :as m] + [matcher-combinators.test :refer [match?]] [tripod.context :as tc])) (defn- run-interceptor [interceptor input] - (tc/execute - (tc/enqueue input interceptor))) + (tc/execute (tc/enqueue input interceptor))) (deftest raise-test (let [raise-interceptor (interceptors.raise/new {})] (testing "should raise the body to be the response on 2xx status" (is (match? {:response {:my :body}} (run-interceptor raise-interceptor {:response {:status 200 :body {:my :body}}})))) - + (testing "should have the request/response on metadata" (is (match? {:request {:my :request} :response {:status 200 @@ -23,6 +23,8 @@ :response {:status 200 :body {:my :body}}}))))) - (testing "should raise exception on 4XX responses" - (is (thrown-match? {:type :unauthorized} - (run-interceptor raise-interceptor {:response {:status 401}})))))) + (testing "return an exception on 4XX responses" + (is (match? (m/via (comp ex-data :kubernetes-api.core/error) + {:type :bad-request, + :response {:status 400}}) + (run-interceptor raise-interceptor {:response {:status 400}})))))) From a9213578cdd206b9adfa278d315df53a8a9c6835 Mon Sep 17 00:00:00 2001 From: Mateus Dubiela Oliveira Date: Mon, 15 May 2023 15:14:11 -0300 Subject: [PATCH 3/5] raise-interceptor: Make internal raise exception comming from interceptors --- .clj-kondo/nubank/matcher-combinators/config.edn | 4 ++++ src/kubernetes_api/internals/martian.clj | 6 +++--- test/kubernetes_api/internals/martian_test.clj | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 .clj-kondo/nubank/matcher-combinators/config.edn create mode 100644 test/kubernetes_api/internals/martian_test.clj diff --git a/.clj-kondo/nubank/matcher-combinators/config.edn b/.clj-kondo/nubank/matcher-combinators/config.edn new file mode 100644 index 0000000..ea4ded7 --- /dev/null +++ b/.clj-kondo/nubank/matcher-combinators/config.edn @@ -0,0 +1,4 @@ +{:linters + {:unresolved-symbol + {:exclude [(cljs.test/is [match? thrown-match?]) + (clojure.test/is [match? thrown-match?])]}}} diff --git a/src/kubernetes_api/internals/martian.clj b/src/kubernetes_api/internals/martian.clj index 71041ef..9e6a702 100644 --- a/src/kubernetes_api/internals/martian.clj +++ b/src/kubernetes_api/internals/martian.clj @@ -4,7 +4,7 @@ (defn response-for "Workaround to throw exceptions in the client like connection timeout" [& args] - (let [response (deref (apply martian/response-for args))] - (if (instance? Throwable (:error response)) - (throw (:error response)) + (let [{:kubernetes-api.core/keys [error] :as response} (deref (apply martian/response-for args))] + (if (instance? Throwable error) + (throw error) response))) diff --git a/test/kubernetes_api/internals/martian_test.clj b/test/kubernetes_api/internals/martian_test.clj new file mode 100644 index 0000000..160961f --- /dev/null +++ b/test/kubernetes_api/internals/martian_test.clj @@ -0,0 +1,15 @@ +(ns kubernetes-api.internals.martian-test + (:require [clojure.test :refer [deftest is testing]] + [kubernetes-api.internals.martian :as internals.martian] + [martian.core :as martian] + [matcher-combinators.test :refer [thrown-match?]] + [mockfn.macros :refer [providing]]) + (:import [clojure.lang ExceptionInfo])) + +(deftest response-for + (testing "Throws exception from client" + (providing [(martian/response-for 'martian 'testing) + (delay {:kubernetes-api.core/error (ex-info "Test error" {:type :error})})] + (is (thrown-match? ExceptionInfo + {:type :error} + (internals.martian/response-for 'martian 'testing)))))) \ No newline at end of file From f4d9c7376c843556f54a90314d5d71213ccd1704 Mon Sep 17 00:00:00 2001 From: Mateus Dubiela Oliveira Date: Wed, 24 May 2023 17:20:37 -0300 Subject: [PATCH 4/5] raise-interceptor: martian/response-for always unwraps :response keys --- src/kubernetes_api/core.clj | 1 - src/kubernetes_api/interceptors/raise.clj | 14 +++++++------- test/kubernetes_api/interceptors/raise_test.clj | 2 +- test/kubernetes_api/internals/martian_test.clj | 10 +++++----- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/kubernetes_api/core.clj b/src/kubernetes_api/core.clj index 5e148d5..85cc3d8 100644 --- a/src/kubernetes_api/core.clj +++ b/src/kubernetes_api/core.clj @@ -123,4 +123,3 @@ schemas" [k8s params] (martian/explore k8s (internals.client/find-preferred-route k8s (dissoc params :request)))) - diff --git a/src/kubernetes_api/interceptors/raise.clj b/src/kubernetes_api/interceptors/raise.clj index b03e9c5..f7268e4 100644 --- a/src/kubernetes_api/interceptors/raise.clj +++ b/src/kubernetes_api/interceptors/raise.clj @@ -49,15 +49,15 @@ (status-error? (:status response)) (throw (make-exception response)) :else (:body response))) -(defn- maybe-assoc-error [{{:keys [error status body] :as response} :response :as context}] +(defn- maybe-assoc-error [{:keys [error status body] :as response}] (cond - error (assoc context :kubernetes-api.core/error error) - (status-error? status) (assoc context :kubernetes-api.core/error (make-exception response)) - :else {:response body})) + error (assoc response :kubernetes-api.core/error error) + (status-error? status) (assoc response :kubernetes-api.core/error (make-exception response)) + :else body)) (defn new [_] {:name ::raise - :leave (fn [context] + :leave (fn [{:keys [request response]}] (with-meta - (maybe-assoc-error context) - (select-keys context [:request :response])))}) + {:response (maybe-assoc-error response)} + {:response response :request request}))}) diff --git a/test/kubernetes_api/interceptors/raise_test.clj b/test/kubernetes_api/interceptors/raise_test.clj index c3039f1..7a614b3 100644 --- a/test/kubernetes_api/interceptors/raise_test.clj +++ b/test/kubernetes_api/interceptors/raise_test.clj @@ -24,7 +24,7 @@ :body {:my :body}}}))))) (testing "return an exception on 4XX responses" - (is (match? (m/via (comp ex-data :kubernetes-api.core/error) + (is (match? (m/via (comp ex-data :kubernetes-api.core/error :response) {:type :bad-request, :response {:status 400}}) (run-interceptor raise-interceptor {:response {:status 400}})))))) diff --git a/test/kubernetes_api/internals/martian_test.clj b/test/kubernetes_api/internals/martian_test.clj index 160961f..5cbe484 100644 --- a/test/kubernetes_api/internals/martian_test.clj +++ b/test/kubernetes_api/internals/martian_test.clj @@ -3,13 +3,13 @@ [kubernetes-api.internals.martian :as internals.martian] [martian.core :as martian] [matcher-combinators.test :refer [thrown-match?]] - [mockfn.macros :refer [providing]]) + [mockfn.macros :refer [providing]]) (:import [clojure.lang ExceptionInfo])) (deftest response-for (testing "Throws exception from client" - (providing [(martian/response-for 'martian 'testing) + (providing [(martian/response-for 'martian 'testing) (delay {:kubernetes-api.core/error (ex-info "Test error" {:type :error})})] - (is (thrown-match? ExceptionInfo - {:type :error} - (internals.martian/response-for 'martian 'testing)))))) \ No newline at end of file + (is (thrown-match? ExceptionInfo + {:type :error} + (internals.martian/response-for 'martian 'testing)))))) \ No newline at end of file From 3b53c41100589c5bda28841a453bf9f9ee007aa1 Mon Sep 17 00:00:00 2001 From: Mateus Dubiela Oliveira Date: Thu, 25 May 2023 18:55:38 -0300 Subject: [PATCH 5/5] k8s-api: Move body evaliation to response-for --- src/kubernetes_api/interceptors/raise.clj | 8 ++++---- src/kubernetes_api/internals/martian.clj | 7 +++---- test/kubernetes_api/interceptors/raise_test.clj | 3 ++- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/kubernetes_api/interceptors/raise.clj b/src/kubernetes_api/interceptors/raise.clj index f7268e4..37ba360 100644 --- a/src/kubernetes_api/interceptors/raise.clj +++ b/src/kubernetes_api/interceptors/raise.clj @@ -49,15 +49,15 @@ (status-error? (:status response)) (throw (make-exception response)) :else (:body response))) -(defn- maybe-assoc-error [{:keys [error status body] :as response}] +(defn- maybe-assoc-error [{:keys [error status] :as response}] (cond error (assoc response :kubernetes-api.core/error error) (status-error? status) (assoc response :kubernetes-api.core/error (make-exception response)) - :else body)) + :else response)) (defn new [_] {:name ::raise - :leave (fn [{:keys [request response]}] + :leave (fn [{:keys [request response] :as context}] (with-meta - {:response (maybe-assoc-error response)} + (update context :response maybe-assoc-error) {:response response :request request}))}) diff --git a/src/kubernetes_api/internals/martian.clj b/src/kubernetes_api/internals/martian.clj index 9e6a702..542b3f5 100644 --- a/src/kubernetes_api/internals/martian.clj +++ b/src/kubernetes_api/internals/martian.clj @@ -4,7 +4,6 @@ (defn response-for "Workaround to throw exceptions in the client like connection timeout" [& args] - (let [{:kubernetes-api.core/keys [error] :as response} (deref (apply martian/response-for args))] - (if (instance? Throwable error) - (throw error) - response))) + (let [{:kubernetes-api.core/keys [error] :keys [body]} (deref (apply martian/response-for args))] + (when (instance? Throwable error) (throw error)) + body)) diff --git a/test/kubernetes_api/interceptors/raise_test.clj b/test/kubernetes_api/interceptors/raise_test.clj index 7a614b3..8b1b3b7 100644 --- a/test/kubernetes_api/interceptors/raise_test.clj +++ b/test/kubernetes_api/interceptors/raise_test.clj @@ -11,7 +11,8 @@ (deftest raise-test (let [raise-interceptor (interceptors.raise/new {})] (testing "should raise the body to be the response on 2xx status" - (is (match? {:response {:my :body}} + (is (match? {:response {:status 200 + :body {:my :body}}} (run-interceptor raise-interceptor {:response {:status 200 :body {:my :body}}})))) (testing "should have the request/response on metadata"