diff options
| -rw-r--r-- | activejob/lib/active_job/exceptions.rb | 17 | ||||
| -rw-r--r-- | activejob/test/cases/exceptions_test.rb | 25 | ||||
| -rw-r--r-- | activerecord/CHANGELOG.md | 41 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 21 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/where_clause.rb | 14 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/where_clause_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/where_test.rb | 55 | ||||
| -rw-r--r-- | guides/source/6_0_release_notes.md | 1 | ||||
| -rw-r--r-- | guides/source/api_app.md | 9 | ||||
| -rw-r--r-- | railties/CHANGELOG.md | 11 | ||||
| -rw-r--r-- | railties/lib/rails/tasks.rb | 1 | ||||
| -rw-r--r-- | railties/lib/rails/tasks/zeitwerk.rake | 78 |
12 files changed, 255 insertions, 20 deletions
diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb index 35c1476368..8e83246303 100644 --- a/activejob/lib/active_job/exceptions.rb +++ b/activejob/lib/active_job/exceptions.rb @@ -49,12 +49,10 @@ module ActiveJob # end def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil) rescue_from(*exceptions) do |error| - # Guard against jobs that were persisted before we started having individual executions counters per retry_on - self.exception_executions ||= {} - self.exception_executions[exceptions.to_s] = (exception_executions[exceptions.to_s] || 0) + 1 + executions = executions_for(exceptions) - if exception_executions[exceptions.to_s] < attempts - retry_job wait: determine_delay(seconds_or_duration_or_algorithm: wait, executions: exception_executions[exceptions.to_s]), queue: queue, priority: priority, error: error + if executions < attempts + retry_job wait: determine_delay(seconds_or_duration_or_algorithm: wait, executions: executions), queue: queue, priority: priority, error: error else if block_given? instrument :retry_stopped, error: error do @@ -146,5 +144,14 @@ module ActiveJob ActiveSupport::Notifications.instrument("#{name}.active_job", payload, &block) end + + def executions_for(exceptions) + if exception_executions + exception_executions[exceptions.to_s] = (exception_executions[exceptions.to_s] || 0) + 1 + else + # Guard against jobs that were persisted before we started having individual executions counters per retry_on + executions + end + end end end diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb index 840f4d40b5..1f07b7b294 100644 --- a/activejob/test/cases/exceptions_test.rb +++ b/activejob/test/cases/exceptions_test.rb @@ -179,6 +179,31 @@ class ExceptionsTest < ActiveSupport::TestCase assert_equal ["Raised ActiveJob::DeserializationError for the 5 time"], JobBuffer.values end + test "running a job enqueued by AJ 5.2" do + job = RetryJob.new("DefaultsError", 6) + job.exception_executions = nil # This is how jobs from Rails 5.2 will look + + assert_raises DefaultsError do + job.enqueue + end + + assert_equal 5, JobBuffer.values.count + end + + test "running a job enqueued and attempted under AJ 5.2" do + job = RetryJob.new("DefaultsError", 6) + + # Fake 4 previous executions under AJ 5.2 + job.exception_executions = nil + job.executions = 4 + + assert_raises DefaultsError do + job.enqueue + end + + assert_equal ["Raised DefaultsError for the 5th time"], JobBuffer.values + end + private def adapter_skips_scheduling?(queue_adapter) [ diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 485547f036..adc7e754a4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,44 @@ +* Deprecate `where.not` working as NOR and will be changed to NAND in Rails 6.1. + + ```ruby + all = [treasures(:diamond), treasures(:sapphire), cars(:honda), treasures(:sapphire)] + assert_equal all, PriceEstimate.all.map(&:estimate_of) + ``` + + In Rails 6.0: + + ```ruby + sapphire = treasures(:sapphire) + + nor = all.reject { |e| + e.estimate_of_type == sapphire.class.polymorphic_name + }.reject { |e| + e.estimate_of_id == sapphire.id + } + assert_equal [cars(:honda)], nor + + without_sapphire = PriceEstimate.where.not( + estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id + ) + assert_equal nor, without_sapphire.map(&:estimate_of) + ``` + + In Rails 6.1: + + ```ruby + sapphire = treasures(:sapphire) + + nand = all - [sapphire] + assert_equal [treasures(:diamond), cars(:honda)], nand + + without_sapphire = PriceEstimate.where.not( + estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id + ) + assert_equal nand, without_sapphire.map(&:estimate_of) + ``` + + *Ryuta Kamizono* + * Fix dirty tracking after rollback. Fixes #15018, #30167, #33868. diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 5d3cea6741..f30428d0a5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -41,18 +41,31 @@ module ActiveRecord # # User.where.not(name: %w(Ko1 Nobu)) # # SELECT * FROM users WHERE name NOT IN ('Ko1', 'Nobu') - # - # User.where.not(name: "Jon", role: "admin") - # # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin' def not(opts, *rest) opts = sanitize_forbidden_attributes(opts) where_clause = @scope.send(:where_clause_factory).build(opts, rest) @scope.references!(PredicateBuilder.references(opts)) if Hash === opts - @scope.where_clause += where_clause.invert + + if not_behaves_as_nor?(opts) + ActiveSupport::Deprecation.warn(<<~MSG.squish) + NOT conditions will no longer behave as NOR in Rails 6.1. + To continue using NOR conditions, NOT each conditions manually + (`#{ opts.keys.map { |key| ".where.not(#{key.inspect} => ...)" }.join }`). + MSG + @scope.where_clause += where_clause.invert(:nor) + else + @scope.where_clause += where_clause.invert + end + @scope end + + private + def not_behaves_as_nor?(opts) + opts.is_a?(Hash) && opts.size > 1 + end end FROZEN_EMPTY_ARRAY = [].freeze diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 47728aac30..b91b135867 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -70,7 +70,15 @@ module ActiveRecord predicates == other.predicates end - def invert + def invert(as = :nand) + if predicates.size == 1 + inverted_predicates = [ invert_predicate(predicates.first) ] + elsif as == :nor + inverted_predicates = predicates.map { |node| invert_predicate(node) } + else + inverted_predicates = [ Arel::Nodes::Not.new(ast) ] + end + WhereClause.new(inverted_predicates) end @@ -115,10 +123,6 @@ module ActiveRecord node.respond_to?(:operator) && node.operator == :== end - def inverted_predicates - predicates.map { |node| invert_predicate(node) } - end - def invert_predicate(node) case node when NilClass diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb index 0b06cec40b..b26a1a1d80 100644 --- a/activerecord/test/cases/relation/where_clause_test.rb +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -106,7 +106,7 @@ class ActiveRecord::Relation Arel::Nodes::Not.new(random_object) ]) - assert_equal expected, original.invert + assert_equal expected, original.invert(:nor) end test "except removes binary predicates referencing a given column" do diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index b045184d7d..2ecccfe1f3 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -115,13 +115,58 @@ module ActiveRecord assert_equal expected.to_sql, actual.to_sql end - def test_polymorphic_shallow_where_not - treasure = treasures(:sapphire) + def test_where_not_polymorphic_association + sapphire = treasures(:sapphire) - expected = [price_estimates(:diamond), price_estimates(:honda)] - actual = PriceEstimate.where.not(estimate_of: treasure) + all = [treasures(:diamond), sapphire, cars(:honda), sapphire] + assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of) - assert_equal expected.sort_by(&:id), actual.sort_by(&:id) + actual = PriceEstimate.where.not(estimate_of: sapphire) + only = PriceEstimate.where(estimate_of: sapphire) + + expected = all - [sapphire] + assert_equal expected, actual.sort_by(&:id).map(&:estimate_of) + assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of) + end + + def test_where_not_polymorphic_id_and_type_as_nand + sapphire = treasures(:sapphire) + + all = [treasures(:diamond), sapphire, cars(:honda), sapphire] + assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of) + + actual = PriceEstimate.where.yield_self do |where_chain| + where_chain.stub(:not_behaves_as_nor?, false) do + where_chain.not(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id) + end + end + only = PriceEstimate.where(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id) + + expected = all - [sapphire] + assert_equal expected, actual.sort_by(&:id).map(&:estimate_of) + assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of) + end + + def test_where_not_polymorphic_id_and_type_as_nor_is_deprecated + sapphire = treasures(:sapphire) + + all = [treasures(:diamond), sapphire, cars(:honda), sapphire] + assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of) + + message = <<~MSG.squish + NOT conditions will no longer behave as NOR in Rails 6.1. + To continue using NOR conditions, NOT each conditions manually + (`.where.not(:estimate_of_type => ...).where.not(:estimate_of_id => ...)`). + MSG + actual = assert_deprecated(message) do + PriceEstimate.where.not(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id) + end + only = PriceEstimate.where(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id) + + expected = all - [sapphire] + # NOT (estimate_of_type = 'Treasure' OR estimate_of_id = sapphire.id) matches only `cars(:honda)` unfortunately. + assert_not_equal expected, actual.sort_by(&:id).map(&:estimate_of) + assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of) end def test_polymorphic_nested_array_where diff --git a/guides/source/6_0_release_notes.md b/guides/source/6_0_release_notes.md index 8c9f55fc85..180cfa371c 100644 --- a/guides/source/6_0_release_notes.md +++ b/guides/source/6_0_release_notes.md @@ -454,6 +454,7 @@ Please refer to the [Changelog][active-storage] for detailed changes. ### Notable changes * Updating an attached model via `update` or `update!` ala `@user.update!(images: [ … ])` now replaces the existing images instead of merely adding to them. + ([Pull Request](https://github.com/rails/rails/pull/33303)) Active Model ------------ diff --git a/guides/source/api_app.md b/guides/source/api_app.md index b8b6cb7874..181d39e7e0 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -420,6 +420,15 @@ Some common modules you might want to add: - `ActionController::MimeResponds`: Support for `respond_to`. - `ActionController::Cookies`: Support for `cookies`, which includes support for signed and encrypted cookies. This requires the cookies middleware. +- `ActionController::Caching`: Support view caching for the API controller. Please notice that + you will need to manually specify cache store inside the controller like: + ```ruby + class ApplicationController < ActionController::API + include ::ActionController::Caching + self.cache_store = :mem_cached_store + end + ``` + Rails does *not* pass this configuration automatically. The best place to add a module is in your `ApplicationController`, but you can also add modules to individual controllers. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index e5f9e37c33..84974d8f03 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,14 @@ +* Applications upgrading to Rails 6 can run the command + + ``` + bin/rails zeitwerk:check + ``` + + to check if the project structure they were using with the classic + autoloader is compatible with `:zeitwerk` mode. + + *Matilda Smeds* & *Xavier Noria* + * Allow loading seeds without ActiveJob. Fixes #35782 diff --git a/railties/lib/rails/tasks.rb b/railties/lib/rails/tasks.rb index 2f644a20c9..0e592e15c3 100644 --- a/railties/lib/rails/tasks.rb +++ b/railties/lib/rails/tasks.rb @@ -15,6 +15,7 @@ require "rake" routes tmp yarn + zeitwerk ).tap { |arr| arr << "statistics" if Rake.application.current_scope.empty? }.each do |task| diff --git a/railties/lib/rails/tasks/zeitwerk.rake b/railties/lib/rails/tasks/zeitwerk.rake new file mode 100644 index 0000000000..cf84bb6948 --- /dev/null +++ b/railties/lib/rails/tasks/zeitwerk.rake @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +ensure_classic_mode = ->() do + if Rails.autoloaders.zeitwerk_enabled? + abort <<~EOS + Please, enable temporarily :classic mode: + + # config/application.rb + config.autoloader = :classic + + and try again. When all is good, you can delete that line. + EOS + end +end + +eager_load = ->() do + Rails.configuration.eager_load_namespaces.each(&:eager_load!) +end + +mismatches = [] +check_directory = ->(directory, parent) do + # test/mailers/previews might not exist. + return unless File.exists?(directory) + + Dir.foreach(directory) do |entry| + next if entry.start_with?(".") + next if parent == Object && entry == "concerns" + + abspath = File.join(directory, entry) + + if File.directory?(abspath) || abspath.end_with?(".rb") + print "." + cname = File.basename(abspath, ".rb").camelize.to_sym + if parent.const_defined?(cname, false) + if File.directory?(abspath) + check_directory[abspath, parent.const_get(cname)] + end + else + mismatches << [abspath, parent, cname] + end + end + end +end + +report = ->() do + puts + if mismatches.empty? + puts "All is good!" + puts "Please, remember to delete `config.autoloader = :classic` from config/application.rb." + else + mismatches.each do |abspath, parent, cname| + relpath = abspath.sub(%r{\A#{Regexp.escape(Rails.root.to_path)}/}, "") + cpath = parent == Object ? cname : "#{parent.name}::#{cname}" + puts "expected #{relpath} to define #{cpath}" + end + puts + puts <<~EOS + Please revise the reported mismatches. You can normally fix them by adding + acronyms to config/initializers/inflections.rb or renaming the constants. + EOS + end +end + +namespace :zeitwerk do + desc "Checks project structure for Zeitwerk compatibility" + task check: :environment do + ensure_classic_mode[] + eager_load[] + + $stdout.sync = true + ActiveSupport::Dependencies.autoload_paths.each do |autoload_path| + check_directory[autoload_path, Object] + end + puts + + report[] + end +end |