diff options
Diffstat (limited to 'activerecord')
19 files changed, 111 insertions, 99 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 14a6c3c9f7..dfd9a07997 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Hashes can once again be passed to setters of `composed_of`, if all of the + mapping methods are methods implemented on `Hash`. + + Fixes #25978. + + *Sean Griffin* + * Fix the SELECT statement in `#table_comment` for MySQL. *Takeshi Akima* diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index 8bed5bca28..9f965052c5 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -261,8 +261,10 @@ module ActiveRecord part = converter.respond_to?(:call) ? converter.call(part) : klass.send(converter, part) end - if part.is_a?(Hash) - raise ArgumentError unless part.size == part.keys.max + hash_from_multiparameter_assignment = part.is_a?(Hash) && + part.each_key.all? { |k| k.is_a?(Integer) } + if hash_from_multiparameter_assignment + raise ArgumentError unless part.size == part.each_key.max part = klass.new(*part.sort.map(&:last)) end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 0eaa0a4f36..c8805d107b 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -152,9 +152,7 @@ module ActiveRecord if loaded? n ? target.take(n) : target.first else - scope.take(n).tap do |record| - set_inverse_instance record if record.is_a? ActiveRecord::Base - end + scope.take(n) end end @@ -457,23 +455,20 @@ module ActiveRecord end private - def get_records - return scope.to_a if skip_statement_cache? - - conn = klass.connection - sc = reflection.association_scope_cache(conn, owner) do - StatementCache.create(conn) { |params| - as = AssociationScope.create { params.bind } - target_scope.merge as.scope(self, conn) - } - end - - binds = AssociationScope.get_bind_values(owner, reflection.chain) - sc.execute binds, klass, klass.connection - end def find_target - records = get_records + return scope.to_a if skip_statement_cache? + + conn = klass.connection + sc = reflection.association_scope_cache(conn, owner) do + StatementCache.create(conn) { |params| + as = AssociationScope.create { params.bind } + target_scope.merge as.scope(self, conn) + } + end + + binds = AssociationScope.get_bind_values(owner, reflection.chain) + records = sc.execute(binds, klass, conn) records.each { |record| set_inverse_instance(record) } records end @@ -656,9 +651,7 @@ module ActiveRecord args.shift if args.first.is_a?(Hash) && args.first.empty? collection = fetch_first_nth_or_last_using_find?(args) ? scope : load_target - collection.send(type, *args).tap do |record| - set_inverse_instance record if record.is_a? ActiveRecord::Base - end + collection.send(type, *args) end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 36fc381343..ddd0b59cc1 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -196,7 +196,7 @@ module ActiveRecord def find_target return [] unless target_reflection_has_associated_record? - get_records + super end # NOTE - not sure that we can actually cope with inverses here diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index f913f0852a..1fe9a23263 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -44,8 +44,8 @@ module ActiveRecord scope.scope_for_create.stringify_keys.except(klass.primary_key) end - def get_records - return scope.limit(1).records if skip_statement_cache? + def find_target + return scope.take if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do @@ -56,11 +56,7 @@ module ActiveRecord end binds = AssociationScope.get_bind_values(owner, reflection.chain) - sc.execute binds, klass, klass.connection - end - - def find_target - if record = get_records.first + if record = sc.execute(binds, klass, conn).first set_inverse_instance record end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index dc5b305843..526adc9efe 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -848,7 +848,6 @@ module ActiveRecord spec = resolver.spec(config) remove_connection(spec.name) - owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec) message_bus = ActiveSupport::Notifications.instrumenter payload = { diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index e667e16964..c2d02a6cc2 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -89,14 +89,14 @@ module ActiveRecord # Executes insert +sql+ statement in the context of this connection using # +binds+ as the bind substitutes. +name+ is logged along with # the executed +sql+ statement. - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) + def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil) exec_query(sql, name, binds) end # Executes delete +sql+ statement in the context of this connection using # +binds+ as the bind substitutes. +name+ is logged along with # the executed +sql+ statement. - def exec_delete(sql, name, binds) + def exec_delete(sql, name = nil, binds = []) exec_query(sql, name, binds) end @@ -108,7 +108,7 @@ module ActiveRecord # Executes update +sql+ statement in the context of this connection using # +binds+ as the bind substitutes. +name+ is logged along with # the executed +sql+ statement. - def exec_update(sql, name, binds) + def exec_update(sql, name = nil, binds = []) exec_query(sql, name, binds) end 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 353cae0f3d..19c265db6e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -785,7 +785,7 @@ module ActiveRecord # [<tt>:type</tt>] # The reference column type. Defaults to +:integer+. # [<tt>:index</tt>] - # Add an appropriate index. Defaults to false. + # Add an appropriate index. Defaults to true. # See #add_index for usage of this option. # [<tt>:foreign_key</tt>] # Add an appropriate foreign key constraint. Defaults to false. 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 610d78245f..5e9705e02f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -753,7 +753,7 @@ module ActiveRecord when ER_DATA_TOO_LONG ValueTooLong.new(message) when ER_LOCK_DEADLOCK - TransactionSerializationError.new(message) + Deadlocked.new(message) else super end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb index 6d1215df2a..fb0eda753f 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb @@ -45,7 +45,7 @@ module ActiveRecord end end - def exec_delete(sql, name, binds) + def exec_delete(sql, name = nil, binds = []) if without_prepared_statement?(binds) execute_and_free(sql, name) { @connection.affected_rows } else diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index f5232127c4..da533463bf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -112,7 +112,7 @@ module ActiveRecord end end - def exec_delete(sql, name = 'SQL', binds = []) + def exec_delete(sql, name = nil, binds = []) execute_and_clear(sql, name, binds) {|result| result.cmd_tuples } end alias :exec_update :exec_delete @@ -133,7 +133,7 @@ module ActiveRecord super end - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) + def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil) val = exec_query(sql, name, binds) if !use_insert_returning? && pk unless sequence_name diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 61a980fda9..8a1fdc9f92 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -407,6 +407,7 @@ module ActiveRecord FOREIGN_KEY_VIOLATION = "23503" UNIQUE_VIOLATION = "23505" SERIALIZATION_FAILURE = "40001" + DEADLOCK_DETECTED = "40P01" def translate_exception(exception, message) return exception unless exception.respond_to?(:result) @@ -419,7 +420,9 @@ module ActiveRecord when VALUE_LIMIT_VIOLATION ValueTooLong.new(message) when SERIALIZATION_FAILURE - TransactionSerializationError.new(message) + SerializationFailure.new(message) + when DEADLOCK_DETECTED + Deadlocked.new(message) else super end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 38e4fbec8b..03e6e1eee3 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -285,14 +285,24 @@ module ActiveRecord class TransactionIsolationError < ActiveRecordError end - # TransactionSerializationError will be raised when a transaction is rolled + # TransactionRollbackError will be raised when a transaction is rolled # back by the database due to a serialization failure or a deadlock. # # See the following: # # * http://www.postgresql.org/docs/current/static/transaction-iso.html # * https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html#error_er_lock_deadlock - class TransactionSerializationError < ActiveRecordError + class TransactionRollbackError < StatementInvalid + end + + # SerializationFailure will be raised when a transaction is rolled + # back by the database due to a serialization failure. + class SerializationFailure < TransactionRollbackError + end + + # Deadlocked will be raised when a transaction is rolled + # back by the database when a deadlock is encountered. + class Deadlocked < TransactionRollbackError end # IrreversibleOrderError is raised when a relation's order is too complex for diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 1ab4e0404f..254550c378 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -1,9 +1,5 @@ module ActiveRecord module NullRelation # :nodoc: - def exec_queries - @records = [].freeze - end - def pluck(*column_names) [] end @@ -20,10 +16,6 @@ module ActiveRecord 0 end - def size - calculate :size, nil - end - def empty? true end @@ -48,28 +40,8 @@ module ActiveRecord "" end - def count(*) - calculate :count, nil - end - - def sum(*) - calculate :sum, nil - end - - def average(*) - calculate :average, nil - end - - def minimum(*) - calculate :minimum, nil - end - - def maximum(*) - calculate :maximum, nil - end - def calculate(operation, _column_name) - if [:count, :sum, :size].include? operation + if [:count, :sum].include? operation group_values.any? ? Hash.new : 0 elsif [:average, :minimum, :maximum].include?(operation) && group_values.any? Hash.new @@ -85,5 +57,11 @@ module ActiveRecord def or(other) other.spawn end + + private + + def exec_queries + @records = [].freeze + end end end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index ec9f498c40..608d072e48 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -12,10 +12,9 @@ module ActiveRecord def validate_each(record, attribute, value) finder_class = find_finder_class_for(record) - table = finder_class.arel_table value = map_enum_attribute(finder_class, attribute, value) - relation = build_relation(finder_class, table, attribute, value) + relation = build_relation(finder_class, attribute, value) if record.persisted? if finder_class.primary_key relation = relation.where.not(finder_class.primary_key => record.id_was || record.id) @@ -23,7 +22,7 @@ module ActiveRecord raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") end end - relation = scope_relation(record, table, relation) + relation = scope_relation(record, relation) relation = relation.merge(options[:conditions]) if options[:conditions] if relation.exists? @@ -50,7 +49,7 @@ module ActiveRecord class_hierarchy.detect { |klass| !klass.abstract_class? } end - def build_relation(klass, table, attribute, value) #:nodoc: + def build_relation(klass, attribute, value) # :nodoc: if reflection = klass._reflect_on_association(attribute) attribute = reflection.foreign_key value = value.attributes[reflection.klass.primary_key] unless value.nil? @@ -63,6 +62,7 @@ module ActiveRecord attribute_name = attribute.to_s + table = klass.arel_table column = klass.columns_hash[attribute_name] cast_type = klass.type_for_attribute(attribute_name) @@ -80,7 +80,7 @@ module ActiveRecord end end - def scope_relation(record, table, relation) + def scope_relation(record, relation) Array(options[:scope]).each do |scope_item| if reflection = record.class._reflect_on_association(scope_item) scope_value = record.send(reflection.foreign_key) diff --git a/activerecord/test/cases/adapters/mysql2/transaction_test.rb b/activerecord/test/cases/adapters/mysql2/transaction_test.rb index 0e37c70e5c..5d8765d63a 100644 --- a/activerecord/test/cases/adapters/mysql2/transaction_test.rb +++ b/activerecord/test/cases/adapters/mysql2/transaction_test.rb @@ -27,32 +27,25 @@ module ActiveRecord @connection.drop_table 'samples', if_exists: true end - test "raises error when a serialization failure occurs" do - assert_raises(ActiveRecord::TransactionSerializationError) do - thread = Thread.new do - Sample.transaction isolation: :serializable do - Sample.delete_all - - 10.times do |i| - sleep 0.1 + test "raises Deadlocked when a deadlock is encountered" do + assert_raises(ActiveRecord::Deadlocked) do + s1 = Sample.create value: 1 + s2 = Sample.create value: 2 - Sample.create value: i - end + thread = Thread.new do + Sample.transaction do + s1.lock! + sleep 1 + s2.update_attributes value: 1 end end - sleep 0.1 - - Sample.transaction isolation: :serializable do - Sample.delete_all - - 10.times do |i| - sleep 0.1 - - Sample.create value: i - end + sleep 0.5 + Sample.transaction do + s2.lock! sleep 1 + s1.update_attributes value: 2 end thread.join diff --git a/activerecord/test/cases/adapters/postgresql/transaction_test.rb b/activerecord/test/cases/adapters/postgresql/transaction_test.rb index e76705a802..8dea92c785 100644 --- a/activerecord/test/cases/adapters/postgresql/transaction_test.rb +++ b/activerecord/test/cases/adapters/postgresql/transaction_test.rb @@ -26,9 +26,9 @@ module ActiveRecord @connection.drop_table 'samples', if_exists: true end - test "raises error when a serialization failure occurs" do + test "raises SerializationFailure when a serialization failure occurs" do with_warning_suppression do - assert_raises(ActiveRecord::TransactionSerializationError) do + assert_raises(ActiveRecord::SerializationFailure) do thread = Thread.new do Sample.transaction isolation: :serializable do Sample.delete_all @@ -51,8 +51,33 @@ module ActiveRecord Sample.create value: i end + end + + thread.join + end + end + end + + test "raises Deadlocked when a deadlock is encountered" do + with_warning_suppression do + assert_raises(ActiveRecord::Deadlocked) do + s1 = Sample.create value: 1 + s2 = Sample.create value: 2 + + thread = Thread.new do + Sample.transaction do + s1.lock! + sleep 1 + s2.update_attributes value: 1 + end + end + + sleep 0.5 + Sample.transaction do + s2.lock! sleep 1 + s1.update_attributes value: 2 end thread.join diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb index 8a728902a8..4e865264c7 100644 --- a/activerecord/test/cases/aggregations_test.rb +++ b/activerecord/test/cases/aggregations_test.rb @@ -143,6 +143,11 @@ class AggregationsTest < ActiveRecord::TestCase customers(:barney).fullname = { first: "Barney", last: "Stinson" } assert_equal "Barney STINSON", customers(:barney).name end + + def test_assigning_hash_without_custom_converter + customers(:barney).fullname_no_converter = { first: "Barney", last: "Stinson" } + assert_equal({ first: "Barney", last: "Stinson" }.to_s, customers(:barney).name) + end end class OverridingAggregationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/models/customer.rb b/activerecord/test/models/customer.rb index 3338aaf7e1..d464759430 100644 --- a/activerecord/test/models/customer.rb +++ b/activerecord/test/models/customer.rb @@ -7,6 +7,7 @@ class Customer < ActiveRecord::Base composed_of :non_blank_gps_location, :class_name => "GpsLocation", :allow_nil => true, :mapping => %w(gps_location gps_location), :converter => lambda { |gps| self.gps_conversion_was_run = true; gps.blank? ? nil : GpsLocation.new(gps)} composed_of :fullname, :mapping => %w(name to_s), :constructor => Proc.new { |name| Fullname.parse(name) }, :converter => :parse + composed_of :fullname_no_converter, :mapping => %w(name to_s), class_name: "Fullname" end class Address |