From ffddf09606b14285fef10f14eae66c60e0994db4 Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Tue, 17 Mar 2026 14:30:58 +0100 Subject: [PATCH 1/7] DEV AbstractAction added validateTask() --- CommonLogic/AbstractAction.php | 106 +++++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 12 deletions(-) diff --git a/CommonLogic/AbstractAction.php b/CommonLogic/AbstractAction.php index 2c012213d..2982fcb23 100644 --- a/CommonLogic/AbstractAction.php +++ b/CommonLogic/AbstractAction.php @@ -6,6 +6,7 @@ use exface\Core\Exceptions\Actions\ActionConfigurationError; use exface\Core\Interfaces\Actions\ActionConfirmationListInterface; use exface\Core\Interfaces\DataSheets\DataSheetInterface; +use exface\Core\Interfaces\Exceptions\ExceptionInterface; use exface\Core\Interfaces\Log\LoggerInterface; use exface\Core\Interfaces\Model\IAffectMetaObjectsInterface; use exface\Core\Interfaces\Model\MetaObjectInterface; @@ -347,6 +348,9 @@ public final function handle(TaskInterface $task, DataTransactionInterface $tran $transaction = $this->getWorkbench()->data()->startTransaction(); } + // TODO What's the correct response here? Throw, silent, message or something else. + $this->validateTask($task, $transaction); + $this->getWorkbench()->eventManager()->dispatch(new OnBeforeActionPerformedEvent($this, $task, $transaction, function() use ($task) { return $this->getInputDataSheet($task); })); @@ -387,6 +391,80 @@ public final function handle(TaskInterface $task, DataTransactionInterface $tran return $result; } + /** + * Validates a given task against the expectations of this action. Throws an error, when encountering issues and + * returns `void` if the task is valid for this action. + * + * Base validation ensures that: + * - The task object and action object match, provided both are defined. + * - The task input data only contains columns present in the action widget, provided both are defined. + * - If any components for a given validation step are missing, the step succeeds. Validation only fails, if the + * required data is present, but does not match expectations. + * + * @throws ExceptionInterface + * Throws an exception with a description of the violation. + */ + protected function validateTask(TaskInterface $task, DataTransactionInterface $transaction) : void + { + $taskObject = $task->hasMetaObject() ? $task->getMetaObject() : null; + $taskInput = $task->hasInputData() ? $task->getInputData() : null; + + // Ensure metaobjects match. + if($taskObject !== null && !$taskObject->isExactly($this->getMetaObject())) { + // See if any input mapper has a matching from object. + $hasMatchingInputMapper = false; + foreach ($this->getInputMappers() as $mapper) { + if($taskObject->isExactly($mapper->getFromMetaObject())) { + $hasMatchingInputMapper = true; + break; + } + } + + // If we couldn't match with an input mapper either, the task is invalid for this action. + if(!$hasMatchingInputMapper) { + throw (new ActionRuntimeError( + $this, + 'Action "' . $this->getAliasWithNamespace() . '" is defined for "' . + $this->getMetaObject()->getAliasWithNamespace() . '", but received a task with object "' . + $task->getMetaObject()->getAliasWithNamespace() . '"!' + ))->setUseExceptionMessageAsTitle(true); + } + } + + // If the metaobjects match, we can check if the input data matches expectations. + $widget = null; + if($task->isTriggeredByWidget()) { + $widget = $task->getWidgetTriggeredBy(); + } elseif ($this->isDefinedInWidget()) { + $widget = $this->getWidgetDefinedIn(); + } + + // TODO How to properly extract widget data expectations? + $expectedData = $widget?->getPrefillData() ?? $widget?->prepareDataSheetToRead(); + if($taskInput !== null && $expectedData !== null) { + $expectedColumns = $expectedData->getColumns(); + $unexpectedColumns = []; + + // Now check if there are input columns that are not present in our expected structure, which would imply + // a mistake or unauthorized attempts at accessing or manipulating data. Note that missing input columns + // are of no concern here, since they might be handled later by mappers or prototype specific logic. + foreach ($taskInput->getColumns() as $inputColumn) { + $inputColumnName = $inputColumn->getName(); + if(!$expectedColumns->has($inputColumnName)) { + $unexpectedColumns[] = '"' . $inputColumnName . '"'; + } + } + + if(!empty($unexpectedColumns)) { + throw (new ActionRuntimeError( + $this, + 'Unexpected task input columns detected for action "' . $this->getAliasWithNamespace() . + '": ' . implode(', ', $unexpectedColumns) . '!' + ))->setUseExceptionMessageAsTitle(true); + } + } + } + /** * * {@inheritdoc} @@ -483,9 +561,10 @@ public function setInputDataSheet(UxonObject $uxon) : ActionInterface * the result message and (if needed) a separate result object should be set within the specific implementation via * set_result_data_sheet(), set_result_message() and set_result() respectively! * - * This method is protected because only get_result...() methods are intended to be used by external objects. In addition to performing - * the action they also take care of saving it to the current context, etc., while perform() ist totally depending on the specific - * action implementation and holds only the actual logic without all the overhead. + * This method is protected because only get_result...() methods are intended to be used by external objects. In + * addition to performing the action they also take care of saving it to the current context, etc., while perform() + * ist totally depending on the specific action implementation and holds only the actual logic without all the + * overhead. * * @return void */ @@ -757,8 +836,8 @@ public function getDisabledBehaviors() /** * Returns the translation string for the given message id. * - * This is a shortcut for calling $this->getApp()->getTranslator()->translate(). Additionally it will automatically append an - * action prefix to the given id: e.g. $action->translate('SOME_MESSAGE') will result in + * This is a shortcut for calling $this->getApp()->getTranslator()->translate(). Additionally it will automatically + * append an action prefix to the given id: e.g. $action->translate('SOME_MESSAGE') will result in * $action->getApp()->getTranslator()->translate('ACTION.ALIAS.SOME_MESSAGE') * * @see Translation::translate() @@ -1069,7 +1148,8 @@ public function setInputMappers(UxonObject $uxon) * * @uxon-property input_mapper * @uxon-type \exface\Core\CommonLogic\DataSheets\DataSheetMapper - * @uxon-template {"from_object_alias": "", "to_object_alias": "", "column_to_column_mappings": [{"from": "", "to": ""}]} + * @uxon-template {"from_object_alias": "", "to_object_alias": "", "column_to_column_mappings": [{"from": "", "to": + * ""}]} * * @see setInputMappers() * @see \exface\Core\Interfaces\Actions\ActionInterface::setInputMapper() @@ -1691,7 +1771,8 @@ protected function getEffectsFromTriggerWidget(iTriggerAction $button, MetaObjec } /** - * Objects and relations that may be affected by the action (in addition to those determined by the action logic automatically). + * Objects and relations that may be affected by the action (in addition to those determined by the action logic + * automatically). * * Most effects of an action can be determined automatically. If not, you can add them * here manually. For example: @@ -1903,7 +1984,8 @@ public function getInputChecks() : ActionDataCheckListInterface * * @uxon-property input_invalid_if * @uxon-type \exface\Core\CommonLogic\DataSheets\DataCheck[] - * @uxon-template [{"error_text": "", "operator": "AND", "conditions": [{"expression": "", "comparator": "", "value": ""}]}] + * @uxon-template [{"error_text": "", "operator": "AND", "conditions": [{"expression": "", "comparator": "", + * "value": ""}]}] * * @param UxonObject $arrayOfDataChecks * @return AbstractAction @@ -2107,13 +2189,13 @@ public function getMonitorAsLongRunningAfterSeconds(?int $default = null) : int| * an alert depending on the monitor configuration. * * Logging long-running actions must be explicitly enabled in `MONITOR.LONG_RUNNERS.ENABLED` System.config.json. - * If enabled, all actions taking longer than `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` will be logged to the - * monitor. + * If enabled, all actions taking longer than `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` will be logged to + * the monitor. * * - Any value >= 0 will override the config setting. * - You can disable this feature by setting the threshold to `-1` or `false`. - * - Not setting this property explicitly applies the default threshold defined in `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` - * in the `System.config.json`. + * - Not setting this property explicitly applies the default threshold defined in + * `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` in the `System.config.json`. * * @uxon-property monitor_as_long_running_after_seconds * @uxon-type int From 328a3716daea245900cd997c270b834cfd82cf06 Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Tue, 17 Mar 2026 14:35:50 +0100 Subject: [PATCH 2/7] DOC AbstractAction --- CommonLogic/AbstractAction.php | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/CommonLogic/AbstractAction.php b/CommonLogic/AbstractAction.php index 2982fcb23..14df21a92 100644 --- a/CommonLogic/AbstractAction.php +++ b/CommonLogic/AbstractAction.php @@ -561,10 +561,9 @@ public function setInputDataSheet(UxonObject $uxon) : ActionInterface * the result message and (if needed) a separate result object should be set within the specific implementation via * set_result_data_sheet(), set_result_message() and set_result() respectively! * - * This method is protected because only get_result...() methods are intended to be used by external objects. In - * addition to performing the action they also take care of saving it to the current context, etc., while perform() - * ist totally depending on the specific action implementation and holds only the actual logic without all the - * overhead. + * This method is protected because only get_result...() methods are intended to be used by external objects. In addition to performing + * the action they also take care of saving it to the current context, etc., while perform() ist totally depending on the specific + * action implementation and holds only the actual logic without all the overhead. * * @return void */ @@ -836,8 +835,8 @@ public function getDisabledBehaviors() /** * Returns the translation string for the given message id. * - * This is a shortcut for calling $this->getApp()->getTranslator()->translate(). Additionally it will automatically - * append an action prefix to the given id: e.g. $action->translate('SOME_MESSAGE') will result in + * This is a shortcut for calling $this->getApp()->getTranslator()->translate(). Additionally it will automatically append an + * action prefix to the given id: e.g. $action->translate('SOME_MESSAGE') will result in * $action->getApp()->getTranslator()->translate('ACTION.ALIAS.SOME_MESSAGE') * * @see Translation::translate() @@ -1148,8 +1147,7 @@ public function setInputMappers(UxonObject $uxon) * * @uxon-property input_mapper * @uxon-type \exface\Core\CommonLogic\DataSheets\DataSheetMapper - * @uxon-template {"from_object_alias": "", "to_object_alias": "", "column_to_column_mappings": [{"from": "", "to": - * ""}]} + * @uxon-template {"from_object_alias": "", "to_object_alias": "", "column_to_column_mappings": [{"from": "", "to":""}]} * * @see setInputMappers() * @see \exface\Core\Interfaces\Actions\ActionInterface::setInputMapper() @@ -1771,8 +1769,7 @@ protected function getEffectsFromTriggerWidget(iTriggerAction $button, MetaObjec } /** - * Objects and relations that may be affected by the action (in addition to those determined by the action logic - * automatically). + * Objects and relations that may be affected by the action (in addition to those determined by the action logic automatically). * * Most effects of an action can be determined automatically. If not, you can add them * here manually. For example: @@ -2189,13 +2186,13 @@ public function getMonitorAsLongRunningAfterSeconds(?int $default = null) : int| * an alert depending on the monitor configuration. * * Logging long-running actions must be explicitly enabled in `MONITOR.LONG_RUNNERS.ENABLED` System.config.json. - * If enabled, all actions taking longer than `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` will be logged to - * the monitor. + * If enabled, all actions taking longer than `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` will be logged to the + * monitor. * * - Any value >= 0 will override the config setting. * - You can disable this feature by setting the threshold to `-1` or `false`. - * - Not setting this property explicitly applies the default threshold defined in - * `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` in the `System.config.json`. + * - Not setting this property explicitly applies the default threshold defined in `MONITOR.LONG_RUNNERS.THRESHOLD_SECONDS_FOR_OTHERS` + * in the `System.config.json`. * * @uxon-property monitor_as_long_running_after_seconds * @uxon-type int From a42e73a3288c21a9ea0d8ceb4af4ecf84648e634 Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Tue, 17 Mar 2026 14:37:40 +0100 Subject: [PATCH 3/7] DOC AbstractAction --- CommonLogic/AbstractAction.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/CommonLogic/AbstractAction.php b/CommonLogic/AbstractAction.php index 14df21a92..446ee764f 100644 --- a/CommonLogic/AbstractAction.php +++ b/CommonLogic/AbstractAction.php @@ -561,7 +561,7 @@ public function setInputDataSheet(UxonObject $uxon) : ActionInterface * the result message and (if needed) a separate result object should be set within the specific implementation via * set_result_data_sheet(), set_result_message() and set_result() respectively! * - * This method is protected because only get_result...() methods are intended to be used by external objects. In addition to performing + * This method is protected because only get_result...() methods are intended to be used by external objects. In addition to performing * the action they also take care of saving it to the current context, etc., while perform() ist totally depending on the specific * action implementation and holds only the actual logic without all the overhead. * @@ -836,7 +836,7 @@ public function getDisabledBehaviors() * Returns the translation string for the given message id. * * This is a shortcut for calling $this->getApp()->getTranslator()->translate(). Additionally it will automatically append an - * action prefix to the given id: e.g. $action->translate('SOME_MESSAGE') will result in + * action prefix to the given id: e.g. $action->translate('SOME_MESSAGE') will result in * $action->getApp()->getTranslator()->translate('ACTION.ALIAS.SOME_MESSAGE') * * @see Translation::translate() @@ -1147,7 +1147,7 @@ public function setInputMappers(UxonObject $uxon) * * @uxon-property input_mapper * @uxon-type \exface\Core\CommonLogic\DataSheets\DataSheetMapper - * @uxon-template {"from_object_alias": "", "to_object_alias": "", "column_to_column_mappings": [{"from": "", "to":""}]} + * @uxon-template {"from_object_alias": "", "to_object_alias": "", "column_to_column_mappings": [{"from": "", "to": ""}]} * * @see setInputMappers() * @see \exface\Core\Interfaces\Actions\ActionInterface::setInputMapper() @@ -1981,8 +1981,7 @@ public function getInputChecks() : ActionDataCheckListInterface * * @uxon-property input_invalid_if * @uxon-type \exface\Core\CommonLogic\DataSheets\DataCheck[] - * @uxon-template [{"error_text": "", "operator": "AND", "conditions": [{"expression": "", "comparator": "", - * "value": ""}]}] + * @uxon-template [{"error_text": "", "operator": "AND", "conditions": [{"expression": "", "comparator": "", "value": ""}]}] * * @param UxonObject $arrayOfDataChecks * @return AbstractAction From 486b7dc9a91c5eda60d65a98229514d08dc906de Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Wed, 18 Mar 2026 09:55:38 +0100 Subject: [PATCH 4/7] DEV AbstractAction simplified task validation --- Actions/SaveData.php | 48 +++++++++++++ CommonLogic/AbstractAction.php | 72 +++++-------------- .../Actions/ActionTaskInvalidException.php | 22 ++++++ 3 files changed, 87 insertions(+), 55 deletions(-) create mode 100644 Exceptions/Actions/ActionTaskInvalidException.php diff --git a/Actions/SaveData.php b/Actions/SaveData.php index ae7ae67a7..2337633a8 100644 --- a/Actions/SaveData.php +++ b/Actions/SaveData.php @@ -1,6 +1,7 @@ setInputRowsMax(null); } + protected function validateApplicability(TaskInterface $task, DataTransactionInterface $transaction): void + { + parent::validateApplicability($task, $transaction); + + // See if the task has input data. + if(!$task->hasInputData()) { + return; + } + $taskInput = $task->getInputData(); + + // See if this action is coupled with a widget. + if($task->isTriggeredByWidget()) { + $widget = $task->getWidgetTriggeredBy(); + } elseif ($this->isDefinedInWidget()) { + $widget = $this->getWidgetDefinedIn(); + } else { + return; + } + + // If it is, we can get the data structure expectations from the widget. + $expectedData = $widget->prepareDataSheetToRead(); + $expectedColumns = $expectedData->getColumns(); + $unexpectedColumns = []; + + // Now check if there are input columns that are not present in our expected structure, which would imply + // a mistake or unauthorized attempts at accessing or manipulating data. Note that missing input columns + // are of no concern here, since they might be handled later by mappers or prototype specific logic. + foreach ($taskInput->getColumns() as $inputColumn) { + $inputColumnName = $inputColumn->getName(); + if(!$expectedColumns->has($inputColumnName)) { + $unexpectedColumns[] = '"' . $inputColumnName . '"'; + } + } + + if(!empty($unexpectedColumns)) { + $error = new ActionTaskInvalidException( + $this, + $task, + 'Unexpected task input columns detected for action "' . $this->getAliasWithNamespace() . + '": ' . implode(', ', $unexpectedColumns) . '!' + ); + $error->setUseExceptionMessageAsTitle(true); + throw $error; + } + } + + /** * * {@inheritDoc} diff --git a/CommonLogic/AbstractAction.php b/CommonLogic/AbstractAction.php index 446ee764f..f3fb3a65d 100644 --- a/CommonLogic/AbstractAction.php +++ b/CommonLogic/AbstractAction.php @@ -4,9 +4,9 @@ use exface\Core\CommonLogic\Actions\ActionConfirmationList; use exface\Core\CommonLogic\Traits\ICanBeConvertedToUxonTrait; use exface\Core\Exceptions\Actions\ActionConfigurationError; +use exface\Core\Exceptions\Actions\ActionTaskInvalidException; use exface\Core\Interfaces\Actions\ActionConfirmationListInterface; use exface\Core\Interfaces\DataSheets\DataSheetInterface; -use exface\Core\Interfaces\Exceptions\ExceptionInterface; use exface\Core\Interfaces\Log\LoggerInterface; use exface\Core\Interfaces\Model\IAffectMetaObjectsInterface; use exface\Core\Interfaces\Model\MetaObjectInterface; @@ -349,7 +349,7 @@ public final function handle(TaskInterface $task, DataTransactionInterface $tran } // TODO What's the correct response here? Throw, silent, message or something else. - $this->validateTask($task, $transaction); + $this->validateApplicability($task, $transaction); $this->getWorkbench()->eventManager()->dispatch(new OnBeforeActionPerformedEvent($this, $task, $transaction, function() use ($task) { return $this->getInputDataSheet($task); @@ -392,75 +392,37 @@ public final function handle(TaskInterface $task, DataTransactionInterface $tran } /** - * Validates a given task against the expectations of this action. Throws an error, when encountering issues and - * returns `void` if the task is valid for this action. + * Validates whether this action can be applied to a given task. Throws an error, when encountering issues + * and returns `void` if the task is valid for this action. * * Base validation ensures that: * - The task object and action object match, provided both are defined. - * - The task input data only contains columns present in the action widget, provided both are defined. + * - TODO The task input data only contains columns present in the action widget, provided both are defined. * - If any components for a given validation step are missing, the step succeeds. Validation only fails, if the * required data is present, but does not match expectations. * - * @throws ExceptionInterface + * @throws ActionTaskInvalidException * Throws an exception with a description of the violation. */ - protected function validateTask(TaskInterface $task, DataTransactionInterface $transaction) : void + protected function validateApplicability(TaskInterface $task, DataTransactionInterface $transaction) : void { $taskObject = $task->hasMetaObject() ? $task->getMetaObject() : null; - $taskInput = $task->hasInputData() ? $task->getInputData() : null; // Ensure metaobjects match. if($taskObject !== null && !$taskObject->isExactly($this->getMetaObject())) { - // See if any input mapper has a matching from object. - $hasMatchingInputMapper = false; - foreach ($this->getInputMappers() as $mapper) { - if($taskObject->isExactly($mapper->getFromMetaObject())) { - $hasMatchingInputMapper = true; - break; - } - } - // If we couldn't match with an input mapper either, the task is invalid for this action. - if(!$hasMatchingInputMapper) { - throw (new ActionRuntimeError( + // See if any input mapper has a matching from object. + // If we can't match with an input mapper either, the task is invalid for this action. + if(!$this->getInputMapper($taskObject) !== null) { + $error = new ActionTaskInvalidException( $this, + $task, 'Action "' . $this->getAliasWithNamespace() . '" is defined for "' . - $this->getMetaObject()->getAliasWithNamespace() . '", but received a task with object "' . - $task->getMetaObject()->getAliasWithNamespace() . '"!' - ))->setUseExceptionMessageAsTitle(true); - } - } - - // If the metaobjects match, we can check if the input data matches expectations. - $widget = null; - if($task->isTriggeredByWidget()) { - $widget = $task->getWidgetTriggeredBy(); - } elseif ($this->isDefinedInWidget()) { - $widget = $this->getWidgetDefinedIn(); - } - - // TODO How to properly extract widget data expectations? - $expectedData = $widget?->getPrefillData() ?? $widget?->prepareDataSheetToRead(); - if($taskInput !== null && $expectedData !== null) { - $expectedColumns = $expectedData->getColumns(); - $unexpectedColumns = []; - - // Now check if there are input columns that are not present in our expected structure, which would imply - // a mistake or unauthorized attempts at accessing or manipulating data. Note that missing input columns - // are of no concern here, since they might be handled later by mappers or prototype specific logic. - foreach ($taskInput->getColumns() as $inputColumn) { - $inputColumnName = $inputColumn->getName(); - if(!$expectedColumns->has($inputColumnName)) { - $unexpectedColumns[] = '"' . $inputColumnName . '"'; - } - } - - if(!empty($unexpectedColumns)) { - throw (new ActionRuntimeError( - $this, - 'Unexpected task input columns detected for action "' . $this->getAliasWithNamespace() . - '": ' . implode(', ', $unexpectedColumns) . '!' - ))->setUseExceptionMessageAsTitle(true); + $this->getMetaObject()->getAliasWithNamespace() . '", but received a task with object "' . + $task->getMetaObject()->getAliasWithNamespace() . '"!' + ); + $error->setUseExceptionMessageAsTitle(true); + throw $error; } } } diff --git a/Exceptions/Actions/ActionTaskInvalidException.php b/Exceptions/Actions/ActionTaskInvalidException.php new file mode 100644 index 000000000..07aca8afd --- /dev/null +++ b/Exceptions/Actions/ActionTaskInvalidException.php @@ -0,0 +1,22 @@ +task = $task; + parent::__construct($action, $message, $alias, $previous); + } + + public function getTask() : TaskInterface + { + return $this->task; + } +} \ No newline at end of file From 9b02cb971949a37829417d201c3c4b9960140595 Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Wed, 18 Mar 2026 16:39:24 +0100 Subject: [PATCH 5/7] DEV AbstractAction improved task validation --- Actions/ReadData.php | 12 ++ Actions/SaveData.php | 54 +----- CommonLogic/AbstractAction.php | 36 +--- CommonLogic/ActionInputValidator.php | 239 +++++++++++++++++++++++++++ 4 files changed, 267 insertions(+), 74 deletions(-) create mode 100644 CommonLogic/ActionInputValidator.php diff --git a/Actions/ReadData.php b/Actions/ReadData.php index 727f5fae6..f4993ff75 100644 --- a/Actions/ReadData.php +++ b/Actions/ReadData.php @@ -1,6 +1,7 @@ getExpectedColumns('prepareDataSheetToPrefill'); + $validator->validateTaskColumns($expectedColumns); + } + /** * * {@inheritDoc} diff --git a/Actions/SaveData.php b/Actions/SaveData.php index 2337633a8..37c1f72a2 100644 --- a/Actions/SaveData.php +++ b/Actions/SaveData.php @@ -1,7 +1,7 @@ setInputRowsMax(null); } - protected function validateApplicability(TaskInterface $task, DataTransactionInterface $transaction): void + /** + * @inheritDoc + */ + protected function validateApplicability(ActionInputValidator $validator): void { - parent::validateApplicability($task, $transaction); - - // See if the task has input data. - if(!$task->hasInputData()) { - return; - } - $taskInput = $task->getInputData(); - - // See if this action is coupled with a widget. - if($task->isTriggeredByWidget()) { - $widget = $task->getWidgetTriggeredBy(); - } elseif ($this->isDefinedInWidget()) { - $widget = $this->getWidgetDefinedIn(); - } else { - return; - } - - // If it is, we can get the data structure expectations from the widget. - $expectedData = $widget->prepareDataSheetToRead(); - $expectedColumns = $expectedData->getColumns(); - $unexpectedColumns = []; - - // Now check if there are input columns that are not present in our expected structure, which would imply - // a mistake or unauthorized attempts at accessing or manipulating data. Note that missing input columns - // are of no concern here, since they might be handled later by mappers or prototype specific logic. - foreach ($taskInput->getColumns() as $inputColumn) { - $inputColumnName = $inputColumn->getName(); - if(!$expectedColumns->has($inputColumnName)) { - $unexpectedColumns[] = '"' . $inputColumnName . '"'; - } - } - - if(!empty($unexpectedColumns)) { - $error = new ActionTaskInvalidException( - $this, - $task, - 'Unexpected task input columns detected for action "' . $this->getAliasWithNamespace() . - '": ' . implode(', ', $unexpectedColumns) . '!' - ); - $error->setUseExceptionMessageAsTitle(true); - throw $error; - } + parent::validateApplicability($validator); + + $expectedColumns = $validator->getExpectedColumns(); + $validator->validateTaskColumns($expectedColumns); } - /** * * {@inheritDoc} diff --git a/CommonLogic/AbstractAction.php b/CommonLogic/AbstractAction.php index f3fb3a65d..58b8406be 100644 --- a/CommonLogic/AbstractAction.php +++ b/CommonLogic/AbstractAction.php @@ -349,7 +349,7 @@ public final function handle(TaskInterface $task, DataTransactionInterface $tran } // TODO What's the correct response here? Throw, silent, message or something else. - $this->validateApplicability($task, $transaction); + $this->validateApplicability(new ActionInputValidator($this, $task)); $this->getWorkbench()->eventManager()->dispatch(new OnBeforeActionPerformedEvent($this, $task, $transaction, function() use ($task) { return $this->getInputDataSheet($task); @@ -395,36 +395,14 @@ public final function handle(TaskInterface $task, DataTransactionInterface $tran * Validates whether this action can be applied to a given task. Throws an error, when encountering issues * and returns `void` if the task is valid for this action. * - * Base validation ensures that: - * - The task object and action object match, provided both are defined. - * - TODO The task input data only contains columns present in the action widget, provided both are defined. - * - If any components for a given validation step are missing, the step succeeds. Validation only fails, if the - * required data is present, but does not match expectations. + * Base validation ensures that the task object and action object match, provided both are defined. * * @throws ActionTaskInvalidException - * Throws an exception with a description of the violation. - */ - protected function validateApplicability(TaskInterface $task, DataTransactionInterface $transaction) : void - { - $taskObject = $task->hasMetaObject() ? $task->getMetaObject() : null; - - // Ensure metaobjects match. - if($taskObject !== null && !$taskObject->isExactly($this->getMetaObject())) { - - // See if any input mapper has a matching from object. - // If we can't match with an input mapper either, the task is invalid for this action. - if(!$this->getInputMapper($taskObject) !== null) { - $error = new ActionTaskInvalidException( - $this, - $task, - 'Action "' . $this->getAliasWithNamespace() . '" is defined for "' . - $this->getMetaObject()->getAliasWithNamespace() . '", but received a task with object "' . - $task->getMetaObject()->getAliasWithNamespace() . '"!' - ); - $error->setUseExceptionMessageAsTitle(true); - throw $error; - } - } + * Throws an exception if validation FAILS, containing a description of the violation. + */ + protected function validateApplicability(ActionInputValidator $validator) : void + { + $validator->validateTaskObject(); } /** diff --git a/CommonLogic/ActionInputValidator.php b/CommonLogic/ActionInputValidator.php new file mode 100644 index 000000000..94879d1d1 --- /dev/null +++ b/CommonLogic/ActionInputValidator.php @@ -0,0 +1,239 @@ +action = $action; + $this->task = $task; + } + + /** + * Validates the task object and throws an error, should validation fail. + * + * NOTE: If either the task or the action do not have an object, validation succeeds. + * + * @return void + * @throws ActionTaskInvalidException + */ + public function validateTaskObject() : void + { + $action = $this->getAction(); + $task = $this->getTask(); + + $taskObject = $task->hasMetaObject() ? $task->getMetaObject() : null; + $actionObject = $action->hasMetaObject() ? $action->getMetaObject() : null; + + if($taskObject === null || $actionObject === null) { + return; + } + + // Ensure metaobjects match. + if(!$taskObject->isExactly($actionObject)) { + + // See if any input mapper has a matching from object. + // If we can't match with an input mapper either, the task is invalid for this action. + if(!$action->getInputMapper($taskObject) !== null) { + $error = new ActionTaskInvalidException( + $action, + $task, + 'Action "' . $action->getAliasWithNamespace() . '" is defined for "' . + $actionObject->getAliasWithNamespace() . '", but received a task with object "' . + $taskObject->getAliasWithNamespace() . '"!' + ); + $error->setUseExceptionMessageAsTitle(true); + throw $error; + } + } + } + + /** + * Returns an array containing the names of columns that the action expects in its input data. + * To explicitly allow certain columns, simply add them to the array. + * + * @param string $widgetPrepareDataSheetFunction + * You can specify what getter you wish to use, to retrieve a datasheet from the input widget. Make sure the + * function is actually supported by the input widget and returns an instance of `DataSheetInterface`. + * @param WidgetInterface|null $inputWidget + * Specify an input widget. If ´null´, the input widget will be determined automatically. + * @return array + */ + public function getExpectedColumns( + string $widgetPrepareDataSheetFunction = 'prepareDataSheetToRead', + WidgetInterface $inputWidget = null + ) : array + { + $expectedColumns = []; + + $inputWidget = $inputWidget ?? $this->getInputWidget(); + if($inputWidget === null) { + return $expectedColumns; + } + + $this->addColumnsFromWidget( + $expectedColumns, + $inputWidget, + $widgetPrepareDataSheetFunction + ); + + if($inputWidget instanceof iHaveConfigurator) { + $this->addColumnsFromWidget( + $expectedColumns, + $inputWidget->getConfiguratorWidget(), + $widgetPrepareDataSheetFunction + ); + } + + if($inputWidget instanceof iHaveColumns) { + foreach ($inputWidget->getColumns() as $column) { + $name = $column->getDataColumnName(); + $expectedColumns[$name] = $name; + } + } + + return $expectedColumns; + } + + /** + * Deduces the input widget for the action. + * + * @return WidgetInterface|null + */ + protected function getInputWidget() : WidgetInterface|null + { + $task = $this->getTask(); + $action = $this->getAction(); + + if($task->isTriggeredByWidget()) { + $widget = $task->getWidgetTriggeredBy(); + } elseif ($action->isDefinedInWidget()) { + $widget = $action->getWidgetDefinedIn(); + } else { + return null; + } + + if($widget instanceof iUseInputWidget) { + $widget = $widget->getInputWidget(); + } + + return $widget; + } + + /** + * Adds all columns from a given widget to + * + * @param array $target + * @param WidgetInterface $widget + * @param string $widgetPrepareDataSheetFunction + * @return void + */ + protected function addColumnsFromWidget( + array &$target, + WidgetInterface $widget, + string $widgetPrepareDataSheetFunction + ) : void + { + try { + foreach ($widget->$widgetPrepareDataSheetFunction()->getColumns() as $column) { + $name = $column->getName(); + $target[$name] = $name; + } + } catch (\Throwable $exception) { + + } + } + + /** + * Validates the task against a given list of columns. If any column in the task's input data is NOT among the + * `$expectedColumns` an error will be thrown. + * + * Validation succeeds if all input columns are expected or if the task doesn't have input data. + * + * @param array $expectedColumns + * @return void + */ + public function validateTaskColumns( + array $expectedColumns + ) : void + { + if(empty($expectedColumns)) { + return; + } + + $action = $this->getAction(); + $task = $this->getTask(); + + // See if the task has input data. + if(!$task->hasInputData()) { + return; + } + + $taskInput = $task->getInputData(); + $unexpectedColumns = []; + + // Now check if there are input columns that are not present in our expected structure, which would imply + // a mistake or unauthorized attempts at accessing or manipulating data. Note that missing input columns + // are of no concern here, since they might be handled later by mappers or prototype specific logic. + foreach ($taskInput->getColumns() as $inputColumn) { + $inputColumnName = $inputColumn->getName(); + if(!key_exists($inputColumnName, $expectedColumns)) { + $unexpectedColumns[] = '"' . $inputColumnName . '"'; + } + } + + if(!empty($unexpectedColumns)) { + $error = new ActionTaskInvalidException( + $action, + $task, + 'Unexpected task input columns detected for action "' . $action->getAliasWithNamespace() . + '": ' . implode(', ', $unexpectedColumns) . '!' + ); + $error->setUseExceptionMessageAsTitle(true); + throw $error; + } + } + + /** + * @return ActionInterface + */ + public function getAction() : ActionInterface + { + return $this->action; + } + + /** + * @return TaskInterface + */ + public function getTask() : TaskInterface + { + return $this->task; + } +} \ No newline at end of file From c86484b18d2b09ff1a9933fa81b297ff9b734795 Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Thu, 19 Mar 2026 13:22:34 +0100 Subject: [PATCH 6/7] DEV ReadData now ignores unexpected columns instead of throwing --- Actions/ReadData.php | 18 ++++++++++++++++-- CommonLogic/ActionInputValidator.php | 16 +++++++++++++--- .../Actions/ActionTaskInvalidException.php | 19 +++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/Actions/ReadData.php b/Actions/ReadData.php index f4993ff75..a002d3fa5 100644 --- a/Actions/ReadData.php +++ b/Actions/ReadData.php @@ -2,6 +2,7 @@ namespace exface\Core\Actions; use exface\Core\CommonLogic\ActionInputValidator; +use exface\Core\Exceptions\Actions\ActionTaskInvalidException; use exface\Core\Interfaces\Actions\iReadData; use exface\Core\CommonLogic\AbstractAction; use exface\Core\Interfaces\DataSheets\DataSheetInterface; @@ -51,8 +52,21 @@ protected function validateApplicability(ActionInputValidator $validator): void { parent::validateApplicability($validator); - $expectedColumns = $validator->getExpectedColumns('prepareDataSheetToPrefill'); - $validator->validateTaskColumns($expectedColumns); + $expectedColumns = $validator->getExpectedColumns(); + + try { + $validator->validateTaskColumns($expectedColumns); + } catch (ActionTaskInvalidException $exception) { + $task = $validator->getTask(); + if(!$task->hasInputData()) { + throw $exception; + } + + $inputData = $task->getInputData(); + foreach ($exception->getIssue(ActionTaskInvalidException::ISSUE_UNEXPECTED_COLUMN) as $badColumn) { + $inputData->getColumns()->removeByKey($badColumn); + } + } } /** diff --git a/CommonLogic/ActionInputValidator.php b/CommonLogic/ActionInputValidator.php index 94879d1d1..880900ecb 100644 --- a/CommonLogic/ActionInputValidator.php +++ b/CommonLogic/ActionInputValidator.php @@ -62,14 +62,19 @@ public function validateTaskObject() : void // See if any input mapper has a matching from object. // If we can't match with an input mapper either, the task is invalid for this action. if(!$action->getInputMapper($taskObject) !== null) { + $taskAlias = $taskObject->getAliasWithNamespace(); + $error = new ActionTaskInvalidException( $action, $task, 'Action "' . $action->getAliasWithNamespace() . '" is defined for "' . $actionObject->getAliasWithNamespace() . '", but received a task with object "' . - $taskObject->getAliasWithNamespace() . '"!' + $taskAlias . '"!' ); + $error->setUseExceptionMessageAsTitle(true); + $error->addIssue(ActionTaskInvalidException::ISSUE_INVALID_OBJECT, $taskAlias); + throw $error; } } @@ -108,7 +113,7 @@ public function getExpectedColumns( $this->addColumnsFromWidget( $expectedColumns, $inputWidget->getConfiguratorWidget(), - $widgetPrepareDataSheetFunction + 'prepareDataSheetToRead' ); } @@ -205,7 +210,7 @@ public function validateTaskColumns( foreach ($taskInput->getColumns() as $inputColumn) { $inputColumnName = $inputColumn->getName(); if(!key_exists($inputColumnName, $expectedColumns)) { - $unexpectedColumns[] = '"' . $inputColumnName . '"'; + $unexpectedColumns[$inputColumnName] = '"' . $inputColumnName . '"'; } } @@ -216,7 +221,12 @@ public function validateTaskColumns( 'Unexpected task input columns detected for action "' . $action->getAliasWithNamespace() . '": ' . implode(', ', $unexpectedColumns) . '!' ); + $error->setUseExceptionMessageAsTitle(true); + foreach (array_keys($unexpectedColumns) as $unexpectedColumn) { + $error->addIssue(ActionTaskInvalidException::ISSUE_UNEXPECTED_COLUMN, $unexpectedColumn); + } + throw $error; } } diff --git a/Exceptions/Actions/ActionTaskInvalidException.php b/Exceptions/Actions/ActionTaskInvalidException.php index 07aca8afd..6d41e07e8 100644 --- a/Exceptions/Actions/ActionTaskInvalidException.php +++ b/Exceptions/Actions/ActionTaskInvalidException.php @@ -7,7 +7,11 @@ class ActionTaskInvalidException extends ActionInputError { + public const ISSUE_INVALID_OBJECT = 'invalidObject'; + public const ISSUE_UNEXPECTED_COLUMN = 'unexpectedColumn'; + private TaskInterface $task; + private array $issues = []; public function __construct(ActionInterface $action, TaskInterface $task, $message, $alias = null, $previous = null) { @@ -19,4 +23,19 @@ public function getTask() : TaskInterface { return $this->task; } + + public function addIssue(string $issue, string $alias) : void + { + $this->issues[$issue][$alias] = $alias; + } + + public function getIssuesAll() : array + { + return $this->issues; + } + + public function getIssue(string $issue) : array + { + return $this->issues[$issue] ?? []; + } } \ No newline at end of file From 03fdbd2cbe36e4ac3bb67e312cb4df6529caa1bd Mon Sep 17 00:00:00 2001 From: hail-cookies <81558994+hail-cookies@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:06:57 +0100 Subject: [PATCH 7/7] DEV ReadData now ignores unexpected system columns --- Actions/ReadData.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Actions/ReadData.php b/Actions/ReadData.php index a002d3fa5..e6e71968b 100644 --- a/Actions/ReadData.php +++ b/Actions/ReadData.php @@ -54,17 +54,25 @@ protected function validateApplicability(ActionInputValidator $validator): void $expectedColumns = $validator->getExpectedColumns(); - try { + try { $validator->validateTaskColumns($expectedColumns); } catch (ActionTaskInvalidException $exception) { $task = $validator->getTask(); if(!$task->hasInputData()) { throw $exception; } - + + // We ignore unexpected columns IF they are system columns. $inputData = $task->getInputData(); foreach ($exception->getIssue(ActionTaskInvalidException::ISSUE_UNEXPECTED_COLUMN) as $badColumn) { - $inputData->getColumns()->removeByKey($badColumn); + $col = $inputData->getColumns()->get($badColumn); + if( + $col !== null && + $col->isAttribute() && + $col->getAttribute()->isSystem() + ) { + $inputData->getColumns()->removeByKey($badColumn); + } } } }