diff options
Diffstat (limited to 'activerecord')
10 files changed, 53 insertions, 42 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 11d4b4a503..4861872129 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -105,13 +105,9 @@ module ActiveRecord !ActiveRecord::SchemaDumper.fk_ignore_pattern.match?(name) if name end - def defined_for?(to_table_ord = nil, to_table: nil, **options) - if to_table_ord - self.to_table == to_table_ord.to_s - else - (to_table.nil? || to_table.to_s == self.to_table) && - options.all? { |k, v| self.options[k].to_s == v.to_s } - end + def defined_for?(to_table: nil, **options) + (to_table.nil? || to_table.to_s == self.to_table) && + options.all? { |k, v| self.options[k].to_s == v.to_s } end private diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index b2cc60f363..3dc81ffccf 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1002,10 +1002,10 @@ module ActiveRecord # with an addition of # [<tt>:to_table</tt>] # The name of the table that contains the referenced primary key. - def remove_foreign_key(from_table, options_or_to_table = {}) + def remove_foreign_key(from_table, to_table = nil, **options) return unless supports_foreign_keys? - fk_name_to_delete = foreign_key_for!(from_table, options_or_to_table).name + fk_name_to_delete = foreign_key_for!(from_table, to_table: to_table, **options).name at = create_alter_table from_table at.drop_foreign_key fk_name_to_delete @@ -1024,8 +1024,8 @@ module ActiveRecord # # Checks to see if a foreign key with a custom name exists. # foreign_key_exists?(:accounts, name: "special_fk_name") # - def foreign_key_exists?(from_table, options_or_to_table = {}) - foreign_key_for(from_table, options_or_to_table).present? + def foreign_key_exists?(from_table, to_table = nil, **options) + foreign_key_for(from_table, to_table: to_table, **options).present? end def foreign_key_column_for(table_name) # :nodoc: @@ -1341,14 +1341,14 @@ module ActiveRecord end end - def foreign_key_for(from_table, options_or_to_table = {}) + def foreign_key_for(from_table, **options) return unless supports_foreign_keys? - foreign_keys(from_table).detect { |fk| fk.defined_for? options_or_to_table } + foreign_keys(from_table).detect { |fk| fk.defined_for?(options) } end - def foreign_key_for!(from_table, options_or_to_table = {}) - foreign_key_for(from_table, options_or_to_table) || \ - raise(ArgumentError, "Table '#{from_table}' has no foreign key for #{options_or_to_table}") + def foreign_key_for!(from_table, to_table: nil, **options) + foreign_key_for(from_table, to_table: to_table, **options) || + raise(ArgumentError, "Table '#{from_table}' has no foreign key for #{to_table || options}") end def extract_foreign_key_action(specifier) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index d694a4f47d..a38c1325c0 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -623,10 +623,10 @@ module ActiveRecord # validate_foreign_key :accounts, name: :special_fk_name # # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key. - def validate_foreign_key(from_table, options_or_to_table = {}) + def validate_foreign_key(from_table, to_table = nil, **options) return unless supports_validate_constraints? - fk_name_to_validate = foreign_key_for!(from_table, options_or_to_table).name + fk_name_to_validate = foreign_key_for!(from_table, to_table: to_table, **options).name validate_constraint from_table, fk_name_to_validate end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index bd649451d6..e64e995e1a 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -72,7 +72,7 @@ module ActiveRecord table = strip_table_name_prefix_and_suffix(table) fk_to_table = strip_table_name_prefix_and_suffix(fk.to_table) fk_to_table == table && options.all? { |k, v| fk.options[k].to_s == v.to_s } - end || raise(ArgumentError, "Table '#{from_table}' has no foreign key for #{to_table}") + end || raise(ArgumentError, "Table '#{from_table}' has no foreign key for #{to_table || options}") foreign_keys.delete(fkey) alter_table(from_table, foreign_keys) diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index ca5ce11d79..5baaea344b 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -5,24 +5,24 @@ module ActiveRecord attr_reader :model, :connection, :inserts, :on_duplicate, :returning, :unique_by def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil) + raise ArgumentError, "Empty list of attributes passed" if inserts.blank? + @model, @connection, @inserts, @on_duplicate, @returning, @unique_by = model, model.connection, inserts, on_duplicate, returning, unique_by + @returning = (connection.supports_insert_returning? ? primary_keys : false) if @returning.nil? @returning = false if @returning == [] + @on_duplicate = :skip if @on_duplicate == :update && updatable_columns.empty? ensure_valid_options_for_connection! end def execute - if inserts.present? - connection.exec_query to_sql, "Bulk Insert" - else - ActiveRecord::Result.new([], []) - end + connection.exec_query to_sql, "Bulk Insert" end def keys - inserts.present? ? inserts.first.keys.map(&:to_s) : [] + inserts.first.keys.map(&:to_s) end def updatable_columns diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index c20274420f..997b7f763a 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -308,7 +308,7 @@ module ActiveRecord # named +column_name+ from the table called +table_name+. # * <tt>remove_columns(table_name, *column_names)</tt>: Removes the given # columns from the table definition. - # * <tt>remove_foreign_key(from_table, options_or_to_table)</tt>: Removes the + # * <tt>remove_foreign_key(from_table, to_table = nil, **options)</tt>: Removes the # given foreign key from the table called +table_name+. # * <tt>remove_index(table_name, column: column_names)</tt>: Removes the index # specified by +column_names+. diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 82f5121d94..8e7f596076 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -231,24 +231,15 @@ module ActiveRecord end def invert_remove_foreign_key(args) - from_table, options_or_to_table, options_or_nil = args + options = args.extract_options! + from_table, to_table = args - to_table = if options_or_to_table.is_a?(Hash) - options_or_to_table[:to_table] - else - options_or_to_table - end - - remove_options = if options_or_to_table.is_a?(Hash) - options_or_to_table.except(:to_table) - else - options_or_nil - end + to_table ||= options.delete(:to_table) raise ActiveRecord::IrreversibleMigration, "remove_foreign_key is only reversible if given a second table" if to_table.nil? reversed_args = [from_table, to_table] - reversed_args << remove_options if remove_options.present? + reversed_args << options unless options.empty? [:add_foreign_key, reversed_args] end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index ef02ca8190..a8433fa0db 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -142,7 +142,7 @@ module ActiveRecord end def for_each - databases = Rails.application.config.database_configuration + databases = Rails.application.config.load_database_yaml database_configs = ActiveRecord::DatabaseConfigurations.new(databases).configs_for(env_name: Rails.env) # if this is a single database application we don't want tasks for each primary database diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index e0d79d2fab..63245719c0 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -7,7 +7,7 @@ class ReadonlyNameBook < Book attr_readonly :name end -class PersistenceTest < ActiveRecord::TestCase +class InsertAllTest < ActiveRecord::TestCase fixtures :books def test_insert @@ -34,7 +34,7 @@ class PersistenceTest < ActiveRecord::TestCase end def test_insert_all_should_handle_empty_arrays - assert_no_difference "Book.count" do + assert_raise ArgumentError do Book.insert_all! [] end end @@ -56,30 +56,35 @@ class PersistenceTest < ActiveRecord::TestCase def test_insert_all_returns_primary_key_if_returning_is_supported skip unless supports_insert_returning? + result = Book.insert_all! [{ name: "Rework", author_id: 1 }] assert_equal %w[ id ], result.columns end def test_insert_all_returns_nothing_if_returning_is_empty skip unless supports_insert_returning? + result = Book.insert_all! [{ name: "Rework", author_id: 1 }], returning: [] assert_equal [], result.columns end def test_insert_all_returns_nothing_if_returning_is_false skip unless supports_insert_returning? + result = Book.insert_all! [{ name: "Rework", author_id: 1 }], returning: false assert_equal [], result.columns end def test_insert_all_returns_requested_fields skip unless supports_insert_returning? + result = Book.insert_all! [{ name: "Rework", author_id: 1 }], returning: [:id, :name] assert_equal %w[ Rework ], result.pluck("name") end def test_insert_all_can_skip_duplicate_records skip unless supports_insert_on_duplicate_skip? + assert_no_difference "Book.count" do Book.insert_all [{ id: 1, name: "Agile Web Development with Rails" }] end @@ -87,6 +92,7 @@ class PersistenceTest < ActiveRecord::TestCase def test_insert_all_will_raise_if_duplicates_are_skipped_only_for_a_certain_conflict_target skip unless supports_insert_on_duplicate_skip? && supports_insert_conflict_target? + assert_raise ActiveRecord::RecordNotUnique do Book.insert_all [{ id: 1, name: "Agile Web Development with Rails" }], unique_by: { columns: %i{author_id name} } @@ -95,6 +101,7 @@ class PersistenceTest < ActiveRecord::TestCase def test_upsert_all_updates_existing_records skip unless supports_insert_on_duplicate_update? + new_name = "Agile Web Development with Rails, 4th Edition" Book.upsert_all [{ id: 1, name: new_name }] assert_equal new_name, Book.find(1).name @@ -102,6 +109,7 @@ class PersistenceTest < ActiveRecord::TestCase def test_upsert_all_does_not_update_readonly_attributes skip unless supports_insert_on_duplicate_update? + new_name = "Agile Web Development with Rails, 4th Edition" ReadonlyNameBook.upsert_all [{ id: 1, name: new_name }] assert_not_equal new_name, Book.find(1).name @@ -109,9 +117,11 @@ class PersistenceTest < ActiveRecord::TestCase def test_upsert_all_does_not_update_primary_keys skip unless supports_insert_on_duplicate_update? && supports_insert_conflict_target? + Book.upsert_all [{ id: 101, name: "Perelandra", author_id: 7 }] Book.upsert_all [{ id: 103, name: "Perelandra", author_id: 7, isbn: "1974522598" }], unique_by: { columns: %i{author_id name} } + book = Book.find_by(name: "Perelandra") assert_equal 101, book.id, "Should not have updated the ID" assert_equal "1974522598", book.isbn, "Should have updated the isbn" @@ -119,9 +129,11 @@ class PersistenceTest < ActiveRecord::TestCase def test_upsert_all_does_not_perform_an_upsert_if_a_partial_index_doesnt_apply skip unless supports_insert_on_duplicate_update? && supports_insert_conflict_target? && supports_partial_index? + Book.upsert_all [{ name: "Out of the Silent Planet", author_id: 7, isbn: "1974522598", published_on: Date.new(1938, 4, 1) }] Book.upsert_all [{ name: "Perelandra", author_id: 7, isbn: "1974522598" }], unique_by: { columns: %w[ isbn ], where: "published_on IS NOT NULL" } + assert_equal ["Out of the Silent Planet", "Perelandra"], Book.where(isbn: "1974522598").order(:name).pluck(:name) end end diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index 6547ebb5d1..da82f62406 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -385,6 +385,18 @@ if ActiveRecord::Base.connection.supports_foreign_keys? assert_equal "Table 'astronauts' has no foreign key for rockets", e.message end + def test_remove_foreign_key_by_the_select_one_on_the_same_table + @connection.add_foreign_key :astronauts, :rockets + @connection.add_reference :astronauts, :myrocket, foreign_key: { to_table: :rockets } + + assert_equal 2, @connection.foreign_keys("astronauts").size + + @connection.remove_foreign_key :astronauts, :rockets, column: "myrocket_id" + + assert_equal [["astronauts", "rockets", "rocket_id"]], + @connection.foreign_keys("astronauts").map { |fk| [fk.from_table, fk.to_table, fk.column] } + end + if ActiveRecord::Base.connection.supports_validate_constraints? def test_add_invalid_foreign_key @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false |