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

Nullable properties with a default of null are not possible according to the 3.0 spec #2057

Open
johncmunson opened this issue Nov 17, 2019 · 2 comments

Comments

@johncmunson
Copy link

@johncmunson johncmunson commented Nov 17, 2019

From the spec...

default - The default value represents what would be assumed by the consumer of the input as the value of the schema if one is not provided. Unlike JSON Schema, the value MUST conform to the defined type for the Schema Object defined at the same level. For example, if type is string, then default can be "foo" but cannot be 1.

This, combined with the fact that properties can be nullable, means that it's not possible to have a property with a default value of null.

You can see below that the folks over at Swagger have interpreted the spec literally, as they should. It's the spec that seems to be wrong.

Here's a sample response body on my /todos/{id} route in the Swagger UI...

{
  "id": 0,
  "title": "string",
  "complete": false,
  "note": "null",
  "dueDate": "null",
  "categoryId": 0
}

And here's the OpenAPI schema for my todo objects...

Todo:
  description: A todo item helps you track what you need to get done
  type: object
  required:
    - title
    - complete
  properties:
    id:
      type: integer
    title:
      type: string
    complete:
      type: boolean
      default: false
    note:
      type: string
      nullable: true
      default: null
    dueDate:
      type: string
      nullable: true
      default: null
    categoryId:
      type: integer
      nullable: true
      default: null

Looking at the note property, we can see that the default value of null gets interpreted as the string "null".

Looking at categoryId, it seems that 0 is the closest integer to null they could think of.

@tedepstein

This comment has been minimized.

Copy link
Contributor

@tedepstein tedepstein commented Nov 17, 2019

@johncmunson , as I read the spec, it is only saying that the default value must conform to the type specified in the schema.

Unlike JSON Schema, the value MUST conform to the defined type for the Schema Object defined at the same level.

This is just to contrast the requirement with JSON Schema's less stringent guidance:

It is RECOMMENDED that a default value be valid against the associated schema.

As we're attempting to clarify in this proposal, nullable should function as a type modifier. Under that interpretation, your note Schema Object would translate to a JSON Schema with "type" : ["string", "null"]. In that case, "default" : null should be interpreted as a literal null value, not the string "null", and it should be allowed.

I'm not sure which Swagger component you were referring to. I would say that the behavior of this Swagger component is wrong; it should allow null as a default value. But honestly, the current spec is very unclear as to how nullable should be interpreted, and how it should interact with other keywords. This is yet another case of that, so I'll add it to the proposal for completeness.

Until the proposal is accepted and the v3.0.3 patch of the specification is released, we're kind of in a gray area here. But you might raise the issue in the appropriate Swagger project, and see if they agree that they should change the default validation logic. They might want to make this change even before the proposed clarification is official.

@johncmunson

This comment has been minimized.

Copy link
Author

@johncmunson johncmunson commented Nov 17, 2019

@tedepstein thanks for some of the background details surrounding this. I'm glad that it's on the team's radar to clear up some of the ambiguity surrounding null values. After your comments, I now see it from your perspective that the spec is a little ambiguous in this regard, rather than contradicting or wrong. I will raise this issue in the appropriate Swagger repo and see what their perspective is on this. Thanks!

Update:
Here is the related issue I created for Swagger.

darrelmiller added a commit that referenced this issue Nov 21, 2019
* Addressing #1389, and clearing the way for PRs #2046 and #1977. This adds a formal proposal to clarify the semantics of nullable, providing the necessary background and links to related resources.

* Corrected table formatting.

* Minor tweaks and corrections.

* Correct Change Log heading.

* Cleaned up notes about translation to JSON Schema and *Of inheritance semantics.

* Fix Change Log heading in the proposal template.

* Snappy answers to stupid questions.

* Change single-quote 'null' to double-quote "null"

Thanks, @handrews for the review.

* Clarified the proposed definition of nullable

Somehow in our collaborative editing, we neglected to state that `nullable` adds `"null"` to the set of allowed types. We just said that it "expands" the `type` value, but didn't state the obvious (to us) manner of said expansion. Correcting that oversight in this commit.

* Corrected the alternative, heavy-handed interpretation of nullable.

* Added more explicit detail about the primary use case.

* Added a more complete explanation of the problems created by disallowing nulls by default.

* Added issue #2057 - interactions between nullable and default value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.