From 51c543ce6b7117531c9dfaf8b1638458b45143fe Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 19:43:17 +1000 Subject: [PATCH 01/13] feat: fill in name of new steps based on step type --- classes/form/step_form.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/classes/form/step_form.php b/classes/form/step_form.php index 01291d11..7e0ad115 100644 --- a/classes/form/step_form.php +++ b/classes/form/step_form.php @@ -361,6 +361,14 @@ protected function get_default_data() { if (!empty($type) && class_exists($type)) { $steptype = new $type(); $data = $steptype->form_get_default_data($data); + + // Automatically fill in the name with something hopefully sane. + $classname = $type; + $position = strrpos($classname, '\\'); + $basename = substr($classname, $position + 1); + if ($position !== false) { + $data->name = str_replace('_step', '', $basename); // So curl_step becomes curl. + } } return $data; From 7f21659ccb5c83ba760a750b4d7f196de7a586a2 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 19:47:26 +1000 Subject: [PATCH 02/13] [wip] feat: flow and connector steps can now be used interchangably --- classes/local/execution/flow_engine_step.php | 18 +++++++++++++++++- classes/local/step/base_step.php | 2 +- classes/step.php | 9 ++++++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/classes/local/execution/flow_engine_step.php b/classes/local/execution/flow_engine_step.php index 433158be..e28a60de 100644 --- a/classes/local/execution/flow_engine_step.php +++ b/classes/local/execution/flow_engine_step.php @@ -16,6 +16,8 @@ namespace tool_dataflows\local\execution; +use tool_dataflows\parser; + /** * Manages the execution of a flow step. * @@ -84,10 +86,24 @@ public function get_variables(): array { // such as 'dataflow.id' and 'steps.mystep.name' for example. $variables = $this->engine->get_variables(); $step = $variables['steps']->{$this->stepdef->alias}; + + // Pull out the config. + $config = $step->config; + + // Set the record as an available variable. + if ($this->iterator) { + $variables['record'] = (array) $this->iterator->current(); + + // Evaluate the config again with the record context, unless the + // step type doesn't want to (e.g. SQL reader does it own handling). + $parser = new parser; + $parser->evaluate_recursive($config, $variables); + } + return array_merge( $variables, ['step' => $step], - (array) $step->config + (array) $config, ); } } diff --git a/classes/local/step/base_step.php b/classes/local/step/base_step.php index 9445d00b..2eac69ce 100644 --- a/classes/local/step/base_step.php +++ b/classes/local/step/base_step.php @@ -477,7 +477,7 @@ public function validate_for_run() { * * @return engine_step */ - final public function get_engine_step(): engine_step { + final public function get_engine_step(): ?engine_step { return $this->enginestep; } diff --git a/classes/step.php b/classes/step.php index 46a2b8e4..4cf53bbd 100644 --- a/classes/step.php +++ b/classes/step.php @@ -239,9 +239,16 @@ protected function get_config($expressions = true, bool $redacted = false): \std // Normally expressions are parsed when evaluating the statement, for use in a dataflow run. if ($expressions) { + // Get variables, based on whether the engine is running or is idle. + $enginestep = $steptype->get_engine_step(); + if ($enginestep) { + $variables = $enginestep->get_variables(); + } else { + $variables = $this->variables; + } + // Prepare this as a php object (stdClass), as it makes expressions easier to write. $parser = new parser(); - $variables = $this->variables; $yaml = $parser->evaluate_recursive($yaml, $variables); // Sets it to the parsed results. From 29440e03bb835df8734960b865402d9f04ed5d6d Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 19:48:33 +1000 Subject: [PATCH 03/13] wip: add curl step with dual step support --- classes/local/step/curl_step.php | 293 +++++++++++++++++++++++++++++++ classes/local/step/flow_step.php | 6 + classes/step.php | 1 + lib.php | 1 + 4 files changed, 301 insertions(+) create mode 100644 classes/local/step/curl_step.php diff --git a/classes/local/step/curl_step.php b/classes/local/step/curl_step.php new file mode 100644 index 00000000..300b142e --- /dev/null +++ b/classes/local/step/curl_step.php @@ -0,0 +1,293 @@ +. + +namespace tool_dataflows\local\step; + +use Symfony\Component\Yaml\Yaml; +use tool_dataflows\helper; + +/** + * CURL step + * + * @package tool_dataflows + * @author Kevin Pham + * @author Ghaly Marc-Alexandre + * @copyright Catalyst IT, 2022 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class curl_step extends flow_step { + + /** @var int time after which curl request is aborted */ + protected $timeout = 60; + + /** + * Returns whether or not the step configured, has a side effect. + * + * For curl connectors, it is considered to have a side effect if the target is + * anywhere outside of the scratch directory, the method is anything other than + * 'get' or 'head', or if the 'has side effects' setting is checked. + * + * @return bool whether or not this step has a side effect + */ + public function has_side_effect(): bool { + if (isset($this->stepdef)) { + $config = $this->stepdef->config; + + // Destination is outside of scratch directory. + if (!(empty($config->destination) || helper::path_is_relative($config->destination))) { + return true; + } + + // Request is anything other than 'get' or 'head'. + if (!($config->method == 'get' || $config->method == 'head')) { + return true; + } + + // Side effects setting is checked. + return !empty($config->sideeffects); + } + + return true; + } + + /** + * Return the definition of the fields available in this form. + * + * @return array + */ + public static function form_define_fields(): array { + return [ + 'curl' => ['type' => PARAM_TEXT], + 'destination' => ['type' => PARAM_PATH], + 'headers' => ['type' => PARAM_RAW], + 'method' => ['type' => PARAM_TEXT], + 'rawpostdata' => ['type' => PARAM_RAW], + 'sideeffects' => ['type' => PARAM_RAW], + 'timeout' => ['type' => PARAM_INT], + ]; + } + + /** + * A list of outputs and their description if applicable. + * + * These fields can be used as aliases in the custom output mapping + * + * @return array of outputs + */ + public function define_outputs(): array { + return [ + 'dbgcommand' => null, + 'response' => [ + 'result' => get_string('connector_curl:output_response_result', 'tool_dataflows'), + 'info' => [ + 'http_code' => null, + 'connect_time' => null, + 'total_time' => null, + 'size_upload' => null, + '*' => null, + ], + 'destination' => get_string('connector_curl:destination', 'tool_dataflows'), + ], + ]; + } + + /** + * Allows each step type to determine a list of optional/required form + * inputs for their configuration + * + * It's recommended you prefix the additional config related fields to avoid + * conflicts with any existing fields. + * + * @param \MoodleQuickForm $mform + */ + public function form_add_custom_inputs(\MoodleQuickForm &$mform) { + $jsonexample = [ + 'header-key' => 'header value', + 'another-key' => '1234', + ]; + $ex['json'] = \html_writer::nonempty_tag('pre', json_encode($jsonexample, JSON_PRETTY_PRINT)); + $ex['yaml'] = '
header-key: header value
another-key: 1234
'; + + $urlarray = []; + $urlarray[] =& $mform->createElement('select', 'config_method', '', [ + 'get' => 'GET', + 'post' => 'POST', + 'head' => 'HEAD', + 'patch' => 'PATCH', + 'put' => 'PUT', + ]); + $urlarray[] =& $mform->createElement('text', 'config_curl', ''); + + $mform->addGroup($urlarray, 'buttonar', get_string('connector_curl:curl', 'tool_dataflows'), [' '], false); + $mform->addRule('buttonar', get_string('required'), 'required', null, 'server'); + + $mform->addElement('textarea', 'config_headers', get_string('connector_curl:headers', 'tool_dataflows'), + ['cols' => 50, 'rows' => 7]); + $mform->addElement('static', 'headers_help', '', get_string('connector_curl:field_headers_help', 'tool_dataflows', $ex)); + + $mform->addElement('textarea' , 'config_rawpostdata', get_string('connector_curl:rawpostdata', 'tool_dataflows'), + ['cols' => 50, 'rows' => 7]); + + $mform->addElement('text', 'config_destination', get_string('connector_curl:destination', 'tool_dataflows')); + $mform->addHelpButton('config_destination', 'connector_curl:destination', 'tool_dataflows'); + $mform->addElement('static', 'config_path_help', '', get_string('path_help', 'tool_dataflows'). + \html_writer::nonempty_tag('pre', get_string('path_help_examples', 'tool_dataflows'))); + + $mform->addElement('checkbox', 'config_sideeffects', get_string('connector_curl:sideeffects', 'tool_dataflows'), + get_string('yes')); + $mform->addHelpButton('config_sideeffects', 'connector_curl:sideeffects', 'tool_dataflows'); + + $mform->hideIf('config_rawpostdata', 'config_method', 'eq', 'get'); + $mform->disabledIf('config_rawpostdata', 'config_method', 'eq', 'get'); + + $mform->addElement('text', 'config_timeout', get_string('connector_curl:timeout', 'tool_dataflows')); + $mform->addHelpButton('config_timeout', 'connector_curl:timeout', 'tool_dataflows'); + } + + /** + * Validate the configuration settings. + * + * @param object $config + * @return true|\lang_string[] true if valid, an array of errors otherwise + */ + public function validate_config($config) { + $errors = []; + if (empty($config->curl)) { + $errors['config_curl'] = get_string('config_field_missing', 'tool_dataflows', 'curl', true); + } + if (empty($config->rawpostdata) && ($config->method === 'put' || $config->method === 'post')) { + $errors['config_rawpostdata'] = get_string('config_field_missing', 'tool_dataflows', 'rawpostdata', true); + } + return empty($errors) ? true : $errors; + } + + /** + * Perform any extra validation that is required only for runs. + * + * @return true|array Will return true or an array of errors. + */ + public function validate_for_run() { + $config = $this->stepdef->config; + + if (!empty($config->destination)) { + $error = helper::path_validate($config->destination); + if ($error !== true) { + return ['config_destination' => $error]; + } + } + + return true; + } + + /** + * Performs a cURL call with the provided configuration. + * + * @return bool|object Returns false on failure, otherwise passes the input through. + */ + public function execute($input) { + // Get variables. + $config = $this->enginestep->stepdef->config; + $isdryrun = $this->enginestep->engine->isdryrun; + $method = $config->method; + + $this->enginestep->log($config->curl); + + $dbgcommand = 'curl -X ' . strtoupper($method) . ' ' . $config->curl; + $result = null; + + if (!empty($config->timeout)) { + $this->timeout = (int) $config->timeout; + $dbgcommand .= ' --max-time ' . $config->timeout; + } + + $options = ['CURLOPT_TIMEOUT' => $this->timeout]; + + if ($method === 'post') { + $options['CURLOPT_POST'] = 1; + } + + if ($method === 'put') { + $options['CURLOPT_PUT'] = 1; + } + + $curl = new \curl(); + + // Provided a header is specified add header to request. + if (!empty($config->headers)) { + $headers = $config->headers; + if (!is_array($headers)) { + $headers = json_decode($headers, true); + } + if (is_null($headers)) { + $headers = Yaml::parse($config->headers); + } + $curl->setHeader($headers); + $dbgcommand .= ' -H \'' . $config->headers . '\''; + } + + // Sets post data. + if (!empty($config->rawpostdata)) { + $options['CURLOPT_POSTFIELDS'] = $config->rawpostdata; + $dbgcommand .= ' -d \'' . $config->rawpostdata . '\''; + } + + // Download response to file provided destination is set. + if (!empty($config->destination) && !$isdryrun) { + if ($config->destination[0] === '/') { + $config->destination = ltrim($config->destination, '/'); + } + $config->destination = $this->enginestep->engine->resolve_path($config->destination); + $file = fopen($config->destination, 'w'); + $options['CURLOPT_FILE'] = $file; + } + + // Perform call. + if (!$isdryrun) { + $result = $curl->$method($config->curl, [], $options); + } + + if (!empty($file)) { + fclose($file); + } + + $info = $curl->get_info(); + // Stores response to be reusable by other steps. + // TODO : Once set_var api is refactored add response. + $response = $curl->getResponse(); + $httpcode = $info['http_code'] ?? null; + $destination = !empty($config->destination) ? $config->destination : null; + $errno = $curl->get_errno(); + + if (($httpcode >= 400 || empty($response) || $errno == 28) && !$isdryrun) { + throw new \moodle_exception($httpcode . ':' . $result); + } + + // Log the raw curl command. + $this->enginestep->log($dbgcommand); + $this->set_variables('dbgcommand', $dbgcommand); + + if (!$isdryrun) { + // TODO: It would be good to define and list any fixed but exposed + // fields which the user can use and map to on the edit page. + $this->set_variables('response', (object) [ + 'result' => $result, + 'info' => (object) $info, + 'destination' => $destination, + ]); + } + return true; + } +} diff --git a/classes/local/step/flow_step.php b/classes/local/step/flow_step.php index 6902f46a..6e826402 100644 --- a/classes/local/step/flow_step.php +++ b/classes/local/step/flow_step.php @@ -70,6 +70,12 @@ public function execute($input) { */ protected function generate_engine_step(engine $engine): engine_step { return new flow_engine_step($engine, $this->stepdef, $this); + if (!isset($this->enginestep)) { + $enginestep = new flow_engine_step($engine, $this->stepdef, $this); + $this->enginestep = $enginestep; + } + + return $this->enginestep; } /** diff --git a/classes/step.php b/classes/step.php index 4cf53bbd..f57d49cf 100644 --- a/classes/step.php +++ b/classes/step.php @@ -704,6 +704,7 @@ protected function validate_link_count(int $count, string $inputoutput, string $ * @throws \coding_exception */ protected function validate_links(array $deps, string $inputoutput) { + return true; $count = count($deps); $errors = []; diff --git a/lib.php b/lib.php index 40c7072d..cc85d6eb 100644 --- a/lib.php +++ b/lib.php @@ -48,6 +48,7 @@ function tool_dataflows_after_config() { */ function tool_dataflows_step_types() { return [ + new step\curl_step, new step\connector_curl, new step\connector_debugging, new step\connector_debug_file_display, From c37e29cc7753d297f7f0da94465d5804c0bcd32b Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 21:22:35 +1000 Subject: [PATCH 04/13] wip: adjust curl step, add helper methods to ease authoring --- classes/local/step/base_step.php | 33 ++++++++++++++++++++++++++++++++ classes/local/step/curl_step.php | 22 +++++++++++---------- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/classes/local/step/base_step.php b/classes/local/step/base_step.php index 2eac69ce..86af2eba 100644 --- a/classes/local/step/base_step.php +++ b/classes/local/step/base_step.php @@ -607,4 +607,37 @@ public function get_output_label(int $position): string { $label = $labels[$position] ?? (string) $position; return $label; } + + /** + * Get the step's (definition) current config. + * + * Helper method to reduce the complexity when authoring step types. + * + * @return \stdClass configuration object + */ + protected function get_config() { + return $this->enginestep->stepdef->config; + } + + /** + * Returns whether the engine's run is dry + * + * Helper method to reduce the complexity when authoring step types. + * + * @return bool mode is in dry run or not + */ + protected function is_dry_run() { + return $this->enginestep->engine->isdryrun; + } + + /** + * Emits a log message + * + * Helper method to reduce the complexity when authoring step types. + * + * @param string $message + */ + protected function log($message) { + $this->enginestep->log($message); + } } diff --git a/classes/local/step/curl_step.php b/classes/local/step/curl_step.php index 300b142e..de59dcdc 100644 --- a/classes/local/step/curl_step.php +++ b/classes/local/step/curl_step.php @@ -129,7 +129,7 @@ public function form_add_custom_inputs(\MoodleQuickForm &$mform) { 'patch' => 'PATCH', 'put' => 'PUT', ]); - $urlarray[] =& $mform->createElement('text', 'config_curl', ''); + $urlarray[] =& $mform->createElement('text', 'config_curl', '', ['style' => 'width: 36rem']); $mform->addGroup($urlarray, 'buttonar', get_string('connector_curl:curl', 'tool_dataflows'), [' '], false); $mform->addRule('buttonar', get_string('required'), 'required', null, 'server'); @@ -199,14 +199,14 @@ public function validate_for_run() { */ public function execute($input) { // Get variables. - $config = $this->enginestep->stepdef->config; - $isdryrun = $this->enginestep->engine->isdryrun; - $method = $config->method; + $config = $this->get_config(); + $isdryrun = $this->is_dry_run(); - $this->enginestep->log($config->curl); + $url = $config->curl; + $method = $config->method; - $dbgcommand = 'curl -X ' . strtoupper($method) . ' ' . $config->curl; - $result = null; + $this->log($url); + $dbgcommand = 'curl -X ' . strtoupper($method) . ' ' . $url; if (!empty($config->timeout)) { $this->timeout = (int) $config->timeout; @@ -255,8 +255,9 @@ public function execute($input) { } // Perform call. + $result = null; if (!$isdryrun) { - $result = $curl->$method($config->curl, [], $options); + $result = $curl->$method($url, [], $options); } if (!empty($file)) { @@ -276,7 +277,7 @@ public function execute($input) { } // Log the raw curl command. - $this->enginestep->log($dbgcommand); + $this->log($dbgcommand); $this->set_variables('dbgcommand', $dbgcommand); if (!$isdryrun) { @@ -288,6 +289,7 @@ public function execute($input) { 'destination' => $destination, ]); } - return true; + + return $input; } } From d76b6d808a86a49feccd9b4faaf0794827c53f5a Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 22:10:05 +1000 Subject: [PATCH 05/13] wip: flip the check in the connector engine step because iterator-esque steps no longer return true. Instead they should explicitly return false when things go bad. --- classes/local/execution/connector_engine_step.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/local/execution/connector_engine_step.php b/classes/local/execution/connector_engine_step.php index dfafea42..5f96467e 100644 --- a/classes/local/execution/connector_engine_step.php +++ b/classes/local/execution/connector_engine_step.php @@ -46,7 +46,7 @@ public function go(): int { try { $result = $this->steptype->execute($this); $this->steptype->prepare_outputs(); - if ($result === true) { + if ($result !== false) { $this->set_status(engine::STATUS_FINISHED); } else { $this->set_status(engine::STATUS_CANCELLED); From 26b8970d0a2b82c5f7a5015fa4f84720193ed1a4 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 22:10:31 +1000 Subject: [PATCH 06/13] everybody needs variables, even connectors --- classes/local/execution/engine_step.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/classes/local/execution/engine_step.php b/classes/local/execution/engine_step.php index 2e4c1a18..c4bcfeb3 100644 --- a/classes/local/execution/engine_step.php +++ b/classes/local/execution/engine_step.php @@ -293,4 +293,26 @@ protected function on_change_status() { break; } } + + /** + * Returns an array with all the variables available, with the context of the step + * + * @return array + */ + public function get_variables(): array { + // Config values are directly referenceable, step values go through + // step.fieldname, everything else is available through expressions, + // such as 'dataflow.id' and 'steps.mystep.name' for example. + $variables = $this->engine->get_variables(); + $step = $variables['steps']->{$this->stepdef->alias}; + + // Pull out the config. + $config = $step->config; + + return array_merge( + $variables, + ['step' => $step], + (array) $config, + ); + } } From 7dbffb9fc76b62e37143c8053c2090854466fc1f Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 5 Aug 2022 22:12:31 +1000 Subject: [PATCH 07/13] the magic: if the step type is resolved to a flow, then it is a flow step, otherwise it is a connector step. We can now use this alongside config to determine which type it is. Certain step types will be fixed (so they cannot change) e.g. readers, writers? --- classes/local/step/flow_step.php | 15 +++++++++------ lang/en/tool_dataflows.php | 1 + 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/classes/local/step/flow_step.php b/classes/local/step/flow_step.php index 6e826402..78150667 100644 --- a/classes/local/step/flow_step.php +++ b/classes/local/step/flow_step.php @@ -16,6 +16,7 @@ namespace tool_dataflows\local\step; +use tool_dataflows\local\execution\connector_engine_step; use tool_dataflows\local\execution\engine; use tool_dataflows\local\execution\engine_step; use tool_dataflows\local\execution\iterators\iterator; @@ -43,6 +44,9 @@ abstract class flow_step extends base_step { * @return bool */ final public function is_flow(): bool { + if ($this->stepdef->id === 131) { + return false; + } return true; } @@ -69,13 +73,12 @@ public function execute($input) { * @return engine_step */ protected function generate_engine_step(engine $engine): engine_step { - return new flow_engine_step($engine, $this->stepdef, $this); - if (!isset($this->enginestep)) { - $enginestep = new flow_engine_step($engine, $this->stepdef, $this); - $this->enginestep = $enginestep; + // Determine if it should return a flow (iterator based) or connector (no iterator) engine step. + if ($this->is_flow()) { + return new flow_engine_step($engine, $this->stepdef, $this); + } else { + return new connector_engine_step($engine, $this->stepdef, $this); } - - return $this->enginestep; } /** diff --git a/lang/en/tool_dataflows.php b/lang/en/tool_dataflows.php index 752ab81e..b99589c8 100644 --- a/lang/en/tool_dataflows.php +++ b/lang/en/tool_dataflows.php @@ -104,6 +104,7 @@ $string['strftimedatetimeaccurate'] = '%d %B %Y, %I:%M:%S %p'; // Step names. +$string['step_name_curl_step'] = 'cURL'; $string['step_name_connector_curl'] = 'Curl connector'; $string['step_name_connector_debugging'] = 'Debugging connector'; $string['step_name_connector_s3'] = 'S3 file copy connector'; From cf43319451e9f82c638569866c9d273b67a15e1b Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Sat, 6 Aug 2022 14:55:06 +1000 Subject: [PATCH 08/13] refactor: simplify generate_engine_step methods for flow/connector steps --- classes/local/step/base_step.php | 15 +++++++++++++-- classes/local/step/connector_step.php | 12 ------------ classes/local/step/flow_step.php | 17 ----------------- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/classes/local/step/base_step.php b/classes/local/step/base_step.php index 86af2eba..5181b9b6 100644 --- a/classes/local/step/base_step.php +++ b/classes/local/step/base_step.php @@ -18,8 +18,10 @@ use Symfony\Component\Yaml\Yaml; use tool_dataflows\helper; +use tool_dataflows\local\execution\connector_engine_step; use tool_dataflows\local\execution\engine; use tool_dataflows\local\execution\engine_step; +use tool_dataflows\local\execution\flow_engine_step; use tool_dataflows\parser; use tool_dataflows\step; @@ -482,12 +484,21 @@ final public function get_engine_step(): ?engine_step { } /** - * Generate the engine step for the step type. + * Generates an engine step for this type. + * + * This should be sufficient for most cases. Override this function if needed. * * @param engine $engine * @return engine_step */ - abstract protected function generate_engine_step(engine $engine): engine_step; + protected function generate_engine_step(engine $engine): engine_step { + // Determine if it should return a flow (iterator based) or connector (no iterator) engine step. + if ($this->is_flow()) { + return new flow_engine_step($engine, $this->stepdef, $this); + } + + return new connector_engine_step($engine, $this->stepdef, $this); + } /** * Returns the group (string) this step type is categorised under. diff --git a/classes/local/step/connector_step.php b/classes/local/step/connector_step.php index 1167ee72..6fdd135b 100644 --- a/classes/local/step/connector_step.php +++ b/classes/local/step/connector_step.php @@ -57,18 +57,6 @@ public function get_group(): string { return 'connectors'; } - /** - * Generates an engine step for this type. - * - * This should be sufficient for most cases. Override this function if needed. - * - * @param engine $engine - * @return engine_step - */ - protected function generate_engine_step(engine $engine): engine_step { - return new connector_engine_step($engine, $this->stepdef, $this); - } - /** * Returns an array with styles used to draw the dot graph * diff --git a/classes/local/step/flow_step.php b/classes/local/step/flow_step.php index 78150667..a9405d44 100644 --- a/classes/local/step/flow_step.php +++ b/classes/local/step/flow_step.php @@ -64,23 +64,6 @@ public function execute($input) { return $input; } - /** - * Generates an engine step for this type. - * - * This should be sufficient for most cases. Override this function if needed. - * - * @param engine $engine - * @return engine_step - */ - protected function generate_engine_step(engine $engine): engine_step { - // Determine if it should return a flow (iterator based) or connector (no iterator) engine step. - if ($this->is_flow()) { - return new flow_engine_step($engine, $this->stepdef, $this); - } else { - return new connector_engine_step($engine, $this->stepdef, $this); - } - } - /** * Get the iterator for the step, based on configurations. * From 4cf39794d1e238ddab81f420a106a0d60ba12793 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Sat, 6 Aug 2022 16:37:52 +1000 Subject: [PATCH 09/13] add connector flag in steps table, update dot graph handling to visually display this change and fix side effect flags --- classes/form/step_form.php | 18 ++++++++++------ classes/local/step/base_step.php | 2 +- classes/local/step/flow_step.php | 36 +++++++++++++++++++++++--------- classes/step.php | 3 ++- db/install.xml | 3 ++- db/upgrade.php | 16 ++++++++++++++ version.php | 4 ++-- 7 files changed, 61 insertions(+), 21 deletions(-) diff --git a/classes/form/step_form.php b/classes/form/step_form.php index 7e0ad115..5feb15d5 100644 --- a/classes/form/step_form.php +++ b/classes/form/step_form.php @@ -87,6 +87,10 @@ public function definition() { ); $select->setMultiple(true); + // Is like a connector step? Does not operate within a flow group. + $mform->addElement('advcheckbox', 'connector', 'Behave like a connector step? (WIP)', false); + $mform->setType('connector', PARAM_BOOL); + // List all the available fields available for configuration, in dot syntax. $variables = $this->get_available_references(); $mform->addElement('html', $this->prepare_available_fields($variables)); @@ -362,12 +366,14 @@ protected function get_default_data() { $steptype = new $type(); $data = $steptype->form_get_default_data($data); - // Automatically fill in the name with something hopefully sane. - $classname = $type; - $position = strrpos($classname, '\\'); - $basename = substr($classname, $position + 1); - if ($position !== false) { - $data->name = str_replace('_step', '', $basename); // So curl_step becomes curl. + // Automatically fill in the name with something hopefully sane for new steps. + if (empty($data->id)) { + $classname = $type; + $position = strrpos($classname, '\\'); + $basename = substr($classname, $position + 1); + if ($position !== false) { + $data->name = str_replace('_step', '', $basename); // So curl_step becomes curl. + } } } diff --git a/classes/local/step/base_step.php b/classes/local/step/base_step.php index 5181b9b6..32bfda31 100644 --- a/classes/local/step/base_step.php +++ b/classes/local/step/base_step.php @@ -627,7 +627,7 @@ public function get_output_label(int $position): string { * @return \stdClass configuration object */ protected function get_config() { - return $this->enginestep->stepdef->config; + return $this->enginestep->stepdef->config ?? $this->stepdef->config; } /** diff --git a/classes/local/step/flow_step.php b/classes/local/step/flow_step.php index a9405d44..a9491b98 100644 --- a/classes/local/step/flow_step.php +++ b/classes/local/step/flow_step.php @@ -44,7 +44,7 @@ abstract class flow_step extends base_step { * @return bool */ final public function is_flow(): bool { - if ($this->stepdef->id === 131) { + if ($this->stepdef->connector == 1) { return false; } return true; @@ -92,16 +92,32 @@ public function get_group(): string { */ public function get_node_styles(): array { $basestyles = parent::get_node_styles(); - $styles = [ - 'shape' => 'rect', - 'fillcolor' => '#8cd0db', - 'fontcolor' => '#000000', - 'style' => 'filled,rounded', - ]; - if ($this->has_side_effect()) { - $styles['fillcolor'] = '#008196'; - $styles['fontcolor'] = '#ffffff'; + if ($this->is_flow()) { + // Flow styles. + $styles = [ + 'shape' => 'rect', + 'fillcolor' => '#8cd0db', + 'fontcolor' => '#000000', + 'style' => 'filled,rounded', + ]; + + if ($this->has_side_effect()) { + $styles['fillcolor'] = '#008196'; + $styles['fontcolor'] = '#ffffff'; + } + } else { + // Connector styles. + $styles = [ + 'shape' => 'record', + 'fillcolor' => '#cccccc', + ]; + + if ($this->has_side_effect()) { + $styles['shape'] = 'rect'; + $styles['style'] = 'filled'; + $styles['fillcolor'] = '#ffc107'; + } } return array_merge($basestyles, $styles); diff --git a/classes/step.php b/classes/step.php index f57d49cf..9e698790 100644 --- a/classes/step.php +++ b/classes/step.php @@ -106,6 +106,7 @@ protected static function define_properties(): array { 'description' => ['type' => PARAM_TEXT, 'default' => ''], 'type' => ['type' => PARAM_TEXT], 'name' => ['type' => PARAM_TEXT], + 'connector' => ['type' => PARAM_BOOL, 'default' => false], 'config' => ['type' => PARAM_TEXT, 'default' => ''], 'timecreated' => ['type' => PARAM_INT, 'default' => 0], 'userid' => ['type' => PARAM_INT, 'default' => 0], @@ -829,7 +830,7 @@ public function get_dotscript($contentonly = false): string { $classname = $this->type; if (class_exists($classname)) { // Styles specific for this step type. - $steptype = new $classname(); + $steptype = $this->steptype; $stepstyles = $steptype->get_node_styles(); } else { // Invalid step type styles. diff --git a/db/install.xml b/db/install.xml index c0a6cc39..97b592d3 100644 --- a/db/install.xml +++ b/db/install.xml @@ -1,5 +1,5 @@ - @@ -37,6 +37,7 @@ + diff --git a/db/upgrade.php b/db/upgrade.php index ffba67b3..f4f86f37 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -128,5 +128,21 @@ function xmldb_tool_dataflows_upgrade($oldversion) { } } + + if ($oldversion < 2022080600) { + + // Define field connector to be added to tool_dataflows_steps. + $table = new xmldb_table('tool_dataflows_steps'); + $field = new xmldb_field('connector', XMLDB_TYPE_INTEGER, '1', null, null, null, null, 'usermodified'); + + // Conditionally launch add field connector. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Dataflows savepoint reached. + upgrade_plugin_savepoint(true, 2022080600, 'tool', 'dataflows'); + } + return true; } diff --git a/version.php b/version.php index f621becd..2a325e54 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2022080500; -$plugin->release = 2022080500; +$plugin->version = 2022080600; +$plugin->release = 2022080600; $plugin->requires = 2017051500; // Our lowest supported Moodle (3.3.0). From dc4ca2e769cbbe5629de88448fd329ce586aa3ab Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Sat, 6 Aug 2022 16:43:55 +1000 Subject: [PATCH 10/13] fix: step chooser / new step form (side quest) --- classes/local/step/connector_curl.php | 2 +- classes/local/step/connector_s3.php | 2 +- classes/local/step/curl_step.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/classes/local/step/connector_curl.php b/classes/local/step/connector_curl.php index 231a5c80..29607f14 100644 --- a/classes/local/step/connector_curl.php +++ b/classes/local/step/connector_curl.php @@ -44,7 +44,7 @@ class connector_curl extends connector_step { * @link https://en.wikipedia.org/wiki/Side_effect_(computer_science) */ public function has_side_effect(): bool { - if (isset($this->stepdef)) { + if (!empty($this->stepdef->id)) { $config = $this->stepdef->config; // Destination is outside of scratch directory. diff --git a/classes/local/step/connector_s3.php b/classes/local/step/connector_s3.php index 6ac653c6..b15f5f7c 100644 --- a/classes/local/step/connector_s3.php +++ b/classes/local/step/connector_s3.php @@ -42,7 +42,7 @@ class connector_s3 extends connector_step { * @link https://en.wikipedia.org/wiki/Side_effect_(computer_science) */ public function has_side_effect(): bool { - if (isset($this->stepdef)) { + if (!empty($this->stepdef->id)) { $config = $this->stepdef->config; return !helper::path_is_relative($config->target); } diff --git a/classes/local/step/curl_step.php b/classes/local/step/curl_step.php index de59dcdc..ccec3c6a 100644 --- a/classes/local/step/curl_step.php +++ b/classes/local/step/curl_step.php @@ -43,7 +43,7 @@ class curl_step extends flow_step { * @return bool whether or not this step has a side effect */ public function has_side_effect(): bool { - if (isset($this->stepdef)) { + if (!empty($this->stepdef->id)) { $config = $this->stepdef->config; // Destination is outside of scratch directory. From e084bb7c63ad582f7bb361cc69e7e00a714e03da Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Sat, 6 Aug 2022 17:44:17 +1000 Subject: [PATCH 11/13] For connector-like steps, the input isn't real and so should be an empty object. Thinking about removing this entirely actually, since the input is irrelevant in most cases unless it needs to be altered --- classes/local/execution/connector_engine_step.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/local/execution/connector_engine_step.php b/classes/local/execution/connector_engine_step.php index 5f96467e..8ba4e072 100644 --- a/classes/local/execution/connector_engine_step.php +++ b/classes/local/execution/connector_engine_step.php @@ -44,7 +44,7 @@ public function go(): int { switch ($this->proceed_status()) { case self::PROCEED_GO: try { - $result = $this->steptype->execute($this); + $result = $this->steptype->execute(new \stdClass); $this->steptype->prepare_outputs(); if ($result !== false) { $this->set_status(engine::STATUS_FINISHED); From 9ed4442696772953fc61876f5d719c4f3c015c67 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Sat, 6 Aug 2022 17:46:47 +1000 Subject: [PATCH 12/13] changed curl_step to curl, updated handling of prefilled names for new steps --- classes/form/step_form.php | 2 +- classes/local/step/{curl_step.php => curl.php} | 2 +- lang/en/tool_dataflows.php | 2 +- lib.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename classes/local/step/{curl_step.php => curl.php} (99%) diff --git a/classes/form/step_form.php b/classes/form/step_form.php index 5feb15d5..8805e97c 100644 --- a/classes/form/step_form.php +++ b/classes/form/step_form.php @@ -372,7 +372,7 @@ protected function get_default_data() { $position = strrpos($classname, '\\'); $basename = substr($classname, $position + 1); if ($position !== false) { - $data->name = str_replace('_step', '', $basename); // So curl_step becomes curl. + $data->name = $basename; } } } diff --git a/classes/local/step/curl_step.php b/classes/local/step/curl.php similarity index 99% rename from classes/local/step/curl_step.php rename to classes/local/step/curl.php index ccec3c6a..32fb94b2 100644 --- a/classes/local/step/curl_step.php +++ b/classes/local/step/curl.php @@ -28,7 +28,7 @@ * @copyright Catalyst IT, 2022 * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class curl_step extends flow_step { +class curl extends flow_step { /** @var int time after which curl request is aborted */ protected $timeout = 60; diff --git a/lang/en/tool_dataflows.php b/lang/en/tool_dataflows.php index b99589c8..199af8fc 100644 --- a/lang/en/tool_dataflows.php +++ b/lang/en/tool_dataflows.php @@ -104,7 +104,7 @@ $string['strftimedatetimeaccurate'] = '%d %B %Y, %I:%M:%S %p'; // Step names. -$string['step_name_curl_step'] = 'cURL'; +$string['step_name_curl'] = 'cURL'; $string['step_name_connector_curl'] = 'Curl connector'; $string['step_name_connector_debugging'] = 'Debugging connector'; $string['step_name_connector_s3'] = 'S3 file copy connector'; diff --git a/lib.php b/lib.php index cc85d6eb..a44a0453 100644 --- a/lib.php +++ b/lib.php @@ -48,7 +48,7 @@ function tool_dataflows_after_config() { */ function tool_dataflows_step_types() { return [ - new step\curl_step, + new step\curl, new step\connector_curl, new step\connector_debugging, new step\connector_debug_file_display, From 396474876b1a4fc51f973ce75c91bbadac4da0ea Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Sat, 6 Aug 2022 17:59:05 +1000 Subject: [PATCH 13/13] fix weird issue where has_side_effect wouldn't enter probably magic getters --- classes/local/step/curl.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/classes/local/step/curl.php b/classes/local/step/curl.php index 32fb94b2..575b3daa 100644 --- a/classes/local/step/curl.php +++ b/classes/local/step/curl.php @@ -43,7 +43,8 @@ class curl extends flow_step { * @return bool whether or not this step has a side effect */ public function has_side_effect(): bool { - if (!empty($this->stepdef->id)) { + $id = $this->stepdef->id; + if ($id) { $config = $this->stepdef->config; // Destination is outside of scratch directory.