From 9b0ed0276db470f9d2d7ec843cc2a5126d3192d5 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Tue, 9 Aug 2022 15:05:19 +0900 Subject: [PATCH 01/11] Update components if referenced in request body or response --- lib/rspec/openapi/components_updater.rb | 55 +++++++++++++++++++++++++ lib/rspec/openapi/hash_helper.rb | 23 +++++++++++ lib/rspec/openapi/hooks.rb | 2 + lib/rspec/openapi/schema_cleaner.rb | 28 ++----------- spec/rails/doc/smart/expected.yaml | 15 ++++--- spec/rails/doc/smart/openapi.yaml | 20 +++++---- 6 files changed, 106 insertions(+), 37 deletions(-) create mode 100644 lib/rspec/openapi/components_updater.rb create mode 100644 lib/rspec/openapi/hash_helper.rb diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb new file mode 100644 index 00000000..724307fb --- /dev/null +++ b/lib/rspec/openapi/components_updater.rb @@ -0,0 +1,55 @@ +require_relative 'hash_helper' + +class << RSpec::OpenAPI::ComponentsUpdater = Object.new + # @param [Hash] base + # @param [Hash] fresh + def update!(base, fresh) + return if (base_schemas = base.dig('components', 'schemas')).nil? + + # Top-level schema: Used as the body of request or response + top_level_refs = paths_to_top_level_refs(base) + fresh_schemas = build_fresh_schemas(top_level_refs, base, fresh) + + # Clear out all properties before merge. + # The properties using $ref are preserved since those are hand-crafted by user. + fresh_schemas.keys.each do |schema_name| + clear_properties_except_refs(base_schemas[schema_name]) + end + RSpec::OpenAPI::SchemaMerger.merge!(base_schemas, fresh_schemas) + end + + private + + def clear_properties_except_refs(obj) + obj.each do |field_name, field_values| + if field_name == 'properties' + obj[field_name] = field_values.select do |key, value| + value['$ref'] || value['type'] == 'object' + end + elsif field_values.is_a?(Hash) + clear_properties_except_refs(field_values) + end + end + end + + def build_fresh_schemas(references, base, fresh) + references.inject({}) do |acc, paths| + ref_link = dig_schema(base, paths).dig('$ref') + schema_name = ref_link.gsub('#/components/schemas/', '') + schema_body = dig_schema(fresh, paths) + RSpec::OpenAPI::SchemaMerger.merge!(acc, { schema_name => schema_body }) + end + end + + def dig_schema(obj, paths) + obj.dig(*paths, 'schema', 'items') || obj.dig(*paths, 'schema') + end + + def paths_to_top_level_refs(base) + request_bodies = RSpec::OpenAPI::HashHelper::matched_paths(base, 'paths.*.*.requestBody.content.application/json') + responses = RSpec::OpenAPI::HashHelper::matched_paths(base, 'paths.*.*.responses.*.content.application/json') + (request_bodies + responses).select do |paths| + dig_schema(base, paths)&.dig('$ref')&.start_with?('#/components/schemas/') + end + end +end diff --git a/lib/rspec/openapi/hash_helper.rb b/lib/rspec/openapi/hash_helper.rb new file mode 100644 index 00000000..fb9e915c --- /dev/null +++ b/lib/rspec/openapi/hash_helper.rb @@ -0,0 +1,23 @@ +class << RSpec::OpenAPI::HashHelper = Object.new + def paths_to_all_fields(obj) + case obj + when Hash + obj.each.flat_map do |k, v| + k = k.to_s + [[k]] + paths_to_all_fields(v).map { |x| [k, *x] } + end + else + [] + end + end + + def matched_paths(obj, selector) + selector_parts = selector.split('.').map(&:to_s) + selectors = 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) + end + end + selectors + end +end diff --git a/lib/rspec/openapi/hooks.rb b/lib/rspec/openapi/hooks.rb index b5c727db..6c8762ba 100644 --- a/lib/rspec/openapi/hooks.rb +++ b/lib/rspec/openapi/hooks.rb @@ -1,4 +1,5 @@ require 'rspec' +require 'rspec/openapi/components_updater' require 'rspec/openapi/default_schema' require 'rspec/openapi/record_builder' require 'rspec/openapi/schema_builder' @@ -35,6 +36,7 @@ end end RSpec::OpenAPI::SchemaCleaner.cleanup!(spec, new_from_zero) + RSpec::OpenAPI::ComponentsUpdater.update!(spec, new_from_zero) end end if error_records.any? diff --git a/lib/rspec/openapi/schema_cleaner.rb b/lib/rspec/openapi/schema_cleaner.rb index 2b0f6bd7..8a41dd17 100644 --- a/lib/rspec/openapi/schema_cleaner.rb +++ b/lib/rspec/openapi/schema_cleaner.rb @@ -1,6 +1,8 @@ # For Ruby 3.0+ require 'set' +require_relative 'hash_helper' + class << RSpec::OpenAPI::SchemaCleaner = Object.new # Cleanup specific elements that exists in the base but not in the spec # @@ -28,34 +30,12 @@ def cleanup!(base, spec) private - def paths_to_all_fields(obj) - case obj - when Hash - obj.each.flat_map do |k,v| - k = k.to_s - [[k]] + paths_to_all_fields(v).map { |x| [k, *x] } - end - else - [] - end - end - - def matched_paths(obj, selector) - selector_parts = selector.split('.').map(&:to_s) - selectors = 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) - end - end - selectors - end - def cleanup_array!(base, spec, selector, fields_for_identity = []) marshal = lambda do |obj| Marshal.dump(slice(obj, fields_for_identity)) end - matched_paths(base, selector).each do |paths| + RSpec::OpenAPI::HashHelper::matched_paths(base, selector).each do |paths| target_array = base.dig(*paths) spec_array = spec.dig(*paths) unless target_array.is_a?(Array) && spec_array.is_a?(Array) @@ -72,7 +52,7 @@ def cleanup_array!(base, spec, selector, fields_for_identity = []) end def cleanup_hash!(base, spec, selector) - matched_paths(base, selector).each do |paths| + RSpec::OpenAPI::HashHelper::matched_paths(base, selector).each do |paths| exist_in_base = !base.dig(*paths).nil? not_in_spec = spec.dig(*paths).nil? if exist_in_base && not_in_spec diff --git a/spec/rails/doc/smart/expected.yaml b/spec/rails/doc/smart/expected.yaml index 53cc93ab..f982808e 100644 --- a/spec/rails/doc/smart/expected.yaml +++ b/spec/rails/doc/smart/expected.yaml @@ -136,12 +136,7 @@ components: description: type: string database: - type: object - properties: - id: - type: integer - name: - type: string + "$ref": "#/components/schemas/Database" null_sample: nullable: true storage_size: @@ -151,3 +146,11 @@ components: type: string updated_at: type: string + # Not updated automatically + Database: + type: object + properties: + id: + type: integer + name: + type: string diff --git a/spec/rails/doc/smart/openapi.yaml b/spec/rails/doc/smart/openapi.yaml index de6d7c19..c15c8968 100644 --- a/spec/rails/doc/smart/openapi.yaml +++ b/spec/rails/doc/smart/openapi.yaml @@ -205,17 +205,15 @@ components: properties: id: type: integer - name: + # This filed should exists in expected + # name: + # type: string + this_field_should_not_exist_in_expected: type: string description: type: string database: - type: object - properties: - id: - type: integer - name: - type: string + "$ref": "#/components/schemas/Database" null_sample: nullable: true storage_size: @@ -225,3 +223,11 @@ components: type: string updated_at: type: string + # Not updated automatically + Database: + type: object + properties: + id: + type: integer + name: + type: string From 8551e3e8a55164b28d7cfa1eeabe0b8e6062969a Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 12 Aug 2022 13:13:50 +0900 Subject: [PATCH 02/11] Support references in component schemas --- lib/rspec/openapi/components_updater.rb | 39 +++++++++++++++++++++++++ spec/rails/doc/smart/expected.yaml | 3 +- spec/rails/doc/smart/openapi.yaml | 10 +++++-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index 724307fb..d5260d56 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -10,6 +10,36 @@ def update!(base, fresh) top_level_refs = paths_to_top_level_refs(base) fresh_schemas = build_fresh_schemas(top_level_refs, base, fresh) + # Nested schema: References in Top-level schemas. May contain some top-level schema. + nested_refs = RSpec::OpenAPI::HashHelper::matched_paths(base, 'components.schemas.*.properties.*.$ref') + + # We assume that super-deeply nested references are not common. + # Loop counter may exhaust if some of referenced are not generated due to removal. No need to raise error. + 5.times.each do + generated_schema_names = fresh_schemas.keys + nested_refs = filter_non_generated_refs(nested_refs, base, generated_schema_names) + + # Complete if all the referenced schemas are generated. + break if nested_refs.empty? + + nested_refs.each do |paths| + parent_name = paths[-4] + + # Skip if parent schema is not generated yet. It may be generated on next iteration. + next if fresh_schemas.dig(parent_name).nil? + + property_name = paths[-2] + sub_schema = fresh_schemas.dig(parent_name, 'properties', property_name) + + # Skip if the property using $ref is not found in the parent schema. The property may be removed. + next if sub_schema.nil? + + schema_name = base.dig(*paths)&.gsub('#/components/schemas/', '') + fresh_schemas[schema_name] ||= {} + RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], sub_schema) + end + end + # Clear out all properties before merge. # The properties using $ref are preserved since those are hand-crafted by user. fresh_schemas.keys.each do |schema_name| @@ -52,4 +82,13 @@ def paths_to_top_level_refs(base) dig_schema(base, paths)&.dig('$ref')&.start_with?('#/components/schemas/') end end + + def filter_non_generated_refs(nested_refs, base, generated_names) + # Reject already-generated schemas to reduce unnecessary loop + nested_refs.reject do |paths| + ref_link = base.dig(*paths) + schema_name = ref_link.gsub('#/components/schemas/', '') + generated_names.include?(schema_name) + end + end end diff --git a/spec/rails/doc/smart/expected.yaml b/spec/rails/doc/smart/expected.yaml index f982808e..f760ac5d 100644 --- a/spec/rails/doc/smart/expected.yaml +++ b/spec/rails/doc/smart/expected.yaml @@ -146,7 +146,6 @@ components: type: string updated_at: type: string - # Not updated automatically Database: type: object properties: @@ -154,3 +153,5 @@ components: type: integer name: type: string + this_reference_should_exist_in_expected_even_if_no_such_field_in_actual: + "$ref": "#/components/schemas/Fire" diff --git a/spec/rails/doc/smart/openapi.yaml b/spec/rails/doc/smart/openapi.yaml index c15c8968..c730eae0 100644 --- a/spec/rails/doc/smart/openapi.yaml +++ b/spec/rails/doc/smart/openapi.yaml @@ -205,7 +205,7 @@ components: properties: id: type: integer - # This filed should exists in expected + # This field should exists in expected # name: # type: string this_field_should_not_exist_in_expected: @@ -223,11 +223,15 @@ components: type: string updated_at: type: string - # Not updated automatically Database: type: object properties: id: type: integer - name: + # This filed should exists in expected + # name: + # type: string + this_field_should_not_exist_in_expected: type: string + this_reference_should_exist_in_expected_even_if_no_such_field_in_actual: + "$ref": "#/components/schemas/Fire" From 26c9b466e049e40fac0821f8eaaab2d4d3fd75dd Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 12 Aug 2022 17:01:39 +0900 Subject: [PATCH 03/11] Use SchemaCleaner so hand-crafted properties can be preserved --- lib/rspec/openapi/components_updater.rb | 18 +----------------- lib/rspec/openapi/schema_cleaner.rb | 8 ++++++++ spec/rails/doc/smart/expected.yaml | 4 ++-- spec/rails/doc/smart/openapi.yaml | 4 +++- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index d5260d56..317a70cd 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -40,28 +40,12 @@ def update!(base, fresh) end end - # Clear out all properties before merge. - # The properties using $ref are preserved since those are hand-crafted by user. - fresh_schemas.keys.each do |schema_name| - clear_properties_except_refs(base_schemas[schema_name]) - end RSpec::OpenAPI::SchemaMerger.merge!(base_schemas, fresh_schemas) + RSpec::OpenAPI::SchemaCleaner.cleanup_components_schemas!(base, { 'components' => { 'schemas' => fresh_schemas } }) end private - def clear_properties_except_refs(obj) - obj.each do |field_name, field_values| - if field_name == 'properties' - obj[field_name] = field_values.select do |key, value| - value['$ref'] || value['type'] == 'object' - end - elsif field_values.is_a?(Hash) - clear_properties_except_refs(field_values) - end - end - end - def build_fresh_schemas(references, base, fresh) references.inject({}) do |acc, paths| ref_link = dig_schema(base, paths).dig('$ref') diff --git a/lib/rspec/openapi/schema_cleaner.rb b/lib/rspec/openapi/schema_cleaner.rb index 8a41dd17..9801b9b4 100644 --- a/lib/rspec/openapi/schema_cleaner.rb +++ b/lib/rspec/openapi/schema_cleaner.rb @@ -4,6 +4,14 @@ require_relative 'hash_helper' class << RSpec::OpenAPI::SchemaCleaner = Object.new + # Cleanup the properties, of component schemas, that exists in the base but not in the spec. + # + # @param [Hash] base + # @param [Hash] spec + def cleanup_components_schemas!(base, spec) + cleanup_hash!(base, spec, 'components.schemas.*.properties.*') + end + # Cleanup specific elements that exists in the base but not in the spec # # @param [Hash] base diff --git a/spec/rails/doc/smart/expected.yaml b/spec/rails/doc/smart/expected.yaml index f760ac5d..dce47efd 100644 --- a/spec/rails/doc/smart/expected.yaml +++ b/spec/rails/doc/smart/expected.yaml @@ -148,10 +148,10 @@ components: type: string Database: type: object + description: 'this should be preserved' properties: id: type: integer + description: 'this should be preserved' name: type: string - this_reference_should_exist_in_expected_even_if_no_such_field_in_actual: - "$ref": "#/components/schemas/Fire" diff --git a/spec/rails/doc/smart/openapi.yaml b/spec/rails/doc/smart/openapi.yaml index c730eae0..d8b20000 100644 --- a/spec/rails/doc/smart/openapi.yaml +++ b/spec/rails/doc/smart/openapi.yaml @@ -225,13 +225,15 @@ components: type: string Database: type: object + description: 'this should be preserved' properties: id: type: integer + description: 'this should be preserved' # This filed should exists in expected # name: # type: string this_field_should_not_exist_in_expected: type: string - this_reference_should_exist_in_expected_even_if_no_such_field_in_actual: + this_reference_should_not_exist_in_expected: "$ref": "#/components/schemas/Fire" From 4773d61561405711240a2e87088eb046decc2713 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sat, 13 Aug 2022 09:26:30 +0900 Subject: [PATCH 04/11] Clarify intention of the loop --- lib/rspec/openapi/components_updater.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index 317a70cd..10da2de0 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -13,8 +13,10 @@ def update!(base, fresh) # Nested schema: References in Top-level schemas. May contain some top-level schema. nested_refs = RSpec::OpenAPI::HashHelper::matched_paths(base, 'components.schemas.*.properties.*.$ref') + # Loop over all the referenced schemas until all schemas are regenerated or loop counter exhausts. + # Repeating loop is needed because a schema may refer another schema which is not generated yet. + # Loop counter exhaust if some schemas can not generated due to removal. No need to raise error on the case. # We assume that super-deeply nested references are not common. - # Loop counter may exhaust if some of referenced are not generated due to removal. No need to raise error. 5.times.each do generated_schema_names = fresh_schemas.keys nested_refs = filter_non_generated_refs(nested_refs, base, generated_schema_names) From c70bbb6f854f40b012c558674404eab7e905ba8d Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sat, 13 Aug 2022 10:11:46 +0900 Subject: [PATCH 05/11] Remove unused schema --- lib/rspec/openapi/schema_cleaner.rb | 1 + spec/rails/doc/smart/openapi.yaml | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/lib/rspec/openapi/schema_cleaner.rb b/lib/rspec/openapi/schema_cleaner.rb index 9801b9b4..f2ed3169 100644 --- a/lib/rspec/openapi/schema_cleaner.rb +++ b/lib/rspec/openapi/schema_cleaner.rb @@ -9,6 +9,7 @@ class << RSpec::OpenAPI::SchemaCleaner = Object.new # @param [Hash] base # @param [Hash] spec def cleanup_components_schemas!(base, spec) + cleanup_hash!(base, spec, 'components.schemas.*') cleanup_hash!(base, spec, 'components.schemas.*.properties.*') end diff --git a/spec/rails/doc/smart/openapi.yaml b/spec/rails/doc/smart/openapi.yaml index d8b20000..e8baf886 100644 --- a/spec/rails/doc/smart/openapi.yaml +++ b/spec/rails/doc/smart/openapi.yaml @@ -237,3 +237,9 @@ components: type: string this_reference_should_not_exist_in_expected: "$ref": "#/components/schemas/Fire" + NoSuchSchema: + type: object + description: 'should be deleted since not in use' + properties: + id: + type: integer From 2fc9d25d101e85b92e8f392a22c62619fea6bc11 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sat, 13 Aug 2022 10:12:58 +0900 Subject: [PATCH 06/11] Better naming --- spec/rails/doc/smart/openapi.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/rails/doc/smart/openapi.yaml b/spec/rails/doc/smart/openapi.yaml index e8baf886..c608b5da 100644 --- a/spec/rails/doc/smart/openapi.yaml +++ b/spec/rails/doc/smart/openapi.yaml @@ -235,11 +235,11 @@ components: # type: string this_field_should_not_exist_in_expected: type: string - this_reference_should_not_exist_in_expected: - "$ref": "#/components/schemas/Fire" - NoSuchSchema: + this_field_should_not_exist_in_expected_2: + "$ref": "#/components/schemas/NoSuchSchemaFound" + SchemaNotInUse: type: object - description: 'should be deleted since not in use' + description: 'should be deleted' properties: id: type: integer From 0f0f049b021cb0b7f6f43e98c5c19d62197ebf74 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sun, 14 Aug 2022 07:13:18 +0900 Subject: [PATCH 07/11] Added in README --- README.md | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/README.md b/README.md index 34be59f8..11a6f9f6 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,115 @@ RSpec::OpenAPI.description_builder = -> (example) { example.description } RSpec::OpenAPI.example_types = %i[request] ``` +### Can I use rspec-openapi with `$ref` to minimize duplication of schema? + +Yes, rspec-openapi v0.7.0+ supports [`$ref` mechanism](https://swagger.io/docs/specification/using-ref/) and generates +schemas under `#/components/schemas` with some manual steps. + +1. First, generate plain OpenAPI file. +2. Then, manually replace the duplications with `$ref` and define the placeholder of schema under +`#/components/schemas/` section. Example: + +```yaml +paths: + "/users": + get: + responses: + '200': + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/User" + "/users/{id}": + get: + responses: + '200': + content: + application/json: + schema: + $ref: "#/components/schemas/User" +components: + schemas: + User: + type: object +``` + +3. Then, re-run rspec-openapi. It tries to find the the referenced schema (`User` for example) and update `properties`. + +```yaml +paths: + "/users": + get: + responses: + '200': + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/User" + "/users/{id}": + get: + responses: + '200': + content: + application/json: + schema: + $ref: "#/components/schemas/User" +components: + schemas: + User: + type: object + properties: + id: + type: string + name: + type: string + role: + type: array + items: + type: string +``` + +rspec-openapi also supports `$ref` in `properties` of schemas. Example) + +```yaml +paths: + "/locations": + get: + responses: + '200': + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/Location" +components: + schemas: + Location: + type: object + properties: + id: + type: string + name: + type: string + Coordinate: + "$ref": "#/components/schemas/Coordinate" + Coordinate: + type: object + properties: + lat: + type: string + lon: + type: string +``` + +Note that automatic `schemas` update feature is still new and may not work in complex scenario. +If you find a room for improvement, open an issue. + ### How can I add information which can't be generated from RSpec? rspec-openapi tries to keep manual modifications as much as possible when generating specs. From e1ce874ba043e5c63399a992f65c900ce9850a75 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sun, 14 Aug 2022 07:24:19 +0900 Subject: [PATCH 08/11] Update README --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 11a6f9f6..b59b1a92 100644 --- a/README.md +++ b/README.md @@ -192,8 +192,7 @@ paths: $ref: "#/components/schemas/User" components: schemas: - User: - type: object + # No need to define User manually ``` 3. Then, re-run rspec-openapi. It tries to find the the referenced schema (`User` for example) and update `properties`. From 51abd2e233b382328844428db37d13033c7f5071 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sun, 14 Aug 2022 07:33:59 +0900 Subject: [PATCH 09/11] Unify "next" --- lib/rspec/openapi/components_updater.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index 10da2de0..f58669e4 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -26,19 +26,17 @@ def update!(base, fresh) nested_refs.each do |paths| parent_name = paths[-4] - - # Skip if parent schema is not generated yet. It may be generated on next iteration. - next if fresh_schemas.dig(parent_name).nil? - property_name = paths[-2] - sub_schema = fresh_schemas.dig(parent_name, 'properties', property_name) + nested_schema = fresh_schemas.dig(parent_name, 'properties', property_name) - # Skip if the property using $ref is not found in the parent schema. The property may be removed. - next if sub_schema.nil? + # A nested schema can not be generated + # - if parent schema is not generated yet. It may be generated on next iteration. + # - 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/', '') fresh_schemas[schema_name] ||= {} - RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], sub_schema) + RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], nested_schema) end end From a0280c9382ac1b48270859eb3f1666c96789418f Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Sun, 14 Aug 2022 07:52:35 +0900 Subject: [PATCH 10/11] Non-top-level schema can be generated now so no need to loop many times --- lib/rspec/openapi/components_updater.rb | 40 +++++++++---------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index f58669e4..0d0b08c3 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -11,33 +11,19 @@ def update!(base, fresh) fresh_schemas = build_fresh_schemas(top_level_refs, base, fresh) # Nested schema: References in Top-level schemas. May contain some top-level schema. - nested_refs = RSpec::OpenAPI::HashHelper::matched_paths(base, 'components.schemas.*.properties.*.$ref') - - # Loop over all the referenced schemas until all schemas are regenerated or loop counter exhausts. - # Repeating loop is needed because a schema may refer another schema which is not generated yet. - # Loop counter exhaust if some schemas can not generated due to removal. No need to raise error on the case. - # We assume that super-deeply nested references are not common. - 5.times.each do - generated_schema_names = fresh_schemas.keys - nested_refs = filter_non_generated_refs(nested_refs, base, generated_schema_names) - - # Complete if all the referenced schemas are generated. - break if nested_refs.empty? + generated_schema_names = fresh_schemas.keys + nested_refs = find_non_top_level_nested_refs(base, generated_schema_names) + nested_refs.each do |paths| + parent_name = paths[-4] + property_name = paths[-2] + nested_schema = fresh_schemas.dig(parent_name, 'properties', property_name) - nested_refs.each do |paths| - parent_name = paths[-4] - property_name = paths[-2] - nested_schema = fresh_schemas.dig(parent_name, 'properties', property_name) + # Skip if the property using $ref is not found in the parent schema. The property may be removed. + next if nested_schema.nil? - # A nested schema can not be generated - # - if parent schema is not generated yet. It may be generated on next iteration. - # - 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/', '') - fresh_schemas[schema_name] ||= {} - RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], nested_schema) - end + schema_name = base.dig(*paths)&.gsub('#/components/schemas/', '') + fresh_schemas[schema_name] ||= {} + RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], nested_schema) end RSpec::OpenAPI::SchemaMerger.merge!(base_schemas, fresh_schemas) @@ -67,7 +53,9 @@ def paths_to_top_level_refs(base) end end - def filter_non_generated_refs(nested_refs, base, generated_names) + def find_non_top_level_nested_refs(base, generated_names) + nested_refs = RSpec::OpenAPI::HashHelper::matched_paths(base, 'components.schemas.*.properties.*.$ref') + # Reject already-generated schemas to reduce unnecessary loop nested_refs.reject do |paths| ref_link = base.dig(*paths) From 07ca178a3f81338f032237beb839c70388cb5ac6 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Wed, 24 Aug 2022 10:01:54 +0900 Subject: [PATCH 11/11] #/components/schemas/ can be omitted to ease usage --- README.md | 9 +++------ lib/rspec/openapi/components_updater.rb | 5 ++--- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index b59b1a92..0b6e17e1 100644 --- a/README.md +++ b/README.md @@ -167,8 +167,7 @@ Yes, rspec-openapi v0.7.0+ supports [`$ref` mechanism](https://swagger.io/docs/s schemas under `#/components/schemas` with some manual steps. 1. First, generate plain OpenAPI file. -2. Then, manually replace the duplications with `$ref` and define the placeholder of schema under -`#/components/schemas/` section. Example: +2. Then, manually replace the duplications with `$ref`. ```yaml paths: @@ -190,12 +189,10 @@ paths: application/json: schema: $ref: "#/components/schemas/User" -components: - schemas: - # No need to define User manually +# Note) #/components/schamas is not needed to be defined. ``` -3. Then, re-run rspec-openapi. It tries to find the the referenced schema (`User` for example) and update `properties`. +3. Then, re-run rspec-openapi. It will generate `#/components/schemas` with the referenced schema (`User` for example) newly-generated or updated. ```yaml paths: diff --git a/lib/rspec/openapi/components_updater.rb b/lib/rspec/openapi/components_updater.rb index 0d0b08c3..8839d6b1 100644 --- a/lib/rspec/openapi/components_updater.rb +++ b/lib/rspec/openapi/components_updater.rb @@ -4,10 +4,9 @@ class << RSpec::OpenAPI::ComponentsUpdater = Object.new # @param [Hash] base # @param [Hash] fresh def update!(base, fresh) - return if (base_schemas = base.dig('components', 'schemas')).nil? - # Top-level schema: Used as the body of request or response top_level_refs = paths_to_top_level_refs(base) + return if top_level_refs.empty? fresh_schemas = build_fresh_schemas(top_level_refs, base, fresh) # Nested schema: References in Top-level schemas. May contain some top-level schema. @@ -26,7 +25,7 @@ def update!(base, fresh) RSpec::OpenAPI::SchemaMerger.merge!(fresh_schemas[schema_name], nested_schema) end - RSpec::OpenAPI::SchemaMerger.merge!(base_schemas, fresh_schemas) + RSpec::OpenAPI::SchemaMerger.merge!(base, { 'components' => { 'schemas' => fresh_schemas }}) RSpec::OpenAPI::SchemaCleaner.cleanup_components_schemas!(base, { 'components' => { 'schemas' => fresh_schemas } }) end