diff --git a/src/com/walmartlabs/lacinia/executor.clj b/src/com/walmartlabs/lacinia/executor.clj index f81a4d6e..d7bc0c62 100644 --- a/src/com/walmartlabs/lacinia/executor.clj +++ b/src/com/walmartlabs/lacinia/executor.clj @@ -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 diff --git a/src/com/walmartlabs/lacinia/internal_utils.clj b/src/com/walmartlabs/lacinia/internal_utils.clj index 880226be..b11c5a2d 100644 --- a/src/com/walmartlabs/lacinia/internal_utils.clj +++ b/src/com/walmartlabs/lacinia/internal_utils.clj @@ -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) diff --git a/test/com/walmartlabs/lacinia/executor_test.clj b/test/com/walmartlabs/lacinia/executor_test.clj index 591899a0..dd178528 100644 --- a/test/com/walmartlabs/lacinia/executor_test.clj +++ b/test/com/walmartlabs/lacinia/executor_test.clj @@ -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)}}}} @@ -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 { @@ -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 { @@ -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)) \ No newline at end of file diff --git a/test/com/walmartlabs/lacinia/internal_utils_tests.clj b/test/com/walmartlabs/lacinia/internal_utils_tests.clj index 68f47eae..083931da 100644 --- a/test/com/walmartlabs/lacinia/internal_utils_tests.clj +++ b/test/com/walmartlabs/lacinia/internal_utils_tests.clj @@ -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))) @@ -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"]])]]))))