Required response parameters not validated if they are only in $ref in definitions

victorsferreira commented 5 years ago

OpenAPI 2.0

In order to force field validations I am adding the required array to my response bodies.

If I add the required field just below the response status code (200, for example), it works.

responses:
        200:
          description: "200 OK"
          schema:
            type: object
            properties:
              data:
                $ref: "#/definitions/SomeDefinition"
            required:
              - data

SomeDefinition could contain the required field as well and as I could see it will validate those fields.

But if I only create a reference with its required fields, it won`t validate

responses:
        200:
          description: "200 OK"
          schema:
            $ref: "#/definitions/SomeDefinition"

If I copy and paste the content of SomeDefinition and put it just below 200, it will work validate the required fields.

Any workarounds, am I creating a bad documenation?

SomeDefinition

SomeDefinition:
    type: "object"
    properties:
      id:
        type: "number"
   required:
        - id

Expected behavior: Required fields should be validated even when it's only referenced, as well as it is when it is directly put inside the response.

victorsferreira commented 5 years ago

I just made extra tests.

The problem occurs when I add a reference inside another reference.

If SomeDefinition has no other reference, the fields are validated. In my case, the test fails (I'm forcing an error).

If SomeDefinition references something else, for instance, SomeOtherDefinition, then it stops validating and the test pass. This is wrong

SomeDefinition:
    type: "object"
    properties:
      id:
        type: "number"
      other:
        $ref: "#/definitions/SomeOtherDefinition"
   required:
        - id

It doesnt matter whether or not SomeOtherDefinition has a required statement

@honzajavorek should I update the original post?

victorsferreira commented 5 years ago

If I get the expected bodySchema and validate it against the response with jsonschema, I get the error I was expecting.

not sure if Dredd is failing silently and letting the test pass, if it is not trying to validate for some reason or if the library used to test is not similar to the one I'm using

honzajavorek commented 5 years ago

Seems like it could be a bug in our JSON Schema validation library. This might take a while to address 😞

kardagan commented 5 years ago

Hi, I have a similar problem

with this, dredd dont see any request to test

swagger.yaml

paths:
  /v1/attribute/{attribute_id}:
    $ref: ./api/attribute.yaml#/attribute

api/attribute.yaml

attribute:
  get:
    tags:
    - attribute
    summary: get attribute infos
    parameters:
    - name: attribute_id
      in: path
      required: true
      type: integer
      default: 1
    responses:
      '200':
        description: attribute's info
        schema:
          $ref: '#/definitions/attribute'

but with this, it's work.

paths:
  /v1/attribute/{attribute_id}:
    get:
      tags:
        - attribute
      summary: get attribute infos
      parameters:
        - name: attribute_id
          in: path
          default: 1
          required: true
          type: integer
      responses:
        '200':
          $ref: './api/attribute.yaml#/attribute/get/responses/200'

Is the problem is same or I create a new issue ?

honzajavorek commented 5 years ago

@kardagan looks same to me. We're cleaning up Gavel (the project which does validation for Dredd) codebase with @artem-zakharchenko now and once that's done, hopefully we can start addressing all the issues labeled as validation here.

imissyouso commented 5 years ago

any progress here?

honzajavorek commented 5 years ago

Yes, we're finishing the major refactoring in exactly these days. It has been a long journey given how many things depend on Gavel in Dredd and the Apiary stack. Now tackling these bugs should be easier for everyone.

gap777 commented 4 years ago

any progress here?

joaosa commented 4 years ago

@honzajavorek wondering if there are any news regarding this :)

honzajavorek commented 4 years ago

@joaosa I'm not working on Dredd anymore, so I'm not aware of its current roadmap. I'm sure the maintainers will get back to you though 🙂

kylef commented 4 years ago

We have work in progress to support this, we're rehauling our JSON Schema validations in gavel.js, the underlying library used in Dredd for validations. We shouldn't be too far off resolving this and countless other problems with JSON Schema handling.

joaosa commented 4 years ago

We have work in progress to support this, we're rehauling our JSON Schema validations in gavel.js, the underlying library used in Dredd for validations. We shouldn't be too far off resolving this and countless other problems with JSON Schema handling.

Thank you for the prompt response. That's great news :)

kylef commented 4 years ago

I believe this problem has been resolved in Dredd 13.0.1, @joaosa / @victorsferreir could you please confirm in your cases after upgrading to Dredd 13.0.1 or newer.

@kardagan your particular example is making use of file referencing which is not supported. That's unrealted to this bug, but instead a feature request so I've moved it out to https://github.com/apiaryio/dredd/issues/1674 for tracking.

apiaryio / dredd

Required response parameters not validated if they are only in $ref in definitions #1329