Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Refactor compute_type to handle situations where the correct class is…

… already loaded

Signed-off-by: wycats <wycats@gmail.com>
  • Loading branch information...
commit 9cea9bc7f0c104095dddc036bea7f6ecb9590075 1 parent ee04aea
Andrew White pixeltrix authored wycats committed
35 activerecord/lib/active_record/base.rb
View
@@ -1080,16 +1080,6 @@ def find_sti_class(type_name)
end
end
- # 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)
- if store_full_sti_class
- type_name
- else
- (/^::/ =~ type_name) ? type_name : "#{parent.name}::#{type_name}"
- end
- end
-
def construct_finder_arel(options = {}, scope = nil)
relation = options.is_a?(Hash) ? unscoped.apply_finder_options(options) : unscoped.merge(options)
relation = scope.merge(relation) if scope
@@ -1316,13 +1306,26 @@ def current_scoped_methods #:nodoc:
# Returns the class type of the record using the current module as a prefix. So descendants of
# MyApp::Business::Account would appear as MyApp::Business::AccountSubclass.
def compute_type(type_name)
- modularized_name = type_name_with_module(type_name)
- silence_warnings do
- begin
- class_eval(modularized_name, __FILE__)
- rescue NameError
- class_eval(type_name, __FILE__)
+ if type_name.match(/^::/)
+ # If the type is prefixed with a scope operator then we assume that
+ # the type_name is an absolute reference.
+ type_name.constantize
+ else
+ # Build a list of candidates to search for
+ candidates = []
+ name.scan(/::|$/) { candidates.unshift "#{$`}::#{type_name}" }
+ candidates << type_name
+
+ candidates.each do |candidate|
+ begin
+ constant = candidate.constantize
+ return constant if candidate == constant.to_s
+ rescue NameError
+ rescue ArgumentError
+ end
end
+
+ raise NameError, "uninitialized constant #{candidates.first}"
end
end
8 activerecord/test/cases/base_test.rb
View
@@ -2205,14 +2205,6 @@ def test_to_xml_with_block
assert xml.include?(%(<arbitrary-element>#{value}</arbitrary-element>))
end
- def test_type_name_with_module_should_handle_beginning
- ActiveRecord::Base.store_full_sti_class = false
- assert_equal 'ActiveRecord::Person', ActiveRecord::Base.send(:type_name_with_module, 'Person')
- assert_equal '::Person', ActiveRecord::Base.send(:type_name_with_module, '::Person')
- ensure
- ActiveRecord::Base.store_full_sti_class = true
- end
-
def test_to_param_should_return_string
assert_kind_of String, Client.find(:first).to_param
end
33 activerecord/test/cases/modules_test.rb
View
@@ -1,8 +1,9 @@
require "cases/helper"
require 'models/company_in_module'
+require 'models/shop'
class ModulesTest < ActiveRecord::TestCase
- fixtures :accounts, :companies, :projects, :developers
+ fixtures :accounts, :companies, :projects, :developers, :collections, :products, :variants
def setup
# need to make sure Object::Firm and Object::Client are not defined,
@@ -110,4 +111,34 @@ def test_module_table_name_prefix_with_global_prefix
ActiveRecord::Base.table_name_prefix = ''
classes.each(&:reset_table_name)
end
+
+ def test_compute_type_can_infer_class_name_of_sibling_inside_module
+ old = ActiveRecord::Base.store_full_sti_class
+ ActiveRecord::Base.store_full_sti_class = true
+ assert_equal MyApplication::Business::Firm, MyApplication::Business::Client.send(:compute_type, "Firm")
+ ensure
+ ActiveRecord::Base.store_full_sti_class = old
+ end
+
+ def test_nested_models_should_not_raise_exception_when_using_delete_all_dependency_on_association
+ old = ActiveRecord::Base.store_full_sti_class
+ ActiveRecord::Base.store_full_sti_class = true
+
+ collection = Shop::Collection.find(:first)
+ assert !collection.products.empty?, "Collection should have products"
+ assert_nothing_raised { collection.destroy }
+ ensure
+ ActiveRecord::Base.store_full_sti_class = old
+ end
+
+ def test_nested_models_should_not_raise_exception_when_using_nullify_dependency_on_association
+ old = ActiveRecord::Base.store_full_sti_class
+ ActiveRecord::Base.store_full_sti_class = true
+
+ product = Shop::Product.find(:first)
+ assert !product.variants.empty?, "Product should have variants"
+ assert_nothing_raised { product.destroy }
+ ensure
+ ActiveRecord::Base.store_full_sti_class = old
+ end
end
3  activerecord/test/fixtures/collections.yml
View
@@ -0,0 +1,3 @@
+collection_1:
+ id: 1
+ name: Collection
4 activerecord/test/fixtures/products.yml
View
@@ -0,0 +1,4 @@
+product_1:
+ id: 1
+ collection_id: 1
+ name: Product
4 activerecord/test/fixtures/variants.yml
View
@@ -0,0 +1,4 @@
+variant_1:
+ id: 1
+ product_id: 1
+ name: Variant
12 activerecord/test/models/shop.rb
View
@@ -0,0 +1,12 @@
+module Shop
+ class Collection < ActiveRecord::Base
+ has_many :products, :dependent => :nullify
+ end
+
+ class Product < ActiveRecord::Base
+ has_many :variants, :dependent => :delete_all
+ end
+
+ class Variant < ActiveRecord::Base
+ end
+end
14 activerecord/test/schema/schema.rb
View
@@ -99,6 +99,10 @@ def create_table(*args, &block)
t.string :name
end
+ create_table :collections, :force => true do |t|
+ t.string :name
+ end
+
create_table :colnametests, :force => true do |t|
t.integer :references, :null => false
end
@@ -394,6 +398,11 @@ def create_table(*args, &block)
t.integer :price
end
+ create_table :products, :force => true do |t|
+ t.references :collection
+ t.string :name
+ end
+
create_table :projects, :force => true do |t|
t.string :name
t.string :type
@@ -499,6 +508,11 @@ def create_table(*args, &block)
t.column :looter_type, :string
end
+ create_table :variants, :force => true do |t|
+ t.references :product
+ t.string :name
+ end
+
create_table :vertices, :force => true do |t|
t.column :label, :string
end
Please sign in to comment.
Something went wrong with that request. Please try again.