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

Consistent empty / null fields #258

Open
smyrman opened this issue Sep 4, 2019 · 4 comments
Open

Consistent empty / null fields #258

smyrman opened this issue Sep 4, 2019 · 4 comments
Labels
proposal A suggestion for change that has not been accepted

Comments

@smyrman
Copy link
Collaborator

smyrman commented Sep 4, 2019

Proposal overview

Only some storage backends (such as MongoDB) would be able to really make the distinction between null and omitted values on top level fields. SQL storage backends would already treat the values equally, no matter what we do in code.

This is a proposal to treat null values and omitted values equally in code, and configure on the resource how all empty fields should be rendered; either as null, or omitted. The change apply specifically to the resource.Schema type (including resource.Object filed validator). It does not apply to field validators such as resource.Dict.

Motivation

This allows for more consistent APIs to be written that is easier to use in e.g. TypeScript. It also allows removing top-level fields via PATCH requests without deferring to JSON-PATCH syntax. This can be simpler to do for some clients.

Changes

Suggested changes include:

  • Remove the Required field in favor of a Nullable field on Schema.
  • Document all fields with Nullable: true as "oneOf": [{..}, {"type": "null"}] in normal JSON Schema instead or as nullable: true if using the OpenAPI JSON Schema dialect.
  • On schema Validation (before save):
    • Strip out null values.
  • On schema Serialization (before format):
    • Insert or strip null values based on config.
  • Add a flag to resource.Conf to determine (default) rendring for empty fields.
@smyrman
Copy link
Collaborator Author

smyrman commented Sep 4, 2019

@Dragomir-Ivanov, FYI.

What do you think?

@smyrman smyrman added the proposal A suggestion for change that has not been accepted label Sep 4, 2019
@Dragomir-Ivanov
Copy link
Contributor

@smyrman I think we need to consider also Default fields. What happens if you omit in regular PATCH a field with Default value. Does it stay the same, is it deleted, or it is reset to its default value.
I think regular PATCH syntax as very limited.
I like the idea of having fields as not set and set to empty, but still haven't used that, nor mixed rest-layer with TS (but planning a rewrite to TS asap).

@smyrman
Copy link
Collaborator Author

smyrman commented Sep 4, 2019

Good ponits @Dragomir-Ivanov.

If files are omitted on PUT (a.k.a update), then yes, reset to default values. If they are explicitly set to nil on PUT, then clear them. This can be done as both operations would be handled by the Validation step, but would not work if the same resource is sent to validation multiple times.

When fetching the resource back out, they will be either omitted or null based on the resource config.

@smyrman
Copy link
Collaborator Author

smyrman commented Sep 4, 2019

PS! we might need to make sure that PATCH results always serialize (the original) resource before calculating changes / base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A suggestion for change that has not been accepted
Projects
None yet
Development

No branches or pull requests

2 participants