Hilzu / express-openapi-validate

Express middleware to validate requests based on an OpenAPI 3 document
Apache License 2.0
75 stars 12 forks source link

Does not validate a route with 3 required parameters in url #43

Closed cmartin81 closed 5 years ago

cmartin81 commented 5 years ago

Hi, When I try to validate a url with 3 url parameters which is required I get an error saying that I have to include the urlparameter.

Example:

/foo/{fooId}/bar/{barId}/foobar/{foobarId}

swagger.file:

/v1/foo/{fooId}/bar/{barId}/foobar/{foobarId}:
parameters:
  - $ref: '#/components/parameters/fooIdPathInput'
  - $ref: '#/components/parameters/barIdPathInput'
  # - $ref: '#/components/parameters/foobarIdPathInput'  // does not work with required with 3 params
  - name: foobarId
    in: path
    required: false   # <---- Only works if I set this to false (this is the 3. parameter)
    schema:
      type: string
      format: uuid
Hilzu commented 5 years ago

Tried reproducing the issue with this test case but couldn't: https://github.com/Hilzu/express-openapi-validate/commit/09032d7b360efb6354b40a2ccad7d8e4d748894b There's probably something more complicated happening. Can you make that test case fail somehow? Also can you include an stack trace of the exact error that you get?

Hilzu commented 5 years ago

Also tried to reproduce the issue using an integration test but still can't get it to fail: https://github.com/Hilzu/express-openapi-validate/compare/issue/43

cmartin81 commented 5 years ago

Thanks for looking into it. I have forked a version and did some changes in the tests which I could not get to run green. Please have a look at it here: https://github.com/cmartin81/express-openapi-validate/tree/issue/43

Hilzu commented 5 years ago

With your changes only the integration test is failing.

  ● Integration tests with real app › several path parameters are validated -- POST

    expect(received).toBe(expected) // Object.is equality

    Expected: 200
    Received: 404

      90 |
      91 |     const res = await request(app).post(`/parameters/several/${uuid1}/a/${uuid2}/b/${uuid3}`).send({bar: 'foobar', baz: 'foobaz'});
    > 92 |     expect(res.status).toBe(200);
         |                        ^
      93 |     expect(res.body).toEqual({ a: uuid1, b: uuid2, c: uuid3 });
      94 |   });

The status code is 404 because the integration test app at test/integration/app.ts is only setup to handle GET requests to that URL. If the error would be caused by validation the status code would be 400. If you copy the route handler starting from line 50 and change app.get to app.post the tests passes.

cmartin81 commented 5 years ago

Thank you! I have tried to reproduce my problem without any luck.... So thanks for excellent help and I'll close this issue.