From 289df4ce9b7e371be02ae8505e8d6513c9728e38 Mon Sep 17 00:00:00 2001 From: Maxime Raverdy Date: Thu, 4 Jun 2020 17:24:36 +0200 Subject: [PATCH 1/6] Add inheritance and discriminator attributes --- lib/grape-swagger/entity/attribute_parser.rb | 6 +- lib/grape-swagger/entity/parser.rb | 42 ++++++++++- spec/grape-swagger/entity/parser_spec.rb | 74 ++++++++++++++----- spec/spec_helper.rb | 1 + .../shared_contexts/inheritance_api.rb | 16 ++++ 5 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 spec/support/shared_contexts/inheritance_api.rb diff --git a/lib/grape-swagger/entity/attribute_parser.rb b/lib/grape-swagger/entity/attribute_parser.rb index cb3c6fe..e971d36 100644 --- a/lib/grape-swagger/entity/attribute_parser.rb +++ b/lib/grape-swagger/entity/attribute_parser.rb @@ -39,7 +39,7 @@ def call(entity_options) add_attribute_documentation(param, documentation) add_extension_documentation(param, documentation) - + add_discriminator_extension(param, documentation) param end end @@ -128,6 +128,10 @@ def add_array_documentation(param, documentation) def add_extension_documentation(param, documentation) GrapeSwagger::DocMethods::Extensions.add_extensions_to_root(documentation, param) end + + def add_discriminator_extension(param, documentation) + param[:documentation] = { is_discriminator: true } if documentation.key?(:is_discriminator) + end end end end diff --git a/lib/grape-swagger/entity/parser.rb b/lib/grape-swagger/entity/parser.rb index 2603285..49bf556 100644 --- a/lib/grape-swagger/entity/parser.rb +++ b/lib/grape-swagger/entity/parser.rb @@ -26,7 +26,13 @@ def initialize(original, renamed) end def extract_params(exposure) - exposure.root_exposures.each_with_object({}) do |value, memo| + root_exposures = + if superclass_contains_discriminator?(exposure) + root_exposures_without_parent(exposure) + else + exposure.root_exposures + end + root_exposures.each_with_object({}) do |value, memo| if value.for_merge && (value.respond_to?(:entity_class) || value.respond_to?(:using_class_name)) entity_class = value.respond_to?(:entity_class) ? value.entity_class : value.using_class_name @@ -39,6 +45,18 @@ def extract_params(exposure) end end + def superclass_contains_discriminator?(exposure) + exposure.superclass.root_exposures.detect do |value| + value.documentation[:is_discriminator] + end + end + + def root_exposures_without_parent(exposure) + exposure.root_exposures.select do |value| + exposure.superclass.root_exposures.find_by(value.attribute).nil? + end + end + def parse_grape_entity_params(params, parent_model = nil) return unless params @@ -63,8 +81,28 @@ def parse_grape_entity_params(params, parent_model = nil) memo[final_entity_name][:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only] memo[final_entity_name][:description] = documentation[:desc] if documentation[:desc] end + if superclass_contains_discriminator?(model) + respond_with_all_of(parsed, params) + else + [parsed, required_params(params)] + end + end - [parsed, required_params(params)] + def respond_with_all_of(parsed, params) + parent_name = + if endpoint.nil? + model.superclass.to_s.demodulize + else + endpoint.send(:expose_params_from_model, model.superclass) + end + { + allOf: [ + { + '$ref' => "#/definitions/#{parent_name}" + }, + [parsed, required_params(params)] + ] + } end def parse_nested(entity_name, entity_options, parent_model = nil) diff --git a/spec/grape-swagger/entity/parser_spec.rb b/spec/grape-swagger/entity/parser_spec.rb index 889a968..6cfdad7 100644 --- a/spec/grape-swagger/entity/parser_spec.rb +++ b/spec/grape-swagger/entity/parser_spec.rb @@ -2,31 +2,69 @@ require_relative '../../../spec/support/shared_contexts/this_api' describe GrapeSwagger::Entity::Parser do - include_context 'this api' + context 'this api' do + include_context 'this api' - describe '#call' do - let(:parsed_entity) { described_class.new(ThisApi::Entities::Something, endpoint).call } - let(:properties) { parsed_entity.first } - let(:required) { parsed_entity.last } + describe '#call' do + let(:parsed_entity) { described_class.new(ThisApi::Entities::Something, endpoint).call } + let(:properties) { parsed_entity.first } + let(:required) { parsed_entity.last } - context 'when no endpoint is passed' do - let(:endpoint) { nil } + context 'when no endpoint is passed' do + let(:endpoint) { nil } - it 'parses the model with the correct :using definition' do - expect(properties[:kind]['$ref']).to eq('#/definitions/Kind') - expect(properties[:kind2]['$ref']).to eq('#/definitions/Kind') - expect(properties[:kind3]['$ref']).to eq('#/definitions/Kind') + it 'parses the model with the correct :using definition' do + expect(properties[:kind]['$ref']).to eq('#/definitions/Kind') + expect(properties[:kind2]['$ref']).to eq('#/definitions/Kind') + expect(properties[:kind3]['$ref']).to eq('#/definitions/Kind') + end + + it 'merges attributes that have merge: true defined' do + expect(properties[:merged_attribute]).to be_nil + expect(properties[:code][:type]).to eq('string') + expect(properties[:message][:type]).to eq('string') + expect(properties[:attr][:type]).to eq('string') + end + + it 'hides hidden attributes' do + expect(properties).to_not include(:hidden_attr) + end end + end + end + context 'inheritance api' do + include_context 'inheritance api' - it 'merges attributes that have merge: true defined' do - expect(properties[:merged_attribute]).to be_nil - expect(properties[:code][:type]).to eq('string') - expect(properties[:message][:type]).to eq('string') - expect(properties[:attr][:type]).to eq('string') + describe '#call for Parent' do + let(:parsed_entity) do + described_class.new(InheritanceApi::Entities::Parent, endpoint).call end + let(:properties) { parsed_entity.first } + + context 'when no endpoint is passed' do + let(:endpoint) { nil } + + it 'parses the model with discriminator' do + expect(properties[:type][:documentation]).to eq(is_discriminator: true) + end + end + end + + describe '#call for Child' do + let(:parsed_entity) do + described_class.new(InheritanceApi::Entities::Child, endpoint).call + end + let(:properties) { parsed_entity } + + context 'when no endpoint is passed' do + let(:endpoint) { nil } - it 'hides hidden attributes' do - expect(properties).to_not include(:hidden_attr) + it 'parses the model with allOf' do + expect(properties).to include(:allOf) + all_of = properties[:allOf] + expect(all_of.first['$ref']).to eq('#/definitions/Parent') + expect(all_of.last.first[:name][:type]).to eq('string') + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f8d6b10..c9bc034 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,6 +7,7 @@ require 'rack' require 'rack/test' +require 'pry' RSpec.configure do |config| require 'rspec/expectations' diff --git a/spec/support/shared_contexts/inheritance_api.rb b/spec/support/shared_contexts/inheritance_api.rb new file mode 100644 index 0000000..bc40812 --- /dev/null +++ b/spec/support/shared_contexts/inheritance_api.rb @@ -0,0 +1,16 @@ +shared_context 'inheritance api' do + before :all do + module InheritanceApi + module Entities + class Parent < Grape::Entity + expose :type, documentation: { type: 'string', is_discriminator: true, required: true } + expose :id, documentation: { type: 'integer' } + end + + class Child < Parent + expose :name, documentation: { type: 'string', desc: 'Name' } + end + end + end + end +end From 2fb7c32781b2cac1fe840ecb9b64840e7c4f30b4 Mon Sep 17 00:00:00 2001 From: Maxime Raverdy Date: Thu, 4 Jun 2020 17:58:08 +0200 Subject: [PATCH 2/6] documentation could be nil --- lib/grape-swagger/entity/parser.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape-swagger/entity/parser.rb b/lib/grape-swagger/entity/parser.rb index 49bf556..0c4d413 100644 --- a/lib/grape-swagger/entity/parser.rb +++ b/lib/grape-swagger/entity/parser.rb @@ -47,7 +47,7 @@ def extract_params(exposure) def superclass_contains_discriminator?(exposure) exposure.superclass.root_exposures.detect do |value| - value.documentation[:is_discriminator] + value.documentation.try(:[], :is_discriminator) end end From f05375d1f57536a4092dfe289fcb5b10fc76c1c4 Mon Sep 17 00:00:00 2001 From: Maxime Raverdy Date: Fri, 5 Jun 2020 11:14:42 +0200 Subject: [PATCH 3/6] Add required type and property --- lib/grape-swagger/entity.rb | 1 + lib/grape-swagger/entity/attribute_parser.rb | 2 +- lib/grape-swagger/entity/helper.rb | 13 ++++++++ lib/grape-swagger/entity/parser.rb | 32 ++++++++++++++++---- spec/grape-swagger/entity/parser_spec.rb | 7 ++++- 5 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 lib/grape-swagger/entity/helper.rb diff --git a/lib/grape-swagger/entity.rb b/lib/grape-swagger/entity.rb index 51828d7..23d5ace 100644 --- a/lib/grape-swagger/entity.rb +++ b/lib/grape-swagger/entity.rb @@ -2,6 +2,7 @@ require 'grape-entity' require 'grape-swagger/entity/version' +require 'grape-swagger/entity/helper' require 'grape-swagger/entity/attribute_parser' require 'grape-swagger/entity/parser' diff --git a/lib/grape-swagger/entity/attribute_parser.rb b/lib/grape-swagger/entity/attribute_parser.rb index e971d36..86de296 100644 --- a/lib/grape-swagger/entity/attribute_parser.rb +++ b/lib/grape-swagger/entity/attribute_parser.rb @@ -12,7 +12,7 @@ def call(entity_options) entity_model = model_from(entity_options) if entity_model - name = endpoint.nil? ? entity_model.to_s.demodulize : endpoint.send(:expose_params_from_model, entity_model) + name = GrapeSwagger::Entity::Helper.model_name(entity_model, endpoint) entity_model_type = entity_model_type(name, entity_options) return entity_model_type unless documentation diff --git a/lib/grape-swagger/entity/helper.rb b/lib/grape-swagger/entity/helper.rb new file mode 100644 index 0000000..44f8f33 --- /dev/null +++ b/lib/grape-swagger/entity/helper.rb @@ -0,0 +1,13 @@ +module GrapeSwagger + module Entity + class Helper + def self.model_name(entity_model, endpoint) + if endpoint.nil? + entity_model.to_s.demodulize + else + endpoint.send(:expose_params_from_model, entity_model) + end + end + end + end +end \ No newline at end of file diff --git a/lib/grape-swagger/entity/parser.rb b/lib/grape-swagger/entity/parser.rb index 0c4d413..7002456 100644 --- a/lib/grape-swagger/entity/parser.rb +++ b/lib/grape-swagger/entity/parser.rb @@ -27,7 +27,7 @@ def initialize(original, renamed) def extract_params(exposure) root_exposures = - if superclass_contains_discriminator?(exposure) + if discriminator(exposure) root_exposures_without_parent(exposure) else exposure.root_exposures @@ -45,7 +45,7 @@ def extract_params(exposure) end end - def superclass_contains_discriminator?(exposure) + def discriminator(exposure) exposure.superclass.root_exposures.detect do |value| value.documentation.try(:[], :is_discriminator) end @@ -81,15 +81,19 @@ def parse_grape_entity_params(params, parent_model = nil) memo[final_entity_name][:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only] memo[final_entity_name][:description] = documentation[:desc] if documentation[:desc] end - if superclass_contains_discriminator?(model) - respond_with_all_of(parsed, params) + + discriminator = discriminator(model) + if discriminator + respond_with_all_of(parsed, params, discriminator) else [parsed, required_params(params)] end end - def respond_with_all_of(parsed, params) + def respond_with_all_of(parsed, params, discriminator) parent_name = + GrapeSwagger::Entity::Helper.model_name(model.superclass, endpoint) + if endpoint.nil? model.superclass.to_s.demodulize else @@ -100,11 +104,27 @@ def respond_with_all_of(parsed, params) { '$ref' => "#/definitions/#{parent_name}" }, - [parsed, required_params(params)] + [ + add_discriminator(parsed, discriminator), + required_params(params).push(discriminator.attribute) + ] ] } end + def add_discriminator(parsed, discriminator) + model_name = GrapeSwagger::Entity::Helper.model_name(model, endpoint) + + parsed.merge( + { + discriminator.attribute => { + type: 'string', + enum: [model_name] + } + } + ) + end + def parse_nested(entity_name, entity_options, parent_model = nil) nested_entity = if parent_model.nil? model.root_exposures.find_by(entity_name) diff --git a/spec/grape-swagger/entity/parser_spec.rb b/spec/grape-swagger/entity/parser_spec.rb index 6cfdad7..f781f3a 100644 --- a/spec/grape-swagger/entity/parser_spec.rb +++ b/spec/grape-swagger/entity/parser_spec.rb @@ -62,8 +62,13 @@ it 'parses the model with allOf' do expect(properties).to include(:allOf) all_of = properties[:allOf] + child_property = all_of.last.first + child_required = all_of.last.last expect(all_of.first['$ref']).to eq('#/definitions/Parent') - expect(all_of.last.first[:name][:type]).to eq('string') + expect(child_property[:name][:type]).to eq('string') + expect(child_property[:type][:type]).to eq('string') + expect(child_property[:type][:enum]).to eq(['Child']) + expect(child_required).to include(:type) end end end From 1286b1e8068b09d58b8bab7aaeb77d8cc6ccc843 Mon Sep 17 00:00:00 2001 From: Maxime Raverdy Date: Fri, 5 Jun 2020 11:40:21 +0200 Subject: [PATCH 4/6] Fix rubocop --- lib/grape-swagger/entity/helper.rb | 35 ++++++++++++++++++++++----- lib/grape-swagger/entity/parser.rb | 38 +++++------------------------- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/lib/grape-swagger/entity/helper.rb b/lib/grape-swagger/entity/helper.rb index 44f8f33..0a01b49 100644 --- a/lib/grape-swagger/entity/helper.rb +++ b/lib/grape-swagger/entity/helper.rb @@ -1,13 +1,36 @@ module GrapeSwagger module Entity + # Helper methods for DRY class Helper - def self.model_name(entity_model, endpoint) - if endpoint.nil? - entity_model.to_s.demodulize - else - endpoint.send(:expose_params_from_model, entity_model) + class << self + def model_name(entity_model, endpoint) + if endpoint.nil? + entity_model.to_s.demodulize + else + endpoint.send(:expose_params_from_model, entity_model) + end + end + + def discriminator(entity_model) + entity_model.superclass.root_exposures.detect do |value| + value.documentation.try(:[], :is_discriminator) + end + end + + def root_exposures_without_parent(entity_model) + entity_model.root_exposures.select do |value| + entity_model.superclass.root_exposures.find_by(value.attribute).nil? + end + end + + def root_exposure_with_discriminator(entity_model) + if discriminator(entity_model) + root_exposures_without_parent(entity_model) + else + entity_model.root_exposures + end end end end end -end \ No newline at end of file +end diff --git a/lib/grape-swagger/entity/parser.rb b/lib/grape-swagger/entity/parser.rb index 7002456..41cc169 100644 --- a/lib/grape-swagger/entity/parser.rb +++ b/lib/grape-swagger/entity/parser.rb @@ -26,13 +26,7 @@ def initialize(original, renamed) end def extract_params(exposure) - root_exposures = - if discriminator(exposure) - root_exposures_without_parent(exposure) - else - exposure.root_exposures - end - root_exposures.each_with_object({}) do |value, memo| + GrapeSwagger::Entity::Helper.root_exposure_with_discriminator(exposure).each_with_object({}) do |value, memo| if value.for_merge && (value.respond_to?(:entity_class) || value.respond_to?(:using_class_name)) entity_class = value.respond_to?(:entity_class) ? value.entity_class : value.using_class_name @@ -45,18 +39,6 @@ def extract_params(exposure) end end - def discriminator(exposure) - exposure.superclass.root_exposures.detect do |value| - value.documentation.try(:[], :is_discriminator) - end - end - - def root_exposures_without_parent(exposure) - exposure.root_exposures.select do |value| - exposure.superclass.root_exposures.find_by(value.attribute).nil? - end - end - def parse_grape_entity_params(params, parent_model = nil) return unless params @@ -82,7 +64,7 @@ def parse_grape_entity_params(params, parent_model = nil) memo[final_entity_name][:description] = documentation[:desc] if documentation[:desc] end - discriminator = discriminator(model) + discriminator = GrapeSwagger::Entity::Helper.discriminator(model) if discriminator respond_with_all_of(parsed, params, discriminator) else @@ -91,14 +73,8 @@ def parse_grape_entity_params(params, parent_model = nil) end def respond_with_all_of(parsed, params, discriminator) - parent_name = - GrapeSwagger::Entity::Helper.model_name(model.superclass, endpoint) + parent_name = GrapeSwagger::Entity::Helper.model_name(model.superclass, endpoint) - if endpoint.nil? - model.superclass.to_s.demodulize - else - endpoint.send(:expose_params_from_model, model.superclass) - end { allOf: [ { @@ -116,11 +92,9 @@ def add_discriminator(parsed, discriminator) model_name = GrapeSwagger::Entity::Helper.model_name(model, endpoint) parsed.merge( - { - discriminator.attribute => { - type: 'string', - enum: [model_name] - } + discriminator.attribute => { + type: 'string', + enum: [model_name] } ) end From 7762b53045694e6d714af617c95e678048ff7c67 Mon Sep 17 00:00:00 2001 From: Maxime Raverdy Date: Fri, 5 Jun 2020 12:12:20 +0200 Subject: [PATCH 5/6] Add changelog --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2279eab..b426c1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,7 @@ #### Features * Your contribution here. - -#### Features - -* Your contribution here. +* [#50](https://github.com/ruby-grape/grape-swagger-entity/pull/50): Features/inheritance and discriminator - [@MaximeRDY](https://github.com/MaximeRDY). ### 0.4.0 (May 30, 2020) From 0422d686af4a1e9ea1a5c9d0870304bca6cf0704 Mon Sep 17 00:00:00 2001 From: Maxime Raverdy Date: Fri, 12 Jun 2020 10:58:17 +0200 Subject: [PATCH 6/6] Fix code --- lib/grape-swagger/entity/helper.rb | 2 +- spec/spec_helper.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/grape-swagger/entity/helper.rb b/lib/grape-swagger/entity/helper.rb index 0a01b49..f88ece0 100644 --- a/lib/grape-swagger/entity/helper.rb +++ b/lib/grape-swagger/entity/helper.rb @@ -13,7 +13,7 @@ def model_name(entity_model, endpoint) def discriminator(entity_model) entity_model.superclass.root_exposures.detect do |value| - value.documentation.try(:[], :is_discriminator) + value.documentation.dig(:is_discriminator) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c9bc034..f8d6b10 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,7 +7,6 @@ require 'rack' require 'rack/test' -require 'pry' RSpec.configure do |config| require 'rspec/expectations'