(s/and …) seems to trip up query checking

[eneroth]

Here's the spec for the query parameters:

(s/def ::customer-id     spec/string?)
(s/def ::requestor-id    spec/string?)
(s/def ::requestor-email spec/string?)
(s/def ::requestor-name  spec/string?)
(s/def ::endpoint        spec/string?)
(s/def ::from-year       spec/int?)
(s/def ::from-month      spec/int?)
(s/def ::to-year         spec/int?)
(s/def ::to-month        spec/int?)

(s/def ::input-settings (s/keys :req-un [::endpoint
                                         ::customer-id
                                         ::requestor-id]
                                :opt-un [::from-year
                                         ::from-month
                                         ::to-year
                                         ::to-month
                                         ::requestor-email
                                         ::requestor-name]))

Here's the API definition:

(def app
  (api
   {:swagger
    {:ui "/"
     :spec "/swagger.json"
     :data {:info {:title "Futomaki"
                   :description "API for counter stats over the Sushi protocol"}
            :tags [{:name "Reports", :description "Retrieve information per report definition"}]}}}

   (context "/api" []
     :tags ["api"]
     :coercion :spec

     (context "/jr1" []
       (resource
         {:post
          {:summary "Number of successful full-text article requests by month and journal"
           :parameters {:query-params ::input-settings}
           :responses {200 {:schema ::response-200}
                       500 {:schema :futomaki.sushi.parse.exception/exception}}
           :handler (fn [{:keys [headers query-params]}]
                      (let [d (d/deferred)]
                        (future
                          (let [{:keys [error data metadata]} (reports/get-jr1 query-params)]
                            (d/success! d (ok {:report-items data
                                               :metadata metadata}))))
                        d))}}))

The above works fine. Now I want to do some more rigorous checking of the query parameters, so I change the query spec to this:

(s/def ::input-settings (s/and (s/keys :req-un [::endpoint
                                                ::customer-id
                                                ::requestor-id]
                                       :opt-un [::from-year
                                                ::from-month
                                                ::to-year
                                                ::to-month
                                                ::requestor-email
                                                ::requestor-name])
                               date/validate-to-date
                               date/validate-from-date
                               date/validate-both-dates))

This makes spec complain that something is invalid, but it's curiously unspecific as to what. I thought maybe one of my predicates was throwing it off, so I tried this:

(s/def ::input-settings (s/and (s/keys :req-un [::endpoint
                                                ::customer-id
                                                ::requestor-id]
                                       :opt-un [::from-year
                                                ::from-month
                                                ::to-year
                                                ::to-month
                                                ::requestor-email
                                                ::requestor-name])))

It still throws the same error. Here's the error, in JSON:

{
  "spec": "(spec-tools.core/spec {:spec (clojure.spec.alpha/and (clojure.spec.alpha/keys :req-un [:futomaki.api.handler/endpoint :futomaki.api.handler/customer-id :futomaki.api.handler/requestor-id] :opt-un [:futomaki.api.handler/from-year :futomaki.api.handler/from-month :futomaki.api.handler/to-year :futomaki.api.handler/to-month :futomaki.api.handler/requestor-email :futomaki.api.handler/requestor-name])), :type :map})",
  "problems": [
    {
      "path": [],
      "pred": [
        "clojure.spec.alpha/and",
        [
          "clojure.spec.alpha/keys",
          "req-un",
          [
            "futomaki.api.handler/endpoint",
            "futomaki.api.handler/customer-id",
            "futomaki.api.handler/requestor-id"
          ],
          "opt-un",
          [
            "futomaki.api.handler/from-year",
            "futomaki.api.handler/from-month",
            "futomaki.api.handler/to-year",
            "futomaki.api.handler/to-month",
            "futomaki.api.handler/requestor-email",
            "futomaki.api.handler/requestor-name"
          ]
        ]
      ],
      "val": {
        "endpoint": "http://sushi.cambridge.org/GetReport",
        "customer-id": "abc",
        "requestor-id": "abc"
      },
      "via": [
        "futomaki.api.handler/input-settings"
      ],
      "in": []
    }
  ],
  "type": "compojure.api.exception/request-validation",
  "coercion": "spec",
  "value": {
    "endpoint": "http://sushi.cambridge.org/GetReport",
    "customer-id": "abc",
    "requestor-id": "abc"
  },
  "in": [
    "request",
    "query-params"
  ]
}

If I check it manually with the same values, no error is thrown:

(s/valid? ::input-settings {:endpoint "http://sushi.cambridge.org/GetReport"
                            :customer-id "abc"
                            :requestor-id "abc"})

=> true

[carocad]

@eneroth I think I had a similar error one time. Are you by any change using conformer ? in your validate predicates?

In my case I was using conformers in the validate functions and since spec-tools still dont support them then I was getting those very cryptic errors.

[ikitommi]

Sorry for the lag, we have been super busy with everything. Will try to resolve this next week.

[ikitommi]

The root cause was in spec-tools: s/and was configured to resolve spec type out of the first spec in it, but the extra information (e.g. keys from s/keys to enable dropping extra keys) was taken from itself. Here, it thought that the (s/and (s/keys...)) was a :map with no keys => run a select-keys while dropping all keys. Refactored spec-tools so, that there is one multimethod instead of two: does both type-resolution & extracts extra info. Less change of messing up.

if you update to latest spec-tools, this should have gone away. And there is a regression test on compojure-api side too.

[metosin/spec-tools "0.4.0-20170910.061347-1"]

[ikitommi]

and here's the new versions for s/and:

(defmethod extract-from-form 'clojure.spec.alpha/and [_ form]
  (extract (second form)))

and s/keys:

(defmethod extract-from-form 'clojure.spec.alpha/keys [_ form]
  (let [{:keys [req opt req-un opt-un]} (some->> form (rest) (apply hash-map))]
    {:type :map
     :keys (set
             (flatten
               (concat
                 (map impl/polish (concat req opt))
                 (map impl/polish-un (concat req-un opt-un)))))}))