Namespaced models and constant loading

rubyonrails-core
1

So i've got this namespaced model like this:

   module Shop      class Cart < ActiveRecord::Base        has_many :cart_items, :dependent => :delete_all      end      class CartItem < ActiveRecord::Base; end    end

If I try to destroy a cart I get the error "Shop is not missing constant CartItem" which comes from the load_missing_constant method in Dependencies. The configure_for_dependency method (in ActiveRecord::Associations) sets up a before_destroy block callback using just CartItem as the class name. When the callback is triggered ruby obviously fires const_missing(CartItem) on Shop::Cart which can't find 'shop/cart/cart_item.rb' and then tries to load the missing constant in the parent (Shop) which then triggers the exception because it's already loaded.

Two questions:

1. What would you consider is the bug(s) - if any? 2. How would you prefer the bug(s) (if any) to be fixed?

My thoughts are:

On a simple level you could consider it a bug that the association callback (and name_for_class_name in reflection.rb) don't take account of the parent module but fixing it in AR could be messy if you need to reference top level models from inside your module. So my personal opinion is that the bug must be in load_missing_constant.

If I comment out the line where exception is raised it actually works okay so maybe the exception is unnecessary? However looking at the code in dependencies.rb I'm not sure that something else is wrong - for example Class.const_missing calls load_missing_constant and then calls parent.const_missing on a NameError exception, however load_missing_constant also calls parent.const_missing and raises NameError if it is not found causing the parent to searched twice for the missing constant.

Another thing is that AFAICS whenever using a demodulized class name from inside another class within a module then load_missing_constant attempts to load the missing file, even though it has already seen it once.

So should I try and refactor the Dependencies code to handle namespaced models more cleanly or should I just try and fix it in AR?

Andrew White

2

So should I try and refactor the Dependencies code to handle namespaced models more cleanly or should I just try and fix it in AR?

Dependencies.rb is a risky area to be tweaking this late in the piece, so try to localise whatever fixes you need to ActiveRecord

3

This is two models. What happens when you split this out into one model per .rb file?

4

Well it looks as though the easy fix is to change the dependent callbacks in associations.rb to use reflection.klass instead of reflection.class_name. However it does break things if you have out of order associations when you declare multiple models in a single file. This happens with the company.rb model file in the ActiveRecord tests - rearranging the order of the classes in the file allows it to work and all it then passes all the tests.

Shall I submit a patch with this fix or is it likely to be rejected?

Andrew White

5

It was in two files - I just combined it in the email for clarity.

Andrew White

6

module Shop      class Cart < ActiveRecord::Base        has_many :cart_items, :dependent => :delete_all, :class=> "Shop::CartItem"      end end

should work then...

7

Well it looks as though the easy fix is to change the dependent callbacks in associations.rb to use reflection.klass instead of reflection.class_name. However it does break things if you have out of order associations when you declare multiple models in a single file. This happens with the company.rb model file in the ActiveRecord tests - rearranging the order of the classes in the file allows it to work and all it then passes all the tests.

Shall I submit a patch with this fix or is it likely to be rejected?

Submit the patch with that fix and we can investigate the potential breakage once it's ready.