From 7d4d68888b95cbb0cf06b42e7ce18ee5b6f4c44b Mon Sep 17 00:00:00 2001 From: Adrianna Chang Date: Fri, 9 Feb 2024 10:36:56 -0500 Subject: [PATCH] Move default task build_enumerator logic back into the job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks can still define `#build_enumerator` to override the default and supply a custom enumerator for their task, but we shouldn't have all of the Job Iteration enumerator logic moved into the Task class. Adds additional documentation for writing tests for tasks that use custom enumerators. Co-authored-by: Étienne Barrié --- README.md | 38 +++++++++++++++- .../maintenance_tasks/task_job_concern.rb | 39 ++++++++++++++++ app/models/maintenance_tasks/task.rb | 44 +------------------ .../maintenance/custom_enumerating_task.rb | 3 +- 4 files changed, 80 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 3d13f7f23..aa8d8a9b4 100644 --- a/README.md +++ b/README.md @@ -436,7 +436,7 @@ module Maintenance def collection Post.where(created_at: 2.days.ago...1.hour.ago) end - + def process(post) post.update!(content: "updated content") end @@ -626,6 +626,42 @@ module Maintenance end ``` +### Writing tests for a Task that uses a custom enumerator + +Tests for tasks that use custom enumerators need to instantiate the task class +in order to call `#build_enumerator`. Once the task instance is set up, validate +that `#build_enumerator` returns an enumerator yielding pairs of [item, cursor] +as expected. + +```ruby +# test/tasks/maintenance/custom_enumerating_task.rb + +require "test_helper" + +module Maintenance + class CustomEnumeratingTaskTest < ActiveSupport::TestCase + setup do + @task = CustomEnumeratingTask.new + end + + test "#build_enumerator returns enumerator yielding pairs of [item, cursor]" do + enum = @task.build_enumerator(cursor: 0) + expected_items = [:b, :c] + + assert_equal 2, enum.size + + enum.each_with_index do |item, cursor| + assert_equal expected_items[cursor], item + end + end + + test "#process performs a task iteration" do + ... + end + end +end +``` + ### Running a Task #### Running a Task from the Web UI diff --git a/app/jobs/concerns/maintenance_tasks/task_job_concern.rb b/app/jobs/concerns/maintenance_tasks/task_job_concern.rb index a39649b29..af9ec6258 100644 --- a/app/jobs/concerns/maintenance_tasks/task_job_concern.rb +++ b/app/jobs/concerns/maintenance_tasks/task_job_concern.rb @@ -33,6 +33,45 @@ def retry_on(*, **) def build_enumerator(_run, cursor:) cursor ||= @run.cursor @collection_enum = @task.enumerator_builder(cursor: cursor) + + @collection_enum ||= case (collection = @task.collection) + when :no_collection + enumerator_builder.build_once_enumerator(cursor: nil) + when ActiveRecord::Relation + enumerator_builder.active_record_on_records(collection, cursor: cursor, columns: @task.cursor_columns) + when ActiveRecord::Batches::BatchEnumerator + if collection.start || collection.finish + raise ArgumentError, <<~MSG.squish + #{@task.class.name}#collection cannot support + a batch enumerator with the "start" or "finish" options. + MSG + end + + # For now, only support automatic count based on the enumerator for + # batches + enumerator_builder.active_record_on_batch_relations( + collection.relation, + cursor: cursor, + batch_size: collection.batch_size, + columns: @task.cursor_columns, + ) + when Array + enumerator_builder.build_array_enumerator(collection, cursor: cursor&.to_i) + when BatchCsvCollectionBuilder::BatchCsv + JobIteration::CsvEnumerator.new(collection.csv).batches( + batch_size: collection.batch_size, + cursor: cursor&.to_i, + ) + when CSV + JobIteration::CsvEnumerator.new(collection).rows(cursor: cursor&.to_i) + else + raise ArgumentError, <<~MSG.squish + #{@task.class.name}#collection must be either an + Active Record Relation, ActiveRecord::Batches::BatchEnumerator, + Array, or CSV. + MSG + end + throttle_enumerator(@collection_enum) end diff --git a/app/models/maintenance_tasks/task.rb b/app/models/maintenance_tasks/task.rb index 780c4877b..66b0062ce 100644 --- a/app/models/maintenance_tasks/task.rb +++ b/app/models/maintenance_tasks/task.rb @@ -260,7 +260,7 @@ def count self.class.collection_builder_strategy.count(self) end - # Default enumeration builder. You may override this method to return any + # Default enumerator builder. You may override this method to return any # Enumerator yielding pairs of `[item, item_cursor]`. # # @param cursor [String, nil] cursor position to resume from, or nil on @@ -268,47 +268,7 @@ def count # # @return [Enumerator] def enumerator_builder(cursor:) - collection = self.collection - - job_iteration_builder = JobIteration::EnumeratorBuilder.new(nil) - - case collection - when :no_collection - job_iteration_builder.build_once_enumerator(cursor: nil) - when ActiveRecord::Relation - job_iteration_builder.active_record_on_records(collection, cursor: cursor, columns: cursor_columns) - when ActiveRecord::Batches::BatchEnumerator - if collection.start || collection.finish - raise ArgumentError, <<~MSG.squish - #{self.class.name}#collection cannot support - a batch enumerator with the "start" or "finish" options. - MSG - end - - # For now, only support automatic count based on the enumerator for - # batches - job_iteration_builder.active_record_on_batch_relations( - collection.relation, - cursor: cursor, - batch_size: collection.batch_size, - columns: cursor_columns, - ) - when Array - job_iteration_builder.build_array_enumerator(collection, cursor: cursor&.to_i) - when BatchCsvCollectionBuilder::BatchCsv - JobIteration::CsvEnumerator.new(collection.csv).batches( - batch_size: collection.batch_size, - cursor: cursor&.to_i, - ) - when CSV - JobIteration::CsvEnumerator.new(collection).rows(cursor: cursor&.to_i) - else - raise ArgumentError, <<~MSG.squish - #{self.class.name}#collection must be either an - Active Record Relation, ActiveRecord::Batches::BatchEnumerator, - Array, or CSV. - MSG - end + nil end end end diff --git a/test/dummy/app/tasks/maintenance/custom_enumerating_task.rb b/test/dummy/app/tasks/maintenance/custom_enumerating_task.rb index 0633efb75..90f2a4fe7 100644 --- a/test/dummy/app/tasks/maintenance/custom_enumerating_task.rb +++ b/test/dummy/app/tasks/maintenance/custom_enumerating_task.rb @@ -12,7 +12,8 @@ def count 3 end - def process(_) + def process(letter) + Rails.logger.debug("letter: #{letter}") end end end