Skip to content

Commit 7073f34

Browse files
hlshiphlship
authored
Merge pull request #463 from green-labs/fragment-merge-error
Fixes a runtime error that occurs when deep-merging fragments
2 parents 83cf74a + ddc267a commit 7073f34

4 files changed

Lines changed: 152 additions & 25 deletions

File tree

‎src/com/walmartlabs/lacinia/executor.clj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@
182182
[left-value right-value]
183183
(if (su/is-result-tuple? right-value)
184184
(let [{:keys [alias value]} right-value
185-
left-alias-value (alias left-value)]
185+
left-alias-value (alias left-value)]
186186
(cond
187187
(= left-alias-value :com.walmartlabs.lacinia.schema/null)
188188
left-value

‎src/com/walmartlabs/lacinia/internal_utils.clj‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,19 @@
407407
nil
408408
coll))
409409

410+
(defn- null?
411+
[v]
412+
(or (nil? v)
413+
(= v :com.walmartlabs.lacinia.schema/null)))
414+
410415
(defn deep-merge
411416
"Merges two maps together. Later map override earlier.
412417
If a key is sequential, then each element in the list is merged."
413418
[left right]
414419
(cond
420+
(null? left)
421+
left
422+
415423
(and (map? left) (map? right))
416424
(merge-with deep-merge left right)
417425

‎test/com/walmartlabs/lacinia/executor_test.clj‎

Lines changed: 122 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
(ns com.walmartlabs.lacinia.executor-test
1616
"Tests for errors and exceptions inside field resolvers, and for the exception converter."
1717
(:require
18-
[clojure.test :refer [deftest is]]
18+
[clojure.test :refer [deftest is testing]]
1919
[com.walmartlabs.lacinia.resolve :refer [resolve-as]]
2020
[com.walmartlabs.test-utils :refer [execute]]
2121
[com.walmartlabs.lacinia.schema :as schema]))
2222

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

39-
:Author
39+
:PublicDomainPost
4040
{:implements [:Node]
4141
:fields {:id {:type '(non-null String)}
42-
:name {:type '(non-null String)
43-
:resolve (fn [_ _ _]
44-
"John Doe")}
45-
:absurd {:type '(non-null String)
42+
:author {:type :Author ;; Author is nullable
43+
:resolve (fn [_ _ _] nil)}
44+
:title {:type 'String
4645
:resolve (fn [_ _ _]
47-
(resolve-as nil {:message "This field can't be resolved."}))}}}}
46+
"Epic of Gilgamesh")}}}
47+
48+
:Author
49+
{:implements [:Node]
50+
:fields {:id {:type '(non-null String)}
51+
:name {:type '(non-null String)
52+
:resolve (fn [_ _ _]
53+
"John Doe")}
54+
:alwaysNull {:type 'String
55+
:resolve (fn [_ _ _]
56+
nil)}
57+
:alwaysFail {:type '(non-null String)
58+
:resolve (fn [_ _ _]
59+
(resolve-as nil {:message "This field can't be resolved."}))}}}}
4860

4961
:queries
5062
{:node {:type '(non-null :Node)
5163
:args {:id {:type '(non-null String)}}
52-
:resolve (fn [ctx args v]
53-
(let [{:keys [episode]} args]
54-
(schema/tag-with-type {:id "1000"} :Post)))}}}
55-
compiled-schema (schema/compile test-schema)]
64+
:resolve (fn [_ctx args _v]
65+
(let [{:keys [id]} args]
66+
(case id
67+
"1000" (schema/tag-with-type {:id id} :Post)
68+
"2000" (schema/tag-with-type {:id id} :PublicDomainPost))))}}}]
69+
(schema/compile test-schema)))
5670

