WorksHub/leona

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

acron0 opened this issue · 0 comments

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.