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

Add support for discriminator #32

Merged
merged 8 commits into from
May 20, 2019
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
23 changes: 23 additions & 0 deletions lib/openapi_parser/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,29 @@ def message
end
end

class NotExistDiscriminatorMappingTarget < OpenAPIError
def initialize(key, reference)
super(reference)
@key = key
end

def message
"discriminator mapping key #{@key} does not exist in #{@reference}"
end
end

class NotExistDiscriminatorPropertyName < OpenAPIError
def initialize(key, value, reference)
super(reference)
@key = key
@value = value
end

def message
"discriminator propertyName #{@key} does not exist in value #{@value} in #{@reference}"
end
end

class NotOneOf < OpenAPIError
def initialize(value, reference)
super(reference)
Expand Down
4 changes: 4 additions & 0 deletions lib/openapi_parser/schema_validators/any_of_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ class AnyOfValidator < Base
# @param [Object] value
# @param [OpenAPIParser::Schemas::Schema] schema
def coerce_and_validate(value, schema)
if schema.discriminator
return validate_discriminator_schema(schema.discriminator, value)
end

# in all schema return error (=true) not any of data
schema.any_of.each do |s|
coerced, err = validatable.validate_schema(value, s)
Expand Down
18 changes: 18 additions & 0 deletions lib/openapi_parser/schema_validators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,23 @@ def initialize(validatable, coerce_value)
def coerce_and_validate(_value, _schema)
raise 'need implement'
end

def validate_discriminator_schema(discriminator, value)
unless value.key?(discriminator.property_name)
return [nil, OpenAPIParser::NotExistDiscriminatorPropertyName.new(discriminator.property_name, value, discriminator.object_reference)]
end
mapping_key = value[discriminator.property_name]

# TODO: it's allowed to have discriminator without mapping, then we need to lookup discriminator.property_name
# but the format is not the full path, just model name in the components
mapping_target = discriminator.mapping[mapping_key]
unless mapping_target
return [nil, OpenAPIParser::NotExistDiscriminatorMappingTarget.new(mapping_key, discriminator.object_reference)]
end

# Find object does O(n) search at worst, then caches the result, so this is ok for repeated search
resolved_schema = discriminator.root.find_object(mapping_target)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ota42y so I am trying to move this to https://github.com/Ladas/openapi_parser/blob/c844e37dba4cbe605f9a3f8d565f84843acfe5bb/lib/openapi_parser/schemas/schema.rb#L115, to my own parser, but after spending several hours there I didn't figure out how it works. :-) Could you give me some hints? Is it even possible to do the lookup there?

If the discriminator.root.find_object(mapping_target) is or would be O(1), I don't really need to do that. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like the find_object does O(n) lookup at worst, then caches the result, so this should be ok performance wise

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, find_object method dose O(n) lookup at worst, but we cached result so I think there is no problem 👍
https://github.com/ota42y/openapi_parser/blob/master/lib/openapi_parser/concerns/findable.rb#L4

validatable.validate_schema(value, resolved_schema)
end
end
end
4 changes: 4 additions & 0 deletions lib/openapi_parser/schema_validators/one_of_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ class OneOfValidator < Base
# @param [Object] value
# @param [OpenAPIParser::Schemas::Schema] schema
def coerce_and_validate(value, schema)
if schema.discriminator
return validate_discriminator_schema(schema.discriminator, value)
end

# if multiple schemas are satisfied, it's not valid
result = schema.one_of.one? do |s|
_coerced, err = validatable.validate_schema(value, s)
Expand Down
1 change: 1 addition & 0 deletions lib/openapi_parser/schemas.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require_relative 'schemas/classes'

require_relative 'schemas/base'
require_relative 'schemas/discriminator'
require_relative 'schemas/openapi'
require_relative 'schemas/paths'
require_relative 'schemas/path_item'
Expand Down
1 change: 1 addition & 0 deletions lib/openapi_parser/schemas/classes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

module OpenAPIParser::Schemas
class Base; end
class Discriminator < Base; end
class OpenAPI < Base; end
class Operation < Base; end
class Parameter < Base; end
Expand Down
11 changes: 11 additions & 0 deletions lib/openapi_parser/schemas/discriminator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module OpenAPIParser::Schemas
class Discriminator < Base
# @!attribute [r] property_name
# @return [String, nil]
openapi_attr_value :property_name, schema_key: :propertyName

# @!attribute [r] mapping
# @return [Hash{String => String]
openapi_attr_value :mapping
end
end
6 changes: 5 additions & 1 deletion lib/openapi_parser/schemas/schema.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# TODO: support 'not' because I need check reference...
# TODO: support 'discriminator', 'xml', 'externalDocs'
# TODO: support 'xml', 'externalDocs'
# TODO: support extended property

module OpenAPIParser::Schemas
Expand Down Expand Up @@ -101,6 +101,10 @@ class Schema < Base
# @return [Hash{String => Schema}, nil]
openapi_attr_hash_object :properties, Schema, reference: true

# @!attribute [r] discriminator
# @return [Discriminator, nil]
openapi_attr_object :discriminator, Discriminator

