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

Add method for injecting defaults #76

Open
chuwy opened this issue Feb 8, 2018 · 7 comments
Open

Add method for injecting defaults #76

chuwy opened this issue Feb 8, 2018 · 7 comments

Comments

@chuwy
Copy link
Contributor

chuwy commented Feb 8, 2018

Following instance:

{"foo": 1}

Is valid against following schema:

{
  "type": "object",
  "properties": {
    "foo": {"type": "integer", "default": 0},
    "bar": {"type": "string", "default": "baz"}
  }
}

Though .validate method won't inject default value for bar. And I believe this is correct behavior. Instead we should have method like .setDefaults that when invoked mutates original JSON by setting missing bar (but not foo of course).

/cc @knservis

@alexanderdean
Copy link
Member

.setDefaults is interesting. It makes schema backwards compatibility easier. What happens in JSON Schema if you have a default, make the field "required", but it's not present in the instance - presumably validation fails?

@chuwy
Copy link
Contributor Author

chuwy commented Feb 8, 2018

Yes, validation fails at least at https://www.jsonschemavalidator.net/

@knservis
Copy link

knservis commented Feb 8, 2018

Would you not run setDefaults before validate?

Also, should you not validate against the previous version of the schema anyway?

@chuwy
Copy link
Contributor Author

chuwy commented Feb 8, 2018

This would be a good idea to call it before, but this is up to consumer.

Question here: should cases like required AND default be invalidated by one of optional igluctl linters? For me it looks like user doesn't quite understand what he wants.

@knservis
Copy link

knservis commented Feb 9, 2018

At least a warning is warranted. Something like "WARN required AND default found for field x! Required is redundant!". An error is also an option if that definition will cause errors. The warning will alert the user to a possible omission, but the execution is possibly unaffected by removing required in the case that default is present. Either way is fine as the user needs to know that this is pointless.

@chuwy
Copy link
Contributor Author

chuwy commented Feb 9, 2018

Yep. This is exactly how all linters work. If it sees type: string and maximum: X (integer's validation) - it yields. Schema is valid, validator can proceed, but this is very likely user missed something, usually just because he/she refactored schema and forgot something.

@chuwy
Copy link
Contributor Author

chuwy commented Sep 15, 2021

I'm thinking this can be a separate enrichment.

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

No branches or pull requests

3 participants