Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3 is breaking existing behavior #342

Closed
kapso opened this issue Oct 11, 2023 · 6 comments · Fixed by #344
Closed

v3 is breaking existing behavior #342

kapso opened this issue Oct 11, 2023 · 6 comments · Fixed by #344
Labels
bug Something isn't working

Comments

@kapso
Copy link

kapso commented Oct 11, 2023

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'
@kapso kapso added the bug Something isn't working label Oct 11, 2023
@atcruice
Copy link

atcruice commented Oct 12, 2023

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

  1. Alba now prefers methods on the resource, Prefer resource method #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
Copy link
Owner

@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.

@okuramasafumi
Copy link
Owner

@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.

@remy727
Copy link

remy727 commented Oct 12, 2023

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
Copy link
Owner

@remy727 Could you submit another issue with reproduction code?

@remy727
Copy link

remy727 commented Oct 12, 2023

@okuramasafumi sure. I created #343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants