diff options
Diffstat (limited to 'activerecord')
27 files changed, 350 insertions, 65 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b09d9e336b..c2e79e9f02 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,30 @@ +* Fix has_many :through relation merging failing when dynamic conditions are + passed as a lambda with an arity of one. + + Fixes #16128 + + *Agis Anastasopoulos* + +* Fixed the `Relation#exists?` to work with polymorphic associations. + + Fixes #15821. + + *Kassio Borges* + +* Currently, Active Record will rescue any errors raised within + after_rollback/after_create callbacks and print them to the logs. Next versions of rails + will not rescue those errors anymore, and just bubble them up, as the other callbacks. + + This adds a opt-in flag to enable that behaviour, of not rescuing the errors. + Example: + + # For not swallow errors in after_commit/after_rollback callbacks. + config.active_record.raise_in_transactional_callbacks = true + + Fixes #13460. + + *arthurnn* + * Fixed an issue where custom accessor methods (such as those generated by `enum`) with the same name as a global method are incorrectly overridden when subclassing. diff --git a/activerecord/RUNNING_UNIT_TESTS.rdoc b/activerecord/RUNNING_UNIT_TESTS.rdoc index ca1f2fd665..569685bd45 100644 --- a/activerecord/RUNNING_UNIT_TESTS.rdoc +++ b/activerecord/RUNNING_UNIT_TESTS.rdoc @@ -32,8 +32,8 @@ defined in +Rakefile+) == Config File -If +test/config.yml+ is present, it's parameters are obeyed. Otherwise, the -parameters in +test/config.example.yml+ are obeyed. +If +test/config.yml+ is present, then its parameters are obeyed; otherwise, the +parameters in +test/config.example.yml+ are. You can override the +connections:+ parameter in either file using the +ARCONN+ (Active Record CONNection) environment variable: diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index d3b9b8251a..4ec1c8d545 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -447,9 +447,11 @@ module ActiveRecord # # Possible callbacks are: +before_add+, +after_add+, +before_remove+ and +after_remove+. # - # Should any of the +before_add+ callbacks throw an exception, the object does not get - # added to the collection. Same with the +before_remove+ callbacks; if an exception is - # thrown the object doesn't get removed. + # If any of the +before_add+ callbacks throw an exception, the object will not be + # added to the collection. + # + # Similarly, if any of the +before_remove+ callbacks throw an exception, the object + # will not be removed from the collection. # # == Association extensions # @@ -647,7 +649,7 @@ module ActiveRecord # belongs_to :commenter # end # - # When using nested association, you will not be able to modify the association because there + # When using a nested association, you will not be able to modify the association because there # is not enough information to know what modification to make. For example, if you tried to # add a <tt>Commenter</tt> in the example above, there would be no way to tell how to set up the # intermediate <tt>Post</tt> and <tt>Comment</tt> objects. @@ -717,7 +719,7 @@ module ActiveRecord # == Eager loading of associations # # Eager loading is a way to find objects of a certain class and a number of named associations. - # This is one of the easiest ways of to prevent the dreaded N+1 problem in which fetching 100 + # It is one of the easiest ways to prevent the dreaded N+1 problem in which fetching 100 # posts that each need to display their author triggers 101 database queries. Through the # use of eager loading, the number of queries will be reduced from 101 to 2. # @@ -749,16 +751,16 @@ module ActiveRecord # Post.includes(:author, :comments).each do |post| # # This will load all comments with a single query. This reduces the total number of queries - # to 3. More generally the number of queries will be 1 plus the number of associations + # to 3. In general, the number of queries will be 1 plus the number of associations # named (except if some of the associations are polymorphic +belongs_to+ - see below). # # To include a deep hierarchy of associations, use a hash: # - # Post.includes(:author, {comments: {author: :gravatar}}).each do |post| + # Post.includes(:author, { comments: { author: :gravatar } }).each do |post| # - # That'll grab not only all the comments but all their authors and gravatar pictures. - # You can mix and match symbols, arrays and hashes in any combination to describe the - # associations you want to load. + # The above code will load all the comments and all of their associated + # authors and gravatars. You can mix and match any combination of symbols, + # arrays, and hashes to retrieve the associations you want to load. # # All of this power shouldn't fool you into thinking that you can pull out huge amounts # of data with no performance penalty just because you've reduced the number of queries. @@ -767,8 +769,8 @@ module ActiveRecord # cut down on the number of queries in a situation as the one described above. # # Since only one table is loaded at a time, conditions or orders cannot reference tables - # other than the main one. If this is the case Active Record falls back to the previously - # used LEFT OUTER JOIN based strategy. For example + # other than the main one. If this is the case, Active Record falls back to the previously + # used LEFT OUTER JOIN based strategy. For example: # # Post.includes([:author, :comments]).where(['comments.approved = ?', true]) # @@ -1133,6 +1135,17 @@ module ActiveRecord # * <tt>Firm#clients.create!</tt> (similar to <tt>c = Client.new("firm_id" => id); c.save!</tt>) # The declaration can also include an +options+ hash to specialize the behavior of the association. # + # === Scopes + # + # You can pass a second argument +scope+ as a callable (i.e. proc or + # lambda) to retrieve a specific set of records or customize the generated + # query when you access the associated collection. + # + # Scope examples: + # has_many :comments, -> { where(author_id: 1) } + # has_many :employees, -> { joins(:address) } + # has_many :posts, ->(post) { where("max_post_length > ?", post.length) } + # # === Options # [:class_name] # Specify the class name of the association. Use it only if that name can't be inferred @@ -1257,6 +1270,17 @@ module ActiveRecord # * <tt>Account#create_beneficiary</tt> (similar to <tt>b = Beneficiary.new("account_id" => id); b.save; b</tt>) # * <tt>Account#create_beneficiary!</tt> (similar to <tt>b = Beneficiary.new("account_id" => id); b.save!; b</tt>) # + # === Scopes + # + # You can pass a second argument +scope+ as a callable (i.e. proc or + # lambda) to retrieve a specific record or customize the generated query + # when you access the associated object. + # + # Scope examples: + # has_one :auther, -> { where(comment_id: 1) } + # has_one :employer, -> { joins(:company) } + # has_one :dob, ->(dob) { where("Date.new(2000, 01, 01) > ?", dob) } + # # === Options # # The declaration can also include an +options+ hash to specialize the behavior of the association. @@ -1366,6 +1390,17 @@ module ActiveRecord # * <tt>Post#create_author!</tt> (similar to <tt>post.author = Author.new; post.author.save!; post.author</tt>) # The declaration can also include an +options+ hash to specialize the behavior of the association. # + # === Scopes + # + # You can pass a second argument +scope+ as a callable (i.e. proc or + # lambda) to retrieve a specific record or customize the generated query + # when you access the associated object. + # + # Scope examples: + # belongs_to :user, -> { where(id: 2) } + # belongs_to :user, -> { joins(:friends) } + # belongs_to :level, ->(level) { where("game_level > ?", level.current) } + # # === Options # # [:class_name] @@ -1543,6 +1578,18 @@ module ActiveRecord # * <tt>Developer#projects.create</tt> (similar to <tt>c = Project.new("developer_id" => id); c.save; c</tt>) # The declaration may include an +options+ hash to specialize the behavior of the association. # + # === Scopes + # + # You can pass a second argument +scope+ as a callable (i.e. proc or + # lambda) to retrieve a specific set of records or customize the generated + # query when you access the associated collection. + # + # Scope examples: + # has_and_belongs_to_many :projects, -> { includes :milestones, :manager } + # has_and_belongs_to_many :categories, ->(category) { + # where("default_category = ?", category.name) + # } + # # === Options # # [:class_name] diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 519d4d8651..4c47af8cb0 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -115,7 +115,7 @@ module ActiveRecord if reflection.type value = owner.class.base_class.name - bind_val = bind scope, table.table_name, reflection.type.to_s, value, tracker + bind_val = bind scope, table.table_name, reflection.type, value, tracker scope = scope.where(table[reflection.type].eq(bind_val)) end else @@ -123,7 +123,7 @@ module ActiveRecord if reflection.type value = chain[i + 1].klass.base_class.name - bind_val = bind scope, table.table_name, reflection.type.to_s, value, tracker + bind_val = bind scope, table.table_name, reflection.type, value, tracker scope = scope.where(table[reflection.type].eq(bind_val)) end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 611d471e62..e47e81aa0f 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -15,7 +15,11 @@ module ActiveRecord scope = super reflection.chain.drop(1).each do |reflection| relation = reflection.klass.all - relation.merge!(reflection.scope) if reflection.scope + + reflection_scope = reflection.scope + if reflection_scope && reflection_scope.arity.zero? + relation.merge!(reflection_scope) + end scope.merge!( relation.except(:select, :create_with, :includes, :preload, :joins, :eager_load) 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 a5fa9d6adc..bc1a670b42 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -234,7 +234,7 @@ module ActiveRecord @spec = spec - @checkout_timeout = spec.config[:checkout_timeout] || 5 + @checkout_timeout = (spec.config[:checkout_timeout] && spec.config[:checkout_timeout].to_f) || 5 @reaper = Reaper.new self, spec.config[:reaping_frequency] @reaper.run 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 9e07e9a5c4..92ac607a3c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -2,6 +2,7 @@ require 'date' require 'set' require 'bigdecimal' require 'bigdecimal/util' +require 'active_support/core_ext/string/strip' module ActiveRecord module ConnectionAdapters #:nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 8f06cf3a1f..90be835d8a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -22,6 +22,10 @@ module ActiveRecord @state == :rolledback end + def completed? + committed? || rolledback? + end + def set_state(state) if !VALID_STATES.include?(state) raise ArgumentError, "Invalid transaction state: #{state}" @@ -63,13 +67,19 @@ module ActiveRecord end def rollback_records - records.uniq.each do |record| + ite = records.uniq + while record = ite.shift begin record.rolledback! full_rollback? rescue => e + raise if ActiveRecord::Base.raise_in_transactional_callbacks record.logger.error(e) if record.respond_to?(:logger) && record.logger end end + ensure + ite.each do |i| + i.rolledback!(full_rollback?, false) + end end def commit @@ -77,13 +87,19 @@ module ActiveRecord end def commit_records - records.uniq.each do |record| + ite = records.uniq + while record = ite.shift begin record.committed! rescue => e + raise if ActiveRecord::Base.raise_in_transactional_callbacks record.logger.error(e) if record.respond_to?(:logger) && record.logger end end + ensure + ite.each do |i| + i.committed!(false) + end end def full_rollback?; true; end @@ -103,14 +119,14 @@ module ActiveRecord end def rollback - super connection.rollback_to_savepoint(savepoint_name) + super rollback_records end def commit - super connection.release_savepoint(savepoint_name) + super parent = connection.transaction_manager.current_transaction records.each { |r| parent.add_record(r) } end @@ -130,14 +146,14 @@ module ActiveRecord end def rollback - super connection.rollback_db_transaction + super rollback_records end def commit - super connection.commit_db_transaction + super commit_records end end @@ -177,7 +193,7 @@ module ActiveRecord begin commit_transaction unless error rescue Exception - transaction.rollback + transaction.rollback unless transaction.state.completed? raise end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 4306b36ae1..727f12103a 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -870,11 +870,11 @@ module ActiveRecord def try_to_load_dependency(file_name) require_dependency file_name rescue LoadError => e - # Let's hope the developer has included it - # Let's warn in case this is a subdependency, otherwise - # subdependency error messages are totally cryptic - if ActiveRecord::Base.logger - ActiveRecord::Base.logger.warn("Unable to load #{file_name}, underlying cause #{e.message} \n\n #{e.backtrace.join("\n")}") + unless fixture_class_names.key?(file_name.pluralize) + if ActiveRecord::Base.logger + ActiveRecord::Base.logger.warn("Unable to load #{file_name}, make sure you added it to ActiveSupport::TestCase.set_fixture_class") + ActiveRecord::Base.logger.warn("underlying cause #{e.message} \n\n #{e.backtrace.join("\n")}") + end end end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index a6847e28c2..d0d9304a36 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -161,21 +161,14 @@ module ActiveRecord # in the <tt>db/migrate/</tt> directory where <tt>timestamp</tt> is the # UTC formatted date and time that the migration was generated. # - # You may then edit the <tt>up</tt> and <tt>down</tt> methods of - # MyNewMigration. - # # There is a special syntactic shortcut to generate migrations that add fields to a table. # # rails generate migration add_fieldname_to_tablename fieldname:string # # This will generate the file <tt>timestamp_add_fieldname_to_tablename</tt>, which will look like this: # class AddFieldnameToTablename < ActiveRecord::Migration - # def up - # add_column :tablenames, :fieldname, :string - # end - # - # def down - # remove_column :tablenames, :fieldname + # def change + # add_column :tablenames, :field, :string # end # end # @@ -188,9 +181,12 @@ module ActiveRecord # # To roll the database back to a previous migration version, use # <tt>rake db:migrate VERSION=X</tt> where <tt>X</tt> is the version to which - # you wish to downgrade. If any of the migrations throw an - # <tt>ActiveRecord::IrreversibleMigration</tt> exception, that step will fail and you'll - # have some manual work to do. + # you wish to downgrade. Alternatively, you can also use the STEP option if you + # wish to rollback last few migrations. <tt>rake db:migrate STEP=2</tt> will rollback + # the latest two migrations. + # + # If any of the migrations throw an <tt>ActiveRecord::IrreversibleMigration</tt> exception, + # that step will fail and you'll have some manual work to do. # # == Database support # @@ -836,21 +832,20 @@ module ActiveRecord SchemaMigration.table_name end - def get_all_versions - SchemaMigration.all.map { |x| x.version.to_i }.sort + def get_all_versions(connection = Base.connection) + if connection.table_exists?(schema_migrations_table_name) + SchemaMigration.all.map { |x| x.version.to_i }.sort + else + [] + end end def current_version(connection = Base.connection) - sm_table = schema_migrations_table_name - if connection.table_exists?(sm_table) - get_all_versions.max || 0 - else - 0 - end + get_all_versions(connection).max || 0 end def needs_migration?(connection = Base.connection) - current_version(connection) < last_version + (migrations(migrations_paths).collect(&:version) - get_all_versions(connection)).size > 0 end def last_version diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 1547c8e3f4..331b6b0698 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -38,7 +38,7 @@ module ActiveRecord ar.aggregate_reflections = ar.aggregate_reflections.merge(name.to_s => reflection) end - # \Reflection enables interrogating Active Record classes and objects + # \Reflection enables interrogating of Active Record classes and objects # about their associations and aggregations. This information can, # for example, be used in a form builder that takes an Active Record object # and creates input fields for all of the attributes depending on their type diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 0c9c761f97..a7899da3a8 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -304,7 +304,7 @@ module ActiveRecord end end - connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false + connection.select_value(relation, "#{name} Exists", relation.arel.bind_values + relation.bind_values) ? true : false end # This method is called whenever no records are found with either a single diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index c8f382ad78..067966321d 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1,9 +1,12 @@ require 'active_support/core_ext/array/wrap' +require 'active_model/forbidden_attributes_protection' module ActiveRecord module QueryMethods extend ActiveSupport::Concern + include ActiveModel::ForbiddenAttributesProtection + # WhereChain objects act as placeholder for queries in which #where does not have any parameter. # In this case, #where must be chained with #not to return a new relation. class WhereChain @@ -574,7 +577,10 @@ WARNING end def where!(opts, *rest) # :nodoc: - references!(PredicateBuilder.references(opts)) if Hash === opts + if Hash === opts + opts = sanitize_forbidden_attributes(opts) + references!(PredicateBuilder.references(opts)) + end self.where_values += build_where(opts, rest) self @@ -723,7 +729,13 @@ WARNING end def create_with!(value) # :nodoc: - self.create_with_value = value ? create_with_value.merge(value) : {} + if value + value = sanitize_forbidden_attributes(value) + self.create_with_value = create_with_value.merge(value) + else + self.create_with_value = {} + end + self end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 7e4dc4c895..1bb7aab8fb 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -3,11 +3,23 @@ module ActiveRecord module Transactions extend ActiveSupport::Concern ACTIONS = [:create, :destroy, :update] + CALLBACK_WARN_MESSAGE = <<-EOF +Currently, Active Record will rescue any errors raised within +after_rollback/after_commit callbacks and print them to the logs. In the next +version, these errors will no longer be rescued. Instead, they will simply +bubble just like other Active Record callbacks. + +You can opt into the new behavior and remove this warning by setting +config.active_record.raise_in_transactional_callbacks to true. +EOF included do define_callbacks :commit, :rollback, terminator: ->(_, result) { result == false }, scope: [:kind, :name] + + mattr_accessor :raise_in_transactional_callbacks, instance_writer: false + self.raise_in_transactional_callbacks = false end # = Active Record Transactions @@ -223,6 +235,9 @@ module ActiveRecord def after_commit(*args, &block) set_options_for_callbacks!(args) set_callback(:commit, :after, *args, &block) + unless ActiveRecord::Base.raise_in_transactional_callbacks + ActiveSupport::Deprecation.warn(CALLBACK_WARN_MESSAGE) + end end # This callback is called after a create, update, or destroy are rolled back. @@ -231,6 +246,9 @@ module ActiveRecord def after_rollback(*args, &block) set_options_for_callbacks!(args) set_callback(:rollback, :after, *args, &block) + unless ActiveRecord::Base.raise_in_transactional_callbacks + ActiveSupport::Deprecation.warn(CALLBACK_WARN_MESSAGE) + end end private @@ -290,16 +308,16 @@ module ActiveRecord # # Ensure that it is not called if the object was never persisted (failed create), # but call it after the commit of a destroyed object. - def committed! #:nodoc: - run_callbacks :commit if destroyed? || persisted? + def committed!(should_run_callbacks = true) #:nodoc: + run_callbacks :commit if should_run_callbacks && destroyed? || persisted? ensure force_clear_transaction_record_state end # Call the +after_rollback+ callbacks. The +force_restore_state+ argument indicates if the record # state should be rolled back to the beginning or just to the last savepoint. - def rolledback!(force_restore_state = false) #:nodoc: - run_callbacks :rollback + def rolledback!(force_restore_state = false, should_run_callbacks = true) #:nodoc: + run_callbacks :rollback if should_run_callbacks ensure restore_transaction_record_state(force_restore_state) clear_transaction_record_state diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 95d006279d..b42a60fea5 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -4,6 +4,7 @@ require 'models/author' require 'models/categorization' require 'models/comment' require 'models/company' +require 'models/tagging' require 'models/topic' require 'models/reply' require 'models/entrant' @@ -78,6 +79,19 @@ class FinderTest < ActiveRecord::TestCase assert_raise(NoMethodError) { Topic.exists?([1,2]) } end + def test_exists_with_polymorphic_relation + post = Post.create!(title: 'Post', body: 'default', taggings: [Tagging.new(comment: 'tagging comment')]) + relation = Post.tagged_with_comment('tagging comment') + + assert_equal true, relation.exists?(title: ['Post']) + assert_equal true, relation.exists?(['title LIKE ?', 'Post%']) + assert_equal true, relation.exists? + assert_equal true, relation.exists?(post.id) + assert_equal true, relation.exists?(post.id.to_s) + + assert_equal false, relation.exists?(false) + end + def test_exists_passing_active_record_object_is_deprecated assert_deprecated do Topic.exists?(Topic.new) diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 042fdaf0bb..d7bbe0df62 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -823,15 +823,20 @@ end class FixtureLoadingTest < ActiveRecord::TestCase def test_logs_message_for_failed_dependency_load - ActiveRecord::TestCase.expects(:require_dependency).with(:does_not_exist).raises(LoadError) - ActiveRecord::Base.logger.expects(:warn) - ActiveRecord::TestCase.try_to_load_dependency(:does_not_exist) + ActiveRecord::Base.logger.expects(:warn).twice + ActiveRecord::TestCase.try_to_load_dependency('does_not_exist') + end + + def test_does_not_logs_message_for_dependency_that_has_been_defined_with_set_fixture_class + ActiveRecord::TestCase.set_fixture_class unknown_dead_parrots: DeadParrot + ActiveRecord::Base.logger.expects(:warn).never + ActiveRecord::TestCase.try_to_load_dependency('unknown_dead_parrot') end def test_does_not_logs_message_for_successful_dependency_load - ActiveRecord::TestCase.expects(:require_dependency).with(:works_out_fine) + ActiveRecord::TestCase.expects(:require_dependency).with('works_out_fine') ActiveRecord::Base.logger.expects(:warn).never - ActiveRecord::TestCase.try_to_load_dependency(:works_out_fine) + ActiveRecord::TestCase.try_to_load_dependency('works_out_fine') end end diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 981a75faf6..f4e7646f03 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -66,4 +66,34 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase person = Person.new assert_nil person.assign_attributes(ProtectedParams.new({})) end + + def test_create_with_checks_permitted + params = ProtectedParams.new(first_name: 'Guille', gender: 'm') + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Person.create_with(params).create! + end + end + + def test_create_with_works_with_params_values + params = ProtectedParams.new(first_name: 'Guille') + + person = Person.create_with(first_name: params[:first_name]).create! + assert_equal 'Guille', person.first_name + end + + def test_where_checks_permitted + params = ProtectedParams.new(first_name: 'Guille', gender: 'm') + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Person.where(params).create! + end + end + + def test_where_works_with_params_values + params = ProtectedParams.new(first_name: 'Guille') + + person = Person.where(first_name: params[:first_name]).create! + assert_equal 'Guille', person.first_name + end end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 6a8aff4b69..e43b796237 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -24,6 +24,9 @@ ActiveSupport::Deprecation.debug = true # Disable available locale checks to avoid warnings running the test suite. I18n.enforce_available_locales = false +# Enable raise errors in after_commit and after_rollback. +ActiveRecord::Base.raise_in_transactional_callbacks = true + # Connect to the database ARTest.connect diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 11338e1fb6..034a0f567e 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -81,6 +81,21 @@ class MigrationTest < ActiveRecord::TestCase assert_equal 0, ActiveRecord::Migrator.current_version assert_equal 3, ActiveRecord::Migrator.last_version assert_equal true, ActiveRecord::Migrator.needs_migration? + + ActiveRecord::SchemaMigration.create!(:version => ActiveRecord::Migrator.last_version) + assert_equal true, ActiveRecord::Migrator.needs_migration? + ensure + ActiveRecord::Migrator.migrations_paths = old_path + end + + def test_migration_detection_without_schema_migration_table + ActiveRecord::Base.connection.drop_table('schema_migrations') if ActiveRecord::Base.connection.table_exists?('schema_migrations') + + migrations_path = MIGRATIONS_ROOT + "/valid" + old_path = ActiveRecord::Migrator.migrations_paths + ActiveRecord::Migrator.migrations_paths = migrations_path + + assert_equal true, ActiveRecord::Migrator.needs_migration? ensure ActiveRecord::Migrator.migrations_paths = old_path end diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 2b5c2fd5a4..0d537fbfe3 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -4,6 +4,7 @@ require 'models/comment' require 'models/developer' require 'models/post' require 'models/project' +require 'models/rating' class RelationMergingTest < ActiveRecord::TestCase fixtures :developers, :comments, :authors, :posts @@ -144,4 +145,16 @@ class MergingDifferentRelationsTest < ActiveRecord::TestCase assert_equal ["Mary", "Mary", "Mary", "David"], posts_by_author_name end + + test "relation merging (using a proc argument)" do + dev = Developer.where(name: "Jamis").first + + comment_1 = dev.comments.create!(body: "I'm Jamis", post: Post.first) + rating_1 = comment_1.ratings.create! + + comment_2 = dev.comments.create!(body: "I'm John", post: Post.first) + comment_2.ratings.create! + + assert_equal dev.ratings, [rating_1] + end end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index b4804aa9d7..580ea98910 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -180,6 +180,10 @@ module ActiveRecord assert_equal 0, Post.where(:id => []).count end + def test_where_with_table_name_and_nested_empty_array + assert_equal [], Post.where(:id => [[]]).to_a + end + def test_where_with_empty_hash_and_no_foreign_key assert_equal 0, Edge.where(:sink => {}).count end diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 4070216733..eb44c4a83c 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -94,7 +94,7 @@ module ActiveRecord # instead examining the SQL content. oracle_ignored = [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im, /^\s*select .* from all_constraints/im, /^\s*select .* from all_tab_cols/im] mysql_ignored = [/^SHOW TABLES/i, /^SHOW FULL FIELDS/, /^SHOW CREATE TABLE /i, /^SHOW VARIABLES /] - postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i] + postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i] sqlite3_ignored = [/^\s*SELECT name\b.*\bFROM sqlite_master/im] [oracle_ignored, mysql_ignored, postgresql_ignored, sqlite3_ignored].each do |db_ignored_sql| diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index a3f39804b7..b5ac1bdaf9 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -267,6 +267,9 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_after_transaction_callbacks_should_prevent_callbacks_from_being_called + old_transaction_config = ActiveRecord::Base.raise_in_transactional_callbacks + ActiveRecord::Base.raise_in_transactional_callbacks = false + def @first.last_after_transaction_error=(e); @last_transaction_error = e; end def @first.last_after_transaction_error; @last_transaction_error; end @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";} @@ -291,6 +294,79 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end assert_equal :rollback, @first.last_after_transaction_error assert_equal [:after_rollback], second.history + ensure + ActiveRecord::Base.raise_in_transactional_callbacks = old_transaction_config + end + + def test_after_commit_should_not_raise_when_raise_in_transactional_callbacks_false + old_transaction_config = ActiveRecord::Base.raise_in_transactional_callbacks + ActiveRecord::Base.raise_in_transactional_callbacks = false + @first.after_commit_block{ fail "boom" } + Topic.transaction { @first.save! } + ensure + ActiveRecord::Base.raise_in_transactional_callbacks = old_transaction_config + end + + def test_after_commit_callback_should_not_swallow_errors + @first.after_commit_block{ fail "boom" } + assert_raises(RuntimeError) do + Topic.transaction do + @first.save! + end + end + end + + def test_after_commit_callback_when_raise_should_not_restore_state + first = TopicWithCallbacks.new + second = TopicWithCallbacks.new + first.after_commit_block{ fail "boom" } + second.after_commit_block{ fail "boom" } + + begin + Topic.transaction do + first.save! + assert_not_nil first.id + second.save! + assert_not_nil second.id + end + rescue + end + assert_not_nil first.id + assert_not_nil second.id + assert first.reload + end + + def test_after_rollback_callback_should_not_swallow_errors_when_set_to_raise + error_class = Class.new(StandardError) + @first.after_rollback_block{ raise error_class } + assert_raises(error_class) do + Topic.transaction do + @first.save! + raise ActiveRecord::Rollback + end + end + end + + def test_after_rollback_callback_when_raise_should_restore_state + error_class = Class.new(StandardError) + + first = TopicWithCallbacks.new + second = TopicWithCallbacks.new + first.after_rollback_block{ raise error_class } + second.after_rollback_block{ raise error_class } + + begin + Topic.transaction do + first.save! + assert_not_nil first.id + second.save! + assert_not_nil second.id + raise ActiveRecord::Rollback + end + rescue error_class + end + assert_nil first.id + assert_nil second.id end def test_after_rollback_callbacks_should_validate_on_condition diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 15970758db..7a88299d08 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -9,6 +9,7 @@ class Comment < ActiveRecord::Base belongs_to :post, :counter_cache => true belongs_to :author, polymorphic: true belongs_to :resource, polymorphic: true + belongs_to :developer has_many :ratings diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 5bd2f00129..3627cfdd09 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -46,6 +46,8 @@ class Developer < ActiveRecord::Base has_many :audit_logs has_many :contracts has_many :firms, :through => :contracts, :source => :firm + has_many :comments, ->(developer) { where(body: "I'm #{developer.name}") } + has_many :ratings, through: :comments scope :jamises, -> { where(:name => 'Jamis') } diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index a29858213b..56a31011da 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -41,6 +41,7 @@ class Post < ActiveRecord::Base scope :with_tags, -> { preload(:taggings) } scope :tagged_with, ->(id) { joins(:taggings).where(taggings: { tag_id: id }) } + scope :tagged_with_comment, ->(comment) { joins(:taggings).where(taggings: { comment: comment }) } has_many :comments do def find_most_recent diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 98f2492ef8..0584df87c6 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -198,6 +198,7 @@ ActiveRecord::Schema.define do t.references :author, polymorphic: true t.string :resource_id t.string :resource_type + t.integer :developer_id end create_table :companies, force: true do |t| |