From f18a3baee2c664e5bf9f6461ae4ece8f5dfd5e85 Mon Sep 17 00:00:00 2001 From: Andreas Haller Date: Sat, 9 Nov 2024 15:31:06 +0100 Subject: [PATCH 1/2] Support discriminator for request bodies This loads the referenced schema for request bodies from the main OpenAPI document instead of creating a new Schema instance. This change makes it necessary to setup after_property_validation globally, which is not optimal. --- lib/openapi_first/builder.rb | 16 +++++--- lib/openapi_first/validators/request_body.rb | 9 ++--- spec/data/discriminator-refs.yaml | 19 ++++++++++ spec/hooks_spec.rb | 20 ++++++++-- spec/middlewares/request_validation_spec.rb | 39 ++++++++++++++++++++ 5 files changed, 90 insertions(+), 13 deletions(-) diff --git a/lib/openapi_first/builder.rb b/lib/openapi_first/builder.rb index 2e25996c..b459ed93 100644 --- a/lib/openapi_first/builder.rb +++ b/lib/openapi_first/builder.rb @@ -20,7 +20,12 @@ def initialize(resolved, filepath:, config:) ref_resolver = JSONSchemer::CachedResolver.new do |uri| Refs.load_file(File.join(File.dirname(filepath), uri.path)) end - @doc = JSONSchemer.openapi(resolved, ref_resolver:) + configuration = JSONSchemer::Configuration.new( + ref_resolver:, + insert_property_defaults: true, + after_property_validation: config.hooks[:after_request_body_property_validation] + ) + @doc = JSONSchemer.openapi(resolved, configuration:) @config = config @openapi_version = (resolved['openapi'] || resolved['swagger'])[0..2] end @@ -32,7 +37,8 @@ def router # rubocop:disable Metrics/MethodLength resolved['paths'].each do |path, path_item_object| path_item_object.slice(*REQUEST_METHODS).keys.map do |request_method| operation_object = path_item_object[request_method] - build_requests(path, request_method, operation_object, path_item_object).each do |request| + operation_pointer = JsonPointer.append('#', 'paths', URI::DEFAULT_PARSER.escape(path), request_method) + build_requests(path:, request_method:, operation_object:, operation_pointer:, path_item_object:).each do |request| router.add_request( request, request_method:, @@ -40,7 +46,6 @@ def router # rubocop:disable Metrics/MethodLength content_type: request.content_type ) end - operation_pointer = JsonPointer.append('#', 'paths', URI::DEFAULT_PARSER.escape(path), request_method) build_responses(operation_pointer:, operation_object:).each do |response| router.add_response( response, @@ -55,14 +60,15 @@ def router # rubocop:disable Metrics/MethodLength router end - def build_requests(path, request_method, operation_object, path_item_object) + def build_requests(path:, request_method:, operation_object:, operation_pointer:, path_item_object:) hooks = config.hooks path_item_parameters = path_item_object['parameters'] parameters = operation_object['parameters'].to_a.chain(path_item_parameters.to_a) required_body = operation_object.dig('requestBody', 'required') == true result = operation_object.dig('requestBody', 'content')&.map do |content_type, content| + content_schema = @doc.ref(JsonPointer.append(operation_pointer, 'requestBody', 'content', content_type, 'schema')) Request.new(path:, request_method:, operation_object:, parameters:, content_type:, - content_schema: content['schema'], required_body:, hooks:, openapi_version:) + content_schema:, required_body:, hooks:, openapi_version:) end || [] return result if required_body diff --git a/lib/openapi_first/validators/request_body.rb b/lib/openapi_first/validators/request_body.rb index 29efc66b..005e804d 100644 --- a/lib/openapi_first/validators/request_body.rb +++ b/lib/openapi_first/validators/request_body.rb @@ -7,10 +7,7 @@ def self.for(request_definition, openapi_version:, hooks: {}) schema = request_definition.content_schema return unless schema - after_property_validation = hooks[:after_request_body_property_validation] - - new(Schema.new(schema, after_property_validation:, openapi_version:), - required: request_definition.required_request_body?) + new(schema, required: request_definition.required_request_body?) end def initialize(schema, required:) @@ -25,7 +22,9 @@ def call(request) return end - validation = @schema.validate(request_body) + validation = Schema::ValidationResult.new( + @schema.validate(request_body, access_mode: 'write') + ) Failure.fail!(:invalid_body, errors: validation.errors) if validation.error? end diff --git a/spec/data/discriminator-refs.yaml b/spec/data/discriminator-refs.yaml index 7851be8c..ccdc9203 100644 --- a/spec/data/discriminator-refs.yaml +++ b/spec/data/discriminator-refs.yaml @@ -21,6 +21,25 @@ paths: propertyName: petType type: array "/pets-file": + post: + requestBody: + required: true + content: + application/json: + schema: + items: + oneOf: + - "$ref": "./components/schemas/cat.yaml" + - "$ref": "./components/schemas/dog.yaml" + discriminator: + propertyName: petType + mapping: + cat: "./components/schemas/cat.yaml" + dog: "./components/schemas/dog.yaml" + type: array + responses: + "201": + description: successful get: summary: Pets responses: diff --git a/spec/hooks_spec.rb b/spec/hooks_spec.rb index 9107d7b9..c12577dc 100644 --- a/spec/hooks_spec.rb +++ b/spec/hooks_spec.rb @@ -92,7 +92,7 @@ def build_request(path, method: 'GET', body: nil) } } ], - 'get' => { + 'post' => { 'parameters' => [ { 'name' => 'page', @@ -102,6 +102,20 @@ def build_request(path, method: 'GET', body: nil) } } ], + 'requestBody' => { + 'content' => { + 'application/json' => { + 'schema' => { + 'type' => 'object', + 'properties' => { + 'name' => { + 'type' => 'string' + } + } + } + } + } + }, 'responses' => { '200' => { 'description' => 'ok' @@ -120,7 +134,7 @@ def build_request(path, method: 'GET', body: nil) end end - definition.validate_request(build_request('/blue?page=2')) + definition.validate_request(build_request('/blue?page=2', method: 'POST', body: '{"name": "Quentin"}')) expect(called).to eq([ [{ 'color' => 'blue' }, 'color', { @@ -138,7 +152,7 @@ def build_request(path, method: 'GET', body: nil) data[property] = 'two' if property == 'page' end end - validated = definition.validate_request(build_request('/blue?page=2')) + validated = definition.validate_request(build_request('/blue?page=2', method: 'POST', body: '{"name": "Quentin"}')) expect(validated.parsed_query['page']).to eq('two') expect(validated).to be_valid end diff --git a/spec/middlewares/request_validation_spec.rb b/spec/middlewares/request_validation_spec.rb index 8b105b42..92903e33 100644 --- a/spec/middlewares/request_validation_spec.rb +++ b/spec/middlewares/request_validation_spec.rb @@ -119,6 +119,45 @@ def status = 409 end end + context 'with discriminator' do + let(:app) do + Rack::Builder.app do + use(OpenapiFirst::Middlewares::RequestValidation, + spec: File.expand_path('../data/discriminator-refs.yaml', __dir__)) + run lambda { |_env| + Rack::Response.new('hello', 200).finish + } + end + end + + context 'with an invalid request' do + let(:request_body) { json_dump([{ id: 1, petType: 'unknown', meow: 'Huh' }]) } + + it 'fails' do + header 'Content-Type', 'application/json' + post '/pets-file', request_body + + expect(last_response.status).to eq 400 + end + end + + context 'with a valid request' do + let(:request_body) do + json_dump([ + { id: 1, petType: 'cat', meow: 'Prrr' }, + { id: 2, petType: 'dog', bark: 'Woof' } + ]) + end + + it 'succeeds' do + header 'Content-Type', 'application/json' + post '/pets-file', request_body + + expect(last_response.status).to eq 200 + end + end + end + context 'with error_response: false' do let(:called) { [] } From 063c78a9c87cd086c5c682c750d5912b91b9f05e Mon Sep 17 00:00:00 2001 From: Andreas Haller Date: Tue, 3 Dec 2024 14:37:58 +0100 Subject: [PATCH 2/2] Refactor setting up validators per request definition --- lib/openapi_first/builder.rb | 8 +++++--- lib/openapi_first/request_validator.rb | 11 +++-------- lib/openapi_first/validators/request_body.rb | 13 +++---------- .../validators/request_parameters.rb | 19 ++++--------------- 4 files changed, 15 insertions(+), 36 deletions(-) diff --git a/lib/openapi_first/builder.rb b/lib/openapi_first/builder.rb index b459ed93..5ebb1f5c 100644 --- a/lib/openapi_first/builder.rb +++ b/lib/openapi_first/builder.rb @@ -38,7 +38,8 @@ def router # rubocop:disable Metrics/MethodLength path_item_object.slice(*REQUEST_METHODS).keys.map do |request_method| operation_object = path_item_object[request_method] operation_pointer = JsonPointer.append('#', 'paths', URI::DEFAULT_PARSER.escape(path), request_method) - build_requests(path:, request_method:, operation_object:, operation_pointer:, path_item_object:).each do |request| + build_requests(path:, request_method:, operation_object:, operation_pointer:, + path_item_object:).each do |request| router.add_request( request, request_method:, @@ -65,8 +66,9 @@ def build_requests(path:, request_method:, operation_object:, operation_pointer: path_item_parameters = path_item_object['parameters'] parameters = operation_object['parameters'].to_a.chain(path_item_parameters.to_a) required_body = operation_object.dig('requestBody', 'required') == true - result = operation_object.dig('requestBody', 'content')&.map do |content_type, content| - content_schema = @doc.ref(JsonPointer.append(operation_pointer, 'requestBody', 'content', content_type, 'schema')) + result = operation_object.dig('requestBody', 'content')&.map do |content_type, _content| + content_schema = @doc.ref(JsonPointer.append(operation_pointer, 'requestBody', 'content', content_type, + 'schema')) Request.new(path:, request_method:, operation_object:, parameters:, content_type:, content_schema:, required_body:, hooks:, openapi_version:) end || [] diff --git a/lib/openapi_first/request_validator.rb b/lib/openapi_first/request_validator.rb index 5be46b1f..d426d3b1 100644 --- a/lib/openapi_first/request_validator.rb +++ b/lib/openapi_first/request_validator.rb @@ -7,15 +7,10 @@ module OpenapiFirst # Validates a Request against a request definition. class RequestValidator - VALIDATORS = [ - Validators::RequestParameters, - Validators::RequestBody - ].freeze - def initialize(request_definition, openapi_version:, hooks: {}) - @validators = VALIDATORS.filter_map do |klass| - klass.for(request_definition, hooks:, openapi_version:) - end + @validators = [] + @validators << Validators::RequestBody.new(request_definition) if request_definition.content_schema + @validators.concat Validators::RequestParameters.for(request_definition, openapi_version:, hooks:) end def call(parsed_request) diff --git a/lib/openapi_first/validators/request_body.rb b/lib/openapi_first/validators/request_body.rb index 005e804d..e5c5b517 100644 --- a/lib/openapi_first/validators/request_body.rb +++ b/lib/openapi_first/validators/request_body.rb @@ -3,16 +3,9 @@ module OpenapiFirst module Validators class RequestBody - def self.for(request_definition, openapi_version:, hooks: {}) - schema = request_definition.content_schema - return unless schema - - new(schema, required: request_definition.required_request_body?) - end - - def initialize(schema, required:) - @schema = schema - @required = required + def initialize(request_definition) + @schema = request_definition.content_schema + @required = request_definition.required_request_body? end def call(request) diff --git a/lib/openapi_first/validators/request_parameters.rb b/lib/openapi_first/validators/request_parameters.rb index 2a63d4ab..22ac88ea 100644 --- a/lib/openapi_first/validators/request_parameters.rb +++ b/lib/openapi_first/validators/request_parameters.rb @@ -2,7 +2,7 @@ module OpenapiFirst module Validators - class RequestParameters + module RequestParameters RequestHeaders = Data.define(:schema) do def call(parsed_values) validation = schema.validate(parsed_values[:headers]) @@ -38,23 +38,12 @@ def call(parsed_values) cookie_schema: RequestCookies }.freeze - def self.for(operation, openapi_version:, hooks: {}) + def self.for(request_definition, openapi_version:, hooks: {}) after_property_validation = hooks[:after_request_parameter_property_validation] - validators = VALIDATORS.filter_map do |key, klass| - schema = operation.send(key) + VALIDATORS.filter_map do |key, klass| + schema = request_definition.send(key) klass.new(Schema.new(schema, after_property_validation:, openapi_version:)) if schema end - return if validators.empty? - - new(validators) - end - - def initialize(validators) - @validators = validators - end - - def call(parsed_values) - @validators.each { |validator| validator.call(parsed_values) } end end end