oliyh / martian

The HTTP abstraction library for Clojure/script, supporting OpenAPI, Swagger, Schema, re-frame and more
MIT License
529 stars 44 forks source link

disallowed-key / missing-required-key bipolar schema validation errors involving a path param #126

Closed enspritz closed 2 years ago

enspritz commented 2 years ago

Using cljs-http to perform a GET on a path that requires a single string path parameter. No matter how the call is written, I've thus far failed to identify a call to Martian that would work. This is the call:

(martian/response-for m "get-status-in-linked-app" {"application-id" "abc-123-xyz-789"})

There are other, identical REST calling patterns in the API and Martian appears to function fine with those, but it somehow fails this call. I have combed over and cross-compared the OpenAPI annotations on the relevant Java methods; I noted that all appear pattern-wise identical, with the string identifiers being the only material difference.

The call functions as expected in the browser, with CURL, etc.

; Calling pattern:

(martian/response-for m "get-status-in-linked-app" {"application-id" "abc-123-xyz-789"})

; yields, contrary to my expectations:

#error {
    :message "Interceptor Exception: Could not coerce value to schema: {:application-id disallowed-key}",      
    :data {
        :execution-id #uuid "1a840990-04d4-405e-8c13-bd2d226f6210",
        :stage :enter,
        :interceptor :martian.interceptors/query-params, 
        :type :schema-tools.coerce/error, 
        :exception #error {
            :message "Could not coerce value to schema: {:application-id disallowed-key}", 
            :data {
                :type :schema-tools.coerce/error, 
                :schema {}, 
                :value {:application-id "9f2d636e-c842-3388-8a66-17c1b951dd45"}, 
                :error {:application-id disallowed-key}}},
                :schema {}, 
                :value {:application-id "9f2d636e-c842-3388-8a66-17c1b951dd45"},
                :error {:application-id disallowed-key}}, 
            :cause #error {
            :message "Could not coerce value to schema: {:application-id disallowed-key}",
            :data {:type :schema-tools.coerce/error, 
                :schema {
                }, 
                :value {:application-id "9f2d636e-c842-3388-8a66-17c1b951dd45"}, 
                :error {:application-id disallowed-key}}}}
; Calling patterns:

(martian/response-for m "get-status-in-linked-app" )      ; INCORRECT CALL
(martian/response-for m "get-status-in-linked-app" {})    ; INCORRECT CALL

; yields as expected:

#error {
    :message "Interceptor Exception: Could not coerce value to schema: {:application-id missing-required-key}", 
    :data {
        :execution-id #uuid "f1cd024b-7fbd-4e95-966e-eababb35251b", 
        :stage :enter,
        :interceptor :martian.interceptors/url, 
        :type :schema-tools.coerce/error, 
        :exception #error {
            :message "Could not coerce value to schema: {:application-id missing-required-key}", 
            :data {
                :type :schema-tools.coerce/error, 
                :schema {
                :application-id #schema.core.Predicate{:p? #object[Uc], :pred-name string?}}, 
                :value {}, 
                :error {:application-id missing-required-key}}}, 
                :schema {:application-id #schema.core.Predicate{:p? #object[Uc], :pred-name string?}}, 
                :value {},
                :error {
                :application-id missing-required-key}}, 
            :cause #error {
            :message "Could not coerce value to schema: {:application-id missing-required-key}", 
            :data {:type :schema-tools.coerce/error, 
                :schema {
                :application-id #schema.core.Predicate{:p? #object[Uc], :pred-name string?}}, 
                :value {}, 
                :error {:application-id missing-required-key}}}}
; Using the clj-http REPL to test different, erroneous calling patterns:

user=> (martian/response-for m :get-status-in-linked-app {:application-id nil})    ; INCORRECT
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:application-id (not (instance? java.lang.String nil))}

; We seem to be able to conclude that Martian somewhere expects a string value for :application-id.

user => (martian/response-for m :get-status-in-linked-app {:application-id 123})    ; INCORRECT
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:application-id (not (instance? java.lang.String 123))}

; As expected.

This is the relevant snippet from the OpenAPI 3.0.1 document:

   "/integrity/linked-app-status/{application-id}":{
      "get":{
         "summary":"Report status in the linked application",
         "operationId":"get-status-in-linked-app",
         "parameters":[
            {
               "name":"application-id",
               "in":"path",
               "description":"The ID of the linked application",
               "required":true,
               "schema":{
                  "type":"string"
               }
            }
         ],
         "responses":{
            "200":{
               "description":"Description of the status of the linked app",
               "content":{
                  "application/json":{
                     "schema":{
                        "type":"string",
                        "format":"application/json"
                     }
                  }
               }
            }
         }
      }
   }

