dredd icon indicating copy to clipboard operation
dredd copied to clipboard

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

Open victorsferreira opened this issue 6 years ago • 13 comments

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 avatar Apr 12 '19 11:04 victorsferreira

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 avatar Apr 12 '19 11:04 victorsferreira

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

victorsferreira avatar Apr 12 '19 12:04 victorsferreira

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

honzajavorek avatar Apr 12 '19 13:04 honzajavorek

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 ?

kardagan avatar Apr 25 '19 08:04 kardagan

@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.

honzajavorek avatar May 09 '19 11:05 honzajavorek

any progress here?

imissyouso avatar Aug 21 '19 11:08 imissyouso

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.

honzajavorek avatar Aug 22 '19 11:08 honzajavorek

any progress here?

gap777 avatar Dec 19 '19 21:12 gap777

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

joaosa avatar Jan 28 '20 16:01 joaosa

@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 🙂

honzajavorek avatar Jan 28 '20 16:01 honzajavorek

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.

kylef avatar Jan 28 '20 16:01 kylef

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 :)

joaosa avatar Jan 28 '20 18:01 joaosa

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.

kylef avatar Feb 17 '20 13:02 kylef