From 20296bc0d10a0270fc2d1a07b42a9320fc3442bf Mon Sep 17 00:00:00 2001 From: Gyeongmin Park <42409229+1e16miin@users.noreply.github.com> Date: Thu, 5 Sep 2024 18:03:11 +0900 Subject: [PATCH] =?UTF-8?q?=EC=BF=BC=EB=A6=AC=20=EB=B3=B5=EC=9E=A1?= =?UTF-8?q?=EB=8F=84=20=EB=B2=84=EA=B7=B8=20=EC=88=98=EC=A0=95=20=EB=B0=8F?= =?UTF-8?q?=20=EA=B2=B0=EA=B3=BC=20=EA=B0=92=20=EC=88=98=EC=A0=95=20(#16)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix case of root query does not have selection * change tc * warning to analyze result * nit * add tc * rename * nit * nit * refactor * apply review * refactor --- dev-resources/complexity-analysis-error.edn | 8 +- src/com/walmartlabs/lacinia.clj | 63 ++++--- src/com/walmartlabs/lacinia/executor.clj | 36 ++-- ...lexity_analysis.clj => query_analyzer.clj} | 15 +- .../lacinia/complexity_analysis_test.clj | 162 ++++++++++-------- 5 files changed, 154 insertions(+), 130 deletions(-) rename src/com/walmartlabs/lacinia/{complexity_analysis.clj => query_analyzer.clj} (82%) diff --git a/dev-resources/complexity-analysis-error.edn b/dev-resources/complexity-analysis-error.edn index 851bc1bd..72601677 100644 --- a/dev-resources/complexity-analysis-error.edn +++ b/dev-resources/complexity-analysis-error.edn @@ -57,7 +57,8 @@ :Seller {:implements [:Node :User] :fields {:id {:type (non-null ID)} - :name {:type (non-null String)} + :name {:type (non-null String) + :resolve :resolve-name} :products {:type (non-null :ProductConnection) :args {:first {:type Int}} @@ -83,4 +84,7 @@ :queries {:node {:type :Node :args {:id {:type (non-null ID)}} - :resolve :resolve-node}}} + :resolve :resolve-node} + :root + {:type :String + :resolve :resolve-root}}} diff --git a/src/com/walmartlabs/lacinia.clj b/src/com/walmartlabs/lacinia.clj index 80ad7d86..f3841642 100644 --- a/src/com/walmartlabs/lacinia.clj +++ b/src/com/walmartlabs/lacinia.clj @@ -21,7 +21,7 @@ [com.walmartlabs.lacinia.util :refer [as-error-map]] [com.walmartlabs.lacinia.resolve :as resolve] [com.walmartlabs.lacinia.tracing :as tracing] - [com.walmartlabs.lacinia.complexity-analysis :as complexity-analysis]) + [com.walmartlabs.lacinia.query-analyzer :as query-analyzer]) (:import (clojure.lang ExceptionInfo))) (defn ^:private as-errors @@ -34,37 +34,32 @@ Returns a [[ResolverResult]] that will deliver the result map, or an exception." {:added "0.16.0"} - ([parsed-query variables context] - (execute-parsed-query-async parsed-query variables context nil)) - ([parsed-query variables context options] - {:pre [(map? parsed-query) - (or (nil? context) - (map? context))]} - (cond-let - :let [{:keys [::tracing/timing-start]} parsed-query - ;; Validation phase encompasses preparing with query variables and actual validation. - ;; It's somewhat all mixed together. - start-offset (tracing/offset-from-start timing-start) - start-nanos (System/nanoTime) - [prepared error-result] (try - [(parser/prepare-with-query-variables parsed-query variables)] - (catch Exception e - [nil (as-errors e)]))] - - (some? error-result) - (resolve/resolve-as error-result) - - :let [validation-errors (validator/validate prepared)] - - (seq validation-errors) - (resolve/resolve-as {:errors validation-errors}) - - :else (let [complexity-warning (when (:max-complexity options) - (complexity-analysis/complexity-analysis prepared options))] - (executor/execute-query (assoc context constants/parsed-query-key prepared - :complexity-warning complexity-warning - ::tracing/validation {:start-offset start-offset - :duration (tracing/duration start-nanos)})))))) + [parsed-query variables context] + {:pre [(map? parsed-query) + (or (nil? context) + (map? context))]} + (cond-let + :let [{:keys [::tracing/timing-start]} parsed-query + ;; Validation phase encompasses preparing with query variables and actual validation. + ;; It's somewhat all mixed together. + start-offset (tracing/offset-from-start timing-start) + start-nanos (System/nanoTime) + [prepared error-result] (try + [(parser/prepare-with-query-variables parsed-query variables)] + (catch Exception e + [nil (as-errors e)]))] + + (some? error-result) + (resolve/resolve-as error-result) + + :let [validation-errors (validator/validate prepared)] + + (seq validation-errors) + (resolve/resolve-as {:errors validation-errors}) + + :else (executor/execute-query (assoc context constants/parsed-query-key prepared + ::tracing/validation {:start-offset start-offset + :duration (tracing/duration start-nanos)})))) (defn execute-parsed-query "Prepares a query, by applying query variables to it, resulting in a prepared @@ -81,7 +76,9 @@ {:keys [timeout-ms timeout-error] :or {timeout-ms 0 timeout-error {:message "Query execution timed out."}}} options - execution-result (execute-parsed-query-async parsed-query variables context options) + context' (cond-> context + (:analyze-query options) query-analyzer/enable-query-analyzer) + execution-result (execute-parsed-query-async parsed-query variables context') result (do (resolve/on-deliver! execution-result *result) ;; Block on that deliver, then return the final result. diff --git a/src/com/walmartlabs/lacinia/executor.clj b/src/com/walmartlabs/lacinia/executor.clj index ed986af8..931cf2a3 100644 --- a/src/com/walmartlabs/lacinia/executor.clj +++ b/src/com/walmartlabs/lacinia/executor.clj @@ -15,18 +15,19 @@ (ns com.walmartlabs.lacinia.executor "Mechanisms for executing parsed queries against compiled schemas." (:require - [com.walmartlabs.lacinia.internal-utils - :refer [cond-let q to-message - deep-merge keepv get-nested]] - [flatland.ordered.map :refer [ordered-map]] - [com.walmartlabs.lacinia.select-utils :as su] - [com.walmartlabs.lacinia.resolve-utils :refer [transform-result aggregate-results]] - [com.walmartlabs.lacinia.schema :as schema] - [com.walmartlabs.lacinia.resolve :as resolve - :refer [resolve-as resolve-promise]] - [com.walmartlabs.lacinia.tracing :as tracing] - [com.walmartlabs.lacinia.constants :as constants] - [com.walmartlabs.lacinia.selection :as selection]) + [com.walmartlabs.lacinia.internal-utils + :refer [cond-let q to-message + deep-merge keepv get-nested]] + [flatland.ordered.map :refer [ordered-map]] + [com.walmartlabs.lacinia.select-utils :as su] + [com.walmartlabs.lacinia.resolve-utils :refer [transform-result aggregate-results]] + [com.walmartlabs.lacinia.schema :as schema] + [com.walmartlabs.lacinia.resolve :as resolve + :refer [resolve-as resolve-promise]] + [com.walmartlabs.lacinia.tracing :as tracing] + [com.walmartlabs.lacinia.constants :as constants] + [com.walmartlabs.lacinia.selection :as selection] + [com.walmartlabs.lacinia.query-analyzer :as query-analyzer]) (:import (clojure.lang PersistentQueue) (java.util.concurrent Executor))) @@ -379,15 +380,14 @@ (let [parsed-query (get context constants/parsed-query-key) {:keys [selections operation-type ::tracing/timing-start]} parsed-query schema (get parsed-query constants/schema-key) - ^Executor executor (::schema/executor schema) - complexity-warning (:complexity-warning context)] + ^Executor executor (::schema/executor schema)] (binding [resolve/*callback-executor* executor] (let [enabled-selections (remove :disabled? selections) *errors (atom []) - *warnings (if complexity-warning - (atom [complexity-warning]) - (atom [])) - *extensions (atom {}) + *warnings (atom []) + *extensions (if (::query-analyzer/enable? context) + (atom {:analysis (query-analyzer/complexity-analysis parsed-query)}) + (atom {})) *resolver-tracing (when (::tracing/enabled? context) (atom [])) context' (assoc context constants/schema-key schema) diff --git a/src/com/walmartlabs/lacinia/complexity_analysis.clj b/src/com/walmartlabs/lacinia/query_analyzer.clj similarity index 82% rename from src/com/walmartlabs/lacinia/complexity_analysis.clj rename to src/com/walmartlabs/lacinia/query_analyzer.clj index 17a6c1f5..33949620 100644 --- a/src/com/walmartlabs/lacinia/complexity_analysis.clj +++ b/src/com/walmartlabs/lacinia/query_analyzer.clj @@ -1,4 +1,4 @@ -(ns com.walmartlabs.lacinia.complexity-analysis +(ns com.walmartlabs.lacinia.query-analyzer (:require [com.walmartlabs.lacinia.selection :as selection])) (defn ^:private list-args? [arguments] @@ -10,7 +10,7 @@ [{:keys [arguments selections field-name leaf? fragment-name] :as selection} fragment-map] (let [selection-kind (selection/selection-kind selection)] (cond - ;; If it's a leaf node or `pageInfo`, return nil. + ;; If it's a leaf or `pageInfo`, return nil. (or leaf? (= :pageInfo field-name)) nil @@ -39,9 +39,12 @@ (+ n-nodes children-complexity)))) (defn complexity-analysis - [query {:keys [max-complexity] :as _options}] + [query] (let [{:keys [fragments selections]} query summarized-selections (mapcat #(summarize-selection % fragments) selections) - complexity (calculate-complexity (first summarized-selections))] - (when (> complexity max-complexity) - {:message (format "Over max complexity! Current number of resources to be queried: %s" complexity)}))) + complexity (apply + (map calculate-complexity summarized-selections))] + {:complexity complexity})) + +(defn enable-query-analyzer + [context] + (assoc context ::enable? true)) diff --git a/test/com/walmartlabs/lacinia/complexity_analysis_test.clj b/test/com/walmartlabs/lacinia/complexity_analysis_test.clj index 2a7abf4b..45afab0f 100644 --- a/test/com/walmartlabs/lacinia/complexity_analysis_test.clj +++ b/test/com/walmartlabs/lacinia/complexity_analysis_test.clj @@ -43,107 +43,127 @@ [_ _ _] nil) +(defn ^:private resolve-name + [_ _ _] + "name") + +(defn ^:private resolve-rooot + [_ _ _] + nil) + (def ^:private schema (utils/compile-schema "complexity-analysis-error.edn" {:resolve-products resolve-products :resolve-followings resolve-followings :resolve-reviews resolve-reviews :resolve-likers resolve-likers - :resolve-node resolve-node})) + :resolve-node resolve-node + :resolve-name resolve-name + :resolve-root resolve-rooot})) (defn ^:private q [query variables] - (utils/simplify (execute schema query variables nil {:max-complexity 10}))) + (utils/simplify (execute schema query variables nil {:analyze-query true}))) -(deftest over-complexity-analysis +(deftest test-complexity-analysis (testing "It is possible to calculate the complexity of a query in the Relay connection spec by taking into account both named fragments and inline fragments." (is (= {:data {:node nil} - :extensions {:warnings [{:message "Over max complexity! Current number of resources to be queried: 27"}]}} + :extensions {:analysis {:complexity 32}}} (q "query ProductDetail($productId: ID){ - node(id: $productId) { - ... on Product { - ...ProductLikersFragment - seller{ - id - products(first: 5){ + node(id: $productId) { + ... on Product { + ...ProductLikersFragment + seller{ + id + products(first: 5){ + edges{ + node{ + id + } + } + } + } + reviews(first: 5){ edges{ node{ id + author{ + id + name + } + product{ + id + } } } } } - reviews(first: 5){ - edges{ - node{ + } + } + fragment ProductLikersFragment on Product { + likers(first: 10){ + edges{ + node{ + ... on Seller{ + id + } + ... on Buyer{ id - author{ - id - } } } } } - } - } - fragment ProductLikersFragment on Product { - likers(first: 10){ - edges{ - node{ - ... on Seller{ + }" {:productId "id"})))) + (testing "If no arguments are passed in the query, the calculation uses the default value defined in the schema." + (is (= {:data {:node nil} + :extensions {:analysis {:complexity 22}}} + (q "query ProductDetail($productId: ID){ + node(id: $productId) { + ... on Product { + ...ProductLikersFragment + seller{ id + products(first: 5){ + edges{ + node{ + id + } + } + } } - ... on Buyer{ - id + reviews(first: 5){ + edges{ + node{ + id + author{ + id + } + } + } } } } } - }" {:productId "id"})))) - (testing "If no arguments are passed in the query, the calculation uses the default value defined in the schema." - (is (= {:data {:node nil} - :extensions {:warnings [{:message "Over max complexity! Current number of resources to be queried: 22"}]}} - (q "query ProductDetail($productId: ID){ - node(id: $productId) { - ... on Product { - ...ProductLikersFragment - seller{ - id - products(first: 5){ - edges{ - node{ - id - } - } - } - } - reviews(first: 5){ - edges{ - node{ - id - author{ - id - } - } - } - } - } - } - } - fragment ProductLikersFragment on Product { - likers{ - edges{ - node{ - ... on Seller{ - id - } - ... on Buyer{ - id - } - } - } - } - }" {:productId "id"}))))) + fragment ProductLikersFragment on Product { + likers{ + edges{ + node{ + ... on Seller{ + id + } + ... on Buyer{ + id + } + } + } + } + }" {:productId "id"})))) + (testing "If return type of root query is scala, then complexity is 0" + (is (= {:data {:root nil} + :extensions {:analysis {:complexity 0}}} + (q "query root{ + root + }" nil))))) (comment - (run-test over-complexity-analysis)) + (run-test test-complexity-analysis))