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

This is a small addition that enables default attribute #154

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kokeksibir
Copy link

Currently default attribute is ignored. This pull request includes simple implementation and its test steps.

@awwright
Copy link
Collaborator

The "default" property in JSON Schema doesn't have anything to do with actual validation, it's strictly informational. It's supposed to be used for things like creating new JSON documents from scratch, where one value is just as arbitrary as any other. It doesn't have anything to do with validation when a value is undefined.

For example, if I'm editing a JSON array in a program, and I click a button to add a new item at the end, the program might use the "default" property to provide the initial value of that item.

There's a separate class of uses for JSON Schema, for casting an invalid instance into a validating one... For instance, if I'm expecting a string but was provided an integer, then cast the number to a string. Or similar to the case here, if a property is marked as required but was omitted, then fill it with some default value. I think this sort of behavior is better suited for a different library, though.

What's your use case, exactly?

@kokeksibir
Copy link
Author

@ACubed you are right, it is not an "actual" validation attribute, but it is defined in "validation" spec [http://json-schema.org/latest/json-schema-validation.html#anchor101]. IMHO, it makes this behavior more native than rewrite option which is already implemented inside the same function.

I use jsonschema wrapped by express-jsonschema. I validate request body with json schema and need to set defaults for some fields which are not defined. Previously I was using options.rewrite to setup defaults, however every time i needed to implement this option. Once I noticed that it is a spec defined behavior, I have come out with this solution.

I see that it may break previous implementations, hence I have added a new option setDefaults. So now it works only if options.setDefaults is true.

@jsdevel
Copy link
Contributor

jsdevel commented Feb 8, 2016

@kokeksibir so the spec is actually fairly vague on this topic.

This keyword can be used to supply a default JSON value associated with a particular schema. It is RECOMMENDED that a default value be valid against the associated schema.

All that says (in no authoritative vernacular), is that the keyword can be used to supply a default value. It in no way suggests that validation implementations SHOULD use the provided default value during validation.

IMO, this can be handled by separate libraries. I'm currently using express-openapi-defaults with express-openapi. I have the defaults middleware being executed before the validation middleware and it works out quite nicely.

@richardwillars
Copy link

Personally I think this library would be much more useful if it did allow for default values. I'm currently dealing with a schema that has a lot of nested arrays and objects, and as far as I can tell express-openapi-defaults doesn't allow for nested params. I'm now stuck with how to proceed because of this. If it did support it, I'd essentially have to replicate the entire schema, which seems stupid. Allowing for default values in this library would save a lot of duplication of the schema and it seems to be something a lot of people want as there's quite a few issues mentioning it.

@dtgriscom
Copy link

I have a use case for this capability. We're using JSON to store a product's configuration, but from time to time we add new properties to that configuration. So, I just update the schema and update the default configuration to match: no problem, right?

Nope, problem: what happens to a customer's customized configuration when they update to a new version of their firmware that requires these new properties? We don't want to overwrite their old data, but we need to have the new defaults in place or validation (and other dependent code) will fail. So, we have to add code that hand-checks for missing values and fills in the defaults. (We're already hitting this in development, with one developer's additions forcing another developer to hand-edit her configuration. Error-prone, and a pain in the tuchas.)

The solution is already embedded in the JSON schema spec: default values. So, if a value is required, but isn't present, then if a default is specified the default should be filled in and the validation passed. Simple solution, solved in a single place.

Am I missing something here? Would adding this break existing usage of jsonschema, or would the solution fail in strange and confusing ways?

@kokeksibir
Copy link
Author

In order not to break existing usage, I have implemented a flag option setDefaults hence this feature would inject defaults only if flag is set explicitly true

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

Successfully merging this pull request may close these issues.

5 participants