diff options
15 files changed, 95 insertions, 57 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 95dc680c67..7d310294bd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -70,7 +70,7 @@ PATH i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) - zeitwerk (~> 1.3, >= 1.3.1) + zeitwerk (~> 1.3, >= 1.3.2) rails (6.0.0.beta2) actioncable (= 6.0.0.beta2) actionmailbox (= 6.0.0.beta2) @@ -517,7 +517,7 @@ GEM websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (1.3.1) + zeitwerk (1.3.2) PLATFORMS java 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 diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index c92691afaf..a9d08c7e85 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -34,5 +34,5 @@ Gem::Specification.new do |s| s.add_dependency "tzinfo", "~> 1.1" s.add_dependency "minitest", "~> 5.1" s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2" - s.add_dependency "zeitwerk", "~> 1.3", ">= 1.3.1" + s.add_dependency "zeitwerk", "~> 1.3", ">= 1.3.2" end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index af3ec36064..83a7b6cf01 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -184,6 +184,26 @@ module Rails end end + # Load the database YAML without evaluating ERB. This allows us to + # create the rake tasks for multiple databases without filling in the + # configuration values or loading the environment. Do not use this + # method. + # + # This uses a DummyERB custom compiler so YAML can ignore the ERB + # tags and load the database.yml for the rake tasks. + def load_database_yaml # :nodoc: + if path = paths["config/database"].existent.first + require "rails/application/dummy_erb_compiler" + + yaml = Pathname.new(path) + erb = DummyERB.new(yaml.read) + + YAML.load(erb.result) + else + {} + end + end + # Loads and returns the entire raw configuration of database from # values stored in <tt>config/database.yml</tt>. def database_configuration diff --git a/railties/lib/rails/application/dummy_erb_compiler.rb b/railties/lib/rails/application/dummy_erb_compiler.rb new file mode 100644 index 0000000000..4e92526969 --- /dev/null +++ b/railties/lib/rails/application/dummy_erb_compiler.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# These classes are used to strip out the ERB configuration +# values so we can evaluate the database.yml without evaluating +# the ERB values. +class DummyERB < ERB # :nodoc: + def make_compiler(trim_mode) + DummyCompiler.new trim_mode + end +end + +class DummyCompiler < ERB::Compiler # :nodoc: + def compile_content(stag, out) + out.push "_erbout << 'dummy_compiler'" + end +end diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index a8ec1f3663..07d57dea8a 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -54,26 +54,17 @@ module ApplicationTests test "db:create and db:drop respect environment setting" do app_file "config/database.yml", <<-YAML development: - database: db/development.sqlite3 + database: <%= Rails.application.config.database %> adapter: sqlite3 YAML app_file "config/environments/development.rb", <<-RUBY Rails.application.configure do - config.read_encrypted_secrets = true + config.database = "db/development.sqlite3" end RUBY - app_file "lib/tasks/check_env.rake", <<-RUBY - Rake::Task["db:create"].enhance do - File.write("tmp/config_value", Rails.application.config.read_encrypted_secrets) - end - RUBY - - db_create_and_drop("db/development.sqlite3", environment_loaded: false) do - assert File.exist?("tmp/config_value") - assert_equal "true", File.read("tmp/config_value") - end + db_create_and_drop("db/development.sqlite3", environment_loaded: false) end def with_database_existing |