okuramasafumi/alba

v3 is breaking existing behavior

kapso opened this issue · 6 comments

kapso commented

We have an attribute params in our serializer, which has started breaking after v3 upgrade.

class NotificationSerializer < BaseSerializer
  attributes :id
  attributes :params # This causes an error in v3
  # attribute :params, &:params # This works in v3
end

Error

/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/alba-3.0.0/lib/alba/resource.rb:268:in `_fetch_attribute_from_resource_first': wrong number of arguments (given 1, expected 0) (ArgumentError)

        __send__(attribute, obj)

I am guessing params is also an internal method, so its not a big deal, but I just wanted to flag it here.

Environment

  • Ruby version: 3.2.2
  • gem 'alba', '3.0.0'
  • gem 'rails', '7.0.8'
atcruice commented

Encountering a similar issue. Two things contribute to the problem:

  1. Alba now prefers methods on the resource, #323
  2. Alba uses __send__ to dispatch the attribute call, which is capable of hitting private methods

Our scenario

We have a Rails model with a format column. Ruby objects have a private format method thanks to Kernel#format.
To demonstrate all objects knowledge of private format method

> Object.new.format("%.2f", 1) # private method `format' called for #<Object:0x0000000103fce3e8> (NoMethodError)
> Object.new.__send__(:format, "%.2f", 1) # => "1.00"

So the corresponding Alba::Resource understands the format call. Our particular failure is that our ActiveRecord object can't be coerced into the format string expected as the first argument of Kernel#format.

I'm assuming Alba uses BasicObject#__send__ for a good reason (instead of Object#public_send). So in our case the workaround is to leverage prefer_object_method! on this particular resource.

okuramasafumi commented

@kapso As @atcruice mentioned, using prefer_object_method! is a quick workaround for this issue. However, I don't think it's an ideal solution.
Methods Alba inherits from Object class should not be treated as "resource method". In other words, only methods explicitly defined in the resource class should be "resource methods". That's possible with Ruby's method_added hook, so I'll implement it soon.

  • 👍2
okuramasafumi commented

@atcruice The reason behind using public instead of public_send was performance. public is faster than public_send.
So does using public_send solve the problem? For format method, yes. For params, no because it's a public method.
So, although it's a good idea to use public_send instead of __send__ here, some problems still remain.

  • 👍2
remy727 commented

I noticed v3 is not working for boolean field. I confirmed it is working on v2.4.
For boolean field, I got the follow error. Subscription is model.

TypeError: no implicit conversion of Subscription into Integer
okuramasafumi commented

@remy727 Could you submit another issue with reproduction code?

  • 👍1
remy727 commented

@okuramasafumi sure. I created #343

  • 👍1