Changes to rails3_acts_as_paranoid for Rails 3.1 #46

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

softcraft-development commented Dec 7, 2011

I'm hacking around in rails3_acts_as_paranoid. In the process of making my own changes (which I hope to fill you in on later), I found the errors (mentioned here) caused by using the Rails 3.1 version of ActiveRecord... and managed to fix them.

There's only 3 changes of real importance:

  • Rename the call in uniqueness_without_deleted.rb from mount_sql_and_params to build_relation
  • In the same call, pass an Arel::Table instead of a String table name.
  • Remove the aliasing of primary_key_name to foreign_key in ActiveRecord::Reflection::AssociationReflection

Feel free to ignore/revert my changes to the Gemspec; those are mostly for my own internal purpose.

Hope this helps,

Craig

softcraft-development added some commits Dec 6, 2011

@softcraft-development softcraft-development Updated gemspec for forked version 4e328ea
@softcraft-development softcraft-development rolled gemspec version number f7ec28c
@softcraft-development softcraft-development Updated rails to 3.1 d285a89
@softcraft-development softcraft-development switched call from mount_sql_and_params to build_relation
Looks like the method was renamed in rails/rails@a30b440#activerecord/lib/active_record/validations/uniqueness.rb

I Googled up the reference from here: goncalossilva#39
5b57d70
@softcraft-development softcraft-development Switched from string table name to Arel::Table for build_relation
Was getting this exception:

  1) Error:
test_should_validate_without_deleted(ValidatesUniquenessTest):
TypeError: can't convert Symbol into Integer
    /Users/craig/.rvm/gems/ruby-1.9.2-p0@paranoid/gems/activerecord-3.1.3/lib/active_record/validations/uniqueness.rb:65:in `[]'
    /Users/craig/.rvm/gems/ruby-1.9.2-p0@paranoid/gems/activerecord-3.1.3/lib/active_record/validations/uniqueness.rb:65:in `build_relation'
    /Users/craig/Sites/rails3_acts_as_paranoid/lib/validations/uniqueness_without_deleted.rb:13:in `validate_each'
    ...

In addition to the rename, build_relation is now also expecting a Arel::Table.
It then extracts the field using table[attribute].
When table was a String, the dereferencing was expecting an integer array index.
040c0c8
@softcraft-development softcraft-development Removed aliasing of primary_key_name to foreign_key
Prior to this commit, I was getting this message repeatedly during an infinite recursion:

  DEPRECATION WARNING: primary_key_name is deprecated and will be removed from Rails 3.2 (use foreign_key instead). (called from block in destroy at /Users/craig/Sites/rails3_acts_as_paranoid/lib/rails3_acts_as_paranoid.rb:125)

...and then the StackTraceToDeep error, if I let it get that far.

I didn't dig to deep into it, but it looks like goncalossilva et al
had roughly the same idea as the Rails 3.1 team, and made primary_key_name
call foreign_key; with the alias this caused an infinite recursion.

In any case, all of the tests pass fine with this alias removed.
819f99a
Owner

goncalossilva commented Dec 9, 2011

I haven't put much thought to it, but I wonder how easy it would be to have one single gem for both versions (3.0 and 3.1) without a bunch of if statements.

Contributor

softcraft-development commented Dec 9, 2011

As it happens, I just spent a bunch of time on reworking some other changes from 3.0 to 3.1. The changes were pretty significant... the whole AR class structure changed dramatically in 3.1.

So, from my perspective at least, the answer would be "no".

Now, my major changes aren't necessarily good to pull into your main repo, as they take a bit of a functional detour. I was planning on sending you a full explanation later. In the meantime though, you might be interested in a sneak preview.

Owner

goncalossilva commented Dec 9, 2011

Thanks for the link, I'll take a look... and I'll wait for your explanation later, when you find the time.

I'm thinking I could separate 3.0/3.1-specific code into modules. Then it would be a single conditional statement.

When using Rails 3.1 in conjunction with the latest version 0.1.3 of rails3_acts_as_paranoid, I encountered the same issue which the software-development fork commit Removed aliasing of primary_key_name to foreign_key deals with ie.

a multitude of these log messages:

DEPRECATION WARNING: primary_key_name is deprecated and will be removed from Rails 3.2 (use foreign_key instead).

eventually resulting in:

SystemStackError (stack level too deep)

I've partly added this comment as it took a while to track down that rails3_acts_as_paranoid was the culprit as the software-development fork commit comment didn't show up in google. I can see that the software-development fork has catered for a number of Rails 3.1 changes, however, in this particular instance it may be possible to cater for both Rails 3.0 and 3.1 by using the following:

ActiveRecord::Reflection::AssociationReflection.class_eval do
  alias_method :foreign_key, :primary_key_name unless instance_methods.include?(:foreign_key)
end

cure commented on 819f99a Feb 7, 2012

Please commit this. rails3_acts_as_paranoid is useless on rails 3.1+ - it gets into an infinite loop on any update without alias_method :foreign_key removed.

Owner

goncalossilva commented Mar 6, 2012

Merged this from the other PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment