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

support schemas validation in additionalProperties + remove additionalProperties logic from allof #167

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions lib/openapi_parser/schema_validator/all_of_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
8 changes: 3 additions & 5 deletions lib/openapi_parser/schema_validator/object_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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?
Expand Down
4 changes: 2 additions & 2 deletions spec/data/petstore-with-discriminator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ components:
fire_range:
type: integer
format: int64
additionalProperties: false
Hydra:
allOf:
- $ref: '#/components/schemas/DragonBody'
Expand All @@ -222,6 +221,8 @@ components:
head_count:
type: integer
format: int64
mass:
type: integer
additionalProperties:
type: string
DragonBody:
Expand All @@ -233,4 +234,3 @@ components:
type: string
mass:
type: integer
additionalProperties: false
18 changes: 9 additions & 9 deletions spec/openapi_parser/schema_validator/all_of_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" => [
{
Expand All @@ -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
Expand Down