Reflections don't attempt to resolve module nesting of association classes. Simplify type computation.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3637 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

6 files changed, 77 insertions, 14 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 01dd02a2f4..455785e7b9 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Reflections don't attempt to resolve module nesting of association classes. Simplify type computation. [Jeremy Kemper]
+
 * Improved the Oracle OCI Adapter with better performance for column reflection (from #3210), fixes to migrations (from #3476 and #3742), tweaks to unit tests (from #3610), and improved documentation (from #2446) #3879 [Aggregated by schoenm@earthlink.net]
 
 * Fixed that the schema_info table used by ActiveRecord::Schema.define should respect table pre- and suffixes #3834 [rubyonrails@atyp.de]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index e040c86505..681f99b5c0 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -956,10 +956,10 @@ module ActiveRecord #:nodoc:
           object
         end
 
-        # Returns the name of the type of the record using the current module as a prefix. So descendents of
-        # MyApp::Business::Account would appear as "MyApp::Business::AccountSubclass".
+        # Nest the type name in the same module as this class.
+        # Bar is "MyApp::Business::Bar" relative to MyApp::Business::Foo
         def type_name_with_module(type_name)
-          self.name =~ /::/ ? self.name.scan(/(.*)::/).first.first + "::" + type_name : type_name
+          "#{self.name.sub(/(::)?[^:]+$/, '')}#{$1}#{type_name}"
         end
 
         def construct_finder_sql(options)
@@ -1152,8 +1152,10 @@ module ActiveRecord #:nodoc:
         # Returns the class type of the record using the current module as a prefix. So descendents of
         # MyApp::Business::Account would appear as MyApp::Business::AccountSubclass.
         def compute_type(type_name)
-          type_name_with_module(type_name).split("::").inject(Object) do |final_type, part|
-            final_type.const_get(part)
+          begin
+            instance_eval(type_name_with_module(type_name))
+          rescue Object
+            instance_eval(type_name)
           end
         end
 
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 9bac82f5ce..2f73300abf 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -136,13 +136,12 @@ module ActiveRecord
             name
           else
             if options[:class_name]
-              class_name = options[:class_name]
+              options[:class_name]
             else
               class_name = name.to_s.camelize
               class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
+              class_name
             end
-
-            active_record.send(:type_name_with_module, class_name)
           end
         end
     end
diff --git a/activerecord/test/fixtures/company_in_module.rb b/activerecord/test/fixtures/company_in_module.rb
index 52b5d37b8c..df476520b8 100644
--- a/activerecord/test/fixtures/company_in_module.rb
+++ b/activerecord/test/fixtures/company_in_module.rb
@@ -33,11 +33,25 @@ module MyApplication
     end
 
   end
-  
+
   module Billing
+    class Firm < ActiveRecord::Base
+      self.table_name = 'companies'
+    end
+
+    module Nested
+      class Firm < ActiveRecord::Base
+        self.table_name = 'companies'
+      end
+    end
+
     class Account < ActiveRecord::Base
-      belongs_to :firm, :class_name => "MyApplication::Business::Firm"
-      
+      belongs_to :firm, :class_name => 'MyApplication::Business::Firm'
+      belongs_to :qualified_billing_firm, :class_name => 'MyApplication::Billing::Firm'
+      belongs_to :unqualified_billing_firm, :class_name => 'Firm'
+      belongs_to :nested_qualified_billing_firm, :class_name => 'MyApplication::Billing::Nested::Firm'
+      belongs_to :nested_unqualified_billing_firm, :class_name => 'Nested::Firm'
+
       protected
         def validate
           errors.add_on_empty "credit_limit"
diff --git a/activerecord/test/modules_test.rb b/activerecord/test/modules_test.rb
index d34afbb8f8..5c3cc1906e 100644
--- a/activerecord/test/modules_test.rb
+++ b/activerecord/test/modules_test.rb
@@ -18,6 +18,11 @@ class ModulesTest < Test::Unit::TestCase
   end
   
   def test_associations_spanning_cross_modules
-    assert MyApplication::Billing::Account.find(1).has_firm?, "37signals account should be able to backtrack"
+    account = MyApplication::Billing::Account.find(:first)
+    assert_kind_of MyApplication::Business::Firm, account.firm
+    assert_kind_of MyApplication::Billing::Firm, account.qualified_billing_firm
+    assert_kind_of MyApplication::Billing::Firm, account.unqualified_billing_firm
+    assert_kind_of MyApplication::Billing::Nested::Firm, account.nested_qualified_billing_firm
+    assert_kind_of MyApplication::Billing::Nested::Firm, account.nested_unqualified_billing_firm
   end
 end
diff --git a/activerecord/test/reflection_test.rb b/activerecord/test/reflection_test.rb
index 30cc99677e..6e5d7126f5 100644
--- a/activerecord/test/reflection_test.rb
+++ b/activerecord/test/reflection_test.rb
@@ -90,8 +90,41 @@ class ReflectionTest < Test::Unit::TestCase
   end
 
   def test_association_reflection_in_modules
-    assert_equal MyApplication::Business::Client, MyApplication::Business::Firm.reflect_on_association(:clients_of_firm).klass
-    assert_equal MyApplication::Business::Firm, MyApplication::Billing::Account.reflect_on_association(:firm).klass
+    assert_reflection MyApplication::Business::Firm,
+      :clients_of_firm,
+      :klass      => MyApplication::Business::Client,
+      :class_name => 'Client',
+      :table_name => 'companies'
+
+    assert_reflection MyApplication::Billing::Account,
+      :firm,
+      :klass      => MyApplication::Business::Firm,
+      :class_name => 'MyApplication::Business::Firm',
+      :table_name => 'companies'
+
+    assert_reflection MyApplication::Billing::Account,
+      :qualified_billing_firm,
+      :klass      => MyApplication::Billing::Firm,
+      :class_name => 'MyApplication::Billing::Firm',
+      :table_name => 'companies'
+
+    assert_reflection MyApplication::Billing::Account,
+      :unqualified_billing_firm,
+      :klass      => MyApplication::Billing::Firm,
+      :class_name => 'Firm',
+      :table_name => 'companies'
+
+    assert_reflection MyApplication::Billing::Account,
+      :nested_qualified_billing_firm,
+      :klass      => MyApplication::Billing::Nested::Firm,
+      :class_name => 'MyApplication::Billing::Nested::Firm',
+      :table_name => 'companies'
+
+    assert_reflection MyApplication::Billing::Account,
+      :nested_unqualified_billing_firm,
+      :klass      => MyApplication::Billing::Nested::Firm,
+      :class_name => 'Nested::Firm',
+      :table_name => 'companies'
   end
   
   def test_reflection_of_all_associations
@@ -100,4 +133,12 @@ class ReflectionTest < Test::Unit::TestCase
     assert_equal 1, Firm.reflect_on_all_associations(:has_one).size
     assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
   end
+
+  private
+    def assert_reflection(klass, association, options)
+      assert reflection = klass.reflect_on_association(association)
+      options.each do |method, value|
+        assert_equal(value, reflection.send(method))
+      end
+    end
 end