From 4932394910612c318c5ce8e473e73978a1446108 Mon Sep 17 00:00:00 2001 From: Hyunwoo Nam Date: Tue, 25 Jun 2024 22:47:11 +0900 Subject: [PATCH 1/4] Add failing tests for deep-merge'in fragment error --- .../com/walmartlabs/lacinia/executor_test.clj | 91 ++++++++++++++++--- 1 file changed, 77 insertions(+), 14 deletions(-) diff --git a/test/com/walmartlabs/lacinia/executor_test.clj b/test/com/walmartlabs/lacinia/executor_test.clj index 591899a0..9afdd15e 100644 --- a/test/com/walmartlabs/lacinia/executor_test.clj +++ b/test/com/walmartlabs/lacinia/executor_test.clj @@ -15,7 +15,7 @@ (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])) @@ -38,13 +38,16 @@ :Author {:implements [:Node] - :fields {:id {:type '(non-null String)} - :name {:type '(non-null String) - :resolve (fn [_ _ _] - "John Doe")} - :absurd {:type '(non-null String) - :resolve (fn [_ _ _] - (resolve-as nil {:message "This field can't be resolved."}))}}}} + :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) @@ -52,14 +55,15 @@ :resolve (fn [ctx args v] (let [{:keys [episode]} args] (schema/tag-with-type {:id "1000"} :Post)))}}} - compiled-schema (schema/compile test-schema)] + ] + (def compiled-schema (schema/compile test-schema)) (is (= {:data nil, - :errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :absurd]}]} + :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 { @@ -75,11 +79,11 @@ query MyQuery { }"))) (is (= {:data nil, - :errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :absurd]}]} + :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 +96,63 @@ 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 + } +} +")))) + + )) From 11d5c683741f9f72aa567b8046de2a316f8facea Mon Sep 17 00:00:00 2001 From: Hyunwoo Nam Date: Tue, 25 Jun 2024 22:53:24 +0900 Subject: [PATCH 2/4] Fix failing test --- src/com/walmartlabs/lacinia/internal_utils.clj | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/com/walmartlabs/lacinia/internal_utils.clj b/src/com/walmartlabs/lacinia/internal_utils.clj index 880226be..fbe9e15a 100644 --- a/src/com/walmartlabs/lacinia/internal_utils.clj +++ b/src/com/walmartlabs/lacinia/internal_utils.clj @@ -407,11 +407,18 @@ nil coll)) +(defn- null? + [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) From 1602b0cb18b13dff7b54abd9d144cebfb1924943 Mon Sep 17 00:00:00 2001 From: Hyunwoo Nam Date: Wed, 26 Jun 2024 00:30:13 +0900 Subject: [PATCH 3/4] add tests for deep-merge --- .../walmartlabs/lacinia/internal_utils.clj | 3 +- .../com/walmartlabs/lacinia/executor_test.clj | 69 ++++++++++--------- .../lacinia/internal_utils_tests.clj | 57 ++++++++++++++- 3 files changed, 94 insertions(+), 35 deletions(-) diff --git a/src/com/walmartlabs/lacinia/internal_utils.clj b/src/com/walmartlabs/lacinia/internal_utils.clj index fbe9e15a..7f0c3a53 100644 --- a/src/com/walmartlabs/lacinia/internal_utils.clj +++ b/src/com/walmartlabs/lacinia/internal_utils.clj @@ -409,7 +409,8 @@ (defn- null? [v] - (= v :com.walmartlabs.lacinia.schema/null)) + (or (nil? v) + (= v :com.walmartlabs.lacinia.schema/null))) (defn deep-merge "Merges two maps together. Later map override earlier. diff --git a/test/com/walmartlabs/lacinia/executor_test.clj b/test/com/walmartlabs/lacinia/executor_test.clj index 9afdd15e..ae30a846 100644 --- a/test/com/walmartlabs/lacinia/executor_test.clj +++ b/test/com/walmartlabs/lacinia/executor_test.clj @@ -20,7 +20,7 @@ [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)}}}} @@ -28,13 +28,16 @@ :objects {:Post {:implements [:Node] - :fields {:id {:type '(non-null String)} - :author {:type '(non-null :Author) - :resolve (fn [_ _ _] - {:id "2000"})} - :title {:type 'String - :resolve (fn [_ _ _] - "Hello, World!")}}} + :fields {:id {:type '(non-null String)} + :author {:type '(non-null :Author) + :resolve (fn [_ _ _] + {:id "2000"})} + :title {:type 'String + :resolve (fn [_ _ _] + "Hello, World!")} + :comments {:type '(list :Comment) + :resolve (fn [_ _ _] + [{:id "3000"}])}}} :Author {:implements [:Node] @@ -47,20 +50,24 @@ nil)} :alwaysFail {:type '(non-null String) :resolve (fn [_ _ _] - (resolve-as nil {:message "This field can't be resolved."}))}}}} + (resolve-as nil {:message "This field can't be resolved."}))}}} + + :Comment + {:implements [:Node] + :fields {:id {:type '(non-null String)}}}} :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)))}}} - ] - (def compiled-schema (schema/compile test-schema)) + (schema/tag-with-type {:id "1000"} :Post)))}}}] + (schema/compile test-schema))) - (is (= {:data nil, - :errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :alwaysFail]}]} - (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 { alwaysFail @@ -78,9 +85,9 @@ query MyQuery { } }"))) - (is (= {:data nil, - :errors [{:message "This field can't be resolved.", :locations [{:line 4, :column 5}], :path [:node :author :alwaysFail]}]} - (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 { alwaysFail @@ -98,12 +105,12 @@ query MyQuery { } }"))) - (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 " + (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 { @@ -127,11 +134,11 @@ fragment PostFragment2 on Post { } "))) - (is (= {:data nil, - :errors [{:message "This field can't be resolved.", - :locations [{:line 14, :column 5}], - :path [:node :author :alwaysFail]}]} - (execute compiled-schema " + (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 { @@ -153,6 +160,4 @@ fragment PostFragment2 on Post { name } } -")))) - - )) +"))))) diff --git a/test/com/walmartlabs/lacinia/internal_utils_tests.clj b/test/com/walmartlabs/lacinia/internal_utils_tests.clj index 68f47eae..37e27c9a 100644 --- a/test/com/walmartlabs/lacinia/internal_utils_tests.clj +++ b/test/com/walmartlabs/lacinia/internal_utils_tests.clj @@ -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))) @@ -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}})))) From 055058526d4b2733d27f4730c86609f82a3f282e Mon Sep 17 00:00:00 2001 From: Hyunwoo Nam Date: Wed, 26 Jun 2024 11:14:13 +0900 Subject: [PATCH 4/4] Add another test --- .../com/walmartlabs/lacinia/executor_test.clj | 71 ++++++++++++++----- 1 file changed, 52 insertions(+), 19 deletions(-) diff --git a/test/com/walmartlabs/lacinia/executor_test.clj b/test/com/walmartlabs/lacinia/executor_test.clj index ae30a846..dd178528 100644 --- a/test/com/walmartlabs/lacinia/executor_test.clj +++ b/test/com/walmartlabs/lacinia/executor_test.clj @@ -28,16 +28,22 @@ :objects {:Post {:implements [:Node] - :fields {:id {:type '(non-null String)} - :author {:type '(non-null :Author) - :resolve (fn [_ _ _] - {:id "2000"})} - :title {:type 'String - :resolve (fn [_ _ _] - "Hello, World!")} - :comments {:type '(list :Comment) - :resolve (fn [_ _ _] - [{:id "3000"}])}}} + :fields {:id {:type '(non-null String)} + :author {:type '(non-null :Author) + :resolve (fn [_ _ _] + {:id "2000"})} + :title {:type 'String + :resolve (fn [_ _ _] + "Hello, World!")}}} + + :PublicDomainPost + {:implements [:Node] + :fields {:id {:type '(non-null String)} + :author {:type :Author ;; Author is nullable + :resolve (fn [_ _ _] nil)} + :title {:type 'String + :resolve (fn [_ _ _] + "Epic of Gilgamesh")}}} :Author {:implements [:Node] @@ -50,18 +56,16 @@ nil)} :alwaysFail {:type '(non-null String) :resolve (fn [_ _ _] - (resolve-as nil {:message "This field can't be resolved."}))}}} - - :Comment - {:implements [:Node] - :fields {:id {:type '(non-null String)}}}} + (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)))}}}] + :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))) (deftest deep-merge-on-error @@ -160,4 +164,33 @@ fragment PostFragment2 on Post { 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