diff options
Diffstat (limited to 'activerecord')
54 files changed, 403 insertions, 191 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1ff7010c2f..c1c511a65c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* When using `Relation#or`, extract the common conditions and put them before the OR condition. + + *Maxime Handfield Lapointe* + * `Relation#or` now accepts two relations who have different values for `references` only, as `references` can be implicitly called by `where`. @@ -5,7 +9,7 @@ *Sean Griffin* -* ApplicationRecord is no longer generated when generating models. If you +* `ApplicationRecord` is no longer generated when generating models. If you need to generate it, it can be created with `rails g application_record`. *Lisa Ugray* @@ -79,7 +83,7 @@ * Fix transactions to apply state to child transactions - Previously if you had a nested transaction and the outer transaction was rolledback the record from the + Previously, if you had a nested transaction and the outer transaction was rolledback, the record from the inner transaction would still be marked as persisted. This change fixes that by applying the state of the parent transaction to the child transaction when the @@ -145,7 +149,7 @@ *Kir Shatrov* -* Prevent making bind param if casted value is nil. +* Prevent creation of bind param if casted value is nil. *Ryuta Kamizono* @@ -206,7 +210,7 @@ *bogdanvlviv* -* When calling the dynamic fixture accessor method with no arguments it now returns all fixtures of this type. +* When calling the dynamic fixture accessor method with no arguments, it now returns all fixtures of this type. Previously this method always returned an empty array. *Kevin McPhillips* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 840f71bef2..a61c0336db 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -349,6 +349,7 @@ module ActiveRecord # build_other(attributes={}) | X | | X # create_other(attributes={}) | X | | X # create_other!(attributes={}) | X | | X + # reload_other | X | X | X # # === Collection associations (one-to-many / many-to-many) # | | | has_many @@ -378,6 +379,7 @@ module ActiveRecord # others.exists? | X | X | X # others.distinct | X | X | X # others.reset | X | X | X + # others.reload | X | X | X # # === Overriding generated methods # @@ -1190,8 +1192,8 @@ module ActiveRecord # <tt>has_many :clients</tt> would add among others <tt>clients.empty?</tt>. # # [collection] - # Returns an array of all the associated objects. - # An empty array is returned if none are found. + # Returns a Relation of all the associated objects. + # An empty Relation is returned if none are found. # [collection<<(object, ...)] # Adds one or more objects to the collection by setting their foreign keys to the collection's primary key. # Note that this operation instantly fires update SQL without waiting for the save or update call on the @@ -1248,6 +1250,9 @@ module ActiveRecord # [collection.create!(attributes = {})] # Does the same as <tt>collection.create</tt>, but raises ActiveRecord::RecordInvalid # if the record is invalid. + # [collection.reload] + # Returns a Relation of all of the associated objects, forcing a database read. + # An empty Relation is returned if none are found. # # === Example # @@ -1267,6 +1272,7 @@ module ActiveRecord # * <tt>Firm#clients.build</tt> (similar to <tt>Client.new("firm_id" => id)</tt>) # * <tt>Firm#clients.create</tt> (similar to <tt>c = Client.new("firm_id" => id); c.save; c</tt>) # * <tt>Firm#clients.create!</tt> (similar to <tt>c = Client.new("firm_id" => id); c.save!</tt>) + # * <tt>Firm#clients.reload</tt> # The declaration can also include an +options+ hash to specialize the behavior of the association. # # === Scopes @@ -1426,6 +1432,8 @@ module ActiveRecord # [create_association!(attributes = {})] # Does the same as <tt>create_association</tt>, but raises ActiveRecord::RecordInvalid # if the record is invalid. + # [reload_association] + # Returns the associated object, forcing a database read. # # === Example # @@ -1435,6 +1443,7 @@ module ActiveRecord # * <tt>Account#build_beneficiary</tt> (similar to <tt>Beneficiary.new("account_id" => id)</tt>) # * <tt>Account#create_beneficiary</tt> (similar to <tt>b = Beneficiary.new("account_id" => id); b.save; b</tt>) # * <tt>Account#create_beneficiary!</tt> (similar to <tt>b = Beneficiary.new("account_id" => id); b.save!; b</tt>) + # * <tt>Account#reload_beneficiary</tt> # # === Scopes # @@ -1555,6 +1564,8 @@ module ActiveRecord # [create_association!(attributes = {})] # Does the same as <tt>create_association</tt>, but raises ActiveRecord::RecordInvalid # if the record is invalid. + # [reload_association] + # Returns the associated object, forcing a database read. # # === Example # @@ -1564,6 +1575,7 @@ module ActiveRecord # * <tt>Post#build_author</tt> (similar to <tt>post.author = Author.new</tt>) # * <tt>Post#create_author</tt> (similar to <tt>post.author = Author.new; post.author.save; post.author</tt>) # * <tt>Post#create_author!</tt> (similar to <tt>post.author = Author.new; post.author.save!; post.author</tt>) + # * <tt>Post#reload_author</tt> # The declaration can also include an +options+ hash to specialize the behavior of the association. # # === Scopes @@ -1704,8 +1716,8 @@ module ActiveRecord # <tt>has_and_belongs_to_many :categories</tt> would add among others <tt>categories.empty?</tt>. # # [collection] - # Returns an array of all the associated objects. - # An empty array is returned if none are found. + # Returns a Relation of all the associated objects. + # An empty Relation is returned if none are found. # [collection<<(object, ...)] # Adds one or more objects to the collection by creating associations in the join table # (<tt>collection.push</tt> and <tt>collection.concat</tt> are aliases to this method). @@ -1743,6 +1755,9 @@ module ActiveRecord # Returns a new object of the collection type that has been instantiated # with +attributes+, linked to this object through the join table, and that has already been # saved (if it passed the validation). + # [collection.reload] + # Returns a Relation of all of the associated objects, forcing a database read. + # An empty Relation is returned if none are found. # # === Example # @@ -1761,6 +1776,7 @@ module ActiveRecord # * <tt>Developer#projects.exists?(...)</tt> # * <tt>Developer#projects.build</tt> (similar to <tt>Project.new("developer_id" => id)</tt>) # * <tt>Developer#projects.create</tt> (similar to <tt>c = Project.new("developer_id" => id); c.save; c</tt>) + # * <tt>Developer#projects.reload</tt> # The declaration may include an +options+ hash to specialize the behavior of the association. # # === Scopes diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 9b0b50977d..3d79e540b8 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -68,11 +68,11 @@ module ActiveRecord foreign_key = join_keys.foreign_key value = transform_value(owner[foreign_key]) - scope = scope.where(table.name => { key => value }) + scope = apply_scope(scope, table, key, value) if reflection.type polymorphic_type = transform_value(owner.class.base_class.name) - scope = scope.where(table.name => { reflection.type => polymorphic_type }) + scope = apply_scope(scope, table, reflection.type, polymorphic_type) end scope @@ -91,10 +91,10 @@ module ActiveRecord if reflection.type value = transform_value(next_reflection.klass.base_class.name) - scope = scope.where(table.name => { reflection.type => value }) + scope = apply_scope(scope, table, reflection.type, value) end - scope = scope.joins(join(foreign_table, constraint)) + scope.joins!(join(foreign_table, constraint)) end class ReflectionProxy < SimpleDelegator # :nodoc: @@ -165,6 +165,14 @@ module ActiveRecord scope end + def apply_scope(scope, table, key, value) + if scope.table == table + scope.where!(key => value) + else + scope.where!(table.name => { key => value }) + end + end + def eval_scope(reflection, table, scope, owner) reflection.build_scope(table).instance_exec(owner, &scope) end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 1a6984877b..1a424896fe 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -306,15 +306,13 @@ module ActiveRecord return scope.to_a if skip_statement_cache?(scope) 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)) - } + sc = reflection.association_scope_cache(conn, owner) do |params| + as = AssociationScope.create { params.bind } + target_scope.merge!(as.scope(self)) end binds = AssociationScope.get_bind_values(owner, reflection.chain) - sc.execute(binds, klass, conn) do |record| + sc.execute(binds, conn) do |record| set_inverse_instance(record) end end diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index c1eee3c630..441bd715e4 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -41,15 +41,13 @@ module ActiveRecord return scope.take if skip_statement_cache?(scope) 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)).limit(1) - } + sc = reflection.association_scope_cache(conn, owner) do |params| + as = AssociationScope.create { params.bind } + target_scope.merge!(as.scope(self)).limit(1) end binds = AssociationScope.get_bind_values(owner, reflection.chain) - sc.execute(binds, klass, conn) do |record| + sc.execute(binds, conn) do |record| set_inverse_instance record end.first rescue ::RangeError diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 5efe051125..48d33e6744 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -5,7 +5,7 @@ require_relative "../attribute_mutation_tracker" module ActiveRecord module AttributeMethods - module Dirty # :nodoc: + module Dirty extend ActiveSupport::Concern include ActiveModel::Dirty @@ -47,7 +47,7 @@ module ActiveRecord clear_mutation_trackers end - def changes_applied + def changes_applied # :nodoc: @mutations_before_last_save = mutation_tracker @mutations_from_database = AttributeMutationTracker.new(@attributes) @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new @@ -55,27 +55,27 @@ module ActiveRecord clear_mutation_trackers end - def clear_changes_information + def clear_changes_information # :nodoc: @mutations_before_last_save = nil @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new forget_attribute_assignments clear_mutation_trackers end - def write_attribute_without_type_cast(attr_name, *) + def write_attribute_without_type_cast(attr_name, *) # :nodoc: result = super clear_attribute_change(attr_name) result end - def clear_attribute_changes(attr_names) + def clear_attribute_changes(attr_names) # :nodoc: super attr_names.each do |attr_name| clear_attribute_change(attr_name) end end - def changed_attributes + def changed_attributes # :nodoc: # This should only be set by methods which will call changed_attributes # multiple times when it is known that the computed value cannot change. if defined?(@cached_changed_attributes) @@ -85,17 +85,17 @@ module ActiveRecord end end - def changes + def changes # :nodoc: cache_changed_attributes do super end end - def previous_changes + def previous_changes # :nodoc: mutations_before_last_save.changes end - def attribute_changed_in_place?(attr_name) + def attribute_changed_in_place?(attr_name) # :nodoc: mutation_tracker.changed_in_place?(attr_name) end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 6adaeb0121..615b2fa701 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -31,9 +31,11 @@ module ActiveRecord temp_method = "__temp__#{safe_name}" ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name + sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 def #{temp_method} + #{sync_with_transaction_state} name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name} _read_attribute(name) { |n| missing_attribute(n, caller) } end @@ -57,6 +59,7 @@ module ActiveRecord end name = self.class.primary_key if name == "id".freeze && self.class.primary_key + sync_with_transaction_state if name == self.class.primary_key _read_attribute(name, &block) end diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index 6ed45d8737..acd47629dd 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -5,6 +5,16 @@ module ActiveRecord module Serialization extend ActiveSupport::Concern + class ColumnNotSerializableError < StandardError + def initialize(name, type) + super <<-EOS.strip_heredoc + Column `#{name}` of type #{type.class} does not support `serialize` feature. + Usually it means that you are trying to use `serialize` + on a column that already implements serialization natively. + EOS + end + end + module ClassMethods # If you have an attribute that needs to be saved to the database as an # object, and retrieved as the same object, then specify the name of that @@ -60,9 +70,23 @@ module ActiveRecord end decorate_attribute_type(attr_name, :serialize) do |type| + if type_incompatible_with_serialize?(type) + raise ColumnNotSerializableError.new(attr_name, type) + end + Type::Serialized.new(type, coder) end end + + private + + def type_incompatible_with_serialize?(type) + type.is_a?(ActiveRecord::Type::Json) || + ( + defined?(ActiveRecord::ConnectionAdapters::PostgreSQL) && + type.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Array) + ) + end end end end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index fa01f832ac..62c5ce059b 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -15,10 +15,12 @@ module ActiveRecord def define_method_attribute=(name) safe_name = name.unpack("h*".freeze).first ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name + sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 def __temp__#{safe_name}=(value) name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name} + #{sync_with_transaction_state} _write_attribute(name, value) end alias_method #{(name + '=').inspect}, :__temp__#{safe_name}= @@ -38,6 +40,7 @@ module ActiveRecord end name = self.class.primary_key if name == "id".freeze && self.class.primary_key + sync_with_transaction_state if name == self.class.primary_key _write_attribute(name, value) 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 c11b7b012f..0759f4d2b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -270,7 +270,7 @@ module ActiveRecord # Connections must be leased while holding the main pool mutex. This is # an internal subclass that also +.leases+ returned connections while # still in queue's critical section (queue synchronizes with the same - # +@lock+ as the main pool) so that a returned connection is already + # <tt>@lock</tt> as the main pool) so that a returned connection is already # leased and there is no need to re-enter synchronized block. class ConnectionLeasingQueue < Queue # :nodoc: include BiasableQueue @@ -326,8 +326,6 @@ module ActiveRecord @spec = spec @checkout_timeout = (spec.config[:checkout_timeout] && spec.config[:checkout_timeout].to_f) || 5 - @reaper = Reaper.new(self, (spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f)) - @reaper.run # default max pool size to 5 @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 @@ -340,7 +338,7 @@ module ActiveRecord # then that +thread+ does indeed own that +conn+. However, an absence of a such # mapping does not mean that the +thread+ doesn't own the said connection. In # that case +conn.owner+ attr should be consulted. - # Access and modification of +@thread_cached_conns+ does not require + # Access and modification of <tt>@thread_cached_conns</tt> does not require # synchronization. @thread_cached_conns = Concurrent::Map.new(initial_capacity: @size) @@ -357,6 +355,9 @@ module ActiveRecord @available = ConnectionLeasingQueue.new self @lock_thread = false + + @reaper = Reaper.new(self, spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f) + @reaper.run end def lock_thread=(lock_thread) @@ -736,10 +737,10 @@ module ActiveRecord # Implementation detail: the connection returned by +acquire_connection+ # will already be "+connection.lease+ -ed" to the current thread. def acquire_connection(checkout_timeout) - # NOTE: we rely on +@available.poll+ and +try_to_checkout_new_connection+ to + # NOTE: we rely on <tt>@available.poll</tt> and +try_to_checkout_new_connection+ to # +conn.lease+ the returned connection (and to do this in a +synchronized+ # section). This is not the cleanest implementation, as ideally we would - # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to +@available.poll+ + # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to <tt>@available.poll</tt> # and +try_to_checkout_new_connection+ we can piggyback on +synchronize+ sections # of the said methods and avoid an additional +synchronize+ overhead. if conn = @available.poll || try_to_checkout_new_connection @@ -763,7 +764,7 @@ module ActiveRecord end end - # If the pool is not at a +@size+ limit, establish new connection. Connecting + # If the pool is not at a <tt>@size</tt> limit, establish new connection. Connecting # to the DB is done outside main synchronized section. #-- # Implementation constraint: a newly established connection returned by this 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 00e54edac0..5febb5a59f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -227,7 +227,7 @@ module ActiveRecord # You should consult the documentation for your database to understand the # semantics of these different levels: # - # * http://www.postgresql.org/docs/current/static/transaction-iso.html + # * https://www.postgresql.org/docs/current/static/transaction-iso.html # * https://dev.mysql.com/doc/refman/5.7/en/set-transaction.html # # An ActiveRecord::TransactionIsolationError will be raised if: @@ -465,12 +465,12 @@ module ActiveRecord @binds = [] end - def << str + def <<(str) @parts << str self end - def add_bind obj + def add_bind(obj) @binds << obj @parts << Arel::Nodes::BindParam.new(1) self diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index ecf5201d12..41d93c4322 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "concurrent/map" + module ActiveRecord module ConnectionAdapters # :nodoc: module QueryCache 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 e21f93856e..3c9b25e411 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -780,7 +780,7 @@ module ActiveRecord def rename_index(table_name, old_name, new_name) validate_index_length!(table_name, new_name) - # this is a naive implementation; some DBs may support this more efficiently (Postgres, for instance) + # this is a naive implementation; some DBs may support this more efficiently (PostgreSQL, for instance) old_index_def = indexes(table_name).detect { |i| i.name == old_name } return unless old_index_def add_index(table_name, old_index_def.columns, name: new_name, unique: old_index_def.unique) 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 32b01b4a31..780e642f21 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -224,7 +224,7 @@ module ActiveRecord # Sets the schema search path to a string of comma-separated schema names. # Names beginning with $ have to be quoted (e.g. $user => '$user'). - # See: http://www.postgresql.org/docs/current/static/ddl-schemas.html + # See: https://www.postgresql.org/docs/current/static/ddl-schemas.html # # This should be not be called manually but set in database.yml. def schema_search_path=(schema_csv) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index bb02d2d38b..3b4439fc46 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -64,11 +64,11 @@ module ActiveRecord # defaults to true. # # Any further options are used as connection parameters to libpq. See - # http://www.postgresql.org/docs/current/static/libpq-connect.html for the + # https://www.postgresql.org/docs/current/static/libpq-connect.html for the # list of parameters. # # In addition, default connection parameters of libpq can be set per environment variables. - # See http://www.postgresql.org/docs/current/static/libpq-envars.html . + # See https://www.postgresql.org/docs/current/static/libpq-envars.html . class PostgreSQLAdapter < AbstractAdapter ADAPTER_NAME = "PostgreSQL".freeze @@ -393,7 +393,7 @@ module ActiveRecord private - # See http://www.postgresql.org/docs/current/static/errcodes-appendix.html + # See https://www.postgresql.org/docs/current/static/errcodes-appendix.html VALUE_LIMIT_VIOLATION = "22001" NUMERIC_VALUE_OUT_OF_RANGE = "22003" NOT_NULL_VIOLATION = "23502" @@ -713,7 +713,7 @@ module ActiveRecord end # SET statements from :variables config hash - # http://www.postgresql.org/docs/current/static/sql-set.html + # https://www.postgresql.org/docs/current/static/sql-set.html variables = @config[:variables] || {} variables.map do |k, v| if v == ":default" || v == :default diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 8b97dbe5bf..fbb4c671a5 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -2,6 +2,7 @@ require "active_support/core_ext/hash/indifferent_access" require "active_support/core_ext/string/filters" +require "concurrent/map" module ActiveRecord module Core @@ -149,7 +150,7 @@ module ActiveRecord end def initialize_find_by_cache # :nodoc: - @find_by_statement_cache = { true => {}.extend(Mutex_m), false => {}.extend(Mutex_m) } + @find_by_statement_cache = { true => Concurrent::Map.new, false => Concurrent::Map.new } end def inherited(child_class) # :nodoc: @@ -168,8 +169,7 @@ module ActiveRecord id = ids.first - return super if id.kind_of?(Array) || - id.is_a?(ActiveRecord::Base) + return super if StatementCache.unsupported_value?(id) key = primary_key @@ -177,7 +177,7 @@ module ActiveRecord where(key => params.bind).limit(1) } - record = statement.execute([id], self, connection).first + record = statement.execute([id], connection).first unless record raise RecordNotFound.new("Couldn't find #{name} with '#{primary_key}'=#{id}", name, primary_key, id) @@ -194,7 +194,7 @@ module ActiveRecord hash = args.first return super if !(Hash === hash) || hash.values.any? { |v| - v.nil? || Array === v || Hash === v || Relation === v || Base === v + StatementCache.unsupported_value?(v) } # We can't cache Post.find_by(author: david) ...yet @@ -209,7 +209,7 @@ module ActiveRecord where(wheres).limit(1) } begin - statement.execute(hash.values, self, connection).first + statement.execute(hash.values, connection).first rescue TypeError raise ActiveRecord::StatementInvalid rescue ::RangeError @@ -282,9 +282,7 @@ module ActiveRecord def cached_find_by_statement(key, &block) cache = @find_by_statement_cache[connection.prepared_statements] - cache[key] || cache.synchronize { - cache[key] ||= StatementCache.create(connection, &block) - } + cache.compute_if_absent(key) { StatementCache.create(connection, &block) } end def relation diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 9feba840b3..e790760292 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -169,7 +169,7 @@ module ActiveRecord class NoDatabaseError < StatementInvalid end - # Raised when Postgres returns 'cached plan must not change result type' and + # Raised when PostgreSQL returns 'cached plan must not change result type' and # we cannot retry gracefully (e.g. inside a transaction) class PreparedStatementCacheExpired < StatementInvalid end @@ -315,7 +315,7 @@ module ActiveRecord # # See the following: # - # * http://www.postgresql.org/docs/current/static/transaction-iso.html + # * https://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 TransactionRollbackError < StatementInvalid end diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb index 8d69bf7d3d..e939a24ad5 100644 --- a/activerecord/lib/active_record/locking/pessimistic.rb +++ b/activerecord/lib/active_record/locking/pessimistic.rb @@ -54,7 +54,7 @@ module ActiveRecord # # Database-specific information on row locking: # MySQL: http://dev.mysql.com/doc/refman/5.7/en/innodb-locking-reads.html - # PostgreSQL: http://www.postgresql.org/docs/current/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE + # PostgreSQL: https://www.postgresql.org/docs/current/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE module Pessimistic # Obtain a row lock on this record. Reloads the record to obtain the requested # lock. Pass an SQL locking clause to append the end of the SELECT statement diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index d6595c9355..784292f3f9 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -39,7 +39,7 @@ module ActiveRecord end end - # Since 5.1 Postgres adapter uses bigserial type for primary + # Since 5.1 PostgreSQL adapter uses bigserial type for primary # keys by default and MySQL uses bigint. This compat layer makes old migrations utilize # serial/int type instead -- the way it used to work before 5.1. unless options.key?(:id) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index efe56454d0..97f1a83033 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "thread" require "active_support/core_ext/string/filters" require "active_support/deprecation" +require "concurrent/map" module ActiveRecord # = Active Record Reflection @@ -443,8 +443,7 @@ module ActiveRecord @type = options[:as] && (options[:foreign_type] || "#{options[:as]}_type") @foreign_type = options[:foreign_type] || "#{name}_type" @constructable = calculate_constructable(macro, options) - @association_scope_cache = {} - @scope_lock = Mutex.new + @association_scope_cache = Concurrent::Map.new if options[:class_name] && options[:class_name].class == Class ActiveSupport::Deprecation.warn(<<-MSG.squish) @@ -458,14 +457,12 @@ module ActiveRecord end end - def association_scope_cache(conn, owner) + def association_scope_cache(conn, owner, &block) key = conn.prepared_statements if polymorphic? key = [key, owner._read_attribute(@foreign_type)] end - @association_scope_cache[key] ||= @scope_lock.synchronize { - @association_scope_cache[key] ||= yield - } + @association_scope_cache.compute_if_absent(key) { StatementCache.create(conn, &block) } end def constructable? # :nodoc: diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 182f654897..03824ffff9 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -135,19 +135,17 @@ module ActiveRecord if other.reordering_value # override any order specified in the original relation relation.reorder! other.order_values - elsif other.order_values + elsif other.order_values.any? # merge in order_values from relation relation.order! other.order_values end - relation.extend(*other.extending_values) unless other.extending_values.blank? + extensions = other.extensions - relation.extensions + relation.extending!(*extensions) if extensions.any? end def merge_single_values - if relation.from_clause.empty? - relation.from_clause = other.from_clause - end - relation.lock_value ||= other.lock_value + relation.lock_value ||= other.lock_value if other.lock_value unless other.create_with_value.blank? relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value) @@ -155,11 +153,15 @@ module ActiveRecord end def merge_clauses - CLAUSE_METHODS.each do |method| - clause = relation.get_value(method) - other_clause = other.get_value(method) - relation.set_value(method, clause.merge(other_clause)) + if relation.from_clause.empty? && !other.from_clause.empty? + relation.from_clause = other.from_clause end + + where_clause = relation.where_clause.merge(other.where_clause) + relation.where_clause = where_clause unless where_clause.empty? + + having_clause = relation.having_clause.merge(other.having_clause) + relation.having_clause = having_clause unless having_clause.empty? end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index f7fe968b55..bdc5c27328 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -902,16 +902,17 @@ module ActiveRecord @arel ||= build_arel end - # Returns a relation value with a given name - def get_value(name) # :nodoc: - @values[name] || default_value_for(name) - end + protected + # Returns a relation value with a given name + def get_value(name) # :nodoc: + @values[name] || default_value_for(name) + end - # Sets the relation value with the given name - def set_value(name, value) # :nodoc: - assert_mutability! - @values[name] = value - end + # Sets the relation value with the given name + def set_value(name, value) # :nodoc: + assert_mutability! + @values[name] = value + end private diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index ef2bca9155..752bb38481 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -15,6 +15,12 @@ module ActiveRecord ) end + def -(other) + WhereClause.new( + predicates - other.predicates, + ) + end + def merge(other) WhereClause.new( predicates_unreferenced_by(other) + other.predicates, @@ -26,14 +32,17 @@ module ActiveRecord end def or(other) - if empty? - self - elsif other.empty? - other + left = self - other + common = self - left + right = other - common + + if left.empty? || right.empty? + common else - WhereClause.new( - [ast.or(other.ast)], + or_clause = WhereClause.new( + [left.ast.or(right.ast)], ) + common + or_clause end end diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index 64657089b5..59acd63a0f 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -11,7 +11,7 @@ module ActiveRecord # The cached statement is executed by using the # {connection.execute}[rdoc-ref:ConnectionAdapters::DatabaseStatements#execute] method: # - # cache.execute([], Book, Book.connection) + # cache.execute([], Book.connection) # # The relation returned by the block is cached, and for each # {execute}[rdoc-ref:ConnectionAdapters::DatabaseStatements#execute] @@ -26,7 +26,7 @@ module ActiveRecord # # And pass the bind values as the first argument of +execute+ call. # - # cache.execute(["my book"], Book, Book.connection) + # cache.execute(["my book"], Book.connection) class StatementCache # :nodoc: class Substitute; end # :nodoc: @@ -87,27 +87,35 @@ module ActiveRecord end end - attr_reader :bind_map, :query_builder - def self.create(connection, block = Proc.new) relation = block.call Params.new query_builder, binds = connection.cacheable_query(self, relation.arel) bind_map = BindMap.new(binds) - new query_builder, bind_map + new(query_builder, bind_map, relation.klass) end - def initialize(query_builder, bind_map) + def initialize(query_builder, bind_map, klass) @query_builder = query_builder - @bind_map = bind_map + @bind_map = bind_map + @klass = klass end - def execute(params, klass, connection, &block) + def execute(params, connection, &block) bind_values = bind_map.bind params sql = query_builder.sql_for bind_values, connection klass.find_by_sql(sql, bind_values, preparable: true, &block) end - alias :call :execute + + def self.unsupported_value?(value) + case value + when NilClass, Array, Range, Hash, Relation, Base then true + end + end + + protected + + attr_reader :query_builder, :bind_map, :klass end end diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index a2e74efc2b..955b1d4e94 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -22,7 +22,7 @@ module ActiveRecord configuration.merge("encoding" => encoding) establish_connection configuration rescue ActiveRecord::StatementInvalid => error - if /database .* already exists/.match?(error.message) + if error.cause.is_a?(PG::DuplicateDatabase) raise DatabaseAlreadyExists else raise diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 3e8a0789df..b2f5e39e09 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -432,8 +432,8 @@ module ActiveRecord @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] pk = self.class.primary_key - if pk && read_attribute(pk) != restore_state[:id] - write_attribute(pk, restore_state[:id]) + if pk && _read_attribute(pk) != restore_state[:id] + _write_attribute(pk, restore_state[:id]) end freeze if restore_state[:frozen?] end @@ -472,7 +472,7 @@ module ActiveRecord # if it's associated with a transaction, then the state of the Active Record # object will be updated to reflect the current state of the transaction. # - # The +@transaction_state+ variable stores the states of the associated + # The <tt>@transaction_state</tt> variable stores the states of the associated # transaction. This relies on the fact that a transaction can only be in # one rollback or commit (otherwise a list of states would be required). # Each Active Record object inside of a transaction carries that transaction's diff --git a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb index 0174c7ea31..856fcc5897 100644 --- a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb @@ -28,7 +28,7 @@ module ActiveRecord def set_local_assigns! @migration_template = "migration.rb" case file_name - when /^(add|remove)_.*_(?:to|from)_(.*)/ + when /^(add)_.*_to_(.*)/, /^(remove)_.*?_from_(.*)/ @migration_action = $1 @table_name = normalize_table_name($2) when /join_table/ diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index abb7a696ce..6e04578576 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -158,26 +158,6 @@ module ActiveRecord end end - # test resetting sequences in odd tables in PostgreSQL - if ActiveRecord::Base.connection.respond_to?(:reset_pk_sequence!) - require "models/movie" - require "models/subscriber" - - def test_reset_empty_table_with_custom_pk - Movie.delete_all - Movie.connection.reset_pk_sequence! "movies" - assert_equal 1, Movie.create(name: "fight club").id - end - - def test_reset_table_with_non_integer_pk - Subscriber.delete_all - Subscriber.connection.reset_pk_sequence! "subscribers" - sub = Subscriber.new(name: "robert drake") - sub.id = "bob drake" - assert_nothing_raised { sub.save! } - end - end - def test_uniqueness_violations_are_translated_to_specific_exception @connection.execute "INSERT INTO subscribers(nick) VALUES('me')" error = assert_raises(ActiveRecord::RecordNotUnique) do @@ -368,5 +348,25 @@ module ActiveRecord assert !@connection.transaction_open? end end + + # test resetting sequences in odd tables in PostgreSQL + if ActiveRecord::Base.connection.respond_to?(:reset_pk_sequence!) + require "models/movie" + require "models/subscriber" + + def test_reset_empty_table_with_custom_pk + Movie.delete_all + Movie.connection.reset_pk_sequence! "movies" + assert_equal 1, Movie.create(name: "fight club").id + end + + def test_reset_table_with_non_integer_pk + Subscriber.delete_all + Subscriber.connection.reset_pk_sequence! "subscribers" + sub = Subscriber.new(name: "robert drake") + sub.id = "bob drake" + assert_nothing_raised { sub.save! } + end + end end end diff --git a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb index eb602af812..62abd694bb 100644 --- a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb +++ b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb @@ -3,6 +3,8 @@ require "cases/helper" class SchemaMigrationsTest < ActiveRecord::Mysql2TestCase + self.use_transactional_tests = false + def test_renaming_index_on_foreign_key connection.add_index "engines", "car_id" connection.add_foreign_key :engines, :cars, name: "fk_engines_cars" @@ -33,6 +35,8 @@ class SchemaMigrationsTest < ActiveRecord::Mysql2TestCase assert connection.column_exists?(table_name, :key, :string) end + ensure + ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment end private diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 8507dbb463..08b17f37e2 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -47,6 +47,15 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase assert ratings_column.array? end + def test_not_compatible_with_serialize + new_klass = Class.new(PgArray) do + serialize :tags, Array + end + assert_raises(ActiveRecord::AttributeMethods::Serialization::ColumnNotSerializableError) do + new_klass.new + end + end + def test_default @connection.add_column "pg_arrays", "score", :integer, array: true, default: [4, 4, 2] PgArray.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index 7d8a83a4dd..97a8a257c5 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -21,12 +21,7 @@ if ActiveRecord::Base.connection.supports_extensions? def setup @connection = ActiveRecord::Base.connection - unless @connection.extension_enabled?("hstore") - @connection.enable_extension "hstore" - @connection.commit_db_transaction - end - - @connection.reconnect! + enable_extension!("hstore", @connection) @connection.transaction do @connection.create_table("hstores") do |t| @@ -42,6 +37,7 @@ if ActiveRecord::Base.connection.supports_extensions? teardown do @connection.drop_table "hstores", if_exists: true + disable_extension!("hstore", @connection) end def test_hstore_included_in_extensions diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index aa5e03df41..79dcfe110c 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -33,6 +33,15 @@ module PostgresqlJSONSharedTestCases x.reload assert_equal ["foo" => "bar"], x.objects end + + def test_not_compatible_with_serialize_macro + new_klass = Class.new(klass) do + serialize :payload, JSON + end + assert_raises(ActiveRecord::AttributeMethods::Serialization::ColumnNotSerializableError) do + new_klass.new + end + end end class PostgresqlJSONTest < ActiveRecord::PostgreSQLTestCase diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 5d81a9c258..f199519d86 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -327,15 +327,18 @@ module ActiveRecord end def test_only_reload_type_map_once_for_every_unrecognized_type + reset_connection + connection = ActiveRecord::Base.connection + silence_warnings do assert_queries 2, ignore_none: true do - @connection.select_all "select 'pg_catalog.pg_class'::regclass" + connection.select_all "select 'pg_catalog.pg_class'::regclass" end assert_queries 1, ignore_none: true do - @connection.select_all "select 'pg_catalog.pg_class'::regclass" + connection.select_all "select 'pg_catalog.pg_class'::regclass" end assert_queries 2, ignore_none: true do - @connection.select_all "SELECT NULL::anyarray" + connection.select_all "SELECT NULL::anyarray" end end ensure @@ -343,10 +346,13 @@ module ActiveRecord end def test_only_warn_on_first_encounter_of_unrecognized_oid + reset_connection + connection = ActiveRecord::Base.connection + warning = capture(:stderr) { - @connection.select_all "select 'pg_catalog.pg_class'::regclass" - @connection.select_all "select 'pg_catalog.pg_class'::regclass" - @connection.select_all "select 'pg_catalog.pg_class'::regclass" + connection.select_all "select 'pg_catalog.pg_class'::regclass" + connection.select_all "select 'pg_catalog.pg_class'::regclass" + connection.select_all "select 'pg_catalog.pg_class'::regclass" } assert_match(/\Aunknown OID \d+: failed to recognize type of 'regclass'\. It will be treated as String\.\n\z/, warning) ensure diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 2ab8a6bf03..cedb621b4f 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -465,7 +465,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_create_resets_cached_counters + Reader.delete_all + person = Person.create!(first_name: "tenderlove") + post = Post.first assert_equal [], person.readers diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 2528d3ee70..4c2723addc 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -817,7 +817,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase sarah = Person.create!(first_name: "Sarah", primary_contact_id: people(:susan).id, gender: "F", number1_fan_id: 1) john = Person.create!(first_name: "John", primary_contact_id: sarah.id, gender: "M", number1_fan_id: 1) assert_equal sarah.agents, [john] - assert_equal people(:susan).agents.flat_map(&:agents), people(:susan).agents_of_agents + assert_equal people(:susan).agents.flat_map(&:agents).sort, people(:susan).agents_of_agents.sort end def test_associate_existing_with_nonstandard_primary_key_on_belongs_to diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 2f68bc5141..9d3d5353ff 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -404,7 +404,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_polymorphic_has_one - assert_equal Tagging.find(1, 2).sort_by(&:id), authors(:david).taggings_2 + assert_equal Tagging.find(1, 2).sort_by(&:id), authors(:david).taggings_2.sort_by(&:id) end def test_has_many_through_polymorphic_has_many diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index c2e2bca924..ae492f1c1c 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -1372,7 +1372,7 @@ module AutosaveAssociationOnACollectionAssociationTests @pirate.send(@association_name).each_with_index { |child, i| child.name = new_names[i] } @pirate.save - assert_equal new_names, @pirate.reload.send(@association_name).map(&:name) + assert_equal new_names.sort, @pirate.reload.send(@association_name).map(&:name).sort end def test_should_automatically_save_bang_the_associated_models @@ -1380,7 +1380,7 @@ module AutosaveAssociationOnACollectionAssociationTests @pirate.send(@association_name).each_with_index { |child, i| child.name = new_names[i] } @pirate.save! - assert_equal new_names, @pirate.reload.send(@association_name).map(&:name) + assert_equal new_names.sort, @pirate.reload.send(@association_name).map(&:name).sort end def test_should_update_children_when_autosave_is_true_and_parent_is_new_but_child_is_not diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 92d071187d..39dff19b78 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -747,21 +747,27 @@ class CalculationsTest < ActiveRecord::TestCase end def test_pluck_loaded_relation + Company.attribute_names # Load schema information so we don't query below companies = Company.order(:id).limit(3).load + assert_no_queries do assert_equal ["37signals", "Summit", "Microsoft"], companies.pluck(:name) end end def test_pluck_loaded_relation_multiple_columns + Company.attribute_names # Load schema information so we don't query below companies = Company.order(:id).limit(3).load + assert_no_queries do assert_equal [[1, "37signals"], [2, "Summit"], [3, "Microsoft"]], companies.pluck(:id, :name) end end def test_pluck_loaded_relation_sql_fragment + Company.attribute_names # Load schema information so we don't query below companies = Company.order(:name).limit(3).load + assert_queries 1 do assert_equal ["37signals", "Apex", "Ex Nihilo"], companies.pluck("DISTINCT name") end diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 9ab6607668..f2ec5d6518 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -119,8 +119,6 @@ if ActiveRecord::Base.connection.supports_comments? assert_match %r[t\.integer\s+"rating",\s+precision: 38,\s+comment: "I am running out of imagination"], output else assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output - end - unless current_adapter?(:OracleAdapter) assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output assert_match %r[t\.index\s+.+\s+name: "idx_obvious",\s+comment: "We need to see obvious comments"], output end diff --git a/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb b/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb index 7c41050eaa..02e76ce146 100644 --- a/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb +++ b/activerecord/test/cases/connection_adapters/mysql_type_lookup_test.rb @@ -1,15 +1,22 @@ # frozen_string_literal: true require "cases/helper" +require "support/connection_helper" if current_adapter?(:Mysql2Adapter) module ActiveRecord module ConnectionAdapters class MysqlTypeLookupTest < ActiveRecord::TestCase + include ConnectionHelper + setup do @connection = ActiveRecord::Base.connection end + def teardown + reset_connection + end + def test_boolean_types emulate_booleans(true) do assert_lookup_type :boolean, "tinyint(1)" diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index fb3d691d51..a602f83d8c 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -466,11 +466,10 @@ class DirtyTest < ActiveRecord::TestCase def test_save_should_not_save_serialized_attribute_with_partial_writes_if_not_present with_partial_writes(Topic) do - Topic.create!(author_name: "Bill", content: { a: "a" }) - topic = Topic.select("id, author_name").first + topic = Topic.create!(author_name: "Bill", content: { a: "a" }) + topic = Topic.select("id, author_name").find(topic.id) topic.update_columns author_name: "John" - topic = Topic.first - assert_not_nil topic.content + assert_not_nil topic.reload.content end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index be14287442..55496147c1 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1179,6 +1179,10 @@ class FinderTest < ActiveRecord::TestCase assert_equal posts(:eager_other), Post.find_by("id = ?", posts(:eager_other).id) end + test "find_by with range conditions returns the first matching record" do + assert_equal posts(:eager_other), Post.find_by(id: posts(:eager_other).id...posts(:misc_by_bob).id) + end + test "find_by returns nil if the record is missing" do assert_nil Post.find_by("1 = 0") end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 8197d09621..6b014bcb3d 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -397,6 +397,7 @@ if Account.connection.respond_to?(:reset_pk_sequence!) class FixturesResetPkSequenceTest < ActiveRecord::TestCase fixtures :accounts fixtures :companies + self.use_transactional_tests = false def setup @instances = [Account.new(credit_limit: 50), Company.new(name: "RoR Consulting"), Course.new(name: "Test")] @@ -841,6 +842,8 @@ class FasterFixturesTest < ActiveRecord::TestCase end class FoxyFixturesTest < ActiveRecord::TestCase + # Set to false to blow away fixtures cache and ensure our fixtures are loaded + self.use_transactional_tests = false fixtures :parrots, :parrots_pirates, :pirates, :treasures, :mateys, :ships, :computers, :developers, :"admin/accounts", :"admin/users", :live_parrots, :dead_parrots, :books diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 36fc143a31..6ea02ac191 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -143,6 +143,8 @@ def load_schema if File.exist?(adapter_specific_schema_file) load adapter_specific_schema_file end + + ActiveRecord::FixtureSet.reset_cache ensure $stdout = original_stdout end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 33a4cc0dcb..c887f54560 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -1002,33 +1002,19 @@ class PersistenceTest < ActiveRecord::TestCase end class SaveTest < ActiveRecord::TestCase - self.use_transactional_tests = false - def test_save_touch_false - widget = Class.new(ActiveRecord::Base) do - connection.create_table :widgets, force: true do |t| - t.string :name - t.timestamps null: false - end - - self.table_name = :widgets - end - - instance = widget.create!( + pet = Pet.create!( name: "Bob", created_at: 1.day.ago, updated_at: 1.day.ago) - created_at = instance.created_at - updated_at = instance.updated_at + created_at = pet.created_at + updated_at = pet.updated_at - instance.name = "Barb" - instance.save!(touch: false) - assert_equal instance.created_at, created_at - assert_equal instance.updated_at, updated_at - ensure - ActiveRecord::Base.connection.drop_table widget.table_name - widget.reset_column_information + pet.name = "Barb" + pet.save!(touch: false) + assert_equal pet.created_at, created_at + assert_equal pet.updated_at, updated_at end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index bda82c095f..df83fe0ea1 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -326,7 +326,7 @@ class CompositePrimaryKeyTest < ActiveRecord::TestCase def setup @connection = ActiveRecord::Base.connection @connection.schema_cache.clear! - @connection.create_table(:barcodes, primary_key: ["region", "code"], force: true) do |t| + @connection.create_table(:uber_barcodes, primary_key: ["region", "code"], force: true) do |t| t.string :region t.integer :code end @@ -337,11 +337,11 @@ class CompositePrimaryKeyTest < ActiveRecord::TestCase end def teardown - @connection.drop_table(:barcodes, if_exists: true) + @connection.drop_table(:uber_barcodes, if_exists: true) end def test_composite_primary_key - assert_equal ["region", "code"], @connection.primary_keys("barcodes") + assert_equal ["region", "code"], @connection.primary_keys("uber_barcodes") end def test_composite_primary_key_out_of_order @@ -352,7 +352,7 @@ class CompositePrimaryKeyTest < ActiveRecord::TestCase def test_primary_key_issues_warning model = Class.new(ActiveRecord::Base) do def self.table_name - "barcodes" + "uber_barcodes" end end warning = capture(:stderr) do @@ -361,9 +361,9 @@ class CompositePrimaryKeyTest < ActiveRecord::TestCase assert_match(/WARNING: Active Record does not support composite primary key\./, warning) end - def test_dumping_composite_primary_key - schema = dump_table_schema "barcodes" - assert_match %r{create_table "barcodes", primary_key: \["region", "code"\]}, schema + def test_collectly_dump_composite_primary_key + schema = dump_table_schema "uber_barcodes" + assert_match %r{create_table "uber_barcodes", primary_key: \["region", "code"\]}, schema end def test_dumping_composite_primary_key_out_of_order diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index f843368a09..d3f4b5bf75 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -133,7 +133,7 @@ class QueryCacheTest < ActiveRecord::TestCase assert_cache :off, conn end ensure - ActiveRecord::Base.clear_all_connections! + ActiveRecord::Base.connection_pool.disconnect! end end end diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb index f3a81f3c70..e5eb159d36 100644 --- a/activerecord/test/cases/relation/where_clause_test.rb +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -181,6 +181,53 @@ class ActiveRecord::Relation assert_equal WhereClause.empty, WhereClause.empty.or(where_clause) end + test "or places common conditions before the OR" do + a = WhereClause.new( + [table["id"].eq(bind_param(1)), table["name"].eq(bind_param("Sean"))], + ) + b = WhereClause.new( + [table["id"].eq(bind_param(1)), table["hair_color"].eq(bind_param("black"))], + ) + + common = WhereClause.new( + [table["id"].eq(bind_param(1))], + ) + + or_clause = WhereClause.new([table["name"].eq(bind_param("Sean"))]) + .or(WhereClause.new([table["hair_color"].eq(bind_param("black"))])) + + assert_equal common + or_clause, a.or(b) + end + + test "or can detect identical or as being a common condition" do + common_or = WhereClause.new([table["name"].eq(bind_param("Sean"))]) + .or(WhereClause.new([table["hair_color"].eq(bind_param("black"))])) + + a = common_or + WhereClause.new([table["id"].eq(bind_param(1))]) + b = common_or + WhereClause.new([table["foo"].eq(bind_param("bar"))]) + + new_or = WhereClause.new([table["id"].eq(bind_param(1))]) + .or(WhereClause.new([table["foo"].eq(bind_param("bar"))])) + + assert_equal common_or + new_or, a.or(b) + end + + test "or will use only common conditions if one side only has common conditions" do + only_common = WhereClause.new([ + table["id"].eq(bind_param(1)), + "foo = bar", + ]) + + common_with_extra = WhereClause.new([ + table["id"].eq(bind_param(1)), + "foo = bar", + table["extra"].eq(bind_param("pluto")), + ]) + + assert_equal only_common, only_common.or(common_with_extra) + assert_equal only_common, common_with_extra.or(only_common) + end + private def table diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index f22fcd7b5a..c1805aa592 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -100,6 +100,14 @@ module ActiveRecord assert_equal({ "hello" => "world", "id" => 10 }, relation.scope_for_create) end + def test_empty_scope + relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + assert relation.empty_scope? + + relation.merge!(relation) + assert relation.empty_scope? + end + def test_bad_constants_raise_errors assert_raises(NameError) do ActiveRecord::Relation::HelloWorld diff --git a/activerecord/test/cases/reload_models_test.rb b/activerecord/test/cases/reload_models_test.rb index 67386725cb..72f4bfaf6d 100644 --- a/activerecord/test/cases/reload_models_test.rb +++ b/activerecord/test/cases/reload_models_test.rb @@ -5,6 +5,8 @@ require "models/owner" require "models/pet" class ReloadModelsTest < ActiveRecord::TestCase + include ActiveSupport::Testing::Isolation + fixtures :pets, :owners def test_has_one_with_reload @@ -21,4 +23,4 @@ class ReloadModelsTest < ActiveRecord::TestCase pet.owner = Owner.find_by_name("ashley") assert_equal pet.owner, Owner.find_by_name("ashley") end -end +end unless in_memory_db? diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 6f035b594b..716ca29eda 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -521,6 +521,8 @@ class DefaultScopingWithThreadTest < ActiveRecord::TestCase end def test_default_scope_is_threadsafe + 2.times { ThreadsafeDeveloper.unscoped.create! } + threads = [] assert_not_equal 1, ThreadsafeDeveloper.unscoped.count @@ -539,5 +541,7 @@ class DefaultScopingWithThreadTest < ActiveRecord::TestCase ThreadsafeDeveloper.connection.close end threads.each(&:join) + ensure + ThreadsafeDeveloper.unscoped.destroy_all end end unless in_memory_db? diff --git a/activerecord/test/cases/statement_cache_test.rb b/activerecord/test/cases/statement_cache_test.rb index e2142267ea..1f715e41a6 100644 --- a/activerecord/test/cases/statement_cache_test.rb +++ b/activerecord/test/cases/statement_cache_test.rb @@ -21,9 +21,9 @@ module ActiveRecord Book.where(name: params.bind) end - b = cache.execute([ "my book" ], Book, Book.connection) + b = cache.execute([ "my book" ], Book.connection) assert_equal "my book", b[0].name - b = cache.execute([ "my other book" ], Book, Book.connection) + b = cache.execute([ "my other book" ], Book.connection) assert_equal "my other book", b[0].name end @@ -35,9 +35,9 @@ module ActiveRecord Book.where(id: params.bind) end - b = cache.execute([ b1.id ], Book, Book.connection) + b = cache.execute([ b1.id ], Book.connection) assert_equal b1.name, b[0].name - b = cache.execute([ b2.id ], Book, Book.connection) + b = cache.execute([ b2.id ], Book.connection) assert_equal b2.name, b[0].name end @@ -60,7 +60,7 @@ module ActiveRecord Book.create(name: "my book", author_id: 4) - books = cache.execute([], Book, Book.connection) + books = cache.execute([], Book.connection) assert_equal "my book", books[0].name end @@ -73,7 +73,7 @@ module ActiveRecord molecule = salty.molecules.create(name: "dioxane") molecule.electrons.create(name: "lepton") - liquids = cache.execute([], Book, Book.connection) + liquids = cache.execute([], Book.connection) assert_equal "salty", liquids[0].name end @@ -86,13 +86,13 @@ module ActiveRecord Book.create(name: "my book") end - first_books = cache.execute([], Book, Book.connection) + first_books = cache.execute([], Book.connection) 3.times do Book.create(name: "my book") end - additional_books = cache.execute([], Book, Book.connection) + additional_books = cache.execute([], Book.connection) assert first_books != additional_books end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index b771a1d12c..7fd125ab74 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -686,7 +686,7 @@ class TransactionTest < ActiveRecord::TestCase raise ActiveRecord::Rollback end - assert_nil movie.id + assert_nil movie.movieid end def test_assign_id_after_rollback @@ -709,8 +709,54 @@ class TransactionTest < ActiveRecord::TestCase raise ActiveRecord::Rollback end - movie.id = nil - assert_nil movie.id + movie.movieid = nil + assert_nil movie.movieid + end + + def test_read_attribute_after_rollback + topic = Topic.new + + Topic.transaction do + topic.save! + raise ActiveRecord::Rollback + end + + assert_nil topic.read_attribute(:id) + end + + def test_read_attribute_with_custom_primary_key_after_rollback + movie = Movie.new(name: "foo") + + Movie.transaction do + movie.save! + raise ActiveRecord::Rollback + end + + assert_nil movie.read_attribute(:movieid) + end + + def test_write_attribute_after_rollback + topic = Topic.create! + + Topic.transaction do + topic.save! + raise ActiveRecord::Rollback + end + + topic.write_attribute(:id, nil) + assert_nil topic.id + end + + def test_write_attribute_with_custom_primary_key_after_rollback + movie = Movie.create!(name: "foo") + + Movie.transaction do + movie.save! + raise ActiveRecord::Rollback + end + + movie.write_attribute(:movieid, nil) + assert_nil movie.movieid end def test_rollback_of_frozen_records diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 05420a4240..47749c07d2 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -192,6 +192,7 @@ ActiveRecord::Schema.define do t.string :resource_type t.integer :developer_id t.datetime :deleted_at + t.integer :comments end create_table :companies, force: true do |t| |