diff --git a/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php b/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php index cceb3b7afe..db1b56d739 100644 --- a/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php +++ b/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php @@ -4,8 +4,6 @@ namespace MailPoet\Automation\Engine\Data; use DateTimeImmutable; use InvalidArgumentException; -use MailPoet\Automation\Engine\Exceptions\InvalidStateException; -use MailPoet\Automation\Engine\Exceptions\UnexpectedValueException; use MailPoet\Automation\Engine\Utils\Json; use Throwable; @@ -92,16 +90,8 @@ class WorkflowRunLog { * @return void */ public function setData(string $key, $value): void { - try { - $newData = $this->getData(); - $newData[$key] = $value; - $encoded = Json::encode($newData); - $decoded = Json::decode($encoded); - if ($decoded !== $newData) { - throw new InvalidArgumentException('$value must be serializable'); - } - } catch (InvalidStateException | InvalidArgumentException | UnexpectedValueException $e) { - throw new InvalidArgumentException("Invalid data provided for key $key."); + if (!$this->isDataStorable($value)) { + throw new InvalidArgumentException("Invalid data provided for key '$key'. Only scalar values and arrays of scalar values are allowed."); } $this->data[$key] = $value; } @@ -157,4 +147,30 @@ class WorkflowRunLog { return $workflowRunLog; } + + /** + * @param mixed $data + * @return bool + */ + private function isDataStorable($data): bool { + if (is_object($data)) { + return false; + } + + if (is_scalar($data)) { + return true; + } + + if (!is_array($data)) { + return false; + } + + foreach ($data as $value) { + if (!$this->isDataStorable($value)) { + return false; + } + } + + return true; + } } diff --git a/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php b/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php index 8a88bef0aa..98411c2702 100644 --- a/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php +++ b/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php @@ -1,4 +1,4 @@ -wp = new WPFunctions(); } - public function testItAllowsSettingData(): void { + public function testItAllowsSettingSimpleData(): void { $log = new WorkflowRunLog(1, 'step-id'); $this->assertSame([], $log->getData()); $log->setData('key', 'value'); @@ -58,16 +59,77 @@ class WorkflowRunLogTest extends \MailPoetTest { $this->assertSame('value', $data['key']); } - public function testItDoesNotAllowSettingDataThatCannotBeSaved(): void { + public function testItAllowsSettingArraysOfScalarValues(): void { + $log = new WorkflowRunLog(1, 'step-id'); + $data = [ + 'string', + 11.1, + 10, + true, + false + ]; + $log->setData('data', $data); + $this->workflowRunLogStorage->createWorkflowRunLog($log); + $retrieved = $this->workflowRunLogStorage->getLogsForWorkflowRun(1)[0]; + expect($retrieved->getData()['data'])->equals($data); + } + + public function testItAllowsSettingMultidimensionalArraysOfScalarValues(): void { + $log = new WorkflowRunLog(1, 'step-id'); + $data = [ + 'values' => [ + 'string', + 11.1, + 10, + true, + false + ] + ]; + $log->setData('data', $data); + $this->workflowRunLogStorage->createWorkflowRunLog($log); + $retrieved = $this->workflowRunLogStorage->getLogsForWorkflowRun(1)[0]; + expect($retrieved->getData()['data'])->equals($data); + } + + public function testItDoesNotAllowSettingDataThatIncludesClosures(): void { $log = new WorkflowRunLog(1, 'step-id'); $badData = [ - function() { echo 'closures cannot be serialized'; } + function() { + echo 'closures cannot be serialized'; + } ]; $this->expectException(\InvalidArgumentException::class); $log->setData('badData', $badData); expect($log->getData())->count(0); } + public function testItDoesNotAllowSettingObjectsForData(): void { + $log = new WorkflowRunLog(1, 'step-id'); + $object = new stdClass(); + $object->key = 'value'; + $this->expectException(\InvalidArgumentException::class); + $log->setData('object', $object); + expect($log->getData())->count(0); + } + + public function testItDoesNotAllowSettingMultidimensionalArrayThatContainsNonScalarValue(): void { + $log = new WorkflowRunLog(1, 'step-id'); + $data = [ + 'test' => [ + 'multidimensional' => [ + 'array' => [ + 'values' => [ + new stdClass() + ] + ] + ] + ] + ]; + $this->expectException(\InvalidArgumentException::class); + $log->setData('data', $data); + expect($log->getData())->count(0); + } + public function testItGetsExposedViaAction(): void { $this->wp->addAction(Hooks::WORKFLOW_RUN_LOG_AFTER_STEP_RUN, function(WorkflowRunLog $log) { $log->setData('test', 'value'); @@ -171,7 +233,9 @@ class WorkflowRunLogTest extends \MailPoetTest { private function getLogsForAction($callback = null) { if ($callback === null) { - $callback = function() { return true; }; + $callback = function() { + return true; + }; } $testAction = $this->getRegisteredTestAction($callback); $actionStep = new Step('action-step-id', Step::TYPE_ACTION, $testAction->getKey(), [], []); @@ -196,7 +260,9 @@ class WorkflowRunLogTest extends \MailPoetTest { private function getRegisteredTestAction($callback = null) { if ($callback === null) { - $callback = function() { return true; }; + $callback = function() { + return true; + }; } $action = new TestAction(); $action->setCallback($callback);