From c941c62b4a3e90076d0c4b317a8393519bed557e Mon Sep 17 00:00:00 2001 From: exoego Date: Thu, 28 Mar 2024 14:44:39 +0900 Subject: [PATCH 1/3] Add failing test --- spec/rspec/scheme_merger_spec.rb | 55 ++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 spec/rspec/scheme_merger_spec.rb diff --git a/spec/rspec/scheme_merger_spec.rb b/spec/rspec/scheme_merger_spec.rb new file mode 100644 index 0000000..aaaac4c --- /dev/null +++ b/spec/rspec/scheme_merger_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'schema merger spec' do + include SpecHelper + + describe 'mixed symbol and strings' do + let(:base) do + { + 'n' => 1, + 'required' => %w[foo bar], + 'a' => { + b1: 1, + b2: %w[foo bar], + 'b3' => { + 'c1' => 2, + c2: 3, + }, + }, + } + end + + let(:spec) do + { + n: 1, + required: ['buz'], + a: { + 'b1' => 1, + 'b2' => %w[foo bar], + b3: { + c1: 2, + 'c2' => 3, + }, + }, + } + end + + it 'normalize keys to symbol' do + result = RSpec::OpenAPI::SchemaMerger.merge!(base, spec) + expect(result).to eq({ + n: 1, + required: [], + a: { + b1: 1, + b2: %w[foo bar], + b3: { + c1: 2, + c2: 3, + }, + }, + }) + end + end +end From 1100d1f2761b4ffc43deeaa0335536fcd66824c4 Mon Sep 17 00:00:00 2001 From: exoego Date: Thu, 28 Mar 2024 14:51:20 +0900 Subject: [PATCH 2/3] Process JSON tree in Symbol until dump --- lib/rspec/openapi/components_updater.rb | 27 ++++++++++++++----------- lib/rspec/openapi/hash_helper.rb | 10 +++++---- lib/rspec/openapi/record_builder.rb | 2 +- lib/rspec/openapi/schema_cleaner.rb | 20 +++++++++--------- lib/rspec/openapi/schema_file.rb | 26 ++++++++++++++++++++++-- lib/rspec/openapi/schema_merger.rb | 23 +++++++++++---------- lib/rspec/openapi/schema_sorter.rb | 2 +- 7 files changed, 69 insertions(+), 41 deletions(-) diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index 9cac029..bca7311 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -23,28 +23,29 @@ def update!(base, fresh) # 0 1 2 ^...............................^ # ["components", "schema", "Table", "properties", "owner", "properties", "company", "$ref"] # 0 1 2 ^...........................................^ - needle = paths.reject { |path| path.is_a?(Integer) || path == 'oneOf' } + needle = paths.reject { |path| path.is_a?(Integer) || path == :oneOf } needle = needle.slice(2, needle.size - 3) nested_schema = fresh_schemas.dig(*needle) # Skip if the property using $ref is not found in the parent schema. The property may be removed. next if nested_schema.nil? - schema_name = base.dig(*paths)&.gsub('#/components/schemas/', '') + schema_name = base.dig(*paths)&.gsub('#/components/schemas/', '')&.to_sym fresh_schemas[schema_name] ||= {} RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], nested_schema) end - RSpec::OpenAPI::SchemaMerger.merge!(base, { 'components' => { 'schemas' => fresh_schemas } }) - RSpec::OpenAPI::SchemaCleaner.cleanup_components_schemas!(base, { 'components' => { 'schemas' => fresh_schemas } }) + RSpec::OpenAPI::SchemaMerger.merge!(base, { components: { schemas: fresh_schemas } }) + RSpec::OpenAPI::SchemaCleaner.cleanup_components_schemas!(base, { components: { schemas: fresh_schemas } }) end private def build_fresh_schemas(references, base, fresh) references.inject({}) do |acc, paths| - ref_link = dig_schema(base, paths)['$ref'] - schema_name = ref_link.gsub('#/components/schemas/', '') + ref_link = dig_schema(base, paths)[:$ref] + puts "ref_link: #{ref_link}" + schema_name = ref_link.to_s.gsub('#/components/schemas/', '') schema_body = dig_schema(fresh, paths.reject { |path| path.is_a?(Integer) }) RSpec::OpenAPI::SchemaMerger.merge!(acc, { schema_name => schema_body }) @@ -52,9 +53,11 @@ def build_fresh_schemas(references, base, fresh) end def dig_schema(obj, paths) - item_schema = obj.dig(*paths, 'schema', 'items') - object_schema = obj.dig(*paths, 'schema') - one_of_schema = obj.dig(*paths.take(paths.size - 1), 'schema', 'oneOf', paths.last) + # Response code can be an integer + paths = paths.map { |path| path.is_a?(Integer) ? path : path.to_sym } + item_schema = obj.dig(*paths, :schema, :items) + object_schema = obj.dig(*paths, :schema) + one_of_schema = obj.dig(*paths.take(paths.size - 1), :schema, :oneOf, paths.last) item_schema || object_schema || one_of_schema end @@ -85,12 +88,12 @@ def find_non_top_level_nested_refs(base, generated_names) end def find_one_of_refs(base, paths) - dig_schema(base, paths)&.dig('oneOf')&.map&.with_index do |schema, index| - paths + [index] if schema&.dig('$ref')&.start_with?('#/components/schemas/') + dig_schema(base, paths)&.dig(:oneOf)&.map&.with_index do |schema, index| + paths + [index] if schema&.dig(:$ref)&.start_with?('#/components/schemas/') end&.compact end def find_object_refs(base, paths) - [paths] if dig_schema(base, paths)&.dig('$ref')&.start_with?('#/components/schemas/') + [paths] if dig_schema(base, paths)&.dig(:$ref)&.start_with?('#/components/schemas/') end end diff --git a/lib/rspec/openapi/hash_helper.rb b/lib/rspec/openapi/hash_helper.rb index 390bbec..54e60a5 100644 --- a/lib/rspec/openapi/hash_helper.rb +++ b/lib/rspec/openapi/hash_helper.rb @@ -5,7 +5,7 @@ def paths_to_all_fields(obj) case obj when Hash obj.each.flat_map do |k, v| - k = k.to_s + k = k.to_sym [[k]] + paths_to_all_fields(v).map { |x| [k, *x] } end when Array @@ -18,10 +18,10 @@ def paths_to_all_fields(obj) end def matched_paths(obj, selector) - selector_parts = selector.split('.').map(&:to_s) + selector_parts = selector.split('.').map(&:to_sym) paths_to_all_fields(obj).select do |key_parts| key_parts.size == selector_parts.size && key_parts.zip(selector_parts).all? do |kp, sp| - kp == sp || (sp == '*' && !kp.nil?) + kp == sp || (sp == :* && !kp.nil?) end end end @@ -29,7 +29,9 @@ def matched_paths(obj, selector) def matched_paths_deeply_nested(obj, begin_selector, end_selector) path_depth_sizes = paths_to_all_fields(obj).map(&:size).uniq path_depth_sizes.map do |depth| - diff = depth - begin_selector.count('.') - end_selector.count('.') + begin_selector_count = begin_selector.is_a?(Symbol) ? 0 : begin_selector.count('.') + end_selector_count = end_selector.is_a?(Symbol) ? 0 : end_selector.count('.') + diff = depth - begin_selector_count - end_selector_count if diff >= 0 selector = "#{begin_selector}.#{'*.' * diff}#{end_selector}" matched_paths(obj, selector) diff --git a/lib/rspec/openapi/record_builder.rb b/lib/rspec/openapi/record_builder.rb index 80cf2e4..7523fae 100644 --- a/lib/rspec/openapi/record_builder.rb +++ b/lib/rspec/openapi/record_builder.rb @@ -53,7 +53,7 @@ def safe_parse_body(response, media_type) def extract_headers(request, response) request_headers = RSpec::OpenAPI.request_headers.each_with_object([]) do |header, headers_arr| - header_key = header.gsub('-', '_').upcase + header_key = header.gsub('-', '_').upcase.to_sym header_value = request.get_header(['HTTP', header_key].join('_')) || request.get_header(header_key) headers_arr << [header, header_value] if header_value end diff --git a/lib/rspec/openapi/schema_cleaner.rb b/lib/rspec/openapi/schema_cleaner.rb index a95d016..87021f5 100644 --- a/lib/rspec/openapi/schema_cleaner.rb +++ b/lib/rspec/openapi/schema_cleaner.rb @@ -27,7 +27,7 @@ def cleanup!(base, spec) cleanup_hash!(base, spec, 'paths.*.*') # cleanup parameters - cleanup_array!(base, spec, 'paths.*.*.parameters', %w[name in]) + cleanup_array!(base, spec, 'paths.*.*.parameters', %i[name in]) # cleanup requestBody cleanup_hash!(base, spec, 'paths.*.*.requestBody.content.application/json.schema.properties.*') @@ -40,7 +40,7 @@ def cleanup!(base, spec) end def cleanup_conflicting_security_parameters!(base) - security_schemes = base.dig('components', 'securitySchemes') || {} + security_schemes = base.dig(:components, :securitySchemes) || {} return if security_schemes.empty? @@ -65,22 +65,22 @@ def cleanup_empty_required_array!(base) paths_to_objects.each do |path| parent = base.dig(*path.take(path.length - 1)) # "required" array must not be present if empty - parent.delete('required') if parent['required'] && parent['required'].empty? + parent.delete(:required) if parent[:required] && parent[:required].empty? end end private def remove_parameters_conflicting_with_security_sceheme!(path_definition, security_scheme, security_scheme_name) - return unless path_definition['security'] - return unless path_definition['parameters'] - return unless path_definition.dig('security', 0).keys.include?(security_scheme_name) + return unless path_definition[:security] + return unless path_definition[:parameters] + return unless path_definition.dig(:security, 0).keys.include?(security_scheme_name) - path_definition['parameters'].reject! do |parameter| - parameter['in'] == security_scheme['in'] && # same location (ie. header) - parameter['name'] == security_scheme['name'] # same name (ie. AUTHORIZATION) + path_definition[:parameters].reject! do |parameter| + parameter[:in] == security_scheme[:in] && # same location (ie. header) + parameter[:name] == security_scheme[:name] # same name (ie. AUTHORIZATION) end - path_definition.delete('parameters') if path_definition['parameters'].empty? + path_definition.delete(:parameters) if path_definition[:parameters].empty? end def cleanup_array!(base, spec, selector, fields_for_identity = []) diff --git a/lib/rspec/openapi/schema_file.rb b/lib/rspec/openapi/schema_file.rb index c432202..bea640f 100644 --- a/lib/rspec/openapi/schema_file.rb +++ b/lib/rspec/openapi/schema_file.rb @@ -15,7 +15,7 @@ def edit(&block) spec = read block.call(spec) ensure - write(spec) + write(stringify(spec)) end private @@ -24,7 +24,29 @@ def edit(&block) def read return {} unless File.exist?(@path) - YAML.safe_load(File.read(@path)) # this can also parse JSON + symbolize(YAML.safe_load(File.read(@path))) # this can also parse JSON + end + + def symbolize(value) + case value + when Hash + value.to_h { |k, v| [k.to_sym, symbolize(v)] } + when Array + value.map { |v| symbolize(v) } + else + value + end + end + + def stringify(value) + case value + when Hash + value.to_h { |k, v| [k.to_s, stringify(v)] } + when Array + value.map { |v| stringify(v) } + else + value + end end # @param [Hash] spec diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index ee83564..00f92a8 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -5,6 +5,7 @@ class << RSpec::OpenAPI::SchemaMerger = Object.new # @param [Hash] spec def merge!(base, spec) spec = normalize_keys(spec) + base.replace(normalize_keys(base)) merge_schema!(base, spec) end @@ -14,7 +15,7 @@ def normalize_keys(spec) case spec when Hash spec.to_h do |key, value| - [key.to_s, normalize_keys(value)] + [key.to_sym, normalize_keys(value)] end when Array spec.map { |s| normalize_keys(s) } @@ -29,7 +30,7 @@ def normalize_keys(spec) # # TODO: Should we probably force-merge `summary` regardless of manual modifications? def merge_schema!(base, spec) - if (options = base['oneOf']) + if (options = base[:oneOf]) merge_closest_match!(options, spec) return base @@ -37,13 +38,13 @@ def merge_schema!(base, spec) spec.each do |key, value| if base[key].is_a?(Hash) && value.is_a?(Hash) - merge_schema!(base[key], value) unless base[key].key?('$ref') + merge_schema!(base[key], value) unless base[key].key?(:$ref) elsif base[key].is_a?(Array) && value.is_a?(Array) # parameters need to be merged as if `name` and `in` were the Hash keys. merge_arrays(base, key, value) else # do not ADD `properties` or `required` fields if `additionalProperties` field is present - base[key] = value unless base.key?('additionalProperties') && %w[properties required].include?(key) + base[key] = value unless base.key?(:additionalProperties) && %i[properties required].include?(key) end end base @@ -51,9 +52,9 @@ def merge_schema!(base, spec) def merge_arrays(base, key, value) base[key] = case key - when 'parameters' + when :parameters merge_parameters(base, key, value) - when 'required' + when :required # Preserve properties that appears in all test cases value & base[key] else @@ -65,13 +66,13 @@ def merge_arrays(base, key, value) def merge_parameters(base, key, value) all_parameters = value | base[key] - unique_base_parameters = base[key].index_by { |parameter| [parameter['name'], parameter['in']] } + unique_base_parameters = base[key].index_by { |parameter| [parameter[:name], parameter[:in]] } all_parameters = all_parameters.map do |parameter| - base_parameter = unique_base_parameters[[parameter['name'], parameter['in']]] || {} + base_parameter = unique_base_parameters[[parameter[:name], parameter[:in]]] || {} base_parameter ? base_parameter.merge(parameter) : parameter end - all_parameters.uniq! { |param| param.slice('name', 'in') } + all_parameters.uniq! { |param| param.slice(:name, :in) } base[key] = all_parameters end @@ -80,7 +81,7 @@ def merge_parameters(base, key, value) def merge_closest_match!(options, spec) score, option = options.map { |option| [similarity(option, spec), option] }.max_by(&:first) - return if option&.key?('$ref') + return if option&.key?(:$ref) if score.to_f > SIMILARITY_THRESHOLD merge_schema!(option, spec) @@ -97,7 +98,7 @@ def similarity(first, second) when [Array, Array] (first & second).size / [first.size, second.size].max.to_f when [Hash, Hash] - return 1 if first.merge(second).key?('$ref') + return 1 if first.merge(second).key?(:$ref) intersection = first.keys & second.keys total_size = [first.size, second.size].max.to_f diff --git a/lib/rspec/openapi/schema_sorter.rb b/lib/rspec/openapi/schema_sorter.rb index 9428235..ddfbb5c 100644 --- a/lib/rspec/openapi/schema_sorter.rb +++ b/lib/rspec/openapi/schema_sorter.rb @@ -29,7 +29,7 @@ def deep_sort_by_selector!(base, selector) end def deep_sort_hash!(hash) - sorted = hash.entries.sort_by { |k, _| k }.to_h + sorted = hash.entries.sort_by { |k, _| k.to_s }.to_h.transform_keys(&:to_sym) hash.replace(sorted) end end From e5f1549b046b0fa65e95552e4c0d1a3e12ab67a7 Mon Sep 17 00:00:00 2001 From: exoego Date: Thu, 28 Mar 2024 14:55:16 +0900 Subject: [PATCH 3/3] Extract common function --- lib/rspec/openapi.rb | 1 + lib/rspec/openapi/key_transformer.rb | 25 +++++++++++++++++++++++++ lib/rspec/openapi/schema_file.rb | 26 ++------------------------ lib/rspec/openapi/schema_merger.rb | 17 ++--------------- 4 files changed, 30 insertions(+), 39 deletions(-) create mode 100644 lib/rspec/openapi/key_transformer.rb diff --git a/lib/rspec/openapi.rb b/lib/rspec/openapi.rb index 898b731..995093b 100644 --- a/lib/rspec/openapi.rb +++ b/lib/rspec/openapi.rb @@ -10,6 +10,7 @@ require 'rspec/openapi/schema_merger' require 'rspec/openapi/schema_cleaner' require 'rspec/openapi/schema_sorter' +require 'rspec/openapi/key_transformer' require 'rspec/openapi/minitest_hooks' if Object.const_defined?('Minitest') require 'rspec/openapi/rspec_hooks' if ENV['OPENAPI'] && Object.const_defined?('RSpec') diff --git a/lib/rspec/openapi/key_transformer.rb b/lib/rspec/openapi/key_transformer.rb new file mode 100644 index 0000000..dcf5924 --- /dev/null +++ b/lib/rspec/openapi/key_transformer.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class << RSpec::OpenAPI::KeyTransformer = Object.new + def symbolize(value) + case value + when Hash + value.to_h { |k, v| [k.to_sym, symbolize(v)] } + when Array + value.map { |v| symbolize(v) } + else + value + end + end + + def stringify(value) + case value + when Hash + value.to_h { |k, v| [k.to_s, stringify(v)] } + when Array + value.map { |v| stringify(v) } + else + value + end + end +end diff --git a/lib/rspec/openapi/schema_file.rb b/lib/rspec/openapi/schema_file.rb index bea640f..53f14f9 100644 --- a/lib/rspec/openapi/schema_file.rb +++ b/lib/rspec/openapi/schema_file.rb @@ -15,7 +15,7 @@ def edit(&block) spec = read block.call(spec) ensure - write(stringify(spec)) + write(RSpec::OpenAPI::KeyTransformer.stringify(spec)) end private @@ -24,29 +24,7 @@ def edit(&block) def read return {} unless File.exist?(@path) - symbolize(YAML.safe_load(File.read(@path))) # this can also parse JSON - end - - def symbolize(value) - case value - when Hash - value.to_h { |k, v| [k.to_sym, symbolize(v)] } - when Array - value.map { |v| symbolize(v) } - else - value - end - end - - def stringify(value) - case value - when Hash - value.to_h { |k, v| [k.to_s, stringify(v)] } - when Array - value.map { |v| stringify(v) } - else - value - end + RSpec::OpenAPI::KeyTransformer.symbolize(YAML.safe_load(File.read(@path))) # this can also parse JSON end # @param [Hash] spec diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index 00f92a8..1dcfaeb 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -4,26 +4,13 @@ class << RSpec::OpenAPI::SchemaMerger = Object.new # @param [Hash] base # @param [Hash] spec def merge!(base, spec) - spec = normalize_keys(spec) - base.replace(normalize_keys(base)) + spec = RSpec::OpenAPI::KeyTransformer.symbolize(spec) + base.replace(RSpec::OpenAPI::KeyTransformer.symbolize(base)) merge_schema!(base, spec) end private - def normalize_keys(spec) - case spec - when Hash - spec.to_h do |key, value| - [key.to_sym, normalize_keys(value)] - end - when Array - spec.map { |s| normalize_keys(s) } - else - spec - end - end - # Not doing `base.replace(deep_merge(base, spec))` to preserve key orders. # Also this needs to be aware of OpenAPI details because a Hash-like structure # may be an array whose Hash elements have a key name.