From b4a72a1794e164e92ab27dada434c3c8e7b21f72 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Mon, 6 Jul 2026 17:41:13 +0200 Subject: [PATCH] Read route metadata via readers instead of route.options[] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grape-swagger reached into the route's options Hash for metadata. Replace every route.options[...] read with the equivalent reader method — including the success/failure aliases of entity/http_codes and the dynamic producer lookup (via public_send) — so grape-swagger consumes routes through their public method interface rather than the internal options Hash. This lets Grape restructure route.options (reserving it for user custom keys) without breaking grape-swagger. build_body_parameter now takes a resolved body_name value rather than the options Hash. The readers resolve on supported Grape (verified on 2.4.0). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + lib/grape-swagger/doc_methods/move_params.rb | 6 ++-- lib/grape-swagger/doc_methods/operation_id.rb | 13 ++++---- lib/grape-swagger/endpoint.rb | 33 ++++++++++--------- spec/lib/move_params_spec.rb | 8 ++--- spec/swagger_v2/api_swagger_v2_detail_spec.rb | 15 +++++++++ .../endpoint_versioned_path_spec.rb | 17 ++++++++++ 7 files changed, 63 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c1cfc1c..3c12ae22b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * [#978](https://github.com/ruby-grape/grape-swagger/pull/978): Fix Grape 3.2+ compatibility: desc kwargs, custom types, multi-type param recovery; bump Grape to `>= 2.1, < 5.0`. See [UPGRADING](UPGRADING.md) - [@numbata](https://github.com/numbata). * [#982](https://github.com/ruby-grape/grape-swagger/pull/982): Fix test suite compatibility with Grape 4.0 (grape=HEAD CI) - [@numbata](https://github.com/numbata). * [#981](https://github.com/ruby-grape/grape-swagger/pull/981): Use `endpoint.endpoints` instead of `endpoint.options[:app]` - [@ericproulx](https://github.com/ericproulx). +* [#983](https://github.com/ruby-grape/grape-swagger/pull/983): Read route metadata via reader methods instead of `route.options[...]` - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 2.1.4 (2026-02-02) diff --git a/lib/grape-swagger/doc_methods/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index c8417ce5a..ad2311da0 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -34,7 +34,7 @@ def parent_definition_of_params(params, path, route) definition[:description] = route.description if route.try(:description) - build_body_parameter(definition_name, route.options) + build_body_parameter(definition_name, route.body_name) end def move_params_to_new(definition, params) @@ -139,9 +139,9 @@ def add_to_required(definition, value) definition[:required].push(*value) end - def build_body_parameter(name, options) + def build_body_parameter(name, body_name) {}.tap do |x| - x[:name] = options[:body_name] || name + x[:name] = body_name || name x[:in] = 'body' x[:required] = true x[:schema] = { '$ref' => "#/definitions/#{name}" } diff --git a/lib/grape-swagger/doc_methods/operation_id.rb b/lib/grape-swagger/doc_methods/operation_id.rb index e46236a66..ea4d707c5 100644 --- a/lib/grape-swagger/doc_methods/operation_id.rb +++ b/lib/grape-swagger/doc_methods/operation_id.rb @@ -5,13 +5,12 @@ module DocMethods class OperationId class << self def build(route, path = nil) - if route.options[:nickname] - route.options[:nickname] - else - verb = route.request_method.to_s.downcase - operation = manipulate(path) unless path.nil? - "#{verb}#{operation}" - end + nickname = route.nickname + return nickname if nickname + + verb = route.request_method.to_s.downcase + operation = manipulate(path) unless path.nil? + "#{verb}#{operation}" end def manipulate(path) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 7d0079e0b..98b0c141f 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -100,7 +100,7 @@ def path_item(routes, options) next if hidden?(route, options) @item, path = GrapeSwagger::DocMethods::PathString.build(route, options) - @entity = route.entity || route.options[:success] + @entity = route.entity || route.success verb, method_object = method_object(route, options, path) @@ -123,7 +123,7 @@ def method_object(route, options, path) method[:parameters] = params_object(route, options, path, method[:consumes]) method[:security] = security_object(route) method[:responses] = response_object(route, options) - method[:tags] = route.options.fetch(:tags, tag_object(route, path)) + method[:tags] = route.options.key?(:tags) ? route.tags : tag_object(route, path) method[:operationId] = GrapeSwagger::DocMethods::OperationId.build(route, path) method[:deprecated] = deprecated_object(route) method.delete_if { |_, value| value.nil? } @@ -132,36 +132,36 @@ def method_object(route, options, path) end def deprecated_object(route) - route.options[:deprecated] if route.options.key?(:deprecated) + route.deprecated end def security_object(route) - route.options[:security] if route.options.key?(:security) + route.security end def summary_object(route) - summary = route.options[:desc] if route.options.key?(:desc) - summary = route.description if route.description.present? && route.options.key?(:detail) - summary = route.options[:summary] if route.options.key?(:summary) + summary = route.desc if route.desc + summary = route.description if route.description.present? && route.detail + summary = route.summary if route.summary summary end def description_object(route) description = route.description if route.description.present? - description = route.options[:detail] if route.options.key?(:detail) + description = route.detail if route.detail description end def produces_object(route, format) - return ['application/octet-stream'] if file_response?(route.options[:success]) && - !route.options[:produces].present? + return ['application/octet-stream'] if file_response?(route.success) && + !route.produces.present? mime_types = GrapeSwagger::DocMethods::ProducesConsumes.call(format) route_mime_types = %i[formats content_types produces].map do |producer| - possible = route.options[producer] + possible = route.public_send(producer) GrapeSwagger::DocMethods::ProducesConsumes.call(possible) if possible.present? end.flatten.compact.uniq @@ -241,7 +241,7 @@ def http_codes_from_route(route) route.http_codes.clone else success_codes_from_route(route) + default_code_from_route(route) + - (route.http_codes || route.options[:failure] || []) + (route.http_codes || route.failure || []) end end @@ -269,7 +269,7 @@ def tag_object(route, path) private def default_code_from_route(route) - entity = route.options[:default_response] + entity = route.default_response return [] if entity.nil? default_code = { code: 'default', message: 'Default Response' } @@ -346,7 +346,7 @@ def build_reference(route, value, response_model, settings) if value.key?(:as) && value.key?(:is_array) reference[value[:as]] = build_reference_array(reference[value[:as]]) - elsif route.options[:is_array] + elsif route.is_array reference = build_reference_array(reference) end @@ -363,7 +363,7 @@ def build_reference_array(reference) def build_root(route, reference, response_model, settings) default_root = response_model.underscore - default_root = default_root.pluralize if route.options[:is_array] + default_root = default_root.pluralize if route.is_array case route.settings.dig(:swagger, :root) when true { type: 'object', properties: { default_root => reference } } @@ -438,7 +438,8 @@ def model_name(name) def hidden?(route, options) route_hidden = route.settings.try(:[], :swagger).try(:[], :hidden) - route_hidden = route.options[:hidden] if route.options.key?(:hidden) + hidden_val = route.hidden + route_hidden = hidden_val unless hidden_val.nil? return route_hidden unless route_hidden.is_a?(Proc) return route_hidden.call unless options[:token_owner] diff --git a/spec/lib/move_params_spec.rb b/spec/lib/move_params_spec.rb index f0528ac52..d2fad07fc 100644 --- a/spec/lib/move_params_spec.rb +++ b/spec/lib/move_params_spec.rb @@ -218,17 +218,17 @@ { name: name, in: 'body', required: true, schema: { '$ref' => "#/definitions/#{name}" } } end specify do - parameter = subject.send(:build_body_parameter, name, {}) + parameter = subject.send(:build_body_parameter, name, nil) expect(parameter).to eql expected_param end describe 'body_name option specified' do - let(:route_options) { { body_name: 'body' } } + let(:body_name) { 'body' } let(:expected_param) do - { name: route_options[:body_name], in: 'body', required: true, schema: { '$ref' => "#/definitions/#{name}" } } + { name: body_name, in: 'body', required: true, schema: { '$ref' => "#/definitions/#{name}" } } end specify do - parameter = subject.send(:build_body_parameter, name, route_options) + parameter = subject.send(:build_body_parameter, name, body_name) expect(parameter).to eql expected_param end end diff --git a/spec/swagger_v2/api_swagger_v2_detail_spec.rb b/spec/swagger_v2/api_swagger_v2_detail_spec.rb index 6e157d654..61b7961da 100644 --- a/spec/swagger_v2/api_swagger_v2_detail_spec.rb +++ b/spec/swagger_v2/api_swagger_v2_detail_spec.rb @@ -48,6 +48,12 @@ class DetailApi < Grape::API { 'declared_params' => declared(params) } end + desc 'This returns something', detail: nil, entity: Entities::UseResponse, + failure: [{ code: 400, model: Entities::ApiError }] + get '/use_detail_nil' do + { 'declared_params' => declared(params) } + end + add_swagger_documentation end end @@ -75,5 +81,14 @@ def app expect(subject['paths']['/use_detail_block']['get']).to include('description') expect(subject['paths']['/use_detail_block']['get']['description']).to eql 'detailed description of the route inside the `desc` block' end + + # Reading metadata through route readers (instead of route.options.key?(:detail)) + # means an explicit `detail: nil` is treated the same as an absent detail: + # the description holds the text and there is no separate summary. + specify do + expect(subject['paths']['/use_detail_nil']['get']).not_to include('summary') + expect(subject['paths']['/use_detail_nil']['get']).to include('description') + expect(subject['paths']['/use_detail_nil']['get']['description']).to eql 'This returns something' + end end end diff --git a/spec/swagger_v2/endpoint_versioned_path_spec.rb b/spec/swagger_v2/endpoint_versioned_path_spec.rb index 0a2d84e91..d6cc2f1b9 100644 --- a/spec/swagger_v2/endpoint_versioned_path_spec.rb +++ b/spec/swagger_v2/endpoint_versioned_path_spec.rb @@ -53,6 +53,23 @@ end end + context 'when tags are explicitly set to nil' do + let(:item) do + Class.new(Grape::API) do + version 'v1', using: :path + + resource :item do + desc 'Item description', tags: nil + get '/' + end + end + end + + it 'omits the tags key instead of falling back to the default tag' do + expect(subject.first['/v1/item'][:get]).not_to have_key(:tags) + end + end + context 'when parameter with a custom type is specified' do let(:item) do Class.new(Grape::API) do