feat: Add ugrade step config helper

classes/local/execution/engine.php

@@ -572,7 +572,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();

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;
         }
 

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;
     }

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 < 2023122201) {
@@ -319,6 +306,28 @@ function xmldb_tool_dataflows_upgrade($oldversion) {
         upgrade_plugin_savepoint(true, 2023122201, '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);
+        }
+
+        // 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, 2024030201, 'tool', 'dataflows');
+    }
+
     return true;
 }
 
@@ -348,3 +357,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();
+}
+

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
+    type: tool_dataflows\local\step\writer_debugging

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();

tests/tool_dataflows_scheduler_test.php

@@ -16,7 +16,6 @@
 
 namespace tool_dataflows;
 
-use Aws\Arn\Arn;
 use tool_dataflows\local\scheduler;
 
 /**
@@ -71,6 +70,11 @@ public function test_update_next_scheduled_time() {
         $this->assertEquals((object) ['lastruntime' => 160, 'nextruntime' => 220], scheduler::get_scheduled_times(12));
     }
 
+    /**
+     * Tests the scheduling of retry runs.
+     *
+     * @covers \tool_dataflows\local\scheduler::set_scheduled_retry
+     */
     public function test_set_scheduled_retry() {
         global $DB;
 

tests/tool_dataflows_upgrade_test.php

@@ -0,0 +1,71 @@
+<?php
+// This file is part of Moodle - https://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+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 <[email protected]>
+ * @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));
+    }
+}