diff --git a/CHANGELOG.md b/CHANGELOG.md index 32777b61..795bf49e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +## 4.0.1 + +- Fix: Reduced memory usage with large API descriptions. Schemas no longer each deep-copy (stringify keys) the whole OAD to build a root schema. This is a json_schemer specific optimization. + ## 4.0.0 This release has no stricter or less strict request validation. It changes mostly internal stuff and adds a Sinatra integration. It's a major version, but it should be safe to upgrade. diff --git a/Gemfile.lock b/Gemfile.lock index 8b67a9c3..dc657fb4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - openapi_first (4.0.0) + openapi_first (4.0.1) drb (~> 2.0) hana (~> 1.3) json_schemer (>= 2.1, < 3.0) @@ -12,29 +12,29 @@ GEM specs: action_text-trix (2.1.19) railties - actioncable (8.1.3.1) - actionpack (= 8.1.3.1) - activesupport (= 8.1.3.1) + actioncable (8.1.4) + actionpack (= 8.1.4) + activesupport (= 8.1.4) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.1.3.1) - actionpack (= 8.1.3.1) - activejob (= 8.1.3.1) - activerecord (= 8.1.3.1) - activestorage (= 8.1.3.1) - activesupport (= 8.1.3.1) + actionmailbox (8.1.4) + actionpack (= 8.1.4) + activejob (= 8.1.4) + activerecord (= 8.1.4) + activestorage (= 8.1.4) + activesupport (= 8.1.4) mail (>= 2.8.0) - actionmailer (8.1.3.1) - actionpack (= 8.1.3.1) - actionview (= 8.1.3.1) - activejob (= 8.1.3.1) - activesupport (= 8.1.3.1) + actionmailer (8.1.4) + actionpack (= 8.1.4) + actionview (= 8.1.4) + activejob (= 8.1.4) + activesupport (= 8.1.4) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.1.3.1) - actionview (= 8.1.3.1) - activesupport (= 8.1.3.1) + actionpack (8.1.4) + actionview (= 8.1.4) + activesupport (= 8.1.4) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -42,36 +42,36 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.1.3.1) + actiontext (8.1.4) action_text-trix (~> 2.1.15) - actionpack (= 8.1.3.1) - activerecord (= 8.1.3.1) - activestorage (= 8.1.3.1) - activesupport (= 8.1.3.1) + actionpack (= 8.1.4) + activerecord (= 8.1.4) + activestorage (= 8.1.4) + activesupport (= 8.1.4) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.1.3.1) - activesupport (= 8.1.3.1) + actionview (8.1.4) + activesupport (= 8.1.4) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (8.1.3.1) - activesupport (= 8.1.3.1) + activejob (8.1.4) + activesupport (= 8.1.4) globalid (>= 0.3.6) - activemodel (8.1.3.1) - activesupport (= 8.1.3.1) - activerecord (8.1.3.1) - activemodel (= 8.1.3.1) - activesupport (= 8.1.3.1) + activemodel (8.1.4) + activesupport (= 8.1.4) + activerecord (8.1.4) + activemodel (= 8.1.4) + activesupport (= 8.1.4) timeout (>= 0.4.0) - activestorage (8.1.3.1) - actionpack (= 8.1.3.1) - activejob (= 8.1.3.1) - activerecord (= 8.1.3.1) - activesupport (= 8.1.3.1) + activestorage (8.1.4) + actionpack (= 8.1.4) + activejob (= 8.1.4) + activerecord (= 8.1.4) + activesupport (= 8.1.4) marcel (~> 1.0) - activesupport (8.1.3.1) + activesupport (8.1.4) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -87,7 +87,7 @@ GEM ast (2.4.3) base64 (0.3.0) benchmark (0.5.0) - bigdecimal (4.1.2) + bigdecimal (4.1.3) builder (3.3.0) concurrent-ruby (1.3.8) connection_pool (3.0.2) @@ -102,7 +102,7 @@ GEM hana (1.3.7) i18n (1.15.2) concurrent-ruby (~> 1.0) - io-console (0.9.2) + io-console (0.9.4) irb (1.18.0) pp (>= 0.6.0) prism (>= 1.3.0) @@ -137,7 +137,7 @@ GEM net-protocol net-pop (0.1.2) net-protocol - net-protocol (0.3.0) + net-protocol (0.4.0) timeout net-smtp (0.5.1) net-protocol @@ -167,20 +167,20 @@ GEM rack (>= 1.3) rackup (2.3.1) rack (>= 3) - rails (8.1.3.1) - actioncable (= 8.1.3.1) - actionmailbox (= 8.1.3.1) - actionmailer (= 8.1.3.1) - actionpack (= 8.1.3.1) - actiontext (= 8.1.3.1) - actionview (= 8.1.3.1) - activejob (= 8.1.3.1) - activemodel (= 8.1.3.1) - activerecord (= 8.1.3.1) - activestorage (= 8.1.3.1) - activesupport (= 8.1.3.1) + rails (8.1.4) + actioncable (= 8.1.4) + actionmailbox (= 8.1.4) + actionmailer (= 8.1.4) + actionpack (= 8.1.4) + actiontext (= 8.1.4) + actionview (= 8.1.4) + activejob (= 8.1.4) + activemodel (= 8.1.4) + activerecord (= 8.1.4) + activestorage (= 8.1.4) + activesupport (= 8.1.4) bundler (>= 1.15.0) - railties (= 8.1.3.1) + railties (= 8.1.4) rails-dom-testing (2.3.0) activesupport (>= 5.0.0) minitest @@ -188,9 +188,9 @@ GEM rails-html-sanitizer (1.7.1) loofah (~> 2.25, >= 2.25.2) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.1.3.1) - actionpack (= 8.1.3.1) - activesupport (= 8.1.3.1) + railties (8.1.4) + actionpack (= 8.1.4) + activesupport (= 8.1.4) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -208,7 +208,7 @@ GEM prism (>= 1.6.0) rbs (>= 4.0.0) tsort - regexp_parser (2.12.0) + regexp_parser (2.13.0) reline (0.7.0) io-console (~> 0.5) rspec (3.13.2) @@ -246,7 +246,7 @@ GEM rubydex (0.4.1-arm64-darwin) rubydex (0.4.1-x86_64-linux) securerandom (0.4.1) - simplecov (1.2.0) + simplecov (1.3.1) simpleidn (0.3.0) sinatra (4.2.1) logger (>= 1.6.0) @@ -261,9 +261,9 @@ GEM tsort (0.2.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) - unicode-display_width (3.2.0) - unicode-emoji (~> 4.1) - unicode-emoji (4.2.0) + unicode-display_width (3.3.0) + unicode-emoji (~> 4.3) + unicode-emoji (4.3.0) uri (1.1.1) useragent (0.16.11) websocket-driver (0.8.2) diff --git a/benchmarks/Gemfile.lock b/benchmarks/Gemfile.lock index 7fa3b07e..703dd6e1 100644 --- a/benchmarks/Gemfile.lock +++ b/benchmarks/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - openapi_first (4.0.0) + openapi_first (4.0.1) drb (~> 2.0) hana (~> 1.3) json_schemer (>= 2.1, < 3.0) @@ -15,8 +15,8 @@ GEM benchmark-ips (2.15.1) benchmark-memory (0.2.0) memory_profiler (~> 1) - bigdecimal (4.1.2) - committee (5.6.3) + bigdecimal (4.1.3) + committee (5.6.4) json_schema (~> 0.14, >= 0.14.3) openapi_parser (~> 2.0) rack (>= 1.5) @@ -48,7 +48,7 @@ GEM rack-session (2.1.2) base64 (>= 0.1.0) rack (>= 3.0.0) - regexp_parser (2.12.0) + regexp_parser (2.13.0) simpleidn (0.3.0) sinatra (4.2.1) logger (>= 1.6.0) diff --git a/lib/openapi_first.rb b/lib/openapi_first.rb index 69f9834f..4f76218b 100644 --- a/lib/openapi_first.rb +++ b/lib/openapi_first.rb @@ -4,6 +4,7 @@ require_relative 'openapi_first/file_loader' require_relative 'openapi_first/errors' require_relative 'openapi_first/registry' +require_relative 'openapi_first/utils' require_relative 'openapi_first/configuration' require_relative 'openapi_first/child_configuration' require_relative 'openapi_first/definition' @@ -82,7 +83,7 @@ def self.load(filepath_or_definition, only: nil, path_prefix: nil, &) # @return [Definition] # TODO: This needs to work with unresolved contents as well def self.parse(contents, only: nil, filepath: nil, path_prefix: nil, &) - contents = ::JSON.parse(::JSON.generate(contents)) # Deeply stringify keys, because of YAML. See https://github.com/ahx/openapi_first/issues/367 + contents = Utils.deep_stringify_keys(contents) # Deeply stringify keys, because of YAML. See https://github.com/ahx/openapi_first/issues/367 contents['paths'].filter!(&->(key, _) { only.call(key) }) if only Definition.new(contents, filepath, path_prefix, &) end diff --git a/lib/openapi_first/file_loader.rb b/lib/openapi_first/file_loader.rb index aafd67c6..2b45973e 100644 --- a/lib/openapi_first/file_loader.rb +++ b/lib/openapi_first/file_loader.rb @@ -26,12 +26,20 @@ def load(file_path) if extname == '.json' ::JSON.parse(body) elsif ['.yaml', '.yml'].include?(extname) - YAML.unsafe_load(body) + load_yaml(body, file_path) else body end end end end + + private + + def load_yaml(body, file_path) + Utils.deep_stringify_keys(YAML.unsafe_load(body)) + rescue ::JSON::GeneratorError => e + raise Error, "Could not load #{file_path.inspect}: #{e.message}" + end end end diff --git a/lib/openapi_first/ref_resolver.rb b/lib/openapi_first/ref_resolver.rb index f2f93902..34f2b268 100644 --- a/lib/openapi_first/ref_resolver.rb +++ b/lib/openapi_first/ref_resolver.rb @@ -184,6 +184,17 @@ class Schema vocabulary: { 'https://json-schema.org/draft/2020-12/vocab/core' => true } ) + # This class is here to monkey-patch JSONSchemer::Schema when we are passing + # an instance to sub-schemas. It skips stringifying all keys of the passed object to + # allocate fewer objects. We don't have to stringify the keys, because they are already stringified upstream. + # @visibility private + class DocumentRootSchema < JSONSchemer::Schema + private + + def deep_stringify_keys(value) = value + end + private_constant :DocumentRootSchema + def initialize(value:, context:, base_uri:, options:) @value = value @context = context @@ -197,7 +208,7 @@ def initialize(value:, context:, base_uri:, options:) def schema @schema ||= begin - root_schema = JSONSchemer::Schema.new(context, base_uri:, **options, meta_schema: DOCUMENT_META_SCHEMA) + root_schema = DocumentRootSchema.new(context, base_uri:, **options, meta_schema: DOCUMENT_META_SCHEMA) apply_dialect(root_schema) JSONSchemer::Schema.new(value, nil, root_schema, base_uri:, **options) end diff --git a/lib/openapi_first/utils.rb b/lib/openapi_first/utils.rb new file mode 100644 index 00000000..fa71ec83 --- /dev/null +++ b/lib/openapi_first/utils.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module OpenapiFirst + # @visibility private + module Utils + module_function + + def deep_stringify_keys(contents) = ::JSON.parse(::JSON.generate(contents)) + end +end diff --git a/lib/openapi_first/version.rb b/lib/openapi_first/version.rb index f812d346..48fd27e9 100644 --- a/lib/openapi_first/version.rb +++ b/lib/openapi_first/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module OpenapiFirst - VERSION = '4.0.0' + VERSION = '4.0.1' end diff --git a/spec/file_loader_spec.rb b/spec/file_loader_spec.rb index 88e74085..4ec85e2a 100644 --- a/spec/file_loader_spec.rb +++ b/spec/file_loader_spec.rb @@ -26,6 +26,25 @@ expect(contents['openapi']).to eq('3.0.0') end + it 'loads YAML keys as strings' do + Tempfile.create(['codes', '.yaml']) do |file| + file.write("200:\n description: ok\n") + file.flush + + expect(described_class.load(file.path)).to eq({ '200' => { 'description' => 'ok' } }) + end + end + + it 'names the file when YAML contains a value that cannot be represented' do + Tempfile.create(['limits', '.yaml']) do |file| + file.write("maximum: .inf\n") + file.flush + + expect { described_class.load(file.path) } + .to raise_error(OpenapiFirst::Error, /\ACould not load "#{Regexp.escape(file.path)}": Infinity/) + end + end + it 'loads .json' do contents = described_class.load('./spec/data/petstore.json') expect(contents['openapi']).to eq('3.0.0') diff --git a/spec/hooks_spec.rb b/spec/hooks_spec.rb index 2c37fa42..3f881c95 100644 --- a/spec/hooks_spec.rb +++ b/spec/hooks_spec.rb @@ -379,4 +379,49 @@ def build_request(path, method: 'GET', body: nil) ]) end end + + describe 'body property hooks with a schema referenced by request and response' do + let(:spec) do + { + 'openapi' => '3.1.0', + 'paths' => { + '/pets' => { + 'post' => { + 'requestBody' => { + 'content' => { 'application/json' => { 'schema' => { '$ref' => '#/components/schemas/Pet' } } } + }, + 'responses' => { + '200' => { + 'description' => 'ok', + 'content' => { 'application/json' => { 'schema' => { '$ref' => '#/components/schemas/Pet' } } } + } + } + } + } + }, + 'components' => { + 'schemas' => { + 'Pet' => { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } } } + } + } + } + end + + it 'calls only the hook of the validated body' do + request_calls = [] + response_calls = [] + definition = OpenapiFirst.parse(spec) do |config| + config.after_request_body_property_validation { |data, property| request_calls << [data, property] } + config.after_response_body_property_validation { |data, property| response_calls << [data, property] } + end + request = build_request('/pets', method: 'POST', body: '{"name": "Quentin"}') + response = Rack::Response.new('{"name": "Rex"}', 200, { 'Content-Type' => 'application/json' }) + + definition.validate_request(request) + definition.validate_response(request, response) + + expect(request_calls).to eq([[{ 'name' => 'Quentin' }, 'name']]) + expect(response_calls).to eq([[{ 'name' => 'Rex' }, 'name']]) + end + end end diff --git a/spec/openapi_first_spec.rb b/spec/openapi_first_spec.rb index cdd66a82..60e361c5 100644 --- a/spec/openapi_first_spec.rb +++ b/spec/openapi_first_spec.rb @@ -40,6 +40,44 @@ definition = OpenapiFirst.parse(YAML.safe_load_file('./spec/data/petstore.yaml')) expect(definition.paths).to include('/pets') end + + it 'resolves $refs through keys that are not strings' do + pet = { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } } } + definition = OpenapiFirst.parse({ + 'openapi' => '3.1.0', + 'paths' => { + '/pets' => { + 'get' => { + 'responses' => { + 200 => { 'description' => 'ok', + 'content' => { 'application/json' => { 'schema' => pet } } } + } + }, + 'post' => { + 'requestBody' => { + 'content' => { + 'application/json' => { + 'schema' => { + 'type' => 'object', + 'properties' => { + 'pet' => { + '$ref' => '#/paths/~1pets/get/responses/200/content/application~1json/schema' + } + } + } + } + } + }, + 'responses' => { 201 => { 'description' => 'created' } } + } + } + } + }) + request = Rack::Request.new(Rack::MockRequest.env_for('/pets', method: 'POST', input: '{"pet": {"name": 1}}', + 'CONTENT_TYPE' => 'application/json')) + + expect(definition.validate_request(request, raise_error: false).error.type).to eq(:invalid_body) + end end describe '.configure' do diff --git a/spec/ref_resolver_spec.rb b/spec/ref_resolver_spec.rb index f691e1ad..baad433d 100644 --- a/spec/ref_resolver_spec.rb +++ b/spec/ref_resolver_spec.rb @@ -107,6 +107,34 @@ expect(schema.valid?({ data: [{ has_bicycle: true }] })).to eq(true) expect(schema.valid?({ data: [{ has_bicycle: 'red' }] })).to eq(false) end + + it 'resolves refs the same way regardless of which schemas were validated before' do + contents = { 'components' => { 'schemas' => { + 'Named' => { '$defs' => { 'name' => { '$anchor' => 'name', 'type' => 'string' } } }, + 'Pet' => { 'properties' => { 'name' => { '$ref' => '#name' } } } + } } } + validate_pet = lambda do |after:| + doc = described_class.new(file_loader: OpenapiFirst::FileLoader.new).for(contents) + schema = ->(name) { doc.dig('components', 'schemas', name).schema(configuration: JSONSchemer.configuration) } + after.each { schema.call(_1).valid?('Rex') } + schema.call('Pet').valid?({ 'name' => 'Rex' }) + rescue JSONSchemer::UnknownRef => e + e.class + end + + expect(validate_pet.call(after: ['Named'])).to eq(validate_pet.call(after: [])) + end + + it 'resolves refs through keys that YAML loads as integers' do + Tempfile.create(['codes', '.yaml']) do |file| + file.write("properties:\n n:\n $ref: '#/codes/200'\ncodes:\n 200:\n type: integer\n") + file.flush + schema = resolver.load(file.path).schema(configuration: JSONSchemer.configuration) + + expect(schema.valid?({ 'n' => 1 })).to eq(true) + expect(schema.valid?({ 'n' => 'x' })).to eq(false) + end + end end describe '#[]' do