Skip to content

Commit

Permalink
쿼리 복잡도 버그 수정 및 결과 값 수정 (#16)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@1e16miin
1e16miin authored Sep 5, 2024
1 parent e297fe6 commit 20296bc
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 130 deletions.
8 changes: 6 additions & 2 deletions dev-resources/complexity-analysis-error.edn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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}}}
63 changes: 30 additions & 33 deletions src/com/walmartlabs/lacinia.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down
36 changes: 18 additions & 18 deletions src/com/walmartlabs/lacinia/executor.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

Expand Down Expand Up @@ -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)
Expand Down
15 changes: 9 additions & 6 deletions ...lmartlabs/lacinia/complexity_analysis.clj → ...om/walmartlabs/lacinia/query_analyzer.clj
View file
Original file line number Diff line number Diff line change
@@ -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]
Expand All @@ -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

Expand Down Expand Up @@ -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))
162 changes: 91 additions & 71 deletions test/com/walmartlabs/lacinia/complexity_analysis_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

0 comments on commit 20296bc

Please sign in to comment.