aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md15
-rw-r--r--activerecord/lib/active_record/associations/join_dependency.rb17
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_adapter.rb19
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb14
-rw-r--r--activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb18
-rw-r--r--activerecord/lib/active_record/migration.rb95
-rw-r--r--activerecord/lib/active_record/querying.rb2
-rw-r--r--activerecord/lib/active_record/relation.rb2
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb42
-rw-r--r--activerecord/test/cases/adapters/mysql/connection_test.rb28
-rw-r--r--activerecord/test/cases/adapters/mysql2/connection_test.rb28
-rw-r--r--activerecord/test/cases/adapters/postgresql/connection_test.rb42
-rw-r--r--activerecord/test/cases/associations/left_outer_join_association_test.rb79
-rw-r--r--activerecord/test/cases/migration_test.rb97
-rw-r--r--activerecord/test/cases/relation/mutation_test.rb2
15 files changed, 471 insertions, 29 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index bee9a099af..97df382bb1 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,18 @@
+* Use advisory locking to raise a ConcurrentMigrationError instead of
+ attempting to migrate when another migration is currently running.
+
+ *Sam Davies*
+
+* Added `ActiveRecord::Relation#left_outer_joins`.
+
+ Example:
+
+ User.left_outer_joins(:posts)
+ # => SELECT "users".* FROM "users" LEFT OUTER JOIN "posts" ON
+ "posts"."user_id" = "users"."id"
+
+ *Florian Thomas*
+
* Support passing an array to `order` for SQL parameter sanitization.
*Aaron Suggs*
diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index 0e98a3b3a4..9f183c3e7e 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -103,9 +103,14 @@ module ActiveRecord
join_root.drop(1).map!(&:reflection)
end
- def join_constraints(outer_joins)
+ def join_constraints(outer_joins, join_type)
joins = join_root.children.flat_map { |child|
- make_inner_joins join_root, child
+
+ if join_type == Arel::Nodes::OuterJoin
+ make_left_outer_joins join_root, child
+ else
+ make_inner_joins join_root, child
+ end
}
joins.concat outer_joins.flat_map { |oj|
@@ -176,6 +181,14 @@ module ActiveRecord
[info] + child.children.flat_map { |c| make_outer_joins(child, c) }
end
+ def make_left_outer_joins(parent, child)
+ tables = child.tables
+ join_type = Arel::Nodes::OuterJoin
+ info = make_constraints parent, child, tables, join_type
+
+ [info] + child.children.flat_map { |c| make_left_outer_joins(child, c) }
+ end
+
def make_inner_joins(parent, child)
tables = child.tables
join_type = Arel::Nodes::InnerJoin
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 402159ac13..f5b2e9fa9d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -214,6 +214,11 @@ module ActiveRecord
false
end
+ # Does this adapter support application-enforced advisory locking?
+ def supports_advisory_locks?
+ false
+ end
+
# Should primary key values be selected from their corresponding
# sequence before the insert statement? If true, next_sequence_value
# is called before each insert to set the record's primary key.
@@ -280,6 +285,20 @@ module ActiveRecord
def enable_extension(name)
end
+ # This is meant to be implemented by the adapters that support advisory
+ # locks
+ #
+ # Return true if we got the lock, otherwise false
+ def get_advisory_lock(key) # :nodoc:
+ end
+
+ # This is meant to be implemented by the adapters that support advisory
+ # locks.
+ #
+ # Return true if we released the lock, otherwise false
+ def release_advisory_lock(key) # :nodoc:
+ end
+
# A list of extensions, to be filled in by adapters that support them.
def extensions
[]
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 251acf1c83..b775c18c1b 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -220,6 +220,20 @@ module ActiveRecord
version >= '5.6.4'
end
+ # 5.0.0 definitely supports it, possibly supported by earlier versions but
+ # not sure
+ def supports_advisory_locks?
+ version >= '5.0.0'
+ end
+
+ def get_advisory_lock(key, timeout = 0) # :nodoc:
+ select_value("SELECT GET_LOCK('#{key}', #{timeout});").to_s == '1'
+ end
+
+ def release_advisory_lock(key) # :nodoc:
+ select_value("SELECT RELEASE_LOCK('#{key}')").to_s == '1'
+ end
+
def native_database_types
NATIVE_DATABASE_TYPES
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 787dadfdbf..6200cc8d29 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -284,6 +284,10 @@ module ActiveRecord
true
end
+ def supports_advisory_locks?
+ true
+ end
+
def supports_explain?
true
end
@@ -302,6 +306,20 @@ module ActiveRecord
postgresql_version >= 90300
end
+ def get_advisory_lock(key) # :nodoc:
+ unless key.is_a?(Integer) && key.bit_length <= 63
+ raise(ArgumentError, "Postgres requires advisory lock keys to be a signed 64 bit integer")
+ end
+ select_value("SELECT pg_try_advisory_lock(#{key});")
+ end
+
+ def release_advisory_lock(key) # :nodoc:
+ unless key.is_a?(Integer) && key.bit_length <= 63
+ raise(ArgumentError, "Postgres requires advisory lock keys to be a signed 64 bit integer")
+ end
+ select_value("SELECT pg_advisory_unlock(#{key})")
+ end
+
def enable_extension(name)
exec_query("CREATE EXTENSION IF NOT EXISTS \"#{name}\"").tap {
reload_type_map
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index c8b96b8de0..63ec8f6745 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -135,6 +135,14 @@ module ActiveRecord
end
end
+ class ConcurrentMigrationError < MigrationError #:nodoc:
+ DEFAULT_MESSAGE = "Cannot run migrations because another migration process is currently running.".freeze
+
+ def initialize(message = DEFAULT_MESSAGE)
+ super
+ end
+ end
+
# = Active Record Migrations
#
# Migrations can manage the evolution of a schema used by several physical
@@ -1042,32 +1050,18 @@ module ActiveRecord
alias :current :current_migration
def run
- migration = migrations.detect { |m| m.version == @target_version }
- raise UnknownMigrationVersionError.new(@target_version) if migration.nil?
- unless (up? && migrated.include?(migration.version.to_i)) || (down? && !migrated.include?(migration.version.to_i))
- begin
- execute_migration_in_transaction(migration, @direction)
- rescue => e
- canceled_msg = use_transaction?(migration) ? ", this migration was canceled" : ""
- raise StandardError, "An error has occurred#{canceled_msg}:\n\n#{e}", e.backtrace
- end
+ if use_advisory_lock?
+ with_advisory_lock { run_without_lock }
+ else
+ run_without_lock
end
end
def migrate
- if !target && @target_version && @target_version > 0
- raise UnknownMigrationVersionError.new(@target_version)
- end
-
- runnable.each do |migration|
- Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
-
- begin
- execute_migration_in_transaction(migration, @direction)
- rescue => e
- canceled_msg = use_transaction?(migration) ? "this and " : ""
- raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace
- end
+ if use_advisory_lock?
+ with_advisory_lock { migrate_without_lock }
+ else
+ migrate_without_lock
end
end
@@ -1092,10 +1086,45 @@ module ActiveRecord
end
def migrated
- @migrated_versions ||= Set.new(self.class.get_all_versions)
+ @migrated_versions || load_migrated
+ end
+
+ def load_migrated
+ @migrated_versions = Set.new(self.class.get_all_versions)
end
private
+
+ def run_without_lock
+ migration = migrations.detect { |m| m.version == @target_version }
+ raise UnknownMigrationVersionError.new(@target_version) if migration.nil?
+ unless (up? && migrated.include?(migration.version.to_i)) || (down? && !migrated.include?(migration.version.to_i))
+ begin
+ execute_migration_in_transaction(migration, @direction)
+ rescue => e
+ canceled_msg = use_transaction?(migration) ? ", this migration was canceled" : ""
+ raise StandardError, "An error has occurred#{canceled_msg}:\n\n#{e}", e.backtrace
+ end
+ end
+ end
+
+ def migrate_without_lock
+ if !target && @target_version && @target_version > 0
+ raise UnknownMigrationVersionError.new(@target_version)
+ end
+
+ runnable.each do |migration|
+ Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
+
+ begin
+ execute_migration_in_transaction(migration, @direction)
+ rescue => e
+ canceled_msg = use_transaction?(migration) ? "this and " : ""
+ raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace
+ end
+ end
+ end
+
def ran?(migration)
migrated.include?(migration.version.to_i)
end
@@ -1157,5 +1186,25 @@ module ActiveRecord
def use_transaction?(migration)
!migration.disable_ddl_transaction && Base.connection.supports_ddl_transactions?
end
+
+ def use_advisory_lock?
+ Base.connection.supports_advisory_locks?
+ end
+
+ def with_advisory_lock
+ key = generate_migrator_advisory_lock_key
+ got_lock = Base.connection.get_advisory_lock(key)
+ raise ConcurrentMigrationError unless got_lock
+ load_migrated # reload schema_migrations to be sure it wasn't changed by another process before we got the lock
+ yield
+ ensure
+ Base.connection.release_advisory_lock(key) if got_lock
+ end
+
+ MIGRATOR_SALT = 2053462845
+ def generate_migrator_advisory_lock_key
+ db_name_hash = Zlib.crc32(Base.connection.current_database)
+ MIGRATOR_SALT * db_name_hash
+ end
end
end
diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb
index 87a1988f2f..1f429cfd94 100644
--- a/activerecord/lib/active_record/querying.rb
+++ b/activerecord/lib/active_record/querying.rb
@@ -7,7 +7,7 @@ module ActiveRecord
delegate :find_by, :find_by!, to: :all
delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, to: :all
delegate :find_each, :find_in_batches, :in_batches, to: :all
- delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :or,
+ delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or,
:where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly,
:having, :create_with, :uniq, :distinct, :references, :none, :unscope, to: :all
delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 392b462aa9..f100476374 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -4,7 +4,7 @@ module ActiveRecord
# = Active Record \Relation
class Relation
MULTI_VALUE_METHODS = [:includes, :eager_load, :preload, :select, :group,
- :order, :joins, :references,
+ :order, :joins, :left_joins, :left_outer_joins, :references,
:extending, :unscope]
SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering,
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index ad6c7fa2e5..800de78fe3 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -428,6 +428,27 @@ module ActiveRecord
self
end
+ # Performs a left outer joins on +args+:
+ #
+ # User.left_outer_joins(:posts)
+ # => SELECT "users".* FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" = "users"."id"
+ #
+ def left_outer_joins(*args)
+ check_if_method_has_arguments!(:left_outer_joins, args)
+
+ args.compact!
+ args.flatten!
+
+ spawn.left_outer_joins!(*args)
+ end
+ alias :left_joins :left_outer_joins
+
+ def left_outer_joins!(*args) # :nodoc:
+ self.left_outer_joins_values += args
+ self
+ end
+ alias :left_joins! :left_outer_joins!
+
# Returns a new relation, which is the result of filtering the current relation
# according to the conditions in the arguments.
#
@@ -878,6 +899,7 @@ module ActiveRecord
arel = Arel::SelectManager.new(table)
build_joins(arel, joins_values.flatten) unless joins_values.empty?
+ build_left_outer_joins(arel, left_outer_joins_values.flatten) unless left_outer_joins_values.empty?
arel.where(where_clause.ast) unless where_clause.empty?
arel.having(having_clause.ast) unless having_clause.empty?
@@ -937,6 +959,19 @@ module ActiveRecord
end
end
+ def build_left_outer_joins(manager, outer_joins)
+ buckets = outer_joins.group_by do |join|
+ case join
+ when Hash, Symbol, Array
+ :association_join
+ else
+ raise ArgumentError, 'only Hash, Symbol and Array are allowed'
+ end
+ end
+
+ build_join_query(manager, buckets, Arel::Nodes::OuterJoin)
+ end
+
def build_joins(manager, joins)
buckets = joins.group_by do |join|
case join
@@ -952,6 +987,11 @@ module ActiveRecord
raise 'unknown class: %s' % join.class.name
end
end
+
+ build_join_query(manager, buckets, Arel::Nodes::InnerJoin)
+ end
+
+ def build_join_query(manager, buckets, join_type)
buckets.default = []
association_joins = buckets[:association_join]
@@ -967,7 +1007,7 @@ module ActiveRecord
join_list
)
- join_infos = join_dependency.join_constraints stashed_association_joins
+ join_infos = join_dependency.join_constraints stashed_association_joins, join_type
join_infos.each do |info|
info.joins.each { |join| manager.from(join) }
diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb
index decac9e83b..75653ee9af 100644
--- a/activerecord/test/cases/adapters/mysql/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql/connection_test.rb
@@ -170,6 +170,34 @@ class MysqlConnectionTest < ActiveRecord::MysqlTestCase
end
end
+ def test_get_and_release_advisory_lock
+ key = "test_key"
+
+ got_lock = @connection.get_advisory_lock(key)
+ assert got_lock, "get_advisory_lock should have returned true but it didn't"
+
+ assert_equal test_lock_free(key), false,
+ "expected the test advisory lock to be held but it wasn't"
+
+ released_lock = @connection.release_advisory_lock(key)
+ assert released_lock, "expected release_advisory_lock to return true but it didn't"
+
+ assert test_lock_free(key), 'expected the test key to be available after releasing'
+ end
+
+ def test_release_non_existent_advisory_lock
+ fake_key = "fake_key"
+ released_non_existent_lock = @connection.release_advisory_lock(fake_key)
+ assert_equal released_non_existent_lock, false,
+ 'expected release_advisory_lock to return false when there was no lock to release'
+ end
+
+ protected
+
+ def test_lock_free(key)
+ @connection.select_value("SELECT IS_FREE_LOCK('#{key}');") == '1'
+ end
+
private
def with_example_table(&block)
diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb
index 000bcadebe..71c4028675 100644
--- a/activerecord/test/cases/adapters/mysql2/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb
@@ -131,4 +131,32 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase
ensure
@connection.execute "DROP TABLE `bar_baz`"
end
+
+ def test_get_and_release_advisory_lock
+ key = "test_key"
+
+ got_lock = @connection.get_advisory_lock(key)
+ assert got_lock, "get_advisory_lock should have returned true but it didn't"
+
+ assert_equal test_lock_free(key), false,
+ "expected the test advisory lock to be held but it wasn't"
+
+ released_lock = @connection.release_advisory_lock(key)
+ assert released_lock, "expected release_advisory_lock to return true but it didn't"
+
+ assert test_lock_free(key), 'expected the test key to be available after releasing'
+ end
+
+ def test_release_non_existent_advisory_lock
+ fake_key = "fake_key"
+ released_non_existent_lock = @connection.release_advisory_lock(fake_key)
+ assert_equal released_non_existent_lock, false,
+ 'expected release_advisory_lock to return false when there was no lock to release'
+ end
+
+ protected
+
+ def test_lock_free(key)
+ @connection.select_value("SELECT IS_FREE_LOCK('#{key}');") == 1
+ end
end
diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb
index 722e2377c1..b12beb91de 100644
--- a/activerecord/test/cases/adapters/postgresql/connection_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb
@@ -209,5 +209,47 @@ module ActiveRecord
ActiveRecord::Base.establish_connection(orig_connection.deep_merge({:variables => {:debug_print_plan => :default}}))
end
end
+
+ def test_get_and_release_advisory_lock
+ key = 5295901941911233559
+ list_advisory_locks = <<-SQL
+ SELECT locktype,
+ (classid::bigint << 32) | objid::bigint AS lock_key
+ FROM pg_locks
+ WHERE locktype = 'advisory'
+ SQL
+
+ got_lock = @connection.get_advisory_lock(key)
+ assert got_lock, "get_advisory_lock should have returned true but it didn't"
+
+ advisory_lock = @connection.query(list_advisory_locks).find {|l| l[1] == key}
+ assert advisory_lock,
+ "expected to find an advisory lock with key #{key} but there wasn't one"
+
+ released_lock = @connection.release_advisory_lock(key)
+ assert released_lock, "expected release_advisory_lock to return true but it didn't"
+
+ advisory_locks = @connection.query(list_advisory_locks).select {|l| l[1] == key}
+ assert_empty advisory_locks,
+ "expected to have released advisory lock with key #{key} but it was still held"
+ end
+
+ def test_release_non_existent_advisory_lock
+ fake_key = 2940075057017742022
+ with_warning_suppression do
+ released_non_existent_lock = @connection.release_advisory_lock(fake_key)
+ assert_equal released_non_existent_lock, false,
+ 'expected release_advisory_lock to return false when there was no lock to release'
+ end
+ end
+
+ protected
+
+ def with_warning_suppression
+ log_level = @connection.client_min_messages
+ @connection.client_min_messages = 'error'
+ yield
+ @connection.client_min_messages = log_level
+ end
end
end
diff --git a/activerecord/test/cases/associations/left_outer_join_association_test.rb b/activerecord/test/cases/associations/left_outer_join_association_test.rb
new file mode 100644
index 0000000000..4af791b758
--- /dev/null
+++ b/activerecord/test/cases/associations/left_outer_join_association_test.rb
@@ -0,0 +1,79 @@
+require "cases/helper"
+require 'models/post'
+require 'models/comment'
+require 'models/author'
+require 'models/essay'
+require 'models/categorization'
+require 'models/person'
+
+class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
+ fixtures :authors, :essays, :posts, :comments, :categorizations, :people
+
+ def test_construct_finder_sql_applies_aliases_tables_on_association_conditions
+ result = Author.left_outer_joins(:thinking_posts, :welcome_posts).to_a
+ assert_equal authors(:david), result.first
+ end
+
+ def test_construct_finder_sql_does_not_table_name_collide_on_duplicate_associations
+ assert_nothing_raised do
+ queries = capture_sql do
+ Person.left_outer_joins(:agents => {:agents => :agents})
+ .left_outer_joins(:agents => {:agents => {:primary_contact => :agents}}).to_a
+ end
+ assert queries.any? { |sql| /agents_people_4/i =~ sql }
+ end
+ end
+
+ def test_construct_finder_sql_executes_a_left_outer_join
+ assert_not_equal Author.count, Author.joins(:posts).count
+ assert_equal Author.count, Author.left_outer_joins(:posts).count
+ end
+
+ def test_left_outer_join_by_left_joins
+ assert_not_equal Author.count, Author.joins(:posts).count
+ assert_equal Author.count, Author.left_joins(:posts).count
+ end
+
+ def test_construct_finder_sql_ignores_empty_left_outer_joins_hash
+ queries = capture_sql { Author.left_outer_joins({}) }
+ assert queries.none? { |sql| /LEFT OUTER JOIN/i =~ sql }
+ end
+
+ def test_construct_finder_sql_ignores_empty_left_outer_joins_array
+ queries = capture_sql { Author.left_outer_joins([]) }
+ assert queries.none? { |sql| /LEFT OUTER JOIN/i =~ sql }
+ end
+
+ def test_left_outer_joins_forbids_to_use_string_as_argument
+ assert_raise(ArgumentError){ Author.left_outer_joins('LEFT OUTER JOIN "posts" ON "posts"."user_id" = "users"."id"').to_a }
+ end
+
+ def test_join_conditions_added_to_join_clause
+ queries = capture_sql { Author.left_outer_joins(:essays).to_a }
+ assert queries.any? { |sql| /writer_type.*?=.*?(Author|\?|\$1)/i =~ sql }
+ assert queries.none? { |sql| /WHERE/i =~ sql }
+ end
+
+ def test_find_with_sti_join
+ scope = Post.left_outer_joins(:special_comments).where(:id => posts(:sti_comments).id)
+
+ # The join should match SpecialComment and its subclasses only
+ assert scope.where("comments.type" => "Comment").empty?
+ assert !scope.where("comments.type" => "SpecialComment").empty?
+ assert !scope.where("comments.type" => "SubSpecialComment").empty?
+ end
+
+ def test_does_not_override_select
+ authors = Author.select("authors.name, #{%{(authors.author_address_id || ' ' || authors.author_address_extra_id) as addr_id}}").left_outer_joins(:posts)
+ assert authors.any?
+ assert authors.first.respond_to?(:addr_id)
+ end
+
+ test "the default scope of the target is applied when joining associations" do
+ author = Author.create! name: "Jon"
+ author.categorizations.create!
+ author.categorizations.create! special: true
+
+ assert_equal [author], Author.where(id: author).left_outer_joins(:special_categorizations)
+ end
+end
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 10f1c7216f..741bec6017 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -522,6 +522,79 @@ class MigrationTest < ActiveRecord::TestCase
end
end
+ if ActiveRecord::Base.connection.supports_advisory_locks?
+ def test_migrator_generates_valid_lock_key
+ migration = Class.new(ActiveRecord::Migration).new
+ migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
+
+ lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+
+ assert ActiveRecord::Base.connection.get_advisory_lock(lock_key),
+ "the Migrator should have generated a valid lock key, but it didn't"
+ assert ActiveRecord::Base.connection.release_advisory_lock(lock_key),
+ "the Migrator should have generated a valid lock key, but it didn't"
+ end
+
+ def test_generate_migrator_advisory_lock_key
+ # It is important we are consistent with how we generate this so that
+ # exclusive locking works across migrator versions
+ migration = Class.new(ActiveRecord::Migration).new
+ migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
+
+ lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+
+ current_database = ActiveRecord::Base.connection.current_database
+ salt = ActiveRecord::Migrator::MIGRATOR_SALT
+ expected_key = Zlib.crc32(current_database) * salt
+
+ assert lock_key == expected_key, "expected lock key generated by the migrator to be #{expected_key}, but it was #{lock_key} instead"
+ assert lock_key.is_a?(Fixnum), "expected lock key to be a Fixnum, but it wasn't"
+ assert lock_key.bit_length <= 63, "lock key must be a signed integer of max 63 bits magnitude"
+ end
+
+ def test_migrator_one_up_with_unavailable_lock
+ assert_no_column Person, :last_name
+
+ migration = Class.new(ActiveRecord::Migration) {
+ def version; 100 end
+ def migrate(x)
+ add_column "people", "last_name", :string
+ end
+ }.new
+
+ migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
+ lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+
+ with_another_process_holding_lock(lock_key) do
+ assert_raise(ActiveRecord::ConcurrentMigrationError) { migrator.migrate }
+ end
+
+ assert_no_column Person, :last_name,
+ "without an advisory lock, the Migrator should not make any changes, but it did."
+ end
+
+ def test_migrator_one_up_with_unavailable_lock_using_run
+ assert_no_column Person, :last_name
+
+ migration = Class.new(ActiveRecord::Migration) {
+ def version; 100 end
+ def migrate(x)
+ add_column "people", "last_name", :string
+ end
+ }.new
+
+ migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
+ lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+
+ with_another_process_holding_lock(lock_key) do
+ assert_raise(ActiveRecord::ConcurrentMigrationError) { migrator.run }
+ end
+
+ assert_no_column Person, :last_name,
+ "without an advisory lock, the Migrator should not make any changes, but it did."
+ end
+ end
+
protected
# This is needed to isolate class_attribute assignments like `table_name_prefix`
# for each test case.
@@ -531,6 +604,30 @@ class MigrationTest < ActiveRecord::TestCase
def self.base_class; self; end
}
end
+
+ def with_another_process_holding_lock(lock_key)
+ thread_lock = Concurrent::CountDownLatch.new
+ test_terminated = Concurrent::CountDownLatch.new
+
+ other_process = Thread.new do
+ begin
+ conn = ActiveRecord::Base.connection_pool.checkout
+ conn.get_advisory_lock(lock_key)
+ thread_lock.count_down
+ test_terminated.wait # hold the lock open until we tested everything
+ ensure
+ conn.release_advisory_lock(lock_key)
+ ActiveRecord::Base.connection_pool.checkin(conn)
+ end
+ end
+
+ thread_lock.wait # wait until the 'other process' has the lock
+
+ yield
+
+ test_terminated.count_down
+ other_process.join
+ end
end
class ReservedWordsMigrationTest < ActiveRecord::TestCase
diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb
index 88d2dd55ab..cc0034ffd1 100644
--- a/activerecord/test/cases/relation/mutation_test.rb
+++ b/activerecord/test/cases/relation/mutation_test.rb
@@ -28,7 +28,7 @@ module ActiveRecord
@relation ||= Relation.new FakeKlass.new('posts'), Post.arel_table, Post.predicate_builder
end
- (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select]).each do |method|
+ (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select, :left_joins]).each do |method|
test "##{method}!" do
assert relation.public_send("#{method}!", :foo).equal?(relation)
assert_equal [:foo], relation.public_send("#{method}_values")