mkon/openapi_contracts

Validation for query parameters

Closed this issue ยท 12 comments

I'd like to have my OpenAPI contracts check the query params, similar to how they now check the request body.

Example

To build on the /pets path from the fixtures, I'd imagine a spec that lets you specify what type of pet you'd like and whether or not they're a good boi (even though we know they are).

Spec

paths:
  /pets:
    get:
      operationId: pets
      summary: Pets
      parameters:
        - description: type of animal to return
          in: query
          name: type
          required: true
          schema:
            type: string
            enum: [cat, dog]
        - description: Is good boi?
          in: query
          name: good_boi
          schema:
            type: boolean
      responses:
        '200':
          description: Ok
          content:
            application/json:
              schema:
                type: array
                items:
                  oneOf:
                    - $ref: 'components/schemas/Polymorphism.yaml#/Pet'

HTTP Call

Good!

GET /pets?type=dog&good_boi=true

Bad.

GET /pets?type=bird&good_boi=never
mkon commented

Hey @SalvatoreT, this is indeed not verified yet and it makes sense to implement it. I will see if I find time this week.

Thank you!

mkon commented

hey @SalvatoreT, do you want to test the gem from the branch validate-query-params to see if it covers your needs?

Parameter validation needs to be enabled with an option, for example

it { is_expected.to match_openapi_doc(doc, parameters: true) }

Hey @mkon, I'm working with @SalvatoreT on this, so I tested it out. First of all, thank you for your work! The gem has been very helpful for us and we appreciate the update :)

For the most part, this works great. I noticed an issue with non-string query params, however. Here's some query params that we have defined in our OpenAPI spec (v3.1.0):

in: query
name: page
style: deepObject
schema:
  type: object
  properties:
    after:
      description: Object ID for pagination cursor
      type: string
      examples:
        - obj_ABC123
    before:
      description: Object ID for pagination cursor
      examples:
        - obj_ABC123
      type: string
    size:
      description: Limit on the number of objects returned
      examples:
        - 5
      type: integer

For a spec that makes this request: GET /api/v1/object?page[size]=101
We receive this failure: {"size"=>"101"} is not a valid value for the query parameter "page"

Clearly, the problem is the value of size is getting encoded as a string in the URI and deserialized as a string as well. I did some looking and I think the way we have this defined in the OAS is correct and tooling should be able to validate non-string query params (for example).

mkon commented

Hey, thanks for trying this out. Yes indeed the issue is that query parameters always deserialise to a string which will make them invalid when comparing against the schema definition of integer. I will have to add some middle step that attempts some kind of casting based on the schema. Might take a couple of weeks time since I am soon out traveling.

No worries, I hope you enjoy your travels!

mkon commented

Hey @samanthawritescode and @SalvatoreT, sorry for the long delay. I updated the branch with a fix that utilizes ahx/openapi_parameters to cast the query parameters before validating them. Does this solve it for you?

Hey @mkon no worries, thanks for getting back to it! I just tested it and it looks great :)

I did find one "issue", but it might be a better fit for ahx/openapi_parameters. Here's the stack trace and repro steps. I say "issue" in quotes because in this case I am purposefully passing in an invalid value for the param to test the error case, so it's not necessary to do contract testing with parameters: true in this case. That said, better error handling would probably be nice.

Repro with these params in the spec:

{
  filter: 'badfilter'
}

OpenAPI:

- in: query
  name: filter
  style: deepObject
  schema:
    additionalProperties: false
    type: object
    properties:
      note:
        type: string
      created-at-start:
        format: date-time
        type: string
      created-at-end:
        format: date-time
        type: string 

Stack trace:

 Failure/Error: expect(response).to match_openapi_doc(openapi_doc, parameters: true, path: '/inquiries')

 NoMethodError:
   undefined method `each_with_object' for "badfilter":String

           object.each_with_object({}) do |(key, value), hsh|
                 ^^^^^^^^^^^^^^^^^
 # /usr/local/bundle/gems/openapi_parameters-0.3.3/lib/openapi_parameters/converter.rb:47:in `convert_object'
 # /usr/local/bundle/gems/openapi_parameters-0.3.3/lib/openapi_parameters/converter.rb:34:in `convert'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/doc/parameter.rb:22:in `matches?'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/validators/parameters.rb:12:in `block in validate'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/validators/parameters.rb:9:in `each'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/validators/parameters.rb:9:in `validate'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/validators/base.rb:15:in `call'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/validators/base.rb:20:in `call'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/match.rb:24:in `valid?'
 # /usr/local/bundle/bundler/gems/openapi_contracts-4bbd0590844c/lib/openapi_contracts/rspec.rb:10:in `block (2 levels) in <main>'
mkon commented

@samanthawritescode thanks for finding this edge case. I am now rescuing the error when conversion through ahx/openapi_parameters fails, it should lead to an error message that makes sense.

Maybe openapi_parameters should return the original value if it cannot be casted due to incompatible types.

I confirmed that it errors much more nicely in this case now. Thanks again, I will get this implemented on our end as soon as it's merged :)

ahx commented

Maybe openapi_parameters should return the original value if it cannot be casted due to incompatible types.

This is solved with v0.3.4

mkon commented

Released v0.14.0