diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 675e4af2..495a94ff 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -8,7 +8,6 @@ jobs: os: - ubuntu-latest ruby: - - "3.2" - "3.3" - "3.4" - "4.0" diff --git a/.rubocop.yml b/.rubocop.yml index 4b5d93fa..e5eaabd9 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,7 +5,7 @@ inherit_mode: plugins: rubocop-performance AllCops: - TargetRubyVersion: 3.2.0 + TargetRubyVersion: 3.3.0 NewCops: enable SuggestExtensions: false Exclude: diff --git a/CHANGELOG.md b/CHANGELOG.md index f771df84..7df7bfb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,14 @@ ## Unreleased +## 3.3.1 + +- Optimized caching towards reducing retained memory after calling `OpenapiFirst.load` without using a global cache. (Removed `OpenapiFirst.clear_cache!`.) +- Require ruby >= 3.3.0 + ## 3.3.0 -- OpenapiFirst will now cache the contents of files that have been loaded. If you need to reload your OpenAPI definition for tests or server hot reloading, you can call `OpenapiFirst.clear_cache!`. +- ~OpenapiFirst will now cache the contents of files that have been loaded. If you need to reload your OpenAPI definition for tests or server hot reloading, you can call `OpenapiFirst.clear_cache!`.~ - Optimized `OpenapiFirst::Router#match` for faster path matching and reduced memory allocation. ## 3.2.1 diff --git a/Gemfile.lock b/Gemfile.lock index 9a9c01d5..3bf697b3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - openapi_first (3.3.0) + openapi_first (3.3.1) drb (~> 2.0) hana (~> 1.3) json_schemer (>= 2.1, < 3.0) @@ -151,7 +151,7 @@ GEM racc (~> 1.4) openapi_parameters (0.11.0) rack (>= 2.2) - parallel (1.28.0) + parallel (2.0.1) parser (3.3.11.1) ast (~> 2.4.1) racc @@ -227,11 +227,11 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.7) - rubocop (1.86.0) + rubocop (1.86.1) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) - parallel (~> 1.10) + parallel (>= 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) @@ -302,4 +302,4 @@ DEPENDENCIES sinatra BUNDLED WITH - 4.0.6 + 4.0.10 diff --git a/benchmarks/Gemfile.lock b/benchmarks/Gemfile.lock index e07af089..07f1e3b6 100644 --- a/benchmarks/Gemfile.lock +++ b/benchmarks/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - openapi_first (3.3.0) + openapi_first (3.3.1) drb (~> 2.0) hana (~> 1.3) json_schemer (>= 2.1, < 3.0) @@ -42,7 +42,7 @@ GEM optparse uri webrick - puma (7.2.0) + puma (8.0.0) nio4r (~> 2.0) rack (3.2.6) rack-protection (4.2.1) diff --git a/benchmarks/apps/large.ru b/benchmarks/apps/large.ru deleted file mode 100644 index fa41e490..00000000 --- a/benchmarks/apps/large.ru +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -require 'json' -require 'openapi_first' -require_relative 'app' - -OpenapiFirst.register File.absolute_path('../../spec/data/large.yaml', __dir__) - -use OpenapiFirst::Middlewares::RequestValidation -run App diff --git a/benchmarks/large.rb b/benchmarks/large.rb index 94a9263b..85660223 100644 --- a/benchmarks/large.rb +++ b/benchmarks/large.rb @@ -4,5 +4,9 @@ require 'openapi_first' Benchmark.memory do |x| - x.report { OpenapiFirst.load('../spec/data/large.yaml') } + x.report do + oad = OpenapiFirst.load('../spec/data/large.yaml') + request = Rack::Request.new(Rack::MockRequest.env_for('/workspaces')) + oad.validate_request(request) + end end diff --git a/lib/openapi_first.rb b/lib/openapi_first.rb index 2aab739c..b8922e29 100644 --- a/lib/openapi_first.rb +++ b/lib/openapi_first.rb @@ -22,11 +22,6 @@ module OpenapiFirst FAILURE = :openapi_first_validation_failure - # Clears cached files - def self.clear_cache! - FileLoader.clear_cache! - end - # @return [Configuration] def self.configuration @configuration ||= Configuration.new diff --git a/lib/openapi_first/builder.rb b/lib/openapi_first/builder.rb index 3e28f59e..952a6aa4 100644 --- a/lib/openapi_first/builder.rb +++ b/lib/openapi_first/builder.rb @@ -27,18 +27,20 @@ def initialize(contents, filepath:, config:) meta_schema = detect_meta_schema(contents, filepath) @schemer_configuration = build_schemer_config(filepath:, meta_schema:) @config = config - @contents = RefResolver.for(contents, filepath:) + @file_loader = FileLoader.new + ref_resolver = RefResolver.new(file_loader:) + @contents = ref_resolver.for(contents, filepath:) end attr_reader :config - private attr_reader :schemer_configuration + private attr_reader :schemer_configuration, :file_loader 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) + file_loader.load(uri.path) end result.meta_schema = meta_schema result.insert_property_defaults = true diff --git a/lib/openapi_first/file_loader.rb b/lib/openapi_first/file_loader.rb index 8dc4dcd1..aafd67c6 100644 --- a/lib/openapi_first/file_loader.rb +++ b/lib/openapi_first/file_loader.rb @@ -5,11 +5,15 @@ module OpenapiFirst # @!visibility private - module FileLoader - @cache = {} - @mutex = Mutex.new + class FileLoader + def self.load(file_path) + new.load(file_path) + end - module_function + def initialize + @cache = {} + @mutex = Mutex.new + end def load(file_path) @cache[file_path] || @mutex.synchronize do @@ -29,9 +33,5 @@ def load(file_path) end end end - - def clear_cache! - @mutex.synchronize { @cache.clear } - end end end diff --git a/lib/openapi_first/ref_resolver.rb b/lib/openapi_first/ref_resolver.rb index 3d35c469..65ebed7d 100644 --- a/lib/openapi_first/ref_resolver.rb +++ b/lib/openapi_first/ref_resolver.rb @@ -5,30 +5,42 @@ module OpenapiFirst # This is here to give traverse an OAD while keeping $refs intact # @visibility private - module RefResolver - def self.load(filepath) - contents = OpenapiFirst::FileLoader.load(filepath) + class RefResolver + def initialize(file_loader:) + @file_loader = file_loader + end + + private attr_reader :file_loader + + def load(filepath) + contents = file_loader.load(filepath) self.for(contents, filepath:) end - def self.for(value, filepath: nil, context: value) + def for(value, filepath: nil, context: value) case value when ::Hash - resolver = Hash.new(value, context:, filepath:) + resolver = Hash.new(value, context:, filepath:, ref_resolver: self) if value.key?('$ref') probe = resolver.resolve_ref(value['$ref']) return probe if probe.is_a?(Array) end resolver when ::Array - Array.new(value, context:, filepath:) + Array.new(value, context:, filepath:, ref_resolver: self) when ::NilClass nil else - Simple.new(value) + Simple.new(value, ref_resolver: self) end end + def file_at(filepath, file_pointer) + file_contents = file_loader.load(filepath) + value = Hana::Pointer.new(file_pointer).eval(file_contents) + self.for(value, filepath: filepath, context: file_contents) + end + # @visibility private module Diggable def dig(*keys) @@ -42,10 +54,11 @@ def dig(*keys) # @visibility private module Resolvable - def initialize(value, context: value, filepath: nil) + def initialize(value, ref_resolver:, context: value, filepath: nil) @value = value @context = context @filepath = filepath + @ref_resolver = ref_resolver @dir = if filepath File.dirname(File.absolute_path(filepath)) else @@ -62,6 +75,8 @@ def initialize(value, context: value, filepath: nil) attr_reader :filepath + private attr_reader :ref_resolver + def ==(_other) raise "Don't call == on an unresolved value. Use .value == other instead." end @@ -71,16 +86,14 @@ def resolve_ref(pointer) value = Hana::Pointer.new(pointer[1..]).eval(context) raise "Unknown reference #{pointer} in #{context}" unless value - return RefResolver.for(value, filepath:, context:) + return ref_resolver.for(value, filepath:, context:) end relative_path, file_pointer = pointer.split('#') full_path = File.expand_path(relative_path, dir) - return RefResolver.load(full_path) unless file_pointer + return ref_resolver.load(full_path) unless file_pointer - file_contents = FileLoader.load(full_path) - value = Hana::Pointer.new(file_pointer).eval(file_contents) - RefResolver.for(value, filepath: full_path, context: file_contents) + ref_resolver.file_at(full_path, file_pointer) rescue OpenapiFirst::FileNotFoundError => e message = "Problem with reference resolving #{pointer.inspect} in " \ "file #{File.absolute_path(filepath).inspect}: #{e.message}" @@ -114,13 +127,13 @@ def resolved def [](key) return resolve_ref(@value['$ref'])[key] if !@value.key?(key) && @value.key?('$ref') - RefResolver.for(@value[key], filepath:, context:) + ref_resolver.for(@value[key], filepath:, context:) end def fetch(key) return resolve_ref(@value['$ref']).fetch(key) if !@value.key?(key) && @value.key?('$ref') - RefResolver.for(@value.fetch(key), filepath:, context:) + ref_resolver.for(@value.fetch(key), filepath:, context:) end def each @@ -170,7 +183,7 @@ def [](index) item = @value[index] return resolve_ref(item['$ref']) if item.is_a?(::Hash) && item.key?('$ref') - RefResolver.for(item, filepath:, context:) + ref_resolver.for(item, filepath:, context:) end def each diff --git a/lib/openapi_first/version.rb b/lib/openapi_first/version.rb index 48e8f575..af891c87 100644 --- a/lib/openapi_first/version.rb +++ b/lib/openapi_first/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module OpenapiFirst - VERSION = '3.3.0' + VERSION = '3.3.1' end diff --git a/openapi_first.gemspec b/openapi_first.gemspec index 56acfa60..2394ba8c 100644 --- a/openapi_first.gemspec +++ b/openapi_first.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |spec| spec.files = Dir['{lib}/**/*.rb', 'LICENSE.txt', 'README.md', 'CHANGELOG.md'] spec.require_paths = ['lib'] - spec.required_ruby_version = '>= 3.2.0' + spec.required_ruby_version = '>= 3.3.0' spec.add_dependency 'drb', '~> 2.0' spec.add_dependency 'hana', '~> 1.3' diff --git a/spec/openapi_first_spec.rb b/spec/openapi_first_spec.rb index 222b0102..ff56eda8 100644 --- a/spec/openapi_first_spec.rb +++ b/spec/openapi_first_spec.rb @@ -52,15 +52,6 @@ end end - describe '.clear_cache!' do - it 'clears the file cache so files are reloaded on next load' do - first = OpenapiFirst.load(spec_path) - OpenapiFirst.clear_cache! - second = OpenapiFirst.load(spec_path) - expect(second).not_to be(first) - end - end - describe '.load' do begin require 'multi_json' diff --git a/spec/ref_resolver_spec.rb b/spec/ref_resolver_spec.rb index b9368786..771b071a 100644 --- a/spec/ref_resolver_spec.rb +++ b/spec/ref_resolver_spec.rb @@ -19,14 +19,16 @@ } end + let(:resolver) { described_class.new(file_loader: OpenapiFirst::FileLoader.new) } + subject(:doc) do - described_class.for(contents) + resolver.for(contents) end describe '#dir' do it 'returns the directory path' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' - doc = described_class.load(filepath) + doc = resolver.load(filepath) expect(doc.dir).to eq(File.expand_path('./spec/data/splitted-train-travel-api')) node = doc['paths']['/stations']['get'] @@ -40,28 +42,28 @@ describe '#context' do it 'returns contents of the main file' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' - doc = described_class.load(filepath) + doc = resolver.load(filepath) node = doc['info'] expect(node.context['openapi']).to eq('3.1.0') end it 'returns contents of the main file when following internal refs' do filepath = './spec/data/train-travel-api/openapi.yaml' - doc = described_class.load(filepath) + doc = resolver.load(filepath) node = doc['paths']['/stations']['get']['responses']['200']['headers']['RateLimit']['schema'] expect(node.context['openapi']).to eq('3.1.0') end it 'returns contents of the referenced file' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' - doc = described_class.load(filepath) + doc = resolver.load(filepath) node = doc['paths']['/stations']['get']['responses'] expect(node.context.dig('get', 'description')).to start_with('Returns a list of all train stations') end it 'returns contents of the referenced file when pointing inside referenced files' do filepath = './spec/data/petstore.yaml' - doc = described_class.load(filepath) + doc = resolver.load(filepath) node = doc['components']['schemas']['Pet']['required'] expect(node.context).to eq(YAML.load_file('./spec/data/components/schemas/pet.yaml')) end @@ -73,19 +75,19 @@ end it 'returns a schema' do - node = described_class.load('./spec/data/components/schemas/dog.yaml') + node = resolver.load('./spec/data/components/schemas/dog.yaml') schema = node.schema(ref_resolver:) expect(schema.valid?({ bark: 'woff' })).to eq(true) expect(schema.valid?({ bark: 2 })).to eq(false) end it 'accepts options' do - node = described_class.for({ 'properties' => { - 'color' => { - 'type' => 'string', - 'default' => 'black' - } - } }) + node = resolver.for({ 'properties' => { + 'color' => { + 'type' => 'string', + 'default' => 'black' + } + } }) data = {} schema = node.schema(insert_property_defaults: true) expect(schema.valid?(data)).to eq(true) @@ -93,14 +95,14 @@ end it 'uses the right context' do - node = described_class.load('./spec/data/petstore.yaml') + node = resolver.load('./spec/data/petstore.yaml') 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') + node = resolver.load('./spec/data/splitted-train-travel-api/openapi.yaml') 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) @@ -111,7 +113,7 @@ it 'works across files' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' contents = OpenapiFirst::FileLoader.load(filepath) - doc = described_class.for(contents, filepath:) + doc = resolver.for(contents, filepath:) node = doc['paths']['/stations']['get']['responses']['200']['headers']['RateLimit']['schema'] expect(node.context['description']).to start_with('The RateLimit header') expect(node.resolved['type']).to eq('string') @@ -120,7 +122,7 @@ it 'follows pointers through files' do filepath = './spec/data/petstore.yaml' contents = OpenapiFirst::FileLoader.load(filepath) - doc = described_class.for(contents, filepath:) + doc = resolver.for(contents, filepath:) target = doc['components']['schemas']['Pet'] expect(target.value).to eq({ '$ref' => './components/schemas/pet.yaml#/Pet' }) @@ -128,12 +130,12 @@ end it 'returns nil if key is not found' do - doc = described_class.for(contents) + doc = resolver.for(contents) expect(doc['definitions']['unknown']).to eq(nil) end it 'works with arrays' do - doc = described_class.for(contents)['array'] + doc = resolver.for(contents)['array'] expect(doc[0].resolved).to eq({ 'name' => 'A' }) expect(doc[1].resolved).to eq({ 'name' => 'B' }) # expect(doc.dig('array', 1, 'name').value).to eq('B') @@ -147,7 +149,7 @@ it 'works across files' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' contents = OpenapiFirst::FileLoader.load(filepath) - doc = described_class.for(contents, filepath:) + doc = resolver.for(contents, filepath:) node = doc.dig('paths', '/stations', 'get', 'responses', '200', 'headers', 'RateLimit', 'schema') expect(node.context['description']).to start_with('The RateLimit header') expect(node.resolved['type']).to eq('string') @@ -156,7 +158,7 @@ it 'follows pointers through files' do filepath = './spec/data/petstore.yaml' contents = OpenapiFirst::FileLoader.load(filepath) - doc = described_class.for(contents, filepath:) + doc = resolver.for(contents, filepath:) target = doc.dig('components', 'schemas', 'Pet') expect(target.value).to eq({ '$ref' => './components/schemas/pet.yaml#/Pet' }) @@ -164,7 +166,7 @@ end it 'works with arrays' do - doc = described_class.for(contents) + doc = resolver.for(contents) expect(doc.dig('array', 0, 'name').value).to eq('A') expect(doc.dig('array', 1, 'name').value).to eq('B') expect(doc.dig('array', 0, 'unknown')).to eq(nil) @@ -173,12 +175,12 @@ end it 'returns nil if key is not found' do - doc = described_class.for(contents) + doc = resolver.for(contents) expect(doc.dig('definitions', 'unknown')).to eq(nil) end it 'returns nil if multiple keys are not found is not found' do - doc = described_class.for(contents) + doc = resolver.for(contents) expect(doc.dig('definitions', 'unknown', 'funknown')).to eq(nil) end end @@ -187,13 +189,13 @@ it 'works across files' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' contents = OpenapiFirst::FileLoader.load(filepath) - doc = described_class.for(contents, filepath:) + doc = resolver.for(contents, filepath:) target = doc.fetch('paths').fetch('/stations')['get']['responses']['200']['headers']['RateLimit']['schema'] expect(target.resolved['type']).to eq('string') end it 'raises KeyError if key is not found' do - doc = described_class.for(contents) + doc = resolver.for(contents) expect do doc.fetch('unknown') end.to raise_error KeyError @@ -224,7 +226,7 @@ end it 'works with arrays' do - doc = described_class.for(contents) + doc = resolver.for(contents) expect(doc['array'].resolved).to eq([{ 'name' => 'A' }, { 'name' => 'B' }]) end end @@ -233,7 +235,7 @@ it 'works across files' do filepath = './spec/data/splitted-train-travel-api/openapi.yaml' contents = OpenapiFirst::FileLoader.load(filepath) - doc = described_class.for(contents, filepath:) + doc = resolver.for(contents, filepath:) items = [] doc.fetch('paths').each { |path, path_item| items << [path, path_item] } @@ -266,7 +268,7 @@ } } - doc = described_class.for(contents) + doc = resolver.for(contents) parameters = doc.fetch('paths').first[1]['parameters'] expect(parameters.resolved[0]['name']).to eq('page') end @@ -308,7 +310,7 @@ }, 'array' => { '$ref' => '#/definitions/array' } } - doc = described_class.for(contents) + doc = resolver.for(contents) results = [] doc['array'].each do |node|