The Java implementation:

@Path("integrity")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public class IntegrityResource {

    @GET
    @Path("linked-app-status/{application-id}")
    @Operation(
            summary = "Report status of the linked application",
            operationId = "get-status-in-linked-app"
    )
    @ApiResponse(
            responseCode = "200", description = "Describes status of the linked app",
            content = @Content(schema = @Schema(implementation = String.class, format = MediaType.APPLICATION_JSON))
    )
    public Response getStatusInLinkedApp(
            @Parameter(
                    description = "The ID of the linked application",
                    required = true
            )
            @PathParam("application-id")
            final String applicationIdRaw
    ) {
        // ... Actual implementation ...
    }

}

Changing from a path param to a query param results in the same error pattern. I notice that :schema in each #error differs.

And, Happy New Year Oliyh! Thanks for all the work you have put into this tooling.

oliyh commented 2 years ago

Hello,

Thanks for the detailed write up.

Two things I can suggest right now:

  1. I'd normally expect you to use a keyword to address the route, i.e. :get-status-in-linked-app instead of the string
  2. You can explore the routes using martian.core/explore to see what routes martian knows about. There are two arities:
    • (martian.core/explore m) to see if it knows about your route, and what it's called
    • (martian.core/explore m :get-status-in-linked-app) to see what input schema martian sees for that route

This should help you debug a bit further. The martian instance itself is just a normal data structure anyway so you can dig into it as well, although that's mostly what explore does anyway.

Hope this helps

enspritz commented 2 years ago

Sorry, some identifiers have changed slightly, but it makes no material difference and hopefully they read the same anyways.

Addressing your suggestion #1, yes, I noticed that after submitting. Using keywords now.

Suggestion #2, utilizing martian.core/explore at the REPL:

; martian.core/explore

user=> (pprint (martian.core/explore m))
[[:get-linked-app-status
  "Report status in the linked application"]
 ...]
nil
user=> (pprint (martian.core/explore m :get-linked-app-status))
{:summary "Report status in the linked application",
 :parameters {:application-id java.lang.String},
 :returns {200 java.lang.String}}
nil

; And, the relevant snippet from the martian instance data structure:

user=> (pprint (nth (get m :handlers) 2))
{:description nil,
 :method :get,
 :produces ["application/json"],
 :path-schema {:application-id java.lang.String},
 :query-schema {},
 :parameter-aliases
 {:path-schema {},
  :query-schema {},
  :body-schema {},
  :form-schema {},
  :headers-schema {}},
 :form-schema {},
 :path-parts ["/integrity/linked-app-status/" :application-id],
 :headers-schema {},
 :openapi-definition
 {:tags ["integrity"],
  :summary "Report status in the linked application",
  :operationId "get-linked-app-status",
  :parameters
  [{:name "application-id",
    :in "path",
    :description "ID of the linked application",
    :required true,
    :schema {:type "string"}}],
  :responses
  {:200
   {:description
    "Description of the status in the linked app",
    :content
    #:application{:json
                  {:schema
                   {:type "string", :format "application/json"}}}}}},
 :consumes [nil],
 :summary "Report status in the linked application",
 :body-schema nil,
 :route-name :get-linked-app-status,
 :response-schemas [{:status (eq 200), :body java.lang.String}]}
nil

; Let's try exercising the API call.

user=> (pprint (martian.core/request-for m :get-linked-app-status {:application-id "abc-123"}))
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:application-id disallowed-key}

user=> (pprint (martian.core/request-for m :get-linked-app-status))
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:application-id missing-required-key}

user=> (pprint (martian.core/request-for m :get-linked-app-status nil))
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: (not (map? nil))

user=> (pprint (martian.core/request-for m :get-linked-app-status {}))
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:application-id missing-required-key}

user=> (pprint (martian.core/request-for m :get-linked-app-status {:abc 123}))
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:application-id missing-required-key}
enspritz commented 2 years ago

The above info uses lein dependency [com.github.oliyh/martian-clj-http "0.1.18"].

The following is with lein dependency [martian-clj-http "0.1.15"] :

user=> (martian.core/request-for m :get-linked-app-status {:application-id "abc-123"})
{:method :get, :url "http://localhost:8080/integrity/linked-app-status/abc-123", :as :text, :headers {"Accept" "application/json"}}
enspritz commented 2 years ago

