diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bf5377b2..a8be12b3 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,22 +1,28 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-03-23 14:23:53 UTC using RuboCop version 1.74.0. +# on 2025-04-02 15:12:27 UTC using RuboCop version 1.74.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 9 +# Offense count: 12 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: - Max: 28 + Max: 32 -# Offense count: 1 +# Offense count: 4 # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/CyclomaticComplexity: - Max: 9 + Max: 14 + +# Offense count: 1 +# Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns. +Metrics/MethodLength: + Exclude: + - 'lib/openapi_first/schema/validation_error.rb' -# Offense count: 3 +# Offense count: 2 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: Max: 8 @@ -25,10 +31,3 @@ Metrics/ParameterLists: # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/PerceivedComplexity: Max: 9 - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings. -# URISchemes: http, https -Layout/LineLength: - Max: 121 diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e7d22da..31b6c971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog ## Unreleased +- No longer merge parameter schemas per of the same location (for example "query") in order to fix https://github.com/ahx/openapi_first/issues/320 + - `OpenapiFirst::Test::Methods[MyApplication]` returns a Module which adds an `app` method to be used by rack-test alonside the `assert_api_conform` method. - Make default coverage report less verbose The default formatter (TerminalFormatter) no longer prints all un-requested requests by default. You can set `test.coverage_formatter_options = { focused: false }` to get back the old behavior diff --git a/lib/openapi_first/builder.rb b/lib/openapi_first/builder.rb index c3d6a0ba..9f89aacf 100644 --- a/lib/openapi_first/builder.rb +++ b/lib/openapi_first/builder.rb @@ -1,6 +1,13 @@ # frozen_string_literal: true require 'json_schemer' + +require_relative 'failure' +require_relative 'router' +require_relative 'header' +require_relative 'request' +require_relative 'response' +require_relative 'schema/hash' require_relative 'ref_resolver' module OpenapiFirst @@ -17,10 +24,8 @@ def self.build_router(contents, filepath:, config:) end def initialize(contents, filepath:, config:) - @schemer_configuration = JSONSchemer.configuration.clone - @schemer_configuration.meta_schema = detect_meta_schema(contents, filepath) - @schemer_configuration.insert_property_defaults = true - + meta_schema = detect_meta_schema(contents, filepath) + @schemer_configuration = build_schemer_config(filepath:, meta_schema:) @config = config @contents = RefResolver.for(contents, filepath:) end @@ -28,6 +33,18 @@ def initialize(contents, filepath:, config:) attr_reader :config private attr_reader :schemer_configuration + def build_schemer_config(filepath:, meta_schema:) + result = JSONSchemer.configuration.clone + dir = (filepath && File.absolute_path(File.dirname(filepath))) || Dir.pwd + result.base_uri = URI::File.build({ path: "#{dir}/" }) + result.ref_resolver = JSONSchemer::CachedResolver.new do |uri| + FileLoader.load(uri.path) + end + result.meta_schema = meta_schema + result.insert_property_defaults = true + result + end + def detect_meta_schema(document, filepath) # Copied from JSONSchemer 🙇🏻‍♂️ version = document['openapi'] @@ -46,10 +63,10 @@ def detect_meta_schema(document, filepath) def router # rubocop:disable Metrics/MethodLength router = OpenapiFirst::Router.new @contents.fetch('paths').each do |path, path_item_object| - path_parameters = resolve_parameters(path_item_object['parameters']) + path_parameters = path_item_object['parameters'] || [] path_item_object.resolved.keys.intersection(REQUEST_METHODS).map do |request_method| operation_object = path_item_object[request_method] - operation_parameters = resolve_parameters(operation_object['parameters']) + operation_parameters = operation_object['parameters'] || [] parameters = parse_parameters(operation_parameters.chain(path_parameters)) build_requests(path:, request_method:, operation_object:, @@ -79,10 +96,10 @@ def router # rubocop:disable Metrics/MethodLength def parse_parameters(parameters) grouped_parameters = group_parameters(parameters) ParsedParameters.new( - query: grouped_parameters[:query], - path: grouped_parameters[:path], - cookie: grouped_parameters[:cookie], - header: grouped_parameters[:header], + query: resolve_parameters(grouped_parameters[:query]), + path: resolve_parameters(grouped_parameters[:path]), + cookie: resolve_parameters(grouped_parameters[:cookie]), + header: resolve_parameters(grouped_parameters[:header]), query_schema: build_parameter_schema(grouped_parameters[:query]), path_schema: build_parameter_schema(grouped_parameters[:path]), cookie_schema: build_parameter_schema(grouped_parameters[:cookie]), @@ -99,11 +116,18 @@ def resolve_parameters(parameters) end def build_parameter_schema(parameters) - schema = build_parameters_schema(parameters) + return unless parameters - JSONSchemer.schema(schema, - configuration: schemer_configuration, - after_property_validation: config.hooks[:after_request_parameter_property_validation]) + required = [] + schemas = parameters.each_with_object({}) do |parameter, result| + schema = parameter['schema'].schema(configuration: schemer_configuration) + name = parameter['name']&.value + required << name if parameter['required']&.value + result[name] = schema if schema + end + + Schema::Hash.new(schemas, required:, configuration: schemer_configuration, + after_property_validation: config.hooks[:after_request_parameter_property_validation]) end def build_requests(path:, request_method:, operation_object:, parameters:) @@ -141,20 +165,15 @@ def build_responses(responses:, request:) return [] unless responses responses.flat_map do |status, response_object| - headers = response_object['headers']&.resolved - headers_schema = JSONSchemer::Schema.new( - build_headers_schema(headers), - configuration: schemer_configuration - ) + headers = build_response_headers(response_object['headers']) response_object['content']&.map do |content_type, content_object| content_schema = content_object['schema'].schema(configuration: schemer_configuration) Response.new(status:, headers:, - headers_schema:, content_type:, content_schema:, key: [request.key, status, content_type].join(':')) - end || Response.new(status:, headers:, headers_schema:, content_type: nil, + end || Response.new(status:, headers:, content_type: nil, content_schema: nil, key: [request.key, status, nil].join(':')) end end @@ -162,49 +181,32 @@ def build_responses(responses:, request:) IGNORED_HEADER_PARAMETERS = Set['Content-Type', 'Accept', 'Authorization'].freeze private_constant :IGNORED_HEADER_PARAMETERS - def group_parameters(parameter_definitions) - result = {} - parameter_definitions&.each do |parameter| - (result[parameter['in'].to_sym] ||= []) << parameter - end - result[:header]&.reject! { IGNORED_HEADER_PARAMETERS.include?(_1['name']) } - result - end - - def build_headers_schema(headers_object) - return unless headers_object&.any? + def build_response_headers(headers_object) + return if headers_object.nil? - properties = {} - required = [] + result = [] headers_object.each do |name, header| - schema = header['schema'] - next if name.casecmp('content-type').zero? - - properties[name] = schema if schema - required << name if header['required'] + next if header['schema'].nil? + next if IGNORED_HEADER_PARAMETERS.include?(name) + + header = Header.new( + name:, + schema: header['schema'].schema(configuration: schemer_configuration), + required?: header['required']&.value == true, + node: header + ) + result << header end - { - 'properties' => properties, - 'required' => required - } + result end - def build_parameters_schema(parameters) - return unless parameters - - properties = {} - required = [] - parameters.each do |parameter| - schema = parameter['schema'] - name = parameter['name'] - properties[name] = schema if schema - required << name if parameter['required'] + def group_parameters(parameter_definitions) + result = {} + parameter_definitions&.each do |parameter| + (result[parameter['in']&.value&.to_sym] ||= []) << parameter end - - { - 'properties' => properties, - 'required' => required - } + result[:header]&.reject! { IGNORED_HEADER_PARAMETERS.include?(_1['name']&.value) } + result end ParsedParameters = Data.define(:path, :query, :header, :cookie, :path_schema, :query_schema, :header_schema, diff --git a/lib/openapi_first/definition.rb b/lib/openapi_first/definition.rb index f4ec8b99..30ad9c0d 100644 --- a/lib/openapi_first/definition.rb +++ b/lib/openapi_first/definition.rb @@ -1,9 +1,5 @@ # frozen_string_literal: true -require_relative 'failure' -require_relative 'router' -require_relative 'request' -require_relative 'response' require_relative 'builder' require 'forwardable' diff --git a/lib/openapi_first/header.rb b/lib/openapi_first/header.rb new file mode 100644 index 00000000..50cd6d6c --- /dev/null +++ b/lib/openapi_first/header.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module OpenapiFirst + Header = Data.define(:name, :required?, :schema, :node) do + def resolved_schema + node['schema']&.resolved + end + end +end diff --git a/lib/openapi_first/middlewares/response_validation.rb b/lib/openapi_first/middlewares/response_validation.rb index bfe4963c..454dda2c 100644 --- a/lib/openapi_first/middlewares/response_validation.rb +++ b/lib/openapi_first/middlewares/response_validation.rb @@ -25,7 +25,8 @@ def initialize(app, options = {}) def call(env) status, headers, body = @app.call(env) - @definition.validate_response(Rack::Request.new(env), Rack::Response[status, headers, body], raise_error: @raise) + @definition.validate_response(Rack::Request.new(env), Rack::Response[status, headers, body], + raise_error: @raise) [status, headers, body] end end diff --git a/lib/openapi_first/ref_resolver.rb b/lib/openapi_first/ref_resolver.rb index e329df17..7d8831b5 100644 --- a/lib/openapi_first/ref_resolver.rb +++ b/lib/openapi_first/ref_resolver.rb @@ -41,8 +41,11 @@ def initialize(value, context: value, filepath: nil) @value = value @context = context @filepath = filepath - dir = File.dirname(File.expand_path(filepath)) if filepath - @dir = (dir && File.absolute_path(dir)) || Dir.pwd + @dir = if filepath + File.dirname(File.absolute_path(filepath)) + else + Dir.pwd + end end # The value of this node @@ -52,7 +55,11 @@ def initialize(value, context: value, filepath: nil) # The object where this node was found in attr_reader :context - private attr_reader :filepath + attr_reader :filepath + + def ==(_other) + raise "Don't call == on an unresolved value. Use .value == other instead." + end def resolve_ref(pointer) if pointer.start_with?('#') @@ -89,6 +96,10 @@ class Hash include Diggable include Enumerable + def ==(_other) + raise "Don't call == on an unresolved value. Use .value == other instead." + end + def resolved return resolve_ref(value['$ref']).value if value.key?('$ref') @@ -108,17 +119,16 @@ def fetch(key) end def each - resolved.each do |key, value| - yield key, RefResolver.for(value, filepath:, context:) + resolved.each_key do |key| + yield key, self[key] end end - def schema(options = {}) - ref_resolver = JSONSchemer::CachedResolver.new do |uri| - FileLoader.load(uri.path) - end + # You have to pass configuration or ref_resolver + def schema(options) base_uri = URI::File.build({ path: "#{dir}/" }) - root = JSONSchemer::Schema.new(context, base_uri:, ref_resolver:, **options) + root = JSONSchemer::Schema.new(context, base_uri:, **options) + # binding.irb if value['maxItems'] == 4 JSONSchemer::Schema.new(value, nil, root, base_uri:, **options) end end @@ -137,8 +147,8 @@ def [](index) end def each - resolved.each do |item| - yield RefResolver.for(item, filepath:, context:) + resolved.each_with_index do |_item, index| + yield self[index] end end diff --git a/lib/openapi_first/response.rb b/lib/openapi_first/response.rb index ee4dfdbb..e260fdb4 100644 --- a/lib/openapi_first/response.rb +++ b/lib/openapi_first/response.rb @@ -9,21 +9,20 @@ module OpenapiFirst # This is not a direct reflecton of the OpenAPI 3.X response definition, but a combination of # status, content type and content schema. class Response - def initialize(status:, headers:, headers_schema:, content_type:, content_schema:, key:) + def initialize(status:, headers:, content_type:, content_schema:, key:) @status = status @content_type = content_type @content_schema = content_schema @headers = headers - @headers_schema = headers_schema @key = key @parser = ResponseParser.new(headers:, content_type:) - @validator = ResponseValidator.new(self) + @validator = ResponseValidator.new(content_schema:, headers:) end # @attr_reader [Integer] status The HTTP status code of the response definition. # @attr_reader [String, nil] content_type Content type of this response. # @attr_reader [Schema, nil] content_schema the Schema of the response body. - attr_reader :status, :content_type, :content_schema, :headers, :headers_schema, :key + attr_reader :status, :content_type, :content_schema, :headers, :key def validate(response) parsed_values = nil diff --git a/lib/openapi_first/response_parser.rb b/lib/openapi_first/response_parser.rb index 36181a9d..a4bd5145 100644 --- a/lib/openapi_first/response_parser.rb +++ b/lib/openapi_first/response_parser.rb @@ -15,7 +15,7 @@ def initialize(headers:, content_type:) def parse(rack_response) ParsedResponse.new( body: @body_parser.call(read_body(rack_response)), - headers: @headers_parser.call(rack_response.headers) + headers: @headers_parser&.call(rack_response.headers) || {} ) end @@ -30,9 +30,16 @@ def read_body(rack_response) rack_response.body end - def build_headers_parser(header_definitions) - headers_as_parameters = header_definitions.to_a.map do |name, definition| - definition.merge('name' => name, 'in' => 'header') + def build_headers_parser(headers) + return unless headers&.any? + + headers_as_parameters = headers.map do |header| + { + 'name' => header.name, + 'explode' => false, + 'in' => 'header', + 'schema' => header.resolved_schema + } end OpenapiParameters::Header.new(headers_as_parameters).method(:unpack) end diff --git a/lib/openapi_first/response_validator.rb b/lib/openapi_first/response_validator.rb index 3fafc593..f13e1ce2 100644 --- a/lib/openapi_first/response_validator.rb +++ b/lib/openapi_first/response_validator.rb @@ -11,10 +11,10 @@ class ResponseValidator Validators::ResponseBody ].freeze - def initialize(response_definition) - @validators = VALIDATORS.filter_map do |klass| - klass.for(response_definition) - end + def initialize(content_schema:, headers:) + @validators = [] + @validators << Validators::ResponseBody.new(content_schema) if content_schema + @validators << Validators::ResponseHeaders.new(headers) if headers&.any? end def call(parsed_response) diff --git a/lib/openapi_first/schema/hash.rb b/lib/openapi_first/schema/hash.rb new file mode 100644 index 00000000..2e416a11 --- /dev/null +++ b/lib/openapi_first/schema/hash.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require_relative 'validation_error' + +module OpenapiFirst + class Schema + # A hash of Schemas + class Hash + # @param schema Hash of schemas + # @param required Array of required keys + def initialize(schemas, required: nil, **options) + @schemas = schemas + @options = options + @after_property_validation = options.delete(:after_property_validation) + schema = { 'type' => 'object' } + schema['required'] = required if required + @root_schema = JSONSchemer.schema(schema, **options) + end + + def validate(root_value) + validation = @root_schema.validate(root_value) + validations = @schemas.reduce(validation) do |enum, (key, schema)| + root_value[key] = schema.value['default'] if schema.value.key?('default') && !root_value.key?(key) + next enum unless root_value.key?(key) + + value = root_value[key] + key_validation = schema.validate(value) + @after_property_validation&.each do |hook| + hook.call(root_value, key, schema.value, nil) + end + enum.chain(key_validation.map do |err| + data_pointer = "/#{key}" + err.merge( + 'error' => JSONSchemer::Errors.pretty(err), + 'data_pointer' => data_pointer + ) + end) + end + ValidationResult.new(validations) + end + end + end +end diff --git a/lib/openapi_first/schema/validation_error.rb b/lib/openapi_first/schema/validation_error.rb index 02e3f975..37bb9802 100644 --- a/lib/openapi_first/schema/validation_error.rb +++ b/lib/openapi_first/schema/validation_error.rb @@ -3,7 +3,44 @@ module OpenapiFirst class Schema # One of multiple validation errors. Returned by Schema::ValidationResult#errors. - ValidationError = Data.define(:value, :message, :data_pointer, :schema_pointer, :type, :details, :schema) do + ValidationError = Data.define(:value, :data_pointer, :schema_pointer, :type, :details, :schema) do + # This returns an error message for this specific error. + # This it copied from json_schemer here to be easier to customize when passing custom data_pointers. + def message + location = data_pointer.empty? ? 'root' : "`#{data_pointer}`" + + case type + when 'required' + keys = details.fetch('missing_keys', []).join(', ') + "object at #{location} is missing required properties: #{keys}" + when 'dependentRequired' + keys = details.fetch('missing_keys').join(', ') + "object at #{location} is missing required properties: #{keys}" + when 'string', 'boolean', 'number' + "value at #{location} is not a #{type}" + when 'array', 'object', 'integer' + "value at #{location} is not an #{type}" + when 'null' + "value at #{location} is not #{type}" + when 'pattern' + "string at #{location} does not match pattern: #{schema.fetch('pattern')}" + when 'format' + "value at #{location} does not match format: #{schema.fetch('format')}" + when 'const' + "value at #{location} is not: #{schema.fetch('const').inspect}" + when 'enum' + "value at #{location} is not one of: #{schema.fetch('enum')}" + when 'minimum' + "number at #{location} is less than: #{schema['minimum']}" + when 'maximum' + "number at #{location} is greater than: #{schema['maximum']}" + when 'readOnly' + "value at #{location} is `readOnly`" + else + "value at #{location} is invalid (#{type.inspect})" + end + end + # @deprecated Please use {#message} instead def error warn 'OpenapiFirst::Schema::ValidationError#error is deprecated. Use #message instead.' diff --git a/lib/openapi_first/schema/validation_result.rb b/lib/openapi_first/schema/validation_result.rb index dee52f32..1cc17bc3 100644 --- a/lib/openapi_first/schema/validation_result.rb +++ b/lib/openapi_first/schema/validation_result.rb @@ -17,7 +17,6 @@ def errors @errors ||= @validation.map do |err| ValidationError.new( value: err['data'], - message: err['error'], data_pointer: err['data_pointer'], schema_pointer: err['schema_pointer'], type: err['type'], diff --git a/lib/openapi_first/validators/request_parameters.rb b/lib/openapi_first/validators/request_parameters.rb index b56bceaf..8bd1271a 100644 --- a/lib/openapi_first/validators/request_parameters.rb +++ b/lib/openapi_first/validators/request_parameters.rb @@ -6,7 +6,6 @@ module RequestParameters RequestHeaders = Data.define(:schema) do def call(parsed_request) validation = schema.validate(parsed_request.headers) - validation = Schema::ValidationResult.new(validation.to_a) Failure.fail!(:invalid_header, errors: validation.errors) if validation.error? end end @@ -14,7 +13,6 @@ def call(parsed_request) Path = Data.define(:schema) do def call(parsed_request) validation = schema.validate(parsed_request.path) - validation = Schema::ValidationResult.new(validation.to_a) Failure.fail!(:invalid_path, errors: validation.errors) if validation.error? end end @@ -22,7 +20,6 @@ def call(parsed_request) Query = Data.define(:schema) do def call(parsed_request) validation = schema.validate(parsed_request.query) - validation = Schema::ValidationResult.new(validation.to_a) Failure.fail!(:invalid_query, errors: validation.errors) if validation.error? end end @@ -30,7 +27,6 @@ def call(parsed_request) RequestCookies = Data.define(:schema) do def call(parsed_request) validation = schema.validate(parsed_request.cookies) - validation = Schema::ValidationResult.new(validation.to_a) Failure.fail!(:invalid_cookie, errors: validation.errors) if validation.error? end end @@ -45,7 +41,7 @@ def call(parsed_request) def self.for(args) VALIDATORS.filter_map do |key, klass| schema = args[key] - klass.new(schema) if schema.value + klass.new(schema) if schema end end end diff --git a/lib/openapi_first/validators/response_body.rb b/lib/openapi_first/validators/response_body.rb index a35356d2..7cd16fe5 100644 --- a/lib/openapi_first/validators/response_body.rb +++ b/lib/openapi_first/validators/response_body.rb @@ -5,13 +5,6 @@ module OpenapiFirst module Validators class ResponseBody - def self.for(response_definition) - schema = response_definition&.content_schema - return unless schema - - new(schema) - end - def initialize(schema) @schema = schema end diff --git a/lib/openapi_first/validators/response_headers.rb b/lib/openapi_first/validators/response_headers.rb index 8c1c73af..e5554d35 100644 --- a/lib/openapi_first/validators/response_headers.rb +++ b/lib/openapi_first/validators/response_headers.rb @@ -3,24 +3,37 @@ module OpenapiFirst module Validators class ResponseHeaders - def self.for(response_definition) - schema = response_definition&.headers_schema - return unless schema&.value - - new(schema) + def initialize(headers) + @headers = headers end - def initialize(schema) - @schema = schema + attr_reader :headers + + def call(parsed_response) + headers.each do |header| + header_value = parsed_response.headers[header.name] + next if header_value.nil? && !header.required? + + validation_errors = header.schema.validate(header_value) + next unless validation_errors.any? + + Failure.fail!(:invalid_response_header, + errors: [error_for(data_pointer: "/#{header.name}", value: header_value, + error: validation_errors.first)]) + end end - attr_reader :schema + private - def call(parsed_request) - validation = Schema::ValidationResult.new( - schema.validate(parsed_request.headers) + def error_for(data_pointer:, value:, error:) + Schema::ValidationError.new( + value: value, + data_pointer:, + schema_pointer: error['schema_pointer'], + type: error['type'], + details: error['details'], + schema: error['schema'] ) - Failure.fail!(:invalid_response_header, errors: validation.errors) if validation.error? end end end diff --git a/spec/data/components/headers/x-authors.yaml b/spec/data/components/headers/x-authors.yaml new file mode 100644 index 00000000..2f765841 --- /dev/null +++ b/spec/data/components/headers/x-authors.yaml @@ -0,0 +1,4 @@ +schema: + type: array + items: + $ref: '../schemas/name.yaml' diff --git a/spec/data/components/parameters/filter.yaml b/spec/data/components/parameters/filter.yaml new file mode 100644 index 00000000..d8ef3cb6 --- /dev/null +++ b/spec/data/components/parameters/filter.yaml @@ -0,0 +1,15 @@ +name: filter +in: query +description: Filter results +example: "filter[name]" +style: deepObject +schema: + type: object + required: [name] + properties: + name: + $ref: '../schemas/name.yaml' + other: + type: object + id: + type: integer diff --git a/spec/data/components/parameters/strings.yaml b/spec/data/components/parameters/strings.yaml new file mode 100644 index 00000000..441dd1aa --- /dev/null +++ b/spec/data/components/parameters/strings.yaml @@ -0,0 +1,9 @@ +name: strings +in: query +required: false +explode: false +schema: + type: array + maxItems: 4 + items: + $ref: '../schemas/name.yaml' diff --git a/spec/data/components/schemas/name.yaml b/spec/data/components/schemas/name.yaml new file mode 100644 index 00000000..74812036 --- /dev/null +++ b/spec/data/components/schemas/name.yaml @@ -0,0 +1,2 @@ +type: string +minLength: 2 diff --git a/spec/data/parameters-array.yaml b/spec/data/parameters-array.yaml index dbeafcad..691c2cdb 100644 --- a/spec/data/parameters-array.yaml +++ b/spec/data/parameters-array.yaml @@ -12,15 +12,7 @@ paths: summary: Get some info operationId: info parameters: - - name: strings - in: query - required: false - explode: false - schema: - type: array - maxItems: 4 - items: - type: string + - $ref: './components/parameters/strings.yaml' - name: integers in: query required: false diff --git a/spec/data/path-parameter-validation.yaml b/spec/data/path-parameter-validation.yaml index 34d969fc..4edcf124 100644 --- a/spec/data/path-parameter-validation.yaml +++ b/spec/data/path-parameter-validation.yaml @@ -3,6 +3,17 @@ info: version: 1.0.0 title: Path Parameter Validation paths: + /friends: + parameters: + - name: search[name] + in: query + explode: false + schema: + type: array + items: + $ref: './components/schemas/name.yaml' + get: + summary: Search a friend /pets/{petId}: parameters: - name: petId diff --git a/spec/data/query-parameter-validation.yaml b/spec/data/query-parameter-validation.yaml index 4a4762a5..50ad31cd 100644 --- a/spec/data/query-parameter-validation.yaml +++ b/spec/data/query-parameter-validation.yaml @@ -21,21 +21,7 @@ paths: required: true schema: type: string - - name: filter - in: query - description: Filter results - example: "filter[tag]" - style: deepObject - schema: - type: object - required: [tag] - properties: - tag: - type: string - other: - type: object - id: - type: integer + - $ref: './components/parameters/filter.yaml' - name: limit in: query description: How many items to return at one time (max 100) diff --git a/spec/data/response-header.yaml b/spec/data/response-header.yaml index 904df2ba..57614b31 100644 --- a/spec/data/response-header.yaml +++ b/spec/data/response-header.yaml @@ -16,7 +16,7 @@ paths: description: Ok headers: OptionalWithoutSchema: - description: "optonal" + description: "optional" "Content-Type": required: true schema: @@ -30,3 +30,5 @@ paths: X-Id: schema: type: integer + X-Authors: + $ref: './components/headers/x-authors.yaml' diff --git a/spec/definition_spec.rb b/spec/definition_spec.rb index 1d14a843..d7dcf416 100644 --- a/spec/definition_spec.rb +++ b/spec/definition_spec.rb @@ -74,9 +74,8 @@ def build_request(path, method: 'GET') validated = definition.validate_request(request) expect(validated.error.errors).to contain_exactly(have_attributes( value: 'foo', - message: String, data_pointer: '/id', - schema_pointer: '/properties/id', + schema_pointer: '', type: 'integer', details: nil, schema: { 'type' => 'integer' } diff --git a/spec/middlewares/request_validation/path_parameter_validation_spec.rb b/spec/middlewares/request_validation/path_parameter_validation_spec.rb index 23047af6..80e155db 100644 --- a/spec/middlewares/request_validation/path_parameter_validation_spec.rb +++ b/spec/middlewares/request_validation/path_parameter_validation_spec.rb @@ -22,49 +22,56 @@ end end - describe '#call' do - it 'returns 400 if path parameter is invalid' do - get '/pets/not-an-integer' + it 'returns 400 if path parameter is invalid' do + get '/pets/not-an-integer' - expect(last_response.status).to eq 400 - end + expect(last_response.status).to eq 400 + end - it 'adds the converted path parameter to env' do - get '/pets/42' - expect(last_request.env[OpenapiFirst::REQUEST].parsed_path_parameters['petId']).to eq 42 - end + it 'adds the converted path parameter to env' do + get '/pets/42' + expect(last_request.env[OpenapiFirst::REQUEST].parsed_path_parameters['petId']).to eq 42 + end + + it 'works with nested refs' do + get '/friends?search[name]=a,b' + expect(last_response.status).to eq(400) - context 'with valid parameters' do - it 'succeeds for valid integer' do - get '/pets/42' - expect(last_response.status).to eq(200) - end + get '/friends?search[name]=ahmed,bo' + expect(last_response.status).to eq(200) + expect(last_request.env[OpenapiFirst::REQUEST].parsed_query['search[name]']).to eq(%w[ahmed bo]) + end - it 'succeeds for valid string' do - get '/users/joe' - expect(last_response.status).to eq(200) - end + context 'with valid parameters' do + it 'succeeds for valid integer' do + get '/pets/42' + expect(last_response.status).to eq(200) + end - it 'succeds for string with special characters' do - get '/users/joe!doe' - expect(last_response.status).to eq(200) - end + it 'succeeds for valid string' do + get '/users/joe' + expect(last_response.status).to eq(200) end - it 'returns 404 if path parameter is empty' do - get '/pets//' - expect(last_response.status).to be 404 + it 'succeds for string with special characters' do + get '/users/joe!doe' + expect(last_response.status).to eq(200) end + end + + it 'returns 404 if path parameter is empty' do + get '/pets//' + expect(last_response.status).to be 404 + end - context 'when raise_error: true' do - let(:raise_error_option) { true } + context 'when raise_error: true' do + let(:raise_error_option) { true } - it 'raises an error if query parameter is missing' do - message = 'Path segment is invalid: value at `/petId` is not an integer' - expect do - get '/pets/not-an-integer' - end.to raise_error OpenapiFirst::RequestInvalidError, message - end + it 'raises an error if query parameter is missing' do + message = 'Path segment is invalid: value at `/petId` is not an integer' + expect do + get '/pets/not-an-integer' + end.to raise_error OpenapiFirst::RequestInvalidError, message end end end diff --git a/spec/middlewares/request_validation/query_parameter_validation_spec.rb b/spec/middlewares/request_validation/query_parameter_validation_spec.rb index 245faa1a..fa7d02dc 100644 --- a/spec/middlewares/request_validation/query_parameter_validation_spec.rb +++ b/spec/middlewares/request_validation/query_parameter_validation_spec.rb @@ -104,20 +104,39 @@ expect(last_request.env[OpenapiFirst::REQUEST].parsed_query).to eq expected_params end + context 'multiple parameter validation failures' do + let(:spec) { OpenapiFirst.load('./spec/data/parameters.yaml') } + + it 'returns multiple errors' do + params = { + birthdate: '1', + limit: 'a' + } + get '/search', params + expect(last_response.status).to eq(400) + errors = JSON.parse(last_response.body)['errors'] + expect(errors).to include( + hash_including('parameter' => 'limit', 'code' => 'integer'), + hash_including('parameter' => 'birthdate', 'code' => 'format'), + hash_including('parameter' => '', 'code' => 'required') + ) + end + end + context 'with array query parameters' do let(:spec) { OpenapiFirst.load('./spec/data/parameters-array.yaml') } context 'with form style no explode parameters (default)' do it 'parses the array' do params = { - strings: 'a,b,c', + strings: 'a1,b1,c1', integers: '2,3,4' } get '/default-style', params expect(last_response.status).to eq(200), last_response.body parsed_parameters = last_request.env[OpenapiFirst::REQUEST].parsed_query - expect(parsed_parameters['strings']).to eq %w[a b c] + expect(parsed_parameters['strings']).to eq %w[a1 b1 c1] expect(parsed_parameters['integers']).to eq [2, 3, 4] end @@ -186,7 +205,8 @@ { 'term' => 'Oscar', 'filter[id]' => '1', - 'filter[other]' => 'things' + 'filter[other]' => 'things', + 'filter[name]' => 'ahmed' } end @@ -197,8 +217,8 @@ expect(last_response.status).to eq 400 end - it 'returns 400 if non-required array parameter is nil' do - params['filter[tag]'] = nil + it 'returns 400 if non-required parameter is nil' do + params['filter[id]'] = nil get '/search', params expect(last_response.status).to eq 400 end @@ -305,7 +325,7 @@ def last_params end it 'converts nested params' do - get '/search', params.merge(filter: { id: '100', tag: 'foo' }) + get '/search', params.merge(filter: { id: '100', name: 'foo' }) expect(last_response.status).to eq(200), last_response.body expect(last_params.dig('filter', 'id')).to eq 100 diff --git a/spec/middlewares/response_validation/response_header_validation_spec.rb b/spec/middlewares/response_validation/response_header_validation_spec.rb new file mode 100644 index 00000000..24d1a260 --- /dev/null +++ b/spec/middlewares/response_validation/response_header_validation_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'rack' +require 'rack/test' +require 'openapi_first' + +RSpec.describe 'Response Header validation' do + include Rack::Test::Methods + + let(:app) do + Rack::Builder.app do + use OpenapiFirst::Middlewares::ResponseValidation, spec: './spec/data/response-header.yaml' + run(lambda do |env| + res = Rack::Response.new + res.status = 201 + res.headers.merge!(JSON.parse(Rack::Request.new(env).body.read)) + res.finish + end) + end + end + + before do + header Rack::CONTENT_TYPE, 'application/json' + end + + it 'succeeds with a valid header' do + post '/echo', JSON.generate({ 'Location' => '/echos/42', 'X-Id' => '42', 'OptionalWithoutSchema' => '432' }) + expect(last_response.status).to eq 201 + expect(last_response.headers['Location']).to eq '/echos/42' + expect(last_response.headers['X-Id']).to eq '42' + end + + it 'fails with an invalid header' do + expect do + post '/echo', JSON.generate({ 'Location' => '/echos/42', 'X-Id' => 'not-an-integer' }) + end.to raise_error OpenapiFirst::ResponseInvalidError, 'Response header is invalid: value at `/X-Id` is not an integer' + end + + it 'ignores "Content-Type" header' do + post '/echo', JSON.generate({ 'Location' => '/echos/42', 'Content-Type' => 'unknown' }) + expect(last_response.status).to eq 201 + end + + it 'succeeds with a ref in headers scheme' do + post '/echo', JSON.generate({ 'X-Authors' => 'Frank,Gabriela', 'Location' => '/echos/42' }) + expect(last_response.status).to eq 201 + end + + it 'fails with a ref in headers scheme' do + expect do + post '/echo', JSON.generate({ 'X-Authors' => 'A,B,C,D,E', 'Location' => '/echos/42' }) + end.to raise_error OpenapiFirst::ResponseInvalidError + end + + it 'fails with a missing header' do + expect do + post '/echo', JSON.generate({ 'X-Id' => '42' }) + end.to raise_error OpenapiFirst::ResponseInvalidError + end +end diff --git a/spec/middlewares/response_validation_spec.rb b/spec/middlewares/response_validation_spec.rb index f0af4fe2..a777b2ac 100644 --- a/spec/middlewares/response_validation_spec.rb +++ b/spec/middlewares/response_validation_spec.rb @@ -355,48 +355,6 @@ end end - describe 'response header validation' do - let(:app) do - Rack::Builder.app do - use OpenapiFirst::Middlewares::ResponseValidation, spec: './spec/data/response-header.yaml' - run(lambda do |env| - res = Rack::Response.new - res.status = 201 - res.headers.merge!(JSON.parse(Rack::Request.new(env).body.read)) - res.finish - end) - end - end - - before do - header Rack::CONTENT_TYPE, 'application/json' - end - - it 'succeeds with a valid header' do - post '/echo', JSON.generate({ 'Location' => '/echos/42', 'X-Id' => '42', 'OptionalWithoutSchema' => '432' }) - expect(last_response.status).to eq 201 - expect(last_response.headers['Location']).to eq '/echos/42' - expect(last_response.headers['X-Id']).to eq '42' - end - - it 'fails with an invalid header' do - expect do - post '/echo', JSON.generate({ 'Location' => '/echos/42', 'X-Id' => 'not-an-integer' }) - end.to raise_error OpenapiFirst::ResponseInvalidError - end - - it 'ignores "Content-Type" header' do - post '/echo', JSON.generate({ 'Location' => '/echos/42', 'Content-Type' => 'unknown' }) - expect(last_response.status).to eq 201 - end - - it 'fails with a missing header' do - expect do - post '/echo', JSON.generate({ 'X-Id' => '42' }) - end.to raise_error OpenapiFirst::ResponseInvalidError - end - end - describe 'with Response Object references' do let(:spec) { 'spec/data/petstore-openapi-object-references.yaml' } diff --git a/spec/ref_resolver_spec.rb b/spec/ref_resolver_spec.rb index 668e5bae..1ef16be0 100644 --- a/spec/ref_resolver_spec.rb +++ b/spec/ref_resolver_spec.rb @@ -68,9 +68,13 @@ end describe '#schema' do + let(:ref_resolver) do + ->(uri) { OpenapiFirst::FileLoader.load(uri.path) } + end + it 'returns a schema' do node = described_class.load('./spec/data/components/schemas/dog.yaml') - schema = node.schema + schema = node.schema(ref_resolver:) expect(schema.valid?({ bark: 'woff' })).to eq(true) expect(schema.valid?({ bark: 2 })).to eq(false) end @@ -90,14 +94,14 @@ it 'uses the right context' do node = described_class.load('./spec/data/petstore.yaml') - schema = node.dig('paths', '/pets/{petId}', 'get', 'responses', '200', 'content', 'application/json', 'schema').schema + schema = node.dig('paths', '/pets/{petId}', 'get', 'responses', '200', 'content', 'application/json', 'schema').schema(ref_resolver:) expect(schema.valid?([{ id: 2, name: 'Spet' }])).to eq(true) expect(schema.valid?([{ id: 'two', name: 'Spet' }])).to eq(false) end it 'works with relative paths in the schema' do node = described_class.load('./spec/data/splitted-train-travel-api/openapi.yaml') - schema = node.dig('paths', '/bookings', 'get', 'responses', '200', 'content', 'application/json', 'schema').schema + schema = node.dig('paths', '/bookings', 'get', 'responses', '200', 'content', 'application/json', 'schema').schema(ref_resolver:) expect(schema.valid?({ data: [{ has_bicycle: true }] })).to eq(true) expect(schema.valid?({ data: [{ has_bicycle: 'red' }] })).to eq(false) end @@ -205,6 +209,14 @@ end end + describe '==' do + it 'raises an error' do + expect do + doc['hash']['type'] == 'Dog' + end.to raise_error "Don't call == on an unresolved value. Use .value == other instead." + end + end + describe '#resolved' do it 'returns the resolved value' do expect(doc['hash'].resolved).to eq('type' => 'object') diff --git a/spec/response_parser_spec.rb b/spec/response_parser_spec.rb index 74b07f85..2e6c1c37 100644 --- a/spec/response_parser_spec.rb +++ b/spec/response_parser_spec.rb @@ -4,7 +4,7 @@ RSpec.describe OpenapiFirst::ResponseParser do let(:content_type) { 'application/json' } - let(:headers) { {} } + let(:headers) { nil } subject(:parsed) do described_class.new(content_type:, headers:).parse(rack_response) @@ -78,26 +78,23 @@ let(:content_type) { nil } let(:headers) do - { - 'OptionalWithoutSchema' => { description: 'optonal' }, - 'Content-Type' => { - 'required' => true, - 'schema' => { - 'type' => 'string', - 'const' => 'this should be ignored' - } - }, - 'Location' => { - 'required' => true, - 'schema' => { - 'type' => 'string', - format: 'uri-reference' - } - }, - 'X-Id' => { - 'schema' => { 'type' => 'integer' } - } + location_schema = { + 'type' => 'string', + format: 'uri-reference' } + x_id_schema = { 'type' => 'integer' } + [ + instance_double(OpenapiFirst::Header, + name: 'Location', + required?: true, + schema: JSONSchemer::Schema.new(location_schema), + resolved_schema: location_schema), + instance_double(OpenapiFirst::Header, + name: 'X-Id', + required?: false, + schema: JSONSchemer::Schema.new(x_id_schema), + resolved_schema: x_id_schema) + ] end let(:rack_response) do @@ -111,7 +108,6 @@ it 'returns the unpacked headers as defined in the API description' do expect(parsed.headers).to eq( - 'Content-Type' => 'application/json', 'X-Id' => 42 ) end diff --git a/spec/response_validator_spec.rb b/spec/response_validator_spec.rb index e87425f4..a38ebc21 100644 --- a/spec/response_validator_spec.rb +++ b/spec/response_validator_spec.rb @@ -1,25 +1,27 @@ # frozen_string_literal: true RSpec.describe OpenapiFirst::ResponseValidator do + let(:content_schema) do + JSONSchemer.schema({ + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'required' => %w[id name], + 'properties' => { + 'id' => { 'type' => 'integer' }, + 'name' => { 'type' => 'string' }, + 'tag' => { 'type' => 'string' } + } + } + }) + end + + let(:headers) do + nil + end + subject(:validator) do - response_definition = instance_double(OpenapiFirst::Response, - status: '200', - content_type: 'application/json', - content_schema: JSONSchemer.schema({ - 'type' => 'array', - 'items' => { - 'type' => 'object', - 'required' => %w[id name], - 'properties' => { - 'id' => { 'type' => 'integer' }, - 'name' => { 'type' => 'string' }, - 'tag' => { 'type' => 'string' } - } - } - }), - headers: {}, - headers_schema: nil) - described_class.new(response_definition) + described_class.new(content_schema:, headers:) end context 'with a valid response' do @@ -29,7 +31,7 @@ { 'id' => 42, 'name' => 'hans' }, { 'id' => 2, 'name' => 'Voldemort' } ], - headers: {} + headers: [] ) end @@ -54,13 +56,32 @@ end context 'with an invalid response' do + context 'with an invalid header' do + let(:content_schema) do + nil + end + let(:headers) do + [ + instance_double(OpenapiFirst::Header, + name: 'x-id', + schema: JSONSchemer.schema({ type: 'integer' }), + required?: false) + ] + end + + it 'fails' do + parsed_response = double(headers: { 'x-id' => 'abc' }) + expect(subject.call(parsed_response).type).to eq(:invalid_response_header) + end + end + context 'with missing property' do let(:parsed_response) do double( body: [ { 'id' => 42 } ], - headers: {} + headers: nil ) end @@ -75,7 +96,7 @@ body: [ { 'id' => 'string', 'name' => 'hans' } ], - headers: {} + headers: nil ) end diff --git a/spec/schema/hash_spec.rb b/spec/schema/hash_spec.rb new file mode 100644 index 00000000..de9e3091 --- /dev/null +++ b/spec/schema/hash_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.describe OpenapiFirst::Schema::Hash do + it 'validates all schemas' do + a_schema = JSONSchemer.schema({ 'type' => 'integer' }) + b_schema = JSONSchemer.schema({ 'type' => 'string' }) + + schema_hash = described_class.new({ 'a' => a_schema, 'b' => b_schema }) + errors = schema_hash.validate('a' => 'a', 'b' => 1).errors + + expect(errors.size).to eq(2) + expect(errors[0]).to have_attributes( + message: 'value at `/a` is not an integer', + data_pointer: '/a', + schema_pointer: '', + schema: { 'type' => 'integer' }, + type: 'integer', + details: nil + ) + expect(errors[1]).to have_attributes( + message: 'value at `/b` is not a string', + data_pointer: '/b', + schema_pointer: '', + schema: { 'type' => 'string' }, + type: 'string', + details: nil + ) + end +end