From 3360742396a00c9e2ff9838373788bed432d5ea7 Mon Sep 17 00:00:00 2001 From: Devin Christensen Date: Wed, 12 Apr 2017 14:45:10 -0600 Subject: Switch to LIFO for the connection pool Using a FIFO for the connection pool can lead to issues when there are upstream components (pgbouncer, haproxy, etc.) that terminate connections that are idle after a period of time. Switching to a LIFO reduces the probability that a thread will checkout a connection that is about to be closed by an idle timeout in an upstream component. --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') 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 53dbbd8c21..ea76ff983e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -80,7 +80,7 @@ module ActiveRecord # * private methods that require being called in a +synchronize+ blocks # are now explicitly documented class ConnectionPool - # Threadsafe, fair, FIFO queue. Meant to be used by ConnectionPool + # Threadsafe, fair, LIFO queue. Meant to be used by ConnectionPool # with which it shares a Monitor. But could be a generic Queue. # # The Queue in stdlib's 'thread' could replace this class except @@ -111,7 +111,7 @@ module ActiveRecord # Add +element+ to the queue. Never blocks. def add(element) synchronize do - @queue.push element + @queue.unshift element @cond.signal end end -- cgit v1.2.3 From 6116d7bc052839646f448b8403a7287f52b97ed7 Mon Sep 17 00:00:00 2001 From: Devin Christensen Date: Thu, 13 Apr 2017 13:56:42 -0600 Subject: Improve documentation and add test --- .../connection_adapters/abstract/connection_pool.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'activerecord/lib') 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 ea76ff983e..3dc265622d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -80,11 +80,8 @@ module ActiveRecord # * private methods that require being called in a +synchronize+ blocks # are now explicitly documented class ConnectionPool - # Threadsafe, fair, LIFO queue. Meant to be used by ConnectionPool - # with which it shares a Monitor. But could be a generic Queue. - # - # The Queue in stdlib's 'thread' could replace this class except - # stdlib's doesn't support waiting with a timeout. + # Threadsafe, fair, FIFO queue. Meant to be used by ConnectionPool + # with which it shares a Monitor. class Queue def initialize(lock = Monitor.new) @lock = lock @@ -111,7 +108,7 @@ module ActiveRecord # Add +element+ to the queue. Never blocks. def add(element) synchronize do - @queue.unshift element + @queue.push element @cond.signal end end @@ -173,7 +170,7 @@ module ActiveRecord # Removes and returns the head of the queue if possible, or +nil+. def remove - @queue.shift + @queue.pop end # Remove and return the head the queue if the number of -- cgit v1.2.3 From 47d678d9b2ab6d3e888691e8327e657ff0b5dcb0 Mon Sep 17 00:00:00 2001 From: Devin Christensen Date: Thu, 13 Apr 2017 14:05:39 -0600 Subject: Fix typos --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') 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 3dc265622d..9f660a419d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -80,7 +80,7 @@ module ActiveRecord # * private methods that require being called in a +synchronize+ blocks # are now explicitly documented class ConnectionPool - # Threadsafe, fair, FIFO queue. Meant to be used by ConnectionPool + # Threadsafe, fair, LIFO queue. Meant to be used by ConnectionPool # with which it shares a Monitor. class Queue def initialize(lock = Monitor.new) -- cgit v1.2.3 From 11e32c1d841c08bd85842eb059fbf30536e804dc Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Mon, 24 Apr 2017 23:26:13 +0100 Subject: Enable query cache on all connection pools Since the query cache no longer eagerly checks out a connection, we can enable it on all connection pools at the start of every request, and it will only take effect for requests that actually use those pools. --- activerecord/lib/active_record/query_cache.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index ec246e97bc..92dd52b068 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -24,16 +24,19 @@ module ActiveRecord end def self.run - caching_pool = ActiveRecord::Base.connection_pool - caching_was_enabled = caching_pool.query_cache_enabled + ActiveRecord::Base.connection_handler.connection_pool_list.map do |pool| + caching_was_enabled = pool.query_cache_enabled - caching_pool.enable_query_cache! + pool.enable_query_cache! - [caching_pool, caching_was_enabled] + [pool, caching_was_enabled] + end end - def self.complete((caching_pool, caching_was_enabled)) - caching_pool.disable_query_cache! unless caching_was_enabled + def self.complete(caching_pools) + caching_pools.each do |pool, caching_was_enabled| + pool.disable_query_cache! unless caching_was_enabled + end ActiveRecord::Base.connection_handler.connection_pool_list.each do |pool| pool.release_connection if pool.active_connection? && !pool.connection.transaction_open? -- cgit v1.2.3 From 87598c8c80f4cbeef114267d75c46f1c87ca057a Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Mon, 2 Oct 2017 15:46:30 +0900 Subject: Make automatically synchronize test schema work inside engine In Rails engine, migration files are in under `db/migrate` of engine. Therefore, when rake task is executed in engine, `db/migrate` is automatically added to `DatabaseTasks.migrations_paths`. https://github.com/rails/rails/blob/a18cf23a9cbcbeed61e8049442640c7153e0a8fb/activerecord/lib/active_record/railtie.rb#L39..L43 However, if execute the rake task under dummy app, migration files will not be loaded because engine's migration path setting process is not called. Therefore, in order to load migration files correctly, it is necessary to execute rake task under engine. Fixes #30765 --- activerecord/lib/active_record/migration.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 8845e26ab7..ec8119e5da 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -581,7 +581,8 @@ module ActiveRecord def load_schema_if_pending! if ActiveRecord::Migrator.needs_migration? || !ActiveRecord::Migrator.any_migrations? # Roundtrip to Rake to allow plugins to hook into database initialization. - FileUtils.cd Rails.root do + root = defined?(ENGINE_ROOT) ? ENGINE_ROOT : Rails.root + FileUtils.cd(root) do current_config = Base.connection_config Base.clear_all_connections! system("bin/rails db:test:prepare") -- cgit v1.2.3 From 71b15603fddd85602813d1c96857e2674b6075d0 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Tue, 3 Oct 2017 08:41:12 -0300 Subject: Add update_only example to AR nested attributes doc [ci_skip] --- activerecord/lib/active_record/nested_attributes.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 435c81c153..fa20bce3a9 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -63,6 +63,18 @@ module ActiveRecord # member.update params[:member] # member.avatar.icon # => 'sad' # + # If you want to update the current avatar without providing the id, you must add :update_only option. + # + # class Member < ActiveRecord::Base + # has_one :avatar + # accepts_nested_attributes_for :avatar, update_only: true + # end + # + # params = { member: { avatar_attributes: { icon: 'sad' } } } + # member.update params[:member] + # member.avatar.id # => 2 + # member.avatar.icon # => 'sad' + # # By default you will only be able to set and update attributes on the # associated model. If you want to destroy the associated model through the # attributes hash, you have to enable it first using the -- cgit v1.2.3 From c4867b88f6eed16230a90077b82ad02293ea45b3 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Fri, 25 Aug 2017 23:05:21 +0300 Subject: Raise error if unsupported charset for mysql `blog$ bin/rails db:create` Before: ``` Couldn't create database for {"adapter"=>"mysql2", "encoding"=>"utf42", "pool"=>5, "username"=>"root", "password"=>nil, "socket"=>"/var/run/mysqld/mysqld.sock", "database"=>"blog_development"}, {:charset=>"utf42"} (If you set the charset manually, make sure you have a matching collation) Created database 'blog_development' Couldn't create database for {"adapter"=>"mysql2", "encoding"=>"utf42", "pool"=>5, "username"=>"root", "password"=>nil, "socket"=>"/var/run/mysqld/mysqld.sock", "database"=>"blog_test"}, {:charset=>"utf42"} (If you set the charset manually, make sure you have a matching collation) Created database 'blog_test' ``` After: ``` Unsupported charset: '"utf42"' Couldn't create database for {"adapter"=>"mysql2", "encoding"=>"utf42", "pool"=>5, "username"=>"root", "password"=>nil, "socket"=>"/var/run/mysqld/mysqld.sock", "database"=>"blog_development"} rails aborted! Mysql2::Error: Unsupported charset: '"utf42"' ... (stack trace) ... bin/rails:4:in `
' Tasks: TOP => db:create (See full trace by running task with --trace) ``` Closes #29683 Related to #27398 --- activerecord/lib/active_record/tasks/mysql_database_tasks.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 2f91884f59..d6a46f7743 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -31,9 +31,7 @@ module ActiveRecord end establish_connection configuration else - $stderr.puts error.inspect - $stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}" - $stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration["encoding"] + raise end end -- cgit v1.2.3 From a37d03c4b08451765acc721b51a4b5b7eab7e8cc Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sat, 26 Aug 2017 00:06:10 +0300 Subject: Simplify implementation of `MySQLDatabaseTasks` Don't process MySQL ERROR 1045, raise error instead Make behavior of `MySQLDatabaseTasks` more consistent with behavior of `PostgreSQLDatabaseTasks` --- .../lib/active_record/railties/jdbcmysql_error.rb | 18 --------- .../active_record/tasks/mysql_database_tasks.rb | 45 ---------------------- 2 files changed, 63 deletions(-) delete mode 100644 activerecord/lib/active_record/railties/jdbcmysql_error.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/railties/jdbcmysql_error.rb b/activerecord/lib/active_record/railties/jdbcmysql_error.rb deleted file mode 100644 index 72c75ddd52..0000000000 --- a/activerecord/lib/active_record/railties/jdbcmysql_error.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -#FIXME Remove if ArJdbcMysql will give. -module ArJdbcMySQL #:nodoc: - class Error < StandardError #:nodoc: - attr_accessor :error_number, :sql_state - - def initialize(msg) - super - @error_number = nil - @sql_state = nil - end - - # Mysql gem compatibility - alias_method :errno, :error_number - alias_method :error, :message - end -end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index d6a46f7743..e697fa6def 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -3,8 +3,6 @@ module ActiveRecord module Tasks # :nodoc: class MySQLDatabaseTasks # :nodoc: - ACCESS_DENIED_ERROR = 1045 - delegate :connection, :establish_connection, to: ActiveRecord::Base def initialize(configuration) @@ -21,18 +19,6 @@ module ActiveRecord else raise end - rescue error_class => error - if error.respond_to?(:errno) && error.errno == ACCESS_DENIED_ERROR - $stdout.print error.message - establish_connection root_configuration_without_database - connection.create_database configuration["database"], creation_options - if configuration["username"] != "root" - connection.execute grant_statement.gsub(/\s+/, " ").strip - end - establish_connection configuration - else - raise - end end def drop @@ -97,37 +83,6 @@ module ActiveRecord end end - def error_class - if configuration["adapter"].include?("jdbc") - require "active_record/railties/jdbcmysql_error" - ArJdbcMySQL::Error - elsif defined?(Mysql2) - Mysql2::Error - else - StandardError - end - end - - def grant_statement - <<-SQL -GRANT ALL PRIVILEGES ON `#{configuration['database']}`.* - TO '#{configuration['username']}'@'localhost' -IDENTIFIED BY '#{configuration['password']}' WITH GRANT OPTION; - SQL - end - - def root_configuration_without_database - configuration_without_database.merge( - "username" => "root", - "password" => root_password - ) - end - - def root_password - $stdout.print "Please provide the root password for your MySQL installation\n>" - $stdin.gets.strip - end - def prepare_command_options args = { "host" => "--host", -- cgit v1.2.3 From f989b341eccc6a86fd1ddfff7f1441920855c84e Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 8 Feb 2017 11:23:26 -0700 Subject: add config to check arguments to unsafe AR methods --- .../lib/active_record/attribute_methods.rb | 16 +++++++ activerecord/lib/active_record/core.rb | 9 ++++ activerecord/lib/active_record/querying.rb | 21 ++++++---- .../lib/active_record/relation/calculations.rb | 42 ++++++++++++------- .../lib/active_record/relation/query_methods.rb | 49 ++++++++++++++++++++++ 5 files changed, 116 insertions(+), 21 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 23d2aef214..fa0d79ba5f 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -167,6 +167,22 @@ module ActiveRecord end end + # Can the given name be treated as a column name? Returns true if name + # is attribute or attribute alias. + # + # class Person < ActiveRecord::Base + # end + # + # Person.respond_to_attribute?(:name) + # # => true + # + # Person.respond_to_attribute?("foo") + # # => false + def respond_to_attribute?(name) + name = name.to_s + attribute_names.include?(name) || attribute_aliases.include?(name) + end + # Returns true if the given attribute exists, otherwise false. # # class Person < ActiveRecord::Base diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 0f7a503c90..b1e3f71dfe 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -76,6 +76,15 @@ module ActiveRecord # scope being ignored is error-worthy, rather than a warning. mattr_accessor :error_on_ignored_order, instance_writer: false, default: false + # :singleton-method: + # Specify the behavior for unsafe raw query methods. Values are as follows + # enabled - Unsafe raw SQL can be passed to query methods. + # deprecated - Warnings are logged when unsafe raw SQL is passed to + # query methods. + # disabled - Unsafe raw SQL passed to query methods results in + # ArguementError. + mattr_accessor :allow_unsafe_raw_sql, instance_writer: false, default: :enabled + ## # :singleton-method: # Specify whether or not to use timestamps for migration versions diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 3996d5661f..0233835bcf 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -2,18 +2,25 @@ module ActiveRecord module Querying - delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, :none?, :one?, to: :all - delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!, :second_to_last, :second_to_last!, to: :all + delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, + :any?, :many?, :none?, :one?, to: :all + delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, + :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!, + :second_to_last, :second_to_last!, to: :all delegate :first_or_create, :first_or_create!, :first_or_initialize, to: :all - delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, to: :all + delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, + to: :all delegate :find_by, :find_by!, to: :all delegate :destroy_all, :delete_all, :update_all, to: :all delegate :find_each, :find_in_batches, :in_batches, to: :all - delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or, - :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :extending, - :having, :create_with, :distinct, :references, :none, :unscope, :merge, to: :all + delegate :select, :group, :order, :unsafe_raw_order, :except, :reorder, + :unsafe_raw_reorder, :limit, :offset, :joins, :left_joins, + :left_outer_joins, :or, :where, :rewhere, :preload, :eager_load, + :includes, :from, :lock, :readonly, :extending, :having, + :create_with, :distinct, :references, :none, :unscope, :merge, + to: :all delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all - delegate :pluck, :ids, to: :all + delegate :pluck, :unsafe_raw_pluck, :ids, to: :all # Executes a custom SQL query against your database and returns all the results. The results will # be returned as an array with columns requested encapsulated as attributes of the model you call diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 11256ab3d9..dea7542f03 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -175,21 +175,13 @@ module ActiveRecord # See also #ids. # def pluck(*column_names) - if loaded? && (column_names.map(&:to_s) - @klass.attribute_names - @klass.attribute_aliases.keys).empty? - return records.pluck(*column_names) - end + _pluck(column_names, @klass.allow_unsafe_raw_sql == :enabled) + end - if has_include?(column_names.first) - relation = apply_join_dependency - relation.pluck(*column_names) - else - relation = spawn - relation.select_values = column_names.map { |cn| - @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn - } - result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } - result.cast_values(klass.attribute_types) - end + # Same as #pluck but allows raw SQL regardless of `allow_unsafe_raw_sql` + # config setting. + def unsafe_raw_pluck(*column_names) + _pluck(column_names, true) end # Pluck all the ID's for the relation using the table's primary key @@ -202,6 +194,28 @@ module ActiveRecord private + def _pluck(column_names, unsafe_raw) + unrecognized = column_names.reject do |cn| + @klass.respond_to_attribute?(cn) + end + + if loaded? && unrecognized.none? + records.pluck(*column_names) + elsif has_include?(column_names.first) + relation = apply_join_dependency + relation.pluck(*column_names) + elsif unsafe_raw || unrecognized.none? + relation = spawn + relation.select_values = column_names.map { |cn| + @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn + } + result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } + result.cast_values(klass.attribute_types) + else + raise ArgumentError, "Invalid column name: #{unrecognized}" + end + end + def has_include?(column_name) eager_loading? || (includes_values.present? && column_name && column_name != :all) end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 897ff5c8af..63b1d8e154 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -295,7 +295,22 @@ module ActiveRecord spawn.order!(*args) end + # Same as #order but allows raw SQL regardless of `allow_unsafe_raw_sql` + # config setting. + def unsafe_raw_order(*args) # :nodoc: + check_if_method_has_arguments!(:order, args) + spawn.unsafe_raw_order!(*args) + end + + # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: + restrict_order_args(args) unless klass.allow_unsafe_raw_sql == :enabled + unsafe_raw_order!(*args) + end + + # Same as #order! but allows raw SQL regardless of `allow_unsafe_raw_sql` + # config setting. + def unsafe_raw_order!(*args) # :nodoc: preprocess_order_args(args) self.order_values += args @@ -316,7 +331,22 @@ module ActiveRecord spawn.reorder!(*args) end + # Same as #reorder but allows raw SQL regardless of `allow_unsafe_raw_sql` + # config setting. + def unsafe_raw_reorder(*args) # :nodoc: + check_if_method_has_arguments!(:reorder, args) + spawn.unsafe_raw_reorder!(*args) + end + + # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: + restrict_order_args(args) unless klass.allow_unsafe_raw_sql == :enabled + unsafe_raw_reorder! + end + + # Same as #reorder! but allows raw SQL regardless of `allow_unsafe_raw_sql` + # config setting. + def unsafe_raw_reorder!(*args) # :nodoc: preprocess_order_args(args) self.reordering_value = true @@ -1139,6 +1169,25 @@ module ActiveRecord end.flatten! end + # Only allow column names and directions as arguments to #order and + # #reorder. Other arguments will cause an ArugmentError to be raised. + def restrict_order_args(args) + args = args.dup + orderings = args.extract_options! + columns = args | orderings.keys + + unrecognized = columns.reject { |c| klass.respond_to_attribute?(c) } + if unrecognized.any? + raise ArgumentError, "Invalid order column: #{unrecognized}" + end + + # TODO: find a better list of modifiers. + unrecognized = orderings.values.reject { |d| VALID_DIRECTIONS.include?(d.to_s) } + if unrecognized.any? + raise ArgumentError, "Invalid order direction: #{unrecognized}" + end + end + # Checks to make sure that the arguments are not blank. Note that if some # blank-like object were initially passed into the query method, then this # method will not raise an error. -- cgit v1.2.3 From 864b16063d14977096d9d24ac894fee605dfb7a7 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Tue, 21 Feb 2017 11:17:16 -0700 Subject: allow Arel.sql() for pluck --- .../lib/active_record/attribute_methods.rb | 38 ++++++++++++++- activerecord/lib/active_record/errors.rb | 25 ++++++++++ activerecord/lib/active_record/querying.rb | 21 +++------ .../lib/active_record/relation/calculations.rb | 20 +++++--- .../lib/active_record/relation/query_methods.rb | 54 +++------------------- 5 files changed, 90 insertions(+), 68 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index fa0d79ba5f..b3d3c0559f 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -167,6 +167,30 @@ module ActiveRecord end end + def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc: + return if allow_unsafe_raw_sql == :enabled + + unexpected = args.reject do |arg| + whitelist.include?(arg.to_s) || + arg.kind_of?(Arel::Node) || arg.is_a?(Arel::Nodes::SqlLiteral) + end + + return if unexpected.none? + + if allow_unsafe_raw_sql == :deprecated + ActiveSupport::Deprecation.warn( + "Dangerous query method used with non-attribute argument(s): " + + "#{unexpected.map(&:inspect).join(", ")}. Non-argument " + + "arguments will be disallowed in Rails 5.3." + ) + else + raise(ActiveRecord::UnknownAttributeReference, + "Query method called with non-attribute argument(s): " + + unexpected.map(&:inspect).join(", ") + ) + end + end + # Can the given name be treated as a column name? Returns true if name # is attribute or attribute alias. # @@ -178,7 +202,7 @@ module ActiveRecord # # Person.respond_to_attribute?("foo") # # => false - def respond_to_attribute?(name) + def respond_to_attribute?(name) # :nodoc: name = name.to_s attribute_names.include?(name) || attribute_aliases.include?(name) end @@ -214,6 +238,18 @@ module ActiveRecord ConnectionAdapters::NullColumn.new(name) end end + + # An Array of String attribute names and aliases for accessing those + # attributes. + # + # class Person < ActiveRecord::Base + # end + # + # Person.attribute_names_and_aliases + # # => ["id", "created_at", "updated_at", "name", "age"] + def attribute_names_and_aliases # :nodoc: + attribute_names + attribute_aliases.keys + end end # A Person object with a name attribute can ask person.respond_to?(:name), diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 9ef3316393..3c039eef82 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -339,4 +339,29 @@ module ActiveRecord # Wait time value is set by innodb_lock_wait_timeout. class TransactionTimeout < StatementInvalid end + + # UnknownAttributeReference is raised when an unknown and potentially unsafe + # value is passed to a query method when allow_unsafe_raw_sql is set to + # :disabled. For example, passing a non column name value to a relation's + # #order method might cause this exception. + # + # When working around this exception, caution should be taken to avoid SQL + # injection vulnerabilities when passing user-provided values to query + # methods. Known-safe values can be passed to query methods by wrapping them + # in Arel.sql. + # + # For example, with allow_unsafe_raw_sql set to :disabled, the following + # code would raise this exception: + # + # Post.order("length(title)").first + # + # The desired result can be accomplished by wrapping the known-safe string + # in Arel.sql: + # + # Post.order(Arel.sql("length(title)")).first + # + # Again, such a workaround should *not* be used when passing user-provided + # values, such as request parameters or model attributes to query methods. + class UnknownAttributeReference < ActiveRecordError + end end diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 0233835bcf..3996d5661f 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -2,25 +2,18 @@ module ActiveRecord module Querying - delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, - :any?, :many?, :none?, :one?, to: :all - delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, - :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!, - :second_to_last, :second_to_last!, to: :all + delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, :none?, :one?, to: :all + delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!, :second_to_last, :second_to_last!, to: :all delegate :first_or_create, :first_or_create!, :first_or_initialize, to: :all - delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, - to: :all + delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, to: :all delegate :find_by, :find_by!, to: :all delegate :destroy_all, :delete_all, :update_all, to: :all delegate :find_each, :find_in_batches, :in_batches, to: :all - delegate :select, :group, :order, :unsafe_raw_order, :except, :reorder, - :unsafe_raw_reorder, :limit, :offset, :joins, :left_joins, - :left_outer_joins, :or, :where, :rewhere, :preload, :eager_load, - :includes, :from, :lock, :readonly, :extending, :having, - :create_with, :distinct, :references, :none, :unscope, :merge, - to: :all + delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or, + :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :extending, + :having, :create_with, :distinct, :references, :none, :unscope, :merge, to: :all delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all - delegate :pluck, :unsafe_raw_pluck, :ids, to: :all + delegate :pluck, :ids, to: :all # Executes a custom SQL query against your database and returns all the results. The results will # be returned as an array with columns requested encapsulated as attributes of the model you call diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index dea7542f03..236d36e15f 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -175,13 +175,21 @@ module ActiveRecord # See also #ids. # def pluck(*column_names) - _pluck(column_names, @klass.allow_unsafe_raw_sql == :enabled) - end + if loaded? && (column_names.map(&:to_s) - @klass.attribute_names_and_aliases).empty? + return records.pluck(*column_names) + end - # Same as #pluck but allows raw SQL regardless of `allow_unsafe_raw_sql` - # config setting. - def unsafe_raw_pluck(*column_names) - _pluck(column_names, true) + if has_include?(column_names.first) + construct_relation_for_association_calculations.pluck(*column_names) + else + enforce_raw_sql_whitelist(column_names) + relation = spawn + relation.select_values = column_names.map { |cn| + @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn + } + result = klass.connection.select_all(relation.arel, nil, bound_attributes) + result.cast_values(klass.attribute_types) + end end # Pluck all the ID's for the relation using the table's primary key diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 63b1d8e154..4c63d0450a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -295,22 +295,9 @@ module ActiveRecord spawn.order!(*args) end - # Same as #order but allows raw SQL regardless of `allow_unsafe_raw_sql` - # config setting. - def unsafe_raw_order(*args) # :nodoc: - check_if_method_has_arguments!(:order, args) - spawn.unsafe_raw_order!(*args) - end - # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: - restrict_order_args(args) unless klass.allow_unsafe_raw_sql == :enabled - unsafe_raw_order!(*args) - end - - # Same as #order! but allows raw SQL regardless of `allow_unsafe_raw_sql` - # config setting. - def unsafe_raw_order!(*args) # :nodoc: + enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) preprocess_order_args(args) self.order_values += args @@ -331,22 +318,9 @@ module ActiveRecord spawn.reorder!(*args) end - # Same as #reorder but allows raw SQL regardless of `allow_unsafe_raw_sql` - # config setting. - def unsafe_raw_reorder(*args) # :nodoc: - check_if_method_has_arguments!(:reorder, args) - spawn.unsafe_raw_reorder!(*args) - end - # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: - restrict_order_args(args) unless klass.allow_unsafe_raw_sql == :enabled - unsafe_raw_reorder! - end - - # Same as #reorder! but allows raw SQL regardless of `allow_unsafe_raw_sql` - # config setting. - def unsafe_raw_reorder!(*args) # :nodoc: + enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) preprocess_order_args(args) self.reordering_value = true @@ -946,6 +920,11 @@ module ActiveRecord private + # Extract column names from arguments passed to #order or #reorder. + def column_names_from_order_arguments(args) + args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg } + end + def assert_mutability! raise ImmutableRelation if @loaded raise ImmutableRelation if defined?(@arel) && @arel @@ -1169,25 +1148,6 @@ module ActiveRecord end.flatten! end - # Only allow column names and directions as arguments to #order and - # #reorder. Other arguments will cause an ArugmentError to be raised. - def restrict_order_args(args) - args = args.dup - orderings = args.extract_options! - columns = args | orderings.keys - - unrecognized = columns.reject { |c| klass.respond_to_attribute?(c) } - if unrecognized.any? - raise ArgumentError, "Invalid order column: #{unrecognized}" - end - - # TODO: find a better list of modifiers. - unrecognized = orderings.values.reject { |d| VALID_DIRECTIONS.include?(d.to_s) } - if unrecognized.any? - raise ArgumentError, "Invalid order direction: #{unrecognized}" - end - end - # Checks to make sure that the arguments are not blank. Note that if some # blank-like object were initially passed into the query method, then this # method will not raise an error. -- cgit v1.2.3 From 310c3a8f2d043f3d00d3f703052a1e160430a2c2 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Thu, 10 Aug 2017 11:21:22 -0600 Subject: beef up deprecation warning --- activerecord/lib/active_record/attribute_methods.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index b3d3c0559f..e167c4ad6f 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -179,9 +179,13 @@ module ActiveRecord if allow_unsafe_raw_sql == :deprecated ActiveSupport::Deprecation.warn( - "Dangerous query method used with non-attribute argument(s): " + - "#{unexpected.map(&:inspect).join(", ")}. Non-argument " + - "arguments will be disallowed in Rails 5.3." + "Dangerous query method (method whose arguments are used as raw " \ + "SQL) called with non-attribute argument(s): " \ + "#{unexpected.map(&:inspect).join(", ")}. Non-attribute " \ + "arguments will be disallowed in Rails 6.0. This method should " \ + "not be called with user-provided values, such as request " \ + "parameters or model attributes. Known-safe values can be passed " \ + "by wrapping them in Arel.sql()." ) else raise(ActiveRecord::UnknownAttributeReference, -- cgit v1.2.3 From 0d8626ad3f1bac3b814d554147ff7926c0380fad Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Fri, 11 Aug 2017 09:55:54 -0600 Subject: remove :enabled option --- activerecord/lib/active_record/attribute_methods.rb | 2 -- activerecord/lib/active_record/core.rb | 5 ++--- 2 files changed, 2 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index e167c4ad6f..5ce3ba7a63 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -168,8 +168,6 @@ module ActiveRecord end def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc: - return if allow_unsafe_raw_sql == :enabled - unexpected = args.reject do |arg| whitelist.include?(arg.to_s) || arg.kind_of?(Arel::Node) || arg.is_a?(Arel::Nodes::SqlLiteral) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index b1e3f71dfe..b97b14644e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -78,12 +78,11 @@ module ActiveRecord # :singleton-method: # Specify the behavior for unsafe raw query methods. Values are as follows - # enabled - Unsafe raw SQL can be passed to query methods. # deprecated - Warnings are logged when unsafe raw SQL is passed to # query methods. # disabled - Unsafe raw SQL passed to query methods results in - # ArguementError. - mattr_accessor :allow_unsafe_raw_sql, instance_writer: false, default: :enabled + # UnknownAttributeReference exception. + mattr_accessor :allow_unsafe_raw_sql, instance_writer: false, default: :deprecated ## # :singleton-method: -- cgit v1.2.3 From 92e78593ee056bb73a7a87c10af3f2587eca1150 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Mon, 25 Sep 2017 10:13:53 -0600 Subject: work with actual string when reversing order --- activerecord/lib/active_record/relation/query_methods.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 4c63d0450a..7fe0129b4a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1075,6 +1075,9 @@ module ActiveRecord when Arel::Nodes::Ordering o.reverse when String + # ensure we're not dealing with string subclass (Eg. Arel::Nodes::SqlLiteral) + o = String.new(o) + if does_not_support_reverse?(o) raise IrreversibleOrderError, "Order #{o.inspect} can not be reversed automatically" end -- cgit v1.2.3 From 65328025917f0b60d0fbbeca87f173f34d9c91c5 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Mon, 25 Sep 2017 10:39:35 -0600 Subject: call enforce_raw_sql_whitelist on @klass so it works with FakeKlass --- activerecord/lib/active_record/relation/query_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 7fe0129b4a..f3b44d19d6 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -297,7 +297,7 @@ module ActiveRecord # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: - enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) + @klass.enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) preprocess_order_args(args) self.order_values += args @@ -320,7 +320,7 @@ module ActiveRecord # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: - enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) + @klass.enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) preprocess_order_args(args) self.reordering_value = true -- cgit v1.2.3 From 40d302d880f247ef9547708b1d26a390945b6fe9 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Mon, 25 Sep 2017 10:54:53 -0600 Subject: always allow Arel::Attributes::Attribute also --- activerecord/lib/active_record/attribute_methods.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 5ce3ba7a63..ff381b4e0b 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -170,7 +170,9 @@ module ActiveRecord def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc: unexpected = args.reject do |arg| whitelist.include?(arg.to_s) || - arg.kind_of?(Arel::Node) || arg.is_a?(Arel::Nodes::SqlLiteral) + arg.kind_of?(Arel::Node) || + arg.is_a?(Arel::Nodes::SqlLiteral) || + arg.is_a?(Arel::Attributes::Attribute) end return if unexpected.none? -- cgit v1.2.3 From 5180fe2cd8233169935065efe8762bd5d7b2709c Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Tue, 26 Sep 2017 09:29:24 -0600 Subject: allow table name and direction in string order arg --- .../lib/active_record/relation/calculations.rb | 31 ++++++---------------- .../lib/active_record/relation/query_methods.rb | 26 ++++++++++++++++-- 2 files changed, 32 insertions(+), 25 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 236d36e15f..75795fe493 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -180,14 +180,15 @@ module ActiveRecord end if has_include?(column_names.first) - construct_relation_for_association_calculations.pluck(*column_names) + relation = apply_join_dependency + relation.pluck(*column_names) else - enforce_raw_sql_whitelist(column_names) + enforce_raw_sql_whitelist(column_names, whitelist: allowed_pluck_columns) relation = spawn relation.select_values = column_names.map { |cn| @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn } - result = klass.connection.select_all(relation.arel, nil, bound_attributes) + result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } result.cast_values(klass.attribute_types) end end @@ -202,26 +203,10 @@ module ActiveRecord private - def _pluck(column_names, unsafe_raw) - unrecognized = column_names.reject do |cn| - @klass.respond_to_attribute?(cn) - end - - if loaded? && unrecognized.none? - records.pluck(*column_names) - elsif has_include?(column_names.first) - relation = apply_join_dependency - relation.pluck(*column_names) - elsif unsafe_raw || unrecognized.none? - relation = spawn - relation.select_values = column_names.map { |cn| - @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn - } - result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } - result.cast_values(klass.attribute_types) - else - raise ArgumentError, "Invalid column name: #{unrecognized}" - end + def allowed_pluck_columns + @klass.attribute_names_and_aliases.map do |name| + [name, "#{table_name}.#{name}"] + end.flatten end def has_include?(column_name) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index f3b44d19d6..094e5aa733 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -297,7 +297,11 @@ module ActiveRecord # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: - @klass.enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) + @klass.enforce_raw_sql_whitelist( + column_names_from_order_arguments(args), + whitelist: allowed_order_columns + ) + preprocess_order_args(args) self.order_values += args @@ -320,7 +324,11 @@ module ActiveRecord # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: - @klass.enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) + @klass.enforce_raw_sql_whitelist( + column_names_from_order_arguments(args), + whitelist: allowed_order_columns + ) + preprocess_order_args(args) self.reordering_value = true @@ -920,6 +928,20 @@ module ActiveRecord private + def allowed_order_columns + @klass.attribute_names_and_aliases.map do |name| + [name, "#{table_name}.#{name}"].map do |name| + [ + name, + "#{name} asc", + "#{name} ASC", + "#{name} desc", + "#{name} DESC" + ] + end + end.flatten + end + # Extract column names from arguments passed to #order or #reorder. def column_names_from_order_arguments(args) args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg } -- cgit v1.2.3 From 798557145c727b2abef2487783f02e57f04197c9 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 11 Oct 2017 13:16:57 -0600 Subject: try using regexes --- .../lib/active_record/attribute_methods.rb | 48 ++++++++-------------- .../lib/active_record/relation/calculations.rb | 12 ++---- .../lib/active_record/relation/query_methods.rb | 21 +--------- 3 files changed, 21 insertions(+), 60 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index ff381b4e0b..64f81ca582 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -167,12 +167,24 @@ module ActiveRecord end end - def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc: + # Regexp whitelist. Matches the following: + # "#{table_name}.#{column_name}" + # "#{column_name}" + COLUMN_NAME_WHITELIST = /\A(?:\w+\.)?\w+\z/i + + # Regexp whitelist. Matches the following: + # "#{table_name}.#{column_name}" + # "#{table_name}.#{column_name} #{direction}" + # "#{column_name}" + # "#{column_name} #{direction}" + COLUMN_NAME_ORDER_WHITELIST = /\A(?:\w+\.)?\w+(?:\s+asc|\s+desc)?\z/i + + def enforce_raw_sql_whitelist(args, whitelist: COLUMN_NAME_WHITELIST) # :nodoc: unexpected = args.reject do |arg| - whitelist.include?(arg.to_s) || - arg.kind_of?(Arel::Node) || + arg.kind_of?(Arel::Node) || arg.is_a?(Arel::Nodes::SqlLiteral) || - arg.is_a?(Arel::Attributes::Attribute) + arg.is_a?(Arel::Attributes::Attribute) || + arg.to_s.split(/\s*,\s*/).all? { |part| whitelist.match?(part) } end return if unexpected.none? @@ -195,22 +207,6 @@ module ActiveRecord end end - # Can the given name be treated as a column name? Returns true if name - # is attribute or attribute alias. - # - # class Person < ActiveRecord::Base - # end - # - # Person.respond_to_attribute?(:name) - # # => true - # - # Person.respond_to_attribute?("foo") - # # => false - def respond_to_attribute?(name) # :nodoc: - name = name.to_s - attribute_names.include?(name) || attribute_aliases.include?(name) - end - # Returns true if the given attribute exists, otherwise false. # # class Person < ActiveRecord::Base @@ -242,18 +238,6 @@ module ActiveRecord ConnectionAdapters::NullColumn.new(name) end end - - # An Array of String attribute names and aliases for accessing those - # attributes. - # - # class Person < ActiveRecord::Base - # end - # - # Person.attribute_names_and_aliases - # # => ["id", "created_at", "updated_at", "name", "age"] - def attribute_names_and_aliases # :nodoc: - attribute_names + attribute_aliases.keys - end end # A Person object with a name attribute can ask person.respond_to?(:name), diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 75795fe493..d49472fc70 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -175,7 +175,7 @@ module ActiveRecord # See also #ids. # def pluck(*column_names) - if loaded? && (column_names.map(&:to_s) - @klass.attribute_names_and_aliases).empty? + if loaded? && (column_names.map(&:to_s) - @klass.attribute_names - @klass.attribute_aliases.keys).empty? return records.pluck(*column_names) end @@ -183,10 +183,10 @@ module ActiveRecord relation = apply_join_dependency relation.pluck(*column_names) else - enforce_raw_sql_whitelist(column_names, whitelist: allowed_pluck_columns) + enforce_raw_sql_whitelist(column_names) relation = spawn relation.select_values = column_names.map { |cn| - @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn + @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn } result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } result.cast_values(klass.attribute_types) @@ -203,12 +203,6 @@ module ActiveRecord private - def allowed_pluck_columns - @klass.attribute_names_and_aliases.map do |name| - [name, "#{table_name}.#{name}"] - end.flatten - end - def has_include?(column_name) eager_loading? || (includes_values.present? && column_name && column_name != :all) end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 094e5aa733..59a732168c 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -299,7 +299,7 @@ module ActiveRecord def order!(*args) # :nodoc: @klass.enforce_raw_sql_whitelist( column_names_from_order_arguments(args), - whitelist: allowed_order_columns + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST ) preprocess_order_args(args) @@ -326,7 +326,7 @@ module ActiveRecord def reorder!(*args) # :nodoc: @klass.enforce_raw_sql_whitelist( column_names_from_order_arguments(args), - whitelist: allowed_order_columns + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST ) preprocess_order_args(args) @@ -928,20 +928,6 @@ module ActiveRecord private - def allowed_order_columns - @klass.attribute_names_and_aliases.map do |name| - [name, "#{table_name}.#{name}"].map do |name| - [ - name, - "#{name} asc", - "#{name} ASC", - "#{name} desc", - "#{name} DESC" - ] - end - end.flatten - end - # Extract column names from arguments passed to #order or #reorder. def column_names_from_order_arguments(args) args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg } @@ -1097,9 +1083,6 @@ module ActiveRecord when Arel::Nodes::Ordering o.reverse when String - # ensure we're not dealing with string subclass (Eg. Arel::Nodes::SqlLiteral) - o = String.new(o) - if does_not_support_reverse?(o) raise IrreversibleOrderError, "Order #{o.inspect} can not be reversed automatically" end -- cgit v1.2.3 From ab03eb9f576312c75e61caaf9705a8ac5175c769 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 11 Oct 2017 13:56:42 -0600 Subject: use << instead of #concat in #reverse_sql_order because we might be working with Arel SQL literator which overrides #concat --- activerecord/lib/active_record/relation/query_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 59a732168c..34723b0b4f 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1088,7 +1088,7 @@ module ActiveRecord end o.split(",").map! do |s| s.strip! - s.gsub!(/\sasc\Z/i, " DESC") || s.gsub!(/\sdesc\Z/i, " ASC") || s.concat(" DESC") + s.gsub!(/\sasc\Z/i, " DESC") || s.gsub!(/\sdesc\Z/i, " ASC") || (s << " DESC") end else o -- cgit v1.2.3 From c711a27d29a3201ff47751a1d788f1e634186dd3 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 11 Oct 2017 14:15:47 -0600 Subject: convert order arg to string before checking if we can reverse it --- activerecord/lib/active_record/relation/query_methods.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 34723b0b4f..db7fe8123d 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1097,6 +1097,10 @@ module ActiveRecord end def does_not_support_reverse?(order) + # Account for String subclasses like Arel::Nodes::SqlLiteral that + # override methods like #count. + order = String.new(order) unless order.instance_of?(String) + # Uses SQL function with multiple arguments. (order.include?(",") && order.split(",").find { |section| section.count("(") != section.count(")") }) || # Uses "nulls first" like construction. -- cgit v1.2.3 From b76cc29865fb69389ffdb7bd9f8085aa86354f82 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Thu, 12 Oct 2017 11:48:48 -0600 Subject: deal with Array arguments to #order --- activerecord/lib/active_record/relation/query_methods.rb | 14 +++++++++++++- activerecord/lib/active_record/sanitization.rb | 6 ++++++ 2 files changed, 19 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index db7fe8123d..1dcd786498 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -930,7 +930,19 @@ module ActiveRecord # Extract column names from arguments passed to #order or #reorder. def column_names_from_order_arguments(args) - args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg } + args.flat_map do |arg| + case arg + when Hash + # Tag.order(id: :desc) + arg.keys + when Array + # Tag.order([Arel.sql("field(id, ?)"), [1, 3, 2]]) + arg.flatten + else + # Tag.order(:id) + arg + end + end end def assert_mutability! diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 1c3099f55c..4caf1145f0 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -63,6 +63,12 @@ module ActiveRecord # # => "id ASC" def sanitize_sql_for_order(condition) # :doc: if condition.is_a?(Array) && condition.first.to_s.include?("?") + # Ensure we aren't dealing with a subclass of String that might + # override methods we use (eg. Arel::Nodes::SqlLiteral). + if condition.first.kind_of?(String) && !condition.first.instance_of?(String) + condition = [String.new(condition.first), *condition[1..-1]] + end + sanitize_sql_array(condition) else condition -- cgit v1.2.3 From 8ef71ac4a119a4c03d78db2372b41ddcc8a95035 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 18 Oct 2017 10:21:45 -0600 Subject: push order arg checks down to allow for binds --- .../lib/active_record/relation/query_methods.rb | 33 ++++------------------ activerecord/lib/active_record/sanitization.rb | 6 +++- 2 files changed, 11 insertions(+), 28 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 1dcd786498..cef0847651 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -297,11 +297,6 @@ module ActiveRecord # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: - @klass.enforce_raw_sql_whitelist( - column_names_from_order_arguments(args), - whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST - ) - preprocess_order_args(args) self.order_values += args @@ -324,11 +319,6 @@ module ActiveRecord # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: - @klass.enforce_raw_sql_whitelist( - column_names_from_order_arguments(args), - whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST - ) - preprocess_order_args(args) self.reordering_value = true @@ -928,23 +918,6 @@ module ActiveRecord private - # Extract column names from arguments passed to #order or #reorder. - def column_names_from_order_arguments(args) - args.flat_map do |arg| - case arg - when Hash - # Tag.order(id: :desc) - arg.keys - when Array - # Tag.order([Arel.sql("field(id, ?)"), [1, 3, 2]]) - arg.flatten - else - # Tag.order(:id) - arg - end - end - end - def assert_mutability! raise ImmutableRelation if @loaded raise ImmutableRelation if defined?(@arel) && @arel @@ -1146,6 +1119,12 @@ module ActiveRecord klass.send(:sanitize_sql_for_order, arg) end order_args.flatten! + + @klass.enforce_raw_sql_whitelist( + order_args.flat_map { |a| a.is_a?(Hash) ? a.keys : a }, + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST + ) + validate_order_args(order_args) references = order_args.grep(String) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 4caf1145f0..232743d3b6 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -63,13 +63,17 @@ module ActiveRecord # # => "id ASC" def sanitize_sql_for_order(condition) # :doc: if condition.is_a?(Array) && condition.first.to_s.include?("?") + enforce_raw_sql_whitelist([condition.first], + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST + ) + # Ensure we aren't dealing with a subclass of String that might # override methods we use (eg. Arel::Nodes::SqlLiteral). if condition.first.kind_of?(String) && !condition.first.instance_of?(String) condition = [String.new(condition.first), *condition[1..-1]] end - sanitize_sql_array(condition) + Arel.sql(sanitize_sql_array(condition)) else condition end -- cgit v1.2.3 From 1f9f6f6cfc57020ccb35f77872c56f069f337075 Mon Sep 17 00:00:00 2001 From: Brent Wheeldon Date: Thu, 2 Nov 2017 14:53:43 -0400 Subject: Prevent deadlocks with load interlock and DB lock. This fixes an issue where competing threads deadlock each other. - Thread A holds the load interlock but is blocked on getting the DB lock - Thread B holds the DB lock but is blocked on getting the load interlock (for example when there is a `Model.transaction` block that needs to autoload) This solution allows for dependency loading in other threads while a thread is waiting to acquire the DB lock. Fixes #31019 --- activerecord/lib/active_record/connection_adapters/abstract_adapter.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index e3aab8dad8..e91ef3b779 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -6,6 +6,7 @@ require "active_record/connection_adapters/schema_cache" require "active_record/connection_adapters/sql_type_metadata" require "active_record/connection_adapters/abstract/schema_dumper" require "active_record/connection_adapters/abstract/schema_creation" +require "active_support/concurrency/load_interlock_aware_monitor" require "arel/collectors/bind" require "arel/collectors/composite" require "arel/collectors/sql_string" @@ -108,7 +109,7 @@ module ActiveRecord @schema_cache = SchemaCache.new self @quoted_column_names, @quoted_table_names = {}, {} @visitor = arel_visitor - @lock = Monitor.new + @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true -- cgit v1.2.3 From c3675f50d2e59b7fc173d7b332860c4b1a24a726 Mon Sep 17 00:00:00 2001 From: Lisa Ugray Date: Thu, 19 Oct 2017 12:45:07 -0400 Subject: Move Attribute and AttributeSet to ActiveModel Use these to back the attributes API. Stop automatically including ActiveModel::Dirty in ActiveModel::Attributes, and make it optional. --- activerecord/lib/active_record.rb | 8 +- activerecord/lib/active_record/attribute.rb | 242 --------------------- .../attribute/user_provided_default.rb | 32 --- .../lib/active_record/attribute_methods/dirty.rb | 96 +------- .../active_record/attribute_mutation_tracker.rb | 111 ---------- activerecord/lib/active_record/attribute_set.rb | 113 ---------- .../lib/active_record/attribute_set/builder.rb | 126 ----------- .../active_record/attribute_set/yaml_encoder.rb | 43 ---- activerecord/lib/active_record/attributes.rb | 6 +- .../lib/active_record/legacy_yaml_adapter.rb | 2 +- activerecord/lib/active_record/model_schema.rb | 6 +- .../lib/active_record/relation/query_attribute.rb | 4 +- .../lib/active_record/relation/query_methods.rb | 4 +- 13 files changed, 19 insertions(+), 774 deletions(-) delete mode 100644 activerecord/lib/active_record/attribute.rb delete mode 100644 activerecord/lib/active_record/attribute/user_provided_default.rb delete mode 100644 activerecord/lib/active_record/attribute_mutation_tracker.rb delete mode 100644 activerecord/lib/active_record/attribute_set.rb delete mode 100644 activerecord/lib/active_record/attribute_set/builder.rb delete mode 100644 activerecord/lib/active_record/attribute_set/yaml_encoder.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index bf6dfd46e1..0e036f05f5 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -27,14 +27,14 @@ require "active_support" require "active_support/rails" require "active_model" require "arel" +require "yaml" require "active_record/version" -require "active_record/attribute_set" +require "active_model/attribute_set" module ActiveRecord extend ActiveSupport::Autoload - autoload :Attribute autoload :Base autoload :Callbacks autoload :Core @@ -181,3 +181,7 @@ end ActiveSupport.on_load(:i18n) do I18n.load_path << File.expand_path("active_record/locale/en.yml", __dir__) end + +YAML.load_tags["!ruby/object:ActiveRecord::AttributeSet"] = "ActiveModel::AttributeSet" +YAML.load_tags["!ruby/object:ActiveRecord::Attribute::FromDatabase"] = "ActiveModel::Attribute::FromDatabase" +YAML.load_tags["!ruby/object:ActiveRecord::LazyAttributeHash"] = "ActiveModel::LazyAttributeHash" diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb deleted file mode 100644 index fc474edc15..0000000000 --- a/activerecord/lib/active_record/attribute.rb +++ /dev/null @@ -1,242 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - class Attribute # :nodoc: - class << self - def from_database(name, value, type) - FromDatabase.new(name, value, type) - end - - def from_user(name, value, type, original_attribute = nil) - FromUser.new(name, value, type, original_attribute) - end - - def with_cast_value(name, value, type) - WithCastValue.new(name, value, type) - end - - def null(name) - Null.new(name) - end - - def uninitialized(name, type) - Uninitialized.new(name, type) - end - end - - attr_reader :name, :value_before_type_cast, :type - - # This method should not be called directly. - # Use #from_database or #from_user - def initialize(name, value_before_type_cast, type, original_attribute = nil) - @name = name - @value_before_type_cast = value_before_type_cast - @type = type - @original_attribute = original_attribute - end - - def value - # `defined?` is cheaper than `||=` when we get back falsy values - @value = type_cast(value_before_type_cast) unless defined?(@value) - @value - end - - def original_value - if assigned? - original_attribute.original_value - else - type_cast(value_before_type_cast) - end - end - - def value_for_database - type.serialize(value) - end - - def changed? - changed_from_assignment? || changed_in_place? - end - - def changed_in_place? - has_been_read? && type.changed_in_place?(original_value_for_database, value) - end - - def forgetting_assignment - with_value_from_database(value_for_database) - end - - def with_value_from_user(value) - type.assert_valid_value(value) - self.class.from_user(name, value, type, original_attribute || self) - end - - def with_value_from_database(value) - self.class.from_database(name, value, type) - end - - def with_cast_value(value) - self.class.with_cast_value(name, value, type) - end - - def with_type(type) - if changed_in_place? - with_value_from_user(value).with_type(type) - else - self.class.new(name, value_before_type_cast, type, original_attribute) - end - end - - def type_cast(*) - raise NotImplementedError - end - - def initialized? - true - end - - def came_from_user? - false - end - - def has_been_read? - defined?(@value) - end - - def ==(other) - self.class == other.class && - name == other.name && - value_before_type_cast == other.value_before_type_cast && - type == other.type - end - alias eql? == - - def hash - [self.class, name, value_before_type_cast, type].hash - end - - def init_with(coder) - @name = coder["name"] - @value_before_type_cast = coder["value_before_type_cast"] - @type = coder["type"] - @original_attribute = coder["original_attribute"] - @value = coder["value"] if coder.map.key?("value") - end - - def encode_with(coder) - coder["name"] = name - coder["value_before_type_cast"] = value_before_type_cast unless value_before_type_cast.nil? - coder["type"] = type if type - coder["original_attribute"] = original_attribute if original_attribute - coder["value"] = value if defined?(@value) - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :original_attribute - alias_method :assigned?, :original_attribute - - def original_value_for_database - if assigned? - original_attribute.original_value_for_database - else - _original_value_for_database - end - end - - private - def initialize_dup(other) - if defined?(@value) && @value.duplicable? - @value = @value.dup - end - end - - def changed_from_assignment? - assigned? && type.changed?(original_value, value, value_before_type_cast) - end - - def _original_value_for_database - type.serialize(original_value) - end - - class FromDatabase < Attribute # :nodoc: - def type_cast(value) - type.deserialize(value) - end - - def _original_value_for_database - value_before_type_cast - end - end - - class FromUser < Attribute # :nodoc: - def type_cast(value) - type.cast(value) - end - - def came_from_user? - !type.value_constructed_by_mass_assignment?(value_before_type_cast) - end - end - - class WithCastValue < Attribute # :nodoc: - def type_cast(value) - value - end - - def changed_in_place? - false - end - end - - class Null < Attribute # :nodoc: - def initialize(name) - super(name, nil, Type.default_value) - end - - def type_cast(*) - nil - end - - def with_type(type) - self.class.with_cast_value(name, nil, type) - end - - def with_value_from_database(value) - raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{name}`" - end - alias_method :with_value_from_user, :with_value_from_database - end - - class Uninitialized < Attribute # :nodoc: - UNINITIALIZED_ORIGINAL_VALUE = Object.new - - def initialize(name, type) - super(name, nil, type) - end - - def value - if block_given? - yield name - end - end - - def original_value - UNINITIALIZED_ORIGINAL_VALUE - end - - def value_for_database - end - - def initialized? - false - end - - def with_type(type) - self.class.new(name, type) - end - end - private_constant :FromDatabase, :FromUser, :Null, :Uninitialized, :WithCastValue - end -end diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activerecord/lib/active_record/attribute/user_provided_default.rb deleted file mode 100644 index f746960fac..0000000000 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require "active_record/attribute" - -module ActiveRecord - class Attribute # :nodoc: - class UserProvidedDefault < FromUser # :nodoc: - def initialize(name, value, type, database_default) - @user_provided_value = value - super(name, value, type, database_default) - end - - def value_before_type_cast - if user_provided_value.is_a?(Proc) - @memoized_value_before_type_cast ||= user_provided_value.call - else - @user_provided_value - end - end - - def with_type(type) - self.class.new(name, user_provided_value, type, original_attribute) - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :user_provided_value - end - end -end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 79110d04f4..3de6fe566d 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/module/attribute_accessors" -require "active_record/attribute_mutation_tracker" module ActiveRecord module AttributeMethods @@ -33,65 +32,13 @@ module ActiveRecord # reload the record and clears changed attributes. def reload(*) super.tap do + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new @mutations_from_database = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new end end - def initialize_dup(other) # :nodoc: - super - @attributes = self.class._default_attributes.map do |attr| - attr.with_value_from_user(@attributes.fetch_value(attr.name)) - end - @mutations_from_database = nil - end - - def changes_applied # :nodoc: - @mutations_before_last_save = mutations_from_database - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_changes_information # :nodoc: - @mutations_before_last_save = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_attribute_changes(attr_names) # :nodoc: - super - attr_names.each do |attr_name| - clear_attribute_change(attr_name) - end - end - - 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) - @cached_changed_attributes - else - super.reverse_merge(mutations_from_database.changed_values).freeze - end - end - - def changes # :nodoc: - cache_changed_attributes do - super - end - end - - def previous_changes # :nodoc: - mutations_before_last_save.changes - end - - def attribute_changed_in_place?(attr_name) # :nodoc: - mutations_from_database.changed_in_place?(attr_name) - end - # Did this attribute change when we last saved? This method can be invoked # as +saved_change_to_name?+ instead of saved_change_to_attribute?("name"). # Behaves similarly to +attribute_changed?+. This method is useful in @@ -182,26 +129,6 @@ module ActiveRecord result end - def mutations_from_database - unless defined?(@mutations_from_database) - @mutations_from_database = nil - end - @mutations_from_database ||= AttributeMutationTracker.new(@attributes) - end - - def changes_include?(attr_name) - super || mutations_from_database.changed?(attr_name) - end - - def clear_attribute_change(attr_name) - mutations_from_database.forget_change(attr_name) - end - - def attribute_will_change!(attr_name) - super - mutations_from_database.force_change(attr_name) - end - def _update_record(*) partial_writes? ? super(keys_for_partial_write) : super end @@ -213,25 +140,6 @@ module ActiveRecord def keys_for_partial_write changed_attribute_names_to_save & self.class.column_names end - - def forget_attribute_assignments - @attributes = @attributes.map(&:forgetting_assignment) - end - - def mutations_before_last_save - @mutations_before_last_save ||= NullMutationTracker.instance - end - - def cache_changed_attributes - @cached_changed_attributes = changed_attributes - yield - ensure - clear_changed_attributes_cache - end - - def clear_changed_attributes_cache - remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) - end end end end diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb deleted file mode 100644 index 94bf641a5d..0000000000 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ /dev/null @@ -1,111 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - class AttributeMutationTracker # :nodoc: - OPTION_NOT_GIVEN = Object.new - - def initialize(attributes) - @attributes = attributes - @forced_changes = Set.new - end - - def changed_values - attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| - if changed?(attr_name) - result[attr_name] = attributes[attr_name].original_value - end - end - end - - def changes - attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| - change = change_to_attribute(attr_name) - if change - result[attr_name] = change - end - end - end - - def change_to_attribute(attr_name) - attr_name = attr_name.to_s - if changed?(attr_name) - [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] - end - end - - def any_changes? - attr_names.any? { |attr| changed?(attr) } - end - - def changed?(attr_name, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) - attr_name = attr_name.to_s - forced_changes.include?(attr_name) || - attributes[attr_name].changed? && - (OPTION_NOT_GIVEN == from || attributes[attr_name].original_value == from) && - (OPTION_NOT_GIVEN == to || attributes[attr_name].value == to) - end - - def changed_in_place?(attr_name) - attributes[attr_name.to_s].changed_in_place? - end - - def forget_change(attr_name) - attr_name = attr_name.to_s - attributes[attr_name] = attributes[attr_name].forgetting_assignment - forced_changes.delete(attr_name) - end - - def original_value(attr_name) - attributes[attr_name.to_s].original_value - end - - def force_change(attr_name) - forced_changes << attr_name.to_s - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :attributes, :forced_changes - - private - - def attr_names - attributes.keys - end - end - - class NullMutationTracker # :nodoc: - include Singleton - - def changed_values(*) - {} - end - - def changes(*) - {} - end - - def change_to_attribute(attr_name) - end - - def any_changes?(*) - false - end - - def changed?(*) - false - end - - def changed_in_place?(*) - false - end - - def forget_change(*) - end - - def original_value(*) - end - end -end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb deleted file mode 100644 index 9785666e77..0000000000 --- a/activerecord/lib/active_record/attribute_set.rb +++ /dev/null @@ -1,113 +0,0 @@ -# frozen_string_literal: true - -require "active_record/attribute_set/builder" -require "active_record/attribute_set/yaml_encoder" - -module ActiveRecord - class AttributeSet # :nodoc: - delegate :each_value, :fetch, to: :attributes - - def initialize(attributes) - @attributes = attributes - end - - def [](name) - attributes[name] || Attribute.null(name) - end - - def []=(name, value) - attributes[name] = value - end - - def values_before_type_cast - attributes.transform_values(&:value_before_type_cast) - end - - def to_hash - initialized_attributes.transform_values(&:value) - end - alias_method :to_h, :to_hash - - def key?(name) - attributes.key?(name) && self[name].initialized? - end - - def keys - attributes.each_key.select { |name| self[name].initialized? } - end - - if defined?(JRUBY_VERSION) - # This form is significantly faster on JRuby, and this is one of our biggest hotspots. - # https://github.com/jruby/jruby/pull/2562 - def fetch_value(name, &block) - self[name].value(&block) - end - else - def fetch_value(name) - self[name].value { |n| yield n if block_given? } - end - end - - def write_from_database(name, value) - attributes[name] = self[name].with_value_from_database(value) - end - - def write_from_user(name, value) - attributes[name] = self[name].with_value_from_user(value) - end - - def write_cast_value(name, value) - attributes[name] = self[name].with_cast_value(value) - end - - def freeze - @attributes.freeze - super - end - - def deep_dup - self.class.allocate.tap do |copy| - copy.instance_variable_set(:@attributes, attributes.deep_dup) - end - end - - def initialize_dup(_) - @attributes = attributes.dup - super - end - - def initialize_clone(_) - @attributes = attributes.clone - super - end - - def reset(key) - if key?(key) - write_from_database(key, nil) - end - end - - def accessed - attributes.select { |_, attr| attr.has_been_read? }.keys - end - - def map(&block) - new_attributes = attributes.transform_values(&block) - AttributeSet.new(new_attributes) - end - - def ==(other) - attributes == other.attributes - end - - protected - - attr_reader :attributes - - private - - def initialized_attributes - attributes.select { |_, attr| attr.initialized? } - end - end -end diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb deleted file mode 100644 index 349cc7e403..0000000000 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -require "active_record/attribute" - -module ActiveRecord - class AttributeSet # :nodoc: - class Builder # :nodoc: - attr_reader :types, :always_initialized, :default - - def initialize(types, always_initialized = nil, &default) - @types = types - @always_initialized = always_initialized - @default = default - end - - def build_from_database(values = {}, additional_types = {}) - if always_initialized && !values.key?(always_initialized) - values[always_initialized] = nil - end - - attributes = LazyAttributeHash.new(types, values, additional_types, &default) - AttributeSet.new(attributes) - end - end - end - - class LazyAttributeHash # :nodoc: - delegate :transform_values, :each_key, :each_value, :fetch, to: :materialize - - def initialize(types, values, additional_types, &default) - @types = types - @values = values - @additional_types = additional_types - @materialized = false - @delegate_hash = {} - @default = default || proc {} - end - - def key?(key) - delegate_hash.key?(key) || values.key?(key) || types.key?(key) - end - - def [](key) - delegate_hash[key] || assign_default_value(key) - end - - def []=(key, value) - if frozen? - raise RuntimeError, "Can't modify frozen hash" - end - delegate_hash[key] = value - end - - def deep_dup - dup.tap do |copy| - copy.instance_variable_set(:@delegate_hash, delegate_hash.transform_values(&:dup)) - end - end - - def initialize_dup(_) - @delegate_hash = Hash[delegate_hash] - super - end - - def select - keys = types.keys | values.keys | delegate_hash.keys - keys.each_with_object({}) do |key, hash| - attribute = self[key] - if yield(key, attribute) - hash[key] = attribute - end - end - end - - def ==(other) - if other.is_a?(LazyAttributeHash) - materialize == other.materialize - else - materialize == other - end - end - - def marshal_dump - materialize - end - - def marshal_load(delegate_hash) - @delegate_hash = delegate_hash - @types = {} - @values = {} - @additional_types = {} - @materialized = true - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :types, :values, :additional_types, :delegate_hash, :default - - def materialize - unless @materialized - values.each_key { |key| self[key] } - types.each_key { |key| self[key] } - unless frozen? - @materialized = true - end - end - delegate_hash - end - - private - - def assign_default_value(name) - type = additional_types.fetch(name, types[name]) - value_present = true - value = values.fetch(name) { value_present = false } - - if value_present - delegate_hash[name] = Attribute.from_database(name, value, type) - elsif types.key?(name) - delegate_hash[name] = default.call(name) || Attribute.uninitialized(name, type) - end - end - end -end diff --git a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb b/activerecord/lib/active_record/attribute_set/yaml_encoder.rb deleted file mode 100644 index 9254ce16ab..0000000000 --- a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - class AttributeSet - # Attempts to do more intelligent YAML dumping of an - # ActiveRecord::AttributeSet to reduce the size of the resulting string - class YAMLEncoder # :nodoc: - def initialize(default_types) - @default_types = default_types - end - - def encode(attribute_set, coder) - coder["concise_attributes"] = attribute_set.each_value.map do |attr| - if attr.type.equal?(default_types[attr.name]) - attr.with_type(nil) - else - attr - end - end - end - - def decode(coder) - if coder["attributes"] - coder["attributes"] - else - attributes_hash = Hash[coder["concise_attributes"].map do |attr| - if attr.type.nil? - attr = attr.with_type(default_types[attr.name]) - end - [attr.name, attr] - end] - AttributeSet.new(attributes_hash) - end - end - - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :default_types - end - end -end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 9224d58928..0b7c9398a8 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "active_record/attribute/user_provided_default" +require "active_model/attribute/user_provided_default" module ActiveRecord # See ActiveRecord::Attributes::ClassMethods for documentation @@ -250,14 +250,14 @@ module ActiveRecord if value == NO_DEFAULT_PROVIDED default_attribute = _default_attributes[name].with_type(type) elsif from_user - default_attribute = Attribute::UserProvidedDefault.new( + default_attribute = ActiveModel::Attribute::UserProvidedDefault.new( name, value, type, _default_attributes.fetch(name.to_s) { nil }, ) else - default_attribute = Attribute.from_database(name, value, type) + default_attribute = ActiveModel::Attribute.from_database(name, value, type) end _default_attributes[name] = default_attribute end diff --git a/activerecord/lib/active_record/legacy_yaml_adapter.rb b/activerecord/lib/active_record/legacy_yaml_adapter.rb index 23644aab8f..ffa095dd94 100644 --- a/activerecord/lib/active_record/legacy_yaml_adapter.rb +++ b/activerecord/lib/active_record/legacy_yaml_adapter.rb @@ -8,7 +8,7 @@ module ActiveRecord case coder["active_record_yaml_version"] when 1, 2 then coder else - if coder["attributes"].is_a?(AttributeSet) + if coder["attributes"].is_a?(ActiveModel::AttributeSet) Rails420.convert(klass, coder) else Rails41.convert(klass, coder) diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index bed9400f51..12ee4a4137 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -323,7 +323,7 @@ module ActiveRecord end def attributes_builder # :nodoc: - @attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key) do |name| + @attributes_builder ||= ActiveModel::AttributeSet::Builder.new(attribute_types, primary_key) do |name| unless columns_hash.key?(name) _default_attributes[name].dup end @@ -346,7 +346,7 @@ module ActiveRecord end def yaml_encoder # :nodoc: - @yaml_encoder ||= AttributeSet::YAMLEncoder.new(attribute_types) + @yaml_encoder ||= ActiveModel::AttributeSet::YAMLEncoder.new(attribute_types) end # Returns the type of the attribute with the given name, after applying @@ -376,7 +376,7 @@ module ActiveRecord end def _default_attributes # :nodoc: - @default_attributes ||= AttributeSet.new({}) + @default_attributes ||= ActiveModel::AttributeSet.new({}) end # Returns an array of column names as strings. diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index fad08e2613..3532f28858 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" module ActiveRecord class Relation - class QueryAttribute < Attribute # :nodoc: + class QueryAttribute < ActiveModel::Attribute # :nodoc: def type_cast(value) value end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 897ff5c8af..9c5be4ad9b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -930,7 +930,7 @@ module ActiveRecord arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? if limit_value - limit_attribute = Attribute.with_cast_value( + limit_attribute = ActiveModel::Attribute.with_cast_value( "LIMIT".freeze, connection.sanitize_limit(limit_value), Type.default_value, @@ -938,7 +938,7 @@ module ActiveRecord arel.take(Arel::Nodes::BindParam.new(limit_attribute)) end if offset_value - offset_attribute = Attribute.with_cast_value( + offset_attribute = ActiveModel::Attribute.with_cast_value( "OFFSET".freeze, offset_value.to_i, Type.default_value, -- cgit v1.2.3 From de40c45f2b7d4a2ba47d28e9a4967134e45df91f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 8 Nov 2017 22:21:12 +0900 Subject: Remove useless preloader classes They are only different by one line of code which doesn't deserve a hierarchy of 7 classes. Closes #31079. [Ryuta Kamizono & Bogdan Gusiev] --- .../lib/active_record/associations/preloader.rb | 26 ++++++---------------- .../associations/preloader/association.rb | 8 ++++++- .../associations/preloader/belongs_to.rb | 10 --------- .../preloader/collection_association.rb | 16 ------------- .../associations/preloader/has_many.rb | 10 --------- .../associations/preloader/has_many_through.rb | 11 --------- .../associations/preloader/has_one.rb | 10 --------- .../associations/preloader/has_one_through.rb | 11 --------- .../associations/preloader/singular_association.rb | 15 ------------- .../associations/preloader/through_association.rb | 2 +- 10 files changed, 15 insertions(+), 104 deletions(-) delete mode 100644 activerecord/lib/active_record/associations/preloader/belongs_to.rb delete mode 100644 activerecord/lib/active_record/associations/preloader/collection_association.rb delete mode 100644 activerecord/lib/active_record/associations/preloader/has_many.rb delete mode 100644 activerecord/lib/active_record/associations/preloader/has_many_through.rb delete mode 100644 activerecord/lib/active_record/associations/preloader/has_one.rb delete mode 100644 activerecord/lib/active_record/associations/preloader/has_one_through.rb delete mode 100644 activerecord/lib/active_record/associations/preloader/singular_association.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 5a93a89d0a..e1087be9b3 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -44,16 +44,8 @@ module ActiveRecord extend ActiveSupport::Autoload eager_autoload do - autoload :Association, "active_record/associations/preloader/association" - autoload :SingularAssociation, "active_record/associations/preloader/singular_association" - autoload :CollectionAssociation, "active_record/associations/preloader/collection_association" - autoload :ThroughAssociation, "active_record/associations/preloader/through_association" - - autoload :HasMany, "active_record/associations/preloader/has_many" - autoload :HasManyThrough, "active_record/associations/preloader/has_many_through" - autoload :HasOne, "active_record/associations/preloader/has_one" - autoload :HasOneThrough, "active_record/associations/preloader/has_one_through" - autoload :BelongsTo, "active_record/associations/preloader/belongs_to" + autoload :Association, "active_record/associations/preloader/association" + autoload :ThroughAssociation, "active_record/associations/preloader/through_association" end # Eager loads the named associations for the given Active Record record(s). @@ -182,8 +174,7 @@ module ActiveRecord end # Returns a class containing the logic needed to load preload the data - # and attach it to a relation. For example +Preloader::Association+ or - # +Preloader::HasManyThrough+. The class returned implements a `run` method + # and attach it to a relation. The class returned implements a `run` method # that accepts a preloader. def preloader_for(reflection, owners) if owners.first.association(reflection.name).loaded? @@ -191,13 +182,10 @@ module ActiveRecord end reflection.check_preloadable! - case reflection.macro - when :has_many - reflection.options[:through] ? HasManyThrough : HasMany - when :has_one - reflection.options[:through] ? HasOneThrough : HasOne - when :belongs_to - BelongsTo + if reflection.options[:through] + ThroughAssociation + else + Association end end end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 19c337dc39..735da152b7 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -42,7 +42,13 @@ module ActiveRecord end def associate_records_to_owner(owner, records) - raise NotImplementedError + association = owner.association(reflection.name) + if reflection.collection? + association.loaded! + association.target.concat(records) + else + association.target = records.first + end end def owner_keys diff --git a/activerecord/lib/active_record/associations/preloader/belongs_to.rb b/activerecord/lib/active_record/associations/preloader/belongs_to.rb deleted file mode 100644 index a8e3340b23..0000000000 --- a/activerecord/lib/active_record/associations/preloader/belongs_to.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class BelongsTo < SingularAssociation #:nodoc: - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb deleted file mode 100644 index fc2029f54a..0000000000 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class CollectionAssociation < Association #:nodoc: - private - def associate_records_to_owner(owner, records) - association = owner.association(reflection.name) - association.loaded! - association.target.concat(records) - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_many.rb b/activerecord/lib/active_record/associations/preloader/has_many.rb deleted file mode 100644 index 72f55bc43f..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_many.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasMany < CollectionAssociation #:nodoc: - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb deleted file mode 100644 index 3e17d07a33..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasManyThrough < CollectionAssociation #:nodoc: - include ThroughAssociation - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_one.rb b/activerecord/lib/active_record/associations/preloader/has_one.rb deleted file mode 100644 index e339b65fb5..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_one.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasOne < SingularAssociation #:nodoc: - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_one_through.rb b/activerecord/lib/active_record/associations/preloader/has_one_through.rb deleted file mode 100644 index 17734d0257..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_one_through.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class HasOneThrough < SingularAssociation #:nodoc: - include ThroughAssociation - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb deleted file mode 100644 index 30a92411e3..0000000000 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module Associations - class Preloader - class SingularAssociation < Association #:nodoc: - private - def associate_records_to_owner(owner, records) - association = owner.association(reflection.name) - association.target = records.first - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 762275fbad..a6b7ab80a2 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -3,7 +3,7 @@ module ActiveRecord module Associations class Preloader - module ThroughAssociation #:nodoc: + class ThroughAssociation < Association # :nodoc: def run(preloader) already_loaded = owners.first.association(through_reflection.name).loaded? through_scope = through_scope() -- cgit v1.2.3 From df49896a1e67feea56062639c3cf51e8e0b12a51 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 10 Nov 2017 22:22:55 +0900 Subject: Ensure `apply_join_dependency` for subqueries in `from` and `where` Fixes #21577. --- .../lib/active_record/relation/predicate_builder/relation_handler.rb | 4 ++++ activerecord/lib/active_record/relation/query_methods.rb | 3 +++ 2 files changed, 7 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb index f51ea4fde0..c8bbfa5051 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/relation_handler.rb @@ -4,6 +4,10 @@ module ActiveRecord class PredicateBuilder class RelationHandler # :nodoc: def call(attribute, value) + if value.eager_loading? + value = value.send(:apply_join_dependency) + end + if value.select_values.empty? value = value.select(value.arel_attribute(value.klass.primary_key)) end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 9c5be4ad9b..34554450dd 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -963,6 +963,9 @@ module ActiveRecord name = from_clause.name case opts when Relation + if opts.eager_loading? + opts = opts.send(:apply_join_dependency) + end name ||= "subquery" opts.arel.as(name.to_s) else -- cgit v1.2.3 From b1e068bd30a88fbcc93a835edd6dbacf1d2d251c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 10 Nov 2017 23:33:39 +0900 Subject: Consolidate duplicated `to_ary`/`to_a` definitions in `Relation` and `CollectionProxy` --- .../lib/active_record/associations/collection_proxy.rb | 10 ++++++---- activerecord/lib/active_record/relation.rb | 3 ++- activerecord/lib/active_record/relation/delegation.rb | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 07f7303f8d..8b4a48a38c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -988,6 +988,12 @@ module ActiveRecord load_target == other end + ## + # :method: to_ary + # + # :call-seq: + # to_ary() + # # Returns a new array of objects from the collection. If the collection # hasn't been loaded, it fetches the records from the database. # @@ -1021,10 +1027,6 @@ module ActiveRecord # # #, # # # # # ] - def to_ary - load_target.dup - end - alias_method :to_a, :to_ary def records # :nodoc: load_target diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7615fb6ee9..e2d2f45503 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -243,9 +243,10 @@ module ActiveRecord end # Converts relation objects to Array. - def to_a + def to_ary records.dup end + alias to_a to_ary def records # :nodoc: load diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 48af777b69..4863befec8 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -38,7 +38,7 @@ module ActiveRecord # may vary depending on the klass of a relation, so we create a subclass of Relation # for each different klass, and the delegations are compiled into that subclass only. - delegate :to_xml, :encode_with, :length, :each, :uniq, :to_ary, :join, + delegate :to_xml, :encode_with, :length, :each, :uniq, :join, :[], :&, :|, :+, :-, :sample, :reverse, :rotate, :compact, :in_groups, :in_groups_of, :to_sentence, :to_formatted_s, :as_json, :shuffle, :split, :slice, :index, :rindex, to: :records -- cgit v1.2.3 From 4528dd6327f35d3139a48cbac9c9192f2253cbad Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 11 Nov 2017 03:54:10 +0900 Subject: Relation merging should keep joining order `joins_values.partition` will break joins values order. It should be kept as user intended order. Fixes #15488. --- activerecord/lib/active_record/relation/merger.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index ebc72d28fd..b736b21525 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -112,22 +112,20 @@ module ActiveRecord if other.klass == relation.klass relation.joins!(*other.joins_values) else - joins_dependency, rest = other.joins_values.partition do |join| + alias_tracker = nil + joins_dependency = other.joins_values.map do |join| case join when Hash, Symbol, Array - true + alias_tracker ||= other.alias_tracker + ActiveRecord::Associations::JoinDependency.new( + other.klass, other.table, join, alias_tracker + ) else - false + join end end - join_dependency = ActiveRecord::Associations::JoinDependency.new( - other.klass, other.table, joins_dependency, other.alias_tracker - ) - - relation.joins! rest - - @relation = relation.joins join_dependency + relation.joins!(*joins_dependency) end end -- cgit v1.2.3 From 24b59434e6aca9679b9f86a41cfbb1a33e3d5619 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 11 Nov 2017 06:43:54 +0900 Subject: Add missing autoload `Type` (#31123) Attribute modules (`Attribute`, `Attributes`, `AttributeSet`) uses `Type`, but referencing `Type` before the modules still fail. ``` % ./bin/test -w test/cases/attribute_test.rb -n test_with_value_from_user_validates_the_value Run options: -n test_with_value_from_user_validates_the_value --seed 31876 E Error: ActiveModel::AttributeTest#test_with_value_from_user_validates_the_value: NameError: uninitialized constant ActiveModel::AttributeTest::Type /Users/kamipo/src/github.com/rails/rails/activemodel/test/cases/attribute_test.rb:233:in `block in ' bin/test test/cases/attribute_test.rb:232 Finished in 0.002985s, 335.0479 runs/s, 335.0479 assertions/s. 1 runs, 1 assertions, 0 failures, 1 errors, 0 skips ``` Probably we need more autoloading at least `Type`. --- activerecord/lib/active_record.rb | 1 + activerecord/lib/active_record/connection_adapters/abstract_adapter.rb | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 0e036f05f5..5de6503144 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -104,6 +104,7 @@ module ActiveRecord autoload :Result autoload :TableMetadata + autoload :Type end module Coders diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index e3aab8dad8..345983a655 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_record/type" require "active_record/connection_adapters/determine_if_preparable_visitor" require "active_record/connection_adapters/schema_cache" require "active_record/connection_adapters/sql_type_metadata" -- cgit v1.2.3 From 4a65dfcb9adae8fb12a86521c1a34b392e6084c2 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 11 Nov 2017 19:53:40 +0900 Subject: Raise `TransactionTimeout` when lock wait timeout exceeded for PG adapter Follow up of #30360. --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 2c3c1df2a9..46863c41ab 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -391,6 +391,7 @@ module ActiveRecord UNIQUE_VIOLATION = "23505" SERIALIZATION_FAILURE = "40001" DEADLOCK_DETECTED = "40P01" + LOCK_NOT_AVAILABLE = "55P03" def translate_exception(exception, message) return exception unless exception.respond_to?(:result) @@ -410,6 +411,8 @@ module ActiveRecord SerializationFailure.new(message) when DEADLOCK_DETECTED Deadlocked.new(message) + when LOCK_NOT_AVAILABLE + TransactionTimeout.new(message) else super end -- cgit v1.2.3 From a968a7609db56f56298c462aa26809588f9375de Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 13 Nov 2017 20:15:16 +0900 Subject: Add new error class `StatementTimeout` which will be raised when statement timeout exceeded (#31129) We are sometimes using The MAX_EXECUTION_TIME hint for MySQL depending on the situation. It will prevent catastrophic performance down by wrong performing queries. The new error class `StatementTimeout` will make to be easier to handle that case. https://dev.mysql.com/doc/refman/5.7/en/optimizer-hints.html#optimizer-hints-execution-time --- .../active_record/connection_adapters/abstract_mysql_adapter.rb | 3 +++ .../lib/active_record/connection_adapters/postgresql_adapter.rb | 3 +++ activerecord/lib/active_record/errors.rb | 7 +++++-- 3 files changed, 11 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') 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 bfec6fb784..ca651ef390 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -635,6 +635,7 @@ module ActiveRecord ER_CANNOT_ADD_FOREIGN = 1215 ER_CANNOT_CREATE_TABLE = 1005 ER_LOCK_WAIT_TIMEOUT = 1205 + ER_QUERY_TIMEOUT = 3024 def translate_exception(exception, message) case error_number(exception) @@ -660,6 +661,8 @@ module ActiveRecord Deadlocked.new(message) when ER_LOCK_WAIT_TIMEOUT TransactionTimeout.new(message) + when ER_QUERY_TIMEOUT + StatementTimeout.new(message) else super end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 46863c41ab..5ce6765dd8 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -392,6 +392,7 @@ module ActiveRecord SERIALIZATION_FAILURE = "40001" DEADLOCK_DETECTED = "40P01" LOCK_NOT_AVAILABLE = "55P03" + QUERY_CANCELED = "57014" def translate_exception(exception, message) return exception unless exception.respond_to?(:result) @@ -413,6 +414,8 @@ module ActiveRecord Deadlocked.new(message) when LOCK_NOT_AVAILABLE TransactionTimeout.new(message) + when QUERY_CANCELED + StatementTimeout.new(message) else super end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 9ef3316393..f77cd23e22 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -335,8 +335,11 @@ module ActiveRecord class IrreversibleOrderError < ActiveRecordError end - # TransactionTimeout will be raised when lock wait timeout expires. - # Wait time value is set by innodb_lock_wait_timeout. + # TransactionTimeout will be raised when lock wait timeout exceeded. class TransactionTimeout < StatementInvalid end + + # StatementTimeout will be raised when statement timeout exceeded. + class StatementTimeout < StatementInvalid + end end -- cgit v1.2.3 From 6acde9578fa55ceab8ef6520bbd5aab2a860d051 Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Sun, 5 Feb 2017 16:40:03 -0500 Subject: Do not use `Arel.star` when `ignored_columns` If there are any ignored columns, we will now list out all columns we want to be returned from the database. Includes a regression test. --- activerecord/lib/active_record/relation/query_methods.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 897ff5c8af..a8800e432a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1035,6 +1035,8 @@ module ActiveRecord def build_select(arel) if select_values.any? arel.project(*arel_columns(select_values.uniq)) + elsif @klass.ignored_columns.any? + arel.project(*arel_columns(@klass.column_names)) else arel.project(table[Arel.star]) end -- cgit v1.2.3 From 8dd76a7a6ff1bb7105beabb8f834ca54ab1e5fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 13 Nov 2017 15:23:28 -0500 Subject: Use .tt extension to all the template files Make clear that the files are not to be run for interpreters. Fixes #23847. Fixes #30690. Closes #23878. --- .../templates/application_record.rb | 5 --- .../templates/application_record.rb.tt | 5 +++ .../migration/templates/create_table_migration.rb | 24 ----------- .../templates/create_table_migration.rb.tt | 24 +++++++++++ .../active_record/migration/templates/migration.rb | 46 ---------------------- .../migration/templates/migration.rb.tt | 46 ++++++++++++++++++++++ .../active_record/model/templates/model.rb | 13 ------ .../active_record/model/templates/model.rb.tt | 13 ++++++ .../active_record/model/templates/module.rb | 7 ---- .../active_record/model/templates/module.rb.tt | 7 ++++ 10 files changed, 95 insertions(+), 95 deletions(-) delete mode 100644 activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb create mode 100644 activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb.tt delete mode 100644 activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb create mode 100644 activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt delete mode 100644 activerecord/lib/rails/generators/active_record/migration/templates/migration.rb create mode 100644 activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt delete mode 100644 activerecord/lib/rails/generators/active_record/model/templates/model.rb create mode 100644 activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt delete mode 100644 activerecord/lib/rails/generators/active_record/model/templates/module.rb create mode 100644 activerecord/lib/rails/generators/active_record/model/templates/module.rb.tt (limited to 'activerecord/lib') diff --git a/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb b/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb deleted file mode 100644 index 60050e0bf8..0000000000 --- a/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb +++ /dev/null @@ -1,5 +0,0 @@ -<% module_namespacing do -%> -class ApplicationRecord < ActiveRecord::Base - self.abstract_class = true -end -<% end -%> diff --git a/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb.tt b/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb.tt new file mode 100644 index 0000000000..60050e0bf8 --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/application_record/templates/application_record.rb.tt @@ -0,0 +1,5 @@ +<% module_namespacing do -%> +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end +<% end -%> diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb deleted file mode 100644 index 5f7201cfe1..0000000000 --- a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb +++ /dev/null @@ -1,24 +0,0 @@ -class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>] - def change - create_table :<%= table_name %><%= primary_key_type %> do |t| -<% attributes.each do |attribute| -%> -<% if attribute.password_digest? -%> - t.string :password_digest<%= attribute.inject_options %> -<% elsif attribute.token? -%> - t.string :<%= attribute.name %><%= attribute.inject_options %> -<% else -%> - t.<%= attribute.type %> :<%= attribute.name %><%= attribute.inject_options %> -<% end -%> -<% end -%> -<% if options[:timestamps] %> - t.timestamps -<% end -%> - end -<% attributes.select(&:token?).each do |attribute| -%> - add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %>, unique: true -<% end -%> -<% attributes_with_index.each do |attribute| -%> - add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> -<% end -%> - end -end diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt new file mode 100644 index 0000000000..5f7201cfe1 --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt @@ -0,0 +1,24 @@ +class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>] + def change + create_table :<%= table_name %><%= primary_key_type %> do |t| +<% attributes.each do |attribute| -%> +<% if attribute.password_digest? -%> + t.string :password_digest<%= attribute.inject_options %> +<% elsif attribute.token? -%> + t.string :<%= attribute.name %><%= attribute.inject_options %> +<% else -%> + t.<%= attribute.type %> :<%= attribute.name %><%= attribute.inject_options %> +<% end -%> +<% end -%> +<% if options[:timestamps] %> + t.timestamps +<% end -%> + end +<% attributes.select(&:token?).each do |attribute| -%> + add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %>, unique: true +<% end -%> +<% attributes_with_index.each do |attribute| -%> + add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> +<% end -%> + end +end diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb deleted file mode 100644 index 481c70201b..0000000000 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb +++ /dev/null @@ -1,46 +0,0 @@ -class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>] -<%- if migration_action == 'add' -%> - def change -<% attributes.each do |attribute| -%> - <%- if attribute.reference? -%> - add_reference :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_options %> - <%- elsif attribute.token? -%> - add_column :<%= table_name %>, :<%= attribute.name %>, :string<%= attribute.inject_options %> - add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %>, unique: true - <%- else -%> - add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> - <%- if attribute.has_index? -%> - add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> - <%- end -%> - <%- end -%> -<%- end -%> - end -<%- elsif migration_action == 'join' -%> - def change - create_join_table :<%= join_tables.first %>, :<%= join_tables.second %> do |t| - <%- attributes.each do |attribute| -%> - <%- if attribute.reference? -%> - t.references :<%= attribute.name %><%= attribute.inject_options %> - <%- else -%> - <%= '# ' unless attribute.has_index? -%>t.index <%= attribute.index_name %><%= attribute.inject_index_options %> - <%- end -%> - <%- end -%> - end - end -<%- else -%> - def change -<% attributes.each do |attribute| -%> -<%- if migration_action -%> - <%- if attribute.reference? -%> - remove_reference :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_options %> - <%- else -%> - <%- if attribute.has_index? -%> - remove_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> - <%- end -%> - remove_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> - <%- end -%> -<%- end -%> -<%- end -%> - end -<%- end -%> -end diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt new file mode 100644 index 0000000000..481c70201b --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt @@ -0,0 +1,46 @@ +class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>] +<%- if migration_action == 'add' -%> + def change +<% attributes.each do |attribute| -%> + <%- if attribute.reference? -%> + add_reference :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_options %> + <%- elsif attribute.token? -%> + add_column :<%= table_name %>, :<%= attribute.name %>, :string<%= attribute.inject_options %> + add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %>, unique: true + <%- else -%> + add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> + <%- if attribute.has_index? -%> + add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> + <%- end -%> + <%- end -%> +<%- end -%> + end +<%- elsif migration_action == 'join' -%> + def change + create_join_table :<%= join_tables.first %>, :<%= join_tables.second %> do |t| + <%- attributes.each do |attribute| -%> + <%- if attribute.reference? -%> + t.references :<%= attribute.name %><%= attribute.inject_options %> + <%- else -%> + <%= '# ' unless attribute.has_index? -%>t.index <%= attribute.index_name %><%= attribute.inject_index_options %> + <%- end -%> + <%- end -%> + end + end +<%- else -%> + def change +<% attributes.each do |attribute| -%> +<%- if migration_action -%> + <%- if attribute.reference? -%> + remove_reference :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_options %> + <%- else -%> + <%- if attribute.has_index? -%> + remove_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> + <%- end -%> + remove_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> + <%- end -%> +<%- end -%> +<%- end -%> + end +<%- end -%> +end diff --git a/activerecord/lib/rails/generators/active_record/model/templates/model.rb b/activerecord/lib/rails/generators/active_record/model/templates/model.rb deleted file mode 100644 index 55dc65c8ad..0000000000 --- a/activerecord/lib/rails/generators/active_record/model/templates/model.rb +++ /dev/null @@ -1,13 +0,0 @@ -<% module_namespacing do -%> -class <%= class_name %> < <%= parent_class_name.classify %> -<% attributes.select(&:reference?).each do |attribute| -%> - belongs_to :<%= attribute.name %><%= ', polymorphic: true' if attribute.polymorphic? %><%= ', required: true' if attribute.required? %> -<% end -%> -<% attributes.select(&:token?).each do |attribute| -%> - has_secure_token<% if attribute.name != "token" %> :<%= attribute.name %><% end %> -<% end -%> -<% if attributes.any?(&:password_digest?) -%> - has_secure_password -<% end -%> -end -<% end -%> diff --git a/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt new file mode 100644 index 0000000000..55dc65c8ad --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt @@ -0,0 +1,13 @@ +<% module_namespacing do -%> +class <%= class_name %> < <%= parent_class_name.classify %> +<% attributes.select(&:reference?).each do |attribute| -%> + belongs_to :<%= attribute.name %><%= ', polymorphic: true' if attribute.polymorphic? %><%= ', required: true' if attribute.required? %> +<% end -%> +<% attributes.select(&:token?).each do |attribute| -%> + has_secure_token<% if attribute.name != "token" %> :<%= attribute.name %><% end %> +<% end -%> +<% if attributes.any?(&:password_digest?) -%> + has_secure_password +<% end -%> +end +<% end -%> diff --git a/activerecord/lib/rails/generators/active_record/model/templates/module.rb b/activerecord/lib/rails/generators/active_record/model/templates/module.rb deleted file mode 100644 index a3bf1c37b6..0000000000 --- a/activerecord/lib/rails/generators/active_record/model/templates/module.rb +++ /dev/null @@ -1,7 +0,0 @@ -<% module_namespacing do -%> -module <%= class_path.map(&:camelize).join('::') %> - def self.table_name_prefix - '<%= namespaced? ? namespaced_class_path.join('_') : class_path.join('_') %>_' - end -end -<% end -%> diff --git a/activerecord/lib/rails/generators/active_record/model/templates/module.rb.tt b/activerecord/lib/rails/generators/active_record/model/templates/module.rb.tt new file mode 100644 index 0000000000..a3bf1c37b6 --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/model/templates/module.rb.tt @@ -0,0 +1,7 @@ +<% module_namespacing do -%> +module <%= class_path.map(&:camelize).join('::') %> + def self.table_name_prefix + '<%= namespaced? ? namespaced_class_path.join('_') : class_path.join('_') %>_' + end +end +<% end -%> -- cgit v1.2.3 From 68fe6b08ee72cc47263e0d2c9ff07f75c4b42761 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 13 Nov 2017 13:24:28 -0700 Subject: Properly cast input in `update_all` The documentation claims that given values go through "normal AR type casting and serialization", which to me implies `serialize(cast(value))`, not just serialization. The docs were changed to use this wording in #22492. The tests I cited in that PR (which is the same test modified in this commit), is worded in a way that implies it should be using `cast` as well. It's possible that I originally meant "normal type casting" to imply just the call to `serialize`, but given that `update_all(archived: params['archived'])` seems to be pretty common, I'm inclined to make this change as long as no tests are broken from it. --- activerecord/lib/active_record/sanitization.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 1c3099f55c..90cc3373fb 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -110,7 +110,8 @@ module ActiveRecord def sanitize_sql_hash_for_assignment(attrs, table) # :doc: c = connection attrs.map do |attr, value| - value = type_for_attribute(attr.to_s).serialize(value) + type = type_for_attribute(attr.to_s) + value = type.serialize(type.cast(value)) "#{c.quote_table_name_for_assignment(table, attr)} = #{c.quote(value)}" end.join(", ") end -- cgit v1.2.3 From b6d5e46311d7ea59539c1f45c6ffb269eeb23912 Mon Sep 17 00:00:00 2001 From: Yuji Yaginuma Date: Tue, 14 Nov 2017 13:54:58 +0900 Subject: Add `environment` as dependency of `load_config` (#31135) Currently the environment is not loaded in some db tasks. Therefore, if use encrypted secrets values in `database.yml`, `read_encrypted_secrets` will not be true, so the value can not be used correctly. To fix this, added `environment` as dependency of `load_config`. It also removes explicit `environment` dependencies that are no longer needed. Fixes #30717 --- .../lib/active_record/railties/databases.rake | 48 +++++++++++----------- 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 3bca2982e0..fce3e1c5cf 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -4,16 +4,16 @@ require "active_record" db_namespace = namespace :db do desc "Set the environment value for the database" - task "environment:set" => [:environment, :load_config] do + task "environment:set" => :load_config do ActiveRecord::InternalMetadata.create_table ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment end - task check_protected_environments: [:environment, :load_config] do + task check_protected_environments: :load_config do ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! end - task :load_config do + task load_config: :environment do ActiveRecord::Base.configurations = ActiveRecord::Tasks::DatabaseTasks.database_configuration || {} ActiveRecord::Migrator.migrations_paths = ActiveRecord::Tasks::DatabaseTasks.migrations_paths end @@ -56,7 +56,7 @@ db_namespace = namespace :db do end desc "Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog)." - task migrate: [:environment, :load_config] do + task migrate: :load_config do ActiveRecord::Tasks::DatabaseTasks.migrate db_namespace["_dump"].invoke end @@ -78,7 +78,7 @@ db_namespace = namespace :db do namespace :migrate do # desc 'Rollbacks the database one migration and re migrate up (options: STEP=x, VERSION=x).' - task redo: [:environment, :load_config] do + task redo: :load_config do raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty? if ENV["VERSION"] @@ -94,7 +94,7 @@ db_namespace = namespace :db do task reset: ["db:drop", "db:create", "db:migrate"] # desc 'Runs the "up" for a given migration VERSION.' - task up: [:environment, :load_config] do + task up: :load_config do raise "VERSION is required" if !ENV["VERSION"] || ENV["VERSION"].empty? ActiveRecord::Tasks::DatabaseTasks.check_target_version @@ -108,7 +108,7 @@ db_namespace = namespace :db do end # desc 'Runs the "down" for a given migration VERSION.' - task down: [:environment, :load_config] do + task down: :load_config do raise "VERSION is required - To go down one migration, use db:rollback" if !ENV["VERSION"] || ENV["VERSION"].empty? ActiveRecord::Tasks::DatabaseTasks.check_target_version @@ -122,7 +122,7 @@ db_namespace = namespace :db do end desc "Display status of migrations" - task status: [:environment, :load_config] do + task status: :load_config do unless ActiveRecord::SchemaMigration.table_exists? abort "Schema migrations table does not exist yet." end @@ -140,14 +140,14 @@ db_namespace = namespace :db do end desc "Rolls the schema back to the previous version (specify steps w/ STEP=n)." - task rollback: [:environment, :load_config] do + task rollback: :load_config do step = ENV["STEP"] ? ENV["STEP"].to_i : 1 ActiveRecord::Migrator.rollback(ActiveRecord::Tasks::DatabaseTasks.migrations_paths, step) db_namespace["_dump"].invoke end # desc 'Pushes the schema to the next version (specify steps w/ STEP=n).' - task forward: [:environment, :load_config] do + task forward: :load_config do step = ENV["STEP"] ? ENV["STEP"].to_i : 1 ActiveRecord::Migrator.forward(ActiveRecord::Tasks::DatabaseTasks.migrations_paths, step) db_namespace["_dump"].invoke @@ -157,12 +157,12 @@ db_namespace = namespace :db do task reset: [ "db:drop", "db:setup" ] # desc "Retrieves the charset for the current environment's database" - task charset: [:environment, :load_config] do + task charset: :load_config do puts ActiveRecord::Tasks::DatabaseTasks.charset_current end # desc "Retrieves the collation for the current environment's database" - task collation: [:environment, :load_config] do + task collation: :load_config do begin puts ActiveRecord::Tasks::DatabaseTasks.collation_current rescue NoMethodError @@ -171,12 +171,12 @@ db_namespace = namespace :db do end desc "Retrieves the current schema version number" - task version: [:environment, :load_config] do + task version: :load_config do puts "Current version: #{ActiveRecord::Migrator.current_version}" end # desc "Raises an error if there are pending migrations" - task abort_if_pending_migrations: [:environment, :load_config] do + task abort_if_pending_migrations: :load_config do pending_migrations = ActiveRecord::Migrator.open(ActiveRecord::Tasks::DatabaseTasks.migrations_paths).pending_migrations if pending_migrations.any? @@ -199,7 +199,7 @@ db_namespace = namespace :db do namespace :fixtures do desc "Loads fixtures into the current environment's database. Load specific fixtures using FIXTURES=x,y. Load from subdirectory in test/fixtures using FIXTURES_DIR=z. Specify an alternative path (eg. spec/fixtures) using FIXTURES_PATH=spec/fixtures." - task load: [:environment, :load_config] do + task load: :load_config do require "active_record/fixtures" base_dir = ActiveRecord::Tasks::DatabaseTasks.fixtures_path @@ -221,7 +221,7 @@ db_namespace = namespace :db do end # desc "Search for a fixture given a LABEL or ID. Specify an alternative path (eg. spec/fixtures) using FIXTURES_PATH=spec/fixtures." - task identify: [:environment, :load_config] do + task identify: :load_config do require "active_record/fixtures" label, id = ENV["LABEL"], ENV["ID"] @@ -247,7 +247,7 @@ db_namespace = namespace :db do namespace :schema do desc "Creates a db/schema.rb file that is portable against any DB supported by Active Record" - task dump: [:environment, :load_config] do + task dump: :load_config do require "active_record/schema_dumper" filename = ENV["SCHEMA"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "schema.rb") File.open(filename, "w:utf-8") do |file| @@ -257,7 +257,7 @@ db_namespace = namespace :db do end desc "Loads a schema.rb file into the database" - task load: [:environment, :load_config, :check_protected_environments] do + task load: [:load_config, :check_protected_environments] do ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:ruby, ENV["SCHEMA"]) end @@ -267,14 +267,14 @@ db_namespace = namespace :db do namespace :cache do desc "Creates a db/schema_cache.yml file." - task dump: [:environment, :load_config] do + task dump: :load_config do conn = ActiveRecord::Base.connection filename = File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "schema_cache.yml") ActiveRecord::Tasks::DatabaseTasks.dump_schema_cache(conn, filename) end desc "Clears a db/schema_cache.yml file." - task clear: [:environment, :load_config] do + task clear: :load_config do filename = File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "schema_cache.yml") rm_f filename, verbose: false end @@ -284,7 +284,7 @@ db_namespace = namespace :db do namespace :structure do desc "Dumps the database structure to db/structure.sql. Specify another file with SCHEMA=db/my_structure.sql" - task dump: [:environment, :load_config] do + task dump: :load_config do filename = ENV["SCHEMA"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, "structure.sql") current_config = ActiveRecord::Tasks::DatabaseTasks.current_config ActiveRecord::Tasks::DatabaseTasks.structure_dump(current_config, filename) @@ -299,7 +299,7 @@ db_namespace = namespace :db do end desc "Recreates the databases from the structure.sql file" - task load: [:environment, :load_config, :check_protected_environments] do + task load: [:load_config, :check_protected_environments] do ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:sql, ENV["SCHEMA"]) end @@ -338,12 +338,12 @@ db_namespace = namespace :db do end # desc "Empty the test database" - task purge: %w(environment load_config check_protected_environments) do + task purge: %w(load_config check_protected_environments) do ActiveRecord::Tasks::DatabaseTasks.purge ActiveRecord::Base.configurations["test"] end # desc 'Load the test schema' - task prepare: %w(environment load_config) do + task prepare: :load_config do unless ActiveRecord::Base.configurations.blank? db_namespace["test:load"].invoke end -- cgit v1.2.3 From 67dbfc69f690f231d5b8257e03b8338af19c1d05 Mon Sep 17 00:00:00 2001 From: Nikolai B Date: Tue, 14 Nov 2017 14:12:40 +0000 Subject: Update `exists?` documentation Make it clear that `exists?` can be chained onto a relation --- activerecord/lib/active_record/relation/finder_methods.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 18566b5662..706fd57704 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -284,7 +284,7 @@ module ActiveRecord # * Hash - Finds the record that matches these +find+-style conditions # (such as {name: 'David'}). # * +false+ - Returns always +false+. - # * No args - Returns +false+ if the table is empty, +true+ otherwise. + # * No args - Returns +false+ if the relation is empty, +true+ otherwise. # # For more information about specifying conditions as a hash or array, # see the Conditions section in the introduction to ActiveRecord::Base. @@ -300,6 +300,7 @@ module ActiveRecord # Person.exists?(name: 'David') # Person.exists?(false) # Person.exists? + # Person.where(name: 'Spartacus', rating: 4).exists? def exists?(conditions = :none) if Base === conditions raise ArgumentError, <<-MSG.squish -- cgit v1.2.3 From df82237a45e930a7ab53b95bee78a3f34c5b92fb Mon Sep 17 00:00:00 2001 From: Rich Date: Tue, 14 Nov 2017 19:24:00 +0000 Subject: Add a #populate method to migrations (#31082) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add a #populate method to migrations * Address rubocop issues * Rename to #up_only and use #execute in the examples intead of the model * Update CHANGELOG [Rich Daley & Rafael Mendonça França] --- activerecord/lib/active_record/migration.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index c13efa9d70..67c8c9fc08 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -734,6 +734,24 @@ module ActiveRecord execute_block { yield helper } end + # Used to specify an operation that is only run when migrating up + # (for example, populating a new column with its initial values). + # + # In the following example, the new column `published` will be given + # the value `true` for all existing records. + # + # class AddPublishedToPosts < ActiveRecord::Migration[5.3] + # def change + # add_column :posts, :published, :boolean, default: false + # up_only do + # execute "update posts set published = 'true'" + # end + # end + # end + def up_only + execute_block { yield } unless reverting? + end + # Runs the given migration classes. # Last argument can specify options: # - :direction (default is :up) -- cgit v1.2.3 From 46a2f93614ccf0d1628e6fc3c4666cee476d17c8 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 14 Nov 2017 20:02:52 +0000 Subject: Fix migration version in doc of #up_only --- activerecord/lib/active_record/migration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 67c8c9fc08..360bf25a8c 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -740,7 +740,7 @@ module ActiveRecord # In the following example, the new column `published` will be given # the value `true` for all existing records. # - # class AddPublishedToPosts < ActiveRecord::Migration[5.3] + # class AddPublishedToPosts < ActiveRecord::Migration[5.2] # def change # add_column :posts, :published, :boolean, default: false # up_only do -- cgit v1.2.3 From bbae710a405ce92074c1666dcf859196c29e2ed2 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 17 Nov 2017 03:12:57 +0900 Subject: Avoid creating extra `relation` and `build_arel` in `_create_record` and `_update_record` (#29999) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently `_create_record` and `_update_record` in `Persistence` are creating extra `unscoped` and calling `build_arel` in the relation. But `compile_insert` and `compile_update` can be done without those expensive operation for `SelectManager` creation. So I moved the implementation to `Persistence` to avoid creating extra relation and refactored to avoid calling `build_arel`. https://gist.github.com/kamipo/8ed73d760112cfa5f6263c9413633419 Before: ``` Warming up -------------------------------------- _update_record 150.000 i/100ms Calculating ------------------------------------- _update_record 1.548k (±12.3%) i/s - 7.650k in 5.042603s ``` After: ``` Warming up -------------------------------------- _update_record 201.000 i/100ms Calculating ------------------------------------- _update_record 2.002k (±12.8%) i/s - 9.849k in 5.027681s ``` 30% faster for STI classes. --- activerecord/lib/active_record/persistence.rb | 43 ++++++++++++++++++- activerecord/lib/active_record/relation.rb | 61 --------------------------- 2 files changed, 41 insertions(+), 63 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index a57c60ffac..4e1b05dbf6 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -165,6 +165,38 @@ module ActiveRecord where(primary_key => id_or_array).delete_all end + def _insert_record(values) # :nodoc: + primary_key_value = nil + + if primary_key && Hash === values + arel_primary_key = arel_attribute(primary_key) + primary_key_value = values[arel_primary_key] + + if !primary_key_value && prefetch_primary_key? + primary_key_value = next_sequence_value + values[arel_primary_key] = primary_key_value + end + end + + if values.empty? + im = arel_table.compile_insert(connection.empty_insert_statement_value) + im.into arel_table + else + im = arel_table.compile_insert(_substitute_values(values)) + end + + connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) + end + + def _update_record(values, id, id_was) # :nodoc: + bind = predicate_builder.build_bind_attribute(primary_key, id_was || id) + um = arel_table.where( + arel_attribute(primary_key).eq(bind) + ).compile_update(_substitute_values(values), primary_key) + + connection.update(um, "#{self} Update") + end + private # Called by +instantiate+ to decide which class to use for a new # record instance. @@ -174,6 +206,13 @@ module ActiveRecord def discriminate_class_for_record(record) self end + + def _substitute_values(values) + values.map do |attr, value| + bind = predicate_builder.build_bind_attribute(attr.name, value) + [attr, bind] + end + end end # Returns true if this object hasn't been saved yet -- that is, a record @@ -671,7 +710,7 @@ module ActiveRecord rows_affected = 0 @_trigger_update_callback = true else - rows_affected = self.class.unscoped._update_record attributes_values, id, id_in_database + rows_affected = self.class._update_record(attributes_values, id, id_in_database) @_trigger_update_callback = rows_affected > 0 end @@ -685,7 +724,7 @@ module ActiveRecord def _create_record(attribute_names = self.attribute_names) attributes_values = arel_attributes_with_values_for_create(attribute_names) - new_id = self.class.unscoped.insert attributes_values + new_id = self.class._insert_record(attributes_values) self.id ||= new_id if self.class.primary_key @new_record = false diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index e2d2f45503..081ef5771f 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -36,67 +36,6 @@ module ActiveRecord reset end - def insert(values) # :nodoc: - primary_key_value = nil - - if primary_key && Hash === values - primary_key_value = values[values.keys.find { |k| - k.name == primary_key - }] - - if !primary_key_value && klass.prefetch_primary_key? - primary_key_value = klass.next_sequence_value - values[arel_attribute(klass.primary_key)] = primary_key_value - end - end - - im = arel.create_insert - im.into @table - - substitutes = substitute_values values - - if values.empty? # empty insert - im.values = Arel.sql(connection.empty_insert_statement_value) - else - im.insert substitutes - end - - @klass.connection.insert( - im, - "#{@klass} Create", - primary_key || false, - primary_key_value, - nil, - ) - end - - def _update_record(values, id, id_was) # :nodoc: - substitutes = substitute_values values - - scope = @klass.unscoped - - if @klass.finder_needs_type_condition? - scope.unscope!(where: @klass.inheritance_column) - end - - relation = scope.where(@klass.primary_key => (id_was || id)) - um = relation - .arel - .compile_update(substitutes, @klass.primary_key) - - @klass.connection.update( - um, - "#{@klass} Update", - ) - end - - def substitute_values(values) # :nodoc: - values.map do |arel_attr, value| - bind = predicate_builder.build_bind_attribute(arel_attr.name, value) - [arel_attr, bind] - end - end - def arel_attribute(name) # :nodoc: klass.arel_attribute(name, table) end -- cgit v1.2.3 From eeaf9cf61c3cd14929583878785c31dab79e2196 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 20 Nov 2017 06:24:50 +0900 Subject: Prevent extra `spawn` to make `klass.all` faster (#29009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These extra `spawn` are called via `klass.all` and `klass.all` is called everywhere in the internal. Avoiding the extra `spawn` makes` klass.all` 30% faster for STI classes. https://gist.github.com/kamipo/684d03817a8115848cec8e8b079560b7 ``` Warming up -------------------------------------- fast relation 4.410k i/100ms slow relation 3.334k i/100ms Calculating ------------------------------------- fast relation 47.373k (± 5.2%) i/s - 238.140k in 5.041836s slow relation 35.757k (±15.9%) i/s - 176.702k in 5.104625s Comparison: fast relation: 47373.2 i/s slow relation: 35756.7 i/s - 1.32x slower ``` --- activerecord/lib/active_record/core.rb | 3 ++- activerecord/lib/active_record/scoping/default.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index b97b14644e..481159e501 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -277,7 +277,8 @@ module ActiveRecord relation = Relation.create(self, arel_table, predicate_builder) if finder_needs_type_condition? && !ignore_default_scope? - relation.where(type_condition).create_with(inheritance_column.to_s => sti_name) + relation.where!(type_condition) + relation.create_with!(inheritance_column.to_s => sti_name) else relation end diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 86ae374318..8c612df27a 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -111,7 +111,7 @@ module ActiveRecord # The user has defined their own default scope method, so call that evaluate_default_scope do if scope = default_scope - (base_rel ||= relation).merge(scope) + (base_rel ||= relation).merge!(scope) end end elsif default_scopes.any? @@ -119,7 +119,7 @@ module ActiveRecord evaluate_default_scope do default_scopes.inject(base_rel) do |default_scope, scope| scope = scope.respond_to?(:to_proc) ? scope : scope.method(:call) - default_scope.merge(base_rel.instance_exec(&scope)) + default_scope.merge!(base_rel.instance_exec(&scope)) end end end -- cgit v1.2.3