From b29fd5356e5aee976b00dbfefd34d5c3f493aa5b Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 27 May 2022 10:00:31 +0900 Subject: [PATCH 1/7] Use a marker to sort newer example can be preserved --- lib/rspec/openapi/schema_builder.rb | 5 ++ lib/rspec/openapi/schema_merger.rb | 7 ++- spec/rails/doc/openapi.json | 62 ++++++++++++------------- spec/rails/doc/openapi.yaml | 32 ++++++------- spec/rails/doc/smart/expected.yaml | 26 +++++------ spec/requests/rails_smart_merge_spec.rb | 3 +- 6 files changed, 73 insertions(+), 62 deletions(-) diff --git a/lib/rspec/openapi/schema_builder.rb b/lib/rspec/openapi/schema_builder.rb index 435d4992..1511e3bc 100644 --- a/lib/rspec/openapi/schema_builder.rb +++ b/lib/rspec/openapi/schema_builder.rb @@ -48,6 +48,8 @@ def example_enabled? def build_parameters(record) parameters = [] + marker_to_keep_last_duplicate = Time.now.utc.to_s + record.path_params.each do |key, value| parameters << { name: build_parameter_name(key, value), @@ -55,6 +57,7 @@ def build_parameters(record) required: true, schema: build_property(try_cast(value)), example: (try_cast(value) if example_enabled?), + __marker: marker_to_keep_last_duplicate, }.compact end @@ -64,6 +67,7 @@ def build_parameters(record) in: 'query', schema: build_property(try_cast(value)), example: (try_cast(value) if example_enabled?), + __marker: marker_to_keep_last_duplicate, }.compact end @@ -74,6 +78,7 @@ def build_parameters(record) required: true, schema: build_property(try_cast(value)), example: (try_cast(value) if example_enabled?), + __marker: marker_to_keep_last_duplicate, }.compact end diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index b7b20d27..b908e15f 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -36,7 +36,12 @@ def merge_schema!(base, spec) # parameters need to be merged as if `name` and `in` were the Hash keys. if key == 'parameters' base[key] |= value - base[key].uniq! { |param| param.slice('name', 'in') } + # 9999 is a dummy for old spec. + dummy = '9999-99-99 99:99:99 UTC' + base[key] + .sort_by! { |param| param['__marker'] || dummy } + .uniq! { |param| param.slice('name', 'in') } + .each { |param| param.delete('__marker') } else base[key] = value end diff --git a/spec/rails/doc/openapi.json b/spec/rails/doc/openapi.json index a9847448..dc1285ea 100644 --- a/spec/rails/doc/openapi.json +++ b/spec/rails/doc/openapi.json @@ -106,29 +106,19 @@ }, "parameters": [ { - "name": "page", - "in": "query", - "schema": { - "type": "integer" - }, - "example": 1 - }, - { - "name": "per", + "name": "filter[price]", "in": "query", "schema": { - "type": "integer" - }, - "example": 10 - }, - { - "name": "X-Authorization-Token", - "in": "header", - "required": true, - "schema": { - "type": "string" + "type": "object", + "properties": { + "price": { + "type": "string" + } + } }, - "example": "token" + "example": { + "price": "0" + } }, { "name": "filter[name]", @@ -146,19 +136,29 @@ } }, { - "name": "filter[price]", + "name": "page", "in": "query", "schema": { - "type": "object", - "properties": { - "price": { - "type": "string" - } - } + "type": "integer" }, - "example": { - "price": "0" - } + "example": 1 + }, + { + "name": "per", + "in": "query", + "schema": { + "type": "integer" + }, + "example": 10 + }, + { + "name": "X-Authorization-Token", + "in": "header", + "required": true, + "schema": { + "type": "string" + }, + "example": "token" } ] }, @@ -268,7 +268,7 @@ "schema": { "type": "integer" }, - "example": 1 + "example": 2 } ], "responses": { diff --git a/spec/rails/doc/openapi.yaml b/spec/rails/doc/openapi.yaml index 642fee47..d984f334 100644 --- a/spec/rails/doc/openapi.yaml +++ b/spec/rails/doc/openapi.yaml @@ -20,16 +20,15 @@ paths: tags: - Table parameters: - - name: page - in: query - schema: - type: integer - example: 1 - - name: per + - name: filter[price] in: query schema: - type: integer - example: 10 + type: object + properties: + price: + type: string + example: + price: '0' - name: filter[name] in: query schema: @@ -39,15 +38,16 @@ paths: type: string example: name: Example Table - - name: filter[price] + - name: page in: query schema: - type: object - properties: - price: - type: string - example: - price: '0' + type: integer + example: 1 + - name: per + in: query + schema: + type: integer + example: 10 - name: X-Authorization-Token in: header required: true @@ -180,7 +180,7 @@ paths: required: true schema: type: integer - example: 1 + example: 2 responses: '200': description: returns a table diff --git a/spec/rails/doc/smart/expected.yaml b/spec/rails/doc/smart/expected.yaml index 5b68c1fa..ee033ea9 100644 --- a/spec/rails/doc/smart/expected.yaml +++ b/spec/rails/doc/smart/expected.yaml @@ -24,36 +24,36 @@ paths: in: query schema: type: integer - example: 1 + example: 42 - name: per in: query schema: type: integer example: 10 - - name: filter[name] + - name: X-Authorization-Token + in: header + required: true + schema: + type: string + example: token + - name: filter[price] in: query schema: type: object properties: - name: + price: type: string example: - name: Example Table - - name: filter[price] + price: '0' + - name: filter[name] in: query schema: type: object properties: - price: + name: type: string example: - price: '0' - - name: X-Authorization-Token - in: header - required: true - schema: - type: string - example: token + name: Example Table responses: '200': description: with flat query parameters diff --git a/spec/requests/rails_smart_merge_spec.rb b/spec/requests/rails_smart_merge_spec.rb index 96b8ba28..a59e8562 100644 --- a/spec/requests/rails_smart_merge_spec.rb +++ b/spec/requests/rails_smart_merge_spec.rb @@ -27,7 +27,8 @@ describe '#index' do context it 'returns a list of tables' do it 'with flat query parameters' do - get '/tables', params: { page: '1', per: '10' }, + # These new params replace them in old spec + get '/tables', params: { page: '42', per: '10' }, headers: { authorization: 'k0kubun', "X-Authorization-Token": 'token' } expect(response.status).to eq(200) end From cb01aeda7c18d27a517e353dd135ff04fb523fb2 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 27 May 2022 14:26:26 +0900 Subject: [PATCH 2/7] Round to align digits so comparison is stable --- lib/rspec/openapi/schema_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rspec/openapi/schema_builder.rb b/lib/rspec/openapi/schema_builder.rb index 1511e3bc..b5f80126 100644 --- a/lib/rspec/openapi/schema_builder.rb +++ b/lib/rspec/openapi/schema_builder.rb @@ -48,7 +48,7 @@ def example_enabled? def build_parameters(record) parameters = [] - marker_to_keep_last_duplicate = Time.now.utc.to_s + marker_to_keep_last_duplicate = Time.now.utc.round.to_s record.path_params.each do |key, value| parameters << { From 8545a81ad3b425d6313e5d84fbab03ac2f65b6c2 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 27 May 2022 22:12:00 +0900 Subject: [PATCH 3/7] Stabilize order of parameters --- lib/rspec/openapi/schema_merger.rb | 2 +- spec/rails/doc/openapi.json | 30 ++++++++++++++-------------- spec/rails/doc/openapi.yaml | 24 +++++++++++----------- spec/rails/doc/smart/expected.yaml | 32 +++++++++++++++--------------- 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index b908e15f..ea8292ae 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -39,7 +39,7 @@ def merge_schema!(base, spec) # 9999 is a dummy for old spec. dummy = '9999-99-99 99:99:99 UTC' base[key] - .sort_by! { |param| param['__marker'] || dummy } + .sort_by! { |param| [param['in'], param['name'], param['__marker'] || dummy].join('-') } .uniq! { |param| param.slice('name', 'in') } .each { |param| param.delete('__marker') } else diff --git a/spec/rails/doc/openapi.json b/spec/rails/doc/openapi.json index dc1285ea..c44cfa51 100644 --- a/spec/rails/doc/openapi.json +++ b/spec/rails/doc/openapi.json @@ -106,33 +106,42 @@ }, "parameters": [ { - "name": "filter[price]", + "name": "X-Authorization-Token", + "in": "header", + "required": true, + "schema": { + "type": "string" + }, + "example": "token" + }, + { + "name": "filter[name]", "in": "query", "schema": { "type": "object", "properties": { - "price": { + "name": { "type": "string" } } }, "example": { - "price": "0" + "name": "Example Table" } }, { - "name": "filter[name]", + "name": "filter[price]", "in": "query", "schema": { "type": "object", "properties": { - "name": { + "price": { "type": "string" } } }, "example": { - "name": "Example Table" + "price": "0" } }, { @@ -150,15 +159,6 @@ "type": "integer" }, "example": 10 - }, - { - "name": "X-Authorization-Token", - "in": "header", - "required": true, - "schema": { - "type": "string" - }, - "example": "token" } ] }, diff --git a/spec/rails/doc/openapi.yaml b/spec/rails/doc/openapi.yaml index d984f334..481edf78 100644 --- a/spec/rails/doc/openapi.yaml +++ b/spec/rails/doc/openapi.yaml @@ -20,24 +20,30 @@ paths: tags: - Table parameters: - - name: filter[price] + - name: X-Authorization-Token + in: header + required: true + schema: + type: string + example: token + - name: filter[name] in: query schema: type: object properties: - price: + name: type: string example: - price: '0' - - name: filter[name] + name: Example Table + - name: filter[price] in: query schema: type: object properties: - name: + price: type: string example: - name: Example Table + price: '0' - name: page in: query schema: @@ -48,12 +54,6 @@ paths: schema: type: integer example: 10 - - name: X-Authorization-Token - in: header - required: true - schema: - type: string - example: token responses: '200': description: with different deep query parameters diff --git a/spec/rails/doc/smart/expected.yaml b/spec/rails/doc/smart/expected.yaml index ee033ea9..b69cdce0 100644 --- a/spec/rails/doc/smart/expected.yaml +++ b/spec/rails/doc/smart/expected.yaml @@ -20,40 +20,40 @@ paths: tags: - Table parameters: - - name: page - in: query - schema: - type: integer - example: 42 - - name: per - in: query - schema: - type: integer - example: 10 - name: X-Authorization-Token in: header required: true schema: type: string example: token - - name: filter[price] + - name: filter[name] in: query schema: type: object properties: - price: + name: type: string example: - price: '0' - - name: filter[name] + name: Example Table + - name: filter[price] in: query schema: type: object properties: - name: + price: type: string example: - name: Example Table + price: '0' + - name: page + in: query + schema: + type: integer + example: 42 + - name: per + in: query + schema: + type: integer + example: 10 responses: '200': description: with flat query parameters From 6326392058708f25889524086fb4d82f11f32d16 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 8 Jul 2022 17:14:12 +0900 Subject: [PATCH 4/7] Merger should be responsible with this --- lib/rspec/openapi/schema_builder.rb | 5 ----- lib/rspec/openapi/schema_merger.rb | 5 +++++ spec/rspec/rails_spec.rb | 5 +++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/rspec/openapi/schema_builder.rb b/lib/rspec/openapi/schema_builder.rb index b5f80126..435d4992 100644 --- a/lib/rspec/openapi/schema_builder.rb +++ b/lib/rspec/openapi/schema_builder.rb @@ -48,8 +48,6 @@ def example_enabled? def build_parameters(record) parameters = [] - marker_to_keep_last_duplicate = Time.now.utc.round.to_s - record.path_params.each do |key, value| parameters << { name: build_parameter_name(key, value), @@ -57,7 +55,6 @@ def build_parameters(record) required: true, schema: build_property(try_cast(value)), example: (try_cast(value) if example_enabled?), - __marker: marker_to_keep_last_duplicate, }.compact end @@ -67,7 +64,6 @@ def build_parameters(record) in: 'query', schema: build_property(try_cast(value)), example: (try_cast(value) if example_enabled?), - __marker: marker_to_keep_last_duplicate, }.compact end @@ -78,7 +74,6 @@ def build_parameters(record) required: true, schema: build_property(try_cast(value)), example: (try_cast(value) if example_enabled?), - __marker: marker_to_keep_last_duplicate, }.compact end diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index ea8292ae..276f7e70 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -27,6 +27,7 @@ def normalize_keys(spec) # # TODO: Should we probably force-merge `summary` regardless of manual modifications? def merge_schema!(base, spec) + marker_to_keep_last_duplicate = Time.now.utc.round.to_s spec.each do |key, value| if base[key].is_a?(Hash) && value.is_a?(Hash) if !base[key].key?("$ref") @@ -35,6 +36,10 @@ def merge_schema!(base, spec) elsif base[key].is_a?(Array) && value.is_a?(Array) # parameters need to be merged as if `name` and `in` were the Hash keys. if key == 'parameters' + value.each do |param| + param['__marker'] = marker_to_keep_last_duplicate + end + base[key] |= value # 9999 is a dummy for old spec. dummy = '9999-99-99 99:99:99 UTC' diff --git a/spec/rspec/rails_spec.rb b/spec/rspec/rails_spec.rb index cd16c7db..42989b09 100644 --- a/spec/rspec/rails_spec.rb +++ b/spec/rspec/rails_spec.rb @@ -37,6 +37,10 @@ File.expand_path('spec/rails/doc/smart/openapi.yaml', repo_root) end + let(:fallback_path) do + File.expand_path('spec/rails/doc/smart/openapi.fail.yaml', repo_root) + end + let(:expected_path) do File.expand_path('spec/rails/doc/smart/expected.yaml', repo_root) end @@ -47,6 +51,7 @@ rspec 'spec/requests/rails_smart_merge_spec.rb', openapi: true, output: :yaml new_yaml = YAML.load(File.read(openapi_path)) expected_yaml = YAML.load(File.read(expected_path)) + File.write(fallback_path, YAML.dump(new_yaml)) expect(new_yaml).to eq expected_yaml ensure File.write(openapi_path, original_source) From 1128a0c0e60b8b12bd06c8c431f4b3d4ca4dd86e Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 8 Jul 2022 17:17:45 +0900 Subject: [PATCH 5/7] it turned out that markers are not needed --- lib/rspec/openapi/schema_merger.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index 276f7e70..5b7c942d 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -27,7 +27,6 @@ def normalize_keys(spec) # # TODO: Should we probably force-merge `summary` regardless of manual modifications? def merge_schema!(base, spec) - marker_to_keep_last_duplicate = Time.now.utc.round.to_s spec.each do |key, value| if base[key].is_a?(Hash) && value.is_a?(Hash) if !base[key].key?("$ref") @@ -36,17 +35,10 @@ def merge_schema!(base, spec) elsif base[key].is_a?(Array) && value.is_a?(Array) # parameters need to be merged as if `name` and `in` were the Hash keys. if key == 'parameters' - value.each do |param| - param['__marker'] = marker_to_keep_last_duplicate - end - - base[key] |= value - # 9999 is a dummy for old spec. - dummy = '9999-99-99 99:99:99 UTC' + base[key] = value | base[key] base[key] - .sort_by! { |param| [param['in'], param['name'], param['__marker'] || dummy].join('-') } + .sort_by! { |param| [param['in'], param['name']].join('-') } .uniq! { |param| param.slice('name', 'in') } - .each { |param| param.delete('__marker') } else base[key] = value end From 2e88f7661a01181dc6994a71110e3c4cde7f379b Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 8 Jul 2022 17:31:48 +0900 Subject: [PATCH 6/7] Simplify: avoid sorting --- lib/rspec/openapi/schema_merger.rb | 4 +--- spec/rails/doc/openapi.json | 30 +++++++++++++++--------------- spec/rails/doc/openapi.yaml | 24 ++++++++++++------------ spec/rails/doc/smart/expected.yaml | 20 ++++++++++---------- 4 files changed, 38 insertions(+), 40 deletions(-) diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index 5b7c942d..fa5a729b 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -36,9 +36,7 @@ def merge_schema!(base, spec) # parameters need to be merged as if `name` and `in` were the Hash keys. if key == 'parameters' base[key] = value | base[key] - base[key] - .sort_by! { |param| [param['in'], param['name']].join('-') } - .uniq! { |param| param.slice('name', 'in') } + base[key].uniq! { |param| param.slice('name', 'in') } else base[key] = value end diff --git a/spec/rails/doc/openapi.json b/spec/rails/doc/openapi.json index c44cfa51..dc1285ea 100644 --- a/spec/rails/doc/openapi.json +++ b/spec/rails/doc/openapi.json @@ -106,42 +106,33 @@ }, "parameters": [ { - "name": "X-Authorization-Token", - "in": "header", - "required": true, - "schema": { - "type": "string" - }, - "example": "token" - }, - { - "name": "filter[name]", + "name": "filter[price]", "in": "query", "schema": { "type": "object", "properties": { - "name": { + "price": { "type": "string" } } }, "example": { - "name": "Example Table" + "price": "0" } }, { - "name": "filter[price]", + "name": "filter[name]", "in": "query", "schema": { "type": "object", "properties": { - "price": { + "name": { "type": "string" } } }, "example": { - "price": "0" + "name": "Example Table" } }, { @@ -159,6 +150,15 @@ "type": "integer" }, "example": 10 + }, + { + "name": "X-Authorization-Token", + "in": "header", + "required": true, + "schema": { + "type": "string" + }, + "example": "token" } ] }, diff --git a/spec/rails/doc/openapi.yaml b/spec/rails/doc/openapi.yaml index 481edf78..d984f334 100644 --- a/spec/rails/doc/openapi.yaml +++ b/spec/rails/doc/openapi.yaml @@ -20,30 +20,24 @@ paths: tags: - Table parameters: - - name: X-Authorization-Token - in: header - required: true - schema: - type: string - example: token - - name: filter[name] + - name: filter[price] in: query schema: type: object properties: - name: + price: type: string example: - name: Example Table - - name: filter[price] + price: '0' + - name: filter[name] in: query schema: type: object properties: - price: + name: type: string example: - price: '0' + name: Example Table - name: page in: query schema: @@ -54,6 +48,12 @@ paths: schema: type: integer example: 10 + - name: X-Authorization-Token + in: header + required: true + schema: + type: string + example: token responses: '200': description: with different deep query parameters diff --git a/spec/rails/doc/smart/expected.yaml b/spec/rails/doc/smart/expected.yaml index b69cdce0..66b6fcfa 100644 --- a/spec/rails/doc/smart/expected.yaml +++ b/spec/rails/doc/smart/expected.yaml @@ -20,6 +20,16 @@ paths: tags: - Table parameters: + - name: page + in: query + schema: + type: integer + example: 42 + - name: per + in: query + schema: + type: integer + example: 10 - name: X-Authorization-Token in: header required: true @@ -44,16 +54,6 @@ paths: type: string example: price: '0' - - name: page - in: query - schema: - type: integer - example: 42 - - name: per - in: query - schema: - type: integer - example: 10 responses: '200': description: with flat query parameters From d71682f1a310ff75ee68d86950f407e1da84cb97 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Fri, 8 Jul 2022 17:32:38 +0900 Subject: [PATCH 7/7] Remove debug code --- spec/rspec/rails_spec.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/rspec/rails_spec.rb b/spec/rspec/rails_spec.rb index 42989b09..cd16c7db 100644 --- a/spec/rspec/rails_spec.rb +++ b/spec/rspec/rails_spec.rb @@ -37,10 +37,6 @@ File.expand_path('spec/rails/doc/smart/openapi.yaml', repo_root) end - let(:fallback_path) do - File.expand_path('spec/rails/doc/smart/openapi.fail.yaml', repo_root) - end - let(:expected_path) do File.expand_path('spec/rails/doc/smart/expected.yaml', repo_root) end @@ -51,7 +47,6 @@ rspec 'spec/requests/rails_smart_merge_spec.rb', openapi: true, output: :yaml new_yaml = YAML.load(File.read(openapi_path)) expected_yaml = YAML.load(File.read(expected_path)) - File.write(fallback_path, YAML.dump(new_yaml)) expect(new_yaml).to eq expected_yaml ensure File.write(openapi_path, original_source)