Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes a runtime error that occurs when deep-merging fragments #463

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fixes a runtime error that occurs when deep-merging fragments #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4932394
Add failing tests for deep-merge'in fragment error
namenu Jun 25, 2024
b6154df
add tests for deep-merge
namenu Jun 25, 2024
701b4cd
Add another test
namenu Jun 26, 2024
b7b8d5e
another trial
namenu Jul 1, 2024
ddc267a
add only failing test for deep-merge, simplify the solution
namenu Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/com/walmartlabs/lacinia/executor.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@
[left-value right-value]
(if (su/is-result-tuple? right-value)
(let [{:keys [alias value]} right-value
left-alias-value (alias left-value)]
left-alias-value (alias left-value)]
(cond
(= left-alias-value :com.walmartlabs.lacinia.schema/null)
left-value
Expand Down
8 changes: 8 additions & 0 deletions src/com/walmartlabs/lacinia/internal_utils.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,11 +407,19 @@
nil
coll))

(defn- null?
[v]
(or (nil? v)
(= v :com.walmartlabs.lacinia.schema/null)))

(defn deep-merge
"Merges two maps together. Later map override earlier.
If a key is sequential, then each element in the list is merged."
[left right]
(cond
(null? left)
left

(and (map? left) (map? right))
(merge-with deep-merge left right)

Expand Down
143 changes: 122 additions & 21 deletions test/com/walmartlabs/lacinia/executor_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
(ns com.walmartlabs.lacinia.executor-test
"Tests for errors and exceptions inside field resolvers, and for the exception converter."
(:require
[clojure.test :refer [deftest is]]
[clojure.test :refer [deftest is testing]]
[com.walmartlabs.lacinia.resolve :refer [resolve-as]]
[com.walmartlabs.test-utils :refer [execute]]
[com.walmartlabs.lacinia.schema :as schema]))

(deftest deep-merge-on-error
(def compiled-schema
(let [test-schema {:interfaces
{:Node
{:fields {:id {:type '(non-null String)}}}}
Expand All @@ -36,30 +36,45 @@
:resolve (fn [_ _ _]
"Hello, World!")}}}

:Author
:PublicDomainPost
{:implements [:Node]
:fields {:id {:type '(non-null String)}
:name {:type '(non-null String)
:resolve (fn [_ _ _]
"John Doe")}
:absurd {:type '(non-null String)
:author {:type :Author ;; Author is nullable
:resolve (fn [_ _ _] nil)}
:title {:type 'String
:resolve (fn [_ _ _]
(resolve-as nil {:message "This field can't be resolved."}))}}}}
"Epic of Gilgamesh")}}}

:Author
{:implements [:Node]
:fields {:id {:type '(non-null String)}
:name {:type '(non-null String)
:resolve (fn [_ _ _]
"John Doe")}
:alwaysNull {:type 'String
:resolve (fn [_ _ _]
nil)}
:alwaysFail {:type '(non-null String)
:resolve (fn [_ _ _]
(resolve-as nil {:message "This field can't be resolved."}))}}}}

:queries
{:node {:type '(non-null :Node)
:args {:id {:type '(non-null String)}}
:resolve (fn [ctx args v]
(let [{:keys [episode]} args]
(schema/tag-with-type {:id "1000"} :Post)))}}}
compiled-schema (schema/compile test-schema)]
:resolve (fn [_ctx args _v]
(let [{:keys [id]} args]
(case id
"1000" (schema/tag-with-type {:id id} :Post)
"2000" (schema/tag-with-type {:id id} :PublicDomainPost))))}}}]
(schema/compile test-schema)))

(is (= {:data nil,
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :absurd]}]}
(execute compiled-schema "
(deftest deep-merge-on-error
(is (= {:data nil,
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :alwaysFail]}]}
(execute compiled-schema "
fragment PostFragment on Post {
author {
absurd
alwaysFail
}
}
query MyQuery {
Expand All @@ -74,12 +89,12 @@ query MyQuery {
}
}")))

(is (= {:data nil,
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :absurd]}]}
(execute compiled-schema "
(is (= {:data nil,
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :alwaysFail]}]}
(execute compiled-schema "
fragment PostFragment on Post {
author {
absurd
alwaysFail
}
}
query MyQuery {
Expand All @@ -92,4 +107,90 @@ query MyQuery {
}
id
}
}")))))
}")))

(testing "when non-null field is resolved to nil, deep-merge should return nil"
(is (= {:data nil,
:errors [{:message "This field can't be resolved.",
:locations [{:line 13, :column 5}],
:path [:node :author :alwaysFail]}]}
(execute compiled-schema "
query MyQuery {
node(id: \"1000\") {
... on Post {
id
...PostFragment
}
}
}

fragment PostFragment on Post {
author {
alwaysFail
}
...PostFragment2
}

fragment PostFragment2 on Post {
author {
name
}
}
")))

(is (= {:data nil,
:errors [{:message "This field can't be resolved.",
:locations [{:line 14, :column 5}],
:path [:node :author :alwaysFail]}]}
(execute compiled-schema "
query MyQuery {
node(id: \"1000\") {
... on Post {
id
...PostFragment
}
}
}

fragment PostFragment on Post {
...PostFragment2
author {
alwaysFail
}
}

fragment PostFragment2 on Post {
author {
name
}
}
")))

(testing "Nullable parent (PublicDomainPost) with failing non-null child (Author)"
(is (= {:data {:node {:id "2000", :author nil}}}
(execute compiled-schema "
query MyQuery {
node(id: \"2000\") {
... on PublicDomainPost {
id
...PostFragment
}
}
}

fragment PostFragment on PublicDomainPost {
...PostFragment2
author {
alwaysFail
}
}

fragment PostFragment2 on PublicDomainPost {
author {
name
}
}
"))))))

(comment
(deep-merge-on-error))
24 changes: 21 additions & 3 deletions test/com/walmartlabs/lacinia/internal_utils_tests.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@

(ns com.walmartlabs.lacinia.internal-utils-tests
(:require
[clojure.test :refer [deftest is]]
[com.walmartlabs.lacinia.internal-utils :refer [assoc-in! update-in!]]
[clojure.string :as str])
[clojure.test :refer [deftest testing is]]
[com.walmartlabs.lacinia.internal-utils :refer [assoc-in! update-in! deep-merge]]
[clojure.string :as str]
[flatland.ordered.map :refer [ordered-map]])
(:import
(clojure.lang ExceptionInfo)))

Expand Down Expand Up @@ -63,3 +64,20 @@
:map {:name {:type String}}
:more-keys (:description)}
(ex-data e)))))

(deftest test-deep-merge
(= (ordered-map [[:author :com.walmartlabs.lacinia.schema/null]])
(deep-merge
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]])
(ordered-map [[:author :com.walmartlabs.lacinia.schema/null]]))
(deep-merge
(ordered-map [[:author :com.walmartlabs.lacinia.schema/null]])
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]])))

(= (ordered-map [[:author nil]])
(deep-merge
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]])
(ordered-map [[:author nil]]))
(deep-merge
(ordered-map [[:author nil]])
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]]))))
Loading