support schemas validation in additionalProperties + remove additionalProperties logic from allof

[epaillous]

Description

For now, when passing schemas like :

  type: object
  required:
    - head_count
  properties:
    head_count:
      type: integer
      format: int64
    mass:
      type: integer
  additionalProperties:
    type: string  

there is no error triggered on value like :

{
  "color"       => "gold",
  "head_count" => 20,
  "speed"      => 20
}

because the gem does not support validating schemas inside additionalProperties field. This PR intends to support this behaviour.

While implementing this PR, some specs in all_of_validator_spec were failing (or not failing) because of some combination of subschemas in allOf having additionalProperties to false. But according to OpenApi specification, "allOf takes an array of object definitions that are validated independently but together compose a single object.", meaning validating allOf schemas should be equivalent to validating each schema one by one. So there is no reason to check remainingKeys in AllOfValidator object.

Let me know if you need anything else 😇

[sasamuku]

@epaillous
Thank you for your contribution!
LGTM!

[n-shay]

@sasamuku These changes is breaking the allOf functionality that is working properly in 2.1.0. Please do not publish them into an official release.

I am not sure about the original issue described here, and you are probably missing some specs to cover the scenario.

If you have a request body schema like this:

allOf:
  - type: object
    properties:
       id:
         type: string
         format: uuid
       timestamp:
         type: string
         format: date-time
  - type: object
    properties:
       event:
         type: string
       event_info:
         "$ref": "#/components/schemas/EventInfo"
  additionalProperties: false

and you're passing a correct object with all fields, you will still receive a OpenAPIParser::NotExistPropertyDefinition validation error:

... does not define properties: event, event_info

That is happening because the parent_all_of is no longer respected in ObjectValidator and AllOfValidator doesn't keep track of the remaining keys anymore.

[MrChoclate]

@n-shay can you explain why we would need a parent_all_of ?

From https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/#allof I see that:

allOf takes an array of object definitions that are used for independent validation but together compose a single object. [...] To be valid against allOf, the data provided by the client must be valid against all of the given subschemas.

My understanding from this sentence is that the validation should be something like schema.all_of.all? { valid?(value) } otherwise they are not independent 🤔

[n-shay]

@MrChoclate In simple cases, the validation of each schema under allOf individually should suffice, but the complexity comes with polymorphism and usage of additionalProperties.
I will try to explain and give an example.

To specify inheritance between schemas, you would use allOf and as a reminder, inheritance can be cascaded. For example, ModelA derives from ModelB and adds a property, ModelB derives from ModelC and adds a property, and so on.

As long as you don't use additionalProperties, OpenAPI assumes that additional properties are allowed. But once you use additionalProperties: false in one of the schemas, schema.all_of.all? { valid?(value) } would start to fail since there are additional properties.

Here are the example models:

ModelA:
  allOf:
    - "$ref": "#/components/schemas/ModelB"
    - type: object
      properties:
        field_a:
          type: string
      additionalProperties: false
ModelB:
  allOf:
    - "$ref": "#/components/schemas/ModelB"
    - type: object
      properties:
        field_b:
          type: string
      additionalProperties: false
ModelC:
  type: object
  properties:
    field_c:
      type: string
  additionalProperties: false

First run of the schema validation will cascade all the way down to ModelC and try to validate a value with 3 properties/keys (field_a, field_b and field_c) but since additional fields are not allowed it will fail.

That is the reason why we need to monitor the parent_all_of state of the schema and defer the additionalProperties validation to its root schema.

That being said, we don't pass upwards which properties were found/validated and which remained but that's another issue.

I've implemented the validation in a forked repo that we're using: all_of_validator.rb and object_validator.rb.
I added remaining_keys argument as well.
The additionalProperties only validates false value, more complex rules are ignored atm (just like the current gem does).

[MrChoclate]

@n-shay I understand your use case, but according to json-schema it is explicitly said that using allOf + additionalProperties is restricting the schema and that we should rather use unevaluatedProperties to specify "inheritance"

[n-shay]

@MrChoclate Only in OAS3.1, the schema object was made to fully support json-schema. unevaluatedProperties is not supported by OAS3.0 and this project was never updated to OAS3.1 (See #152)

[Vincent14]

Hi @sasamuku when do you plan to release a new version with this fix please? 😃