Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

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.
  • Loading branch information...
commit 46492949b8c09f99db78b9f7a02d039e7bc6a702 1 parent 4bbd35f
@pixeltrix pixeltrix authored
View
39 activerecord/CHANGELOG.md
@@ -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.
View
30 activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -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
View
2  activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -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
View
2  activerecord/lib/active_record/associations/join_helper.rb
@@ -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
View
2  activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb
@@ -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
View
2  activerecord/lib/active_record/fixtures.rb
@@ -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) }
View
16 activerecord/lib/active_record/reflection.rb
@@ -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
View
12 activerecord/test/cases/adapters/mysql/reserved_word_test.rb
@@ -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
View
12 activerecord/test/cases/adapters/mysql2/reserved_word_test.rb
@@ -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
View
4 activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -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
View
6 ...ures/reserved_words/distincts_selects.yml → ...xtures/reserved_words/distinct_select.yml
@@ -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
View
5 activerecord/test/models/member.rb
@@ -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
View
5 activerecord/test/schema/schema.rb
@@ -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
Please sign in to comment.
Something went wrong with that request. Please try again.