From d1521719c5ac61a0c0e59827fe8cb197f5fe56f5 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sun, 16 Jan 2011 21:28:47 +0000 Subject: Removed support for accessing attributes on a has_and_belongs_to_many join table. This has been documented as deprecated behaviour since April 2006. Please use has_many :through instead. A deprecation warning will be added to the 3-0-stable branch for the 3.0.4 release. --- activerecord/CHANGELOG | 6 +- activerecord/lib/active_record/associations.rb | 6 -- .../has_and_belongs_to_many_association.rb | 68 ++---------------- .../has_and_belongs_to_many_associations_test.rb | 83 ---------------------- activerecord/test/cases/readonly_test.rb | 9 --- 5 files changed, 11 insertions(+), 161 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 1d2f814d32..43272160bd 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,9 @@ *Rails 3.1.0 (unreleased)* +* Removed support for accessing attributes on a has_and_belongs_to_many join table. This has been + documented as deprecated behaviour since April 2006. Please use has_many :through instead. + [Jon Leighton] + * Migration files generated from model and constructive migration generators (for example, add_name_to_users) use the reversible migration's `change` method instead of the ordinary `up` and `down` methods. [Prem Sichanugrist] @@ -26,7 +30,7 @@ along with attribute conditionals as a replacement. User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user -* When a model is generated add_index is added by default for belongs_to or references columns +* When a model is generated add_index is added by default for belongs_to or references columns rails g model post user:belongs_to will generate the following: diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index e539e4968b..891ac52f8a 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1301,12 +1301,6 @@ module ActiveRecord # end # end # - # Deprecated: Any additional fields added to the join table will be placed as attributes when - # pulling records out through +has_and_belongs_to_many+ associations. Records returned from join - # tables with additional attributes will be marked as readonly (because we can't save changes - # to the additional attributes). It's strongly recommended that you upgrade any - # associations with attributes to a real join model (see introduction). - # # Adds the following methods for retrieval and query: # # [collection(force_reload = false)] 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 bc7894173d..b28554dce1 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 @@ -10,18 +10,6 @@ module ActiveRecord super end - def columns - @reflection.columns(@join_table_name, "#{@join_table_name} Columns") - end - - def reset_column_information - @reflection.reset_column_information - end - - def has_primary_key? - @has_primary_key ||= @owner.connection.supports_primary_key? && @owner.connection.primary_key(@join_table_name) - end - protected def count_records @@ -36,26 +24,11 @@ module ActiveRecord if @reflection.options[:insert_sql] @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record)) else - relation = join_table - timestamps = record_timestamp_columns(record) - timezone = record.send(:current_time_from_proper_timezone) if timestamps.any? - - attributes = columns.map do |column| - name = column.name - value = case name.to_s - when @reflection.foreign_key.to_s - @owner.id - when @reflection.association_foreign_key.to_s - record.id - when *timestamps - timezone - else - @owner.send(:quote_value, record[name], column) if record.has_attribute?(name) - end - [relation[name], value] unless value.nil? - end + stmt = join_table.compile_insert( + join_table[@reflection.foreign_key] => @owner.id, + join_table[@reflection.association_foreign_key] => record.id + ) - stmt = relation.compile_insert Hash[attributes] @owner.connection.insert stmt.to_sql end @@ -89,46 +62,17 @@ module ActiveRecord end def association_scope - scope = super.joins(construct_joins) - scope = scope.readonly if ambiguous_select?(@reflection.options[:select]) - scope + super.joins(construct_joins) end def select_value - super || [@reflection.klass.arel_table[Arel.star], join_table[Arel.star]] - end - - # Join tables with additional columns on top of the two foreign keys must be considered - # ambiguous unless a select clause has been explicitly defined. Otherwise you can get - # broken records back, if, for example, the join column also has an id column. This will - # then overwrite the id column of the records coming back. - def ambiguous_select?(select) - extra_join_columns? && select.nil? - end - - def extra_join_columns? - columns.size > 2 + super || @reflection.klass.arel_table[Arel.star] end private - def record_timestamp_columns(record) - if record.record_timestamps - record.send(:all_timestamp_attributes).map { |x| x.to_s } - else - [] - end - end - def invertible_for?(record) false end - - def find_by_sql(*args) - options = args.extract_options! - ambiguous = ambiguous_select?(@reflection.options[:select] || options[:select]) - - scoped.readonly(ambiguous).find(*(args << options)) - end end end end 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 705550216c..30730c7094 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 @@ -101,38 +101,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 't1', record[1] end - def test_should_record_timestamp_for_join_table - setup_data_for_habtm_case - - con = ActiveRecord::Base.connection - sql = 'select * from countries_treaties' - record = con.select_rows(sql).last - assert_not_nil record[2] - assert_not_nil record[3] - if current_adapter?(:Mysql2Adapter, :OracleAdapter) - assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[2].to_s(:db) - assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[3].to_s(:db) - else - assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[2] - assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[3] - end - end - - def test_should_record_timestamp_for_join_table_only_if_timestamp_should_be_recorded - begin - Treaty.record_timestamps = false - setup_data_for_habtm_case - - con = ActiveRecord::Base.connection - sql = 'select * from countries_treaties' - record = con.select_rows(sql).last - assert_nil record[2] - assert_nil record[3] - ensure - Treaty.record_timestamps = true - end - end - def test_has_and_belongs_to_many david = Developer.find(1) @@ -218,34 +186,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, aredridel.projects(true).size end - def test_adding_uses_default_values_on_join_table - ac = projects(:action_controller) - assert !developers(:jamis).projects.include?(ac) - developers(:jamis).projects << ac - - assert developers(:jamis, :reload).projects.include?(ac) - project = developers(:jamis).projects.detect { |p| p == ac } - assert_equal 1, project.access_level.to_i - end - - def test_habtm_attribute_access_and_respond_to - project = developers(:jamis).projects[0] - assert project.has_attribute?("name") - assert project.has_attribute?("joined_on") - assert project.has_attribute?("access_level") - assert project.respond_to?("name") - assert project.respond_to?("name=") - assert project.respond_to?("name?") - assert project.respond_to?("joined_on") - # given that the 'join attribute' won't be persisted, I don't - # think we should define the mutators - #assert project.respond_to?("joined_on=") - assert project.respond_to?("joined_on?") - assert project.respond_to?("access_level") - #assert project.respond_to?("access_level=") - assert project.respond_to?("access_level?") - end - def test_habtm_adding_before_save no_of_devels = Developer.count no_of_projects = Project.count @@ -430,10 +370,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert DeveloperWithBeforeDestroyRaise.connection.select_all("SELECT * FROM developers_projects WHERE developer_id = 1").empty? end - def test_additional_columns_from_join_table - assert_date_from_db Date.new(2004, 10, 10), Developer.find(1).projects.first.joined_on.to_date - end - def test_destroying david = Developer.find(1) active_record = Project.find(1) @@ -675,25 +611,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_respond_to categories(:technology).select_testing_posts.find(:first), :correctness_marker end - def test_updating_attributes_on_rich_associations - david = projects(:action_controller).developers.first - david.name = "DHH" - assert_raise(ActiveRecord::ReadOnlyRecord) { david.save! } - end - - def test_updating_attributes_on_rich_associations_with_limited_find_from_reflection - david = projects(:action_controller).selected_developers.first - david.name = "DHH" - assert_nothing_raised { david.save! } - end - - - def test_updating_attributes_on_rich_associations_with_limited_find - david = projects(:action_controller).developers.find(:all, :select => "developers.*").first - david.name = "DHH" - assert david.save! - end - def test_join_table_alias assert_equal 3, Developer.find(:all, :include => {:projects => :developers}, :conditions => 'developers_projects_join.joined_on IS NOT NULL').size end diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb index 8d694900ff..e21109baae 100644 --- a/activerecord/test/cases/readonly_test.rb +++ b/activerecord/test/cases/readonly_test.rb @@ -44,15 +44,6 @@ class ReadOnlyTest < ActiveRecord::TestCase Developer.joins(', projects').readonly(false).each { |d| assert !d.readonly? } end - - def test_habtm_find_readonly - dev = Developer.find(1) - assert !dev.projects.empty? - assert dev.projects.all?(&:readonly?) - assert dev.projects.find(:all).all?(&:readonly?) - assert dev.projects.readonly(true).all?(&:readonly?) - end - def test_has_many_find_readonly post = Post.find(1) assert !post.comments.empty? -- cgit v1.2.3