Unnecessary exception raised in AS::Dependencies.load_missing_constant

We are experiencing the same issue from within migrations:

@@ class FirstMigration < ActiveRecord::Migration

class Book < ActiveRecord::Base

belongs_to :author

end

class Author < ActiveRecord::Base end

def self.up

Book.create!(:name => 'blah')
Book.first.author # blows up here with 'FirstMigration' is not missing constant Author!

end

def self.down end end

@@

This is awfully helpful, since this has been driving us nuts.

In addition, here's the above real patch turned into a monkey patch (via an initializer), for anybody that stumbles onto this issue and doesn't want to (or can't) patch the version of Rails their using.

Thanks for the monkeypatch & workaround! :)

I wasted two days trying to debug this error because my Rails plugin absolutely must have namespaced models (so that my plugin's models do not interfere with the the Rails app that is using my plugin).

I hope this bug is fixed in Rails mainline soon.

Rebased patch against master

The problem is that compute_type is looking for a nested constant first (type_name_with_module) and then falling back to normal constant lookup. So in the example above the has_one :account call tries Admin::User::Account and when that fails it then tries Account which uses the normal constant lookup. The Account constant doesn't exist within User so the const_missing defined in AS::Dependencies::ClassConstMissing is called. Since we're still inside a nested constant load_missing_constant is called. This fails and then the parent constant is sent const_missing. In the example above this is the Admin module and the const_missing in ModuleConstMissing calls load_missing_constant again. However if the constant is previously been loaded the exception removed in the patch is triggered.

Removing the exception seemed the cleanest way of fixing it rather than trying to refactor the const_missing behaviour.

FWIW, I have run into a similar problem. I traced it through AR far enough to determine that commenting out the line

ActiveRecord::Base.store_full_sti_class = true

in new_rails_defaults.rb would work around it. This is clearly not a fix, but perhaps it's a clue. Since I'm not using STI with namespaced classes, I haven't looked into it further.

As Rhett suggested, this seems to be more of an issue with the store_full_sti_class option, specifically in how it's used in ActiveRecord::Base#type_name_with_module:

def type_name_with_module(type_name)
  if store_full_sti_class
    type_name
  else      
    (/^::/ =~ type_name) ? type_name : "#{parent.name}::#{type_name}"
  end
end

Returning the type_name when store_full_sti_class is true seems to have no useful effect, and always prepending the parent module name when the type name is not fully-qualified seems to fix this issue. I've attached a patch w/the updated method and a a test to demonstrate.

I don't think we want to make this fix in the load_missing_constant method. I presume that the reason that it's raising the error is that it's not expecting the constant to be defined already, since we've just called a method that's designed to load missing constants.

I believe that the correct place to make the fix for this issue is in the AS::Dependencies override of the Class#const_missing method, since this is the specific place that we're seeing the issue occur.

I've attached a patch that fixes the issue that Andrew mentioned above in the Class#const_missing method.

Okay, so to summarize:

I think that there are a few issues that are all manifesting themselves here:

• The AR::Base.store_full_sti_class option seems to be used incorrectly in type_name_with_module. This patch I've submitted fixes that particular issue.

• AS::Dependencies has a bug in how it currently resolves a constant that is evaluated from within a class when the constant is defined in a parent module of the evaluating class. This issue is manifesting itself most often when using namespaced AR models that have defined associations to other AR models defined in the same namespace, since this situation will cause the failing behavior to be invoked. However, since the AS::Dependencies logic affects everything, the problem could occur in any Ruby code that requires AS::Dependencies. This patch fixes that issue.

• AS::Dependencies behavior itself overwrites the Ruby constant resolution logic to add file autoloading functionality. However, it implements the constant resolution logic in a way that is inconsistent with normal Ruby constant resolution logic, causing an ArgumentError to be raised about "constant not undefined" when a constant is already defined that it thinks it needs to try and load from the filesystem. Normal Ruby behavior in some cases is to raise a NameError instead, if the constant is being evaluated from within a class whose parent module defines the constant that is being evaluated.

As I see it, there are two ways to deal with this. The first way is to keep things pretty close to the way they are now with the dependency loading logic, and make some minor changes to resolve the issues identified in this ticket; that might be a good short-term solution. The second way is to move some behavior around and refactor some things so that AR::Base is truly independent of AS::Dependencies for computing types from strings, and all that AS::Dependencies does is autoload files and attempt to resolve constants from them.

Going with the second option would mean that AR::Base may need to do the module parent chain traversal as part of its compute_type logic. This makes sense, since the parent chain traversal logic is at least partly inconsistent with the default Ruby behavior for constant resolution, and so probably should be scoped to the needs of AR::Base when resolving constants from things like association reflections.

Thoughts?

I'm happy with Ryan's patch if that'd be preferred. I'd thought of doing it that way but I think I was trying to be as unobtrusive as possible at the time due to the impending release of 2.3. Wouldn't it need to take account of the changes to const_defined? in Ruby 1.9 though?

You're probably right, Andrew. Good catch. I hadn't tested it with 1.9. I can do that sometime this week or next, unless you want to handle it. Let me know, and I won't.

I'm quite prepared to write the patch, though we could probably do with some direction from a core team member at this point. I noticed a commit in a experimental branch which implies that AS::Dependencies is being looked at for Rails 3.0. If this is the case do core want a patch for 2.3-stable which fixes it in ClassConstMissing and another patch with just a failing test case for edge. Should I raise the matter on the rails-core mailing list?

I've attached a revised version of Ryan's 10/1 patch. The old one no longer would apply to master. I tested it (i.e., verified that the test suite passes) on MRI 1.8.7-p174 and MRI 1.9.1-p376.

I also verified that the other patch (Ryan's from 10/13) works on 1.9.1-p376.

I'd like to see that at least the first of these (the STI one) get applied to master before 3.0 comes out -- this is a longstanding irritation with namespaced models that can be fixed with a small code change. (I can see where the second one might require more consideration.) What else can I do to help?

thanks for working on this, guys, I can confirm having the same problem on Rails 2.3.4 using both STI and namespaced controllers. Do you know if there will be another release of Rails 2.3.x now that Rails 3 beta is out?

I'm also still watching this. Any input from people on the core team who are familiar with the direction that this stuff is going, so we can come to a lasting solution?

I've posted to the rails-core list about this issue. Maybe we'll get someone's attention there.

http://groups.google.com/group/rubyonrails-core/t/534dda4205adb692

Updated patch for HEAD with whitespace changes removed and updated offsets, no other code changes.

+1, applies & tests pass, and fixes the problem I had with this

I can't say that I'm following all of this very clearly, but I wanted to add something to the discussion just in case it might be useful in some way. I read through the thread on the rails-core list, but didn't see anything relating to this mentioned. I'm probably just going to expose my ignorance here, but oh well.

I am getting the same error (x is not missing constant y) in 2.3.5, but I am not namespacing my models at all. I am, however, namespacing my controllers.

class Web::BaseController < ApplicationController
end

class Web::SessionsController < Web::BaseController
  def create
    user_id = User.authenticate(params[:email], params[:password])
  end
end

class User < ActiveRecord::Base
end

I get the error in Web::SessionsController#create, but only in development. In testing, either through RSpec or Cucumber+Culerity, I didn't see it. So I compared environment files for test and development and noticed config.cache_classes was false in development but true in test. When I changed it to true in development, I didn't get the error.

There may be more to it than that as I was also creating a very simple project that mimicked my structure but was unable to reproduce the problem. And I'm doing the same thing in another project without any problems. So this bit about cache_classes might not help in any way.

Phillip, what was the error message? If it was 'Object is not missing constant User' then it's still related to namespaces and models. If it's something like 'Web is not missing constant BaseController then it's something I've not come across before even though I do something similar with admin controllers.

Andrew,

It was 'Web is not missing constant User!'. I assumed it was related, but didn't know if the additional information of cache_classes might be helpful.

Oh, and I completely forgot to mention that I applied the changes in the patch, but they didn't help. [Why would they when they were on ActiveRecord? heh heh]

Something I just thought of that might be contributing to my particular case. I'm namespacing my controllers in an effort to make role restrictions very clean, so I have the base namespace of Web which is for users not logged in, Admin for administrative functionality, and User for users who are logged in. It looks like

class Web::User::BaseController < Web::BaseController
    before_filter :require_logged_in
    
    private
    
    def require_logged_in
        redirect_to(web_login_path) and return false unless logged_in?
        return true
    end
end

And that might be causing some confusion with the User model. If so, I'm confused why it would work in testing with cache_classes == true and not in development with cache_classes == false.

Yep, the Web::User::BaseController will be the cause of the problem. The difference due to cache_classes is probably down to a different load order. A fix in your case is to use ::User rather than User or change the Web::User namespace to something else.

Thanks, Andrew. Sorry to have added noise to the conversation.

Updated patch with spelling fixed - somehow I managed to spell candidates wrong consistently.

Backport patch to 2-3-stable.

Updated patch for master with tests for has_many with dependencies from ticket #2627 has_many :dependent => :delete_all throws exception on models in modules.

Updated patch for 2-3-stable with tests for has_many with dependencies from ticket #2627 has_many :dependent => :delete_all throws exception on models in modules. This needs the backport of lazy evaluation of has_many ..., :dependent => :___ patch before the tests will pass.

Rebased patch for latest HEAD.

Is there anything that's holding this patch up? Anything I can do to help?