From abfcbb6737bcc2ed58262ffd29886dbb8cf7edd7 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 8 Apr 2026 22:53:18 +0200 Subject: [PATCH 1/2] Update RuboCop to 1.86.0 and autocorrect offenses Co-Authored-By: Claude Sonnet 4.6 --- .rubocop.yml | 1 + .rubocop_todo.yml | 14 +- CHANGELOG.md | 1 + Gemfile | 2 +- lib/grape/request.rb | 4 +- lib/grape/util/stackable_values.rb | 4 +- lib/grape/validations/params_documentation.rb | 4 +- lib/grape/validations/params_scope.rb | 2 +- spec/grape/api_spec.rb | 2 +- spec/grape/dsl/desc_spec.rb | 128 ++++++++++-------- spec/grape/dsl/request_response_spec.rb | 20 +-- .../exceptions/validation_errors_spec.rb | 6 +- spec/grape/validations_spec.rb | 26 ++-- spec/support/cookie_jar.rb | 12 +- 14 files changed, 117 insertions(+), 109 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 1592efee4..6072644bb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,6 +5,7 @@ AllCops: Exclude: - vendor/**/* - bin/**/* + - benchmark/**/* plugins: - rubocop-performance diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1cdd82025..2635ef5dd 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2026-01-31 18:13:50 UTC using RuboCop version 1.84.0. +# on 2026-04-08 20:39:39 UTC using RuboCop version 1.86.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -59,7 +59,7 @@ RSpec/IndexedLet: - 'spec/grape/presenters/presenter_spec.rb' - 'spec/shared/versioning_examples.rb' -# Offense count: 40 +# Offense count: 41 # Configuration parameters: AssignmentOnly. RSpec/InstanceVariable: Exclude: @@ -84,19 +84,16 @@ RSpec/MissingExampleGroupArgument: Exclude: - 'spec/grape/middleware/exception_spec.rb' -# Offense count: 12 +# Offense count: 6 RSpec/RepeatedDescription: Exclude: - 'spec/grape/api_spec.rb' - 'spec/grape/endpoint_spec.rb' - - 'spec/grape/validations/validators/allow_blank_validator_spec.rb' - - 'spec/grape/validations/validators/values_validator_spec.rb' -# Offense count: 6 +# Offense count: 2 RSpec/RepeatedExample: Exclude: - 'spec/grape/middleware/versioner/accept_version_header_spec.rb' - - 'spec/grape/validations/validators/allow_blank_validator_spec.rb' # Offense count: 8 RSpec/RepeatedExampleGroupDescription: @@ -144,7 +141,7 @@ Style/CombinableLoops: Exclude: - 'spec/grape/endpoint_spec.rb' -# Offense count: 10 +# Offense count: 9 # Configuration parameters: AllowedMethods. # AllowedMethods: respond_to_missing? Style/OptionalBooleanParameter: @@ -152,7 +149,6 @@ Style/OptionalBooleanParameter: - 'lib/grape/dsl/parameters.rb' - 'lib/grape/endpoint.rb' - 'lib/grape/serve_stream/sendfile_response.rb' - - 'lib/grape/validations/params_scope.rb' - 'lib/grape/validations/types/array_coercer.rb' - 'lib/grape/validations/types/custom_type_collection_coercer.rb' - 'lib/grape/validations/types/dry_type_coercer.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index 86c91709c..860b833e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ #### Fixes +* [#2678](https://github.com/ruby-grape/grape/pull/2678): Update rubocop to 1.86.0 and autocorrect offenses - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.2.0 (2026-04-08) diff --git a/Gemfile b/Gemfile index 763727412..468c73fd0 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,7 @@ group :development, :test do gem 'builder', require: false gem 'bundler' gem 'rake' - gem 'rubocop', '1.84.0', require: false + gem 'rubocop', '1.86.0', require: false gem 'rubocop-performance', '1.26.1', require: false gem 'rubocop-rspec', '3.9.0', require: false end diff --git a/lib/grape/request.rb b/lib/grape/request.rb index 9831b336c..d0f4be1e2 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -132,8 +132,8 @@ class Request < Rack::Request X-UA-Compatible X-WebKit-CS X-XSS-Protection - ].each_with_object({}) do |header, response| - response["HTTP_#{header.upcase.tr('-', '_')}"] = header + ].to_h do |header| + ["HTTP_#{header.upcase.tr('-', '_')}", header] end.freeze alias rack_params params diff --git a/lib/grape/util/stackable_values.rb b/lib/grape/util/stackable_values.rb index 64336182c..5c0c2c5e3 100644 --- a/lib/grape/util/stackable_values.rb +++ b/lib/grape/util/stackable_values.rb @@ -19,8 +19,8 @@ def []=(name, value) end def to_hash - keys.each_with_object({}) do |key, result| - result[key] = self[key] + keys.to_h do |key| + [key, self[key]] end end diff --git a/lib/grape/validations/params_documentation.rb b/lib/grape/validations/params_documentation.rb index 92d50f019..0d4e94313 100644 --- a/lib/grape/validations/params_documentation.rb +++ b/lib/grape/validations/params_documentation.rb @@ -9,8 +9,8 @@ module ParamsDocumentation def document_params(attrs, validations, type = nil, values = nil, except_values = nil) return validations.except!(:desc, :description, :documentation) if @api.inheritable_setting.namespace_inheritable[:do_not_document] - documented_attrs = attrs.each_with_object({}) do |name, memo| - memo[full_name(name)] = extract_details(validations, type, values, except_values) + documented_attrs = attrs.to_h do |name| + [full_name(name), extract_details(validations, type, values, except_values)] end @api.inheritable_setting.namespace_stackable[:params] = documented_attrs end diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 335cdad6c..f285a2511 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -317,7 +317,7 @@ def configure_declared_params push_renamed_param(full_path, @element_renamed) if @element_renamed if nested? - @parent.push_declared_params [@element => @declared_params] + @parent.push_declared_params [{ @element => @declared_params }] else @api.inheritable_setting.namespace_stackable[:declared_params] = @declared_params end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index a2f374003..75f56c02c 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -119,7 +119,7 @@ dummy_presenter_klass = Class.new represent_object = Class.new subject.represent represent_object, with: dummy_presenter_klass - expect(subject.inheritable_setting.namespace_stackable[:representations]).to eq([represent_object => dummy_presenter_klass]) + expect(subject.inheritable_setting.namespace_stackable[:representations]).to eq([{ represent_object => dummy_presenter_klass }]) end end diff --git a/spec/grape/dsl/desc_spec.rb b/spec/grape/dsl/desc_spec.rb index 0741f11ca..0c1c6ae1c 100644 --- a/spec/grape/dsl/desc_spec.rb +++ b/spec/grape/dsl/desc_spec.rb @@ -19,68 +19,76 @@ expect(subject.route_setting(:description)).to eq(options.merge(description: desc_text)) end - it 'can be set with a block' do - expected_options = { - summary: 'summary', - description: 'The description', - detail: 'more details', - params: { first: :param }, - entity: Object, - default: { code: 400, message: 'Invalid' }, - http_codes: [[401, 'Unauthorized', 'Entities::Error']], - named: 'My named route', - body_name: 'My body name', - headers: [ - XAuthToken: { - description: 'Valdates your identity', - required: true - }, - XOptionalHeader: { - description: 'Not really needed', - required: false - } - ], - hidden: false, - deprecated: false, - is_array: true, - nickname: 'nickname', - produces: %w[array of mime_types], - consumes: %w[array of mime_types], - tags: %w[tag1 tag2], - security: %w[array of security schemes] - } - - subject.desc 'The description' do - summary 'summary' - detail 'more details' - params(first: :param) - success Object - default code: 400, message: 'Invalid' - failure [[401, 'Unauthorized', 'Entities::Error']] - named 'My named route' - body_name 'My body name' - headers [ - XAuthToken: { - description: 'Valdates your identity', - required: true - }, - XOptionalHeader: { - description: 'Not really needed', - required: false - } - ] - hidden false - deprecated false - is_array true - nickname 'nickname' - produces %w[array of mime_types] - consumes %w[array of mime_types] - tags %w[tag1 tag2] - security %w[array of security schemes] + context 'when a block is passed' do + let(:expected_options) do + { + summary: 'summary', + description: 'The description', + detail: 'more details', + params: { first: :param }, + entity: Object, + default: { code: 400, message: 'Invalid' }, + http_codes: [[401, 'Unauthorized', 'Entities::Error']], + named: 'My named route', + body_name: 'My body name', + headers: [ + { + XAuthToken: { + description: 'Valdates your identity', + required: true + }, + XOptionalHeader: { + description: 'Not really needed', + required: false + } + } + ], + hidden: false, + deprecated: false, + is_array: true, + nickname: 'nickname', + produces: %w[array of mime_types], + consumes: %w[array of mime_types], + tags: %w[tag1 tag2], + security: %w[array of security schemes] + } end - expect(subject.namespace_setting(:description)).to eq(expected_options) - expect(subject.route_setting(:description)).to eq(expected_options) + it 'can be set with a block' do + subject.desc 'The description' do + summary 'summary' + detail 'more details' + params(first: :param) + success Object + default code: 400, message: 'Invalid' + failure [[401, 'Unauthorized', 'Entities::Error']] + named 'My named route' + body_name 'My body name' + headers [ + { + XAuthToken: { + description: 'Valdates your identity', + required: true + }, + XOptionalHeader: { + description: 'Not really needed', + required: false + } + } + ] + hidden false + deprecated false + is_array true + nickname 'nickname' + produces %w[array of mime_types] + consumes %w[array of mime_types] + tags %w[tag1 tag2] + security %w[array of security schemes] + end + + expect(subject.namespace_setting(:description)).to eq(expected_options) + expect(subject.route_setting(:description)).to eq(expected_options) + end end end end diff --git a/spec/grape/dsl/request_response_spec.rb b/spec/grape/dsl/request_response_spec.rb index fff0bca59..b1d5f8687 100644 --- a/spec/grape/dsl/request_response_spec.rb +++ b/spec/grape/dsl/request_response_spec.rb @@ -36,14 +36,14 @@ describe '.formatter' do it 'sets the formatter for a content type' do subject.formatter c_type, :formatter - expect(subject.inheritable_setting.namespace_stackable[:formatters]).to eq([c_type.to_sym => :formatter]) + expect(subject.inheritable_setting.namespace_stackable[:formatters]).to eq([{ c_type.to_sym => :formatter }]) end end describe '.parser' do it 'sets a parser for a content type' do subject.parser c_type, :parser - expect(subject.inheritable_setting.namespace_stackable[:parsers]).to eq([c_type.to_sym => :parser]) + expect(subject.inheritable_setting.namespace_stackable[:parsers]).to eq([{ c_type.to_sym => :parser }]) end end @@ -70,7 +70,7 @@ describe '.content_type' do it 'sets a content type for a format' do subject.content_type format, c_type - expect(subject.inheritable_setting.namespace_stackable[:content_types]).to eq([format.to_sym => c_type]) + expect(subject.inheritable_setting.namespace_stackable[:content_types]).to eq([{ format.to_sym => c_type }]) end end @@ -201,34 +201,34 @@ describe 'list of exceptions is passed' do it 'sets hash of exceptions as rescue handlers' do subject.rescue_from StandardError - expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([StandardError => nil]) + expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => nil }]) expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) end it 'rescues only base handlers if rescue_subclasses: false option is passed' do subject.rescue_from StandardError, rescue_subclasses: false - expect(subject.inheritable_setting.namespace_reverse_stackable[:base_only_rescue_handlers]).to eq([StandardError => nil]) - expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([rescue_subclasses: false]) + expect(subject.inheritable_setting.namespace_reverse_stackable[:base_only_rescue_handlers]).to eq([{ StandardError => nil }]) + expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{ rescue_subclasses: false }]) end it 'sets given proc as rescue handler for each key in hash' do rescue_handler_proc = proc {} subject.rescue_from StandardError, rescue_handler_proc - expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([StandardError => rescue_handler_proc]) + expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => rescue_handler_proc }]) expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) end it 'sets given block as rescue handler for each key in hash' do rescue_handler_proc = proc {} subject.rescue_from StandardError, &rescue_handler_proc - expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([StandardError => rescue_handler_proc]) + expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => rescue_handler_proc }]) expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) end it 'sets a rescue handler declared through :with option for each key in hash' do with_block = -> { 'hello' } subject.rescue_from StandardError, with: with_block - expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([StandardError => with_block]) + expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => with_block }]) expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) end end @@ -238,7 +238,7 @@ it 'sets a presenter for a class' do presenter = Class.new subject.represent :ThisClass, with: presenter - expect(subject.inheritable_setting.namespace_stackable[:representations]).to eq([ThisClass: presenter]) + expect(subject.inheritable_setting.namespace_stackable[:representations]).to eq([{ ThisClass: presenter }]) end end end diff --git a/spec/grape/exceptions/validation_errors_spec.rb b/spec/grape/exceptions/validation_errors_spec.rb index c75930af9..921a7c18e 100644 --- a/spec/grape/exceptions/validation_errors_spec.rb +++ b/spec/grape/exceptions/validation_errors_spec.rb @@ -81,8 +81,10 @@ def app expect(last_response).to be_bad_request expect(JSON.parse(last_response.body)).to eq( [ - 'params' => %w[beer wine], - 'messages' => ['are mutually exclusive'] + { + 'params' => %w[beer wine], + 'messages' => ['are mutually exclusive'] + } ] ) end diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index 63c1e4598..f3bdc3186 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -361,9 +361,9 @@ def define_requires_none subject.get('/') do declared(params) end - env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [key: 'my_key'] }) + env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [{ key: 'my_key' }] }) response = Rack::MockResponse[*subject.call(env)] - expect(JSON.parse(response.body)).to eq('items' => ['key' => 'my_key']) + expect(JSON.parse(response.body)).to eq('items' => [{ 'key' => 'my_key' }]) end end @@ -439,9 +439,9 @@ def define_requires_none subject.get('/') do declared(params) end - env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [key: :my_key] }) + env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [{ key: :my_key }] }) response = Rack::MockResponse[*subject.call(env)] - expect(JSON.parse(response.body)).to eq('items' => ['key' => 'my_key']) + expect(JSON.parse(response.body)).to eq('items' => [{ 'key' => 'my_key' }]) end end @@ -493,7 +493,7 @@ def define_requires_none end it "doesn't throw a missing param when param is present" do - get '/required', items: [key: 'hello'] + get '/required', items: [{ key: 'hello' }] expect(last_response.status).to eq(200) expect(last_response.body).to eq('required works') end @@ -508,9 +508,9 @@ def define_requires_none subject.get('/') do declared(params) end - env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [key: :my_key] }) + env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [{ key: :my_key }] }) response = Rack::MockResponse[*subject.call(env)] - expect(JSON.parse(response.body)).to eq('items' => ['key' => 'my_key']) + expect(JSON.parse(response.body)).to eq('items' => [{ 'key' => 'my_key' }]) end end @@ -669,7 +669,7 @@ def validate_param!(attr_name, params) 'children[0][parents][0][name] is empty' ) - get '/within_array', children: [name: 'Jay'] + get '/within_array', children: [{ name: 'Jay' }] expect(last_response.status).to eq(400) expect(last_response.body).to eq('children[0][parents] is missing, children[0][parents][0][name] is missing, children[0][parents][0][name] is empty') end @@ -683,7 +683,7 @@ def validate_param!(attr_name, params) expect(last_response.status).to eq(400) expect(last_response.body).to eq('children is invalid') - get '/within_array', children: [name: 'Jay', parents: { name: 'Fred' }] + get '/within_array', children: [{ name: 'Jay', parents: { name: 'Fred' } }] expect(last_response.status).to eq(400) expect(last_response.body).to eq('children[0][parents] is invalid') end @@ -817,7 +817,7 @@ def validate_param!(attr_name, params) it 'safely handles empty arrays and blank parameters' do put_with_json '/within_array', children: [] expect(last_response.status).to eq(200) - put_with_json '/within_array', children: [name: 'Jay'] + put_with_json '/within_array', children: [{ name: 'Jay' }] expect(last_response.status).to eq(400) expect(last_response.body).to eq('children[0][parents] is missing, children[0][parents][0][name] is missing') end @@ -873,9 +873,9 @@ def validate_param!(attr_name, params) subject.get('/') do declared(params) end - env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [key: :my_key] }) + env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [{ key: :my_key }] }) response = Rack::MockResponse[*subject.call(env)] - expect(JSON.parse(response.body)).to eq('items' => ['key' => 'my_key']) + expect(JSON.parse(response.body)).to eq('items' => [{ 'key' => 'my_key' }]) end end @@ -943,7 +943,7 @@ def validate_param!(attr_name, params) subject.get('/') do declared(params) end - env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [{ key: :my_key, required_subitems: [value: 'my_value'] }] }) + env = Rack::MockRequest.env_for('/', method: Rack::GET, params: { items: [{ key: :my_key, required_subitems: [{ value: 'my_value' }] }] }) response = Rack::MockResponse[*subject.call(env)] expect(JSON.parse(response.body)).to eq('items' => [{ 'key' => 'my_key', 'optional_subitems' => [], 'required_subitems' => [{ 'value' => 'my_value' }] }]) end diff --git a/spec/support/cookie_jar.rb b/spec/support/cookie_jar.rb index 64d684878..b9b943c5b 100644 --- a/spec/support/cookie_jar.rb +++ b/spec/support/cookie_jar.rb @@ -45,13 +45,13 @@ def parse_value(attribute, value) end end end - end -end -module Rack - class MockResponse - def cookie_jar - @cookie_jar ||= Array(headers[Rack::SET_COOKIE]).flat_map { |h| h.split("\n") }.map { |c| Spec::Support::CookieJar.new(c).to_h } + module MockResponseCookieJar + def cookie_jar + @cookie_jar ||= Array(headers[Rack::SET_COOKIE]).flat_map { |h| h.split("\n") }.map { |c| Spec::Support::CookieJar.new(c).to_h } + end end end end + +Rack::MockResponse.prepend(Spec::Support::MockResponseCookieJar) From 96e15890c9b85529a9bea4166d08d348737b5880 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 8 Apr 2026 23:04:36 +0200 Subject: [PATCH 2/2] Extract Entity DSL and refactor :with to keyword argument - Move entity-related methods (present, entity_class_for_obj, entity_representation_for) from InsideRoute into a new Grape::DSL::Entity module - Convert options[:with] hash pattern to explicit `with:` keyword argument in rescue_from, represent Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 1 + lib/grape/dsl/entity.rb | 85 +++++++++++++++++++++++++++ lib/grape/dsl/inside_route.rb | 82 +------------------------- lib/grape/dsl/request_response.rb | 40 +++++-------- lib/grape/error_formatter/base.rb | 25 ++++---- lib/grape/validations/params_scope.rb | 7 --- spec/grape/api_spec.rb | 2 +- spec/grape/dsl/parameters_spec.rb | 7 --- spec/grape/middleware/error_spec.rb | 10 ---- 9 files changed, 114 insertions(+), 145 deletions(-) create mode 100644 lib/grape/dsl/entity.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 860b833e2..5af9a62fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#2679](https://github.com/ruby-grape/grape/pull/2679): Extract entity dsl and refactor :with to keyword argument - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/dsl/entity.rb b/lib/grape/dsl/entity.rb new file mode 100644 index 000000000..fec11c1a9 --- /dev/null +++ b/lib/grape/dsl/entity.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Grape + module DSL + module Entity + # Allows you to make use of Grape Entities by setting + # the response body to the serializable hash of the + # entity provided in the `:with` option. This has the + # added benefit of automatically passing along environment + # and version information to the serialization, making it + # very easy to do conditional exposures. See Entity docs + # for more info. + # + # @param args [Array] either `(object)` or `(key, object)` where key is a Symbol + # used to nest the representation under that key in the response body. + # @param root [Symbol, String, nil] wraps the representation under this root key. + # @param with [Class, nil] the entity class to use for representation. + # If omitted, the entity class is inferred from the object via {#entity_class_for_obj}. + # @param options [Hash] additional options forwarded to the entity's `represent` call. + # + # @example + # + # get '/users/:id' do + # present User.find(params[:id]), + # with: API::Entities::User, + # admin: current_user.admin? + # end + def present(*args, root: nil, with: nil, **options) + key, object = args.count == 2 && args.first.is_a?(Symbol) ? args : [nil, args.first] + entity_class = with || entity_class_for_obj(object) + representation = entity_class ? entity_representation_for(entity_class, object, options) : object + representation = { root => representation } if root + + if key + representation = (body || {}).merge(key => representation) + elsif entity_class.present? && body + raise ArgumentError, "Representation of type #{representation.class} cannot be merged." unless representation.respond_to?(:merge) + + representation = body.merge(representation) + end + + body representation + end + + # Attempt to locate the Entity class for a given object, if not given + # explicitly. This is done by looking for the presence of Klass::Entity, + # where Klass is the class of the `object` parameter, or one of its + # ancestors. + # @param object [Object] the object to locate the Entity class for + # @return [Class] the located Entity class, or nil if none is found + def entity_class_for_obj(object) + object_class = + if object.respond_to?(:klass) + object.klass + elsif object.respond_to?(:first) + object.first.class + else + object.class + end + + representations = inheritable_setting.namespace_stackable_with_hash(:representations) + if representations + potential = object_class.ancestors.detect { |potential| representations.key?(potential) } + return representations[potential] if potential && representations[potential] + end + + return unless object_class.const_defined?(:Entity) + + entity = object_class.const_get(:Entity) + entity if entity.respond_to?(:represent) + end + + private + + # @param entity_class [Class] the entity class to use for representation. + # @param object [Object] the object to represent. + # @param options [Hash] additional options forwarded to the entity's `represent` call. + # @return the representation of the given object as done through the given entity_class. + def entity_representation_for(entity_class, object, options) + embeds = env.key?(Grape::Env::API_VERSION) ? { env:, version: env[Grape::Env::API_VERSION] } : { env: } + entity_class.represent(object, **embeds, **options) + end + end + end +end diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index fff856884..7e9ec208a 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -4,6 +4,7 @@ module Grape module DSL module InsideRoute include Declared + include Entity # Backward compatibility: alias exception class to previous location MethodNotYetAvailable = Declared::MethodNotYetAvailable @@ -184,50 +185,6 @@ def stream(value = nil) end end - # Allows you to make use of Grape Entities by setting - # the response body to the serializable hash of the - # entity provided in the `:with` option. This has the - # added benefit of automatically passing along environment - # and version information to the serialization, making it - # very easy to do conditional exposures. See Entity docs - # for more info. - # - # @example - # - # get '/users/:id' do - # present User.find(params[:id]), - # with: API::Entities::User, - # admin: current_user.admin? - # end - def present(*args, **options) - key, object = if args.count == 2 && args.first.is_a?(Symbol) - args - else - [nil, args.first] - end - entity_class = entity_class_for_obj(object, options) - - root = options.delete(:root) - - representation = if entity_class - entity_representation_for(entity_class, object, options) - else - object - end - - representation = { root => representation } if root - - if key - representation = (body || {}).merge(key => representation) - elsif entity_class.present? && body - raise ArgumentError, "Representation of type #{representation.class} cannot be merged." unless representation.respond_to?(:merge) - - representation = body.merge(representation) - end - - body representation - end - # Returns route information for the current request. # # @example @@ -240,43 +197,6 @@ def route env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info] end - # Attempt to locate the Entity class for a given object, if not given - # explicitly. This is done by looking for the presence of Klass::Entity, - # where Klass is the class of the `object` parameter, or one of its - # ancestors. - # @param object [Object] the object to locate the Entity class for - # @param options [Hash] - # @option options :with [Class] the explicit entity class to use - # @return [Class] the located Entity class, or nil if none is found - def entity_class_for_obj(object, options) - entity_class = options.delete(:with) - return entity_class if entity_class - - # entity class not explicitly defined, auto-detect from relation#klass or first object in the collection - object_class = if object.respond_to?(:klass) - object.klass - else - object.respond_to?(:first) ? object.first.class : object.class - end - - representations = inheritable_setting.namespace_stackable_with_hash(:representations) - if representations - potential = object_class.ancestors.detect { |potential| representations.key?(potential) } - entity_class = representations[potential] if potential - end - - entity_class = object_class.const_get(:Entity) if !entity_class && object_class.const_defined?(:Entity) && object_class.const_get(:Entity).respond_to?(:represent) - entity_class - end - - # @return the representation of the given object as done through - # the given entity_class. - def entity_representation_for(entity_class, object, options) - embeds = { env: } - embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION) - entity_class.represent(object, **embeds, **options) - end - def http_version env.fetch('HTTP_VERSION') { env[Rack::SERVER_PROTOCOL] } end diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb index 8956db2f3..b1a5e34d2 100644 --- a/lib/grape/dsl/request_response.rb +++ b/lib/grape/dsl/request_response.rb @@ -44,13 +44,8 @@ def default_error_formatter(new_formatter_name = nil) inheritable_setting.namespace_inheritable[:default_error_formatter] = new_formatter end - def error_formatter(format, options) - formatter = if options.is_a?(Hash) && options.key?(:with) - options[:with] - else - options - end - + def error_formatter(format, options = nil, with: nil) + formatter = with || options inheritable_setting.namespace_stackable[:error_formatters] = { format.to_sym => formatter } end @@ -93,16 +88,8 @@ def default_error_status(new_status = nil) # @option options [Boolean] :rescue_subclasses Also rescue subclasses of exception classes # @param [Proc] handler Execution proc to handle the given exception as an # alternative to passing a block. - def rescue_from(*args, **options, &block) - if args.last.is_a?(Proc) - handler = args.pop - elsif block - handler = block - end - - raise ArgumentError, 'both :with option and block cannot be passed' if block && options.key?(:with) - - handler ||= extract_with(options) + def rescue_from(*args, with: nil, **options, &block) + handler = extract_handler(args, with:, block:) if args.include?(:all) inheritable_setting.namespace_inheritable[:rescue_all] = true @@ -146,22 +133,23 @@ def rescue_from(*args, **options, &block) # # @param model_class [Class] The model class that will be represented. # @option options [Class] :with The entity class that will represent the model. - def represent(model_class, options) - raise Grape::Exceptions::InvalidWithOptionForRepresent.new unless options[:with].is_a?(Class) + def represent(model_class, with:) + raise Grape::Exceptions::InvalidWithOptionForRepresent.new unless with.is_a?(Class) - inheritable_setting.namespace_stackable[:representations] = { model_class => options[:with] } + inheritable_setting.namespace_stackable[:representations] = { model_class => with } end private - def extract_with(options) - return unless options.key?(:with) + def extract_handler(args, with:, block:) + raise ArgumentError, 'both :with option and block cannot be passed' if block && with - with_option = options.delete(:with) - return with_option if with_option.instance_of?(Proc) - return with_option.to_sym if with_option.instance_of?(Symbol) || with_option.instance_of?(String) + return args.pop if args.last.is_a?(Proc) + return block if block + return with if with.instance_of?(Proc) || with.instance_of?(Symbol) + return with.to_sym if with.instance_of?(String) - raise ArgumentError, "with: #{with_option.class}, expected Symbol, String or Proc" + raise ArgumentError, "with: #{with.class}, expected Symbol, String or Proc" if with end end end diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb index 3e16429aa..1bbb48ff0 100644 --- a/lib/grape/error_formatter/base.rb +++ b/lib/grape/error_formatter/base.rb @@ -18,20 +18,21 @@ def call(message, backtrace, options = {}, env = nil, original_exception = nil) end def present(message, env) - present_options = {} - presented_message = message - if presented_message.is_a?(Hash) - presented_message = presented_message.dup - present_options[:with] = presented_message.delete(:with) + # error! accepts a message hash with an optional :with key specifying the entity presenter. + # Extract it here so the presenter can be resolved and the key is not serialized in the response. + # See spec/integration/grape_entity/entity_spec.rb for examples. + with = nil + if message.is_a?(Hash) && message.key?(:with) + message = message.dup + with = message.delete(:with) end - presenter = env[Grape::Env::API_ENDPOINT].entity_class_for_obj(presented_message, present_options) + presenter = with || env[Grape::Env::API_ENDPOINT].entity_class_for_obj(message) unless presenter || env[Grape::Env::GRAPE_ROUTING_ARGS].nil? # env['api.endpoint'].route does not work when the error occurs within a middleware # the Endpoint does not have a valid env at this moment http_codes = env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info].http_codes || [] - found_code = http_codes.find do |http_code| (http_code[0].to_i == env[Grape::Env::API_ENDPOINT].status) && http_code[2].respond_to?(:represent) end if env[Grape::Env::API_ENDPOINT].request @@ -39,13 +40,11 @@ def present(message, env) presenter = found_code[2] if found_code end - if presenter - embeds = { env: } - embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION) - presented_message = presenter.represent(presented_message, embeds).serializable_hash - end + return message unless presenter - presented_message + embeds = { env: } + embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION) + presenter.represent(message, embeds).serializable_hash end def wrap_message(message) diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index f285a2511..0a65c5622 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -492,13 +492,6 @@ def validate_value_coercion(coerce_type, *values_list) end end - def extract_message_option(attrs) - return nil unless attrs.is_a?(Array) - - opts = attrs.last.is_a?(Hash) ? attrs.pop : {} - opts.key?(:message) && !opts[:message].nil? ? opts.delete(:message) : nil - end - def options_key?(type, key, validations) validations[type].respond_to?(:key?) && validations[type].key?(key) && !validations[type][key].nil? end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 75f56c02c..c844d3906 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -112,7 +112,7 @@ describe '.represent' do it 'requires a :with option' do - expect { subject.represent Object, {} }.to raise_error(Grape::Exceptions::InvalidWithOptionForRepresent) + expect { subject.represent Object, with: 1 }.to raise_error(Grape::Exceptions::InvalidWithOptionForRepresent) end it 'adds the association to the :representations setting' do diff --git a/spec/grape/dsl/parameters_spec.rb b/spec/grape/dsl/parameters_spec.rb index 33eea9a66..90e4a0905 100644 --- a/spec/grape/dsl/parameters_spec.rb +++ b/spec/grape/dsl/parameters_spec.rb @@ -49,13 +49,6 @@ def new_group_scope(group) yield @group = prev_group end - - def extract_message_option(attrs) - return nil unless attrs.is_a?(Array) - - opts = attrs.last.is_a?(Hash) ? attrs.pop : {} - opts.key?(:message) && !opts[:message].nil? ? opts.delete(:message) : nil - end end end diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index 88eb88725..e12a76156 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -1,16 +1,6 @@ # frozen_string_literal: true describe Grape::Middleware::Error do - let(:error_entity) do - Class.new(Grape::Entity) do - expose :code - expose :static - - def static - 'static text' - end - end - end let(:err_app) do Class.new do class << self