From 541c637529e68420d633756c1c67a80c5ac95bdb Mon Sep 17 00:00:00 2001 From: Adrianna Chang Date: Thu, 3 Jun 2021 14:24:52 -0400 Subject: [PATCH] Support Tasks with parameters through CLI (#428) --- README.md | 17 +++++++++++++ .../maintenance_tasks/tasks_controller.rb | 8 +------ app/models/maintenance_tasks/run.rb | 16 +++++++++++++ lib/maintenance_tasks/cli.rb | 10 ++++++-- test/lib/maintenance_tasks/cli_test.rb | 24 +++++++++++++++---- test/models/maintenance_tasks/run_test.rb | 20 ++++++++++++++++ 6 files changed, 82 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index dea5fe28..d27c5055 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,13 @@ To run a Task that processes CSVs from the command line, use the --csv option: $ bundle exec maintenance_tasks perform Maintenance::ImportPostsTask --csv 'path/to/my_csv.csv' ``` +To run a Task that takes arguments from the command line, use the --arguments +option, passing arguments as a set of : pairs: + +```bash +$ bundle exec maintenance_tasks perform Maintenance::ParamsTask --arguments post_ids:1,2,3 content:"Hello, World!" +``` + You can also run a Task in Ruby by sending `run` with a Task name to Runner: ```ruby @@ -332,6 +339,16 @@ MaintenanceTasks::Runner.run( ) ``` +To run a Task that takes arguments using the Runner, provide a Hash containing +the set of arguments (`{ parameter_name: argument_value }`) to `run`: + +```ruby +MaintenanceTasks::Runner.run( + name: "Maintenance::ParamsTask", + arguments: { post_ids: "1,2,3" } +) +``` + ### Monitoring your Task's status The web UI will provide updates on the status of your Task. Here are the states diff --git a/app/controllers/maintenance_tasks/tasks_controller.rb b/app/controllers/maintenance_tasks/tasks_controller.rb index 72134d2f..2e307a2e 100644 --- a/app/controllers/maintenance_tasks/tasks_controller.rb +++ b/app/controllers/maintenance_tasks/tasks_controller.rb @@ -28,7 +28,7 @@ def run task = Runner.run( name: params.fetch(:id), csv_file: params[:csv_file], - arguments: task_arguments, + arguments: params.fetch(:task_arguments, {}).permit!.to_h, ) redirect_to(task_path(task)) rescue ActiveRecord::RecordInvalid => error @@ -42,12 +42,6 @@ def run private - def task_arguments - return {} unless params[:task_arguments].present? - task_attributes = Task.named(params[:id]).attribute_names - params.require(:task_arguments).permit(*task_attributes).to_h - end - def set_refresh @refresh = 3 end diff --git a/app/models/maintenance_tasks/run.rb b/app/models/maintenance_tasks/run.rb index f7c855ad..12d7b352 100644 --- a/app/models/maintenance_tasks/run.rb +++ b/app/models/maintenance_tasks/run.rb @@ -214,6 +214,7 @@ def csv_attachment_presence # Performs validation on the arguments to use for the Task. If the Task is # invalid, the errors are added to the Run. def validate_task_arguments + arguments_match_task_attributes if arguments.present? if task.invalid? error_messages = task.errors .map { |attribute, message| "#{attribute.inspect} #{message}" } @@ -229,6 +230,7 @@ def validate_task_arguments # Performs validation on the arguments to use for the Task. If the Task is # invalid, the errors are added to the Run. def validate_task_arguments + arguments_match_task_attributes if arguments.present? if task.invalid? error_messages = task.errors .map { |error| "#{error.attribute.inspect} #{error.message}" } @@ -266,6 +268,20 @@ def task task.assign_attributes(arguments) end task + rescue ActiveModel::UnknownAttributeError + task + end + end + + private + + def arguments_match_task_attributes + invalid_argument_keys = arguments.keys - task.attribute_names + if invalid_argument_keys.any? + error_message = <<~MSG.squish + Unknown parameters: #{invalid_argument_keys.map(&:to_sym).join(", ")} + MSG + errors.add(:base, error_message) end end end diff --git a/lib/maintenance_tasks/cli.rb b/lib/maintenance_tasks/cli.rb index 35cf1ecb..ee85084b 100644 --- a/lib/maintenance_tasks/cli.rb +++ b/lib/maintenance_tasks/cli.rb @@ -28,15 +28,21 @@ def exit_on_failure? option :csv, desc: "Supply a CSV file to be processed by a CSV Task, "\ '--csv "path/to/csv/file.csv"' + # Specify arguments to supply to a Task supporting parameters + option :arguments, type: :hash, desc: "Supply arguments for a Task that "\ + "accepts parameters as a set of : pairs." + # Command to run a Task. # # It instantiates a Runner and sends a run message with the given Task name. # If a CSV file is supplied using the --csv option, an attachable with the - # File IO object is sent along with the Task name to run. + # File IO object is sent along with the Task name to run. If arguments are + # supplied using the --arguments option, these are also passed to run. # # @param name [String] the name of the Task to be run. def perform(name) - task = Runner.run(name: name, csv_file: csv_file) + arguments = options[:arguments] || {} + task = Runner.run(name: name, csv_file: csv_file, arguments: arguments) say_status(:success, "#{task.name} was enqueued.", :green) rescue => error say_status(:error, error.message, :red) diff --git a/test/lib/maintenance_tasks/cli_test.rb b/test/lib/maintenance_tasks/cli_test.rb index e13b59ce..b7aebdd3 100644 --- a/test/lib/maintenance_tasks/cli_test.rb +++ b/test/lib/maintenance_tasks/cli_test.rb @@ -16,14 +16,16 @@ class CLITest < ActiveSupport::TestCase test "#perfom runs the given Task and prints a success message" do task = mock(name: "MyTask") - Runner.expects(:run).with(name: "MyTask", csv_file: nil).returns(task) + Runner.expects(:run).with(name: "MyTask", csv_file: nil, arguments: {}) + .returns(task) @cli.expects(:say_status).with(:success, "MyTask was enqueued.", :green) @cli.perform("MyTask") end test "#perfom prints an error message when the runner raises" do - Runner.expects(:run).with(name: "Wrong", csv_file: nil).raises("Invalid!") + Runner.expects(:run).with(name: "Wrong", csv_file: nil, arguments: {}) + .raises("Invalid!") @cli.expects(:say_status).with(:error, "Invalid!", :red) @cli.perform("Wrong") @@ -35,15 +37,29 @@ class CLITest < ActiveSupport::TestCase opened_csv_file = File.open(csv_file_path) expected_attachable = { io: opened_csv_file, filename: "sample.csv" } - @cli.expects(:options).returns(csv: csv_file_path) + @cli.expects(:options).at_least_once.returns(csv: csv_file_path) File.expects(:open).with(csv_file_path).returns(opened_csv_file) Runner.expects(:run) - .with(name: "MyCsvTask", csv_file: expected_attachable) + .with(name: "MyCsvTask", csv_file: expected_attachable, arguments: {}) .returns(task) @cli.expects(:say_status) .with(:success, "MyCsvTask was enqueued.", :green) @cli.perform("MyCsvTask") end + + test "#perform runs a Task with the supplied arguments when --arguments option used" do + task = mock(name: "MyParamsTask") + arguments = { "post_ids": "1,2,3" } + + @cli.expects(:options).at_least_once.returns(arguments: arguments) + Runner.expects(:run) + .with(name: "MyParamsTask", csv_file: nil, arguments: arguments) + .returns(task) + @cli.expects(:say_status) + .with(:success, "MyParamsTask was enqueued.", :green) + + @cli.perform("MyParamsTask") + end end end diff --git a/test/models/maintenance_tasks/run_test.rb b/test/models/maintenance_tasks/run_test.rb index c699aab3..fc54e56c 100644 --- a/test/models/maintenance_tasks/run_test.rb +++ b/test/models/maintenance_tasks/run_test.rb @@ -28,6 +28,26 @@ class RunTest < ActiveSupport::TestCase refute_predicate run, :valid? end + test "invalid if arguments used do not match parameters on Task" do + run = Run.new( + task_name: "Maintenance::ParamsTask", + arguments: { post_ids: "1,2,3", bad_argument: "1,2,3" } + ) + refute_predicate run, :valid? + assert_equal run.errors.full_messages.first, + "Unknown parameters: bad_argument" + end + + test "invalid if arguments are supplied but Task does not support parameters" do + run = Run.new( + task_name: "Maintenance::UpdatePostsTask", + arguments: { post_ids: "1,2,3" } + ) + refute_predicate run, :valid? + assert_equal run.errors.full_messages.first, + "Unknown parameters: post_ids" + end + test "#persist_progress persists increments to tick count and time_running" do run = Run.create!( task_name: "Maintenance::UpdatePostsTask",