From e70e85801be6b828bf21a60986063dea2aeb6041 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 13:25:29 +0200 Subject: [PATCH] Look up an entity by the object's own class before its element's Grape::DSL::Entity#object_class decided which class to look an entity up for by duck-typing: return object.klass if object.respond_to?(:klass) return object.first.class if object.respond_to?(:first) object.class Both tests are answered by plenty of single objects, and the object's own class was consulted only last. A Struct is Enumerable, so it responds to #first; so does any model that includes Enumerable; and #klass is hardly exclusive to ActiveRecord::Relation. For all of those, `represent Model, with: Entity` was silently ignored and the entity for whatever #first returned was looked up instead -- usually nothing, so the raw object was serialized. Try the object's own class first and fall back to the collection or wrapped class only when that comes up empty. An Array still resolves through its element class, since Array itself has no entity, and a relation still resolves through #klass. Deferring the fallback also stops #first from being called at all when the object resolves on its own class, which matters for anything where taking the first element is expensive or has side effects. Longstanding: the duck-typing dates to 2014 (v0.10.0) and behaves the same way in 3.3.4. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + lib/grape/dsl/entity.rb | 44 +++++++++++++++++-------- spec/grape/api_spec.rb | 72 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0049465b9..c84e8a119 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ * [#2827](https://github.com/ruby-grape/grape/pull/2827): Make the `cascade` DSL getter return the configured value (`cascade false` read back as `true`) - [@ericproulx](https://github.com/ericproulx). * [#2829](https://github.com/ruby-grape/grape/pull/2829): Fix a cascading route handing over only to the last route registered for the path, making a middle version (3+ mounted versions with a catch-all) answer 406 - [@ericproulx](https://github.com/ericproulx). * [#2826](https://github.com/ruby-grape/grape/pull/2826): Fix `api.version` not being set for the root route of a path-versioned API (`GET /v1`) - [@ericproulx](https://github.com/ericproulx). +* [#2844](https://github.com/ruby-grape/grape/pull/2844): Look an entity up by the presented object's own class before treating it as a collection, so `represent` is no longer skipped for models that respond to `#first` or `#klass` - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/dsl/entity.rb b/lib/grape/dsl/entity.rb index 9dbb42d99..735753a16 100644 --- a/lib/grape/dsl/entity.rb +++ b/lib/grape/dsl/entity.rb @@ -50,11 +50,39 @@ def present(*args, root: nil, with: nil, **options) # @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) - klass = object_class(object) + entity_for_class(object.class) || entity_for_class(element_class(object)) + end + + private + + # The class standing in for a collection or wrapper: ActiveRecord::Relation + # and the like expose #klass, anything else falls back to the class of its + # first element. + # + # Consulted only once the object's own class has come up empty, because + # both tests are duck-typed and plenty of single objects answer them — + # a Struct is Enumerable, so it responds to #first, and so does any model + # that includes Enumerable. Asking this first meant `represent Model, + # with: Entity` was silently ignored for those, the entity for the + # *element* type being looked up instead. Deferring it also keeps #first + # from being called at all when the object resolves on its own class. + # + # @param object [Object] the object to represent. + # @return [Class, nil] + def element_class(object) + return object.klass if object.respond_to?(:klass) + + object.first.class if object.respond_to?(:first) + end + + # @param klass [Class, nil] the class to look an entity up for. + # @return [Class, nil] the registered or conventionally named entity. + def entity_for_class(klass) + return if klass.nil? representations = inheritable_setting.representations if representations - potential = klass.ancestors.detect { |potential| representations.key?(potential) } + potential = klass.ancestors.detect { |ancestor| representations.key?(ancestor) } return representations[potential] if potential && representations[potential] end @@ -65,18 +93,6 @@ def entity_class_for_obj(object) entity if entity.respond_to?(:represent) end - private - - # Resolves the class used to look up the Entity for +object+. - # @param object [Object] the object to represent. - # @return [Class] the object's collection element class, wrapped class, or its own class. - def object_class(object) - return object.klass if object.respond_to?(:klass) - return object.first.class if object.respond_to?(:first) - - object.class - end - # @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. diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ff3454f15..f23e84fbd 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -177,6 +177,78 @@ subject.represent represent_object, with: dummy_presenter_klass expect(subject.inheritable_setting.representations).to eq(represent_object => dummy_presenter_klass) end + + # Both the collection tests are duck-typed, and plenty of single objects + # answer them, so a registered entity used to be skipped for those in favour + # of the entity for whatever #first returned. + context 'when the presented object also looks like a collection' do + let(:entity) do + Class.new do + def self.represent(object, **) + { presented: object.class.name.to_s } + end + end + end + + def present_with(api, model, object, entity) + api.format :json + api.represent model, with: entity + api.get('/') { present object } + end + + it 'uses the entity registered for a Struct' do + model = Struct.new(:name) + present_with(subject, model, model.new('x'), entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => model.name.to_s) + end + + it 'uses the entity registered for an Enumerable model' do + model = Class.new do + include Enumerable + + def each(&) = [1, 2].each(&) + end + present_with(subject, model, model.new, entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => model.name.to_s) + end + + it 'uses the entity registered for an object exposing #klass' do + model = Class.new do + def klass = String + end + present_with(subject, model, model.new, entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => model.name.to_s) + end + + it 'still resolves an array through its element class' do + model = Class.new + present_with(subject, model, [model.new], entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => 'Array') + end + + # A relation resolves through #klass, so #first must not be reached. + it 'does not call #first when the object resolves without it' do + model = Class.new + relation = Class.new do + def initialize(klass) = (@klass = klass) + attr_reader :klass + + def first = raise('#first should not have been called') + end + present_with(subject, model, relation.new(model), entity) + + expect { get '/' }.not_to raise_error + expect(last_response.status).to eq(200) + end + end end describe '.namespace' do