commit ce4b2983fe095034db5fb825f71c777222bd005b Author: fago Date: Fri Feb 11 14:30:05 2011 +0100 #1057484 patch by me and klausi towards making components in code more useful: * Allow the definition of variables that do not require a passed argument, as useful for returning variables that are not passed as parameter. * Improved the data_set action to be able to set variables too. * Fix handling of NULL values so Rules bails out if NULL values are passed as argument to an action. Including test coverage. * Introduced the optional 'allow null' key for parameters, so actions can go with NULL values. Made use of that for the data_set action. * Added tests for having components providing new variables. diff --git includes/rules.core.inc includes/rules.core.inc index ea7a33a..dd2076b 100644 --- includes/rules.core.inc +++ includes/rules.core.inc @@ -441,14 +441,13 @@ abstract class RulesPlugin extends RulesExtendable { /** * Sets up the execution state for the given arguments. */ - protected function setUpState(array $args) { + public function setUpState(array $args) { $state = new RulesState(); $vars = $this->setUpVariables(); // Fix numerically indexed args to start with 0. if (!isset($args[key($vars)])) { $args = array_values($args); } - foreach (array_keys($vars) as $i => $name) { $info = $vars[$name]; // Support numerically indexed arrays as well as named parameter style. @@ -458,7 +457,7 @@ abstract class RulesPlugin extends RulesExtendable { elseif (isset($args[$name])) { $state->addVariable($name, $args[$name], $info); } - elseif (!empty($info['handler'])) { + elseif (!empty($info['handler']) || (isset($info['parameter']) && $info['parameter'] === FALSE)) { $state->addVariable($name, NULL, $info); } elseif (empty($info['optional']) && $info['type'] != 'hidden') { @@ -651,16 +650,22 @@ abstract class RulesPlugin extends RulesExtendable { else { $arg = !empty($info['default value']) ? $info['default value'] : NULL; $skip_sanitize = TRUE; + $info['allow null'] = TRUE; + } + // Make sure the given value is set if required (default). + if (!isset($arg) && empty($info['allow null'])) { + throw new RulesException('The provided argument for parameter %name is empty.', array('%name' => $name), $this); } + // Apply any configured data processors. if (!empty($this->settings[$name . ':process'])) { // For processing, make sure the data is unwrapped now. $return = rules_unwrap_data(array($arg), array($info)); - return $this->settings[$name . ':process']->process($return[0], $info, $state, $this); + return isset($return[0]) ? $this->settings[$name . ':process']->process($return[0], $info, $state, $this) : NULL; } // Support passing already sanitized values. if ($info['type'] == 'text' && !isset($skip_sanitize) && !empty($info['sanitize']) && !($arg instanceof EntityMetadataWrapper)) { - return check_plain((string)$arg); + return check_plain((string) $arg); } return $arg; } @@ -1597,10 +1602,15 @@ abstract class RulesContainerPlugin extends RulesPlugin implements IteratorAggre } public function parameterInfo($optional = FALSE) { + $params = parent::parameterInfo($optional); if (isset($this->info['variables'])) { - return parent::parameterInfo($optional) + rules_filter_array($this->info['variables'], 'handler', FALSE); + foreach ($this->info['variables'] as $name => $var_info) { + if (empty($var_info['handler']) && (!isset($var_info['parameter']) || $var_info['parameter'])) { + $params[$name] = $var_info; + } + } } - return parent::parameterInfo($optional); + return $params; } public function availableVariables() { diff --git includes/rules.state.inc includes/rules.state.inc index 1d85bc6..f2e072e 100644 --- includes/rules.state.inc +++ includes/rules.state.inc @@ -61,7 +61,7 @@ class RulesState { 'type' => 'unknown', 'handler' => FALSE, ); - if (isset($data)) { + if (empty($this->info[$name]['handler'])) { $this->variables[$name] = rules_wrap_data($data, $this->info[$name]); } } @@ -141,9 +141,9 @@ class RulesState { * The variable or a EntityMetadataWrapper containing the variable. */ public function &get($name) { - if (!isset($this->variables[$name])) { + if (!array_key_exists($name, $this->variables)) { // If there is handler to load the variable, do it now. - if (!array_key_exists($name, $this->variables) && !empty($this->info[$name]['handler'])) { + if (!empty($this->info[$name]['handler'])) { $data = call_user_func($this->info[$name]['handler'], rules_unwrap_data($this->variables), $name, $this->info[$name]); $this->variables[$name] = rules_wrap_data($data, $this->info[$name]); $this->info[$name]['handler'] = FALSE; diff --git modules/data.eval.inc modules/data.eval.inc index a6f2035..b60cb91 100644 --- modules/data.eval.inc +++ modules/data.eval.inc @@ -13,19 +13,28 @@ * Action: Modify data. */ function rules_action_data_set($wrapper, $value, $settings, $state, $element) { - try { - // Update the value first then save changes, if possible. - $wrapper->set($value); - } - catch (EntityMetadataWrapperException $e) { - throw new RulesException('Unable to modify data "@selector": ' . $e->getMessage(), array('@selector' => $settings['data:select'])); + if ($wrapper instanceof EntityMetadataWrapper) { + try { + // Update the value first then save changes, if possible. + $wrapper->set($value); + } + catch (EntityMetadataWrapperException $e) { + throw new RulesException('Unable to modify data "@selector": ' . $e->getMessage(), array('@selector' => $settings['data:select'])); + } + // Save changes if a property of a variable has been changed. + if (strpos($element->settings['data:select'], ':') !== FALSE) { + $info = $wrapper->info(); + // We always have to save the changes in the parent entity. E.g. when the + // node author is changed, we don't want to save the author but the node. + $state->saveChanges(implode(':', explode(':', $settings['data:select'], -1)), $info['parent']); + } } - // Save changes if a property of a variable has been changed. - if (strpos($element->settings['data:select'], ':') !== FALSE) { - $info = $wrapper->info(); - // We always have to save the changes in the parent entity. E.g. when the - // node author is changed, we don't want to save the author but the node. - $state->saveChanges(implode(':', explode(':', $settings['data:select'], -1)), $info['parent']); + else { + // A not wrapped variable (e.g. a number) is being set. As we don't have + // a reference on the variable, we need to directly update the variable in + // the state. + $var_name = $element->settings['data:select']; + $state->variables[$var_name] = $value; } } diff --git modules/data.rules.inc modules/data.rules.inc index b750b8c..898ac36 100644 --- modules/data.rules.inc +++ modules/data.rules.inc @@ -30,6 +30,7 @@ function rules_data_action_info() { 'description' => t('Specifies the data to be modified using a data selector, e.g. "node:author:name".'), 'restriction' => 'selector', 'wrapped' => TRUE, + 'allow null' => TRUE, ), 'value' => array( 'type' => '*', @@ -313,7 +314,6 @@ function rules_action_data_set_validate(RulesAbstractPlugin $element) { $element->settings += array('data:select' => NULL); $info = $element->applyDataSelector($element->settings['data:select'])->info(); if (strpos($element->settings['data:select'], ':') !== FALSE && empty($info['setter callback'])) { - // Modifying rules variables itself doesn't make any sense. throw new RulesException("The selected data property doesn't support writing.", array(), array($element, 'parameter', 'data')); } } diff --git modules/rules_core.eval.inc modules/rules_core.eval.inc index 0835b10..e2826ce 100644 --- modules/rules_core.eval.inc +++ modules/rules_core.eval.inc @@ -30,16 +30,10 @@ function rules_element_invoke_component($arguments, RulesPlugin $element) { rules_log('Evaluating @plugin %label.', $replacements, RulesLog::INFO, TRUE); module_invoke_all('rules_config_execute', $component); - // Manually create a new evaluation state. - $new_state = new RulesState(); - - // Register all parameters as variables. - $args = array_intersect_key($wrapped_args, $info['parameter']); - foreach ($args as $name => $data) { - $new_state->addVariable($name, $data, $info['parameter'][$name]); - } - + // Manually create a new evaluation state and evaluate the component. + $new_state = $component->setUpState($wrapped_args); $return = $component->evaluate($new_state); + // Care for the right return value in case we have to provide vars. if ($component instanceof RulesActionInterface && !empty($info['provides'])) { $return = array(); diff --git rules.api.php rules.api.php index 3d96366..80c6bad 100644 --- rules.api.php +++ rules.api.php @@ -41,26 +41,26 @@ * - group: A group for this element, used for grouping the actions in the * interface. Should start with a capital letter and be translated. * Required. - * - parameter: An array describing all parameter of the action with - * the parameter's name as key. Optional. Each parameter has to be + * - parameter: (optional) An array describing all parameter of the action + * with the parameter's name as key. Each parameter has to be * described by a sub-array with possible attributes as described * afterwards, whereas the name of a parameter needs to be a lowercase, * valid PHP variable name. - * - provides: An array describing the variables the action provides to the - * evaluation state with the variable name as key. Optional. Each variable - * has to be described by a sub-array with possible attributes as described - * afterwards, whereas the name of a parameter needs to be a lowercase, - * valid PHP variable name. - * - 'named parameter': If set to TRUE, the arguments will be passed as a - * single array with the parameter names as keys. This emulates named - * parameters in PHP and is in particular useful if the number of parameters - * can vary. Optionally, defaults to FALSE. - * - base: The base for action implementation callbacks to use instead of the - * action's name. Optional (defaults to the name). - * - callbacks: An array which allows to set specific function callbacks for - * the action. The default for each callback is the actions base appended - * by '_' and the callback name. - * - 'access callback': An optional callback, which has to return whether the + * - provides: (optional) An array describing the variables the action + * provides to the evaluation state with the variable name as key. Each + * variable has to be described by a sub-array with possible attributes as + * described afterwards, whereas the name of a parameter needs to be a + * lowercase, valid PHP variable name. + * - 'named parameter': (optional) If set to TRUE, the arguments will be + * passed as a single array with the parameter names as keys. This emulates + * named parameters in PHP and is in particular useful if the number of + * parameters can vary. Defaults to FALSE. + * - base: (optional) The base for action implementation callbacks to use + * instead of the action's name. Defaults to the action name. + * - callbacks: (optional) An array which allows to set specific function + * callbacks for the action. The default for each callback is the actions + * base appended by '_' and the callback name. + * - 'access callback': (optional) A callback which has to return whether the * currently logged in user is allowed to configure this action. See * rules_node_integration_access() for an example callback. * Each 'parameter' array may contain the following properties: @@ -71,38 +71,42 @@ * can be specified by using the notating list as introduced by * the entity metadata module. The special keyword '*' can be used when all * types should be allowed. Required. - * - bundles: Optionally, an array of bundle names. When the specified type is + * - bundles: (optional) An array of bundle names. When the specified type is * set to a single entity type, this may be used to restrict the allowed * bundles. - * - description: If necessary, a further description of the parameter. - * Optional. - * - options list: Optionally, a callback that returns an array of possible + * - description: (optional) If necessary, a further description of the + * parameter. + * - options list: (optional) A callback that returns an array of possible * values for this parameter. The callback has to return an array as used * by hook_options_list(). For an example implementation see * rules_data_action_type_options(). - * - save: If this is set to TRUE, the parameter will be saved by rules when - * the rules evaluation ends. This is only supported for savable data - * types. If the action returns FALSE, saving is skipped. - * - optional: May be set to TRUE, when the parameter isn't required. - * - 'default value': The value to pass to the action, in case the parameter - * is optional and there is no specified value. Optional. - * - restriction: Restrict how the argument for this parameter may be - * provided. Supported values are 'selector' and 'input'. Optional. - * - sanitize: Optionally. Allows parameters of type 'text' to demand an + * - save: (optional) If this is set to TRUE, the parameter will be saved by + * rules when the rules evaluation ends. This is only supported for savable + * data types. If the action returns FALSE, saving is skipped. + * - optional: (optional) May be set to TRUE, when the parameter isn't + * required. + * - 'default value': (optional) The value to pass to the action, in case the + * parameter is optional and there is no specified value. + * - 'allow null': (optional) Usually Rules will not pass any NULL values as + * argument, but abort the evaluation if a NULL value is present. If set to + * TRUE, Rules will not abort and pass the NULL value through. Defaults to + * FALSE. + * - restriction: (optional) Restrict how the argument for this parameter may + * be provided. Supported values are 'selector' and 'input'. + * - sanitize: (optional) Allows parameters of type 'text' to demand an * already sanitized argument. If enabled, any user specified value won't be - * sanitized itself, but replacements applied by input evaluators are. - * - wrapped: Optionally, set this to TRUE in case the data should be passed + * sanitized itself, but replacements applied by input evaluators are as + * well as values retrieved from selected data sources. + * - wrapped: (optional) Set this to TRUE in case the data should be passed * wrapped. This only applies to wrapped data types, e.g. entities. * Each 'provides' array may contain the following properties: * - label: The label of the variable. Start capitalized. Required. * - type: The rules data type of the variable. All types declared in * hook_rules_data_info() may be specified. Types may be parametrized e.g. * the types node or list are valid. - * - save: If this is set to TRUE, the provided variable is saved by rules - * when the rules evaluation ends. Only possible for savable data types. - * Optional (defaults to FALSE). - * - 'label callback': A callback to improve the variables label using the - * action's configuration settings. Optional. + * - save: (optional) If this is set to TRUE, the provided variable is saved + * by rules when the rules evaluation ends. Only possible for savable data + * types. Defaults to FALSE. * * The module has to provide an implementation for each action, being a * function named as specified in the 'base' key or for the execution callback. diff --git rules.module rules.module index 7b9c622..8826bc4 100644 --- rules.module +++ rules.module @@ -72,8 +72,19 @@ function rules_condition($name, $settings = array()) { * @param $variables * The array of variables to setup in the evaluation state, making them * available for the configuraion elements. Values for the variables need to - * be passed as argument when the rule is executed. - * Only instances with no variables can be embedded in configurations. + * be passed as argument when the rule is executed. Only Rule instances with + * no variables can be embedded in other configurations, e.g. rule sets. + * The array has to be keyed by variable name and contain a sub-array for each + * variable that has the same structure as the arrays used for describing + * parameters of an action, see hook_rules_action_info(). However, in addition + * to that the following keys are supported: + * - parameter: (optional) If set to FALSE, no parameter for the variable + * is created - thus no argument needs to be passed to the rule for the + * variable upon execution. As a consequence no value will be set + * initially, but the "Set data value" action may be used to do so. This is + * in particular useful for defining variables which can be provided to the + * caller (see e.g. rules_action_set()) but need not be passed in as + * parameter. * @return Rule */ function rule($variables = NULL) { @@ -348,6 +359,9 @@ function rules_unwrap_data(array $data, $info = array()) { } $options = (isset($info[$key]) ? $info[$key] : array()) + array('decode' => empty($info[$key]['sanitize'])); $data[$key] = $entry->value($options); + if (!isset($data[$key]) && empty($info[$key]['allow null'])) { + throw new RulesException('The provided argument for parameter %name is empty.', array('%name' => $key)); + } } } return $data; diff --git tests/rules.test tests/rules.test index 3b51f0c..d614c01 100644 --- tests/rules.test +++ tests/rules.test @@ -524,6 +524,29 @@ class RulesTestCase extends DrupalWebTestCase { list($node2) = $set->execute($node); $this->assertTrue($node2->nid == $node->nid, 'Rule set returned a variable.'); + + // Create an action set returning a variable that is no parameter. + $set = rules_action_set(array( + 'node' => array( + 'type' => 'node', + 'parameter' => FALSE, + )), array('node')); + $set->action('entity_fetch', array('type' => 'node', 'id' => $node->nid)) + ->action('data_set', array('data:select' => 'node', 'value:select' => 'entity_fetched')); + $set->integrityCheck(); + list($node3) = $set->execute(); + $this->assertTrue($node3->nid == $node->nid, 'Action set returned a variable that has not been passed as parameter.'); + + // Test the same again with a variable holding a not wrapped data type. + $set = rules_action_set(array( + 'number' => array( + 'type' => 'integer', + 'parameter' => FALSE, + )), array('number')); + $set->action('data_set', array('data:select' => 'number', 'value' => 3)); + $set->integrityCheck(); + list($number) = $set->execute(); + $this->assertTrue($number == 3, 'Actions set returned a number.'); } /** @@ -622,6 +645,24 @@ class RulesTestCase extends DrupalWebTestCase { $this->assertTrue($node->title == 'foo', 'Action with named parameters has been correctly executed.'); RulesLog::logger()->checkLog(); } + + /** + * Make sure Rules aborts when NULL values are used. + */ + function testAbortOnNULLValues() { + $rule = rule(array('node' => array('type' => 'node'))); + $rule->action('drupal_message', array('message:select' => 'node:type')); + $rule->integrityCheck(); + + // Test the rule. + $node = $this->drupalCreateNode(); + $node->type = NULL; + $rule->execute($node); + + $text = RulesLog::logger()->render(); + $msg = RulesTestCase::t('The provided argument for parameter %message is empty.', array('message')); + $this->assertTrue(strpos($text, $msg) !== FALSE, 'Evaluation aborted due to an empty argument value.'); + } } function rules_test_condition_true($settings, $state, $element) { diff --git ui/ui.core.inc ui/ui.core.inc index 57c1cbd..ca4a6cf 100644 --- ui/ui.core.inc +++ ui/ui.core.inc @@ -417,8 +417,10 @@ class RulesPluginUI extends FacesExtender implements RulesPluginUIInterface { ); $weight = 0; foreach ($this->element->componentVariables() as $name => $var_info) { - $form['settings']['vars']['items'][$name] = RulesPluginUI::getVariableForm($name, $var_info); - $form['settings']['vars']['items'][$name]['weight']['#default_value'] = $weight++; + if (!(isset($var_info['parameter']) && $var_info['parameter'] == FALSE)) { + $form['settings']['vars']['items'][$name] = RulesPluginUI::getVariableForm($name, $var_info); + $form['settings']['vars']['items'][$name]['weight']['#default_value'] = $weight++; + } } // Always add three empty forms. for ($i = 0; $i < 3; $i++) {