diff options
Diffstat (limited to 'activerecord')
129 files changed, 1429 insertions, 724 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0832652d32..3bb4b5236e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,119 @@ +* 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 + validations. There's no reason to validate a record you aren't planning on saving. + + *Eileen M. Uchitelle* + +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* Ensure that mutations of the array returned from `ActiveRecord::Relation#to_a` + do not affect the original relation, by returning a duplicate array each time. + + This brings the behavior in line with `CollectionProxy#to_a`, which was + already more careful. + + *Matthew Draper* + +* Fixed `where` for polymorphic associations when passed an array containing different types. + + Fixes #17011. + + Example: + + PriceEstimate.where(estimate_of: [Treasure.find(1), Car.find(2)]) + # => SELECT "price_estimates".* FROM "price_estimates" + WHERE (("price_estimates"."estimate_of_type" = 'Treasure' AND "price_estimates"."estimate_of_id" = 1) + OR ("price_estimates"."estimate_of_type" = 'Car' AND "price_estimates"."estimate_of_id" = 2)) + + *Philippe Huibonhoa* + +* Fix a bug where using `t.foreign_key` twice with the same `to_table` within + the same table definition would only create one foreign key. + + *George Millo* + +* Fix a regression on has many association, where calling a child from parent in child's callback + results in same child records getting added repeatedly to target. + + Fixes #13387. + + *Bogdan Gusiev*, *Jon Hinson* + +* Rework `ActiveRecord::Relation#last`. + + 1. Never perform additional SQL on loaded relation + 2. Use SQL reverse order instead of loading relation if relation doesn't have limit + 3. Deprecated relation loading when SQL order can not be automatically reversed + + Topic.order("title").load.last(3) + # before: SELECT ... + # after: No SQL + + Topic.order("title").last + # before: SELECT * FROM `topics` + # after: SELECT * FROM `topics` ORDER BY `topics`.`title` DESC LIMIT 1 + + Topic.order("coalesce(author, title)").last + # before: SELECT * FROM `topics` + # after: Deprecation Warning for irreversible order + + *Bogdan Gusiev* + +* Allow `joins` to be unscoped. + + Fixes #13775. + + *Takashi Kokubun* + +* Add ActiveRecord `#second_to_last` and `#third_to_last` methods. + + *Brian Christian* + +* Added `numeric` helper into migrations. + + Example: + + create_table(:numeric_types) do |t| + t.numeric :numeric_type, precision: 10, scale: 2 + end + + *Mehmet Emin İNAÇ* + * Bumped the minimum supported version of PostgreSQL to >= 9.1. Both PG 9.0 and 8.4 are past their end of life date: http://www.postgresql.org/support/versioning/ @@ -602,7 +718,7 @@ *Ben Murphy*, *Matthew Draper* -* `bin/rake db:migrate` uses +* `bin/rails db:migrate` uses `ActiveRecord::Tasks::DatabaseTasks.migrations_paths` instead of `Migrator.migrations_paths`. @@ -1378,18 +1494,6 @@ *Chris Sinjakli* -* Validation errors would be raised for parent records when an association - was saved when the parent had `validate: false`. It should not be the - responsibility of the model to validate an associated object unless the - object was created or modified by the parent. - - This fixes the issue by skipping validations if the parent record is - persisted, not changed, and not marked for destruction. - - Fixes #17621. - - *Eileen M. Uchitelle*, *Aaron Patterson* - * Fix n+1 query problem when eager loading nil associations (fixes #18312) *Sammy Larbi* 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/association_relation.rb b/activerecord/lib/active_record/association_relation.rb index ee0bb8fafe..c18e88e4cf 100644 --- a/activerecord/lib/active_record/association_relation.rb +++ b/activerecord/lib/active_record/association_relation.rb @@ -10,7 +10,7 @@ module ActiveRecord end def ==(other) - other == to_a + other == records end def build(*args, &block) 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/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index b888148841..5fbd79d118 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -76,6 +76,9 @@ module ActiveRecord::Associations::Builder # :nodoc: left_model.retrieve_connection end + def self.primary_key + false + end } join_model.name = "HABTM_#{association_name.to_s.camelize}" diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 9f2c7292ea..2dca6b612e 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -136,6 +136,14 @@ module ActiveRecord first_nth_or_last(:forty_two, *args) end + def third_to_last(*args) + first_nth_or_last(:third_to_last, *args) + end + + def second_to_last(*args) + first_nth_or_last(:second_to_last, *args) + end + def last(*args) first_nth_or_last(:last, *args) end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index fe693cfbb6..b9aed05135 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -197,6 +197,16 @@ module ActiveRecord @association.forty_two(*args) end + # Same as #first except returns only the third-to-last record. + def third_to_last(*args) + @association.third_to_last(*args) + end + + # Same as #first except returns only the second-to-last record. + def second_to_last(*args) + @association.second_to_last(*args) + end + # Returns the last record, or the last +n+ records, from the collection. # If the collection is empty, the first form returns +nil+, and the second # form returns an empty array. @@ -969,6 +979,10 @@ module ActiveRecord end alias_method :to_a, :to_ary + def records # :nodoc: + load_target + end + # Adds one or more +records+ to the collection by setting their foreign keys # to the association's primary key. Returns +self+, so several appends may be # chained together. diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 708b3af5bd..c5fbe0d1d1 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -54,12 +54,18 @@ module ActiveRecord end scope_chain_index += 1 - relation = ActiveRecord::Relation.create( - klass, - table, - predicate_builder, - ) - scope_chain_items.concat [klass.send(:build_default_scope, relation)].compact + klass_scope = + if klass.current_scope + klass.current_scope.clone + else + relation = ActiveRecord::Relation.create( + klass, + table, + predicate_builder, + ) + klass.send(:build_default_scope, relation) + end + scope_chain_items.concat [klass_scope].compact rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right| left.merge right 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/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index c7cc48ba16..f913f0852a 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -45,7 +45,7 @@ module ActiveRecord end def get_records - return scope.limit(1).to_a if skip_statement_cache? + return scope.limit(1).records if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index a6d81c82b4..4c22be8235 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -29,14 +29,6 @@ module ActiveRecord assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty? end - # Tries to assign given value to given attribute. - # In case of an error, re-raises with the ActiveRecord constant. - def _assign_attribute(k, v) # :nodoc: - super - rescue ActiveModel::UnknownAttributeError - raise UnknownAttributeError.new(self, k) - end - # Assign any deferred nested attributes after the base attributes have been set. def assign_nested_parameter_attributes(pairs) pairs.each { |k, v| _assign_attribute(k, v) } diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 423a93964e..e902eb7531 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -34,30 +34,6 @@ module ActiveRecord BLACKLISTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass) - class AttributeMethodCache - def initialize - @module = Module.new - @method_cache = Concurrent::Map.new - end - - def [](name) - @method_cache.compute_if_absent(name) do - safe_name = name.unpack('h*'.freeze).first - temp_method = "__temp__#{safe_name}" - ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name - @module.module_eval method_body(temp_method, safe_name), __FILE__, __LINE__ - @module.instance_method temp_method - end - end - - private - - # Override this method in the subclasses for method body. - def method_body(method_name, const_name) - raise NotImplementedError, "Subclasses must implement a method_body(method_name, const_name) method." - end - end - class GeneratedAttributeMethods < Module; end # :nodoc: module ClassMethods diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 5197e21fa4..ab2ecaa7c5 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -1,8 +1,11 @@ module ActiveRecord module AttributeMethods module Read - ReaderMethodCache = Class.new(AttributeMethodCache) { - private + extend ActiveSupport::Concern + + module ClassMethods + protected + # We want to generate the methods via module_eval rather than # define_method, because define_method is slower on dispatch. # Evaluating many similar methods may use more memory as the instruction @@ -21,21 +24,6 @@ module ActiveRecord # to allocate an object on each call to the attribute method. # Making it frozen means that it doesn't get duped when used to # key the @attributes in read_attribute. - def method_body(method_name, const_name) - <<-EOMETHOD - def #{method_name} - name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{const_name} - _read_attribute(name) { |n| missing_attribute(n, caller) } - end - EOMETHOD - end - }.new - - extend ActiveSupport::Concern - - module ClassMethods - protected - def define_method_attribute(name) safe_name = name.unpack('h*'.freeze).first temp_method = "__temp__#{safe_name}" diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index bbf2a51a0e..5599b590ca 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -1,19 +1,6 @@ module ActiveRecord module AttributeMethods module Write - WriterMethodCache = Class.new(AttributeMethodCache) { - private - - def method_body(method_name, const_name) - <<-EOMETHOD - def #{method_name}(value) - name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{const_name} - write_attribute(name, value) - end - EOMETHOD - end - }.new - extend ActiveSupport::Concern included do 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/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 854f9776a3..1f1b11eb68 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -179,7 +179,7 @@ module ActiveRecord # # If the +before_validation+ callback throws +:abort+, the process will be # aborted and {ActiveRecord::Base#save}[rdoc-ref:Persistence#save] will return +false+. - # If {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] is called it will raise a ActiveRecord::RecordInvalid exception. + # If {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] is called it will raise an ActiveRecord::RecordInvalid exception. # Nothing will be appended to the errors object. # # == Canceling 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 7e0c9f7837..824040775d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -27,10 +27,10 @@ module ActiveRecord end # Returns an ActiveRecord::Result instance. - def select_all(arel, name = nil, binds = []) + def select_all(arel, name = nil, binds = [], preparable: nil) arel, binds = binds_from_relation arel, binds sql = to_sql(arel, binds) - if arel.is_a?(String) + if !prepared_statements || (arel.is_a?(String) && preparable.nil?) preparable = false else preparable = visitor.preparable @@ -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 5e27cfe507..0bdfd4f900 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -61,11 +61,11 @@ module ActiveRecord @query_cache.clear end - def select_all(arel, name = nil, binds = []) + def select_all(arel, name = nil, binds = [], preparable: nil) 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) } + cache_sql(sql, binds) { super(sql, name, binds, preparable: preparable) } else super end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 690e0ba957..4f97c7c065 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -182,6 +182,7 @@ module ActiveRecord end CODE end + alias_method :numeric, :decimal end # Represents the schema of an SQL table in an abstract way. This class @@ -211,7 +212,7 @@ module ActiveRecord def initialize(name, temporary, options, as = nil) @columns_hash = {} @indexes = {} - @foreign_keys = {} + @foreign_keys = [] @primary_keys = nil @temporary = temporary @options = options @@ -329,7 +330,7 @@ module ActiveRecord end def foreign_key(table_name, options = {}) # :nodoc: - foreign_keys[table_name] = options + foreign_keys.push([table_name, options]) end # Appends <tt>:datetime</tt> columns <tt>:created_at</tt> and @@ -436,6 +437,7 @@ module ActiveRecord # t.bigint # t.float # t.decimal + # t.numeric # t.datetime # t.timestamp # t.time 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 a95109fdae..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: column.type.inspect } - spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type].include?(key) }) + spec = { id: schema_type(column).inspect } + 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) - 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 @@ -57,7 +57,7 @@ module ActiveRecord private def schema_type(column) - column.type.to_s + column.type end def schema_limit(column) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 983c4340c6..f0f855963a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -459,7 +459,7 @@ module ActiveRecord # The +type+ parameter is normally one of the migrations native types, # which is one of the following: # <tt>:primary_key</tt>, <tt>:string</tt>, <tt>:text</tt>, - # <tt>:integer</tt>, <tt>:bigint</tt>, <tt>:float</tt>, <tt>:decimal</tt>, + # <tt>:integer</tt>, <tt>:bigint</tt>, <tt>:float</tt>, <tt>:decimal</tt>, <tt>:numeric</tt>, # <tt>:datetime</tt>, <tt>:time</tt>, <tt>:date</tt>, # <tt>:binary</tt>, <tt>:boolean</tt>. # @@ -477,9 +477,9 @@ module ActiveRecord # Allows or disallows +NULL+ values in the column. This option could # have been named <tt>:null_allowed</tt>. # * <tt>:precision</tt> - - # Specifies the precision for a <tt>:decimal</tt> column. + # Specifies the precision for the <tt>:decimal</tt> and <tt>:numeric</tt> columns. # * <tt>:scale</tt> - - # Specifies the scale for a <tt>:decimal</tt> column. + # Specifies the scale for the <tt>:decimal</tt> and <tt>:numeric</tt> columns. # # Note: The precision is the total number of significant digits # and the scale is the number of digits that can be stored following 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 d9b42d4283..fcc1ef9d5f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -1,5 +1,4 @@ require 'active_record/type' -require 'active_support/core_ext/benchmark' require 'active_record/connection_adapters/determine_if_preparable_visitor' require 'active_record/connection_adapters/schema_cache' require 'active_record/connection_adapters/sql_type_metadata' @@ -28,7 +27,6 @@ module ActiveRecord autoload_at 'active_record/connection_adapters/abstract/connection_pool' do autoload :ConnectionHandler - autoload :ConnectionManagement end autoload_under 'abstract' do @@ -398,7 +396,7 @@ module ActiveRecord if can_perform_case_insensitive_comparison_for?(column) table[attribute].lower.eq(table.lower(Arel::Nodes::BindParam.new)) else - case_sensitive_comparison(table, attribute, column, value) + table[attribute].eq(Arel::Nodes::BindParam.new) end end 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 8751b6da4b..b12bac2737 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -615,13 +615,10 @@ module ActiveRecord end end - def case_insensitive_comparison(table, attribute, column, value) - if column.case_sensitive? - super - else - table[attribute].eq(Arel::Nodes::BindParam.new) - end + def can_perform_case_insensitive_comparison_for?(column) + column.case_sensitive? end + private :can_perform_case_insensitive_comparison_for? # In MySQL 5.7.5 and up, ONLY_FULL_GROUP_BY affects handling of queries that use # DISTINCT and ORDER BY. It requires the ORDER BY columns in the select list for 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 9dee3172f4..3cdd46236b 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -12,8 +12,8 @@ module ActiveRecord spec[:unsigned] = 'true' if column.unsigned? return if spec.empty? else - spec[:id] = column.type.inspect - spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type, :null].include?(key) }) + spec[:id] = schema_type(column).inspect + spec.merge!(prepare_column_options(column).delete_if { |key, _| key == :null }) end spec end @@ -32,7 +32,7 @@ module ActiveRecord def schema_type(column) if column.sql_type == 'tinyblob' - 'blob' + :blob else super end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 23b33a3555..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,13 +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 - - alias exec_without_stmt exec_query - - 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/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 6aa264d766..6f2e03b370 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -118,7 +118,7 @@ module ActiveRecord alias :exec_update :exec_delete def sql_for_insert(sql, pk, id_value, sequence_name, binds) # :nodoc: - unless pk + if pk.nil? # Extract the table from the insert sql. Yuck. table_ref = extract_table_ref_from_insert_sql(sql) pk = primary_key(table_ref) if table_ref 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 cc7721ddd8..19761618cf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb @@ -11,8 +11,8 @@ module ActiveRecord spec[:id] = ':uuid' spec[:default] = schema_default(column) || 'nil' else - spec[:id] = column.type.inspect - spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type, :null].include?(key) }) + spec[:id] = schema_type(column).inspect + spec.merge!(prepare_column_options(column).delete_if { |key, _| key == :null }) end spec end @@ -35,9 +35,9 @@ module ActiveRecord return super unless column.serial? if column.bigint? - 'bigserial' + :bigserial else - 'serial' + :serial end 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/gem_version.rb b/activerecord/lib/active_record/gem_version.rb index aa1f5c4fb4..73be4cb271 100644 --- a/activerecord/lib/active_record/gem_version.rb +++ b/activerecord/lib/active_record/gem_version.rb @@ -8,7 +8,7 @@ module ActiveRecord MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end 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/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 45e35a4f71..09d55adcd7 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -102,7 +102,7 @@ module ActiveRecord module Legacy include FourTwoShared - def run(*) + def migrate(*) ActiveSupport::Deprecation.warn \ "Directly inheriting from ActiveRecord::Migration is deprecated. " \ "Please specify the Rails release the migration was written for:\n" \ diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index fa2522c2d2..fe68869143 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -195,15 +195,23 @@ module ActiveRecord # Nested attributes for an associated collection can also be passed in # the form of a hash of hashes instead of an array of hashes: # - # Member.create(name: 'joe', - # posts_attributes: { first: { title: 'Foo' }, - # second: { title: 'Bar' } }) + # Member.create( + # name: 'joe', + # posts_attributes: { + # first: { title: 'Foo' }, + # second: { title: 'Bar' } + # } + # ) # # has the same effect as # - # Member.create(name: 'joe', - # posts_attributes: [ { title: 'Foo' }, - # { title: 'Bar' } ]) + # Member.create( + # name: 'joe', + # posts_attributes: [ + # { title: 'Foo' }, + # { title: 'Bar' } + # ] + # ) # # The keys of the hash which is the value for +:posts_attributes+ are # ignored in this case. diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 0b500346bc..1ab4e0404f 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -1,7 +1,7 @@ module ActiveRecord module NullRelation # :nodoc: def exec_queries - @records = [] + @records = [].freeze end def pluck(*column_names) 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/querying.rb b/activerecord/lib/active_record/querying.rb index 1f429cfd94..de5b42e987 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -1,7 +1,7 @@ module ActiveRecord module Querying delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, to: :all - delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, 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_by, :find_by!, to: :all @@ -35,8 +35,8 @@ module ActiveRecord # # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }] - def find_by_sql(sql, binds = []) - result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds) + def find_by_sql(sql, binds = [], preparable: nil) + result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable) column_types = result_set.column_types.dup columns_hash.each_key { |k| column_types.delete k } message_bus = ActiveSupport::Notifications.instrumenter 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 99c0e71f97..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 @@ -239,6 +243,10 @@ module ActiveRecord def alias_candidate(name) "#{plural_name}_#{name}" end + + def chain + collect_join_chain + end end # Base class for AggregateReflection and AssociationReflection. Objects of @@ -421,7 +429,7 @@ module ActiveRecord # A chain of reflections from this one back to the owner. For more see the explanation in # ThroughReflection. - def chain + def collect_join_chain [self] end @@ -441,6 +449,10 @@ module ActiveRecord scope ? [[scope]] : [[]] end + def has_scope? + scope + end + def has_inverse? inverse_name end @@ -495,6 +507,18 @@ module ActiveRecord VALID_AUTOMATIC_INVERSE_MACROS = [:has_many, :has_one, :belongs_to] INVALID_AUTOMATIC_INVERSE_OPTIONS = [:conditions, :through, :polymorphic, :foreign_key] + def add_as_source(seed) + seed + end + + def add_as_polymorphic_through(reflection, seed) + seed + [PolymorphicReflection.new(self, reflection)] + end + + def add_as_through(seed) + seed + [self] + end + protected def actual_source_reflection # FIXME: this is a horrible name @@ -684,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 @@ -742,25 +770,13 @@ module ActiveRecord # # => [<ActiveRecord::Reflection::ThroughReflection: @delegate_reflection=#<ActiveRecord::Reflection::HasManyReflection: @name=:tags...>, # <ActiveRecord::Reflection::HasManyReflection: @name=:taggings, @options={}, @active_record=Post>] # - def chain - @chain ||= begin - a = source_reflection.chain - b = through_reflection.chain.map(&:dup) - - if options[:source_type] - b[0] = PolymorphicReflection.new(b[0], self) - end - - chain = a + b - chain[0] = self # Use self so we don't lose the information from :source_type - chain - end + def collect_join_chain + collect_join_reflections [self] end # 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 @@ -807,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 @@ -913,6 +935,27 @@ module ActiveRecord scope_chain end + def add_as_source(seed) + collect_join_reflections seed + end + + def add_as_polymorphic_through(reflection, seed) + collect_join_reflections(seed + [PolymorphicReflection.new(self, reflection)]) + end + + def add_as_through(seed) + collect_join_reflections(seed + [self]) + end + + def collect_join_reflections(seed) + a = source_reflection.add_as_source seed + if options[:source_type] + through_reflection.add_as_polymorphic_through self, a + else + through_reflection.add_as_through a + end + end + protected def actual_source_reflection # FIXME: this is a horrible name @@ -969,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.rb b/activerecord/lib/active_record/relation.rb index 7e842668c6..09afdc6c69 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -253,17 +253,21 @@ module ActiveRecord # Converts relation objects to Array. def to_a + records.dup + end + + def records # :nodoc: load @records end # Serializes the relation objects Array. def encode_with(coder) - coder.represent_seq(nil, to_a) + coder.represent_seq(nil, records) end def as_json(options = nil) #:nodoc: - to_a.as_json(options) + records.as_json(options) end # Returns size of the records. @@ -298,13 +302,13 @@ module ActiveRecord # Returns true if there is exactly one record. def one? return super if block_given? - limit_value ? to_a.one? : size == 1 + limit_value ? records.one? : size == 1 end # Returns true if there is more than one record. def many? return super if block_given? - limit_value ? to_a.many? : size > 1 + limit_value ? records.many? : size > 1 end # Returns a cache key that can be used to identify the records fetched by @@ -418,7 +422,7 @@ module ActiveRecord if id.is_a?(Array) id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) } elsif id == :all - to_a.each { |record| record.update(attributes) } + records.each { |record| record.update(attributes) } else if ActiveRecord::Base === id id = id.id @@ -457,7 +461,7 @@ module ActiveRecord MESSAGE where(conditions).destroy_all else - to_a.each(&:destroy).tap { reset } + records.each(&:destroy).tap { reset } end end @@ -587,7 +591,7 @@ module ActiveRecord def reset @last = @to_sql = @order_clause = @scope_for_create = @arel = @loaded = nil @should_eager_load = @join_dependency = nil - @records = [] + @records = [].freeze @offsets = {} self end @@ -654,21 +658,21 @@ module ActiveRecord def ==(other) case other when Associations::CollectionProxy, AssociationRelation - self == other.to_a + self == other.records when Relation other.to_sql == to_sql when Array - to_a == other + records == other end end def pretty_print(q) - q.pp(self.to_a) + q.pp(self.records) end # Returns true if relation is blank. def blank? - to_a.blank? + records.blank? end def values @@ -676,7 +680,7 @@ module ActiveRecord end def inspect - entries = to_a.take([limit_value, 11].compact.min).map!(&:inspect) + entries = records.take([limit_value, 11].compact.min).map!(&:inspect) entries[10] = '...' if entries.size == 11 "#<#{self.class.name} [#{entries.join(', ')}]>" @@ -685,14 +689,14 @@ module ActiveRecord protected def load_records(records) - @records = records + @records = records.freeze @loaded = true end private def exec_queries - @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bound_attributes) + @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes).freeze preload = preload_values preload += includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index de005e2810..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) @@ -187,7 +197,7 @@ module ActiveRecord loop do if load - records = batch_relation.to_a + records = batch_relation.records ids = records.map(&:id) yielded_relation = self.where(primary_key => ids) yielded_relation.load_records(records) @@ -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/batches/batch_enumerator.rb b/activerecord/lib/active_record/relation/batches/batch_enumerator.rb index c6e39814dd..13393dc605 100644 --- a/activerecord/lib/active_record/relation/batches/batch_enumerator.rb +++ b/activerecord/lib/active_record/relation/batches/batch_enumerator.rb @@ -35,7 +35,7 @@ module ActiveRecord return to_enum(:each_record) unless block_given? @relation.to_enum(:in_batches, of: @of, start: @start, finish: @finish, load: true).each do |relation| - relation.to_a.each { |record| yield record } + relation.records.each { |record| yield record } end end diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index e4e5d63006..f2578f5f96 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -37,7 +37,8 @@ module ActiveRecord # for each different klass, and the delegations are compiled into that subclass only. delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, - :[], :&, :|, :+, :-, :sample, :shuffle, :reverse, :compact, to: :to_a + :[], :&, :|, :+, :-, :sample, :reverse, :compact, :in_groups, :in_groups_of, + :shuffle, :split, to: :records delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :to => :klass diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index d48bcea28a..c3053f0b13 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -145,15 +145,21 @@ module ActiveRecord # # [#<Person id:4>, #<Person id:3>, #<Person id:2>] def last(limit = nil) - if limit - if order_values.empty? && primary_key - order(arel_attribute(primary_key).desc).limit(limit).reverse - else - to_a.last(limit) - end - else - find_last - end + return find_last(limit) if loaded? || limit_value + + result = limit(limit || 1) + result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key + result = result.reverse_order! + + limit ? result.reverse : result.first + rescue ActiveRecord::IrreversibleOrderError + ActiveSupport::Deprecation.warn(<<-WARNING.squish) + Finding a last element by loading the relation when SQL ORDER + can not be reversed is deprecated. + Rails 5.1 will raise ActiveRecord::IrreversibleOrderError in this case. + Please call `to_a.last` if you still want to load the relation. + WARNING + find_last(limit) end # Same as #last but raises ActiveRecord::RecordNotFound if no record @@ -242,6 +248,38 @@ module ActiveRecord find_nth! 41 end + # Find the third-to-last record. + # If no order is defined it will order by primary key. + # + # Person.third_to_last # returns the third-to-last object fetched by SELECT * FROM people + # 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_from_last 3 + end + + # Same as #third_to_last but raises ActiveRecord::RecordNotFound if no record + # is found. + def third_to_last! + 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. + # If no order is defined it will order by primary key. + # + # Person.second_to_last # returns the second-to-last object fetched by SELECT * FROM people + # 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_from_last 2 + end + + # Same as #second_to_last but raises ActiveRecord::RecordNotFound if no record + # is found. + def second_to_last! + 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 # conditions given, or false otherwise. The argument can take six forms: # @@ -298,7 +336,7 @@ module ActiveRecord end # This method is called whenever no records are found with either a single - # id or multiple ids and raises a ActiveRecord::RecordNotFound exception. + # id or multiple ids and raises an ActiveRecord::RecordNotFound exception. # # The error message is different depending on whether a single id or # multiple ids are provided. If multiple ids are provided, then the number @@ -468,7 +506,7 @@ module ActiveRecord def find_some_ordered(ids) ids = ids.slice(offset_value || 0, limit_value || ids.size) || [] - result = except(:limit, :offset).where(primary_key => ids).to_a + result = except(:limit, :offset).where(primary_key => ids).records if result.size == ids.size pk_type = @klass.type_for_attribute(primary_key) @@ -484,7 +522,7 @@ module ActiveRecord if loaded? @records.first else - @take ||= limit(1).to_a.first + @take ||= limit(1).records.first end end @@ -523,19 +561,25 @@ module ActiveRecord relation.limit(limit).to_a end - def find_last + def find_nth_from_last(index) if loaded? - @records.last + @records[-index] else - @last ||= - if limit_value - to_a.last - else - reverse_order.limit(1).to_a.first - end + 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: @@ -546,5 +590,9 @@ module ActiveRecord find_nth_with_limit(index, limit) end end + + def find_last(limit) + limit ? records.last(limit) : records.last + end end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 0f88791d92..953495a8b6 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -5,6 +5,7 @@ module ActiveRecord require 'active_record/relation/predicate_builder/base_handler' require 'active_record/relation/predicate_builder/basic_object_handler' require 'active_record/relation/predicate_builder/class_handler' + require 'active_record/relation/predicate_builder/polymorphic_array_handler' require 'active_record/relation/predicate_builder/range_handler' require 'active_record/relation/predicate_builder/relation_handler' @@ -22,6 +23,7 @@ module ActiveRecord register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) register_handler(AssociationQueryValue, AssociationQueryHandler.new(self)) + register_handler(PolymorphicArrayValue, PolymorphicArrayHandler.new(self)) end def build_from_hash(attributes) @@ -40,10 +42,7 @@ module ActiveRecord # # For polymorphic relationships, find the foreign key and type: # PriceEstimate.where(estimate_of: treasure) - if table.associated_with?(column) - value = AssociationQueryValue.new(table.associated_table(column), value) - end - + value = AssociationQueryHandler.value_for(table, column, value) if table.associated_with?(column) build(table.arel_attribute(column), value) end diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb index e81be63cd3..d7fd878265 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb @@ -1,6 +1,16 @@ module ActiveRecord class PredicateBuilder class AssociationQueryHandler # :nodoc: + def self.value_for(table, column, value) + klass = if table.associated_table(column).polymorphic_association? && ::Array === value && value.first.is_a?(Base) + PolymorphicArrayValue + else + AssociationQueryValue + end + + klass.new(table.associated_table(column), value) + end + def initialize(predicate_builder) @predicate_builder = predicate_builder end diff --git a/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb new file mode 100644 index 0000000000..b6c6240343 --- /dev/null +++ b/activerecord/lib/active_record/relation/predicate_builder/polymorphic_array_handler.rb @@ -0,0 +1,57 @@ +module ActiveRecord + class PredicateBuilder + class PolymorphicArrayHandler # :nodoc: + def initialize(predicate_builder) + @predicate_builder = predicate_builder + end + + def call(attribute, value) + table = value.associated_table + queries = value.type_to_ids_mapping.map do |type, ids| + { table.association_foreign_type.to_s => type, table.association_foreign_key.to_s => ids } + end + + predicates = queries.map { |query| predicate_builder.build_from_hash(query) } + + if predicates.size > 1 + type_and_ids_predicates = predicates.map { |type_predicate, id_predicate| Arel::Nodes::Grouping.new(type_predicate.and(id_predicate)) } + type_and_ids_predicates.inject(&:or) + else + predicates.first + end + end + + protected + + attr_reader :predicate_builder + end + + class PolymorphicArrayValue # :nodoc: + attr_reader :associated_table, :values + + def initialize(associated_table, values) + @associated_table = associated_table + @values = values + end + + def type_to_ids_mapping + default_hash = Hash.new { |hsh, key| hsh[key] = [] } + values.each_with_object(default_hash) { |value, hash| hash[base_class(value).name] << convert_to_id(value) } + end + + private + + def primary_key(value) + associated_table.association_primary_key(base_class(value)) + end + + def base_class(value) + value.class.base_class + end + + def convert_to_id(value) + value._read_attribute(primary_key(value)) + end + end + end +end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 91bfa4d131..4533f3263f 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -655,6 +655,10 @@ module ActiveRecord # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'author_id = 3')) # def or(other) + unless other.is_a? Relation + raise ArgumentError, "You have passed #{other.class.name} object to #or. Pass an ActiveRecord::Relation object instead." + end + spawn.or!(other) end @@ -1112,6 +1116,8 @@ module ActiveRecord order_query.flat_map do |o| case o + when Arel::Attribute + o.desc when Arel::Nodes::Ordering o.reverse when String diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 67d7f83cb4..d5c18a2a4a 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -29,7 +29,7 @@ module ActiveRecord # This is mainly intended for sharing common conditions between multiple associations. def merge(other) if other.is_a?(Array) - to_a & other + records & other elsif other spawn.merge!(other) else diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 2bfc5ff7ae..a9e1fd0dad 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -60,7 +60,7 @@ module ActiveRecord end # Accepts an array, or string of SQL conditions and sanitizes - # them into a valid SQL fragment for a ORDER clause. + # them into a valid SQL fragment for an ORDER clause. # # sanitize_sql_for_order(["field(id, ?)", [1,3,2]]) # # => "field(id, 1,3,2)" diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index f6b0efb88a..6c896ccea6 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -106,7 +106,7 @@ module ActiveRecord sql = query_builder.sql_for bind_values, connection - klass.find_by_sql sql, bind_values + klass.find_by_sql(sql, bind_values, preparable: true) end alias :call :execute end diff --git a/activerecord/lib/active_record/suppressor.rb b/activerecord/lib/active_record/suppressor.rb index b3644bf569..8ec4b48d31 100644 --- a/activerecord/lib/active_record/suppressor.rb +++ b/activerecord/lib/active_record/suppressor.rb @@ -37,7 +37,11 @@ module ActiveRecord end end - def create_or_update(*args) # :nodoc: + def save(*) # :nodoc: + SuppressorRegistry.suppressed[self.class.name] ? true : super + end + + def save!(*) # :nodoc: SuppressorRegistry.suppressed[self.class.name] ? true : super end end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 8f52e9068a..7dc41fa98c 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -116,7 +116,11 @@ module ActiveRecord end def create_all + old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base) each_local_configuration { |configuration| create configuration } + if old_pool + ActiveRecord::Base.connection_handler.establish_connection(ActiveRecord::Base, old_pool.spec) + end end def create_current(environment = env) 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/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 6677e6dc5f..ecaf04e39e 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -45,7 +45,7 @@ module ActiveRecord end # Attempts to save the record just like {ActiveRecord::Base#save}[rdoc-ref:Base#save] but - # will raise a ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid. + # will raise an ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid. def save!(options={}) perform_validations(options) ? super : raise_validation_error end diff --git a/activerecord/lib/active_record/validations/absence.rb b/activerecord/lib/active_record/validations/absence.rb index 2e19e6dc5c..641d041f3d 100644 --- a/activerecord/lib/active_record/validations/absence.rb +++ b/activerecord/lib/active_record/validations/absence.rb @@ -2,7 +2,6 @@ module ActiveRecord module Validations class AbsenceValidator < ActiveModel::Validations::AbsenceValidator # :nodoc: def validate_each(record, attribute, association_or_value) - return unless should_validate?(record) if record.class._reflect_on_association(attribute) association_or_value = Array.wrap(association_or_value).reject(&:marked_for_destruction?) end diff --git a/activerecord/lib/active_record/validations/length.rb b/activerecord/lib/active_record/validations/length.rb index 69e048eef1..0e0cebce4a 100644 --- a/activerecord/lib/active_record/validations/length.rb +++ b/activerecord/lib/active_record/validations/length.rb @@ -2,23 +2,11 @@ module ActiveRecord module Validations class LengthValidator < ActiveModel::Validations::LengthValidator # :nodoc: def validate_each(record, attribute, association_or_value) - return unless should_validate?(record) || associations_are_dirty?(record) if association_or_value.respond_to?(:loaded?) && association_or_value.loaded? association_or_value = association_or_value.target.reject(&:marked_for_destruction?) end super end - - def associations_are_dirty?(record) - attributes.any? do |attribute| - value = record.read_attribute_for_validation(attribute) - if value.respond_to?(:loaded?) && value.loaded? - value.target.any?(&:marked_for_destruction?) - else - false - end - end - end end module ClassMethods diff --git a/activerecord/lib/active_record/validations/presence.rb b/activerecord/lib/active_record/validations/presence.rb index 7e85ed43ac..ad82ea66c4 100644 --- a/activerecord/lib/active_record/validations/presence.rb +++ b/activerecord/lib/active_record/validations/presence.rb @@ -2,7 +2,6 @@ module ActiveRecord module Validations class PresenceValidator < ActiveModel::Validations::PresenceValidator # :nodoc: def validate_each(record, attribute, association_or_value) - return unless should_validate?(record) if record.class._reflect_on_association(attribute) association_or_value = Array.wrap(association_or_value).reject(&:marked_for_destruction?) end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index a376e2a17f..4a80cda0b8 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -11,15 +11,14 @@ module ActiveRecord end def validate_each(record, attribute, value) - return unless should_validate?(record) finder_class = find_finder_class_for(record) table = finder_class.arel_table value = map_enum_attribute(finder_class, attribute, value) relation = build_relation(finder_class, table, attribute, value) - if record.persisted? && finder_class.primary_key.to_s != attribute.to_s + if record.persisted? if finder_class.primary_key - relation = relation.where.not(finder_class.primary_key => record.id) + relation = relation.where.not(finder_class.primary_key => record.id_was || record.id) else raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") end @@ -57,14 +56,13 @@ module ActiveRecord value = value.attributes[reflection.klass.primary_key] unless value.nil? end - attribute_name = attribute.to_s - # the attribute may be an aliased attribute - if klass.attribute_aliases[attribute_name] - attribute = klass.attribute_aliases[attribute_name] - attribute_name = attribute.to_s + if klass.attribute_alias?(attribute) + attribute = klass.attribute_alias(attribute) end + attribute_name = attribute.to_s + column = klass.columns_hash[attribute_name] cast_type = klass.type_for_attribute(attribute_name) value = cast_type.serialize(value) @@ -82,7 +80,7 @@ module ActiveRecord if value.nil? klass.unscoped.where(comparison) else - bind = Relation::QueryAttribute.new(attribute.to_s, value, Type::Value.new) + bind = Relation::QueryAttribute.new(attribute_name, value, Type::Value.new) klass.unscoped.where(comparison, bind) end rescue RangeError diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb index 7395839fca..f191eff5bf 100644 --- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb +++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb @@ -22,11 +22,13 @@ module ActiveRecord def create_model_file template 'model.rb', File.join('app/models', class_path, "#{file_name}.rb") + generate_application_record end def create_module_file return if regular_class_path.empty? template 'module.rb', File.join('app/models', "#{class_path.join('/')}.rb") if behavior == :invoke + generate_application_record end hook_for :test_framework @@ -37,23 +39,34 @@ module ActiveRecord attributes.select { |a| !a.reference? && a.has_index? } end + # FIXME: Change this file to a symlink once RubyGems 2.5.0 is required. + def generate_application_record + if self.behavior == :invoke && !application_record_exist? + template 'application_record.rb', application_record_file_name + end + end + # Used by the migration template to determine the parent name of the model def parent_class_name options[:parent] || determine_default_parent_class end - def determine_default_parent_class - application_record = nil + def application_record_exist? + file_exist = nil + in_root { file_exist = File.exist?(application_record_file_name) } + file_exist + end - in_root do - application_record = if mountable_engine? - File.exist?("app/models/#{namespaced_path}/application_record.rb") - else - File.exist?('app/models/application_record.rb') - end + def application_record_file_name + @application_record_file_name ||= if mountable_engine? + "app/models/#{namespaced_path}/application_record.rb" + else + 'app/models/application_record.rb' end + end - if application_record + def determine_default_parent_class + if application_record_exist? "ApplicationRecord" else "ActiveRecord::Base" diff --git a/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb b/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb new file mode 100644 index 0000000000..60050e0bf8 --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb @@ -0,0 +1,5 @@ +<% module_namespacing do -%> +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end +<% end -%> 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/prepared_statements_test.rb b/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb new file mode 100644 index 0000000000..f1519db48b --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb @@ -0,0 +1,22 @@ +require "cases/helper" +require "models/developer" + +class PreparedStatementsTest < ActiveRecord::PostgreSQLTestCase + fixtures :developers + + def setup + @default_prepared_statements = Developer.connection_config[:prepared_statements] + Developer.connection_config[:prepared_statements] = false + end + + def teardown + Developer.connection_config[:prepared_statements] = @default_prepared_statements + end + + def nothing_raised_with_falsy_prepared_statements + assert_nothing_raised do + Developer.where(id: 1) + 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 874d53c51f..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 @@ -1216,7 +1248,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_join_eager_with_empty_order_should_generate_valid_sql - assert_nothing_raised(ActiveRecord::StatementInvalid) do + assert_nothing_raised do Post.includes(:comments).order("").where(:comments => {:body => "Thank you for the welcome"}).first end end @@ -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/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 5c4586da19..1bbca84bb2 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -146,6 +146,19 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, country.treaties.count end + def test_join_table_composite_primary_key_should_not_warn + country = Country.new(:name => 'India') + country.country_id = 'c1' + country.save! + + treaty = Treaty.new(:name => 'peace') + treaty.treaty_id = 't1' + warning = capture(:stderr) do + country.treaties << treaty + end + assert_no_match(/WARNING: Rails does not support composite primary key\./, warning) + end + def test_has_and_belongs_to_many david = Developer.find(1) @@ -925,7 +938,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_with_symbol_class_name - assert_nothing_raised NoMethodError do + assert_nothing_raised do DeveloperWithSymbolClassName.new end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ecaa521283..e975f4fbdd 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -408,6 +408,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end assert_no_queries do + bulbs.third_to_last() + bulbs.third_to_last({}) + end + + assert_no_queries do + bulbs.second_to_last() + bulbs.second_to_last({}) + end + + assert_no_queries do bulbs.last() bulbs.last({}) end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 57d1c8feda..c9743e80d3 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -130,15 +130,15 @@ end class InverseAssociationTests < ActiveRecord::TestCase def test_should_allow_for_inverse_of_options_in_associations - assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_many') do + assert_nothing_raised do Class.new(ActiveRecord::Base).has_many(:wheels, :inverse_of => :car) end - assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_one') do + assert_nothing_raised do Class.new(ActiveRecord::Base).has_one(:engine, :inverse_of => :car) end - assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on belongs_to') do + assert_nothing_raised do Class.new(ActiveRecord::Base).belongs_to(:car, :inverse_of => :driver) end end @@ -666,7 +666,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase def test_trying_to_access_inverses_that_dont_exist_shouldnt_raise_an_error # Ideally this would, if only for symmetry's sake with other association types - assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.horrible_polymorphic_man } + assert_nothing_raised { Face.first.horrible_polymorphic_man } end def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_an_error @@ -676,7 +676,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase def test_trying_to_set_polymorphic_inverses_that_dont_exist_on_the_instance_being_set_should_raise_an_error # passes because Man does have the correct inverse_of - assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Man.first } + assert_nothing_raised { Face.first.polymorphic_man = Man.first } # fails because Interest does have the correct inverse_of assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Interest.first } end @@ -688,7 +688,7 @@ class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase fixtures :men, :interests, :zines def test_that_we_can_load_associations_that_have_the_same_reciprocal_name_from_different_models - assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do + assert_nothing_raised do i = Interest.first i.zine i.man @@ -696,7 +696,7 @@ class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase end def test_that_we_can_create_associations_that_have_the_same_reciprocal_name_from_different_models - assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do + assert_nothing_raised do i = Interest.first i.build_zine(:title => 'Get Some in Winter! 2008') i.build_man(:name => 'Gordon') diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index f6dddaf5b4..1d892a0956 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -88,7 +88,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_polymorphic_has_many_going_through_join_model_with_custom_select_and_joins assert_equal tags(:general), tag = posts(:welcome).tags.add_joins_and_select.first - assert_nothing_raised(NoMethodError) { tag.author_id } + assert_nothing_raised { tag.author_id } end def test_polymorphic_has_many_going_through_join_model_with_custom_foreign_key @@ -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 3602ee7ba2..91ff5146fd 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -108,7 +108,7 @@ class EachTest < ActiveRecord::TestCase end end - def test_find_in_batches_should_finish_the_end_option + def test_find_in_batches_should_end_at_the_finish_option assert_queries(6) do Post.find_in_batches(batch_size: 1, finish: 5) do |batch| assert_kind_of Array, batch @@ -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 = [] @@ -316,7 +352,7 @@ class EachTest < ActiveRecord::TestCase end end - def test_in_batches_should_finish_the_end_option + def test_in_batches_should_end_at_the_finish_option post = Post.order('id DESC').where('id <= ?', 5).first assert_queries(7) do relation = Post.in_batches(of: 1, finish: 5, load: true).reverse_each.first 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/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index c922a8d1c2..8f2682c781 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -124,7 +124,7 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_generate_valid_sql_with_joins_and_group - assert_nothing_raised ActiveRecord::StatementInvalid do + assert_nothing_raised do AuditLog.joins(:developer).group(:id).count end end @@ -742,7 +742,7 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association - assert_nothing_raised ActiveRecord::StatementInvalid do + assert_nothing_raised do developer = Developer.create!(name: 'developer') developer.ratings.includes(comment: :post).where(posts: { id: 1 }).count end 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/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index 922cb59280..66b4c3f1ff 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -151,7 +151,7 @@ class CounterCacheTest < ActiveRecord::TestCase test "reset the right counter if two have the same foreign key" do michael = people(:michael) - assert_nothing_raised(ActiveRecord::StatementInvalid) do + assert_nothing_raised do Person.reset_counters(michael.id, :friends_too) 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 75a74c052d..692c6bf2d0 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -43,7 +43,7 @@ class FinderTest < ActiveRecord::TestCase end assert_equal "should happen", exception.message - assert_nothing_raised(RuntimeError) do + assert_nothing_raised do Topic.all.find(-> { raise "should not happen" }) { |e| e.title == topics(:first).title } end end @@ -101,7 +101,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_with_ids_where_and_limit # Please note that Topic 1 is the only not approved so - # if it were among the first 3 it would raise a ActiveRecord::RecordNotFound + # if it were among the first 3 it would raise an ActiveRecord::RecordNotFound records = Topic.where(approved: true).limit(3).find([3,2,5,1,4]) assert_equal 3, records.size assert_equal 'The Third Topic of the day', records[0].title @@ -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! @@ -516,16 +576,44 @@ class FinderTest < ActiveRecord::TestCase assert_equal Topic.order("title").to_a.last(2), Topic.order("title").last(2) end - def test_last_with_integer_and_order_should_not_use_sql_limit - query = assert_sql { Topic.order("title").last(5).entries } - assert_equal 1, query.length - assert_no_match(/LIMIT/, query.first) + def test_last_with_integer_and_order_should_use_sql_limit + relation = Topic.order("title") + assert_queries(1) { relation.last(5) } + assert !relation.loaded? + end + + def test_last_with_integer_and_reorder_should_use_sql_limit + relation = Topic.reorder("title") + assert_queries(1) { relation.last(5) } + assert !relation.loaded? end - def test_last_with_integer_and_reorder_should_not_use_sql_limit - query = assert_sql { Topic.reorder("title").last(5).entries } - assert_equal 1, query.length - assert_no_match(/LIMIT/, query.first) + def test_last_on_loaded_relation_should_not_use_sql + relation = Topic.limit(10).load + assert_no_queries do + relation.last + relation.last(2) + end + end + + def test_last_with_irreversible_order + assert_deprecated do + Topic.order("coalesce(author_name, title)").last + end + end + + def test_last_on_relation_with_limit_and_offset + post = posts('sti_comments') + + comments = post.comments.order(id: :asc) + assert_equal comments.limit(2).to_a.last, comments.limit(2).last + assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2) + assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) + + comments = comments.offset(1) + assert_equal comments.limit(2).to_a.last, comments.limit(2).last + assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2) + assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) end def test_take_and_first_and_last_with_integer_should_return_an_array @@ -1058,7 +1146,7 @@ class FinderTest < ActiveRecord::TestCase end def test_finder_with_offset_string - assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.offset("3").to_a } + assert_nothing_raised { Topic.offset("3").to_a } end test "find_by with hash conditions returns the first matching record" do 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/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 6d5b6243db..60ca90464d 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -21,7 +21,7 @@ module ActiveRecord teardown do connection.drop_table :testings rescue nil ActiveRecord::Migration.verbose = @verbose_was - ActiveRecord::SchemaMigration.delete_all + ActiveRecord::SchemaMigration.delete_all rescue nil end def test_migration_doesnt_remove_named_index @@ -101,6 +101,18 @@ module ActiveRecord assert connection.columns(:testings).find { |c| c.name == 'created_at' }.null assert connection.columns(:testings).find { |c| c.name == 'updated_at' }.null end + + def test_legacy_migrations_get_deprecation_warning_when_run + migration = Class.new(ActiveRecord::Migration) { + def up + add_column :testings, :baz, :string + end + } + + assert_deprecated do + migration.migrate :up + end + end end end end diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index b01415afb2..85435f4dbc 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -144,6 +144,22 @@ module ActiveRecord @connection.drop_table "testing", if_exists: true end end + + test "multiple foreign keys can be added to the same table" do + @connection.create_table :testings do |t| + t.integer :col_1 + t.integer :col_2 + + t.foreign_key :testing_parents, column: :col_1 + t.foreign_key :testing_parents, column: :col_2 + end + + fks = @connection.foreign_keys("testings") + + fk_definitions = fks.map {|fk| [fk.from_table, fk.to_table, fk.column] } + assert_equal([["testings", "testing_parents", "col_1"], + ["testings", "testing_parents", "col_2"]], fk_definitions) + end end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index bae0467e72..6a6250eec3 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -516,13 +516,12 @@ class MigrationTest < ActiveRecord::TestCase data_column = columns.detect { |c| c.name == "data" } assert_nil data_column.default - + ensure Person.connection.drop_table :binary_testings, if_exists: true end unless mysql_enforcing_gtid_consistency? def test_create_table_with_query - Person.connection.drop_table :table_from_query_testings rescue nil Person.connection.create_table(:person, force: true) Person.connection.create_table :table_from_query_testings, as: "SELECT id FROM person" @@ -530,12 +529,11 @@ class MigrationTest < ActiveRecord::TestCase columns = Person.connection.columns(:table_from_query_testings) assert_equal 1, columns.length assert_equal "id", columns.first.name - + ensure Person.connection.drop_table :table_from_query_testings rescue nil end def test_create_table_with_query_from_relation - Person.connection.drop_table :table_from_query_testings rescue nil Person.connection.create_table(:person, force: true) Person.connection.create_table :table_from_query_testings, as: Person.select(:id) @@ -543,7 +541,7 @@ class MigrationTest < ActiveRecord::TestCase columns = Person.connection.columns(:table_from_query_testings) assert_equal 1, columns.length assert_equal "id", columns.first.name - + ensure Person.connection.drop_table :table_from_query_testings rescue nil end end @@ -586,8 +584,7 @@ class MigrationTest < ActiveRecord::TestCase end if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) - def test_out_of_range_limit_should_raise - Person.connection.drop_table :test_limits rescue nil + def test_out_of_range_integer_limit_should_raise e = assert_raise(ActiveRecord::ActiveRecordError, "integer limit didn't raise") do Person.connection.create_table :test_integer_limits, :force => true do |t| t.column :bigone, :integer, :limit => 10 @@ -595,16 +592,22 @@ class MigrationTest < ActiveRecord::TestCase end assert_match(/No integer type has byte size 10/, e.message) + ensure + Person.connection.drop_table :test_integer_limits, if_exists: true + end + end - unless current_adapter?(:PostgreSQLAdapter) - assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do - Person.connection.create_table :test_text_limits, :force => true do |t| - t.column :bigtext, :text, :limit => 0xfffffffff - end + if current_adapter?(:Mysql2Adapter) + def test_out_of_range_text_limit_should_raise + e = assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do + Person.connection.create_table :test_text_limits, force: true do |t| + t.text :bigtext, limit: 0xfffffffff end end - Person.connection.drop_table :test_limits rescue nil + assert_match(/No text type has byte length #{0xfffffffff}/, e.message) + ensure + Person.connection.drop_table :test_text_limits, if_exists: true end end @@ -726,7 +729,7 @@ class ReservedWordsMigrationTest < ActiveRecord::TestCase connection.add_index :values, :value connection.remove_index :values, :column => :value end - + ensure connection.drop_table :values rescue nil end end @@ -738,11 +741,11 @@ class ExplicitlyNamedIndexMigrationTest < ActiveRecord::TestCase t.integer :value end - assert_nothing_raised ArgumentError do + assert_nothing_raised do connection.add_index :values, :value, name: 'a_different_name' connection.remove_index :values, column: :value, name: 'a_different_name' end - + ensure connection.drop_table :values rescue nil end end diff --git a/activerecord/test/cases/modules_test.rb b/activerecord/test/cases/modules_test.rb index 7f31325f47..486bcc22df 100644 --- a/activerecord/test/cases/modules_test.rb +++ b/activerecord/test/cases/modules_test.rb @@ -63,7 +63,7 @@ class ModulesTest < ActiveRecord::TestCase def test_assign_ids firm = MyApplication::Business::Firm.first - assert_nothing_raised NameError, "Should be able to resolve all class constants via reflection" do + assert_nothing_raised do firm.client_ids = [MyApplication::Business::Client.first.id] end end @@ -72,7 +72,7 @@ class ModulesTest < ActiveRecord::TestCase def test_eager_loading_in_modules clients = [] - assert_nothing_raised NameError, "Should be able to resolve all class constants via reflection" do + assert_nothing_raised do clients << MyApplication::Business::Client.references(:accounts).merge!(:includes => {:firm => :account}, :where => 'accounts.id IS NOT NULL').find(3) clients << MyApplication::Business::Client.includes(:firm => :account).find(3) end diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb index 39cdcf5403..af4183a601 100644 --- a/activerecord/test/cases/multiple_db_test.rb +++ b/activerecord/test/cases/multiple_db_test.rb @@ -104,7 +104,7 @@ class MultipleDbTest < ActiveRecord::TestCase def test_associations_should_work_when_model_has_no_connection begin ActiveRecord::Base.remove_connection - assert_nothing_raised ActiveRecord::ConnectionNotEstablished do + assert_nothing_raised do College.first.courses.first end ensure diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 6fbc6196cc..11fb164d50 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -61,6 +61,13 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_equal "No association found for name `honesty'. Has it been defined yet?", exception.message end + def test_should_raise_an_UnknownAttributeError_for_non_existing_nested_attributes + exception = assert_raise ActiveModel::UnknownAttributeError do + Pirate.new(:ship_attributes => { :sail => true }) + end + assert_equal "unknown attribute 'sail' for Ship.", exception.message + end + def test_should_disable_allow_destroy_by_default Pirate.accepts_nested_attributes_for :ship @@ -69,7 +76,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase pirate.update(ship_attributes: { '_destroy' => true, :id => ship.id }) - assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.ship.reload } + assert_nothing_raised { pirate.ship.reload } end def test_a_model_should_respond_to_underscore_destroy_and_return_if_it_is_marked_for_destruction @@ -173,7 +180,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase pirate = Pirate.new(:catchphrase => "Stop wastin' me time") pirate.ship_attributes = { :id => "" } - assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.save! } + assert_nothing_raised { pirate.save! } end def test_first_and_array_index_zero_methods_return_the_same_value_when_nested_attributes_are_set_to_update_existing_record @@ -504,7 +511,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy [nil, '0', 0, 'false', false].each do |not_truth| @ship.update(pirate_attributes: { id: @ship.pirate.id, _destroy: not_truth }) - assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload } + assert_nothing_raised { @ship.pirate.reload } end end @@ -512,7 +519,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase Ship.accepts_nested_attributes_for :pirate, :allow_destroy => false, :reject_if => proc(&:empty?) @ship.update(pirate_attributes: { id: @ship.pirate.id, _destroy: '1' }) - assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload } + assert_nothing_raised { @ship.pirate.reload } ensure Ship.accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc(&:empty?) end @@ -529,7 +536,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase pirate = @ship.pirate @ship.attributes = { :pirate_attributes => { :id => pirate.id, '_destroy' => true } } - assert_nothing_raised(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) } + assert_nothing_raised { Pirate.find(pirate.id) } @ship.save assert_raise(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) } end @@ -582,6 +589,13 @@ module NestedAttributesOnACollectionAssociationTests assert_respond_to @pirate, association_setter end + def test_should_raise_an_UnknownAttributeError_for_non_existing_nested_attributes_for_has_many + exception = assert_raise ActiveModel::UnknownAttributeError do + @pirate.parrots_attributes = [{ peg_leg: true }] + end + assert_equal "unknown attribute 'peg_leg' for Parrot.", exception.message + end + def test_should_save_only_one_association_on_create pirate = Pirate.create!({ :catchphrase => 'Arr', @@ -699,7 +713,7 @@ module NestedAttributesOnACollectionAssociationTests end def test_should_not_assign_destroy_key_to_a_record - assert_nothing_raised ActiveRecord::UnknownAttributeError do + assert_nothing_raised do @pirate.send(association_setter, { 'foo' => { '_destroy' => '0' }}) end end @@ -734,8 +748,8 @@ module NestedAttributesOnACollectionAssociationTests end def test_should_raise_an_argument_error_if_something_else_than_a_hash_is_passed - assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, {}) } - assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, Hash.new) } + assert_nothing_raised { @pirate.send(association_setter, {}) } + assert_nothing_raised { @pirate.send(association_setter, Hash.new) } exception = assert_raise ArgumentError do @pirate.send(association_setter, "foo") @@ -810,7 +824,7 @@ module NestedAttributesOnACollectionAssociationTests def test_can_use_symbols_as_object_identifier @pirate.attributes = { :parrots_attributes => { :foo => { :name => 'Lovely Day' }, :bar => { :name => 'Blown Away' } } } - assert_nothing_raised(NoMethodError) { @pirate.save! } + assert_nothing_raised { @pirate.save! } end def test_numeric_column_changes_from_zero_to_no_empty_string diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index b918b36b94..6aaee8de7c 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -130,7 +130,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase end def test_supports_primary_key - assert_nothing_raised NoMethodError do + assert_nothing_raised do ActiveRecord::Base.connection.supports_primary_key? end end @@ -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/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb index 28a0862f91..ce8c5ca489 100644 --- a/activerecord/test/cases/relation/or_test.rb +++ b/activerecord/test/cases/relation/or_test.rb @@ -82,5 +82,11 @@ module ActiveRecord assert_equal p.loaded?, true assert_equal expected, p.or(Post.where('id = 2')).to_a end + + def test_or_with_non_relation_object_raises_error + assert_raises ArgumentError do + Post.where(id: [1, 2, 3]).or(title: 'Rails') + end + end end end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index bc6378b90e..56a2b5b8c6 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require "models/author" require "models/binary" require "models/cake_designer" +require "models/car" require "models/chef" require "models/comment" require "models/edge" @@ -14,7 +15,7 @@ require "models/vertex" module ActiveRecord class WhereTest < ActiveRecord::TestCase - fixtures :posts, :edges, :authors, :binaries, :essays + fixtures :posts, :edges, :authors, :binaries, :essays, :cars, :treasures, :price_estimates def test_where_copies_bind_params author = authors(:david) @@ -114,6 +115,17 @@ module ActiveRecord assert_equal expected.to_sql, actual.to_sql end + def test_polymorphic_array_where_multiple_types + treasure_1 = treasures(:diamond) + treasure_2 = treasures(:sapphire) + car = cars(:honda) + + expected = [price_estimates(:diamond), price_estimates(:sapphire_1), price_estimates(:sapphire_2), price_estimates(:honda)].sort + actual = PriceEstimate.where(estimate_of: [treasure_1, treasure_2, car]).to_a.sort + + assert_equal expected, actual + end + def test_polymorphic_nested_relation_where expected = PriceEstimate.where(estimate_of_type: 'Treasure', estimate_of_id: Treasure.where(id: [1,2])) actual = PriceEstimate.where(estimate_of: Treasure.where(id: [1,2])) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 090b885dd5..95e4230a58 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -358,6 +358,12 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_sanitized_order query = Tag.order(["field(id, ?)", [1,3,2]]).to_sql assert_match(/field\(id, 1,3,2\)/, query) + + query = Tag.order(["field(id, ?)", []]).to_sql + assert_match(/field\(id, NULL\)/, query) + + query = Tag.order(["field(id, ?)", nil]).to_sql + assert_match(/field\(id, NULL\)/, query) end def test_finding_with_order_limit_and_offset @@ -1273,6 +1279,16 @@ class RelationTest < ActiveRecord::TestCase assert posts.loaded? end + def test_to_a_should_dup_target + posts = Post.all + + original_size = posts.size + removed = posts.to_a.pop + + assert_equal original_size, posts.size + assert_includes posts.to_a, removed + end + def test_build posts = Post.all diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index ad5ca70f36..c918cbdef5 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -374,6 +374,18 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal 10, DeveloperCalledJamis.unscoped { DeveloperCalledJamis.poor }.length end + def test_default_scope_with_joins + assert_equal Comment.where(post_id: SpecialPostWithDefaultScope.pluck(:id)).count, + Comment.joins(:special_post_with_default_scope).count + assert_equal Comment.where(post_id: Post.pluck(:id)).count, + Comment.joins(:post).count + end + + def test_unscoped_with_joins_should_not_have_default_scope + assert_equal SpecialPostWithDefaultScope.unscoped { Comment.joins(:special_post_with_default_scope).to_a }, + Comment.joins(:post).to_a + end + def test_default_scope_select_ignored_by_aggregations assert_equal DeveloperWithSelect.all.to_a.count, DeveloperWithSelect.count end diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index ab63f5825c..bce86875e1 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -177,6 +177,7 @@ class StoreTest < ActiveRecord::TestCase assert_equal [:color], first_model.stored_attributes[:data] assert_equal [:color, :width, :height], second_model.stored_attributes[:data] assert_equal [:color, :area, :volume], third_model.stored_attributes[:data] + assert_equal [:color], first_model.stored_attributes[:data] end test "YAML coder initializes the store when a Nil value is given" do diff --git a/activerecord/test/cases/suppressor_test.rb b/activerecord/test/cases/suppressor_test.rb index 72c5c16555..7d44e36419 100644 --- a/activerecord/test/cases/suppressor_test.rb +++ b/activerecord/test/cases/suppressor_test.rb @@ -46,7 +46,18 @@ class SuppressorTest < ActiveRecord::TestCase Notification.suppress { UserWithNotification.create! } assert_difference -> { Notification.count } do - Notification.create! + Notification.create!(message: "New Comment") + end + end + + def test_suppresses_validations_on_create + assert_no_difference -> { Notification.count } do + Notification.suppress do + User.create + User.create! + User.new.save + User.new.save! + end end end end diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 970f6bcf4a..937b84bccc 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -98,8 +98,11 @@ class TimestampTest < ActiveRecord::TestCase task = Task.first previous_value = task.ending task.touch(:ending) + + now = Time.now.change(usec: 0) + assert_not_equal previous_value, task.ending - assert_in_delta Time.now, task.ending, 1 + assert_in_delta now, task.ending, 1 end def test_touching_an_attribute_updates_timestamp_with_given_time @@ -120,10 +123,12 @@ class TimestampTest < ActiveRecord::TestCase previous_ending = task.ending task.touch(:starting, :ending) + now = Time.now.change(usec: 0) + assert_not_equal previous_starting, task.starting assert_not_equal previous_ending, task.ending - assert_in_delta Time.now, task.starting, 1 - assert_in_delta Time.now, task.ending, 1 + assert_in_delta now, task.starting, 1 + assert_in_delta now, task.ending, 1 end def test_touching_a_record_without_timestamps_is_unexceptional diff --git a/activerecord/test/cases/validations/absence_validation_test.rb b/activerecord/test/cases/validations/absence_validation_test.rb index dd43ee358c..c0b3750bcc 100644 --- a/activerecord/test/cases/validations/absence_validation_test.rb +++ b/activerecord/test/cases/validations/absence_validation_test.rb @@ -57,19 +57,17 @@ class AbsenceValidationTest < ActiveRecord::TestCase assert_nothing_raised { boy_klass.new(face: face_with_to_a).valid? } end - def test_does_not_validate_if_parent_record_is_validate_false + def test_validates_absence_of_virtual_attribute_on_model repair_validations(Interest) do - Interest.validates_absence_of(:topic) - interest = Interest.new(topic: Topic.new(title: "Math")) - interest.save!(validate: false) - assert interest.persisted? + Interest.send(:attr_accessor, :token) + Interest.validates_absence_of(:token) - man = Man.new(interest_ids: [interest.id]) - man.save! - - assert_equal man.interests.size, 1 + interest = Interest.create!(topic: 'Thought Leadering') assert interest.valid? - assert man.valid? + + interest.token = 'tl' + + assert interest.invalid? end end end 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/length_validation_test.rb b/activerecord/test/cases/validations/length_validation_test.rb index c5d8f8895c..78263fd955 100644 --- a/activerecord/test/cases/validations/length_validation_test.rb +++ b/activerecord/test/cases/validations/length_validation_test.rb @@ -61,17 +61,19 @@ class LengthValidationTest < ActiveRecord::TestCase assert_equal pet_count, Pet.count end - def test_does_not_validate_length_of_if_parent_record_is_validate_false - @owner.validates_length_of :name, minimum: 1 - owner = @owner.new - owner.save!(validate: false) - assert owner.persisted? + def test_validates_length_of_virtual_attribute_on_model + repair_validations(Pet) do + Pet.send(:attr_accessor, :nickname) + Pet.validates_length_of(:name, minimum: 1) + Pet.validates_length_of(:nickname, minimum: 1) - pet = Pet.new(owner_id: owner.id) - pet.save! + pet = Pet.create!(name: 'Fancy Pants', nickname: 'Fancy') - assert_equal owner.pets.size, 1 - assert owner.valid? - assert pet.valid? + assert pet.valid? + + pet.nickname = '' + + assert pet.invalid? + end end end diff --git a/activerecord/test/cases/validations/presence_validation_test.rb b/activerecord/test/cases/validations/presence_validation_test.rb index 6f8ad06ab6..868d111b8c 100644 --- a/activerecord/test/cases/validations/presence_validation_test.rb +++ b/activerecord/test/cases/validations/presence_validation_test.rb @@ -65,19 +65,39 @@ class PresenceValidationTest < ActiveRecord::TestCase assert_nothing_raised { s.valid? } end - def test_does_not_validate_presence_of_if_parent_record_is_validate_false + def test_validates_presence_of_virtual_attribute_on_model repair_validations(Interest) do + Interest.send(:attr_accessor, :abbreviation) Interest.validates_presence_of(:topic) + Interest.validates_presence_of(:abbreviation) + + interest = Interest.create!(topic: 'Thought Leadering', abbreviation: 'tl') + assert interest.valid? + + interest.abbreviation = '' + + assert interest.invalid? + end + end + + def test_validations_run_on_persisted_record + repair_validations(Interest) do interest = Interest.new - interest.save!(validate: false) - assert interest.persisted? + interest.save! + assert_predicate interest, :valid? - man = Man.new(interest_ids: [interest.id]) - man.save! + Interest.validates_presence_of(:topic) - assert_equal man.interests.size, 1 - assert interest.valid? - assert man.valid? + assert_not_predicate interest, :valid? + end + end + + def test_validates_presence_with_on_context + repair_validations(Interest) do + Interest.validates_presence_of(:topic, on: :required_name) + interest = Interest.new + interest.save! + assert_not interest.valid?(:required_name) end end end diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 7502a55391..4c14d93c66 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -5,6 +5,7 @@ require 'models/warehouse_thing' require 'models/guid' require 'models/event' require 'models/dashboard' +require 'models/uuid_item' class Wizard < ActiveRecord::Base self.abstract_class = true @@ -48,6 +49,18 @@ class BigIntReverseTest < ActiveRecord::Base validates :engines_count, uniqueness: true end +class CoolTopic < Topic + validates_uniqueness_of :id +end + +class TopicWithAfterCreate < Topic + after_create :set_author + + def set_author + update_attributes!(:author_name => "#{title} #{id}") + end +end + class UniquenessValidationTest < ActiveRecord::TestCase INT_MAX_VALUE = 2147483647 @@ -412,23 +425,6 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert topic.valid? end - def test_does_not_validate_uniqueness_of_if_parent_record_is_validate_false - Reply.validates_uniqueness_of(:content) - - Reply.create!(content: "Topic Title") - - reply = Reply.new(content: "Topic Title") - reply.save!(validate: false) - assert reply.persisted? - - topic = Topic.new(reply_ids: [reply.id]) - topic.save! - - assert_equal topic.replies.size, 1 - assert reply.valid? - assert topic.valid? - end - def test_validate_uniqueness_of_custom_primary_key klass = Class.new(ActiveRecord::Base) do self.table_name = "keyboards" @@ -469,4 +465,46 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert_match(/\AUnknown primary key for table dashboards in model/, e.message) assert_match(/Can not validate uniqueness for persisted record without primary key.\z/, e.message) end + + def test_validate_uniqueness_ignores_itself_when_primary_key_changed + Topic.validates_uniqueness_of(:title) + + t = Topic.new("title" => "This is a unique title") + assert t.save, "Should save t as unique" + + t.id += 1 + assert t.valid?, "Should be valid" + assert t.save, "Should still save t as unique" + end + + def test_validate_uniqueness_with_after_create_performing_save + TopicWithAfterCreate.validates_uniqueness_of(:title) + topic = TopicWithAfterCreate.create!(:title => "Title1") + assert topic.author_name.start_with?("Title1") + + topic2 = TopicWithAfterCreate.new(:title => "Title1") + refute topic2.valid? + assert_equal(["has already been taken"], topic2.errors[:title]) + end + + def test_validate_uniqueness_uuid + skip unless current_adapter?(:PostgreSQLAdapter) + item = UuidItem.create!(uuid: SecureRandom.uuid, title: 'item1') + item.update(title: 'item1-title2') + assert_empty item.errors + + item2 = UuidValidatingItem.create!(uuid: SecureRandom.uuid, title: 'item2') + item2.update(title: 'item2-title2') + assert_empty item2.errors + end + + def test_validate_uniqueness_regular_id + item = CoolTopic.create!(title: 'MyItem') + assert_empty item.errors + + item2 = CoolTopic.new(id: item.id, title: 'MyItem2') + refute item2.valid? + + assert_equal(["has already been taken"], item2.errors[:id]) + end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index d04f4f7ce7..85e33d2218 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -130,7 +130,7 @@ class ValidationsTest < ActiveRecord::TestCase def test_validates_acceptance_of_with_non_existent_table Object.const_set :IncorporealModel, Class.new(ActiveRecord::Base) - assert_nothing_raised ActiveRecord::StatementInvalid do + assert_nothing_raised do IncorporealModel.validates_acceptance_of(:incorporeal_column) end end diff --git a/activerecord/test/fixtures/price_estimates.yml b/activerecord/test/fixtures/price_estimates.yml index 1149ab17a2..406d65a142 100644 --- a/activerecord/test/fixtures/price_estimates.yml +++ b/activerecord/test/fixtures/price_estimates.yml @@ -1,7 +1,16 @@ -saphire_1: +sapphire_1: price: 10 estimate_of: sapphire (Treasure) sapphire_2: price: 20 estimate_of: sapphire (Treasure) + +diamond: + price: 30 + estimate_of: diamond (Treasure) + +honda: + price: 40 + estimate_of_type: Car + estimate_of_id: 1 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/car.rb b/activerecord/test/models/car.rb index 778c22b1f6..0f37e9a289 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -12,6 +12,8 @@ class Car < ActiveRecord::Base has_many :engines, :dependent => :destroy, inverse_of: :my_car has_many :wheels, :as => :wheelable, :dependent => :destroy + has_many :price_estimates, :as => :estimate_of + scope :incl_tyres, -> { includes(:tyres) } scope :incl_engines, -> { includes(:engines) } diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index b38b17e90e..dcc5c5a310 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -14,6 +14,7 @@ class Comment < ActiveRecord::Base has_many :ratings belongs_to :first_post, :foreign_key => :post_id + belongs_to :special_post_with_default_scope, foreign_key: :post_id has_many :children, :class_name => 'Comment', :foreign_key => :parent_id belongs_to :parent, :class_name => 'Comment', :counter_cache => :children_count diff --git a/activerecord/test/models/notification.rb b/activerecord/test/models/notification.rb index b4b4b8f1b6..82edc64b68 100644 --- a/activerecord/test/models/notification.rb +++ b/activerecord/test/models/notification.rb @@ -1,2 +1,3 @@ class Notification < ActiveRecord::Base + validates_presence_of :message end 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 diff --git a/activerecord/test/models/uuid_item.rb b/activerecord/test/models/uuid_item.rb new file mode 100644 index 0000000000..2353e40213 --- /dev/null +++ b/activerecord/test/models/uuid_item.rb @@ -0,0 +1,6 @@ +class UuidItem < ActiveRecord::Base +end + +class UuidValidatingItem < UuidItem + validates_uniqueness_of :uuid +end diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 3a5d73a0ed..24713f722a 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -106,4 +106,9 @@ _SQL t.integer :big_int_data_points, limit: 8, array: true t.decimal :decimal_array_default, array: true, default: [1.23, 3.45] end + + create_table :uuid_items, force: true, id: false do |t| + t.uuid :uuid, primary_key: true + t.string :title + end end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index b9e0706d60..2a8996f35c 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -929,7 +929,7 @@ ActiveRecord::Schema.define do t.string :treaty_id t.string :name end - create_table :countries_treaties, force: true, id: false do |t| + create_table :countries_treaties, force: true, primary_key: [:country_id, :treaty_id] do |t| t.string :country_id, null: false t.string :treaty_id, null: false end |