apiaryio / dredd

Language-agnostic HTTP API Testing Tool
https://dredd.org
MIT License
4.18k stars 279 forks source link

Dredd treats optional properties as required in OAS3 #1430

Open mlesar opened 5 years ago

mlesar commented 5 years ago

⚠️ Note from @honzajavorek: This problem will stay with us for a while. Please remember the OAS3 adapter is still experimental. See workaround to this problem: https://github.com/apiaryio/dredd/issues/1430#issuecomment-510033479


Describe the bug After updating to the v11.2.10 my test started to fail with message: body: At '/by_day' Missing required property: by_day

To Reproduce I have an endpoint that is returning this response:

{
    "total_count": 1
}

Optionaly property by_day can be returned:

{
    "total_count": 1,
    "by_day": [
    {
        "date": "2019-07-10T00:00:00Z",
        "count": 10
    }]
}

Expected behavior Test is passing with the version v11.2.9.

What is in your dredd.yml?

color: true
dry-run: null
hookfiles: null
language: nodejs
require: null
server: null
server-wait: 3
init: false
custom: {}
names: false
only: []
reporter: []
output: []
header: []
sorted: false
user: null
inline-errors: false
details: false
method: []
loglevel: warning
path: []
hooks-worker-timeout: 5000
hooks-worker-connect-timeout: 1500
hooks-worker-connect-retry: 500
hooks-worker-after-connect-wait: 100
hooks-worker-term-timeout: 5000
hooks-worker-term-retry: 500
hooks-worker-handler-host: 127.0.0.1
hooks-worker-handler-port: 61321
config: ./dredd.yml
blueprint: openapi.yaml
endpoint: 'http://127.0.0.1:8080'

openapi.yaml

openapi: 3.0.1
info:
  title: Issue
  version: 1.0.0
paths:
  /:
    get:
      responses:
        "200":
          description: successful operation
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TestData'
components:
  schemas:
    TestData:
      type: object
      required:
        - total_count
      properties:
        total_count:
          type: integer
        by_day:
          type: array
          items:
            type: object
            properties:
              date: 
                type: string
                example: "2019-07-01T00:00:00Z"
              count:
                type: integer

What's your dredd --version output?

dredd v11.2.10 (Darwin 18.6.0; x64)

Does dredd --loglevel=debug uncover something? No

Did something major changed in the latest version or was my OpenAPI definition wrong from the start?

honzajavorek commented 5 years ago

@mlesar Thanks for reporting this! Just to be clear, you were on v11.2.9 previously and everything worked?

mlesar commented 5 years ago

Yes, even now if I do npm install -g dredd@11.2.9 this will go through with that version. This is also happening with Docker image apiaryio/dredd:11.2.10 and with previous image it is working fine(apiaryio/dredd:11.2.9)

honzajavorek commented 5 years ago

Thanks! I did some digging and dredd@11.2.9 should have fury-adapter-oas3-parser@0.8.1. Now dredd@11.2.10 has fury-adapter-oas3-parser@0.9.0. According to the changelog there have been no other versions in between, so the regression is happening in 0.9.0. That would point to https://github.com/apiaryio/api-elements.js/pull/263, but that's strange, as I don't think we're using valueOf in Dredd yet.

@apiaryio/adt any ideas about this?

kylef commented 5 years ago

In older versions of the adapter, we did not produce a JSON examples and thus Dredd would not attempt to validate the body. In fury-adapter-oas3-parser 0.7.0 we've started generating JSON bodies from the Schema Object provided. As of the fury-adapter-oas3-parser 0.9.0 the generated message body will be: {"total_count":0, "by_day": [{"date": "2019-07-01T00:00:00Z","count": 0}]}.

I believe the problem here is that Dredd treats properties as "required" by default in JSON bodies providing they have a value in the example message body. Thus I think this is working as designed (by Adam). The particular scenario in Gavel testing this behaviour is at https://github.com/apiaryio/gavel-spec/blob/b4deb632f2fc49a92d5fee57c7547804a2136fa2/features/expectations/bodyJsonExample.feature#L21.

Dredd doesn't support JSON Schema Draft 6/7 or newer which will be required to support schemas in OpenAPI 3.

honzajavorek commented 5 years ago

@kylef If I read you correctly, this means Dredd won't work well for users until the OAS3 adapter starts to produce JSON Schemas apart from the JSON examples?

mlesar commented 5 years ago

@kylef would that mean that if I remove example: "2019-07-01T00:00:00Z" my test should pass? Because I have tried that and I'm getting the same error message.

honzajavorek commented 5 years ago

No, the problem is that if Dredd gets only a JSON example from the API description, it infers assertions for validation - it basically makes every property in the JSON object required and it ignores values.

In most cases this situation doesn't happen, because the OAS2 adapter gives Dredd not only examples, but also corresponding JSON Schema, where there are more details about the expectations (like what is required and what is not). The same happens for API Blueprint if people use MSON (Attributes section) for describing the bodies, the parser/adapter generates JSON Schema for each body and Dredd uses it for validation.

In this case, the experimental OAS3 adapter generates examples, but not schemas, thus Dredd falls back into making its own assumptions about the bodies and thinks all properties are required. Relevant section of the docs: http://dredd.org/en/latest/how-it-works.html#response-body-expectations

mlesar commented 5 years ago

Thank you @honzajavorek for the explanation, when I added example response it worked fine:

openapi: 3.0.1
info:
  title: Issue
  version: 1.0.0
paths:
  /:
    get:
      responses:
        "200":
          description: successful operation
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TestData'
              examples:
                application/json:
                  value:
                    total_count: 1
components:
  schemas:
    TestData:
      type: object
      required:
        - total_count
      properties:
        total_count:
          type: integer
        by_day:
          type: array
          items:
            type: object
            properties:
              date: 
                type: string
              count:
                type: integer
honzajavorek commented 5 years ago

@mlesar You are right, I didn't realize this. When you provide your own example, you prevent the OAS3 adapter to generate one with optional properties included and Dredd's assertions will be inferred from that (your own) example. This is a viable workaround for the time being.

coatesap commented 4 years ago

I really hope this is fixed in the not too distant future. The workaround does work and may be fine for simple objects, but it becomes a huge amount of work when your objects have many properties, and the examples are needed in many places.

eisenreich commented 3 years ago

So to clarify the workaround for all followings: You need to add a example to the request with only required attributes.

I can confirm the workaround is working. In my case I had a endpoint that return a list of a complex object which was a little bit more tricky to get a example running, so I share it with you:

  /cars:
    get:
      tags:
        - cars
      summary: Get a list of cars
      description: Returns all cars
      operationId: getCars
      responses:
        200:
          description: Return all cars
          content:
            application/json;charset=UTF-8:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Cars'
              example:
                - id: 1005
                  position:
                    x: 10.36
                    y: 10.93
                    z: 10
                  displayContent: 
                    template: empty
...

    Car:
      type: object
      required:
        - id
        - position
        - displayContent
      properties:
        id:
          type: number
        position:
          $ref: '#/components/schemas/Position'
        displayContent:
          $ref: '#/components/schemas/DisplayContent'
    DisplayContent:
      required:
        - template
      type: object
      properties:
        template:
          type: string
          enum:
          - none
          - default
          - custom
        text1:
          type: string
        text2:
          type: string
        text3:
          type: string

In this case the displayContent tells what is currently displayed. Per default it's empty, but when you set it there are additional properties (optional). With the example above dredd is back working.

Update: It's also working with referencing the example and reusing it - if you like to know let me know