Skip to content

Commit

Permalink
Merge pull request #738 from catalyst/set-variable-dryrun
Browse files Browse the repository at this point in the history
Issue #729: Fix set variable step to not persist during dry runs.
  • Loading branch information
keevan authored May 8, 2023
2 parents 0af62f8 + ce98719 commit 47cbcc7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
2 changes: 1 addition & 1 deletion classes/local/step/gpg_trait.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function execute($input = null) {

// We do not need to go any further if it is a dry run.
if ($this->is_dry_run() && $this->has_side_effect()) {
return true;
return $input;
}

$output = [];
Expand Down
28 changes: 24 additions & 4 deletions classes/local/step/set_variable_trait.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,29 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
trait set_variable_trait {
/**
* Returns whether or not the step configured, has a side effect
*
* A side effect if it modifies some state variable value(s) outside its
* local environment, which is to say if it has any observable effect other
* than its primary effect of returning a value to the invoker of the
* operation
*
* @return bool whether or not this step has a side effect
* @link https://en.wikipedia.org/wiki/Side_effect_(computer_science)
*/
public function has_side_effect(): bool {
return true;
}

/**
* Executes the step, fetcing the config and actioning the step.
* Executes the step, fetching the config and actioning the step.
*
* @param mixed|null $input
* @return mixed
*/
public function execute($input = null) {

$stepvars = $this->get_variables();
$config = $stepvars->get('config');
$rootvars = $this->get_variables_root();
Expand All @@ -59,6 +74,14 @@ public function execute($input = null) {
* @param mixed $value
*/
public function run(var_root $varobject, string $field, $value) {
// Set the value in the variable tree.
$varobject->set($field, $value);

// We do not persist the value if it is a dry run.
if ($this->is_dry_run()) {
return;
}

// Check and persist the change if the field points to a dataflow vars.
$levels = var_object_visible::name_to_levels($field);
if (
Expand All @@ -68,9 +91,6 @@ public function run(var_root $varobject, string $field, $value) {
) {
$this->persist_dataflow_vars($levels, $value);
}

// Set the value in the variable tree.
$varobject->set($field, $value);
}

/**
Expand Down

0 comments on commit 47cbcc7

Please sign in to comment.