From 79694945506d231cc89e1fe85bb0e3dd457f4145 Mon Sep 17 00:00:00 2001 From: Emilie Paillous Date: Thu, 2 May 2024 18:16:52 +0200 Subject: [PATCH] support schemas validation in additionalProperties + remove additionalProperties logic from allof --- .../schema_validator/all_of_validator.rb | 17 ----------------- .../schema_validator/object_validator.rb | 8 +++----- spec/data/petstore-with-discriminator.yaml | 4 ++-- .../schema_validator/all_of_validator_spec.rb | 18 +++++++++--------- 4 files changed, 14 insertions(+), 33 deletions(-) diff --git a/lib/openapi_parser/schema_validator/all_of_validator.rb b/lib/openapi_parser/schema_validator/all_of_validator.rb index a3bd207..ef25db9 100644 --- a/lib/openapi_parser/schema_validator/all_of_validator.rb +++ b/lib/openapi_parser/schema_validator/all_of_validator.rb @@ -10,8 +10,6 @@ def coerce_and_validate(value, schema, **keyword_args) end # if any schema return error, it's not valida all of value - remaining_keys = value.kind_of?(Hash) ? value.keys : [] - nested_additional_properties = false schema.all_of.each do |s| # We need to store the reference to all of, so we can perform strict check on allowed properties _coerced, err = validatable.validate_schema( @@ -20,24 +18,9 @@ def coerce_and_validate(value, schema, **keyword_args) :parent_all_of => true, parent_discriminator_schemas: keyword_args[:parent_discriminator_schemas] ) - - if s.type == "object" - remaining_keys -= (s.properties || {}).keys - nested_additional_properties = true if s.additional_properties - else - # If this is not allOf having array of objects inside, but e.g. having another anyOf/oneOf nested - remaining_keys.clear - end - return [nil, err] if err end - # If there are nested additionalProperites, we allow not defined extra properties and lean on the specific - # additionalProperties validation - if !nested_additional_properties && !remaining_keys.empty? - return [nil, OpenAPIParser::NotExistPropertyDefinition.new(remaining_keys, schema.object_reference)] - end - [value, nil] end end diff --git a/lib/openapi_parser/schema_validator/object_validator.rb b/lib/openapi_parser/schema_validator/object_validator.rb index 2850d03..1ee355e 100644 --- a/lib/openapi_parser/schema_validator/object_validator.rb +++ b/lib/openapi_parser/schema_validator/object_validator.rb @@ -27,9 +27,9 @@ def coerce_and_validate(value, schema, parent_all_of: false, parent_discriminato coerced, err = if s remaining_keys.delete(name) validatable.validate_schema(v, s) + elsif schema.additional_properties != true && schema.additional_properties != false + validatable.validate_schema(v, schema.additional_properties) else - # TODO: we need to perform a validation based on schema.additional_properties here, if - # additionalProperties are defined [v, nil] end @@ -41,9 +41,7 @@ def coerce_and_validate(value, schema, parent_all_of: false, parent_discriminato remaining_keys.delete(discriminator_property_name) if discriminator_property_name - if !remaining_keys.empty? && !parent_all_of && !schema.additional_properties - # If object is nested in all of, the validation is already done in allOf validator. Or if - # additionalProperties are defined, we will validate using that + if !remaining_keys.empty? && !schema.additional_properties return [nil, OpenAPIParser::NotExistPropertyDefinition.new(remaining_keys, schema.object_reference)] end return [nil, OpenAPIParser::NotExistRequiredKey.new(required_set.to_a, schema.object_reference)] unless required_set.empty? diff --git a/spec/data/petstore-with-discriminator.yaml b/spec/data/petstore-with-discriminator.yaml index c17bd22..d713fef 100644 --- a/spec/data/petstore-with-discriminator.yaml +++ b/spec/data/petstore-with-discriminator.yaml @@ -211,7 +211,6 @@ components: fire_range: type: integer format: int64 - additionalProperties: false Hydra: allOf: - $ref: '#/components/schemas/DragonBody' @@ -222,6 +221,8 @@ components: head_count: type: integer format: int64 + mass: + type: integer additionalProperties: type: string DragonBody: @@ -233,4 +234,3 @@ components: type: string mass: type: integer - additionalProperties: false diff --git a/spec/openapi_parser/schema_validator/all_of_validator_spec.rb b/spec/openapi_parser/schema_validator/all_of_validator_spec.rb index 542e385..113c971 100644 --- a/spec/openapi_parser/schema_validator/all_of_validator_spec.rb +++ b/spec/openapi_parser/schema_validator/all_of_validator_spec.rb @@ -24,22 +24,20 @@ request_operation.validate_request_body(content_type, body) end - it 'fails when sending unknown properties' do + it 'works when sending unknown properties' do body = { "baskets" => [ { "name" => "dragon", "mass" => 10, "fire_range" => 20, - "speed" => 20 + "speed" => 20, + "color" => 'gold' } ] } - expect { request_operation.validate_request_body(content_type, body) }.to raise_error do |e| - expect(e).to be_kind_of(OpenAPIParser::NotExistPropertyDefinition) - expect(e.message).to end_with("does not define properties: speed") - end + request_operation.validate_request_body(content_type, body) end it 'fails when missing required property' do @@ -88,7 +86,7 @@ request_operation.validate_request_body(content_type, body) end - it 'fails when sending unknown properties of correct type based on additionalProperties' do + it 'fails when sending unknown properties of incorrect type based on additionalProperties' do body = { "baskets" => [ { @@ -100,8 +98,10 @@ ] } - # TODO for now we don't validate on additionalProperites, but this should fail on speed have to be string - request_operation.validate_request_body(content_type, body) + expect { request_operation.validate_request_body(content_type, body) }.to raise_error do |e| + expect(e).to be_kind_of(OpenAPIParser::ValidateError) + expect(e.message).to end_with("expected string, but received Integer: 20") + end end it 'fails when missing required property' do