removeAdditional doesn't work well with anyOf, oneOf or allOf

[gtrindade]

Hi, first of all, awesome work with this validator! Very handy!

Now here is the problem I'm facing, I'm not sure if this is the expected behavior, but if it is, it's not very intuitive... my scenario is a bit more complex, but I'll simplify in the following example.

I'm initializing ajv with:

const ajv = Ajv({removeAdditional: true})

This is the sample schema:

const schema = {
    type: `object`,
    additionalProperties: {
      anyOf: [{
        type: `object`,
        properties: {
          a: {
            type: `string`
          }
        },
        required: [`a`],
        additionalProperties: false
      }, {
        type: `object`,
        properties: {
          b: {
            type: `string`
          }
        },
        required: [`b`],
        additionalProperties: false
      }]
    }
  }

It should accept an object with as many objects as we want, and each of these objects should have at least one property called a, or one called b. If a or b are not present, it should fail. It should also discard any other property that is not a or b.

Now, with this data:

  const data1 = {
    obj1: {
      a: `a`
    },
    obj2: {
      b: `b`
    },
    obj3: {
      c: `c`
    }
  }

It should fail only on obj3, but that's only the case when removeAdditional is false. If removeAdditional is true, it seems like as soon as it evaluates the first option of the anyOf, it removes the additional properties of the object before trying the second option.

the removeAdditional is very useful, but it needs to wait until all possibilities of oneOf to be evaluated before removing anything.

This is the error I get by running the code above:

     Error: the array [
  {
    "dataPath": "['obj2']"
    "keyword": "required"
    "message": "should have required property 'a'"
    "params": {
      "missingProperty": "a"
    }
    "schemaPath": "#/additionalProperties/anyOf/0/required"
  }
  {
    "dataPath": "['obj2']"
    "keyword": "required"
    "message": "should have required property 'b'"
    "params": {
      "missingProperty": "b"
    }
    "schemaPath": "#/additionalProperties/anyOf/1/required"
  }
  {
    "dataPath": "['obj2']"
    "keyword": "anyOf"
    "message": "should match some schema in anyOf"
    "params": {}
    "schemaPath": "#/additionalProperties/anyOf"
  }
  {
    "dataPath": "['obj3']"
    "keyword": "required"
    "message": "should have required property 'a'"
    "params": {
      "missingProperty": "a"
    }
    "schemaPath": "#/additionalProperties/anyOf/0/required"
  }
  {
    "dataPath": "['obj3']"
    "keyword": "required"
    "message": "should have required property 'b'"
    "params": {
      "missingProperty": "b"
    }
    "schemaPath": "#/additionalProperties/anyOf/1/required"
  }
  {
    "dataPath": "['obj3']"
    "keyword": "anyOf"
    "message": "should match some schema in anyOf"
    "params": {}
    "schemaPath": "#/additionalProperties/anyOf"
  }
] was thrown, throw an Error :)

As you can see, obj2 is failing because it's missing property b, which is not true.

I called validate with

const result = ajv.validate(schema, data1)

[epoberezkin]

It works as it should in this situation. Look at this working example.

The first instance of ajv does not have removeAdditional option. And it correctly fails on obj3.

The second instance has removeAdditional option as true. When it validates obj1 it passes because the first schema in anyOf has 'a' property in properties. When it validates obj2 using the first schema in anyOf property 'b' is additional there. So it gets removed, as you can see from the log. But the first schema in anyOf fails and it goes on to try to validate it with the second schema. It fails too because there is no property 'b' (it was removed).

{ removeAdditional: true } removes any property that would otherwise be validated with the schema in additionalProperties: false keyword in the same schema. It does not try to establish whether this property is known to some other branch of the schema - it is simply out of its scope. There is a place for defining unknown properties in the standard - there is actually this proposal for v5. But it is not what this option is doing.

I don't think there is a real need to define the concept of "unknown property" because usually there are ways to refactor your schema to make it work (and remove what you need). Difficult to say how exactly of course without seeing the actual schema, but usually you just need to take out properties from keywords anyOf, oneOf. The example above doesn't need to have properties inside anyOf. The equivalent (and faster) refactored schema is:

{
  type: 'object',
  additionalProperties: {
    type: 'object',
    properties: {
      a: { type: 'string' },
      b: { type: 'string' } 
    },
    additionalProperties: false,
    anyOf: [
      { required: ['a'] },
      { required: ['b'] }
    ]
  }
}

With this schema it will correctly fail on obj3 if you pass { removeAdditional: true }. It can also filter out obj3 if you pass { removeAdditional: 'failing' } (see docs).

[gtrindade]

Thanks for the suggestion, I didn't realize I could combine anyOf with required. I'll give it a try!

I'm not sure if this is just me, but took me quite a while to understand this behavior of the removeAdditional option... To me, it feels like it should only remove whats not matching anything. In this case, it should wait until all the options on anyOf to be evaluated before removing anything.

But again, thanks for your quick response!

[epoberezkin]

It just that it uses the same definition of "additional" as JSON-schema standard - it means "additional" according to the current (sub)schema, i.e. not present in "properties" and not matching "patternProperties" of the same (sub)schema, and it does not mean "unknown" according to the whole schema. For example, this schema should fail on any data because of that:

{
  type: 'object',
  properties: { foo: { type: 'string' } },
  required: ['foo'],
  allOf: [ { additionalProperties: false } ]
}

This would alway fail too:

{
  type: 'object',
  required: ['foo'],
  additionalProperties: false
}

But it definitely not just you, it takes some time to understand how the standard works... The main thing here is that all the keywords are shallow, they can only be affected by siblings in the same object.

I will think again about implementing this "unknown" properties concept - both to fail on unknown properties and to filter them out. There are cases when it is either difficult to refactor schema (I haven't come across the cases where it is impossible though...) or undesirable, because it makes schema lose it's meaning from the application domain point of view.

[epoberezkin]

Closing this issue, please let mw know if there are any questions.

[epoberezkin]

The easiest is to use v5 switch keyword instead of oneOf (and by the way, anyOf is often faster than oneOf - for 50% of data in your case). Otherwise see suggestions in issues mentioned in FAQ

[sberan]

@epoberezkin would you accept a PR that removed the additional properties only after passing validation of the schema? That way, if a subschema fails validation for some other reason, the properties aren't removed.

Something like removeAdditional: 'afterValidation'? I think this would be more intuitive in these cases.

[epoberezkin]

@sberan so rather than removing them straight away, additionalProperties: false would ignore them and once the sub-schema validation passed, you would remove? Maybe it's possible.

[EtaiG]

@epoberezkin

Firstly, thank you for AJV and the continual support you provide for AJV and for the JSON-schema standard in general.
We use AJV at Wix (you can write this in your README!).

I read both this thread and #134, and I must say, I still disagree that this behavior makes sense.
While the JSON-schema spec does specify that validation for sub-schemas are only for the sub-schema, the 'removeAdditional' behavior is not standard and does not need to abide by this.

Furthermore, the compound anyOf/allOf/oneOf keywords specifically modify the validation result of the (parent) schema, and it would make more sense to validate the parent schema, and then to remove additional properties by using the 'correct path' of the validated schema.

Of course the current implementation does not have this notion, but consider for a moment the annotations feature in the JSON schema specification: https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.7.7

Let's assume for a moment that we implemented the non-standard removeAdditional as an annotation on the schemas. Now this behavior is defined as an annotation on the schema which different validators can implement.

In this instance, I would fully expect the annotation to be resolved according to JSON-schema specification - namely, that the 'valid' instance from anyOf would collect the annotation + JSON pointer from the valid subschema, as specified here: https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.9.2.1.2

This feature would still be working in a standard fashion according to how JSON-schema rules apply, but in this case it would also be aligned with the myriad developers who thought this is how it should work to begin with.

Basically, the fact that the implementation of non-standard removeAdditional is aligned with the standard validation specification, does not mean it is expected nor correct. We can easily align it with another standard specification, whether or not the actual implementation uses that feature (annotations - which it could have under the hood, by modifying schemas as they are added, for example)

What do you think?
I would be happy to contribute towards a working solution, if my argument is convincing :)

[epoberezkin]

I think it’s better to keep removeAdditional as is and add removeUnevaluated as proposed in #1346

[epoberezkin]

I do agree that Ajv doesn’t need to align with spec 100% (and in this case it’s indeed non-standard) - strict mode is the step in that direction - but there are other considerations here. Your argument would apply if it was a new feature, but now that it’s used by lots of people it would be more confusing if it’s changed.

[epoberezkin]

We use AJV at Wix (you can write this in your README!).

I’m probably going to steal the idea from svelte.dev website - the new website will have space for logos of all users.

[cyberixae]

now that it’s used by lots of people it would be more confusing if it’s changed.

The setting already has 4 different modes. I'm sure the existing users wouldn't care if a fifth mode was added.

[ChuckJonas]

It seems like allOf doesn't really work with this setup at all?

  const ajv = new Ajv({removeAdditional: 'all'})
  const validate = ajv.compile({
    type: "object",
    allOf: [
      {
        type: "object",
        properties: {
          a: { type: "string" },
        },
        required: ["a"],
      },
      {
        type: "object",
        properties: {
          b: { type: "number" },
        },
        required: ["b"],
      },
    ],
  });

  const data = { a: "hello", b: 3000, c: true }
  const success = validate(data);
  console.log(data);  
  console.log(validate.errors);

Outputs

{ a: 'hello' }
[
  {
    instancePath: '',
    schemaPath: '#/allOf/1/required',
    keyword: 'required',
    params: { missingProperty: 'b' },
    message: "must have required property 'b'"
  }
]

Seems like this is another use case for unevaluatedProperties: false and having the removeAdditional respect that?

[epoberezkin]

allOf actually works as it should, but you probably want anyOf?

Edit: sorry, no, it's removeAdditional. This option defines "additional" locally, with regards to the current schema object – so it removes additional b and c properties when it evaluates the first subschema

[jeremycare]

@epoberezkin,
I'm following this thread, but do there is a workaround for that?
Where for the example of @ChuckJonas, we could end up with:

{ a: "hello", b: 3000 }

For our use case, we have a similar schema where we have anyOf and we would like to remove any properties that is not part of any sub-schemas (part of the anyOf. I'm wondering if this is even possible with AJV implementation today.