cancancan icon indicating copy to clipboard operation
cancancan copied to clipboard

Polymorphic associations do not support computing the class

Open chubchenko opened this issue 3 years ago • 5 comments

Steps to reproduce

https://gist.github.com/chubchenko/883eb5a7c49b194a223be3843774670f

Expected behavior

Does not raise an exception ArgumentError: Polymorphic associations do not support computing the class

Actual behavior

Raises an exception:

Error:
CommentsControllerTest#test_action:
ArgumentError: Polymorphic associations do not support computing the class.
    activerecord-7.0.3/lib/active_record/reflection.rb:423:in `compute_class'
    activerecord-7.0.3/lib/active_record/reflection.rb:382:in `klass'
    cancancan-3.4.0/lib/cancan/model_adapters/active_record_adapter.rb:42:in `block in parent_child_conditions'
    cancancan-3.4.0/lib/cancan/model_adapters/active_record_adapter.rb:41:in `each'
    cancancan-3.4.0/lib/cancan/model_adapters/active_record_adapter.rb:41:in `find'
    cancancan-3.4.0/lib/cancan/model_adapters/active_record_adapter.rb:41:in `parent_child_conditions'
    cancancan-3.4.0/lib/cancan/model_adapters/active_record_adapter.rb:27:in `override_nested_subject_conditions_matching?'
    cancancan-3.4.0/lib/cancan/conditions_matcher.rb:44:in `nested_subject_matches_conditions?'
    cancancan-3.4.0/lib/cancan/conditions_matcher.rb:28:in `matches_non_block_conditions'
...

System configuration

Rails version: 7.0.3

Ruby version: 3.1.2

CanCanCan version 3.4.0

chubchenko avatar Jun 29 '22 10:06 chubchenko

The error occurs in this line in the #parent_child_conditions method during iteration though reflect_on_all_associations(:belongs_to).

If change the order of the association definition from:

class Comment < ActiveRecord::Base
  belongs_to :commentable, polymorphic: true
  belongs_to :user
end

to:

class Comment < ActiveRecord::Base
  belongs_to :user
  belongs_to :commentable, polymorphic: true
end

the issue will be resolved but it's just a temporary life hack 😄

chubchenko avatar Jun 29 '22 10:06 chubchenko

This will only fix the issue if you check user attribute in your case. But if you check Comment model field (can :read, Comment, private: true), it will fail in any case

gryphon avatar Jul 18 '22 20:07 gryphon

I got this error with the devise_invitable gem which adds the belongs_to :invited_by, polymorphic: true association in Rails 6.1. Downgrading to Cancancan 3.3.0 solved it for me.

javinto avatar Jul 28 '22 08:07 javinto

The issue seems to be caused by this change: https://github.com/CanCanCommunity/cancancan/commit/d0ea89a2b762e6facf1db59dc23cc8a62ceab0a9#diff-468c7fa6110fedf2a4ae5259ca4d209daea99810746321092dac9cf373b87fdd

mobilutz avatar Aug 08 '22 11:08 mobilutz

I have the same issue, downgraded for now to 3.3.0

martijn10kb avatar Aug 16 '22 13:08 martijn10kb

The issue seems to be caused by this change: d0ea89a#diff-468c7fa6110fedf2a4ae5259ca4d209daea99810746321092dac9cf373b87fdd

Can confirm, the new offending behaviour is:

def parent_child_conditions(parent, child, all_conditions)
  child_class = child.is_a?(Class) ? child : child.class
  foreign_key = child_class.reflect_on_all_associations(:belongs_to).find do |association|
                  association.klass == parent.class
                end&.foreign_key&.to_sym
  foreign_key.nil? ? nil : all_conditions[foreign_key]
end

For ActiveRecord::Reflection::BelongsToReflection, 'klass' cannot be calculated for polymorphic associations so if one is utilised on the child class, the finder will error. As 'find' halts on the first instance, the temporary fix above to reoder polymorphic associations solves the issue for some.

Suggestion would be to add 'unless association.polymorphic?' to the finder to ensure polymorphic associations are ignored.

bowtiesolutions avatar Nov 03 '22 22:11 bowtiesolutions

Any luck with this? I am currently facing this issue and not finding a good work around so far.

WriterZephos avatar Jan 23 '23 20:01 WriterZephos

Unfortunately I did not implement any Work around except for downgrading to version 3.3.0.

However, with a little experimenting I found some workaround for my situation, although not ideal.

My original Ability code was:

can [:read, :update, :destroy], User, account_id: current_user.account_id

So, this one triggers the above exception with version 3.4.0.

I can rewrite it to

can [:read, :update, :destroy], User do |user|
  user.account_id==current_user.account_id
end

Now the exception is gone away and the right authorization is applied to the individual records BUT there is one drawback:

now the :read does not apply to the listing anymore. In other words: @users gives me all users and not just that of the account they belong to. So, I should apply an extra filter in my controller to make it work. But that could be acceptable.

Hopes this helps for your situation.

javinto avatar Jan 24 '23 08:01 javinto

I ran into this same issue and I think I can explain why changing the order of the association works, but I don't know what the long term fix is.

The issue seems to stem from lib/cancan/model_adapters/active_record_adapter.rb#parent_child_conditions. Look at this snippet of code:

def parent_child_conditions(parent, child, all_conditions)
  ...
  foreign_key = child_class.reflect_on_all_associations(:belongs_to).find do |association|
    association.klass == parent.class
  end&.foreign_key&.to_sym
  ...
end

When you do something like load_and_authorize_resource through: :current_user, it eventually ends up calling this method. This method loops through all your belongs_to associations on the resource you're trying to use.

On each of the belongs_to associations, it calls association.klass and sees if it matches the parent class you're trying to load through.

The problem is this https://github.com/rails/rails/commit/fb86ecd604a362355827bbf05776b847b0ded9a5 doesn't allow you to call association.klass on polymorphic associations. So when it hits that association, the app throws an exception.

The reason the code will work if you re-arrange your associations, is that if the association you're dealing with happens to be near the top and matches BEFORE it hits a polymorphic association, the loop stops. I'm assuming .find just stops after it's found a match. So you never hit the error of calling .klass on a polymorphic association.

So re-arranging your associations may work - but it's quite brittle and won't work in a lot of cases.

Long term fix would be to update this loop code to handle polymorphic associations.

caseyli avatar Jan 24 '23 18:01 caseyli

@caseyli please see my PR linked above. It does just as you suggest and adds support for polymorphic associations.

WriterZephos avatar Jan 31 '23 03:01 WriterZephos

Here it is for convenience: https://github.com/CanCanCommunity/cancancan/pull/814

WriterZephos avatar Jan 31 '23 03:01 WriterZephos

Closed on 3.5.0. Thank you!

coorasse avatar Mar 05 '23 14:03 coorasse