More info: Running a manual bisect on the martian version, it works fine up to 0.1.16. From 0.1.17 it fails as in the above.

Sample session, showing correct behavior with martian 0.1.16:

$ vim project.clj
; ... Change the martian-clj-http dep to 0.1.16 ...
:ZZ
$ lein repl
Retrieving martian-clj-http/martian-clj-http/0.1.16/martian-clj-http-0.1.16.pom from clojars
Retrieving martian-clj-http/martian-clj-http/0.1.16/martian-clj-http-0.1.16.jar from clojars
nREPL server started on port 36165 on host 127.0.0.1 - nrepl://127.0.0.1:36165
REPL-y 0.5.1, nREPL 0.8.3
Clojure 1.10.1
OpenJDK 64-Bit Server VM 1.8.0_312-b07
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (require '[martian.clj-http :as martian-http])
nil
user=> (def m (martian-http/bootstrap-openapi "http://localhost:8080/api.json"))
#'user/m
user=> (martian.core/request-for m :get-linked-app-status {:application-id "abc-123"})
{:method :get, :url "http://localhost:8080/integrity/linked-app-status/abc-123", :as :text, :headers {"Accept" "application/json"}}
bombaywalla commented 2 years ago

I am running into a similar problem. I've managed to whittle down the error to a small repro.

user> (require '[martian.hato :as martian-http] :reload)
nil
user> (def m (martian-http/bootstrap-openapi "http://localhost:8090/repro.json"))
#'user/m
user> (require '[martian.core :as martian] :reload)
nil
user> (martian/explore m)
[[:list-items "Gets a list of items."]]
user> (martian/explore m :list-items)
{:summary "Gets a list of items.",
 :parameters {{:k :client-id} java.lang.String},
 :returns {200 [java.lang.String]}}
user> (martian/request-for m :list-items {:client-id "12345"})
Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:client-id disallowed-key}
user> 

The file repro.json is:

{"openapi": "3.1.0",
 "info":
 {"version": "1.2.3",
  "title": "Repro",
  "description": "Reproduction of issue"},
 "paths":
 {"/items":
  {"get":
   {"summary": "Gets a list of items.",
    "description": "Filler",
    "operationId": "list-items",
    "parameters":
    [{"in": "query",
      "name": "ClientId",
      "description": "Client identifier",
      "schema": {"type": "string"},
      "required": false}],
    "responses":
    {"200":
     {"description": "Success.",
      "content":
      {"application/json":
       {"schema":
        {"type": "array",
         "maxItems": 5000,
         "items": {"type": "string"}}}}}}}}},
 "servers":
 [{"url": "https://sandbox.example.com", "description": "Sandbox"}]}

Latest versions of martian and martian-hato`.

Curiously, the following swagger definition works just fine.

{"swagger":"2.0",
 "info":
 {"title":"Repro that works!",
  "version":"2.0",
  "description":"Working version."},
 "produces":["application/json"],
 "consumes":["application/json"],
 "paths":
 {"/items":
  {"get":
   {"operationId":"list-items",
    "responses":
    {"200":{"schema":{"type":"array","maxItems":5000,"items":{"type":"string"}},
            "description":"Success"}},
    "parameters":
    [{"in":"header",
      "name":"ClientId",
      "description":"Client identifier",
      "required":false,
      "type":"string"}],
    "summary":"Gets a list of items."}}},
 "basePath":""}
bombaywalla commented 2 years ago

One difference I found was that for the non-working version explore returns:

user> (martian/explore m :list-items)
{:summary "Gets a list of items.",
 :parameters {{:k :client-id} java.lang.String},
 :returns {200 [java.lang.String]}}

And for the working version, we get:

user> (martian/explore mp :list-items)
{:summary "Gets a list of items.",
 :parameters {{:k :client-id} (maybe Str)},
 :returns {200 [java.lang.String]}}

Note the difference in the schema/type for the parameter.

oliyh commented 2 years ago

Hi @bombaywalla

Thank you for the investigation, that seems to hint at something. I haven't had a chance to look into this yet but hopefully will soon.

Thanks

oliyh commented 2 years ago

Hi,

Could you try 0.1.21-SNAPSHOT? I believe the issue is fixed (it was specific to OpenAPI parsing)

bombaywalla commented 2 years ago

My repro test case (as above) now works. Thanks!