Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dredd treats optional properties as required in OAS3 #1430

Open
mlesar opened this issue Jul 10, 2019 · 11 comments
Open

Dredd treats optional properties as required in OAS3 #1430

mlesar opened this issue Jul 10, 2019 · 11 comments

Comments

@mlesar
Copy link

mlesar commented Jul 10, 2019

⚠️ 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: #1430 (comment)


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
Copy link
Contributor

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

@mlesar
Copy link
Author

mlesar commented Jul 10, 2019

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
Copy link
Contributor

honzajavorek commented Jul 10, 2019

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 apiaryio/api-elements.js#263, but that's strange, as I don't think we're using valueOf in Dredd yet.

@apiaryio/adt any ideas about this?

@kylef
Copy link
Member

kylef commented Jul 10, 2019

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
Copy link
Contributor

@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
Copy link
Author

mlesar commented Jul 10, 2019

@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
Copy link
Contributor

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
Copy link
Author

mlesar commented Jul 10, 2019

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
Copy link
Contributor

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

@honzajavorek honzajavorek changed the title Optional properties in v11.2.10 Dredd treats optional properties as required in OAS3 Jul 10, 2019
@coatesap
Copy link

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
Copy link

eisenreich commented Nov 18, 2020

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

dblock added a commit to dblock/opensearch-api-specification that referenced this issue May 9, 2024
dblock added a commit to dblock/opensearch-api-specification that referenced this issue May 10, 2024
dblock added a commit to dblock/opensearch-api-specification that referenced this issue May 10, 2024
dblock added a commit to dblock/opensearch-api-specification that referenced this issue May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants