Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Enums can't be nullable #507

Open
honzajavorek opened this issue Jun 1, 2016 · 17 comments · May be fixed by #1648
Open

Enums can't be nullable #507

honzajavorek opened this issue Jun 1, 2016 · 17 comments · May be fixed by #1648

Comments

@honzajavorek
Copy link
Contributor

Problems with nullable enums were reported by multiple users in apiaryio/mson#61. We should investigate the problem.

@pete001
Copy link

pete001 commented Jun 19, 2016

We are running into the same issue, would be great to get this fixed.

pete001 pushed a commit to PerformanceHorizonGroup/apidocs that referenced this issue Jun 19, 2016
@matthewdias
Copy link

Any updates on this?

@honzajavorek
Copy link
Contributor Author

honzajavorek commented Nov 14, 2016

Not yet, thanks for bumping this. I didn't have time to investigate exactly what's the root cause of the problem. Since in apiaryio/mson#61 they report it should not be an issue in the parser, my first guess would be that dredd-transactions do not transform API Elements to transactions correctly. Any help or at least failing tests appreciated.

@JeppeKnockaert
Copy link

I've got an example reproducing this issue. I believe it is the generated JSON schema that is incorrect.

My apib file:

FORMAT: 1A

# Test API

### Test route [GET /test]

+ Response 200 (application/json)
    + Attributes (Item, fixed-type)

# Data Structures

## Item (object)

- type (Type, nullable)

## Type (enum)

- type1
- type2

Generated schema versus the tested response:
https://jsonschemalint.com/#/version/draft-04/markup/json?gist=86082c8c39b53359fe6835163fc65569

If I add null (without quotes) as one of the possible enum types, the generated JSON schema includes "null" (with quotes) as a possible enum type.

So, this:

## Type (enum)

- null
- type1
- type2

results in this generate schema versus the tested response:
https://jsonschemalint.com/#/version/draft-04/markup/json?gist=f80347a08900d60c4d264b9c8bd1ca79

Finally, I'd like to conclude with a working combination:
https://jsonschemalint.com/#/version/draft-04/markup/json?gist=458a3729bd732bd5109dbe1fc5d93619

@honzajavorek
Copy link
Contributor Author

@JeppeKnockaert Awesome! Thanks for the investigation and for filing apiaryio/drafter#455!

@honzajavorek
Copy link
Contributor Author

This was fixed in Drafter. Soon it should propagate to Dredd.

@kylef
Copy link
Member

kylef commented May 3, 2018

The fix from Drafter should be in Dredd by now, is there any remaining problems? Perhaps this issue can be closed.

@honzajavorek
Copy link
Contributor Author

honzajavorek commented May 4, 2018

As your "QA guy" (quoting @pksunkara) I'd like to have this (and this #283 (comment)) tested, at least superficially, in Dredd.

@octalmage
Copy link

I think I'm still having an issue with this and swagger:

      environment:
        type: string
        x-nullable: true
        enum:
        - production
        - staging
        - development

I get:

fail: body: At '/results/3/environment' Invalid type: null (expected string)

But if I add null to the enum, it works. Both x-nullable and null in the enum have to exist though.

@honzajavorek
Copy link
Contributor Author

@apiaryio/adt is the behavior described by @octalmage expected?

@pksunkara
Copy link
Contributor

Yes, x-nullable by default would mean null in enum. But when you provide a different enum, then they would contradict and enum gets priority since it is actually directly JSON Schema instead of an extension.

@kylef
Copy link
Member

kylef commented May 24, 2018

The x-nullable flag only adds null to the type of the JSON Schema. The problem here is when enum is present in JSON Schema, validators don't look at type since the type information isn't needed when you have typed-values. I think it makes sense for nullable to add null as a value to the enum when nullable is set and there is also enum. This logic should be added to fury-adapter-swagger. The specific code for this logic is found at https://github.com/apiaryio/fury-adapter-swagger/blob/46af39bbdd88b5a40910fb7d016e792e77299752/src/json-schema.js#L13-L17 and the tests at https://github.com/apiaryio/fury-adapter-swagger/blob/46af39bbdd88b5a40910fb7d016e792e77299752/test/json-schema.js#L53-L71 if anyone is interested in contributing this change as a PR. We should hopefully schedule this soon into one of our engineering sprints, but external contributions are always welcome.

@andybarilla
Copy link

I'm still having this issue whether I'm including null in the enum list or not.

Swagger example:

swagger: '2.0'
info:
  version: 1.0.0
  title: nullable enum
paths:
  /test:
    get:
      responses:
        200:
          description: OK
          schema:
            $ref: '#/definitions/NullableEnum'
        201:
          description: Created
          schema:
            $ref: '#/definitions/NullableEnumPlus'
definitions:
  NullableEnum:
    type: object
    additionalProperties: false
    properties:
      color:
        type: string
        x-nullable: true
        enum:
          - red
          - blue
          - green
  NullableEnumPlus:
    type: object
    additionalProperties: false
    properties:
      color:
        type: string
        x-nullable: true
        enum:
          - red
          - blue
          - green
          - null
      

Testing:

# npx dredd nullable-enum.yaml https://localhost --dry-run
error: Parser error in file 'nullable-enum.yaml': Cannot read property '$ref' of null on line 11
error: Parser error in file 'nullable-enum.yaml': Cannot read property '$ref' of null on line 15
error: Error when processing API description.

@kylef
Copy link
Member

kylef commented Jan 10, 2019

@andybarilla I've prepared a fix in apiaryio/api-elements.js#59, I hope you don't mind that I've taken parts of your example Swagger 2 document to reproduce in our test fixtures.

@honzajavorek
Copy link
Contributor Author

This is done, the only thing missing is an integration or e2e test in Dredd verifying this works as intended.

@kylef
Copy link
Member

kylef commented Nov 1, 2019

It would seem that in OpenAPI 3, the exact opposite to my prior comment on making nullable add null to enums,. OAI/OpenAPI-Specification#2050 where the proposal from TSC members to clarify the wording of nullable to become:

A true value adds "null" to the allowed type specified by the type keyword, only if type is explicitly defined within the same Schema Object. Other Schema Object constraints retain their defined behavior, and therefore may disallow the use of null as a value. A false value leaves the specified or default type unmodified. The default value is false.

Thus, nullable would only apply to the type, nullable keyword is a modifier of the type keyword. In particular the question at https://github.com/OAI/OpenAPI-Specification/blob/7d94b7c4ac0650318887d8f9588af50c7318a698/proposals/003_Clarify-Nullable.md#questions-answered

If a schema specifies nullable: true and enum: [1, 2, 3], does that schema allow null values?

No. The nullable: true assertion folds into the type assertion, which presumably specifies integer or number.

@artem-zakharchenko artem-zakharchenko linked a pull request Jan 20, 2020 that will close this issue
3 tasks
@dpashkevich
Copy link

Note that the nullable attribute is likely to go away in OAS 3.1

# OAS 3.0
type: string
nullable: true

# OAS 3.1 (unreleased)
type:
  - string
  - 'null'

See OAI/OpenAPI-Specification#2246

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants