WorksHub / leona

spec -> lacinia schema
Eclipse Public License 1.0
36 stars 17 forks source link

[1.17.0] Leona shouldn't fail to validate results when a required field is due to be provided by a field resolver #48

Open acron0 opened 3 years ago

acron0 commented 3 years ago

We have found a problem with Leona whereby if a field is specified as required (using :req-un in the spec) then if that field is not provided by the base resolver, Leona will fail the validation even if the field is due to be serviced by a field resolver.

(deftest field-in-spec-provided-by-field-resolver
  (defn local-resolver
    [& _]
    {:results
     [{:blog-ids  ["a" "b" "c"]
       :series-id "foobar"}]})

  (defn get-blogs-resolver
    [& _]
    [{:blog-id    "a"
      :blog-title "Alice"}
     {:blog-id    "b"
      :blog-title "Bob"}])

  (s/def ::series-id string?)
  (s/def ::blog-id string?)
  (s/def ::blog-ids (s/coll-of ::blog-id))
  (s/def ::blog-title string?)
  (s/def ::blog (s/keys :req-un [::blog-id ::blog-title]))
  (s/def ::blogs (s/coll-of ::blog))
  (s/def ::series (s/keys :req-un [::blog-ids ::series-id ::blogs]))
  (s/def ::results (s/coll-of ::series))
  (s/def ::series-results (s/keys :req-un [::results]))
  (s/def ::vertical string?)
  (s/def ::get-series-args (s/keys :opt-un [::vertical]))

  (let [r (-> (leona/create)
              (leona/attach-query ::get-series-args ::series-results local-resolver)
              (leona/attach-field-resolver ::blogs get-blogs-resolver)
              (leona/compile))
        x (leona/execute schema "query {series_results(vertical: \"Functional\") { results { series_id, blogs { blog_id } }}}")]
    (is (not (:errors x)))))

The above test will fail even though ::blogs has a field resolver attached.

Whilst it's clear to understand what's going on here, it feels disingenuous of Leona to punish the resolver for not returning a field that it knows is due to be serviced by a field resolver (Leona has that information in the context and schema).

Once we have the result of a resolver we should check any fields that appear in the spec that are serviced by field resolvers and exclude them from the validation (create a dynamic spec using spec-tools? or can we use s/merge in line? Not sure). Those field resolvers being excluded will be validated in isolation but because they only matched due to their appearance in the parent spec it should be sufficient that being validated in isolation means the parent is still valid.