From 5714fb4f518ceddd75c713634b42a2021894e70c Mon Sep 17 00:00:00 2001 From: Peter Burnett Date: Sun, 3 Mar 2024 15:09:50 +1000 Subject: [PATCH] feat: Add ugrade step config helper --- classes/local/execution/engine.php | 3 +- classes/local/scheduler.php | 14 +++-- classes/local/step/trigger_cron.php | 4 +- db/upgrade.php | 68 +++++++++++++++------- tests/fixtures/sample-cron.yml | 4 +- tests/tool_dataflows_ad_hoc_task_test.php | 2 +- tests/tool_dataflows_scheduler_test.php | 25 ++++++-- tests/tool_dataflows_upgrade_test.php | 71 +++++++++++++++++++++++ version.php | 4 +- 9 files changed, 158 insertions(+), 37 deletions(-) create mode 100644 tests/tool_dataflows_upgrade_test.php diff --git a/classes/local/execution/engine.php b/classes/local/execution/engine.php index bb9eae21..3f3150f8 100644 --- a/classes/local/execution/engine.php +++ b/classes/local/execution/engine.php @@ -561,7 +561,8 @@ public function abort(?\Throwable $reason = null) { $this->set_current_step($enginestep); $enginestep->abort(); } else { - // We need to signal to finished steps that the dataflow is aborted. This may require handling seperate to the step abort. + // We need to signal to finished steps that the dataflow is aborted. + // This may require handling seperate to the step abort. // This is done seperate to the finalise hook so that concerns are seperated for finalised vs aborted runs. $this->set_current_step($enginestep); $enginestep->dataflow_abort(); diff --git a/classes/local/scheduler.php b/classes/local/scheduler.php index 9cadc3f1..37387795 100644 --- a/classes/local/scheduler.php +++ b/classes/local/scheduler.php @@ -76,15 +76,19 @@ public static function set_scheduled_times(int $dataflowid, int $stepid, int $ne * @param int $scheduledtime when to run next if allowed retries are exhausted. * @param int $retriesallowed the amount of retries allowed before resuming regular schedule. */ - public static function set_scheduled_retry(int $dataflowid, int $stepid, int $retrytime, int $scheduledtime, int $retriesallowed) { - global $DB; + public static function set_scheduled_retry( + int $dataflowid, + int $stepid, + int $retrytime, + int $scheduledtime, + int $retriesallowed) { + global $DB; $schedule = $DB->get_record(self::TABLE, ['dataflowid' => $dataflowid, 'stepid' => $stepid]); if (!$schedule) { // This method has been called incorrectly for a schedule that has never run or doesn't exist. - // Just return early. - return; + throw new \coding_exception("Dataflow retry attempted on a trigger with no step."); } if ($schedule->retrycount >= $retriesallowed) { @@ -93,7 +97,7 @@ public static function set_scheduled_retry(int $dataflowid, int $stepid, int $re $schedule->nextruntime = $scheduledtime; } else { // Increment retry counter, and schedule the retry time. - $schedule->retrycount = $schedule->retrycount + 1; + $schedule->retrycount += 1; $schedule->nextruntime = $retrytime; } diff --git a/classes/local/step/trigger_cron.php b/classes/local/step/trigger_cron.php index 9eed103c..20b12466 100644 --- a/classes/local/step/trigger_cron.php +++ b/classes/local/step/trigger_cron.php @@ -62,8 +62,8 @@ public function form_get_default_data(\stdClass $data): \stdClass { $data->{"config_$field"} = '*'; } } - $data->config_retryinterval = $data->config_retryinterval ?? 0; - $data->config_retrycount = $data->config_retrycount ?? 0; + $data->config_retryinterval ??= 0; + $data->config_retrycount ??= 0; return $data; } diff --git a/db/upgrade.php b/db/upgrade.php index 6aef6c14..95bcde97 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -23,6 +23,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use Symfony\Component\Yaml\Yaml; + /** * Function to upgrade tool_dataflows. * @@ -287,21 +289,6 @@ function xmldb_tool_dataflows_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2023072100, 'tool', 'dataflows'); } - if ($oldversion < 2024030201) { - - // Define field retrycount to be added to tool_dataflows_schedule. - $table = new xmldb_table('tool_dataflows_schedule'); - $field = new xmldb_field('retrycount', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, 0, 'nextruntime'); - - // Conditionally launch add field retrycount. - if (!$dbman->field_exists($table, $field)) { - $dbman->add_field($table, $field); - } - - // Dataflows savepoint reached. - upgrade_plugin_savepoint(true, 2024030201, 'tool', 'dataflows'); - } - // Move log files that exist across to new format. Breaking change if any // dataflows implement logic based on these files based on filename format. if ($oldversion < 2023110901) { @@ -319,19 +306,39 @@ function xmldb_tool_dataflows_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2023110901, 'tool', 'dataflows'); } - if ($oldversion < 2024030200) { - - // Define field loghandlers to be added to tool_dataflows. + if ($oldversion < 2023110903) { + // Define field notifyonabort to be added to tool_dataflows. $table = new xmldb_table('tool_dataflows'); $field = new xmldb_field('notifyonabort', XMLDB_TYPE_CHAR, '255', null, null, null, '', 'confighash'); - // Conditionally launch add field loghandlers. + // Conditionally launch add field notifyonabort. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + upgrade_plugin_savepoint(true, 2023110903, 'tool', 'dataflows'); + } + + if ($oldversion < 2023110904) { + // Define field retrycount to be added to tool_dataflows_schedule. + $table = new xmldb_table('tool_dataflows_schedule'); + $field = new xmldb_field('retrycount', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, 0, 'nextruntime'); + + // Conditionally launch add field retrycount. if (!$dbman->field_exists($table, $field)) { $dbman->add_field($table, $field); } + // Also upgrade any cron_trigger step. + $type = \tool_dataflows\local\step\trigger_cron::class; + $newfields = [ + 'retryinterval' => 0, + 'retrycount' => 0 + ]; + xmldb_tool_dataflows_step_config_helper($type, $newfields); + // Dataflows savepoint reached. - upgrade_plugin_savepoint(true, 2024030200, 'tool', 'dataflows'); + upgrade_plugin_savepoint(true, 2023110904, 'tool', 'dataflows'); } return true; @@ -363,3 +370,24 @@ function xmldb_tool_dataflows_logfile_rename_helper(string $path, string $patter } } } + +/** + * Upgrade step helper function. Appends fields into existing configuration. + * + * @param string $type the classname of the step to upgrade. + * @param array $newfields new fields to append into the configuration array. String => primitive scalar. + */ +function xmldb_tool_dataflows_step_config_helper(string $type, array $newfields) { + global $DB; + + $steps = $DB->get_records('tool_dataflows_steps', ['type' => $type]); + $transaction = $DB->start_delegated_transaction(); + foreach ($steps as $step) { + $config = Yaml::parse($step->config); + $updatedconf = array_merge($config, $newfields); + $step->config = Yaml::dump($updatedconf); + $DB->update_record('tool_dataflows_steps', $step); + } + $transaction->allow_commit(); +} + diff --git a/tests/fixtures/sample-cron.yml b/tests/fixtures/sample-cron.yml index 7d53f584..1dab43ab 100644 --- a/tests/fixtures/sample-cron.yml +++ b/tests/fixtures/sample-cron.yml @@ -10,6 +10,8 @@ steps: day: '*' month: '*' dayofweek: '*' + retryinterval: 0 + retrycount: 0 sql_reader: name: 'SQL reader' depends_on: cron @@ -21,4 +23,4 @@ steps: debugging_writer: name: 'Debugging writer' depends_on: sql_reader - type: tool_dataflows\local\step\writer_debugging \ No newline at end of file + type: tool_dataflows\local\step\writer_debugging diff --git a/tests/tool_dataflows_ad_hoc_task_test.php b/tests/tool_dataflows_ad_hoc_task_test.php index 059ce74a..76fcddbb 100644 --- a/tests/tool_dataflows_ad_hoc_task_test.php +++ b/tests/tool_dataflows_ad_hoc_task_test.php @@ -95,7 +95,7 @@ private function create_dataflow() { $cron = new step(); $cron->name = 'cron'; $cron->type = 'tool_dataflows\local\step\trigger_cron'; - $cron->config = "minute: '*'\nhour: '*'\nday: '*'\nmonth: '*'\ndayofweek: '*'"; + $cron->config = "minute: '*'\nhour: '*'\nday: '*'\nmonth: '*'\ndayofweek: '*'\nretryinterval: 0\nretrycount: 0"; $dataflow->add_step($cron); $reader = new step(); diff --git a/tests/tool_dataflows_scheduler_test.php b/tests/tool_dataflows_scheduler_test.php index bb33f66c..86251031 100644 --- a/tests/tool_dataflows_scheduler_test.php +++ b/tests/tool_dataflows_scheduler_test.php @@ -16,7 +16,7 @@ namespace tool_dataflows; -use Aws\Arn\Arn; +use coding_exception; use tool_dataflows\local\scheduler; /** @@ -71,12 +71,27 @@ public function test_update_next_scheduled_time() { $this->assertEquals((object) ['lastruntime' => 160, 'nextruntime' => 220], scheduler::get_scheduled_times(12)); } - public function test_set_scheduled_retry() { - global $DB; - + /** + * Tests the scheduling of retry run on an invalid dataflow. + * + * @covers \tool_dataflows\local\scheduler::set_scheduled_retry + */ + public function test_set_scheduled_retry_invalid() { // Retry cannot be called for a run that hasn't been regularly scheduled. + $this->expectException(coding_exception::class); + $this->expectExceptionMessage( + 'Coding error detected, it must be fixed by a programmer: Dataflow retry attempted on a trigger with no step.' + ); scheduler::set_scheduled_retry(1, 1, 1, 1, 1); - $this->assertFalse(scheduler::get_scheduled_times(1)); + } + + /** + * Tests the scheduling of retry runs. + * + * @covers \tool_dataflows\local\scheduler::set_scheduled_retry + */ + public function test_set_scheduled_retry() { + global $DB; // Default 0. scheduler::set_scheduled_times(1, 1, 123, 1); diff --git a/tests/tool_dataflows_upgrade_test.php b/tests/tool_dataflows_upgrade_test.php new file mode 100644 index 00000000..a955017e --- /dev/null +++ b/tests/tool_dataflows_upgrade_test.php @@ -0,0 +1,71 @@ +. + +namespace tool_dataflows; +use Symfony\Component\Yaml\Yaml; + +defined('MOODLE_INTERNAL') || die(); +require_once(__DIR__.'/../db/upgrade.php'); + +/** + * Unit test for the dataflows upgrade steps. + * + * @package tool_dataflows + * @author Peter Burnett + * @copyright 2024, Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class tool_dataflows_upgrade_test extends \advanced_testcase { + /** + * Test config upgrade helper. + * + * @covers \tool_dataflows\local\step\trigger_event::validate_config + */ + public function test_step_config_helper() { + global $DB; + $this->resetAfterTest(); + + $dataflow = new dataflow(); + $dataflow->name = 'testupgrade'; + $dataflow->enabled = true; + $dataflow->save(); + + $step = new step(); + $step->name = 'cron'; + $step->type = 'tool_dataflows\local\step\trigger_cron'; + $config = [ + 'minute' => '*', + 'hour' => '*', + 'day' => '*', + 'month' => '*', + 'dayofweek' => '*', + 'retryinterval' => '0', + 'retrycount' => '0', + ]; + $step->config = Yaml::dump($config); + $dataflow->add_step($step); + + // Regular Additional config. + $existingstep = $DB->get_record('tool_dataflows_steps', ['type' => $step->type]); + $this->assertEquals($config, Yaml::parse($existingstep->config)); + + $config['newstep'] = 'upgrade'; + $newfields = ['newstep' => 'upgrade']; + xmldb_tool_dataflows_step_config_helper($step->type, $newfields); + $updatedrec = $DB->get_record('tool_dataflows_steps', ['type' => $step->type]); + $this->assertEquals($config, Yaml::parse($updatedrec->config)); + } +} diff --git a/version.php b/version.php index f1721742..09826aab 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2023110902; -$plugin->release = 2023110902; +$plugin->version = 2023110904; +$plugin->release = 2023110904; $plugin->requires = 2017051500; // Our lowest supported Moodle (3.3.0). $plugin->supported = [35, 401]; // Available as of Moodle 3.9.0 or later. // TODO $plugin->incompatible = ; // Available as of Moodle 3.9.0 or later.