From f395ed1d87d408f1c4c1f01f646ef0c25f905ba5 Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Mon, 16 May 2022 09:34:27 +0900 Subject: [PATCH 1/3] Implement smarter merge --- lib/rspec/openapi/schema_merger.rb | 13 ++--- spec/rails/doc/openapi.json | 46 ++-------------- spec/rails/doc/openapi.yaml | 39 +++----------- spec/rspec/schema_merger/base.json | 53 +++++++++++++++++++ spec/rspec/schema_merger/expected.json | 53 +++++++++++++++++++ spec/rspec/schema_merger/input.json | 39 ++++++++++++++ .../rspec/schema_merger/schema_merger_spec.rb | 27 ++++++++++ 7 files changed, 185 insertions(+), 85 deletions(-) create mode 100644 spec/rspec/schema_merger/base.json create mode 100644 spec/rspec/schema_merger/expected.json create mode 100644 spec/rspec/schema_merger/input.json create mode 100644 spec/rspec/schema_merger/schema_merger_spec.rb diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index 0eef16e0..bac3ee44 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -25,22 +25,15 @@ def normalize_keys(spec) # Also this needs to be aware of OpenAPI details unlike an ordinary deep_reverse_merge # because a Hash-like structure may be an array whose Hash elements have a key name. # - # TODO: Perform more intelligent merges like rerouting edits / merging types # TODO: Should we probably force-merge `summary` regardless of manual modifications? def deep_reverse_merge!(base, spec) spec.each do |key, value| if base[key].is_a?(Hash) && value.is_a?(Hash) - deep_reverse_merge!(base[key], value) - elsif !base.key?(key) - base[key] = value - 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' - base[key] |= value - base[key].uniq! { |param| param.slice('name', 'in') } + if !base[key].key?("$ref") + deep_reverse_merge!(base[key], value) end else - # no-op + base[key] = value end end base diff --git a/spec/rails/doc/openapi.json b/spec/rails/doc/openapi.json index 0a0aceb6..1e556ab3 100644 --- a/spec/rails/doc/openapi.json +++ b/spec/rails/doc/openapi.json @@ -41,7 +41,7 @@ } }, "200": { - "description": "with flat query parameters", + "description": "with different deep query parameters", "content": { "application/json": { "schema": { @@ -105,46 +105,6 @@ } }, "parameters": [ - { - "name": "page", - "in": "query", - "schema": { - "type": "integer" - }, - "example": 1 - }, - { - "name": "per", - "in": "query", - "schema": { - "type": "integer" - }, - "example": 10 - }, - { - "name": "X-Authorization-Token", - "in": "header", - "required": true, - "schema": { - "type": "string" - }, - "example": "token" - }, - { - "name": "filter[name]", - "in": "query", - "schema": { - "type": "object", - "properties": { - "name": { - "type": "string" - } - } - }, - "example": { - "name": "Example Table" - } - }, { "name": "filter[price]", "in": "query", @@ -268,7 +228,7 @@ "schema": { "type": "integer" }, - "example": 1 + "example": 2 } ], "responses": { @@ -406,7 +366,7 @@ }, "example": { "nested": { - "image": "#", + "image": "test.png", "caption": "Some caption" } } diff --git a/spec/rails/doc/openapi.yaml b/spec/rails/doc/openapi.yaml index 1c5bc521..8398d9d2 100644 --- a/spec/rails/doc/openapi.yaml +++ b/spec/rails/doc/openapi.yaml @@ -20,25 +20,6 @@ paths: tags: - Table parameters: - - name: page - in: query - schema: - type: integer - example: 1 - - name: per - in: query - schema: - type: integer - example: 10 - - name: filter[name] - in: query - schema: - type: object - properties: - name: - type: string - example: - name: Example Table - name: filter[price] in: query schema: @@ -48,15 +29,9 @@ paths: type: string example: price: '0' - - name: X-Authorization-Token - in: header - required: true - schema: - type: string - example: token responses: '200': - description: returns a list of tables + description: with different deep query parameters content: application/json: schema: @@ -93,7 +68,7 @@ paths: database: id: 2 name: production - null_sample: + null_sample: storage_size: 12.3 created_at: '2020-07-17T00:00:00+00:00' updated_at: '2020-07-17T00:00:00+00:00' @@ -165,7 +140,7 @@ paths: database: id: 2 name: production - null_sample: + null_sample: storage_size: 12.3 created_at: '2020-07-17T00:00:00+00:00' updated_at: '2020-07-17T00:00:00+00:00' @@ -180,7 +155,7 @@ paths: required: true schema: type: integer - example: 1 + example: 2 responses: '200': description: returns a table @@ -218,7 +193,7 @@ paths: database: id: 2 name: production - null_sample: + null_sample: storage_size: 12.3 created_at: '2020-07-17T00:00:00+00:00' updated_at: '2020-07-17T00:00:00+00:00' @@ -310,7 +285,7 @@ paths: database: id: 2 name: production - null_sample: + null_sample: storage_size: 12.3 created_at: '2020-07-17T00:00:00+00:00' updated_at: '2020-07-17T00:00:00+00:00' @@ -362,7 +337,7 @@ paths: database: id: 2 name: production - null_sample: + null_sample: storage_size: 12.3 created_at: '2020-07-17T00:00:00+00:00' updated_at: '2020-07-17T00:00:00+00:00' diff --git a/spec/rspec/schema_merger/base.json b/spec/rspec/schema_merger/base.json new file mode 100644 index 00000000..c14ed80a --- /dev/null +++ b/spec/rspec/schema_merger/base.json @@ -0,0 +1,53 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "My API", + "version": "1.0.0" + }, + "servers": [], + "paths": { + "/tables": { + "get": { + "summary": "index", + "tags": [ + "Table" + ], + "responses": { + "200": { + "description": "foo", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Table" + }, + "example": [ + { + "id": 1, + "name": "THIS SHOULD BE UPDATED" + } + ] + } + } + } + }, + "parameters": [] + } + } + }, + "components": { + "schemas": { + "Table": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "name": { + "type": "string" + } + } + } + } + } +} diff --git a/spec/rspec/schema_merger/expected.json b/spec/rspec/schema_merger/expected.json new file mode 100644 index 00000000..07c2fba4 --- /dev/null +++ b/spec/rspec/schema_merger/expected.json @@ -0,0 +1,53 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "My API", + "version": "1.0.0" + }, + "servers": [], + "paths": { + "/tables": { + "get": { + "summary": "index", + "tags": [ + "Table" + ], + "responses": { + "200": { + "description": "foo", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Table" + }, + "example": [ + { + "id": 9999, + "name": "UPDATED" + } + ] + } + } + } + }, + "parameters": [] + } + } + }, + "components": { + "schemas": { + "Table": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "name": { + "type": "string" + } + } + } + } + } +} diff --git a/spec/rspec/schema_merger/input.json b/spec/rspec/schema_merger/input.json new file mode 100644 index 00000000..cf5ec2b9 --- /dev/null +++ b/spec/rspec/schema_merger/input.json @@ -0,0 +1,39 @@ +{ + "paths": { + "/tables": { + "get": { + "summary": "index", + "tags": [ + "Table" + ], + "responses": { + "200": { + "description": "foo", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "name": { + "type": "string" + } + } + }, + "example": [ + { + "id": 9999, + "name": "UPDATED" + } + ] + } + } + } + }, + "parameters": [] + } + } + } +} diff --git a/spec/rspec/schema_merger/schema_merger_spec.rb b/spec/rspec/schema_merger/schema_merger_spec.rb new file mode 100644 index 00000000..a87154c9 --- /dev/null +++ b/spec/rspec/schema_merger/schema_merger_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' +require 'json' +require "rspec/openapi/schema_merger" + +RSpec.describe "SchemaMerger" do + include SpecHelper + + let(:base_path) do + File.expand_path('spec/rspec/schema_merger/base.json', repo_root) + end + + let(:input_path) do + File.expand_path('spec/rspec/schema_merger/input.json', repo_root) + end + + let(:expected_path) do + File.expand_path('spec/rspec/schema_merger/expected.json', repo_root) + end + + it "overwrite the supported key, but leaves the unsupported keys" do + base_json = JSON.load(File.read(base_path)) + input_json = JSON.load(File.read(input_path)) + res = RSpec::OpenAPI::SchemaMerger.reverse_merge!(base_json, input_json) + expected_json = JSON.load(File.read(expected_path)) + expect(res).to eq(expected_json) + end +end From 07597de6317ca1509f88e4bc64c226bb3ffcd01f Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Mon, 16 May 2022 14:31:22 +0900 Subject: [PATCH 2/3] Merge arrays --- lib/rspec/openapi/schema_merger.rb | 8 ++++++ spec/rails/doc/openapi.json | 42 +++++++++++++++++++++++++++++- spec/rails/doc/openapi.yaml | 27 ++++++++++++++++++- 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/lib/rspec/openapi/schema_merger.rb b/lib/rspec/openapi/schema_merger.rb index bac3ee44..9d10e4e3 100644 --- a/lib/rspec/openapi/schema_merger.rb +++ b/lib/rspec/openapi/schema_merger.rb @@ -32,6 +32,14 @@ def deep_reverse_merge!(base, spec) if !base[key].key?("$ref") deep_reverse_merge!(base[key], value) end + 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' + base[key] |= value + base[key].uniq! { |param| param.slice('name', 'in') } + else + base[key] = value + end else base[key] = value end diff --git a/spec/rails/doc/openapi.json b/spec/rails/doc/openapi.json index 1e556ab3..a9847448 100644 --- a/spec/rails/doc/openapi.json +++ b/spec/rails/doc/openapi.json @@ -105,6 +105,46 @@ } }, "parameters": [ + { + "name": "page", + "in": "query", + "schema": { + "type": "integer" + }, + "example": 1 + }, + { + "name": "per", + "in": "query", + "schema": { + "type": "integer" + }, + "example": 10 + }, + { + "name": "X-Authorization-Token", + "in": "header", + "required": true, + "schema": { + "type": "string" + }, + "example": "token" + }, + { + "name": "filter[name]", + "in": "query", + "schema": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "example": { + "name": "Example Table" + } + }, { "name": "filter[price]", "in": "query", @@ -228,7 +268,7 @@ "schema": { "type": "integer" }, - "example": 2 + "example": 1 } ], "responses": { diff --git a/spec/rails/doc/openapi.yaml b/spec/rails/doc/openapi.yaml index 8398d9d2..642fee47 100644 --- a/spec/rails/doc/openapi.yaml +++ b/spec/rails/doc/openapi.yaml @@ -20,6 +20,25 @@ paths: tags: - Table parameters: + - name: page + in: query + schema: + type: integer + example: 1 + - name: per + in: query + schema: + type: integer + example: 10 + - name: filter[name] + in: query + schema: + type: object + properties: + name: + type: string + example: + name: Example Table - name: filter[price] in: query schema: @@ -29,6 +48,12 @@ paths: type: string example: price: '0' + - name: X-Authorization-Token + in: header + required: true + schema: + type: string + example: token responses: '200': description: with different deep query parameters @@ -155,7 +180,7 @@ paths: required: true schema: type: integer - example: 2 + example: 1 responses: '200': description: returns a table From 7bf6054ff36821201b842988562cb33cc015bd0e Mon Sep 17 00:00:00 2001 From: TATSUNO Yasuhiro Date: Mon, 16 May 2022 14:36:53 +0900 Subject: [PATCH 3/3] Ensure the base parameters are replaced --- spec/rspec/schema_merger/base.json | 11 +++++++++++ spec/rspec/schema_merger/expected.json | 14 ++++++++++++-- spec/rspec/schema_merger/input.json | 14 ++++++++++++-- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/spec/rspec/schema_merger/base.json b/spec/rspec/schema_merger/base.json index c14ed80a..38449306 100644 --- a/spec/rspec/schema_merger/base.json +++ b/spec/rspec/schema_merger/base.json @@ -12,6 +12,17 @@ "tags": [ "Table" ], + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer" + }, + "example": 1 + } + ], "responses": { "200": { "description": "foo", diff --git a/spec/rspec/schema_merger/expected.json b/spec/rspec/schema_merger/expected.json index 07c2fba4..b4b8db07 100644 --- a/spec/rspec/schema_merger/expected.json +++ b/spec/rspec/schema_merger/expected.json @@ -12,6 +12,17 @@ "tags": [ "Table" ], + "parameters": [ + { + "name": "foo_id", + "in": "path", + "required": true, + "schema": { + "type": "integer" + }, + "example": 9999 + } + ], "responses": { "200": { "description": "foo", @@ -29,8 +40,7 @@ } } } - }, - "parameters": [] + } } } }, diff --git a/spec/rspec/schema_merger/input.json b/spec/rspec/schema_merger/input.json index cf5ec2b9..32c0706e 100644 --- a/spec/rspec/schema_merger/input.json +++ b/spec/rspec/schema_merger/input.json @@ -6,6 +6,17 @@ "tags": [ "Table" ], + "parameters": [ + { + "name": "foo_id", + "in": "path", + "required": true, + "schema": { + "type": "integer" + }, + "example": 9999 + } + ], "responses": { "200": { "description": "foo", @@ -31,8 +42,7 @@ } } } - }, - "parameters": [] + } } } }