diff --git a/.gitignore b/.gitignore index 65775bac9..1962d40aa 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ vendor/bundle spec/test_app/log/*.log spec/test_app/storage/ spec/test_app/tmp/ +spec/tmp/example_app .env doc .yardoc diff --git a/README.md b/README.md index dfb679d8a..e15cb9660 100644 --- a/README.md +++ b/README.md @@ -703,7 +703,9 @@ GoodJob follows semantic versioning, though updates may be encouraged through de #### Upgrading minor versions -Upgrading between minor versions (e.g. v1.4 to v1.5) should not introduce breaking changes, but can introduce new deprecation warnings and database migration notices. +Upgrading between minor versions (e.g. v1.4 to v1.5) should not introduce breaking changes, but can introduce new deprecation warnings and database migration warnings. + +Database migrations introduced in minor releases are _not required_ to be applied until the next major release. If you would like apply newly introduced migrations immediately, assert `GoodJob.migrated?` in your application's test suite. To perform upgrades to the GoodJob database tables: diff --git a/app/models/concerns/good_job/error_events.rb b/app/models/concerns/good_job/error_events.rb index c6f79a0a9..caedfeeb8 100644 --- a/app/models/concerns/good_job/error_events.rb +++ b/app/models/concerns/good_job/error_events.rb @@ -14,15 +14,32 @@ module ErrorEvents ERROR_EVENT_DISCARDED = 'discarded', ].freeze - included do - enum error_event: { - ERROR_EVENT_INTERRUPTED => 0, - ERROR_EVENT_UNHANDLED => 1, - ERROR_EVENT_HANDLED => 2, - ERROR_EVENT_RETRIED => 3, - ERROR_EVENT_RETRY_STOPPED => 4, - ERROR_EVENT_DISCARDED => 5, - }.freeze, _prefix: :error_event + ERROR_EVENT_ENUMS = { + ERROR_EVENT_INTERRUPTED => 0, + ERROR_EVENT_UNHANDLED => 1, + ERROR_EVENT_HANDLED => 2, + ERROR_EVENT_RETRIED => 3, + ERROR_EVENT_RETRY_STOPPED => 4, + ERROR_EVENT_DISCARDED => 5, + }.freeze + + # TODO: GoodJob v4 can make this an `enum` once migrations are guaranteed. + def error_event + return unless self.class.columns_hash['error_event'] + + enum = super + return unless enum + + ERROR_EVENT_ENUMS.key(enum) + end + + def error_event=(event) + return unless self.class.columns_hash['error_event'] + + enum = ERROR_EVENT_ENUMS[event] + raise(ArgumentError, "Invalid error_event: #{event}") if event && !enum + + super(enum) end end end diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index dadc0b339..cdc3739f8 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -384,7 +384,7 @@ def perform error: interrupt_error_string, finished_at: Time.current, } - discrete_execution_attrs[:error_event] = GoodJob::DiscreteExecution.error_events[GoodJob::DiscreteExecution::ERROR_EVENT_INTERRUPTED] if self.class.error_event_migrated? + discrete_execution_attrs[:error_event] = GoodJob::ErrorEvents::ERROR_EVENT_ENUMS[GoodJob::ErrorEvents::ERROR_EVENT_INTERRUPTED] if self.class.error_event_migrated? discrete_executions.where(finished_at: nil).where.not(performed_at: nil).update_all(discrete_execution_attrs) # rubocop:disable Rails/SkipsModelValidations end end diff --git a/app/models/good_job/job.rb b/app/models/good_job/job.rb index 57eabbd2d..80bcf958b 100644 --- a/app/models/good_job/job.rb +++ b/app/models/good_job/job.rb @@ -228,9 +228,12 @@ def discard_job(message) update_execution = proc do execution.update( - finished_at: Time.current, - error: GoodJob::Execution.format_error(job_error), - error_event: :discarded + { + finished_at: Time.current, + error: GoodJob::Execution.format_error(job_error), + }.tap do |attrs| + attrs[:error_event] = ERROR_EVENT_DISCARDED if self.class.error_event_migrated? + end ) end diff --git a/lib/good_job.rb b/lib/good_job.rb index 54f5e5968..597576f08 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -246,5 +246,14 @@ def self.deprecator end end + # Whether all GoodJob migrations have been applied. + # For use in tests/CI to validate GoodJob is up-to-date. + # @return [Boolean] + def self.migrated? + # Always update with the most recent migration check + GoodJob::Execution.reset_column_information + GoodJob::Execution.error_event_migrated? + end + ActiveSupport.run_load_hooks(:good_job, self) end diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index 1c436afa8..5acf687e8 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -64,7 +64,15 @@ def enqueue_all(active_jobs) inline_executions = [] GoodJob::Execution.transaction(requires_new: true, joinable: false) do - results = GoodJob::Execution.insert_all(executions.map(&:attributes), returning: %w[id active_job_id]) # rubocop:disable Rails/SkipsModelValidations + execution_attributes = executions.map do |execution| + if GoodJob::Execution.error_event_migrated? + execution.attributes + else + execution.attributes.except('error_event') + end + end + + results = GoodJob::Execution.insert_all(execution_attributes, returning: %w[id active_job_id]) # rubocop:disable Rails/SkipsModelValidations job_id_to_provider_job_id = results.each_with_object({}) { |result, hash| hash[result['active_job_id']] = result['id'] } active_jobs.each do |active_job| diff --git a/spec/generators/good_job/update_generator_spec.rb b/spec/generators/good_job/update_generator_spec.rb index e9874ed47..e5daaae7e 100644 --- a/spec/generators/good_job/update_generator_spec.rb +++ b/spec/generators/good_job/update_generator_spec.rb @@ -23,6 +23,15 @@ quiet do run_in_example_app 'rails db:migrate' end + + # Check that `GoodJob.pending_migrations?` is updated + expect(GoodJob.migrated?).to be true + quiet do + run_in_example_app 'rails db:rollback' + expect(GoodJob.migrated?).to be false + run_in_example_app 'rails db:migrate' + end + expect(GoodJob.migrated?).to be true end end diff --git a/spec/integration/complex_jobs_spec.rb b/spec/integration/complex_jobs_spec.rb index 8cdc6a441..2a688fb1a 100644 --- a/spec/integration/complex_jobs_spec.rb +++ b/spec/integration/complex_jobs_spec.rb @@ -11,6 +11,11 @@ end describe 'Job with retry stopped but no block' do + after do + # This spec will intentionally raise an error on the thread. + THREAD_ERRORS.clear + end + specify do stub_const "TestJob", (Class.new(ActiveJob::Base) do retry_on StandardError, wait: 0, attempts: 1 @@ -37,7 +42,6 @@ def perform ) expect(THREAD_ERRORS.size).to eq 1 - THREAD_ERRORS.clear end end diff --git a/spec/support/example_app_helper.rb b/spec/support/example_app_helper.rb index 7f4397493..d9524136d 100644 --- a/spec/support/example_app_helper.rb +++ b/spec/support/example_app_helper.rb @@ -53,6 +53,14 @@ def within_example_app good_job_processes good_job_settings ] + models = [ + GoodJob::Job, + GoodJob::BatchRecord, + GoodJob::Execution, + GoodJob::DiscreteExecution, + GoodJob::Process, + GoodJob::Setting, + ] quiet do tables.each do |table_name| ActiveRecord::Migration.drop_table(table_name) if ActiveRecord::Base.connection.table_exists?(table_name) @@ -61,6 +69,7 @@ def within_example_app setup_example_app run_in_test_app("bin/rails db:environment:set RAILS_ENV=test") + models.each(&:reset_column_information) end yield @@ -74,6 +83,7 @@ def within_example_app ActiveRecord::Base.connection.execute("TRUNCATE schema_migrations") run_in_test_app("bin/rails db:schema:load db:environment:set RAILS_ENV=test") + models.each(&:reset_column_information) end end diff --git a/spec/test_app/Rakefile b/spec/test_app/Rakefile index ea6d8af2a..dc78bc1e4 100644 --- a/spec/test_app/Rakefile +++ b/spec/test_app/Rakefile @@ -22,3 +22,18 @@ namespace :heroku do Rake::Task['db:seed'].invoke end end + +def remove_version_from_schema_rb + puts "GoodJob: Sanitizing Rails version from schema.rb...\n" + schema_rb = Rails.root.join("db/schema.rb").to_s + contents = File.read(schema_rb) + contents.sub!(/ActiveRecord::Schema\[\d\.\d\]/, "ActiveRecord::Schema") + File.write(schema_rb, contents) +end + +Rake::Task['db:migrate'].enhance do + remove_version_from_schema_rb +end +Rake::Task['db:rollback'].enhance do + remove_version_from_schema_rb +end