# @!attribute [r] additional_properties
# @return [Boolean, Schema, Reference, nil]
openapi_attr_object :additional_properties, Schema, reference: true, allow_data_type: true, schema_key: :additionalProperties
Expand Down
109 changes: 109 additions & 0 deletions spec/data/petstore-with-discriminator.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
openapi: "3.0.0"
info:
version: 1.0.0
title: Swagger Petstore
description: A sample API that uses a petstore as an example to demonstrate features in the OpenAPI 3.0 specification
termsOfService: http://swagger.io/terms/
contact:
name: Swagger API Team
email: [email protected]
url: http://swagger.io
license:
name: Apache 2.0
url: https://www.apache.org/licenses/LICENSE-2.0.html
servers:
- url: http://petstore.swagger.io/api
paths:
/save_the_pets:
post:
description: Creates a new pet in the store. Duplicates are allowed
operationId: addPet
requestBody:
description: Pet to add to the store
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/PetBaskets'
responses:
'200':
description: pet response
content:
application/json:
schema:
type: object
components:
schemas:
PetBaskets:
type: object
properties:
baskets:
type: array
items:
anyOf:
- "$ref": "#/components/schemas/SquirrelBasket"
- "$ref": "#/components/schemas/CatBasket"
discriminator:
propertyName: name
mapping:
cats: "#/components/schemas/CatBasket"
squirrels: "#/components/schemas/SquirrelBasket"
SquirrelBasket:
type: object
required:
- name
properties:
name:
type: string
content:
type: array
items:
"$ref": "#/components/schemas/Squirrel"
Squirrel:
type: object
required:
- name
- nut_stock
properties:
name:
type: string
born_at:
format: date-time
nullable: true
type: string
description:
nullable: true
type: string
nut_stock:
nullable: true
type: integer
CatBasket:
type: object
required:
- name
properties:
name:
type: string
content:
type: array
items:
"$ref": "#/components/schemas/Cat"
Cat:
type: object
required:
- name
- milk_stock
properties:
name:
type: string
born_at:
format: date-time
nullable: true
type: string
description:
nullable: true
type: string
milk_stock:
nullable: true
type: integer

100 changes: 100 additions & 0 deletions spec/openapi_parser/schemas/discriminator_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
require_relative '../../spec_helper'

RSpec.describe OpenAPIParser::Schemas::RequestBody do
let(:root) { OpenAPIParser.parse(petstore_with_discriminator_schema, {}) }

describe 'discriminator' do
let(:content_type) { 'application/json' }
let(:http_method) { :post }
let(:request_path) { '/save_the_pets' }
let(:request_operation) { root.request_operation(http_method, request_path) }
let(:params) { {} }

it 'picks correct object based on mapping and succeeds' do
body = {
"baskets" => [
{
"name" => "cats",
"content" => [
{
"name" => "Mr. Cat",
"born_at" => "2019-05-16T11:37:02.160Z",
"description" => "Cat gentleman",
"milk_stock" => 10
}
]
},
]
}

request_operation.validate_request_body(content_type, body)
end

it 'picks correct object based on mapping and fails' do
body = {
"baskets" => [
{
"name" => "cats",
"content" => [
{
"name" => "Mr. Cat",
"born_at" => "2019-05-16T11:37:02.160Z",
"description" => "Cat gentleman",
"nut_stock" => 10 # passing squirrel attribute here, but discriminator still picks cats and fails
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect!! 🙆 🙆 🙆 🙆

}
]
},
]
}
expect { request_operation.validate_request_body(content_type, body) }.to raise_error do |e|
expect(e.kind_of?(OpenAPIParser::NotExistRequiredKey)).to eq true
expect(e.message).to match("^required parameters milk_stock not exist.*?$")
end
end

it "throws error when discriminator mapping is not found" do
body = {
"baskets" => [
{
"name" => "dogs",
"content" => [
{
"name" => "Mr. Dog",
"born_at" => "2019-05-16T11:37:02.160Z",
"description" => "Dog bruiser",
"nut_stock" => 10
}
]
},
]
}

expect { request_operation.validate_request_body(content_type, body) }.to raise_error do |e|
expect(e.kind_of?(OpenAPIParser::NotExistDiscriminatorMappingTarget)).to eq true
expect(e.message).to match("^discriminator mapping key dogs does not exist.*?$")
end
end

it "throws error if discriminator propertyName is not present on object" do
body = {
"baskets" => [
{
"content" => [
{
"name" => "Mr. Dog",
"born_at" => "2019-05-16T11:37:02.160Z",
"description" => "Dog bruiser",
"milk_stock" => 10
}
]
},
]
}

expect { request_operation.validate_request_body(content_type, body) }.to raise_error do |e|
expect(e.kind_of?(OpenAPIParser::NotExistDiscriminatorPropertyName)).to eq true
expect(e.message).to match("^discriminator propertyName name does not exist in value.*?$")
end
end
end
end
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ def petstore_schema
YAML.load_file('./spec/data/petstore-expanded.yaml')
end

def petstore_with_discriminator_schema
YAML.load_file('./spec/data/petstore-with-discriminator.yaml')
end

def build_validate_test_schema(new_properties)
b = YAML.load_file('./spec/data/validate_test.yaml')
obj = b['paths']['/validate_test']['post']['requestBody']['content']['application/json']['schema']['properties']
Expand Down