Improve the derivation of HABTM assocation join table names
Improve the derivation of HABTM join table name to take account of nesting. It now takes the table names of the two models, sorts them lexically and then joins them, stripping any common prefix from the second table name. Some examples: Top level models (Category <=> Product) Old: categories_products New: categories_products Top level models with a global table_name_prefix (Category <=> Product) Old: site_categories_products New: site_categories_products Nested models in a module without a table_name_prefix method (Admin::Category <=> Admin::Product) Old: categories_products New: categories_products Nested models in a module with a table_name_prefix method (Admin::Category <=> Admin::Product) Old: categories_products New: admin_categories_products Nested models in a parent model (Catalog::Category <=> Catalog::Product) Old: categories_products New: catalog_categories_products Nested models in different parent models (Catalog::Category <=> Content::Page) Old: categories_pages New: catalog_categories_content_pages Also as part of this commit the validity checks for HABTM assocations have been moved to ActiveRecord::Reflection One side effect of this is to move when the exceptions are raised from the point of declaration to when the association is built. This is consistant with other association validity checks.
This commit is contained in:
parent
4bbd35f7a6
commit
46492949b8
@ -1,5 +1,44 @@
|
||||
## Rails 4.0.0 (unreleased) ##
|
||||
|
||||
* Improve the derivation of HABTM join table name to take account of nesting.
|
||||
It now takes the table names of the two models, sorts them lexically and
|
||||
then joins them, stripping any common prefix from the second table name.
|
||||
|
||||
Some examples:
|
||||
|
||||
Top level models (Category <=> Product)
|
||||
Old: categories_products
|
||||
New: categories_products
|
||||
|
||||
Top level models with a global table_name_prefix (Category <=> Product)
|
||||
Old: site_categories_products
|
||||
New: site_categories_products
|
||||
|
||||
Nested models in a module without a table_name_prefix method (Admin::Category <=> Admin::Product)
|
||||
Old: categories_products
|
||||
New: categories_products
|
||||
|
||||
Nested models in a module with a table_name_prefix method (Admin::Category <=> Admin::Product)
|
||||
Old: categories_products
|
||||
New: admin_categories_products
|
||||
|
||||
Nested models in a parent model (Catalog::Category <=> Catalog::Product)
|
||||
Old: categories_products
|
||||
New: catalog_categories_products
|
||||
|
||||
Nested models in different parent models (Catalog::Category <=> Content::Page)
|
||||
Old: categories_pages
|
||||
New: catalog_categories_content_pages
|
||||
|
||||
*Andrew White*
|
||||
|
||||
* Move HABTM validity checks to ActiveRecord::Relation. One side effect of
|
||||
this is to move when the exceptions are raised from the point of declaration
|
||||
to when the association is built. This is consistant with other association
|
||||
validity checks.
|
||||
|
||||
*Andrew White*
|
||||
|
||||
* Added `stored_attributes` hash which contains the attributes stored using
|
||||
ActiveRecord::Store. This allows you to retrieve the list of attributes
|
||||
you've defined.
|
||||
|
@ -6,7 +6,6 @@ class HasAndBelongsToMany < CollectionAssociation #:nodoc:
|
||||
|
||||
def build
|
||||
reflection = super
|
||||
check_validity(reflection)
|
||||
define_destroy_hook
|
||||
reflection
|
||||
end
|
||||
@ -24,34 +23,5 @@ def destroy_associations
|
||||
RUBY
|
||||
})
|
||||
end
|
||||
|
||||
# TODO: These checks should probably be moved into the Reflection, and we should not be
|
||||
# redefining the options[:join_table] value - instead we should define a
|
||||
# reflection.join_table method.
|
||||
def check_validity(reflection)
|
||||
if reflection.association_foreign_key == reflection.foreign_key
|
||||
raise ActiveRecord::HasAndBelongsToManyAssociationForeignKeyNeeded.new(reflection)
|
||||
end
|
||||
|
||||
reflection.options[:join_table] ||= join_table_name(
|
||||
model.send(:undecorated_table_name, model.to_s),
|
||||
model.send(:undecorated_table_name, reflection.class_name)
|
||||
)
|
||||
end
|
||||
|
||||
# Generates a join table name from two provided table names.
|
||||
# The names in the join table names end up in lexicographic order.
|
||||
#
|
||||
# join_table_name("members", "clubs") # => "clubs_members"
|
||||
# join_table_name("members", "special_clubs") # => "members_special_clubs"
|
||||
def join_table_name(first_table_name, second_table_name)
|
||||
if first_table_name < second_table_name
|
||||
join_table = "#{first_table_name}_#{second_table_name}"
|
||||
else
|
||||
join_table = "#{second_table_name}_#{first_table_name}"
|
||||
end
|
||||
|
||||
model.table_name_prefix + join_table + model.table_name_suffix
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -5,7 +5,7 @@ class HasAndBelongsToManyAssociation < CollectionAssociation #:nodoc:
|
||||
attr_reader :join_table
|
||||
|
||||
def initialize(owner, reflection)
|
||||
@join_table = Arel::Table.new(reflection.options[:join_table])
|
||||
@join_table = Arel::Table.new(reflection.join_table)
|
||||
super
|
||||
end
|
||||
|
||||
|
@ -19,7 +19,7 @@ def construct_tables
|
||||
|
||||
if reflection.source_macro == :has_and_belongs_to_many
|
||||
tables << alias_tracker.aliased_table_for(
|
||||
(reflection.source_reflection || reflection).options[:join_table],
|
||||
(reflection.source_reflection || reflection).join_table,
|
||||
table_alias_for(reflection, true)
|
||||
)
|
||||
end
|
||||
|
@ -6,7 +6,7 @@ class HasAndBelongsToMany < CollectionAssociation #:nodoc:
|
||||
|
||||
def initialize(klass, records, reflection, preload_options)
|
||||
super
|
||||
@join_table = Arel::Table.new(options[:join_table]).alias('t0')
|
||||
@join_table = Arel::Table.new(reflection.join_table).alias('t0')
|
||||
end
|
||||
|
||||
# Unlike the other associations, we want to get a raw array of rows so that we can
|
||||
|
@ -594,7 +594,7 @@ def table_rows
|
||||
when :has_and_belongs_to_many
|
||||
if (targets = row.delete(association.name.to_s))
|
||||
targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/)
|
||||
table_name = association.options[:join_table]
|
||||
table_name = association.join_table
|
||||
rows[table_name].concat targets.map { |target|
|
||||
{ association.foreign_key => row[primary_key_name],
|
||||
association.association_foreign_key => ActiveRecord::Fixtures.identify(target) }
|
||||
|
@ -161,6 +161,10 @@ def quoted_table_name
|
||||
@quoted_table_name ||= klass.quoted_table_name
|
||||
end
|
||||
|
||||
def join_table
|
||||
@join_table ||= options[:join_table] || derive_join_table
|
||||
end
|
||||
|
||||
def foreign_key
|
||||
@foreign_key ||= options[:foreign_key] || derive_foreign_key
|
||||
end
|
||||
@ -208,6 +212,10 @@ def reset_column_information
|
||||
|
||||
def check_validity!
|
||||
check_validity_of_inverse!
|
||||
|
||||
if has_and_belongs_to_many? && association_foreign_key == foreign_key
|
||||
raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(self)
|
||||
end
|
||||
end
|
||||
|
||||
def check_validity_of_inverse!
|
||||
@ -290,6 +298,10 @@ def belongs_to?
|
||||
macro == :belongs_to
|
||||
end
|
||||
|
||||
def has_and_belongs_to_many?
|
||||
macro == :has_and_belongs_to_many
|
||||
end
|
||||
|
||||
def association_class
|
||||
case macro
|
||||
when :belongs_to
|
||||
@ -332,6 +344,10 @@ def derive_foreign_key
|
||||
end
|
||||
end
|
||||
|
||||
def derive_join_table
|
||||
[active_record.table_name, klass.table_name].sort.join("\0").gsub(/^(.*_)(.+)\0\1(.+)/, '\1\2_\3').gsub("\0", "_")
|
||||
end
|
||||
|
||||
def primary_key(klass)
|
||||
klass.primary_key || raise(UnknownPrimaryKey.new(klass))
|
||||
end
|
||||
|
@ -34,11 +34,11 @@ def setup
|
||||
'select'=>'id int auto_increment primary key',
|
||||
'values'=>'id int auto_increment primary key, group_id int',
|
||||
'distinct'=>'id int auto_increment primary key',
|
||||
'distincts_selects'=>'distinct_id int, select_id int'
|
||||
'distinct_select'=>'distinct_id int, select_id int'
|
||||
end
|
||||
|
||||
def teardown
|
||||
drop_tables_directly ['group', 'select', 'values', 'distinct', 'distincts_selects', 'order']
|
||||
drop_tables_directly ['group', 'select', 'values', 'distinct', 'distinct_select', 'order']
|
||||
end
|
||||
|
||||
# create tables with reserved-word names and columns
|
||||
@ -80,7 +80,7 @@ def test_introspect
|
||||
|
||||
#activerecord model class with reserved-word table name
|
||||
def test_activerecord_model
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
x = nil
|
||||
assert_nothing_raised { x = Group.new }
|
||||
x.order = 'x'
|
||||
@ -94,7 +94,7 @@ def test_activerecord_model
|
||||
|
||||
# has_one association with reserved-word table name
|
||||
def test_has_one_associations
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
v = nil
|
||||
assert_nothing_raised { v = Group.find(1).values }
|
||||
assert_equal 2, v.id
|
||||
@ -102,7 +102,7 @@ def test_has_one_associations
|
||||
|
||||
# belongs_to association with reserved-word table name
|
||||
def test_belongs_to_associations
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
gs = nil
|
||||
assert_nothing_raised { gs = Select.find(2).groups }
|
||||
assert_equal gs.length, 2
|
||||
@ -111,7 +111,7 @@ def test_belongs_to_associations
|
||||
|
||||
# has_and_belongs_to_many with reserved-word table name
|
||||
def test_has_and_belongs_to_many
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
s = nil
|
||||
assert_nothing_raised { s = Distinct.find(1).selects }
|
||||
assert_equal s.length, 2
|
||||
|
@ -34,11 +34,11 @@ def setup
|
||||
'select'=>'id int auto_increment primary key',
|
||||
'values'=>'id int auto_increment primary key, group_id int',
|
||||
'distinct'=>'id int auto_increment primary key',
|
||||
'distincts_selects'=>'distinct_id int, select_id int'
|
||||
'distinct_select'=>'distinct_id int, select_id int'
|
||||
end
|
||||
|
||||
def teardown
|
||||
drop_tables_directly ['group', 'select', 'values', 'distinct', 'distincts_selects', 'order']
|
||||
drop_tables_directly ['group', 'select', 'values', 'distinct', 'distinct_select', 'order']
|
||||
end
|
||||
|
||||
# create tables with reserved-word names and columns
|
||||
@ -80,7 +80,7 @@ def test_introspect
|
||||
|
||||
#activerecord model class with reserved-word table name
|
||||
def test_activerecord_model
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
x = nil
|
||||
assert_nothing_raised { x = Group.new }
|
||||
x.order = 'x'
|
||||
@ -94,7 +94,7 @@ def test_activerecord_model
|
||||
|
||||
# has_one association with reserved-word table name
|
||||
def test_has_one_associations
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
v = nil
|
||||
assert_nothing_raised { v = Group.find(1).values }
|
||||
assert_equal 2, v.id
|
||||
@ -102,7 +102,7 @@ def test_has_one_associations
|
||||
|
||||
# belongs_to association with reserved-word table name
|
||||
def test_belongs_to_associations
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
gs = nil
|
||||
assert_nothing_raised { gs = Select.find(2).groups }
|
||||
assert_equal gs.length, 2
|
||||
@ -111,7 +111,7 @@ def test_belongs_to_associations
|
||||
|
||||
# has_and_belongs_to_many with reserved-word table name
|
||||
def test_has_and_belongs_to_many
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
|
||||
create_test_fixtures :select, :distinct, :group, :values, :distinct_select
|
||||
s = nil
|
||||
assert_nothing_raised { s = Distinct.find(1).selects }
|
||||
assert_equal s.length, 2
|
||||
|
@ -773,9 +773,7 @@ def test_symbols_as_keys
|
||||
|
||||
def test_self_referential_habtm_without_foreign_key_set_should_raise_exception
|
||||
assert_raise(ActiveRecord::HasAndBelongsToManyAssociationForeignKeyNeeded) {
|
||||
Member.class_eval do
|
||||
has_and_belongs_to_many :friends, :class_name => "Member", :join_table => "member_friends"
|
||||
end
|
||||
SelfMember.new.friends
|
||||
}
|
||||
end
|
||||
|
||||
|
@ -1,11 +1,11 @@
|
||||
distincts_selects1:
|
||||
distinct_select1:
|
||||
distinct_id: 1
|
||||
select_id: 1
|
||||
|
||||
distincts_selects2:
|
||||
distinct_select2:
|
||||
distinct_id: 1
|
||||
select_id: 2
|
||||
|
||||
distincts_selects3:
|
||||
distinct_select3:
|
||||
distinct_id: 2
|
||||
select_id: 3
|
@ -30,3 +30,8 @@ class Member < ActiveRecord::Base
|
||||
has_many :current_memberships, :conditions => { :favourite => true }
|
||||
has_many :clubs, :through => :current_memberships
|
||||
end
|
||||
|
||||
class SelfMember < ActiveRecord::Base
|
||||
self.table_name = "members"
|
||||
has_and_belongs_to_many :friends, :class_name => "SelfMember", :join_table => "member_friends"
|
||||
end
|
||||
|
@ -361,6 +361,11 @@ def create_table(*args, &block)
|
||||
t.string :extra_data
|
||||
end
|
||||
|
||||
create_table :member_friends, :force => true, :id => false do |t|
|
||||
t.integer :member_id
|
||||
t.integer :friend_id
|
||||
end
|
||||
|
||||
create_table :memberships, :force => true do |t|
|
||||
t.datetime :joined_on
|
||||
t.integer :club_id, :member_id
|
||||
|
Loading…
Reference in New Issue
Block a user