diff options
Diffstat (limited to 'activerecord')
62 files changed, 659 insertions, 412 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 39fe217777..3bb4b5236e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,37 @@ +* Fix an issue when preloading associations with extensions. + Previously every association with extension methods was transformed into an + instance dependent scope. This is no longer the case. + + Fixes #23934. + + *Yves Senn* + +* Deprecate `{insert|update|delete}_sql` in `DatabaseStatements`. + Use the `{insert|update|delete}` public methods instead. + + *Ryuta Kamizono* + +* Added a configuration option to have active record raise an ArgumentError + if the order or limit is ignored in a batch query, rather than logging a + warning message. + + *Scott Ringwelski* + +* Honour the order of the joining model in a `has_many :through` association when eager loading. + + Example: + + The below will now follow the order of `by_lines` when eager loading `authors`. + + class Article < ActiveRecord::Base + has_many :by_lines, -> { order(:position) } + has_many :authors, through: :by_lines + end + + Fixes #17864. + + *Yasyf Mohamedali*, *Joel Turkel* + * Ensure that the Suppressor runs before validations. This moves the suppressor up to be run before validations rather than after @@ -60,7 +94,6 @@ *Bogdan Gusiev* - * Allow `joins` to be unscoped. Fixes #13775. diff --git a/activerecord/RUNNING_UNIT_TESTS.rdoc b/activerecord/RUNNING_UNIT_TESTS.rdoc index a74fcf2df7..cd22f76d01 100644 --- a/activerecord/RUNNING_UNIT_TESTS.rdoc +++ b/activerecord/RUNNING_UNIT_TESTS.rdoc @@ -7,11 +7,11 @@ http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#setting-up- To run a specific test: - $ ruby -Itest test/cases/base_test.rb -n method_name + $ bundle exec ruby -Itest test/cases/base_test.rb -n method_name To run a set of tests: - $ ruby -Itest test/cases/base_test.rb + $ bundle exec ruby -Itest test/cases/base_test.rb You can also run tests that depend upon a specific database backend. For example: @@ -41,7 +41,7 @@ parameters in +test/config.example.yml+ are. You can override the +connections:+ parameter in either file using the +ARCONN+ (Active Record CONNection) environment variable: - $ ARCONN=postgresql ruby -Itest test/cases/base_test.rb + $ ARCONN=postgresql bundle exec ruby -Itest test/cases/base_test.rb You can specify a custom location for the config file using the +ARCONFIG+ environment variable. diff --git a/activerecord/examples/performance.rb b/activerecord/examples/performance.rb index a5a1f284a0..c3c46c19c5 100644 --- a/activerecord/examples/performance.rb +++ b/activerecord/examples/performance.rb @@ -1,4 +1,3 @@ -require File.expand_path('../../../load_paths', __FILE__) require "active_record" require 'benchmark/ips' diff --git a/activerecord/examples/simple.rb b/activerecord/examples/simple.rb index 4ed5d80eb2..5a041e8417 100644 --- a/activerecord/examples/simple.rb +++ b/activerecord/examples/simple.rb @@ -1,4 +1,3 @@ -require File.expand_path('../../../load_paths', __FILE__) require 'active_record' class Person < ActiveRecord::Base diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index ab3846ae65..baa497dc98 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -137,7 +137,6 @@ module ActiveRecord eager_autoload do autoload :AbstractAdapter - autoload :ConnectionManagement, "active_record/connection_adapters/abstract/connection_pool" end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index d64ab64c99..f7edfbfb5f 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -257,7 +257,7 @@ module ActiveRecord # Returns true if statement cache should be skipped on the association reader. def skip_statement_cache? - reflection.scope_chain.any?(&:any?) || + reflection.has_scope? || scope.eager_loading? || klass.scope_attributes? || reflection.source_reflection.active_record.default_scopes.any? diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 56a8dc4e18..f25bd7ca9f 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -70,7 +70,11 @@ module ActiveRecord::Associations::Builder # :nodoc: def self.wrap_scope(scope, mod) if scope - proc { |owner| instance_exec(owner, &scope).extending(mod) } + if scope.arity > 0 + proc { |owner| instance_exec(owner, &scope).extending(mod) } + else + proc { instance_exec(&scope).extending(mod) } + end else proc { extending(mod) } end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 6c83058202..b0203909ce 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -38,12 +38,7 @@ module ActiveRecord } end - record_offset = {} - @preloaded_records.each_with_index do |record,i| - record_offset[record] = i - end - - through_records.each_with_object({}) { |(lhs,center),records_by_owner| + through_records.each_with_object({}) do |(lhs,center), records_by_owner| pl_to_middle = center.group_by { |record| middle_to_pl[record] } records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| @@ -53,13 +48,25 @@ module ActiveRecord target_records_from_association(association) }.compact - rhs_records.sort_by { |rhs| record_offset[rhs] } + # Respect the order on `reflection_scope` if it exists, else use the natural order. + if reflection_scope.values[:order].present? + @id_map ||= id_to_index_map @preloaded_records + rhs_records.sort_by { |rhs| @id_map[rhs] } + else + rhs_records + end end - } + end end private + def id_to_index_map(ids) + id_map = {} + ids.each_with_index { |id, index| id_map[id] = index } + id_map + end + def reset_association(owners, association_name) should_reset = (through_scope != through_reflection.klass.unscoped) || (reflection.options[:source_type] && through_reflection.collection?) diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 5d0405c3be..e0ceafc617 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -119,7 +119,7 @@ module ActiveRecord # # class MoneyType < ActiveRecord::Type::Integer # def cast(value) - # if !value.kind_of(Numeric) && value.include?('$') + # if !value.kind_of?(Numeric) && value.include?('$') # price_in_dollars = value.gsub(/\$/, '').to_f # super(price_in_dollars * 100) # else @@ -154,7 +154,7 @@ module ActiveRecord # end # # class MoneyType < Type::Value - # def initialize(currency_converter) + # def initialize(currency_converter:) # @currency_converter = currency_converter # end # @@ -171,7 +171,7 @@ module ActiveRecord # # class Product < ActiveRecord::Base # currency_converter = ConversionRatesFromTheInternet.new - # attribute :price_in_bitcoins, :money, currency_converter + # attribute :price_in_bitcoins, :money, currency_converter: currency_converter # end # # Product.where(price_in_bitcoins: Money.new(5, "USD")) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index bac5a38a5d..06c7482bf9 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -22,7 +22,7 @@ module ActiveRecord # # == Validation # - # Children records are validated unless <tt>:validate</tt> is +false+. + # Child records are validated unless <tt>:validate</tt> is +false+. # # == Callbacks # 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 ccd2899489..e389d818fd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -951,24 +951,5 @@ module ActiveRecord owner_to_pool && owner_to_pool[owner.name] end end - - class ConnectionManagement - def initialize(app) - @app = app - end - - def call(env) - testing = env['rack.test'] - - status, headers, body = @app.call(env) - proxy = ::Rack::BodyProxy.new(body) do - ActiveRecord::Base.clear_active_connections! unless testing - end - [status, headers, proxy] - rescue Exception - ActiveRecord::Base.clear_active_connections! unless testing - raise - end - end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index aa5ae15285..824040775d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -125,18 +125,21 @@ module ActiveRecord end alias create insert alias insert_sql insert + deprecate insert_sql: :insert # Executes the update statement and returns the number of rows affected. def update(arel, name = nil, binds = []) exec_update(to_sql(arel, binds), name, binds) end alias update_sql update + deprecate update_sql: :update # Executes the delete statement and returns the number of rows affected. def delete(arel, name = nil, binds = []) exec_delete(to_sql(arel, binds), name, binds) end alias delete_sql delete + deprecate delete_sql: :delete # Returns +true+ when the connection adapter supports prepared statement # caching, otherwise returns +false+ diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 33dbab41cb..0bdfd4f900 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -65,7 +65,7 @@ module ActiveRecord if @query_cache_enabled && !locked?(arel) arel, binds = binds_from_relation arel, binds sql = to_sql(arel, binds) - cache_sql(sql, binds) { super(sql, name, binds, preparable: visitor.preparable) } + cache_sql(sql, binds) { super(sql, name, binds, preparable: preparable) } else super end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index b1b6044e72..2209874d0a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -7,15 +7,16 @@ module ActiveRecord # Adapter level by over-writing this code inside the database specific adapters module ColumnDumper def column_spec(column) - spec = prepare_column_options(column) - (spec.keys - [:name, :type]).each{ |k| spec[k].insert(0, "#{k}: ")} + spec = Hash[prepare_column_options(column).map { |k, v| [k, "#{k}: #{v}"] }] + spec[:name] = column.name.inspect + spec[:type] = schema_type(column).to_s spec end def column_spec_for_primary_key(column) return if column.type == :integer spec = { id: schema_type(column).inspect } - spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type].include?(key) }) + spec.merge!(prepare_column_options(column)) end # This can be overridden on an Adapter level basis to support other @@ -23,9 +24,6 @@ module ActiveRecord # PostgreSQL::ColumnDumper) def prepare_column_options(column) spec = {} - spec[:name] = column.name.inspect - spec[:type] = schema_type(column).to_s - spec[:null] = 'false' unless column.null if limit = schema_limit(column) spec[:limit] = limit @@ -42,6 +40,8 @@ module ActiveRecord default = schema_default(column) if column.has_default? spec[:default] = default unless default.nil? + spec[:null] = 'false' unless column.null + if collation = schema_collation(column) spec[:collation] = collation end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 6ecdab6eb0..ca795cb1ad 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -188,7 +188,10 @@ module ActiveRecord transaction = begin_transaction options yield rescue Exception => error - rollback_transaction if transaction + if transaction + rollback_transaction + after_failure_actions(transaction, error) + end raise ensure unless error @@ -214,7 +217,16 @@ module ActiveRecord end private + NULL_TRANSACTION = NullTransaction.new + + # Deallocate invalidated prepared statements outside of the transaction + def after_failure_actions(transaction, error) + return unless transaction.is_a?(RealTransaction) + return unless error.is_a?(ActiveRecord::PreparedStatementCacheExpired) + @connection.clear_cache! + end + end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 5ef434734a..fcc1ef9d5f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -27,7 +27,6 @@ module ActiveRecord autoload_at 'active_record/connection_adapters/abstract/connection_pool' do autoload :ConnectionHandler - autoload :ConnectionManagement end autoload_under 'abstract' do 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 b12bac2737..50f461b746 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -668,7 +668,7 @@ module ActiveRecord register_integer_type m, %r(^smallint)i, limit: 2 register_integer_type m, %r(^tinyint)i, limit: 1 - m.alias_type %r(tinyint\(1\))i, 'boolean' if emulate_booleans + m.register_type %r(^tinyint\(1\))i, Type::Boolean.new if emulate_booleans m.alias_type %r(year)i, 'integer' m.alias_type %r(bit)i, 'binary' diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index ccf5b6cadc..914ea98f79 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -13,7 +13,7 @@ module ActiveRecord return if spec.empty? else spec[:id] = schema_type(column).inspect - spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type, :null].include?(key) }) + spec.merge!(prepare_column_options(column).delete_if { |key, _| key == :null }) end spec end @@ -38,10 +38,6 @@ module ActiveRecord end end - def schema_limit(column) - super unless column.type == :boolean - end - def schema_precision(column) super unless /time/ === column.sql_type && column.precision == 0 end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 57d8867bb4..e7541748de 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -16,7 +16,7 @@ module ActiveRecord if config[:flags].kind_of? Array config[:flags].push "FOUND_ROWS".freeze else - config[:flags] |= Mysql2::Client::FOUND_ROWS + config[:flags] |= Mysql2::Client::FOUND_ROWS end end @@ -131,11 +131,7 @@ module ActiveRecord def exec_query(sql, name = 'SQL', binds = [], prepare: false) result = execute(sql, name) @connection.next_result while @connection.more_results? - ActiveRecord::Result.new(result.fields, result.to_a) - end - - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) - execute to_sql(sql, binds), name + ActiveRecord::Result.new(result.fields, result.to_a) if result end def exec_delete(sql, name, binds) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb index b82bdb8b0c..19761618cf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb @@ -12,7 +12,7 @@ module ActiveRecord spec[:default] = schema_default(column) || 'nil' else spec[:id] = schema_type(column).inspect - spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type, :null].include?(key) }) + spec.merge!(prepare_column_options(column).delete_if { |key, _| key == :null }) end spec end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index beaeef3c78..61c9628de3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -598,25 +598,41 @@ module ActiveRecord @connection.exec_prepared(stmt_key, type_casted_binds) end rescue ActiveRecord::StatementInvalid => e - pgerror = e.cause + raise unless is_cached_plan_failure?(e) - # Get the PG code for the failure. Annoyingly, the code for - # prepared statements whose return value may have changed is - # FEATURE_NOT_SUPPORTED. Check here for more details: - # http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/cache/plancache.c#l573 - begin - code = pgerror.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) - rescue - raise e - end - if FEATURE_NOT_SUPPORTED == code + # Nothing we can do if we are in a transaction because all commands + # will raise InFailedSQLTransaction + if in_transaction? + raise ActiveRecord::PreparedStatementCacheExpired.new(e.cause.message) + else + # outside of transactions we can simply flush this query and retry @statements.delete sql_key(sql) retry - else - raise e end end + # Annoyingly, the code for prepared statements whose return value may + # have changed is FEATURE_NOT_SUPPORTED. + # + # This covers various different error types so we need to do additional + # work to classify the exception definitively as a + # ActiveRecord::PreparedStatementCacheExpired + # + # Check here for more details: + # http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/cache/plancache.c#l573 + CACHED_PLAN_HEURISTIC = 'cached plan must not change result type'.freeze + def is_cached_plan_failure?(e) + pgerror = e.cause + code = pgerror.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) + code == FEATURE_NOT_SUPPORTED && pgerror.message.include?(CACHED_PLAN_HEURISTIC) + rescue + false + end + + def in_transaction? + open_transactions > 0 + end + # Returns the statement identifier for the client side cache # of statements def sql_key(sql) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 24fd0aaecf..86ec8000fb 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -72,6 +72,14 @@ module ActiveRecord ## # :singleton-method: + # Specifies if an error should be raised on query limit or order being + # ignored when doing batch queries. Useful in applications where the + # limit or scope being ignored is error-worthy, rather than a warning. + mattr_accessor :error_on_ignored_order_or_limit, instance_writer: false + self.error_on_ignored_order_or_limit = false + + ## + # :singleton-method: # Specify whether or not to use timestamps for migration versions mattr_accessor :timestamped_migrations, instance_writer: false self.timestamped_migrations = true diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 903c63a7db..7be332fb97 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -152,7 +152,7 @@ module ActiveRecord enum_values = ActiveSupport::HashWithIndifferentAccess.new name = name.to_sym - # def self.statuses statuses end + # def self.statuses() statuses end detect_enum_conflict!(name, name.to_s.pluralize, true) klass.singleton_class.send(:define_method, name.to_s.pluralize) { enum_values } diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 87f32c042c..2ec9bf3d67 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -139,6 +139,11 @@ module ActiveRecord class NoDatabaseError < StatementInvalid end + # Raised when Postgres returns 'cached plan must not change result type' and + # we cannot retry gracefully (e.g. inside a transaction) + class PreparedStatementCacheExpired < StatementInvalid + end + # Raised on attempt to save stale record. Record is stale when it's being saved in another query after # instantiation, for example, when two users edit the same wiki page and one starts editing and saves # the page before the other. diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index b63caa4473..efa2a4df02 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -67,7 +67,7 @@ module ActiveRecord case sql when /\A\s*rollback/mi RED - when /\s*.*?select .*for update/mi, /\A\s*lock/mi + when /select .*for update/mi, /\A\s*lock/mi WHITE when /\A\s*select/i BLUE diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index ae78ceee01..fe68869143 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -215,7 +215,7 @@ module ActiveRecord # # The keys of the hash which is the value for +:posts_attributes+ are # ignored in this case. - # However, it is not allowed to use +'id'+ or +:id+ for one of + # However, it is not allowed to use <tt>'id'</tt> or <tt>:id</tt> for one of # such keys, otherwise the hash will be wrapped in an array and # interpreted as an attribute hash for a single post. # diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index d9a394fb71..afed5e5e85 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -61,7 +61,7 @@ module ActiveRecord # +instantiate+ instead of +new+, finder methods ensure they get new # instances of the appropriate class for each record. # - # See +ActiveRecord::Inheritance#discriminate_class_for_record+ to see + # See <tt>ActiveRecord::Inheritance#discriminate_class_for_record</tt> to see # how this "single-table" inheritance mapping is implemented. def instantiate(attributes, column_types = {}) klass = discriminate_class_for_record(attributes) diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index dcb2bd3d84..f451ed1764 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -23,34 +23,26 @@ module ActiveRecord end end - def initialize(app) - @app = app - end - - def call(env) - connection = ActiveRecord::Base.connection - enabled = connection.query_cache_enabled - connection_id = ActiveRecord::Base.connection_id - connection.enable_query_cache! - - response = @app.call(env) - response[2] = Rack::BodyProxy.new(response[2]) do - restore_query_cache_settings(connection_id, enabled) + def self.install_executor_hooks(executor = ActiveSupport::Executor) + executor.to_run do + connection = ActiveRecord::Base.connection + enabled = connection.query_cache_enabled + connection_id = ActiveRecord::Base.connection_id + connection.enable_query_cache! + + @restore_query_cache_settings = lambda do + ActiveRecord::Base.connection_id = connection_id + ActiveRecord::Base.connection.clear_query_cache + ActiveRecord::Base.connection.disable_query_cache! unless enabled + end end - response - rescue Exception => e - restore_query_cache_settings(connection_id, enabled) - raise e - end - - private + executor.to_complete do + @restore_query_cache_settings.call if defined?(@restore_query_cache_settings) - def restore_query_cache_settings(connection_id, enabled) - ActiveRecord::Base.connection_id = connection_id - ActiveRecord::Base.connection.clear_query_cache - ActiveRecord::Base.connection.disable_query_cache! unless enabled + # FIXME: This should be skipped when env['rack.test'] + ActiveRecord::Base.clear_active_connections! + end end - end end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index f4200e96b7..4c074c93ed 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -16,12 +16,6 @@ module ActiveRecord config.app_generators.orm :active_record, :migration => true, :timestamps => true - config.app_middleware.insert_after ::ActionDispatch::Callbacks, - ActiveRecord::QueryCache - - config.app_middleware.insert_after ::ActionDispatch::Callbacks, - ActiveRecord::ConnectionAdapters::ConnectionManagement - config.action_dispatch.rescue_responses.merge!( 'ActiveRecord::RecordNotFound' => :not_found, 'ActiveRecord::StaleObjectError' => :conflict, @@ -153,11 +147,9 @@ end_warning end end - initializer "active_record.set_reloader_hooks" do |app| - hook = app.config.reload_classes_only_on_change ? :to_prepare : :to_cleanup - + initializer "active_record.set_reloader_hooks" do ActiveSupport.on_load(:active_record) do - ActionDispatch::Reloader.send(hook) do + ActiveSupport::Reloader.before_class_unload do if ActiveRecord::Base.connected? ActiveRecord::Base.clear_cache! ActiveRecord::Base.clear_reloadable_connections! @@ -166,6 +158,12 @@ end_warning end end + initializer "active_record.set_executor_hooks" do + ActiveSupport.on_load(:active_record) do + ActiveRecord::QueryCache.install_executor_hooks + end + end + initializer "active_record.add_watchable_files" do |app| path = app.paths["db"].first config.watchable_files.concat ["#{path}/schema.rb", "#{path}/structure.sql"] diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 956fe7c51e..f8dffce2f1 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -138,6 +138,10 @@ module ActiveRecord # PolymorphicReflection # RuntimeReflection class AbstractReflection # :nodoc: + def through_reflection? + false + end + def table_name klass.table_name end @@ -445,6 +449,10 @@ module ActiveRecord scope ? [[scope]] : [[]] end + def has_scope? + scope + end + def has_inverse? inverse_name end @@ -700,6 +708,10 @@ module ActiveRecord @source_reflection_name = delegate_reflection.options[:source] end + def through_reflection? + true + end + def klass @klass ||= delegate_reflection.compute_class(class_name) end @@ -765,7 +777,6 @@ module ActiveRecord # This is for clearing cache on the reflection. Useful for tests that need to compare # SQL queries on associations. def clear_association_scope_cache # :nodoc: - @chain = nil delegate_reflection.clear_association_scope_cache source_reflection.clear_association_scope_cache through_reflection.clear_association_scope_cache @@ -812,13 +823,19 @@ module ActiveRecord end end + def has_scope? + scope || options[:source_type] || + source_reflection.has_scope? || + through_reflection.has_scope? + end + def join_keys(association_klass) source_reflection.join_keys(association_klass) end # A through association is nested if there would be more than one join table def nested? - chain.length > 2 + source_reflection.through_reflection? || through_reflection.through_reflection? end # We want to use the klass from this reflection, rather than just delegate straight to @@ -995,7 +1012,7 @@ module ActiveRecord end def constraints - [source_type_info] + @reflection.constraints + [source_type_info] end def source_type_info diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 243ef0eae9..b99807adf3 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -2,6 +2,8 @@ require "active_record/relation/batches/batch_enumerator" module ActiveRecord module Batches + ORDER_OR_LIMIT_IGNORED_MESSAGE = "Scoped order and limit are ignored, it's forced to be batch order and batch size" + # Looping through a collection of records from the database # (using the Scoping::Named::ClassMethods.all method, for example) # is very inefficient since it will try to instantiate all the objects at once. @@ -31,6 +33,9 @@ module ActiveRecord # * <tt>:batch_size</tt> - Specifies the size of the batch. Default to 1000. # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value. # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when + # the order and limit have to be ignored due to batching. + # # This is especially useful if you want multiple workers dealing with # the same processing queue. You can make worker 1 handle all the records # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond @@ -48,13 +53,13 @@ module ActiveRecord # # NOTE: You can't set the limit either, that's used to control # the batch sizes. - def find_each(start: nil, finish: nil, batch_size: 1000) + def find_each(start: nil, finish: nil, batch_size: 1000, error_on_ignore: nil) if block_given? - find_in_batches(start: start, finish: finish, batch_size: batch_size) do |records| + find_in_batches(start: start, finish: finish, batch_size: batch_size, error_on_ignore: error_on_ignore) do |records| records.each { |record| yield record } end else - enum_for(:find_each, start: start, finish: finish, batch_size: batch_size) do + enum_for(:find_each, start: start, finish: finish, batch_size: batch_size, error_on_ignore: error_on_ignore) do relation = self apply_limits(relation, start, finish).size end @@ -83,6 +88,9 @@ module ActiveRecord # * <tt>:batch_size</tt> - Specifies the size of the batch. Default to 1000. # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value. # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when + # the order and limit have to be ignored due to batching. + # # This is especially useful if you want multiple workers dealing with # the same processing queue. You can make worker 1 handle all the records # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond @@ -100,16 +108,16 @@ module ActiveRecord # # NOTE: You can't set the limit either, that's used to control # the batch sizes. - def find_in_batches(start: nil, finish: nil, batch_size: 1000) + def find_in_batches(start: nil, finish: nil, batch_size: 1000, error_on_ignore: nil) relation = self unless block_given? - return to_enum(:find_in_batches, start: start, finish: finish, batch_size: batch_size) do + return to_enum(:find_in_batches, start: start, finish: finish, batch_size: batch_size, error_on_ignore: error_on_ignore) do total = apply_limits(relation, start, finish).size (total - 1).div(batch_size) + 1 end end - in_batches(of: batch_size, start: start, finish: finish, load: true) do |batch| + in_batches(of: batch_size, start: start, finish: finish, load: true, error_on_ignore: error_on_ignore) do |batch| yield batch.to_a end end @@ -140,6 +148,8 @@ module ActiveRecord # * <tt>:load</tt> - Specifies if the relation should be loaded. Default to false. # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value. # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value. + # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when + # the order and limit have to be ignored due to batching. # # This is especially useful if you want to work with the # ActiveRecord::Relation object instead of the array of records, or if @@ -171,14 +181,14 @@ module ActiveRecord # # NOTE: You can't set the limit either, that's used to control the batch # sizes. - def in_batches(of: 1000, start: nil, finish: nil, load: false) + def in_batches(of: 1000, start: nil, finish: nil, load: false, error_on_ignore: nil) relation = self unless block_given? return BatchEnumerator.new(of: of, start: start, finish: finish, relation: self) end - if logger && (arel.orders.present? || arel.taken.present?) - logger.warn("Scoped order and limit are ignored, it's forced to be batch order and batch size") + if arel.orders.present? || arel.taken.present? + act_on_order_or_limit_ignored(error_on_ignore) end relation = relation.reorder(batch_order).limit(of) @@ -219,5 +229,15 @@ module ActiveRecord def batch_order "#{quoted_table_name}.#{quoted_primary_key} ASC" end + + def act_on_order_or_limit_ignored(error_on_ignore) + raise_error = (error_on_ignore.nil? ? self.klass.error_on_ignored_order_or_limit : error_on_ignore) + + if raise_error + raise ArgumentError.new(ORDER_OR_LIMIT_IGNORED_MESSAGE) + elsif logger + logger.warn(ORDER_OR_LIMIT_IGNORED_MESSAGE) + end + end end end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 0037398554..c3053f0b13 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -255,13 +255,13 @@ module ActiveRecord # Person.offset(3).third_to_last # returns the third-to-last object from OFFSET 3 # Person.where(["user_name = :u", { u: user_name }]).third_to_last def third_to_last - find_nth(-3) + find_nth_from_last 3 end # Same as #third_to_last but raises ActiveRecord::RecordNotFound if no record # is found. def third_to_last! - find_nth!(-3) + find_nth_from_last 3 or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end # Find the second-to-last record. @@ -271,13 +271,13 @@ module ActiveRecord # Person.offset(3).second_to_last # returns the second-to-last object from OFFSET 3 # Person.where(["user_name = :u", { u: user_name }]).second_to_last def second_to_last - find_nth(-2) + find_nth_from_last 2 end # Same as #second_to_last but raises ActiveRecord::RecordNotFound if no record # is found. def second_to_last! - find_nth!(-2) + find_nth_from_last 2 or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end # Returns true if a record exists in the table that matches the +id+ or @@ -561,6 +561,25 @@ module ActiveRecord relation.limit(limit).to_a end + def find_nth_from_last(index) + if loaded? + @records[-index] + else + relation = if order_values.empty? && primary_key + order(arel_attribute(primary_key).asc) + else + self + end + + relation.to_a[-index] + # TODO: can be made more performant on large result sets by + # for instance, last(index)[-index] (which would require + # refactoring the last(n) finder method to make test suite pass), + # or by using a combination of reverse_order, limit, and offset, + # e.g., reverse_order.offset(index-1).first + end + end + private def find_nth_with_limit_and_offset(index, limit, offset:) # :nodoc: diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 7a49322e06..af0c935342 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -130,7 +130,7 @@ IDENTIFIED BY '#{configuration['password']}' WITH GRANT OPTION; 'sslca' => '--ssl-ca', 'sslcert' => '--ssl-cert', 'sslcapath' => '--ssl-capath', - 'sslcipher' => '--ssh-cipher', + 'sslcipher' => '--ssl-cipher', 'sslkey' => '--ssl-key' }.map { |opt, arg| "#{arg}=#{configuration[opt]}" if configuration[opt] }.compact diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index e210e94f00..4911d93dd9 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -61,7 +61,7 @@ module ActiveRecord register(:binary, Type::Binary, override: false) register(:boolean, Type::Boolean, override: false) register(:date, Type::Date, override: false) - register(:date_time, Type::DateTime, override: false) + register(:datetime, Type::DateTime, override: false) register(:decimal, Type::Decimal, override: false) register(:float, Type::Float, override: false) register(:integer, Type::Integer, override: false) diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 0ee147cdba..a9f1993842 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -11,7 +11,8 @@ module ActiveRecord ## # PostgreSQL does not support null bytes in strings - unless current_adapter?(:PostgreSQLAdapter) + unless current_adapter?(:PostgreSQLAdapter) || + (current_adapter?(:SQLite3Adapter) && !ActiveRecord::Base.connection.prepared_statements) def test_update_prepared_statement b = Book.create(name: "my \x00 book") b.reload diff --git a/activerecord/test/cases/adapters/mysql2/explain_test.rb b/activerecord/test/cases/adapters/mysql2/explain_test.rb index 4fc7414b18..b783b5fcd9 100644 --- a/activerecord/test/cases/adapters/mysql2/explain_test.rb +++ b/activerecord/test/cases/adapters/mysql2/explain_test.rb @@ -2,26 +2,20 @@ require "cases/helper" require 'models/developer' require 'models/computer' -module ActiveRecord - module ConnectionAdapters - class Mysql2Adapter - class ExplainTest < ActiveRecord::Mysql2TestCase - fixtures :developers +class Mysql2ExplainTest < ActiveRecord::Mysql2TestCase + fixtures :developers - def test_explain_for_one_query - explain = Developer.where(:id => 1).explain - assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain - assert_match %r(developers |.* const), explain - end + def test_explain_for_one_query + explain = Developer.where(id: 1).explain + assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain + assert_match %r(developers |.* const), explain + end - def test_explain_with_eager_loading - explain = Developer.where(:id => 1).includes(:audit_logs).explain - assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain - assert_match %r(developers |.* const), explain - assert_match %(EXPLAIN for: SELECT `audit_logs`.* FROM `audit_logs` WHERE `audit_logs`.`developer_id` = 1), explain - assert_match %r(audit_logs |.* ALL), explain - end - end - end + def test_explain_with_eager_loading + explain = Developer.where(id: 1).includes(:audit_logs).explain + assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain + assert_match %r(developers |.* const), explain + assert_match %(EXPLAIN for: SELECT `audit_logs`.* FROM `audit_logs` WHERE `audit_logs`.`developer_id` = 1), explain + assert_match %r(audit_logs |.* ALL), explain end end diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb index 4efd728754..00d23740b6 100644 --- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb @@ -1,10 +1,22 @@ require "cases/helper" +require "support/ddl_helper" class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase + include DdlHelper + def setup @conn = ActiveRecord::Base.connection end + def test_exec_query_nothing_raises_with_no_result_queries + assert_nothing_raised do + with_example_table do + @conn.exec_query('INSERT INTO ex (number) VALUES (1)') + @conn.exec_query('DELETE FROM ex WHERE number = 1') + end + end + end + def test_columns_for_distinct_zero_orders assert_equal "posts.id", @conn.columns_for_distinct("posts.id", []) @@ -41,4 +53,10 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase assert_equal "posts.id, posts.created_at AS alias_0", @conn.columns_for_distinct("posts.id", [order]) end + + private + + def with_example_table(definition = 'id int auto_increment primary key, number int, data varchar(255)', &block) + super(@conn, 'ex', definition, &block) + end end diff --git a/activerecord/test/cases/adapters/postgresql/bit_string_test.rb b/activerecord/test/cases/adapters/postgresql/bit_string_test.rb index 6f72fa6e0f..cec6081aec 100644 --- a/activerecord/test/cases/adapters/postgresql/bit_string_test.rb +++ b/activerecord/test/cases/adapters/postgresql/bit_string_test.rb @@ -57,9 +57,11 @@ class PostgresqlBitStringTest < ActiveRecord::PostgreSQLTestCase assert_match %r{t\.bit_varying\s+"a_bit_varying",\s+limit: 4,\s+default: "0011"$}, output end - def test_assigning_invalid_hex_string_raises_exception - assert_raises(ActiveRecord::StatementInvalid) { PostgresqlBitString.create! a_bit: "FF" } - assert_raises(ActiveRecord::StatementInvalid) { PostgresqlBitString.create! a_bit_varying: "FF" } + if ActiveRecord::Base.connection.prepared_statements + def test_assigning_invalid_hex_string_raises_exception + assert_raises(ActiveRecord::StatementInvalid) { PostgresqlBitString.create! a_bit: "FF" } + assert_raises(ActiveRecord::StatementInvalid) { PostgresqlBitString.create! a_bit_varying: "F" } + end end def test_roundtrip diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index d559de3e28..f8403bfe1a 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -125,14 +125,16 @@ module ActiveRecord assert_equal 'SCHEMA', @subscriber.logged[0][1] end - def test_statement_key_is_logged - bind = Relation::QueryAttribute.new(nil, 1, Type::Value.new) - @connection.exec_query('SELECT $1::integer', 'SQL', [bind], prepare: true) - name = @subscriber.payloads.last[:statement_name] - assert name - res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(1)") - plan = res.column_types['QUERY PLAN'].deserialize res.rows.first.first - assert_operator plan.length, :>, 0 + if ActiveRecord::Base.connection.prepared_statements + def test_statement_key_is_logged + bind = Relation::QueryAttribute.new(nil, 1, Type::Value.new) + @connection.exec_query('SELECT $1::integer', 'SQL', [bind], prepare: true) + name = @subscriber.payloads.last[:statement_name] + assert name + res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(1)") + plan = res.column_types['QUERY PLAN'].deserialize res.rows.first.first + assert_operator plan.length, :>, 0 + end end # Must have PostgreSQL >= 9.2, or with_manual_interventions set to diff --git a/activerecord/test/cases/adapters/postgresql/explain_test.rb b/activerecord/test/cases/adapters/postgresql/explain_test.rb index 4d0fd640aa..29bf2c15ea 100644 --- a/activerecord/test/cases/adapters/postgresql/explain_test.rb +++ b/activerecord/test/cases/adapters/postgresql/explain_test.rb @@ -6,15 +6,15 @@ class PostgreSQLExplainTest < ActiveRecord::PostgreSQLTestCase fixtures :developers def test_explain_for_one_query - explain = Developer.where(:id => 1).explain - assert_match %(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = $1), explain + explain = Developer.where(id: 1).explain + assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = \$?1), explain assert_match %(QUERY PLAN), explain end def test_explain_with_eager_loading - explain = Developer.where(:id => 1).includes(:audit_logs).explain + explain = Developer.where(id: 1).includes(:audit_logs).explain assert_match %(QUERY PLAN), explain - assert_match %(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = $1), explain + assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = \$?1), explain assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain end end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 31e87722d9..8b08ebc3c4 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -5,6 +5,7 @@ require 'support/connection_helper' module ActiveRecord module ConnectionAdapters class PostgreSQLAdapterTest < ActiveRecord::PostgreSQLTestCase + self.use_transactional_tests = false include DdlHelper include ConnectionHelper @@ -59,50 +60,6 @@ module ActiveRecord end end - def test_insert_sql_with_proprietary_returning_clause - with_example_table do - id = @connection.insert_sql("insert into ex (number) values(5150)", nil, "number") - assert_equal 5150, id - end - end - - def test_insert_sql_with_quoted_schema_and_table_name - with_example_table do - id = @connection.insert_sql('insert into "public"."ex" (number) values(5150)') - expect = @connection.query('select max(id) from ex').first.first - assert_equal expect, id - end - end - - def test_insert_sql_with_no_space_after_table_name - with_example_table do - id = @connection.insert_sql("insert into ex(number) values(5150)") - expect = @connection.query('select max(id) from ex').first.first - assert_equal expect, id - end - end - - def test_multiline_insert_sql - with_example_table do - id = @connection.insert_sql(<<-SQL) - insert into ex( - number) - values( - 5152 - ) - SQL - expect = @connection.query('select max(id) from ex').first.first - assert_equal expect, id - end - end - - def test_insert_sql_with_returning_disabled - connection = connection_without_insert_returning - id = connection.insert_sql("insert into postgresql_partitioned_table_parent (number) VALUES (1)") - expect = connection.query('select max(id) from postgresql_partitioned_table_parent').first.first - assert_equal expect.to_i, id - end - def test_exec_insert_with_returning_disabled connection = connection_without_insert_returning result = connection.exec_insert("insert into postgresql_partitioned_table_parent (number) VALUES (1)", nil, [], 'id', 'postgresql_partitioned_table_parent_id_seq') @@ -239,30 +196,6 @@ module ActiveRecord @connection.drop_table 'ex2', if_exists: true end - def test_exec_insert_number - with_example_table do - insert(@connection, 'number' => 10) - - result = @connection.exec_query('SELECT number FROM ex WHERE number = 10') - - assert_equal 1, result.rows.length - assert_equal 10, result.rows.last.last - end - end - - def test_exec_insert_string - with_example_table do - str = 'いただきます！' - insert(@connection, 'number' => 10, 'data' => str) - - result = @connection.exec_query('SELECT number, data FROM ex WHERE number = 10') - - value = result.rows.last.last - - assert_equal str, value - end - end - def test_table_alias_length assert_nothing_raised do @connection.table_alias_length @@ -286,33 +219,35 @@ module ActiveRecord end end - def test_exec_with_binds - with_example_table do - string = @connection.quote('foo') - @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = $1', nil, [bind_param(1)]) + if ActiveRecord::Base.connection.prepared_statements + def test_exec_with_binds + with_example_table do + string = @connection.quote('foo') + @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - assert_equal 1, result.rows.length - assert_equal 2, result.columns.length + bind = Relation::QueryAttribute.new("id", 1, Type::Value.new) + result = @connection.exec_query('SELECT id, data FROM ex WHERE id = $1', nil, [bind]) - assert_equal [[1, 'foo']], result.rows + assert_equal 1, result.rows.length + assert_equal 2, result.columns.length + + assert_equal [[1, 'foo']], result.rows + end end - end - def test_exec_typecasts_bind_vals - with_example_table do - string = @connection.quote('foo') - @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") + def test_exec_typecasts_bind_vals + with_example_table do + string = @connection.quote('foo') + @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - bind = ActiveRecord::Relation::QueryAttribute.new("id", "1-fuu", ActiveRecord::Type::Integer.new) - result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = $1', nil, [bind]) + bind = Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new) + result = @connection.exec_query('SELECT id, data FROM ex WHERE id = $1', nil, [bind]) - assert_equal 1, result.rows.length - assert_equal 2, result.columns.length + assert_equal 1, result.rows.length + assert_equal 2, result.columns.length - assert_equal [[1, 'foo']], result.rows + assert_equal [[1, 'foo']], result.rows + end end end @@ -438,19 +373,6 @@ module ActiveRecord end private - def insert(ctx, data) - binds = data.map { |name, value| - bind_param(value, name) - } - columns = binds.map(&:name) - - bind_subs = columns.length.times.map { |x| "$#{x + 1}" } - - sql = "INSERT INTO ex (#{columns.join(", ")}) - VALUES (#{bind_subs.join(', ')})" - - ctx.exec_insert(sql, 'SQL', binds) - end def with_example_table(definition = 'id serial primary key, number integer, data character varying(255)', &block) super(@connection, 'ex', definition, &block) @@ -459,10 +381,6 @@ module ActiveRecord def connection_without_insert_returning ActiveRecord::Base.postgresql_connection(ActiveRecord::Base.configurations['arunit'].merge(:insert_returning => false)) end - - def bind_param(value, name = nil) - ActiveRecord::Relation::QueryAttribute.new(name, value, ActiveRecord::Type::Value.new) - end end end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb index a0afd922b2..285a92f60e 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb @@ -55,20 +55,22 @@ class SchemaAuthorizationTest < ActiveRecord::PostgreSQLTestCase set_session_auth USERS.each do |u| set_session_auth u - assert_equal u, @connection.exec_query("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [bind_param(1)]).first['name'] + assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = 1") set_session_auth end end end - def test_auth_with_bind - assert_nothing_raised do - set_session_auth - USERS.each do |u| - @connection.clear_cache! - set_session_auth u - assert_equal u, @connection.exec_query("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [bind_param(1)]).first['name'] + if ActiveRecord::Base.connection.prepared_statements + def test_auth_with_bind + assert_nothing_raised do set_session_auth + USERS.each do |u| + @connection.clear_cache! + set_session_auth u + assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [bind_param(1)]) + set_session_auth + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index f50fe88b9b..00ebabc9c5 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -172,16 +172,18 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase end end - def test_schema_change_with_prepared_stmt - altered = false - @connection.exec_query "select * from developers where id = $1", 'sql', [bind_param(1)] - @connection.exec_query "alter table developers add column zomg int", 'sql', [] - altered = true - @connection.exec_query "select * from developers where id = $1", 'sql', [bind_param(1)] - ensure - # We are not using DROP COLUMN IF EXISTS because that syntax is only - # supported by pg 9.X - @connection.exec_query("alter table developers drop column zomg", 'sql', []) if altered + if ActiveRecord::Base.connection.prepared_statements + def test_schema_change_with_prepared_stmt + altered = false + @connection.exec_query "select * from developers where id = $1", 'sql', [bind_param(1)] + @connection.exec_query "alter table developers add column zomg int", 'sql', [] + altered = true + @connection.exec_query "select * from developers where id = $1", 'sql', [bind_param(1)] + ensure + # We are not using DROP COLUMN IF EXISTS because that syntax is only + # supported by pg 9.X + @connection.exec_query("alter table developers drop column zomg", 'sql', []) if altered + end end def test_data_source_exists? diff --git a/activerecord/test/cases/adapters/sqlite3/explain_test.rb b/activerecord/test/cases/adapters/sqlite3/explain_test.rb index 2aec322582..a1a6e5f16a 100644 --- a/activerecord/test/cases/adapters/sqlite3/explain_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/explain_test.rb @@ -2,26 +2,20 @@ require "cases/helper" require 'models/developer' require 'models/computer' -module ActiveRecord - module ConnectionAdapters - class SQLite3Adapter - class ExplainTest < ActiveRecord::SQLite3TestCase - fixtures :developers +class SQLite3ExplainTest < ActiveRecord::SQLite3TestCase + fixtures :developers - def test_explain_for_one_query - explain = Developer.where(:id => 1).explain - assert_match %(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = ?), explain - assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) - end + def test_explain_for_one_query + explain = Developer.where(id: 1).explain + assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\?|1)), explain + assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) + end - def test_explain_with_eager_loading - explain = Developer.where(:id => 1).includes(:audit_logs).explain - assert_match %(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = ?), explain - assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) - assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain - assert_match(/(SCAN )?TABLE audit_logs/, explain) - end - end - end + def test_explain_with_eager_loading + explain = Developer.where(id: 1).includes(:audit_logs).explain + assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\?|1)), explain + assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain) + assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain + assert_match(/(SCAN )?TABLE audit_logs/, explain) end end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 02c3358ba6..bbc9f978bf 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -213,24 +213,12 @@ module ActiveRecord assert_equal "''", @conn.quote_string("'") end - def test_insert_sql - with_example_table do - 2.times do |i| - rv = @conn.insert_sql "INSERT INTO ex (number) VALUES (#{i})" - assert_equal(i + 1, rv) - end - - records = @conn.execute "SELECT * FROM ex" - assert_equal 2, records.length - end - end - - def test_insert_sql_logged + def test_insert_logged with_example_table do sql = "INSERT INTO ex (number) VALUES (10)" name = "foo" assert_logged [[sql, name, []]] do - @conn.insert_sql sql, name + @conn.insert(sql, name) end end end @@ -239,7 +227,7 @@ module ActiveRecord with_example_table do sql = "INSERT INTO ex (number) VALUES (10)" idval = 'vuvuzela' - id = @conn.insert_sql sql, nil, nil, idval + id = @conn.insert(sql, nil, nil, idval) assert_equal idval, id end end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 4f99c57c3c..a3046d526e 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -726,7 +726,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert companies(:first_client).readonly_firm.readonly? end - def test_test_polymorphic_assignment_foreign_key_type_string + def test_polymorphic_assignment_foreign_key_type_string comment = Comment.first comment.author = Author.first comment.resource = Member.first diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index ac478cbb01..7f2a2229ee 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -749,6 +749,38 @@ class EagerAssociationTest < ActiveRecord::TestCase } end + def test_eager_has_many_through_with_order + tag = OrderedTag.create(name: 'Foo') + post1 = Post.create!(title: 'Beaches', body: "I like beaches!") + post2 = Post.create!(title: 'Pools', body: "I like pools!") + + Tagging.create!(taggable_type: 'Post', taggable_id: post1.id, tag: tag) + Tagging.create!(taggable_type: 'Post', taggable_id: post2.id, tag: tag) + + tag_with_includes = OrderedTag.includes(:tagged_posts).find(tag.id) + assert_equal(tag_with_includes.taggings.map(&:taggable).map(&:title), tag_with_includes.tagged_posts.map(&:title)) + end + + def test_eager_has_many_through_multiple_with_order + tag1 = OrderedTag.create!(name: 'Bar') + tag2 = OrderedTag.create!(name: 'Foo') + + post1 = Post.create!(title: 'Beaches', body: "I like beaches!") + post2 = Post.create!(title: 'Pools', body: "I like pools!") + + Tagging.create!(taggable: post1, tag: tag1) + Tagging.create!(taggable: post2, tag: tag1) + Tagging.create!(taggable: post2, tag: tag2) + Tagging.create!(taggable: post1, tag: tag2) + + tags_with_includes = OrderedTag.where(id: [tag1, tag2].map(&:id)).includes(:tagged_posts).order(:id).to_a + tag1_with_includes = tags_with_includes.first + tag2_with_includes = tags_with_includes.last + + assert_equal([post2, post1].map(&:title), tag1_with_includes.tagged_posts.map(&:title)) + assert_equal([post1, post2].map(&:title), tag2_with_includes.tagged_posts.map(&:title)) + end + def test_eager_with_default_scope developer = EagerDeveloperWithDefaultScope.where(:name => 'David').first projects = Project.order(:id).to_a @@ -1360,6 +1392,18 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal('10 was not recognized for preload', exception.message) end + test "associations with extensions are not instance dependent" do + assert_nothing_raised do + Author.includes(:posts_with_extension).to_a + end + end + + test "including associations with extensions and an instance dependent scope is not supported" do + e = assert_raises(ArgumentError) do + Author.includes(:posts_with_extension_and_instance).to_a + end + assert_match(/Preloading instance dependent scopes is not supported/, e.message) + end test "preloading readonly association" do # has-one diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index c850875619..1d892a0956 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -363,6 +363,13 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_equal posts(:welcome, :thinking).sort_by(&:id), tags(:general).tagged_posts.sort_by(&:id) end + def test_has_many_polymorphic_associations_merges_through_scope + Tag.has_many :null_taggings, -> { none }, class_name: :Tagging + Tag.has_many :null_tagged_posts, :through => :null_taggings, :source => 'taggable', :source_type => 'Post' + assert_equal [], tags(:general).null_tagged_posts + refute_equal [], tags(:general).tagged_posts + end + def test_eager_has_many_polymorphic_with_source_type tag_with_include = Tag.all.merge!(:includes => :tagged_posts).find(tags(:general).id) desired = posts(:welcome, :thinking) diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 84aac3e721..91ff5146fd 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -164,6 +164,42 @@ class EachTest < ActiveRecord::TestCase assert_equal posts(:welcome).id, posts.first.id end + def test_find_in_batches_should_error_on_ignore_the_order + assert_raise(ArgumentError) do + PostWithDefaultScope.find_in_batches(error_on_ignore: true){} + end + end + + def test_find_in_batches_should_not_error_if_config_overriden + # Set the config option which will be overriden + prev = ActiveRecord::Base.error_on_ignored_order_or_limit + ActiveRecord::Base.error_on_ignored_order_or_limit = true + assert_nothing_raised do + PostWithDefaultScope.find_in_batches(error_on_ignore: false){} + end + ensure + # Set back to default + ActiveRecord::Base.error_on_ignored_order_or_limit = prev + end + + def test_find_in_batches_should_error_on_config_specified_to_error + # Set the config option + prev = ActiveRecord::Base.error_on_ignored_order_or_limit + ActiveRecord::Base.error_on_ignored_order_or_limit = true + assert_raise(ArgumentError) do + PostWithDefaultScope.find_in_batches(){} + end + ensure + # Set back to default + ActiveRecord::Base.error_on_ignored_order_or_limit = prev + end + + def test_find_in_batches_should_not_error_by_default + assert_nothing_raised do + PostWithDefaultScope.find_in_batches(){} + end + end + def test_find_in_batches_should_not_ignore_the_default_scope_if_it_is_other_then_order special_posts_ids = SpecialPostWithDefaultScope.all.map(&:id).sort posts = [] diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index cd9c76f1f0..fa924fe4cb 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -31,7 +31,8 @@ module ActiveRecord ActiveSupport::Notifications.unsubscribe(@subscription) end - if ActiveRecord::Base.connection.supports_statement_cache? + if ActiveRecord::Base.connection.supports_statement_cache? && + ActiveRecord::Base.connection.prepared_statements def test_bind_from_join_in_subquery subquery = Author.joins(:thinking_posts).where(name: 'David') scope = Author.from(subquery, 'authors').where(id: 1) diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index d43668e57c..c4c2c69d1c 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -19,7 +19,7 @@ module ActiveRecord def setup @env = {} @app = App.new - @management = ConnectionManagement.new(@app) + @management = middleware(@app) # make sure we have an active connection assert ActiveRecord::Base.connection @@ -27,17 +27,12 @@ module ActiveRecord end def test_app_delegation - manager = ConnectionManagement.new(@app) + manager = middleware(@app) manager.call @env assert_equal [@env], @app.calls end - def test_connections_are_active_after_call - @management.call(@env) - assert ActiveRecord::Base.connection_handler.active_connections? - end - def test_body_responds_to_each _, _, body = @management.call(@env) bits = [] @@ -52,45 +47,40 @@ module ActiveRecord end def test_active_connections_are_not_cleared_on_body_close_during_test - @env['rack.test'] = true - _, _, body = @management.call(@env) - body.close - assert ActiveRecord::Base.connection_handler.active_connections? + executor.wrap do + _, _, body = @management.call(@env) + body.close + assert ActiveRecord::Base.connection_handler.active_connections? + end end def test_connections_closed_if_exception app = Class.new(App) { def call(env); raise NotImplementedError; end }.new - explosive = ConnectionManagement.new(app) + explosive = middleware(app) assert_raises(NotImplementedError) { explosive.call(@env) } assert !ActiveRecord::Base.connection_handler.active_connections? end def test_connections_not_closed_if_exception_and_test - @env['rack.test'] = true - app = Class.new(App) { def call(env); raise; end }.new - explosive = ConnectionManagement.new(app) - assert_raises(RuntimeError) { explosive.call(@env) } - assert ActiveRecord::Base.connection_handler.active_connections? - end - - def test_connections_closed_if_exception_and_explicitly_not_test - @env['rack.test'] = false - app = Class.new(App) { def call(env); raise NotImplementedError; end }.new - explosive = ConnectionManagement.new(app) - assert_raises(NotImplementedError) { explosive.call(@env) } - assert !ActiveRecord::Base.connection_handler.active_connections? + executor.wrap do + app = Class.new(App) { def call(env); raise; end }.new + explosive = middleware(app) + assert_raises(RuntimeError) { explosive.call(@env) } + assert ActiveRecord::Base.connection_handler.active_connections? + end end test "doesn't clear active connections when running in a test case" do - @env['rack.test'] = true - @management.call(@env) - assert ActiveRecord::Base.connection_handler.active_connections? + executor.wrap do + @management.call(@env) + assert ActiveRecord::Base.connection_handler.active_connections? + end end test "proxy is polite to its body and responds to it" do body = Class.new(String) { def to_path; "/path"; end }.new app = lambda { |_| [200, {}, body] } - response_body = ConnectionManagement.new(app).call(@env)[2] + response_body = middleware(app).call(@env)[2] assert response_body.respond_to?(:to_path) assert_equal "/path", response_body.to_path end @@ -98,9 +88,23 @@ module ActiveRecord test "doesn't mutate the original response" do original_response = [200, {}, 'hi'] app = lambda { |_| original_response } - ConnectionManagement.new(app).call(@env)[2] + middleware(app).call(@env)[2] assert_equal 'hi', original_response.last end + + private + def executor + @executor ||= Class.new(ActiveSupport::Executor).tap do |exe| + ActiveRecord::QueryCache.install_executor_hooks(exe) + end + end + + def middleware(app) + lambda do |env| + a, b, c = executor.wrap { app.call(env) } + [a, b, Rack::BodyProxy.new(c) { }] + end + end end end end diff --git a/activerecord/test/cases/database_statements_test.rb b/activerecord/test/cases/database_statements_test.rb index ba085991e0..3169408ac0 100644 --- a/activerecord/test/cases/database_statements_test.rb +++ b/activerecord/test/cases/database_statements_test.rb @@ -13,6 +13,12 @@ class DatabaseStatementsTest < ActiveRecord::TestCase assert_not_nil return_the_inserted_id(method: :create) end + def test_insert_update_delete_sql_is_deprecated + assert_deprecated { @connection.insert_sql("INSERT INTO accounts (firm_id,credit_limit) VALUES (42,5000)") } + assert_deprecated { @connection.update_sql("UPDATE accounts SET credit_limit = 6000 WHERE firm_id = 42") } + assert_deprecated { @connection.delete_sql("DELETE FROM accounts WHERE firm_id = 42") } + end + private def return_the_inserted_id(method:) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 3e31874455..692c6bf2d0 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -486,6 +486,66 @@ class FinderTest < ActiveRecord::TestCase end end + def test_second_to_last + assert_equal topics(:fourth).title, Topic.second_to_last.title + + # test with offset + assert_equal topics(:fourth), Topic.offset(1).second_to_last + assert_equal topics(:fourth), Topic.offset(2).second_to_last + assert_equal topics(:fourth), Topic.offset(3).second_to_last + assert_equal nil, Topic.offset(4).second_to_last + assert_equal nil, Topic.offset(5).second_to_last + + #test with limit + # assert_equal nil, Topic.limit(1).second # TODO: currently failing + assert_equal nil, Topic.limit(1).second_to_last + end + + def test_second_to_last_have_primary_key_order_by_default + expected = topics(:fourth) + expected.touch # PostgreSQL changes the default order if no order clause is used + assert_equal expected, Topic.second_to_last + end + + def test_model_class_responds_to_second_to_last_bang + assert Topic.second_to_last! + Topic.delete_all + assert_raises_with_message ActiveRecord::RecordNotFound, "Couldn't find Topic" do + Topic.second_to_last! + end + end + + def test_third_to_last + assert_equal topics(:third).title, Topic.third_to_last.title + + # test with offset + assert_equal topics(:third), Topic.offset(1).third_to_last + assert_equal topics(:third), Topic.offset(2).third_to_last + assert_equal nil, Topic.offset(3).third_to_last + assert_equal nil, Topic.offset(4).third_to_last + assert_equal nil, Topic.offset(5).third_to_last + + # test with limit + # assert_equal nil, Topic.limit(1).third # TODO: currently failing + assert_equal nil, Topic.limit(1).third_to_last + # assert_equal nil, Topic.limit(2).third # TODO: currently failing + assert_equal nil, Topic.limit(2).third_to_last + end + + def test_third_to_last_have_primary_key_order_by_default + expected = topics(:third) + expected.touch # PostgreSQL changes the default order if no order clause is used + assert_equal expected, Topic.third_to_last + end + + def test_model_class_responds_to_third_to_last_bang + assert Topic.third_to_last! + Topic.delete_all + assert_raises_with_message ActiveRecord::RecordNotFound, "Couldn't find Topic" do + Topic.third_to_last! + end + end + def test_last_bang_present assert_nothing_raised do assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").last! diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 95f8706d73..d2fdf03e9d 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -1,5 +1,3 @@ -require File.expand_path('../../../../load_paths', __FILE__) - require 'config' require 'active_support/testing/autorun' diff --git a/activerecord/test/cases/hot_compatibility_test.rb b/activerecord/test/cases/hot_compatibility_test.rb index 5ba9a1029a..9fc75b7377 100644 --- a/activerecord/test/cases/hot_compatibility_test.rb +++ b/activerecord/test/cases/hot_compatibility_test.rb @@ -1,7 +1,9 @@ require 'cases/helper' +require 'support/connection_helper' class HotCompatibilityTest < ActiveRecord::TestCase self.use_transactional_tests = false + include ConnectionHelper setup do @klass = Class.new(ActiveRecord::Base) do @@ -51,4 +53,90 @@ class HotCompatibilityTest < ActiveRecord::TestCase record.reload assert_equal 'bar', record.foo end + + if current_adapter?(:PostgreSQLAdapter) + test "cleans up after prepared statement failure in a transaction" do + with_two_connections do |original_connection, ddl_connection| + record = @klass.create! bar: 'bar' + + # prepare the reload statement in a transaction + @klass.transaction do + record.reload + end + + assert get_prepared_statement_cache(@klass.connection).any?, + "expected prepared statement cache to have something in it" + + # add a new column + ddl_connection.add_column :hot_compatibilities, :baz, :string + + assert_raise(ActiveRecord::PreparedStatementCacheExpired) do + @klass.transaction do + record.reload + end + end + + assert_empty get_prepared_statement_cache(@klass.connection), + "expected prepared statement cache to be empty but it wasn't" + end + end + + test "cleans up after prepared statement failure in nested transactions" do + with_two_connections do |original_connection, ddl_connection| + record = @klass.create! bar: 'bar' + + # prepare the reload statement in a transaction + @klass.transaction do + record.reload + end + + assert get_prepared_statement_cache(@klass.connection).any?, + "expected prepared statement cache to have something in it" + + # add a new column + ddl_connection.add_column :hot_compatibilities, :baz, :string + + assert_raise(ActiveRecord::PreparedStatementCacheExpired) do + @klass.transaction do + @klass.transaction do + @klass.transaction do + record.reload + end + end + end + end + + assert_empty get_prepared_statement_cache(@klass.connection), + "expected prepared statement cache to be empty but it wasn't" + end + end + end + + private + + def get_prepared_statement_cache(connection) + connection.instance_variable_get(:@statements) + .instance_variable_get(:@cache)[Process.pid] + end + + # Rails will automatically clear the prepared statements on the connection + # that runs the migration, so we use two connections to simulate what would + # actually happen on a production system; we'd have one connection running the + # migration from the rake task ("ddl_connection" here), and we'd have another + # connection in a web worker. + def with_two_connections + run_without_connection do |original_connection| + ActiveRecord::Base.establish_connection(original_connection.merge(pool_size: 2)) + begin + ddl_connection = ActiveRecord::Base.connection_pool.checkout + begin + yield original_connection, ddl_connection + ensure + ActiveRecord::Base.connection_pool.checkin ddl_connection + end + ensure + ActiveRecord::Base.clear_all_connections! + end + end + end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 4fe76e563a..6c59d7337a 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -441,7 +441,7 @@ unless in_memory_db? def test_lock_sending_custom_lock_statement Person.transaction do person = Person.find(1) - assert_sql(/LIMIT \$\d FOR SHARE NOWAIT/) do + assert_sql(/LIMIT \$?\d FOR SHARE NOWAIT/) do person.lock!('FOR SHARE NOWAIT') end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index e27a747730..6aaee8de7c 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -228,9 +228,10 @@ class PrimaryKeyAnyTypeTest < ActiveRecord::TestCase def test_any_type_primary_key assert_equal "code", Barcode.primary_key - column_type = Barcode.type_for_attribute(Barcode.primary_key) - assert_equal :string, column_type.type - assert_equal 42, column_type.limit + column = Barcode.column_for_attribute(Barcode.primary_key) + assert_not column.null unless current_adapter?(:SQLite3Adapter) + assert_equal :string, column.type + assert_equal 42, column.limit end test "schema dump primary key includes type and options" do diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index d84653e4c9..e53239cdee 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -16,7 +16,7 @@ class QueryCacheTest < ActiveRecord::TestCase def test_exceptional_middleware_clears_and_disables_cache_on_error assert !ActiveRecord::Base.connection.query_cache_enabled, 'cache off' - mw = ActiveRecord::QueryCache.new lambda { |env| + mw = middleware { |env| Task.find 1 Task.find 1 assert_equal 1, ActiveRecord::Base.connection.query_cache.length @@ -31,7 +31,7 @@ class QueryCacheTest < ActiveRecord::TestCase def test_exceptional_middleware_leaves_enabled_cache_alone ActiveRecord::Base.connection.enable_query_cache! - mw = ActiveRecord::QueryCache.new lambda { |env| + mw = middleware { |env| raise "lol borked" } assert_raises(RuntimeError) { mw.call({}) } @@ -42,7 +42,7 @@ class QueryCacheTest < ActiveRecord::TestCase def test_exceptional_middleware_assigns_original_connection_id_on_error connection_id = ActiveRecord::Base.connection_id - mw = ActiveRecord::QueryCache.new lambda { |env| + mw = middleware { |env| ActiveRecord::Base.connection_id = self.object_id raise "lol borked" } @@ -53,7 +53,7 @@ class QueryCacheTest < ActiveRecord::TestCase def test_middleware_delegates called = false - mw = ActiveRecord::QueryCache.new lambda { |env| + mw = middleware { |env| called = true [200, {}, nil] } @@ -62,7 +62,7 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_middleware_caches - mw = ActiveRecord::QueryCache.new lambda { |env| + mw = middleware { |env| Task.find 1 Task.find 1 assert_equal 1, ActiveRecord::Base.connection.query_cache.length @@ -74,50 +74,13 @@ class QueryCacheTest < ActiveRecord::TestCase def test_cache_enabled_during_call assert !ActiveRecord::Base.connection.query_cache_enabled, 'cache off' - mw = ActiveRecord::QueryCache.new lambda { |env| + mw = middleware { |env| assert ActiveRecord::Base.connection.query_cache_enabled, 'cache on' [200, {}, nil] } mw.call({}) end - def test_cache_on_during_body_write - streaming = Class.new do - def each - yield ActiveRecord::Base.connection.query_cache_enabled - end - end - - mw = ActiveRecord::QueryCache.new lambda { |env| - [200, {}, streaming.new] - } - body = mw.call({}).last - body.each { |x| assert x, 'cache should be on' } - body.close - assert !ActiveRecord::Base.connection.query_cache_enabled, 'cache disabled' - end - - def test_cache_off_after_close - mw = ActiveRecord::QueryCache.new lambda { |env| [200, {}, nil] } - body = mw.call({}).last - - assert ActiveRecord::Base.connection.query_cache_enabled, 'cache enabled' - body.close - assert !ActiveRecord::Base.connection.query_cache_enabled, 'cache disabled' - end - - def test_cache_clear_after_close - mw = ActiveRecord::QueryCache.new lambda { |env| - Post.first - [200, {}, nil] - } - body = mw.call({}).last - - assert !ActiveRecord::Base.connection.query_cache.empty?, 'cache not empty' - body.close - assert ActiveRecord::Base.connection.query_cache.empty?, 'cache should be empty' - end - def test_cache_passing_a_relation post = Post.first Post.cache do @@ -244,6 +207,13 @@ class QueryCacheTest < ActiveRecord::TestCase assert_equal 0, Post.where(title: 'rollback').to_a.count end end + + private + def middleware(&app) + executor = Class.new(ActiveSupport::Executor) + ActiveRecord::QueryCache.install_executor_hooks executor + lambda { |env| executor.wrap { app.call(env) } } + end end class QueryCacheExpiryTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/validations/i18n_validation_test.rb b/activerecord/test/cases/validations/i18n_validation_test.rb index 981239c4d6..b8307d6665 100644 --- a/activerecord/test/cases/validations/i18n_validation_test.rb +++ b/activerecord/test/cases/validations/i18n_validation_test.rb @@ -47,8 +47,6 @@ class I18nValidationTest < ActiveRecord::TestCase # [ "given on condition", {on: :save}, {}] ] - # validates_uniqueness_of w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_uniqueness_of on generated message #{name}" do Topic.validates_uniqueness_of :title, validation_options @@ -59,8 +57,6 @@ class I18nValidationTest < ActiveRecord::TestCase end end - # validates_associated w/ mocha - COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_associated on generated message #{name}" do Topic.validates_associated :replies, validation_options @@ -70,8 +66,6 @@ class I18nValidationTest < ActiveRecord::TestCase end end - # validates_associated w/o mocha - def test_validates_associated_finds_custom_model_key_translation I18n.backend.store_translations 'en', :activerecord => {:errors => {:models => {:topic => {:attributes => {:replies => {:invalid => 'custom message'}}}}}} I18n.backend.store_translations 'en', :activerecord => {:errors => {:messages => {:invalid => 'global message'}}} diff --git a/activerecord/test/cases/validations/presence_validation_test.rb b/activerecord/test/cases/validations/presence_validation_test.rb index 2ae30c7fd3..868d111b8c 100644 --- a/activerecord/test/cases/validations/presence_validation_test.rb +++ b/activerecord/test/cases/validations/presence_validation_test.rb @@ -92,7 +92,7 @@ class PresenceValidationTest < ActiveRecord::TestCase end end - def test_validates_prescence_with_on_context + def test_validates_presence_with_on_context repair_validations(Interest) do Interest.validates_presence_of(:topic, on: :required_name) interest = Interest.new diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index f25e31b13d..38b983eda0 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -144,6 +144,14 @@ class Author < ActiveRecord::Base has_many :posts_with_signature, ->(record) { where("posts.title LIKE ?", "%by #{record.name.downcase}%") }, class_name: "Post" + has_many :posts_with_extension, -> { order(:title) }, class_name: "Post" do + def extension_method; end + end + + has_many :posts_with_extension_and_instance, ->(record) { order(:title) }, class_name: "Post" do + def extension_method; end + end + attr_accessor :post_log after_initialize :set_post_log diff --git a/activerecord/test/models/tag.rb b/activerecord/test/models/tag.rb index 80d4725f7e..b48b9a2155 100644 --- a/activerecord/test/models/tag.rb +++ b/activerecord/test/models/tag.rb @@ -5,3 +5,9 @@ class Tag < ActiveRecord::Base has_many :tagged_posts, :through => :taggings, :source => 'taggable', :source_type => 'Post' end + +class OrderedTag < Tag + self.table_name = "tags" + + has_many :taggings, -> { order('taggings.id DESC') }, foreign_key: 'tag_id' +end |