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

defaults don't work inside anyOf #276

Closed
mbroadst opened this issue Aug 14, 2016 · 9 comments
Closed

defaults don't work inside anyOf #276

mbroadst opened this issue Aug 14, 2016 · 9 comments

Comments

@mbroadst
Copy link
Contributor

mbroadst commented Aug 14, 2016

What version of Ajv are you using? Does the issue happen if you use the latest version?
4.4.0, yes

Ajv options object (see https://github.com/epoberezkin/ajv#options):

{
  ownProperties: true,
  allErrors: true,
  useDefaults: true
}

JSON Schema (please make it as small as possible to reproduce the issue):

not working:

{
  "type": "object",
  "properties": {
    "num": { "allOf": [ { "type": "number", "default": 2 }, { "acceptTerms": true } ] }
  },
  "required": [ "num" ]
}

working:

{
  "type": "object",
  "properties": {
    "num": { "type": "number", "default": 2 }
  },
  "required": [ "num" ]
}

Data (please make it as small as posssible to reproduce the issue):

{}

Your code (please use options, schema and data as variables):

let ajv = new Ajv(options);
ajv.addKeyword('acceptTerms', () => () => true);
let validate = ajv.compile(schema);
let valid = validate(data);
if (!valid) {
  console.log(validate.errors);
}

Validation result, data AFTER validation, error messages:

[ { keyword: 'required',
    dataPath: '',
    schemaPath: '#/required',
    params: { missingProperty: 'num' },
    message: 'should have required property \'num\'' } ]

What results did you expect?
no errors, the default of 2 would be used

Do you intend to resolve the issue?

@epoberezkin
Copy link
Member

That is the way they are implemented. See Assigning defaults:

default keywords in other cases are ignored:

  • not in properties or items subschemas
  • in schemas inside anyOf, oneOf and not (see Option to merge defaults #42)
  • in if subschema of v5 switch keyword
  • in schemas generated by custom macro keywords

@mbroadst
Copy link
Contributor Author

Is that just underspecified in the spec? It seems a natural assumption that defaults would apply wherever they are defined

@epoberezkin
Copy link
Member

Deafult is defined in the spec as optional, its behaviour is not specified in too much detail.

Taking into account defaults in subschemas of compound keywords inside properties is both difficult to implement and in some cases ambiguous. The existing implementation covers 80%+ of real use cases. If you need multiple defaults depending on the valid schema branch, you just have to use v5 switch instead of anyOf, because in case of switch it is deterministic and doesn't require a rollback mechanism.

What is the objection to refactoring the schema in the way that Ajv supports (i.e. putting default keyword on the same level as allOf)?

@mbroadst
Copy link
Contributor Author

Absolutely understandable. Could you expand on the proposed solution at the end there? Maybe I'm just not seeing how to do this in a supported fashion.

@epoberezkin
Copy link
Member

epoberezkin commented Aug 14, 2016

In your case, this schema would produce the result you want (defaults are assigned before schema is validated):

{
  "type": "object",
  "properties": {
    "num": {
      "default": 2,
      "allOf": [ { "type": "number" }, { "acceptTerms": true } ]
    }
  },
  "required": [ "num" ]
}

Another common use case that would produce confusing results if it were supported is this:

{
  "type": "object",
  "anyOf": [
    {
      "properties": {
        "type": { "enum": [ "foo" ] },
        "foo": { "type": "integer", "default": 1 }
      },
      "additionalProperties": false
    },
    {
      "properties": {
        "type": { "enum": [ "bar" ] },
        "bar": { "type": "string", "default": "abc" }
      },
      "additionalProperties": false
    }
  ]
}

The above wouldn't work without implementing complex rollback logic and that's the reason why defaults inside "anyOf"/"oneOf" (at any depth) are ignored by Ajv.

The same logic can be implemented with this schema:

{
  "type": "object",
  "switch": [
    {
      "if": { "properties": { "type": { "constant": "foo" } } },
      "then": {
        "properties": {
          "type": {},
          "foo": { "type": "integer", "default": 1 }
        },
        "additionalProperties": false
      }
    },
    {
      "if": { "properties": { "type": { "constant": "bar" } } },
      "then": {
        "properties": {
          "type": {},
          "bar": { "type": "string", "default": "abc" }
        },
        "additionalProperties": false
      }
    },
    { "then": false }
  ]
}

The above would assign defaults in the expected way because Ajv knows which branch should be validated based on "if" conditions. Defaults inside "if" schemas are still ignored.

@epoberezkin epoberezkin changed the title defaults don't seem to work inside anyOf defaults don't work inside anyOf Aug 28, 2016
@kapouer
Copy link
Contributor

kapouer commented Nov 10, 2016

@epoberezkin you should document this (declare default next to anyOf/allOf/oneOf instead of inside), it's a life saver.

@epoberezkin
Copy link
Member

@kapouer
Copy link
Contributor

kapouer commented Nov 10, 2016

I meant the way you use v5 switch to have defaults inside "then" schemas applied.

@epoberezkin
Copy link
Member

@kapouer got it. Will do.

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

No branches or pull requests

3 participants