Skip to content

Commit

Permalink
Merge pull request #11 from green-labs/fragment-merge-error
Browse files Browse the repository at this point in the history 
Fixes a runtime error that occurs when deep-merging fragments
  • Loading branch information
@namenu
namenu committed Jun 26, 2024
2 parents e563fa4 + 0550585 commit f9538ca
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 23 deletions.
8 changes: 8 additions & 0 deletions src/com/walmartlabs/lacinia/internal_utils.clj
View file
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
(or (null? left) (null? right))
:com.walmartlabs.lacinia.schema/null

(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
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))
57 changes: 55 additions & 2 deletions test/com/walmartlabs/lacinia/internal_utils_tests.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

(ns com.walmartlabs.lacinia.internal-utils-tests
(:require
[clojure.test :refer [deftest is]]
[com.walmartlabs.lacinia.internal-utils :refer [assoc-in! update-in!]]
[clojure.test :refer [deftest testing is]]
[com.walmartlabs.lacinia.internal-utils :refer [assoc-in! update-in! deep-merge]]
[clojure.string :as str])
(:import
(clojure.lang ExceptionInfo)))
Expand Down Expand Up @@ -63,3 +63,56 @@
:map {:name {:type String}}
:more-keys (:description)}
(ex-data e)))))

(deftest test-deep-merge
(testing "Basic map merge"
(is (= (deep-merge {:a 1} {:b 2})
{:a 1, :b 2}))
(is (= (deep-merge {:a 1} {:a 2})
{:a 2})))

(testing "Nested map merge"
(is (= (deep-merge {:a {:b 1}} {:a {:c 2}})
{:a {:b 1, :c 2}}))
(is (= (deep-merge {:a {:b 1}} {:a {:b 2}})
{:a {:b 2}})))

(testing "Mixed map and sequential"
(is (thrown-with-msg? ExceptionInfo #"unable to deep merge"
(deep-merge {:a 1} [1 2 3])))
(is (thrown-with-msg? ExceptionInfo #"unable to deep merge"
(deep-merge [1 2 3] {:a 1}))))

#_(testing "Merge with nil values"
(is (thrown-with-msg? ExceptionInfo #"unable to deep merge" (deep-merge {:a 1} nil)
{:a 1}))
(is (thrown-with-msg? ExceptionInfo #"unable to deep merge" (deep-merge nil {:a 1})
{:a 1})))

(testing "Merging with :com.walmartlabs.lacinia.schema/null values"
(is (= (deep-merge {:a 1} :com.walmartlabs.lacinia.schema/null)
:com.walmartlabs.lacinia.schema/null))
(is (= (deep-merge :com.walmartlabs.lacinia.schema/null {:a 1})
:com.walmartlabs.lacinia.schema/null))
(is (= (deep-merge :com.walmartlabs.lacinia.schema/null :com.walmartlabs.lacinia.schema/null)
:com.walmartlabs.lacinia.schema/null)))

(testing "Merging with empty maps"
(is (= (deep-merge {} {:a 1})
{:a 1}))
(is (= (deep-merge {:a 1} {})
{:a 1})))

(testing "Complex nested structures"
(is (= (deep-merge {:a {:b [1 2]}} {:a {:b [3 4]}})
{:a {:b [3 4]}}))
(is (= (deep-merge {:a [{:b 1} {:c 2}]} {:a [{:d 3} {:e 4}]})
{:a [{:b 1, :d 3} {:c 2, :e 4}]})))

(testing "Nested :com.walmartlabs.lacinia.schema/null values"
(is (= (deep-merge {:a {:b :com.walmartlabs.lacinia.schema/null}} {:a {:b 1}})
{:a {:b :com.walmartlabs.lacinia.schema/null}}))
(is (= (deep-merge {:a {:b 1}} {:a {:b :com.walmartlabs.lacinia.schema/null}})
{:a {:b :com.walmartlabs.lacinia.schema/null}}))
(is (= (deep-merge {:a {:b :com.walmartlabs.lacinia.schema/null, :c 2}} {:a {:b 1, :c :com.walmartlabs.lacinia.schema/null}})
{:a {:b :com.walmartlabs.lacinia.schema/null, :c :com.walmartlabs.lacinia.schema/null}}))))

0 comments on commit f9538ca

Please sign in to comment.