From 46492949b8c09f99db78b9f7a02d039e7bc6a702 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Thu, 21 Jun 2012 20:47:12 +0100 Subject: 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. --- activerecord/CHANGELOG.md | 39 ++++++++++++++++++++++ .../builder/has_and_belongs_to_many.rb | 30 ----------------- .../has_and_belongs_to_many_association.rb | 2 +- .../lib/active_record/associations/join_helper.rb | 2 +- .../preloader/has_and_belongs_to_many.rb | 2 +- activerecord/lib/active_record/fixtures.rb | 2 +- activerecord/lib/active_record/reflection.rb | 16 +++++++++ .../cases/adapters/mysql/reserved_word_test.rb | 12 +++---- .../cases/adapters/mysql2/reserved_word_test.rb | 12 +++---- .../has_and_belongs_to_many_associations_test.rb | 4 +-- .../fixtures/reserved_words/distinct_select.yml | 11 ++++++ .../fixtures/reserved_words/distincts_selects.yml | 11 ------ activerecord/test/models/member.rb | 5 +++ activerecord/test/schema/schema.rb | 5 +++ 14 files changed, 93 insertions(+), 60 deletions(-) create mode 100644 activerecord/test/fixtures/reserved_words/distinct_select.yml delete mode 100644 activerecord/test/fixtures/reserved_words/distincts_selects.yml (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 10bfe1945a..d759b26c65 100644 --- a/activerecord/CHANGELOG.md +++ b/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. diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 30fc44b4c2..f7656ecd47 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -6,7 +6,6 @@ module ActiveRecord::Associations::Builder def build reflection = super - check_validity(reflection) define_destroy_hook reflection end @@ -24,34 +23,5 @@ module ActiveRecord::Associations::Builder 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 diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 58d041ec1d..93618721bb 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -5,7 +5,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb index cea6ad6944..5a41b40c8f 100644 --- a/activerecord/lib/active_record/associations/join_helper.rb +++ b/activerecord/lib/active_record/associations/join_helper.rb @@ -19,7 +19,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb index b77b667219..8e8925f0a9 100644 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb @@ -6,7 +6,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 7e6512501c..96d24b72b3 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -594,7 +594,7 @@ module ActiveRecord 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) } diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e9a8e90538..0d9534acd6 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -161,6 +161,10 @@ module ActiveRecord @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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 diff --git a/activerecord/test/cases/adapters/mysql/reserved_word_test.rb b/activerecord/test/cases/adapters/mysql/reserved_word_test.rb index 6faceaf7c0..8e3eeac81e 100644 --- a/activerecord/test/cases/adapters/mysql/reserved_word_test.rb +++ b/activerecord/test/cases/adapters/mysql/reserved_word_test.rb @@ -34,11 +34,11 @@ class MysqlReservedWordTest < ActiveRecord::TestCase '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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase #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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase # 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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase # 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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase # 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 diff --git a/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb b/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb index 32d4282623..5c2c113c78 100644 --- a/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb +++ b/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb @@ -34,11 +34,11 @@ class MysqlReservedWordTest < ActiveRecord::TestCase '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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase #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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase # 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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase # 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 @@ class MysqlReservedWordTest < ActiveRecord::TestCase # 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 diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 9d693bae0c..24bb4adf0a 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -773,9 +773,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase 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 diff --git a/activerecord/test/fixtures/reserved_words/distinct_select.yml b/activerecord/test/fixtures/reserved_words/distinct_select.yml new file mode 100644 index 0000000000..d96779ade4 --- /dev/null +++ b/activerecord/test/fixtures/reserved_words/distinct_select.yml @@ -0,0 +1,11 @@ +distinct_select1: + distinct_id: 1 + select_id: 1 + +distinct_select2: + distinct_id: 1 + select_id: 2 + +distinct_select3: + distinct_id: 2 + select_id: 3 diff --git a/activerecord/test/fixtures/reserved_words/distincts_selects.yml b/activerecord/test/fixtures/reserved_words/distincts_selects.yml deleted file mode 100644 index 90e8c95fef..0000000000 --- a/activerecord/test/fixtures/reserved_words/distincts_selects.yml +++ /dev/null @@ -1,11 +0,0 @@ -distincts_selects1: - distinct_id: 1 - select_id: 1 - -distincts_selects2: - distinct_id: 1 - select_id: 2 - -distincts_selects3: - distinct_id: 2 - select_id: 3 diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index 11a0f4ff63..1f719b0858 100644 --- a/activerecord/test/models/member.rb +++ b/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 diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 5bcb9652cd..ba01ef35f0 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -361,6 +361,11 @@ ActiveRecord::Schema.define do 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 -- cgit v1.2.3