57-
(is (= {:data nil,
58-
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :absurd]}]}
59-
(execute compiled-schema "
71+
(deftest deep-merge-on-error
72+
(is (= {:data nil,
73+
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :alwaysFail]}]}
74+
(execute compiled-schema "
6075
fragment PostFragment on Post {
6176
author {
62-
absurd
77+
alwaysFail
6378
}
6479
}
6580
query MyQuery {
@@ -74,12 +89,12 @@ query MyQuery {
7489
}
7590
}")))
7691

77-
(is (= {:data nil,
78-
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :absurd]}]}
79-
(execute compiled-schema "
92+
(is (= {:data nil,
93+
:errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :alwaysFail]}]}
94+
(execute compiled-schema "
8095
fragment PostFragment on Post {
8196
author {
82-
absurd
97+
alwaysFail
8398
}
8499
}
85100
query MyQuery {
@@ -92,4 +107,90 @@ query MyQuery {
92107
}
93108
id
94109
}
95-
}")))))
110+
}")))
111+
112+
(testing "when non-null field is resolved to nil, deep-merge should return nil"
113+
(is (= {:data nil,
114+
:errors [{:message "This field can't be resolved.",
115+
:locations [{:line 13, :column 5}],
116+
:path [:node :author :alwaysFail]}]}
117+
(execute compiled-schema "
118+
query MyQuery {
119+
node(id: \"1000\") {
120+
... on Post {
121+
id
122+
...PostFragment
123+
}
124+
}
125+
}
126+
127+
fragment PostFragment on Post {
128+
author {
129+
alwaysFail
130+
}
131+
...PostFragment2
132+
}
133+
134+
fragment PostFragment2 on Post {
135+
author {
136+
name
137+
}
138+
}
139+
")))
140+
141+
(is (= {:data nil,
142+
:errors [{:message "This field can't be resolved.",
143+
:locations [{:line 14, :column 5}],
144+
:path [:node :author :alwaysFail]}]}
145+
(execute compiled-schema "
146+
query MyQuery {
147+
node(id: \"1000\") {
148+
... on Post {
149+
id
150+
...PostFragment
151+
}
152+
}
153+
}
154+
155+
fragment PostFragment on Post {
156+
...PostFragment2
157+
author {
158+
alwaysFail
159+
}
160+
}
161+
162+
fragment PostFragment2 on Post {
163+
author {
164+
name
165+
}
166+
}
167+
")))
168+
169+
(testing "Nullable parent (PublicDomainPost) with failing non-null child (Author)"
170+
(is (= {:data {:node {:id "2000", :author nil}}}
171+
(execute compiled-schema "
172+
query MyQuery {
173+
node(id: \"2000\") {
174+
... on PublicDomainPost {
175+
id
176+
...PostFragment
177+
}
178+
}
179+
}
180+
181+
fragment PostFragment on PublicDomainPost {
182+
...PostFragment2
183+
author {
184+
alwaysFail
185+
}
186+
}
187+
188+
fragment PostFragment2 on PublicDomainPost {
189+
author {
190+
name
191+
}
192+
}
193+
"))))))
194+
195+
(comment
196+
(deep-merge-on-error))

‎test/com/walmartlabs/lacinia/internal_utils_tests.clj‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414

1515
(ns com.walmartlabs.lacinia.internal-utils-tests
1616
(:require
17-
[clojure.test :refer [deftest is]]
18-
[com.walmartlabs.lacinia.internal-utils :refer [assoc-in! update-in!]]
19-
[clojure.string :as str])
17+
[clojure.test :refer [deftest testing is]]
18+
[com.walmartlabs.lacinia.internal-utils :refer [assoc-in! update-in! deep-merge]]
19+
[clojure.string :as str]
20+
[flatland.ordered.map :refer [ordered-map]])
2021
(:import
2122
(clojure.lang ExceptionInfo)))
2223

@@ -63,3 +64,20 @@
6364
:map {:name {:type String}}
6465
:more-keys (:description)}
6566
(ex-data e)))))
67+
68+
(deftest test-deep-merge
69+
(= (ordered-map [[:author :com.walmartlabs.lacinia.schema/null]])
70+
(deep-merge
71+
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]])
72+
(ordered-map [[:author :com.walmartlabs.lacinia.schema/null]]))
73+
(deep-merge
74+
(ordered-map [[:author :com.walmartlabs.lacinia.schema/null]])
75+
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]])))
76+
77+
(= (ordered-map [[:author nil]])
78+
(deep-merge
79+
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]])
80+
(ordered-map [[:author nil]]))
81+
(deep-merge
82+
(ordered-map [[:author nil]])
83+
(ordered-map [[:author (ordered-map [[:name "John Doe"]])]]))))

0 commit comments

Comments
 (0)