From ddddfd2162732daa7419e9e0bade78142e3a60a7 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 5 Feb 2016 15:56:07 +0100 Subject: [PATCH 01/57] Added EventHandlerEntityBundle. --- rules.rules.events.yml | 1 + src/EventHandler/EventHandlerBase.php | 107 ++++++++++++++++++ src/EventHandler/EventHandlerEntityBundle.php | 27 +++++ .../GenericEventSubscriber.php | 2 +- 4 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/EventHandler/EventHandlerBase.php create mode 100644 src/EventHandler/EventHandlerEntityBundle.php diff --git a/rules.rules.events.yml b/rules.rules.events.yml index d8b8c973..ca5b3c51 100644 --- a/rules.rules.events.yml +++ b/rules.rules.events.yml @@ -21,6 +21,7 @@ kernel.request: rules_entity_presave: label: 'Before saving an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityPresaveDeriver' + class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' rules_entity_delete: label: 'After deleting an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityDeleteDeriver' diff --git a/src/EventHandler/EventHandlerBase.php b/src/EventHandler/EventHandlerBase.php new file mode 100644 index 00000000..8f611e51 --- /dev/null +++ b/src/EventHandler/EventHandlerBase.php @@ -0,0 +1,107 @@ +defaultConfiguration() as $key => $configuration) { + $this->configuration[$key] = $form_state->hasValue($key) ? $form_state->getValue($key) : $configuration; + } + } + + /** + * @inheritdoc + */ + public function getConfiguration() { + return $this->configuration; + } + + /** + * @inheritdoc + */ + public function setConfiguration(array $configuration) { + $this->configuration = $configuration + $this->defaultConfiguration(); + return $this; + } + + /** + * @inheritdoc + */ + public function defaultConfiguration() { + return array(); + } + + /** + * @inheritdoc + */ + public function getEventNameSuffix() { + return ''; + } + + /** + * @inheritdoc + */ + public function refineContextDefinitions() { + // Nothing to refine by default. + } + + /** + * @inheritdoc + */ + public function calculateDependencies() { + // Nothing to calculate by default. + } + +} diff --git a/src/EventHandler/EventHandlerEntityBundle.php b/src/EventHandler/EventHandlerEntityBundle.php new file mode 100644 index 00000000..7209fba9 --- /dev/null +++ b/src/EventHandler/EventHandlerEntityBundle.php @@ -0,0 +1,27 @@ +getSubject()->bundle(); + } + return $events_suffixes; + } + +} diff --git a/src/EventSubscriber/GenericEventSubscriber.php b/src/EventSubscriber/GenericEventSubscriber.php index 9f591b9c..ce3edd81 100644 --- a/src/EventSubscriber/GenericEventSubscriber.php +++ b/src/EventSubscriber/GenericEventSubscriber.php @@ -95,7 +95,7 @@ public function onRulesEvent(Event $event, $event_name) { if (is_subclass_of($handler_class, RulesConfigurableEventHandlerInterface::class)) { $qualified_event_suffixes = $handler_class::determineQualifiedEvents($event, $event_name, $event_definition); foreach ($qualified_event_suffixes as $qualified_event_suffix) { - $triggered_event[] = "$event_name--$qualified_event_suffix"; + $triggered_events[] = "$event_name--$qualified_event_suffix"; } } From 52317fdfdc6d9989078938d252e5440bb79e4a48 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 5 Feb 2016 16:23:12 +0100 Subject: [PATCH 02/57] Added EventHandlerEntityBundle to events yml. --- rules.rules.events.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rules.rules.events.yml b/rules.rules.events.yml index ca5b3c51..35672709 100644 --- a/rules.rules.events.yml +++ b/rules.rules.events.yml @@ -25,15 +25,19 @@ rules_entity_presave: rules_entity_delete: label: 'After deleting an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityDeleteDeriver' + class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' rules_entity_insert: label: 'After saving a new entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityInsertDeriver' + class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' rules_entity_update: label: 'After updating an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityUpdateDeriver' + class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' rules_entity_view: label: 'Viewing an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityViewDeriver' + class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' rules_system_cron: label: 'Cron maintenance tasks are performed' category: 'System' From b60b780a7ee741b8f92236767cd77586e4a2b723 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 5 Feb 2016 16:24:39 +0100 Subject: [PATCH 03/57] Started writing EventHandlerTest. WIP. --- .../Integration/Engine/EventHandlerTest.php | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/src/Integration/Engine/EventHandlerTest.php diff --git a/tests/src/Integration/Engine/EventHandlerTest.php b/tests/src/Integration/Engine/EventHandlerTest.php new file mode 100644 index 00000000..31619460 --- /dev/null +++ b/tests/src/Integration/Engine/EventHandlerTest.php @@ -0,0 +1,60 @@ +rulesExpressionManager->createRule(); + $rule1->addAction('rules_entity_presave:node', ContextConfig::create() + ->map('entity', 'entity') + ); + + $rule2 = $this->rulesExpressionManager->createRule(); + $rule2->addAction('rules_entity_presave:node--page', ContextConfig::create() + ->map('entity', 'entity') + ); + + // @todo: save node page and check if both rules are triggered. + } + + /** + * Tests EventHandlerEntityBundle execution. + */ + public function testEntityBundleHandlerExecution() { + // - Second we must cover execution time: Trigger that event and verify a + // reaction rule for the qualified event is correctly executed. + + #1. mock somehow entity: 'node' with bundle 'page' and field 'body' + #2. create somehow rule with action 'rules_entity_presave:node–page' + #3. node->save(). + #4. check somehow that the rule was triggered + } + +} From 753833d5aa56773d323087c9281d8c0f4d57c144 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 5 Feb 2016 18:02:46 +0100 Subject: [PATCH 04/57] EventHandlerTest WIP. --- .../Integration/Engine/EventHandlerTest.php | 60 -------- tests/src/Kernel/EventHandlerTest.php | 133 ++++++++++++++++++ 2 files changed, 133 insertions(+), 60 deletions(-) delete mode 100644 tests/src/Integration/Engine/EventHandlerTest.php create mode 100644 tests/src/Kernel/EventHandlerTest.php diff --git a/tests/src/Integration/Engine/EventHandlerTest.php b/tests/src/Integration/Engine/EventHandlerTest.php deleted file mode 100644 index 31619460..00000000 --- a/tests/src/Integration/Engine/EventHandlerTest.php +++ /dev/null @@ -1,60 +0,0 @@ -rulesExpressionManager->createRule(); - $rule1->addAction('rules_entity_presave:node', ContextConfig::create() - ->map('entity', 'entity') - ); - - $rule2 = $this->rulesExpressionManager->createRule(); - $rule2->addAction('rules_entity_presave:node--page', ContextConfig::create() - ->map('entity', 'entity') - ); - - // @todo: save node page and check if both rules are triggered. - } - - /** - * Tests EventHandlerEntityBundle execution. - */ - public function testEntityBundleHandlerExecution() { - // - Second we must cover execution time: Trigger that event and verify a - // reaction rule for the qualified event is correctly executed. - - #1. mock somehow entity: 'node' with bundle 'page' and field 'body' - #2. create somehow rule with action 'rules_entity_presave:node–page' - #3. node->save(). - #4. check somehow that the rule was triggered - } - -} diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/EventHandlerTest.php new file mode 100644 index 00000000..5add0e9b --- /dev/null +++ b/tests/src/Kernel/EventHandlerTest.php @@ -0,0 +1,133 @@ +installConfig(['system']); + $this->installConfig(['field']); + $this->installSchema('system', ['sequences']); + $this->installEntitySchema('node'); + + $this->storage = $this->container->get('entity_type.manager')->getStorage('rules_reaction_rule'); + } + + /** + * Tests EventHandlerEntityBundle configuration. + */ + public function testEntityBundleHandlerConfiguration() { + // Create a multi-value integer field for testing. + FieldStorageConfig::create([ + 'field_name' => 'field_integer', + 'type' => 'integer', + 'entity_type' => 'node', + 'cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, + ])->save(); + FieldConfig::create([ + 'field_name' => 'field_integer', + 'entity_type' => 'node', + 'bundle' => 'page', + ])->save(); + + // Create test node with a bundle and field. + $entity_type_manager = $this->container->get('entity_type.manager'); + $entity_type_manager->getStorage('node_type') + ->create(['type' => 'page']) + ->save(); + + // Create rule with an action 'rules_entity_presave:node–-page'. + $rule = $this->expressionManager->createRule(); + $config_entity = $this->storage->create([ + 'id' => 'test_rule', + 'expression_id' => 'rules_rule', + 'event' => 'rules_entity_presave:node-–page', + 'configuration' => $rule->getConfiguration(), + ]); + $config_entity->save(); + + // @todo Add integrity check that node.field_integer is detected by Rules. + } + + /** + * Tests EventHandlerEntityBundle execution. + */ + public function testEntityBundleHandlerExecution() { + // Create a multi-value integer field for testing. + FieldStorageConfig::create([ + 'field_name' => 'field_integer', + 'type' => 'integer', + 'entity_type' => 'node', + 'cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, + ])->save(); + FieldConfig::create([ + 'field_name' => 'field_integer', + 'entity_type' => 'node', + 'bundle' => 'page', + ])->save(); + + // Create test node with a bundle and field. + $entity_type_manager = $this->container->get('entity_type.manager'); + $entity_type_manager->getStorage('node_type') + ->create(['type' => 'page']) + ->save(); + $node = $entity_type_manager->getStorage('node') + ->create([ + 'title' => 'test', + 'type' => 'page', + // @todo set node.field_integer.0.value. + ]) + ->save(); + + // Create rule with an action 'rules_entity_presave:node–-page'. + $rule = $this->expressionManager->createRule(); + $rule->addAction('rules_test_log', + ContextConfig::create() + ->map('message', 'node.field_integer.0.value') + ); + $config_entity = $this->storage->create([ + 'id' => 'test_rule', + 'expression_id' => 'rules_rule', + 'event' => 'rules_entity_presave:node-–page', + 'configuration' => $rule->getConfiguration(), + ]); + $config_entity->save(); + + // @todo Dispatch presave. + $dispatcher = $this->container->get('event_dispatcher'); + // $dispatcher->dispatch('entity_presave', $node); + + // @todo Test that the action in the rule logged node value. + // $this->assertRulesLogEntryExists('test_user'); + } + +} From 46f1fbe2687db9873ccbe9f046d31d4ff29fcb46 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 5 Feb 2016 18:31:42 +0100 Subject: [PATCH 05/57] Improved EventHandlerTest WIP. --- tests/src/Kernel/EventHandlerTest.php | 81 ++++++++++++--------------- 1 file changed, 35 insertions(+), 46 deletions(-) diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/EventHandlerTest.php index 5add0e9b..bd694938 100644 --- a/tests/src/Kernel/EventHandlerTest.php +++ b/tests/src/Kernel/EventHandlerTest.php @@ -19,7 +19,10 @@ */ class EventHandlerTest extends RulesDrupalTestBase { - public static $modules = ['rules', 'rules_test', 'system', 'node', 'field']; + /** + * {@inheritdoc} + */ + public static $modules = ['rules', 'rules_test', 'system', 'node', 'field', 'user']; /** * The entity storage for Rules config entities. @@ -28,25 +31,31 @@ class EventHandlerTest extends RulesDrupalTestBase { */ protected $storage; + /** + * A node used for testing. + * + * @var \Drupal\node\NodeInterface + */ + protected $node; + /** * {@inheritdoc} */ public function setUp() { parent::setUp(); - $this->installConfig(['system']); - $this->installConfig(['field']); $this->installSchema('system', ['sequences']); + $this->installEntitySchema('user'); $this->installEntitySchema('node'); + $this->installConfig(['field']); $this->storage = $this->container->get('entity_type.manager')->getStorage('rules_reaction_rule'); - } - /** - * Tests EventHandlerEntityBundle configuration. - */ - public function testEntityBundleHandlerConfiguration() { - // Create a multi-value integer field for testing. + $entity_type_manager = $this->container->get('entity_type.manager'); + $entity_type_manager->getStorage('node_type') + ->create(['type' => 'page']) + ->save(); + FieldStorageConfig::create([ 'field_name' => 'field_integer', 'type' => 'integer', @@ -59,12 +68,17 @@ public function testEntityBundleHandlerConfiguration() { 'bundle' => 'page', ])->save(); - // Create test node with a bundle and field. - $entity_type_manager = $this->container->get('entity_type.manager'); - $entity_type_manager->getStorage('node_type') - ->create(['type' => 'page']) - ->save(); + $this->node = $entity_type_manager->getStorage('node') + ->create([ + 'title' => 'test', + 'type' => 'page', + ]); + } + /** + * Tests EventHandlerEntityBundle configuration. + */ + public function testEntityBundleHandlerConfiguration() { // Create rule with an action 'rules_entity_presave:node–-page'. $rule = $this->expressionManager->createRule(); $config_entity = $this->storage->create([ @@ -82,33 +96,9 @@ public function testEntityBundleHandlerConfiguration() { * Tests EventHandlerEntityBundle execution. */ public function testEntityBundleHandlerExecution() { - // Create a multi-value integer field for testing. - FieldStorageConfig::create([ - 'field_name' => 'field_integer', - 'type' => 'integer', - 'entity_type' => 'node', - 'cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, - ])->save(); - FieldConfig::create([ - 'field_name' => 'field_integer', - 'entity_type' => 'node', - 'bundle' => 'page', - ])->save(); + $this->node->field_integer->setValue(['0' => 1, '1' => 2]); - // Create test node with a bundle and field. - $entity_type_manager = $this->container->get('entity_type.manager'); - $entity_type_manager->getStorage('node_type') - ->create(['type' => 'page']) - ->save(); - $node = $entity_type_manager->getStorage('node') - ->create([ - 'title' => 'test', - 'type' => 'page', - // @todo set node.field_integer.0.value. - ]) - ->save(); - - // Create rule with an action 'rules_entity_presave:node–-page'. + // Create rule with an action 'rules_entity_presave:node--page'. $rule = $this->expressionManager->createRule(); $rule->addAction('rules_test_log', ContextConfig::create() @@ -117,17 +107,16 @@ public function testEntityBundleHandlerExecution() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node-–page', + 'event' => 'rules_entity_presave:node--page', 'configuration' => $rule->getConfiguration(), ]); $config_entity->save(); - // @todo Dispatch presave. - $dispatcher = $this->container->get('event_dispatcher'); - // $dispatcher->dispatch('entity_presave', $node); + // @todo Better dispatch presave event. + $this->node->save(); - // @todo Test that the action in the rule logged node value. - // $this->assertRulesLogEntryExists('test_user'); + // Test that the action in the rule logged node value. + $this->assertRulesLogEntryExists('1'); } } From 066e4ecbb5701d5776a702b78c2c7fac7f50dc5c Mon Sep 17 00:00:00 2001 From: milkovsky Date: Wed, 10 Feb 2016 15:50:45 +0100 Subject: [PATCH 06/57] PR fixes. --- rules.rules.events.yml | 10 +++++----- ...e.php => ConfigurableEventHandlerBase.php} | 12 +++++------ src/EventHandler/EventHandlerEntityBundle.php | 7 ++++--- tests/src/Kernel/EventHandlerTest.php | 20 ++++++++++--------- 4 files changed, 26 insertions(+), 23 deletions(-) rename src/EventHandler/{EventHandlerBase.php => ConfigurableEventHandlerBase.php} (84%) diff --git a/rules.rules.events.yml b/rules.rules.events.yml index 35672709..ee4081e6 100644 --- a/rules.rules.events.yml +++ b/rules.rules.events.yml @@ -21,23 +21,23 @@ kernel.request: rules_entity_presave: label: 'Before saving an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityPresaveDeriver' - class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' + class: '\Drupal\rules\EventHandler\ConfigurableEventHandlerEntityBundle' rules_entity_delete: label: 'After deleting an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityDeleteDeriver' - class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' + class: '\Drupal\rules\EventHandler\ConfigurableEventHandlerEntityBundle' rules_entity_insert: label: 'After saving a new entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityInsertDeriver' - class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' + class: '\Drupal\rules\EventHandler\ConfigurableEventHandlerEntityBundle' rules_entity_update: label: 'After updating an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityUpdateDeriver' - class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' + class: '\Drupal\rules\EventHandler\ConfigurableEventHandlerEntityBundle' rules_entity_view: label: 'Viewing an entity' deriver: '\Drupal\rules\Plugin\RulesEvent\EntityViewDeriver' - class: '\Drupal\rules\EventHandler\EventHandlerEntityBundle' + class: '\Drupal\rules\EventHandler\ConfigurableEventHandlerEntityBundle' rules_system_cron: label: 'Cron maintenance tasks are performed' category: 'System' diff --git a/src/EventHandler/EventHandlerBase.php b/src/EventHandler/ConfigurableEventHandlerBase.php similarity index 84% rename from src/EventHandler/EventHandlerBase.php rename to src/EventHandler/ConfigurableEventHandlerBase.php index 8f611e51..43abeae5 100644 --- a/src/EventHandler/EventHandlerBase.php +++ b/src/EventHandler/ConfigurableEventHandlerBase.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\rules\EventHandler\EventHandlerBase. + * Contains \Drupal\rules\EventHandler\ConfigurableEventHandlerBase. */ namespace Drupal\rules\EventHandler; @@ -15,7 +15,7 @@ /** * Base class for event handler. */ -abstract class EventHandlerBase extends RulesDefaultEventHandler implements RulesConfigurableEventHandlerInterface { +abstract class ConfigurableEventHandlerBase extends RulesDefaultEventHandler implements RulesConfigurableEventHandlerInterface { /** * The event configuration. @@ -28,21 +28,21 @@ abstract class EventHandlerBase extends RulesDefaultEventHandler implements Rule * @inheritdoc */ public static function determineQualifiedEvents(Event $event, $event_name, array &$event_definition) { - return array(); + // Nothing to do by default. } /** * @inheritdoc */ public function summary() { - return ''; + // Nothing to do by default. } /** * @inheritdoc */ public function buildConfigurationForm(array $form, FormStateInterface $form_state) { - return $form; + // Nothing to do by default. } /** @@ -87,7 +87,7 @@ public function defaultConfiguration() { * @inheritdoc */ public function getEventNameSuffix() { - return ''; + // Nothing to do by default. } /** diff --git a/src/EventHandler/EventHandlerEntityBundle.php b/src/EventHandler/EventHandlerEntityBundle.php index 7209fba9..61b132d1 100644 --- a/src/EventHandler/EventHandlerEntityBundle.php +++ b/src/EventHandler/EventHandlerEntityBundle.php @@ -2,16 +2,17 @@ /** * @file - * Contains \Drupal\rules\EventHandler\EventHandlerEntityBundle. + * Contains \Drupal\rules\EventHandler\ConfigurableEventHandlerEntityBundle. */ namespace Drupal\rules\EventHandler; + use Symfony\Component\EventDispatcher\Event; /** * Exposes the bundle of an entity as event setting. */ -class EventHandlerEntityBundle extends EventHandlerBase { +class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase { /** * @inheritdoc @@ -19,7 +20,7 @@ class EventHandlerEntityBundle extends EventHandlerBase { public static function determineQualifiedEvents(Event $event, $event_name, array &$event_definition) { $events_suffixes = []; if ($event instanceof \Drupal\rules\Event\EntityEvent) { - $events[] = $event->getSubject()->bundle(); + $events_suffixes[] = $event->getSubject()->bundle(); } return $events_suffixes; } diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/EventHandlerTest.php index bd694938..53e54625 100644 --- a/tests/src/Kernel/EventHandlerTest.php +++ b/tests/src/Kernel/EventHandlerTest.php @@ -10,7 +10,6 @@ use Drupal\Core\Field\FieldStorageDefinitionInterface; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; -use Drupal\rules\Context\ContextConfig; /** * Tests events with qualified name. @@ -96,14 +95,12 @@ public function testEntityBundleHandlerConfiguration() { * Tests EventHandlerEntityBundle execution. */ public function testEntityBundleHandlerExecution() { - $this->node->field_integer->setValue(['0' => 1, '1' => 2]); + // @todo Add node.field_integer.0.value to rules log message, read result. + //$this->node->field_integer->setValue(['0' => 1, '1' => 2]); - // Create rule with an action 'rules_entity_presave:node--page'. + // Create rule with the 'rules_entity_presave:node--page' event. $rule = $this->expressionManager->createRule(); - $rule->addAction('rules_test_log', - ContextConfig::create() - ->map('message', 'node.field_integer.0.value') - ); + $rule->addAction('rules_test_log'); $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', @@ -112,11 +109,16 @@ public function testEntityBundleHandlerExecution() { ]); $config_entity->save(); - // @todo Better dispatch presave event. + // Trigger node save. $this->node->save(); + // @todo Should we better dispatch rules_entity_presave:node event instead? + /*$entity_type_id = $this->node->getEntityTypeId(); + $event = new EntityEvent($this->node, [$entity_type_id => $this->node]); + $event_dispatcher = \Drupal::service('event_dispatcher'); + $event_dispatcher->dispatch("rules_entity_presave:$entity_type_id", $event);*/ // Test that the action in the rule logged node value. - $this->assertRulesLogEntryExists('1'); + $this->assertRulesLogEntryExists('action called'); } } From 0f1a9e5ff39abe2be3141360fdbd61aba1822ca6 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Wed, 10 Feb 2016 17:41:30 +0100 Subject: [PATCH 07/57] Dispatch events by base name of a configured event name. --- src/Core/RulesEventManager.php | 18 ++++++++++++++++++ src/Entity/ReactionRuleStorage.php | 2 ++ ...> ConfigurableEventHandlerEntityBundle.php} | 0 3 files changed, 20 insertions(+) rename src/EventHandler/{EventHandlerEntityBundle.php => ConfigurableEventHandlerEntityBundle.php} (100%) diff --git a/src/Core/RulesEventManager.php b/src/Core/RulesEventManager.php index 033f6d28..97ff125f 100644 --- a/src/Core/RulesEventManager.php +++ b/src/Core/RulesEventManager.php @@ -60,4 +60,22 @@ public function processDefinition(&$definition, $plugin_id) { } } + /** + * Returns the base name of a configured event name. + * + * For a configured event name like node_view--article the base event name + * node_view is returned. + * + * @return string + * The event base name. + */ + static public function rulesGetEventBaseName($event_name) { + // Cut off any suffix from a configured event name. + if (strpos($event_name, '--') !== FALSE) { + $parts = explode('--', $event_name, 2); + return $parts[0]; + } + return $event_name; + } + } diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index 1de2c0cf..e69af572 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -15,6 +15,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Component\Uuid\UuidInterface; use Drupal\Core\Language\LanguageManagerInterface; +use Drupal\rules\Core\RulesEventManager; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -84,6 +85,7 @@ protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { $event = $rules_config->getEvent(); + $event = RulesEventManager::rulesGetEventBaseName($event); if ($event && !isset($events[$event])) { $events[$event] = $event; } diff --git a/src/EventHandler/EventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php similarity index 100% rename from src/EventHandler/EventHandlerEntityBundle.php rename to src/EventHandler/ConfigurableEventHandlerEntityBundle.php From ecce5578629d472e36fd60431c95876889ff154b Mon Sep 17 00:00:00 2001 From: milkovsky Date: Wed, 10 Feb 2016 17:56:52 +0100 Subject: [PATCH 08/57] Implemented testEntityBundleHandlerExecution. --- tests/src/Kernel/EventHandlerTest.php | 54 +++++++++++++++++++-------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/EventHandlerTest.php index 53e54625..ab3bb19f 100644 --- a/tests/src/Kernel/EventHandlerTest.php +++ b/tests/src/Kernel/EventHandlerTest.php @@ -10,6 +10,8 @@ use Drupal\Core\Field\FieldStorageDefinitionInterface; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\rules\Context\ContextConfig; +use Drupal\rules\Event\EntityEvent; /** * Tests events with qualified name. @@ -95,30 +97,50 @@ public function testEntityBundleHandlerConfiguration() { * Tests EventHandlerEntityBundle execution. */ public function testEntityBundleHandlerExecution() { - // @todo Add node.field_integer.0.value to rules log message, read result. - //$this->node->field_integer->setValue(['0' => 1, '1' => 2]); - - // Create rule with the 'rules_entity_presave:node--page' event. - $rule = $this->expressionManager->createRule(); - $rule->addAction('rules_test_log'); - $config_entity = $this->storage->create([ - 'id' => 'test_rule', + // Create rule1 with the 'rules_entity_presave:node--page' event. + $rule1 = $this->expressionManager->createRule(); + $rule1->addAction('rules_test_log', + ContextConfig::create() + ->map('message', 'node.field_integer.0.value') + ); + $config_entity1 = $this->storage->create([ + 'id' => 'test_rule1', 'expression_id' => 'rules_rule', 'event' => 'rules_entity_presave:node--page', - 'configuration' => $rule->getConfiguration(), + 'configuration' => $rule1->getConfiguration(), ]); - $config_entity->save(); + $config_entity1->save(); + + // Create rule2 with the 'rules_entity_presave:node' event. + $rule2 = $this->expressionManager->createRule(); + $rule2->addAction('rules_test_log', + ContextConfig::create() + ->map('message', 'node.field_integer.1.value') + ); + $config_entity2 = $this->storage->create([ + 'id' => 'test_rule2', + 'expression_id' => 'rules_rule', + 'event' => 'rules_entity_presave:node', + 'configuration' => $rule2->getConfiguration(), + ]); + $config_entity2->save(); + + // The logger instance has changed, refresh it. + $this->logger = $this->container->get('logger.channel.rules'); + + // Add node.field_integer.0.value to rules log message, read result. + $this->node->field_integer->setValue(['0' => 11, '1' => 22]); // Trigger node save. - $this->node->save(); - // @todo Should we better dispatch rules_entity_presave:node event instead? - /*$entity_type_id = $this->node->getEntityTypeId(); + $entity_type_id = $this->node->getEntityTypeId(); $event = new EntityEvent($this->node, [$entity_type_id => $this->node]); $event_dispatcher = \Drupal::service('event_dispatcher'); - $event_dispatcher->dispatch("rules_entity_presave:$entity_type_id", $event);*/ + $event_dispatcher->dispatch("rules_entity_presave:$entity_type_id", $event); - // Test that the action in the rule logged node value. - $this->assertRulesLogEntryExists('action called'); + // Test that the action in the rule1 logged node value. + $this->assertRulesLogEntryExists(11, 1); + // Test that the action in the rule2 logged node value. + $this->assertRulesLogEntryExists(22, 0); } } From 54c3cd4abdc25fb62f55d969b3234f9393d7916f Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 11 Feb 2016 14:03:34 +0100 Subject: [PATCH 09/57] PR fixes. --- src/Core/RulesEventManager.php | 10 ++++--- src/Entity/ReactionRuleStorage.php | 22 +++++++++++++- .../ConfigurableEventHandlerBase.php | 22 +++++++------- .../ConfigurableEventHandlerEntityBundle.php | 2 +- tests/src/Kernel/EventHandlerTest.php | 29 ++++--------------- 5 files changed, 45 insertions(+), 40 deletions(-) diff --git a/src/Core/RulesEventManager.php b/src/Core/RulesEventManager.php index 97ff125f..df8b702f 100644 --- a/src/Core/RulesEventManager.php +++ b/src/Core/RulesEventManager.php @@ -61,15 +61,17 @@ public function processDefinition(&$definition, $plugin_id) { } /** - * Returns the base name of a configured event name. + * Gets the base name of a configured event name. * - * For a configured event name like node_view--article the base event name - * node_view is returned. + * For a configured event name like {EVENT_NAME}--{SUFFIX}, the base event + * name {EVENT_NAME} is returned. * * @return string * The event base name. + * + * @see RulesConfigurableEventHandlerInterface::getEventNameSuffix() */ - static public function rulesGetEventBaseName($event_name) { + public function getEventBaseName($event_name) { // Cut off any suffix from a configured event name. if (strpos($event_name, '--') !== FALSE) { $parts = explode('--', $event_name, 2); diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index e69af572..29f02b42 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -40,6 +40,13 @@ class ReactionRuleStorage extends ConfigEntityStorage { */ protected $drupalKernel; + /** + * The event manager. + * + * @var RulesEventManager + */ + protected $eventManager; + /** * Constructs a ReactionRuleStorage object. * @@ -61,6 +68,19 @@ public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInter $this->drupalKernel = $drupal_kernel; } + /** + * Gets the event manager. + * + * @return RulesEventManager + * The event manager. + */ + protected function eventManager() { + if (!$this->eventManager) { + $this->eventManager = new RulesEventManager($this->moduleHandler()); + } + return $this->eventManager; + } + /** * {@inheritdoc} */ @@ -85,7 +105,7 @@ protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { $event = $rules_config->getEvent(); - $event = RulesEventManager::rulesGetEventBaseName($event); + $event = $this->eventManager()->getEventBaseName($event); if ($event && !isset($events[$event])) { $events[$event] = $event; } diff --git a/src/EventHandler/ConfigurableEventHandlerBase.php b/src/EventHandler/ConfigurableEventHandlerBase.php index 43abeae5..e4590583 100644 --- a/src/EventHandler/ConfigurableEventHandlerBase.php +++ b/src/EventHandler/ConfigurableEventHandlerBase.php @@ -25,35 +25,35 @@ abstract class ConfigurableEventHandlerBase extends RulesDefaultEventHandler imp protected $configuration = array(); /** - * @inheritdoc + * {@inheritdoc} */ public static function determineQualifiedEvents(Event $event, $event_name, array &$event_definition) { // Nothing to do by default. } /** - * @inheritdoc + * {@inheritdoc} */ public function summary() { // Nothing to do by default. } /** - * @inheritdoc + * {@inheritdoc} */ public function buildConfigurationForm(array $form, FormStateInterface $form_state) { // Nothing to do by default. } /** - * @inheritdoc + * {@inheritdoc} */ public function validate() { // Nothing to check by default. } /** - * @inheritdoc + * {@inheritdoc} */ public function extractConfigurationFormValues(array &$form, FormStateInterface $form_state) { foreach ($this->defaultConfiguration() as $key => $configuration) { @@ -62,14 +62,14 @@ public function extractConfigurationFormValues(array &$form, FormStateInterface } /** - * @inheritdoc + * {@inheritdoc} */ public function getConfiguration() { return $this->configuration; } /** - * @inheritdoc + * {@inheritdoc} */ public function setConfiguration(array $configuration) { $this->configuration = $configuration + $this->defaultConfiguration(); @@ -77,28 +77,28 @@ public function setConfiguration(array $configuration) { } /** - * @inheritdoc + * {@inheritdoc} */ public function defaultConfiguration() { return array(); } /** - * @inheritdoc + * {@inheritdoc} */ public function getEventNameSuffix() { // Nothing to do by default. } /** - * @inheritdoc + * {@inheritdoc} */ public function refineContextDefinitions() { // Nothing to refine by default. } /** - * @inheritdoc + * {@inheritdoc} */ public function calculateDependencies() { // Nothing to calculate by default. diff --git a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php index 61b132d1..65e11099 100644 --- a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php +++ b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php @@ -15,7 +15,7 @@ class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase { /** - * @inheritdoc + * {@inheritdoc} */ public static function determineQualifiedEvents(Event $event, $event_name, array &$event_definition) { $events_suffixes = []; diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/EventHandlerTest.php index ab3bb19f..47c9d7b1 100644 --- a/tests/src/Kernel/EventHandlerTest.php +++ b/tests/src/Kernel/EventHandlerTest.php @@ -77,26 +77,11 @@ public function setUp() { } /** - * Tests EventHandlerEntityBundle configuration. - */ - public function testEntityBundleHandlerConfiguration() { - // Create rule with an action 'rules_entity_presave:node–-page'. - $rule = $this->expressionManager->createRule(); - $config_entity = $this->storage->create([ - 'id' => 'test_rule', - 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node-–page', - 'configuration' => $rule->getConfiguration(), - ]); - $config_entity->save(); - - // @todo Add integrity check that node.field_integer is detected by Rules. - } - - /** - * Tests EventHandlerEntityBundle execution. + * Tests ConfigurableEventHandlerEntityBundle. + * + * @todo Add integrity check that node.field_integer is detected by Rules. */ - public function testEntityBundleHandlerExecution() { + public function testEntityBundleHandler() { // Create rule1 with the 'rules_entity_presave:node--page' event. $rule1 = $this->expressionManager->createRule(); $rule1->addAction('rules_test_log', @@ -107,8 +92,7 @@ public function testEntityBundleHandlerExecution() { 'id' => 'test_rule1', 'expression_id' => 'rules_rule', 'event' => 'rules_entity_presave:node--page', - 'configuration' => $rule1->getConfiguration(), - ]); + ])->setExpression($rule1); $config_entity1->save(); // Create rule2 with the 'rules_entity_presave:node' event. @@ -121,8 +105,7 @@ public function testEntityBundleHandlerExecution() { 'id' => 'test_rule2', 'expression_id' => 'rules_rule', 'event' => 'rules_entity_presave:node', - 'configuration' => $rule2->getConfiguration(), - ]); + ])->setExpression($rule2); $config_entity2->save(); // The logger instance has changed, refresh it. From 03ecf3a429a0698797c1eb32c941f48f8e2e2663 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 11 Feb 2016 14:07:39 +0100 Subject: [PATCH 10/57] PR fixes. --- tests/src/Kernel/EventHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/EventHandlerTest.php index 47c9d7b1..98aacf05 100644 --- a/tests/src/Kernel/EventHandlerTest.php +++ b/tests/src/Kernel/EventHandlerTest.php @@ -23,7 +23,7 @@ class EventHandlerTest extends RulesDrupalTestBase { /** * {@inheritdoc} */ - public static $modules = ['rules', 'rules_test', 'system', 'node', 'field', 'user']; + public static $modules = ['rules', 'system', 'node', 'field', 'user']; /** * The entity storage for Rules config entities. From 7fbae659fba6f628d410c11dcab1058323accdf1 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 11 Feb 2016 14:08:09 +0100 Subject: [PATCH 11/57] PR fixes. --- src/EventHandler/ConfigurableEventHandlerBase.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EventHandler/ConfigurableEventHandlerBase.php b/src/EventHandler/ConfigurableEventHandlerBase.php index e4590583..2cfd94bf 100644 --- a/src/EventHandler/ConfigurableEventHandlerBase.php +++ b/src/EventHandler/ConfigurableEventHandlerBase.php @@ -22,7 +22,7 @@ abstract class ConfigurableEventHandlerBase extends RulesDefaultEventHandler imp * * @var array */ - protected $configuration = array(); + protected $configuration = []; /** * {@inheritdoc} @@ -80,7 +80,7 @@ public function setConfiguration(array $configuration) { * {@inheritdoc} */ public function defaultConfiguration() { - return array(); + return []; } /** From 8e4af1be0772b70385bbfca72551a0065d542104 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 11 Feb 2016 16:06:33 +0100 Subject: [PATCH 12/57] PR fixes. --- src/Core/RulesEventManager.php | 2 +- src/Entity/ReactionRuleStorage.php | 4 +- .../ConfigurableEventHandlerBase.php | 50 ------------------- .../ConfigurableEventHandlerEntityBundle.php | 43 ++++++++++++++++ ...t.php => ConfigurableEventHandlerTest.php} | 9 ++-- 5 files changed, 52 insertions(+), 56 deletions(-) rename tests/src/Kernel/{EventHandlerTest.php => ConfigurableEventHandlerTest.php} (91%) diff --git a/src/Core/RulesEventManager.php b/src/Core/RulesEventManager.php index df8b702f..160f7325 100644 --- a/src/Core/RulesEventManager.php +++ b/src/Core/RulesEventManager.php @@ -69,7 +69,7 @@ public function processDefinition(&$definition, $plugin_id) { * @return string * The event base name. * - * @see RulesConfigurableEventHandlerInterface::getEventNameSuffix() + * @see \Drupal\rules\Core\RulesConfigurableEventHandlerInterface::getEventNameSuffix() */ public function getEventBaseName($event_name) { // Cut off any suffix from a configured event name. diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index 29f02b42..eec4da22 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -43,7 +43,7 @@ class ReactionRuleStorage extends ConfigEntityStorage { /** * The event manager. * - * @var RulesEventManager + * @var \Drupal\rules\Core\RulesEventManager */ protected $eventManager; @@ -71,7 +71,7 @@ public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInter /** * Gets the event manager. * - * @return RulesEventManager + * @return \Drupal\rules\Core\RulesEventManager * The event manager. */ protected function eventManager() { diff --git a/src/EventHandler/ConfigurableEventHandlerBase.php b/src/EventHandler/ConfigurableEventHandlerBase.php index 2cfd94bf..9f88c8d8 100644 --- a/src/EventHandler/ConfigurableEventHandlerBase.php +++ b/src/EventHandler/ConfigurableEventHandlerBase.php @@ -10,7 +10,6 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\rules\Core\RulesConfigurableEventHandlerInterface; use Drupal\rules\Core\RulesDefaultEventHandler; -use Symfony\Component\EventDispatcher\Event; /** * Base class for event handler. @@ -24,34 +23,6 @@ abstract class ConfigurableEventHandlerBase extends RulesDefaultEventHandler imp */ protected $configuration = []; - /** - * {@inheritdoc} - */ - public static function determineQualifiedEvents(Event $event, $event_name, array &$event_definition) { - // Nothing to do by default. - } - - /** - * {@inheritdoc} - */ - public function summary() { - // Nothing to do by default. - } - - /** - * {@inheritdoc} - */ - public function buildConfigurationForm(array $form, FormStateInterface $form_state) { - // Nothing to do by default. - } - - /** - * {@inheritdoc} - */ - public function validate() { - // Nothing to check by default. - } - /** * {@inheritdoc} */ @@ -83,25 +54,4 @@ public function defaultConfiguration() { return []; } - /** - * {@inheritdoc} - */ - public function getEventNameSuffix() { - // Nothing to do by default. - } - - /** - * {@inheritdoc} - */ - public function refineContextDefinitions() { - // Nothing to refine by default. - } - - /** - * {@inheritdoc} - */ - public function calculateDependencies() { - // Nothing to calculate by default. - } - } diff --git a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php index 65e11099..86baa041 100644 --- a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php +++ b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php @@ -7,6 +7,7 @@ namespace Drupal\rules\EventHandler; +use Drupal\Core\Form\FormStateInterface; use Symfony\Component\EventDispatcher\Event; /** @@ -25,4 +26,46 @@ public static function determineQualifiedEvents(Event $event, $event_name, array return $events_suffixes; } + /** + * {@inheritdoc} + */ + public function summary() { + // Nothing to do by default. + } + + /** + * {@inheritdoc} + */ + public function buildConfigurationForm(array $form, FormStateInterface $form_state) { + // Nothing to do by default. + } + + /** + * {@inheritdoc} + */ + public function validate() { + // Nothing to check by default. + } + + /** + * {@inheritdoc} + */ + public function getEventNameSuffix() { + // Nothing to do by default. + } + + /** + * {@inheritdoc} + */ + public function refineContextDefinitions() { + // Nothing to refine by default. + } + + /** + * {@inheritdoc} + */ + public function calculateDependencies() { + // Nothing to calculate by default. + } + } diff --git a/tests/src/Kernel/EventHandlerTest.php b/tests/src/Kernel/ConfigurableEventHandlerTest.php similarity index 91% rename from tests/src/Kernel/EventHandlerTest.php rename to tests/src/Kernel/ConfigurableEventHandlerTest.php index 98aacf05..28c2bfa3 100644 --- a/tests/src/Kernel/EventHandlerTest.php +++ b/tests/src/Kernel/ConfigurableEventHandlerTest.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\Tests\rules\Integration\Engine\EventHandlerTest. + * Contains \Drupal\Tests\rules\Integration\Engine\ConfigurableEventHandlerTest. */ namespace Drupal\Tests\rules\Kernel; @@ -18,7 +18,7 @@ * * @group rules */ -class EventHandlerTest extends RulesDrupalTestBase { +class ConfigurableEventHandlerTest extends RulesDrupalTestBase { /** * {@inheritdoc} @@ -79,9 +79,12 @@ public function setUp() { /** * Tests ConfigurableEventHandlerEntityBundle. * + * Test that rules are triggered correctly based upon the fully qualified + * event name as well as the base event name. + * * @todo Add integrity check that node.field_integer is detected by Rules. */ - public function testEntityBundleHandler() { + public function testConfigurableEventHandler() { // Create rule1 with the 'rules_entity_presave:node--page' event. $rule1 = $this->expressionManager->createRule(); $rule1->addAction('rules_test_log', From b57e33d5748d80d1a85624e834595203ba227f26 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 11 Feb 2016 18:11:04 +0100 Subject: [PATCH 13/57] Change rules config to have multiple events. --- config/schema/rules.schema.yml | 36 +++++++++++++++---- src/Engine/RulesComponent.php | 11 +++--- src/Entity/ReactionRuleConfig.php | 18 ++++++---- src/Entity/ReactionRuleStorage.php | 18 ++++++---- .../GenericEventSubscriber.php | 2 +- src/Form/ReactionRuleAddForm.php | 3 +- src/Form/ReactionRuleEditForm.php | 17 +++++---- tests/src/Kernel/EventIntegrationTest.php | 2 +- 8 files changed, 73 insertions(+), 34 deletions(-) diff --git a/config/schema/rules.schema.yml b/config/schema/rules.schema.yml index 4e15b874..d225fb4a 100644 --- a/config/schema/rules.schema.yml +++ b/config/schema/rules.schema.yml @@ -49,9 +49,21 @@ rules.reaction.*: label: type: label label: 'Label' - event: - type: string - label: 'Event' + events: + type: sequence + label: 'Events' + sequence: + type: mapping + label: 'Event' + mapping: + event_name: + type: string + label: 'Name' + configuration: + type: sequence + label: 'Configuration' + sequence: + type: mapping module: type: string label: 'Module' @@ -194,9 +206,21 @@ rules_expression.rules_reaction_rule: uuid: type: string label: 'UUID' - event: - type: string - label: 'Event' + events: + type: sequence + label: 'Events' + sequence: + type: mapping + label: 'Event' + mapping: + event_name: + type: string + label: 'Name' + configuration: + type: sequence + label: 'Configuration' + sequence: + type: mapping conditions: type: rules_expression.[id] label: 'Conditions' diff --git a/src/Engine/RulesComponent.php b/src/Engine/RulesComponent.php index 2788ab59..e65377a4 100644 --- a/src/Engine/RulesComponent.php +++ b/src/Engine/RulesComponent.php @@ -125,11 +125,12 @@ public function getContextDefinitions() { */ public function addContextDefinitionsFrom(ConfigEntityInterface $rules_config) { if ($rules_config instanceof ReactionRuleConfig) { - $event_name = $rules_config->getEvent(); - // @todo Use setter injection for the service. - $event_definition = \Drupal::service('plugin.manager.rules_event')->getDefinition($event_name); - foreach ($event_definition['context'] as $context_name => $context_definition) { - $this->addContextDefinition($context_name, $context_definition); + foreach ($rules_config->getEvents() as $event) { + // @todo Use setter injection for the service. + $event_definition = \Drupal::service('plugin.manager.rules_event')->getDefinition($event['event_name']); + foreach ($event_definition['context'] as $context_name => $context_definition) { + $this->addContextDefinition($context_name, $context_definition); + } } } return $this; diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index d86c8a0a..c3d34423 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -36,7 +36,7 @@ * config_export = { * "id", * "label", - * "event", + * "events", * "module", * "description", * "tag", @@ -121,11 +121,15 @@ class ReactionRuleConfig extends ConfigEntityBase { protected $module = 'rules'; /** - * The event name this reaction rule is reacting on. + * The events this reaction rule is reacting on. * - * @var string + * Events array, key - numeric index, value - event array with next structure: + * - event_name: string with the event machine name. + * - configuration: an array containing the event configuration. + * + * @var array */ - protected $event; + protected $events; /** * Sets a Rules expression instance for this Reaction rule. @@ -208,10 +212,10 @@ public function getTag() { } /** - * Returns the event on which this rule will trigger. + * Returns the array of events on which this rule will trigger. */ - public function getEvent() { - return $this->event; + public function getEvents() { + return $this->events; } /** diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index 1de2c0cf..7a4f88b9 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -83,9 +83,10 @@ public static function createInstance(ContainerInterface $container, EntityTypeI protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { - $event = $rules_config->getEvent(); - if ($event && !isset($events[$event])) { - $events[$event] = $event; + foreach ($rules_config->getEvents() as $event) { + if ($event && !isset($events[$event['event_name']])) { + $events[$event['event_name']] = $event['event_name']; + } } } return $events; @@ -106,10 +107,13 @@ public function save(EntityInterface $entity) { // After the reaction rule is saved, we need to rebuild the container, // otherwise the reaction rule will not fire. However, we can do an - // optimization: if the event was already registered before, we do not have - // to rebuild the container. - if (empty($events_before[$entity->getEvent()])) { - $this->drupalKernel->rebuildContainer(); + // optimization: if every event was already registered before, we do not + // have to rebuild the container. + foreach ($entity->getEvents() as $event) { + if (empty($events_before[$event['event_name']])) { + $this->drupalKernel->rebuildContainer(); + break; + } } return $return; diff --git a/src/EventSubscriber/GenericEventSubscriber.php b/src/EventSubscriber/GenericEventSubscriber.php index 9f591b9c..f4a03c8d 100644 --- a/src/EventSubscriber/GenericEventSubscriber.php +++ b/src/EventSubscriber/GenericEventSubscriber.php @@ -125,7 +125,7 @@ public function onRulesEvent(Event $event, $event_name) { // another rule. foreach ($triggered_events as $triggered_event) { // @todo Only load active reaction rules here. - $configs = $storage->loadByProperties(['event' => $triggered_event]); + $configs = $storage->loadByProperties(['events.*.event_name' => $triggered_event]); // Loop over all rules and execute them. foreach ($configs as $config) { diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index 2fb521bd..ab26b69b 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -65,7 +65,8 @@ public function form(array $form, FormStateInterface $form_state) { } } - $form['event'] = [ + $form['events']['#tree'] = TRUE; + $form['events'][]['event_name'] = [ '#type' => 'select', '#title' => $this->t('React on event'), '#options' => $options, diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index 8e47d496..9e8b936c 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -48,12 +48,17 @@ public static function create(ContainerInterface $container) { public function form(array $form, FormStateInterface $form_state) { $this->addLockInformation($form); - $event_name = $this->entity->getEvent(); - $event_definition = $this->eventManager->getDefinition($event_name); - $form['event']['#markup'] = $this->t('Event: @label (@name)', [ - '@label' => $event_definition['label'], - '@name' => $event_name, - ]); + foreach ($this->entity->getEvents() as $key => $event) { + $event_definition = $this->eventManager->getDefinition($event['event_name']); + $form['event'][$key] = [ + '#type' => 'item', + '#title' => $this->t('Events') . ':', + '#markup' => $this->t('@label (@name)', [ + '@label' => $event_definition['label'], + '@name' => $event['event_name'], + ]), + ]; + } $form_handler = $this->entity->getExpression()->getFormHandler(); $form = $form_handler->form($form, $form_state); return parent::form($form, $form_state); diff --git a/tests/src/Kernel/EventIntegrationTest.php b/tests/src/Kernel/EventIntegrationTest.php index a13f551c..c21a2161 100644 --- a/tests/src/Kernel/EventIntegrationTest.php +++ b/tests/src/Kernel/EventIntegrationTest.php @@ -161,7 +161,7 @@ public function testInitEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'event' => KernelEvents::REQUEST, + 'events' => [['event_name' => KernelEvents::REQUEST]], 'configuration' => $rule->getConfiguration(), ]); $config_entity->save(); From 7d0d7e2bc2ab0657f1538ad50916edfc1e404f69 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 11 Feb 2016 18:26:01 +0100 Subject: [PATCH 14/57] Tests for multiple events. --- tests/src/Kernel/EventIntegrationTest.php | 42 ++++++++++++++++++++--- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/tests/src/Kernel/EventIntegrationTest.php b/tests/src/Kernel/EventIntegrationTest.php index c21a2161..50cfc73c 100644 --- a/tests/src/Kernel/EventIntegrationTest.php +++ b/tests/src/Kernel/EventIntegrationTest.php @@ -54,7 +54,7 @@ public function testUserLoginEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'event' => 'rules_user_login', + 'events' => [['event_name' => 'rules_user_login']], 'configuration' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -81,7 +81,7 @@ public function testUserLogoutEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'event' => 'rules_user_logout', + 'events' => [['event_name' => 'rules_user_logout']], 'configuration' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -108,7 +108,7 @@ public function testCronEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'event' => 'rules_system_cron', + 'events' => [['event_name' => 'rules_system_cron']], 'configuration' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -134,7 +134,7 @@ public function testSystemLoggerEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'event' => 'rules_system_logger_event', + 'events' => [['event_name' => 'rules_system_logger_event']], 'configuration' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -185,4 +185,38 @@ public function testInitEvent() { $this->assertRulesLogEntryExists('action called'); } + /** + * Test that rules config supports multiple events. + */ + public function testMultipleEvents() { + $rule = $this->expressionManager->createRule(); + $rule->addCondition('rules_test_true'); + $rule->addAction('rules_test_log'); + + $config_entity = $this->storage->create([ + 'id' => 'test_rule', + 'expression_id' => 'rules_rule', + 'events' => [ + ['event_name' => 'rules_user_login'], + ['event_name' => 'rules_user_logout'], + ], + 'configuration' => $rule->getConfiguration(), + ]); + $config_entity->save(); + + // The logger instance has changed, refresh it. + $this->logger = $this->container->get('logger.channel.rules'); + + $account = User::create(['name' => 'test_user']); + // Invoke the hook manually which should trigger the rules_user_login event. + rules_user_login($account); + // Invoke the hook manually which should trigger the rules_user_logout + // event. + rules_user_logout($account); + + // Test that the action in the rule logged something. + $this->assertRulesLogEntryExists('action called'); + $this->assertRulesLogEntryExists('action called', 1); + } + } From 87dca7d0d323776b23bb823a49dfca3f1045c118 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Mon, 15 Feb 2016 11:40:56 +0100 Subject: [PATCH 15/57] PR fixes. --- config/schema/rules.schema.yml | 32 ----------------------- src/Engine/RulesComponent.php | 4 +-- src/Entity/ReactionRuleConfig.php | 25 ++++++++++++++++-- src/Entity/ReactionRuleStorage.php | 10 +++---- src/Form/ReactionRuleEditForm.php | 6 ++--- tests/src/Kernel/EventIntegrationTest.php | 10 +++---- 6 files changed, 38 insertions(+), 49 deletions(-) diff --git a/config/schema/rules.schema.yml b/config/schema/rules.schema.yml index d225fb4a..75cbcb76 100644 --- a/config/schema/rules.schema.yml +++ b/config/schema/rules.schema.yml @@ -196,38 +196,6 @@ rules_expression.rules_rule: type: rules_expression.[id] label: 'Actions' -rules_expression.rules_reaction_rule: - type: rules_expression - label: "Reaction Rule" - mapping: - id: - type: string - label: 'Plugin ID' - uuid: - type: string - label: 'UUID' - events: - type: sequence - label: 'Events' - sequence: - type: mapping - label: 'Event' - mapping: - event_name: - type: string - label: 'Name' - configuration: - type: sequence - label: 'Configuration' - sequence: - type: mapping - conditions: - type: rules_expression.[id] - label: 'Conditions' - actions: - type: rules_expression.[id] - label: 'Actions' - rules.context.definition: type: mapping label: 'Context definition' diff --git a/src/Engine/RulesComponent.php b/src/Engine/RulesComponent.php index e65377a4..0d720f2a 100644 --- a/src/Engine/RulesComponent.php +++ b/src/Engine/RulesComponent.php @@ -125,9 +125,9 @@ public function getContextDefinitions() { */ public function addContextDefinitionsFrom(ConfigEntityInterface $rules_config) { if ($rules_config instanceof ReactionRuleConfig) { - foreach ($rules_config->getEvents() as $event) { + foreach ($rules_config->getEventNames() as $event_name) { // @todo Use setter injection for the service. - $event_definition = \Drupal::service('plugin.manager.rules_event')->getDefinition($event['event_name']); + $event_definition = \Drupal::service('plugin.manager.rules_event')->getDefinition($event_name); foreach ($event_definition['context'] as $context_name => $context_definition) { $this->addContextDefinition($context_name, $context_definition); } diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index c3d34423..fce61480 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -123,7 +123,8 @@ class ReactionRuleConfig extends ConfigEntityBase { /** * The events this reaction rule is reacting on. * - * Events array, key - numeric index, value - event array with next structure: + * Events array. The array is numerically indexed and contains arrays with the + * following structure: * - event_name: string with the event machine name. * - configuration: an array containing the event configuration. * @@ -212,12 +213,32 @@ public function getTag() { } /** - * Returns the array of events on which this rule will trigger. + * Gets configuration of all events the rule is reacting on. + * + * @return array + * The events array. The array is numerically indexed and contains arrays + * with the following structure: + * - event_name: string with the event machine name. + * - configuration: an array containing the event configuration. */ public function getEvents() { return $this->events; } + /** + * Gets fully qualified names of all events the rule is reacting on. + * + * @return string[] + * The array of fully qualified event names of the rule. + */ + public function getEventNames() { + $names = []; + foreach ($this->events as $event) { + $names[] = $event['event_name']; + } + return $names; + } + /** * {@inheritdoc} */ diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index 7a4f88b9..50dacac8 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -83,9 +83,9 @@ public static function createInstance(ContainerInterface $container, EntityTypeI protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { - foreach ($rules_config->getEvents() as $event) { - if ($event && !isset($events[$event['event_name']])) { - $events[$event['event_name']] = $event['event_name']; + foreach ($rules_config->getEventNames() as $event_name) { + if (!isset($events[$event_name])) { + $events[$event_name] = $event_name; } } } @@ -109,8 +109,8 @@ public function save(EntityInterface $entity) { // otherwise the reaction rule will not fire. However, we can do an // optimization: if every event was already registered before, we do not // have to rebuild the container. - foreach ($entity->getEvents() as $event) { - if (empty($events_before[$event['event_name']])) { + foreach ($entity->getEventNames() as $event_name) { + if (empty($events_before[$event_name])) { $this->drupalKernel->rebuildContainer(); break; } diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index 9e8b936c..311be4ef 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -48,14 +48,14 @@ public static function create(ContainerInterface $container) { public function form(array $form, FormStateInterface $form_state) { $this->addLockInformation($form); - foreach ($this->entity->getEvents() as $key => $event) { - $event_definition = $this->eventManager->getDefinition($event['event_name']); + foreach ($this->entity->getEventNames() as $key => $event_name) { + $event_definition = $this->eventManager->getDefinition($event_name); $form['event'][$key] = [ '#type' => 'item', '#title' => $this->t('Events') . ':', '#markup' => $this->t('@label (@name)', [ '@label' => $event_definition['label'], - '@name' => $event['event_name'], + '@name' => $event_name, ]), ]; } diff --git a/tests/src/Kernel/EventIntegrationTest.php b/tests/src/Kernel/EventIntegrationTest.php index 50cfc73c..686be03d 100644 --- a/tests/src/Kernel/EventIntegrationTest.php +++ b/tests/src/Kernel/EventIntegrationTest.php @@ -196,12 +196,12 @@ public function testMultipleEvents() { $config_entity = $this->storage->create([ 'id' => 'test_rule', 'expression_id' => 'rules_rule', - 'events' => [ - ['event_name' => 'rules_user_login'], - ['event_name' => 'rules_user_logout'], - ], - 'configuration' => $rule->getConfiguration(), ]); + $config_entity->set('events', [ + ['event_name' => 'rules_user_login'], + ['event_name' => 'rules_user_logout'], + ]); + $config_entity->set('configuration', $rule->getConfiguration()); $config_entity->save(); // The logger instance has changed, refresh it. From a58f03a9240acd1b6ce8e0cba81a5167081d161c Mon Sep 17 00:00:00 2001 From: milkovsky Date: Mon, 15 Feb 2016 18:26:55 +0100 Subject: [PATCH 16/57] Inject the Rules event manager. --- src/Entity/ReactionRuleStorage.php | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index eec4da22..e03cab6d 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -60,25 +60,15 @@ class ReactionRuleStorage extends ConfigEntityStorage { * The language manager. * @param \Drupal\Core\State\StateInterface $state_service * The state service. + * @param \Drupal\rules\Core\RulesEventManager $event_manager + * The Rules event manager. */ - public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, StateInterface $state_service, DrupalKernelInterface $drupal_kernel) { + public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, StateInterface $state_service, DrupalKernelInterface $drupal_kernel, RulesEventManager $event_manager) { parent::__construct($entity_type, $config_factory, $uuid_service, $language_manager); $this->stateService = $state_service; $this->drupalKernel = $drupal_kernel; - } - - /** - * Gets the event manager. - * - * @return \Drupal\rules\Core\RulesEventManager - * The event manager. - */ - protected function eventManager() { - if (!$this->eventManager) { - $this->eventManager = new RulesEventManager($this->moduleHandler()); - } - return $this->eventManager; + $this->eventManager = $event_manager; } /** @@ -91,7 +81,8 @@ public static function createInstance(ContainerInterface $container, EntityTypeI $container->get('uuid'), $container->get('language_manager'), $container->get('state'), - $container->get('kernel') + $container->get('kernel'), + $container->get('plugin.manager.rules_event') ); } @@ -105,7 +96,7 @@ protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { $event = $rules_config->getEvent(); - $event = $this->eventManager()->getEventBaseName($event); + $event = $this->eventManager->getEventBaseName($event); if ($event && !isset($events[$event])) { $events[$event] = $event; } From cb7f7623505c6bc67c6f21b17404f8ce4c07f648 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Tue, 16 Feb 2016 10:04:38 +0100 Subject: [PATCH 17/57] Code formatting. --- src/Entity/ReactionRuleConfig.php | 8 ++++---- src/Form/ReactionRuleEditForm.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index fce61480..7b8c1ddc 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -125,8 +125,8 @@ class ReactionRuleConfig extends ConfigEntityBase { * * Events array. The array is numerically indexed and contains arrays with the * following structure: - * - event_name: string with the event machine name. - * - configuration: an array containing the event configuration. + * - event_name: String with the event machine name. + * - configuration: An array containing the event configuration. * * @var array */ @@ -218,8 +218,8 @@ public function getTag() { * @return array * The events array. The array is numerically indexed and contains arrays * with the following structure: - * - event_name: string with the event machine name. - * - configuration: an array containing the event configuration. + * - event_name: String with the event machine name. + * - configuration: An array containing the event configuration. */ public function getEvents() { return $this->events; diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index 311be4ef..b41df04e 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -52,7 +52,7 @@ public function form(array $form, FormStateInterface $form_state) { $event_definition = $this->eventManager->getDefinition($event_name); $form['event'][$key] = [ '#type' => 'item', - '#title' => $this->t('Events') . ':', + '#title' => $this->t('Events:'), '#markup' => $this->t('@label (@name)', [ '@label' => $event_definition['label'], '@name' => $event_name, From 944aa09ca1c2b37d8b8fe93343c2cac983a82c33 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 18 Feb 2016 16:46:11 +0100 Subject: [PATCH 18/57] After merge fix. --- src/Entity/ReactionRuleConfig.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index 565c6c13..830169d2 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -174,7 +174,7 @@ public function getExpression() { */ public function getComponent() { $component = RulesComponent::create($this->getExpression()); - $component->addContextDefinitionsForEvents([$this->getEvent()]); + $component->addContextDefinitionsForEvents($this->getEventNames()); return $component; } From 9ead81eecc9ad76ad5dfd909da195780f52822d1 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 18 Feb 2016 16:53:14 +0100 Subject: [PATCH 19/57] Initialize events []. --- src/Entity/ReactionRuleConfig.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index 830169d2..4b7395c3 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -130,7 +130,7 @@ class ReactionRuleConfig extends ConfigEntityBase { * * @var array */ - protected $events; + protected $events = []; /** * Sets a Rules expression instance for this Reaction rule. From 92aacdc0b1878f3005c77b66acb6e9056f9b94e9 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 18 Feb 2016 18:34:35 +0100 Subject: [PATCH 20/57] Fix test after merge. --- tests/src/Kernel/ConfigurableEventHandlerTest.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/src/Kernel/ConfigurableEventHandlerTest.php b/tests/src/Kernel/ConfigurableEventHandlerTest.php index 28c2bfa3..084ec8bb 100644 --- a/tests/src/Kernel/ConfigurableEventHandlerTest.php +++ b/tests/src/Kernel/ConfigurableEventHandlerTest.php @@ -94,8 +94,12 @@ public function testConfigurableEventHandler() { $config_entity1 = $this->storage->create([ 'id' => 'test_rule1', 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node--page', - ])->setExpression($rule1); + 'event' => '', + ]); + $config_entity1->set('events', [ + ['event_name' => 'rules_entity_presave:node--page'], + ]); + $config_entity1->set('configuration', $rule1->getConfiguration()); $config_entity1->save(); // Create rule2 with the 'rules_entity_presave:node' event. @@ -107,8 +111,11 @@ public function testConfigurableEventHandler() { $config_entity2 = $this->storage->create([ 'id' => 'test_rule2', 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node', - ])->setExpression($rule2); + ]); + $config_entity2->set('events', [ + ['event_name' => 'rules_entity_presave:node'], + ]); + $config_entity2->set('configuration', $rule2->getConfiguration()); $config_entity2->save(); // The logger instance has changed, refresh it. From b866850ca35cdbad66e7247c6f6d6d645b38d40e Mon Sep 17 00:00:00 2001 From: milkovsky Date: Thu, 18 Feb 2016 18:38:30 +0100 Subject: [PATCH 21/57] Added ajax reload. --- src/Form/ReactionRuleAddForm.php | 1 + src/Form/RulesComponentFormBase.php | 32 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index ab26b69b..52952c2a 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -71,6 +71,7 @@ public function form(array $form, FormStateInterface $form_state) { '#title' => $this->t('React on event'), '#options' => $options, '#required' => TRUE, + '#ajax' => $this->getDefaultAjax(), '#description' => $this->t('Whenever the event occurs, rule evaluation is triggered.'), ]; diff --git a/src/Form/RulesComponentFormBase.php b/src/Form/RulesComponentFormBase.php index ccb8c6dd..a29c7b90 100644 --- a/src/Form/RulesComponentFormBase.php +++ b/src/Form/RulesComponentFormBase.php @@ -19,6 +19,10 @@ abstract class RulesComponentFormBase extends EntityForm { * {@inheritdoc} */ public function form(array $form, FormStateInterface $form_state) { + // Specify the wrapper div used by #ajax. + $form['#prefix'] = '
'; + $form['#suffix'] = '
'; + $form['settings'] = [ '#type' => 'details', '#title' => $this->t('Settings'), @@ -78,4 +82,32 @@ public function exists($id) { return (bool) $this->entityTypeManager->getStorage($type)->load($id); } + /** + * Get default form #ajax properties. + * + * @param string $effect + * (optional) The jQuery effect to use when placing the new HTML (used with + * 'wrapper'). Valid options are 'none' (default), 'slide', or 'fade'. + * + * @return array + */ + public function getDefaultAjax($effect = 'none') { + return array( + 'callback' => '::reloadForm', + 'wrapper' => 'rules-form-wrapper', + 'effect' => $effect, + 'speed' => 'fast', + ); + } + + /** + * Ajax callback to reload the form. + * + * @return array + * The reloaded form. + */ + public function reloadForm(array $form, FormStateInterface $form_state) { + return $form; + } + } From 5b1a9c3eccd4f25ab0d4c7dc190ffe77b55f9fff Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 19 Feb 2016 17:16:07 +0100 Subject: [PATCH 22/57] Bundle select + handler. WIP. --- .../ConfigurableEventHandlerEntityBundle.php | 40 ++++++++++++++++++- src/Form/ReactionRuleAddForm.php | 37 +++++++++++++++++ src/Form/ReactionRuleEditForm.php | 2 +- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php index 86baa041..47afed5c 100644 --- a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php +++ b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php @@ -15,6 +15,24 @@ */ class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase { + protected $bundlesInfo; + protected $entityInfo; + protected $entityType; + + /** + * {@inheritdoc} + */ + public function __construct(array $configuration, $plugin_id, $plugin_definition) { + parent::__construct($configuration, $plugin_id, $plugin_definition); + $this->entityType = $this->getEventNameSuffix(); + // @todo Do it in a non-deprecated way. + $this->entityInfo = \Drupal::entityTypeManager()->getDefinition($this->entityType); + $this->bundlesInfo = \Drupal::entityManager()->getBundleInfo($this->entityType); + if (!$this->bundlesInfo) { + throw new \InvalidArgumentException('Unsupported event name passed.'); + } + } + /** * {@inheritdoc} */ @@ -37,7 +55,24 @@ public function summary() { * {@inheritdoc} */ public function buildConfigurationForm(array $form, FormStateInterface $form_state) { - // Nothing to do by default. + $form['bundle'] = array( + '#type' => 'select', + '#title' => t('Restrict by @bundle', array('@bundle' => $this->entityInfo->getBundleLabel())), + '#description' => t('If you need to filter for multiple values, either add multiple events or use the "Entity is of bundle" condition instead.'), + '#default_value' => $this->configuration['bundle'], + '#empty_value' => '', + ); + foreach ($this->bundlesInfo as $name => $bundle_info) { + $form['bundle']['#options'][$name] = $bundle_info['label']; + } + return $form; + } + + /** + * {@inheritdoc} + */ + public function extractConfigurationFormValues(array &$form, FormStateInterface $form_state) { + $this->configuration['bundle'] = $form_state->getValue('bundle'); } /** @@ -51,7 +86,8 @@ public function validate() { * {@inheritdoc} */ public function getEventNameSuffix() { - // Nothing to do by default. + $parts = explode(':', $this->pluginId); + return $parts[1]; } /** diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index 52952c2a..352051e1 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -8,6 +8,7 @@ namespace Drupal\rules\Form; use Drupal\Core\Form\FormStateInterface; +use Drupal\rules\Core\RulesConfigurableEventHandlerInterface; use Drupal\rules\Core\RulesEventManager; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -75,6 +76,14 @@ public function form(array $form, FormStateInterface $form_state) { '#description' => $this->t('Whenever the event occurs, rule evaluation is triggered.'), ]; + $form['event_configuration'] = array(); + if ($values = $form_state->getValue('events')) { + $event_name = $values[0]['event_name']; + if ($handler = $this->getEventHandler($event_name)) { + $form['event_configuration'] = $handler->buildConfigurationForm(array(), $form_state); + } + } + return $form; } @@ -82,10 +91,38 @@ public function form(array $form, FormStateInterface $form_state) { * {@inheritdoc} */ public function save(array $form, FormStateInterface $form_state) { + $event_name = $form_state->getValue('events')[0]['event_name']; + if ($handler = $this->getEventHandler($event_name)) { + $handler->extractConfigurationFormValues($form['event_configuration'], $form_state); + // @todo Save in 2 places? + $this->entity->set('configuration', $handler->getConfiguration()); + $this->entity->set('events', [['event_name' => $event_name . '--' . $handler->getConfiguration()['bundle']]]); + // @todo Exception: The "rules_entity_view:node–article" plugin does not exist. + } parent::save($form, $form_state); drupal_set_message($this->t('Reaction rule %label has been created.', ['%label' => $this->entity->label()])); $form_state->setRedirect('entity.rules_reaction_rule.edit_form', ['rules_reaction_rule' => $this->entity->id()]); } + /** + * Gets event handler class. + * + * @param $event_name + * The event base name. + * @param array $configuration + * The event configuration. + * + * @return \Drupal\rules\Core\RulesConfigurableEventHandlerInterface|null + * The event handler, null if there is no proper handler. + */ + protected function getEventHandler($event_name, $configuration = []) { + $event_definition = $this->eventManager->getDefinition($event_name); + $handler_class = $event_definition['class']; + if (is_subclass_of($handler_class, RulesConfigurableEventHandlerInterface::class)) { + $handler = new $handler_class($configuration, $this->eventManager->getEventBaseName($event_name)); + return $handler; + } + } + } diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index 6e211b94..f6e37160 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -75,7 +75,7 @@ public function form(array $form, FormStateInterface $form_state) { foreach ($this->entity->getEventNames() as $key => $event_name) { $event_definition = $this->eventManager->getDefinition($event_name); - $form['event'][$key] = [ + $form['events'][$key] = [ '#type' => 'item', '#title' => $this->t('Events:'), '#markup' => $this->t('@label (@name)', [ From df48c33a7a086111a9781f2b2123628a051d0076 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 10:16:46 +0100 Subject: [PATCH 23/57] After merge fixes. --- src/Form/ReactionRuleEditForm.php | 6 +----- tests/src/Kernel/ConfigurableEventHandlerTest.php | 7 ++----- tests/src/Kernel/EventIntegrationTest.php | 3 +-- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index 8a039380..d86dc4fb 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -71,8 +71,6 @@ protected function prepareEntity() { * {@inheritdoc} */ public function form(array $form, FormStateInterface $form_state) { - $form['locked'] = $this->rulesUiHandler->addLockInformation(); - foreach ($this->entity->getEventNames() as $key => $event_name) { $event_definition = $this->eventManager->getDefinition($event_name); $form['events'][$key] = [ @@ -84,9 +82,7 @@ public function form(array $form, FormStateInterface $form_state) { ]), ]; } - $form_handler = $this->rulesUiHandler->getComponent() - ->getExpression()->getFormHandler(); - $form = $form_handler->form($form, $form_state); + $form = $this->rulesUiHandler->getForm()->buildForm($form, $form_state); return parent::form($form, $form_state); } diff --git a/tests/src/Kernel/ConfigurableEventHandlerTest.php b/tests/src/Kernel/ConfigurableEventHandlerTest.php index 084ec8bb..57eb1534 100644 --- a/tests/src/Kernel/ConfigurableEventHandlerTest.php +++ b/tests/src/Kernel/ConfigurableEventHandlerTest.php @@ -93,13 +93,11 @@ public function testConfigurableEventHandler() { ); $config_entity1 = $this->storage->create([ 'id' => 'test_rule1', - 'expression_id' => 'rules_rule', - 'event' => '', ]); $config_entity1->set('events', [ ['event_name' => 'rules_entity_presave:node--page'], ]); - $config_entity1->set('configuration', $rule1->getConfiguration()); + $config_entity1->set('expression', $rule1->getConfiguration()); $config_entity1->save(); // Create rule2 with the 'rules_entity_presave:node' event. @@ -110,12 +108,11 @@ public function testConfigurableEventHandler() { ); $config_entity2 = $this->storage->create([ 'id' => 'test_rule2', - 'expression_id' => 'rules_rule', ]); $config_entity2->set('events', [ ['event_name' => 'rules_entity_presave:node'], ]); - $config_entity2->set('configuration', $rule2->getConfiguration()); + $config_entity2->set('expression', $rule2->getConfiguration()); $config_entity2->save(); // The logger instance has changed, refresh it. diff --git a/tests/src/Kernel/EventIntegrationTest.php b/tests/src/Kernel/EventIntegrationTest.php index f70f91c4..7bc7e5d3 100644 --- a/tests/src/Kernel/EventIntegrationTest.php +++ b/tests/src/Kernel/EventIntegrationTest.php @@ -190,13 +190,12 @@ public function testMultipleEvents() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'expression_id' => 'rules_rule', ]); $config_entity->set('events', [ ['event_name' => 'rules_user_login'], ['event_name' => 'rules_user_logout'], ]); - $config_entity->set('configuration', $rule->getConfiguration()); + $config_entity->set('expression', $rule->getConfiguration()); $config_entity->save(); // The logger instance has changed, refresh it. From 1b353fb30c551a1a4688e6fa6f8f4466092fe133 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 11:59:34 +0100 Subject: [PATCH 24/57] Fixed event discovery by base name. --- src/Core/RulesEventManager.php | 2 +- src/Entity/ReactionRuleConfig.php | 27 ++++++++++++++++++++++++++- src/Entity/ReactionRuleStorage.php | 5 ++--- src/Form/ReactionRuleEditForm.php | 3 ++- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/Core/RulesEventManager.php b/src/Core/RulesEventManager.php index 160f7325..cc58d20e 100644 --- a/src/Core/RulesEventManager.php +++ b/src/Core/RulesEventManager.php @@ -64,7 +64,7 @@ public function processDefinition(&$definition, $plugin_id) { * Gets the base name of a configured event name. * * For a configured event name like {EVENT_NAME}--{SUFFIX}, the base event - * name {EVENT_NAME} is returned. + * name {EVENT_NAME} is returned.s * * @return string * The event base name. diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index 6c10f3ae..9efed6cf 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -164,7 +164,7 @@ public function getExpression() { */ public function getComponent() { $component = RulesComponent::create($this->getExpression()); - $component->addContextDefinitionsForEvents($this->getEventNames()); + $component->addContextDefinitionsForEvents($this->getEventBaseNames()); return $component; } @@ -253,6 +253,31 @@ public function getEventNames() { return $names; } + /** + * Gets the base names of all events the rule is reacting on. + * + * For a configured event name like {EVENT_NAME}--{SUFFIX}, the base event + * name {EVENT_NAME} is returned. + * + * @return string[] + * The array of base event names of the rule. + * + * @see \Drupal\rules\Core\RulesConfigurableEventHandlerInterface::getEventNameSuffix() + */ + public function getEventBaseNames() { + $names = []; + foreach ($this->events as $event) { + $event_name = $event['event_name']; + if (strpos($event_name, '--') !== FALSE) { + // Cut off any suffix from a configured event name. + $parts = explode('--', $event_name, 2); + $event_name = $parts[0]; + } + $names[] = $event_name; + } + return $names; + } + /** * {@inheritdoc} */ diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index 1fabfbbf..29c4713c 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -95,8 +95,7 @@ public static function createInstance(ContainerInterface $container, EntityTypeI protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { - foreach ($rules_config->getEventNames() as $event_name) { - $event_name = $this->eventManager->getEventBaseName($event_name); + foreach ($rules_config->getBaseEventNames() as $event_name) { if (!isset($events[$event_name])) { $events[$event_name] = $event_name; } @@ -122,7 +121,7 @@ public function save(EntityInterface $entity) { // otherwise the reaction rule will not fire. However, we can do an // optimization: if every event was already registered before, we do not // have to rebuild the container. - foreach ($entity->getEventNames() as $event_name) { + foreach ($entity->getEventBaseNames() as $event_name) { if (empty($events_before[$event_name])) { $this->drupalKernel->rebuildContainer(); break; diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index d86dc4fb..4cf4b8c7 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -72,7 +72,8 @@ protected function prepareEntity() { */ public function form(array $form, FormStateInterface $form_state) { foreach ($this->entity->getEventNames() as $key => $event_name) { - $event_definition = $this->eventManager->getDefinition($event_name); + $event_base_name = $this->eventManager->getEventBaseName($event_name); + $event_definition = $this->eventManager->getDefinition($event_base_name); $form['events'][$key] = [ '#type' => 'item', '#title' => $this->t('Events:'), From d179fcf23453762d13a0aecb34ef5fa43a099d01 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 12:02:17 +0100 Subject: [PATCH 25/57] Fixed expression set in rules handler test. --- .../src/Kernel/ConfigurableEventHandlerTest.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/src/Kernel/ConfigurableEventHandlerTest.php b/tests/src/Kernel/ConfigurableEventHandlerTest.php index 28c2bfa3..57eb1534 100644 --- a/tests/src/Kernel/ConfigurableEventHandlerTest.php +++ b/tests/src/Kernel/ConfigurableEventHandlerTest.php @@ -93,9 +93,11 @@ public function testConfigurableEventHandler() { ); $config_entity1 = $this->storage->create([ 'id' => 'test_rule1', - 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node--page', - ])->setExpression($rule1); + ]); + $config_entity1->set('events', [ + ['event_name' => 'rules_entity_presave:node--page'], + ]); + $config_entity1->set('expression', $rule1->getConfiguration()); $config_entity1->save(); // Create rule2 with the 'rules_entity_presave:node' event. @@ -106,9 +108,11 @@ public function testConfigurableEventHandler() { ); $config_entity2 = $this->storage->create([ 'id' => 'test_rule2', - 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node', - ])->setExpression($rule2); + ]); + $config_entity2->set('events', [ + ['event_name' => 'rules_entity_presave:node'], + ]); + $config_entity2->set('expression', $rule2->getConfiguration()); $config_entity2->save(); // The logger instance has changed, refresh it. From 5eea48cc3233b38018f2b5d3007aff3d5fc2f3cd Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 12:07:49 +0100 Subject: [PATCH 26/57] Fix testMultipleEvents test. --- tests/src/Kernel/EventIntegrationTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/src/Kernel/EventIntegrationTest.php b/tests/src/Kernel/EventIntegrationTest.php index f70f91c4..7bc7e5d3 100644 --- a/tests/src/Kernel/EventIntegrationTest.php +++ b/tests/src/Kernel/EventIntegrationTest.php @@ -190,13 +190,12 @@ public function testMultipleEvents() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'expression_id' => 'rules_rule', ]); $config_entity->set('events', [ ['event_name' => 'rules_user_login'], ['event_name' => 'rules_user_logout'], ]); - $config_entity->set('configuration', $rule->getConfiguration()); + $config_entity->set('expression', $rule->getConfiguration()); $config_entity->save(); // The logger instance has changed, refresh it. From c936dfc9f441863d026106eee94b64eeda7d2cf7 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 13:27:41 +0100 Subject: [PATCH 27/57] Implemented summary(). --- .../ConfigurableEventHandlerEntityBundle.php | 23 +++++++++++++++++-- src/Form/ReactionRuleAddForm.php | 4 +--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php index 47afed5c..b8bbcb6c 100644 --- a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php +++ b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php @@ -15,8 +15,25 @@ */ class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase { + /** + * The bundles information for the entity. + * + * @var array + */ protected $bundlesInfo; + + /** + * The entity info plugin definition. + * + * @var mixed + */ protected $entityInfo; + + /** + * The entity type. + * + * @var string + */ protected $entityType; /** @@ -25,7 +42,6 @@ class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase public function __construct(array $configuration, $plugin_id, $plugin_definition) { parent::__construct($configuration, $plugin_id, $plugin_definition); $this->entityType = $this->getEventNameSuffix(); - // @todo Do it in a non-deprecated way. $this->entityInfo = \Drupal::entityTypeManager()->getDefinition($this->entityType); $this->bundlesInfo = \Drupal::entityManager()->getBundleInfo($this->entityType); if (!$this->bundlesInfo) { @@ -48,7 +64,10 @@ public static function determineQualifiedEvents(Event $event, $event_name, array * {@inheritdoc} */ public function summary() { - // Nothing to do by default. + $bundle = $this->configuration['bundle']; + $bundle_label = isset($this->bundlesInfo[$bundle]['label']) ? $this->bundlesInfo[$bundle]['label'] : $bundle; + $suffix = isset($bundle) ? ' ' . t('of @bundle-key %name', array('@bundle-key' => $this->entityInfo->getBundleLabel(), '%name' => $bundle_label)) : ''; + return $this->pluginDefinition['label']->render() . $suffix; } /** diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index 352051e1..46783868 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -94,10 +94,8 @@ public function save(array $form, FormStateInterface $form_state) { $event_name = $form_state->getValue('events')[0]['event_name']; if ($handler = $this->getEventHandler($event_name)) { $handler->extractConfigurationFormValues($form['event_configuration'], $form_state); - // @todo Save in 2 places? $this->entity->set('configuration', $handler->getConfiguration()); $this->entity->set('events', [['event_name' => $event_name . '--' . $handler->getConfiguration()['bundle']]]); - // @todo Exception: The "rules_entity_view:node–article" plugin does not exist. } parent::save($form, $form_state); @@ -120,7 +118,7 @@ protected function getEventHandler($event_name, $configuration = []) { $event_definition = $this->eventManager->getDefinition($event_name); $handler_class = $event_definition['class']; if (is_subclass_of($handler_class, RulesConfigurableEventHandlerInterface::class)) { - $handler = new $handler_class($configuration, $this->eventManager->getEventBaseName($event_name)); + $handler = new $handler_class($configuration, $this->eventManager->getEventBaseName($event_name), $event_definition); return $handler; } } From 8a0f988351fc3efb22a011d5af8ad94e1349d7ac Mon Sep 17 00:00:00 2001 From: Wolfgang Ziegler // fago Date: Fri, 26 Feb 2016 16:26:48 +0100 Subject: [PATCH 28/57] Issue #2666200 by a.milkovsky: Change reaction-rule config to save the event with settings --- config/schema/rules.schema.yml | 18 ++++++-- src/Entity/ReactionRuleConfig.php | 41 ++++++++++++++---- src/Entity/ReactionRuleStorage.php | 20 +++++---- .../GenericEventSubscriber.php | 2 +- src/Form/ReactionRuleAddForm.php | 3 +- src/Form/ReactionRuleEditForm.php | 17 +++++--- .../Kernel/ConfigurableEventHandlerTest.php | 16 ++++--- tests/src/Kernel/EventIntegrationTest.php | 43 ++++++++++++++++--- 8 files changed, 122 insertions(+), 38 deletions(-) diff --git a/config/schema/rules.schema.yml b/config/schema/rules.schema.yml index 7b1b7225..9230169d 100644 --- a/config/schema/rules.schema.yml +++ b/config/schema/rules.schema.yml @@ -35,9 +35,21 @@ rules.reaction.*: label: type: label label: 'Label' - event: - type: string - label: 'Event' + events: + type: sequence + label: 'Events' + sequence: + type: mapping + label: 'Event' + mapping: + event_name: + type: string + label: 'Name' + configuration: + type: sequence + label: 'Configuration' + sequence: + type: mapping module: type: string label: 'Module' diff --git a/src/Entity/ReactionRuleConfig.php b/src/Entity/ReactionRuleConfig.php index 8620d9ec..6c10f3ae 100644 --- a/src/Entity/ReactionRuleConfig.php +++ b/src/Entity/ReactionRuleConfig.php @@ -37,7 +37,7 @@ * config_export = { * "id", * "label", - * "event", + * "events", * "module", * "description", * "tag", @@ -116,11 +116,16 @@ class ReactionRuleConfig extends ConfigEntityBase implements RulesUiComponentPro protected $module = 'rules'; /** - * The event name this reaction rule is reacting on. + * The events this reaction rule is reacting on. * - * @var string + * Events array. The array is numerically indexed and contains arrays with the + * following structure: + * - event_name: String with the event machine name. + * - configuration: An array containing the event configuration. + * + * @var array */ - protected $event; + protected $events = []; /** * Sets a Rules expression instance for this Reaction rule. @@ -159,7 +164,7 @@ public function getExpression() { */ public function getComponent() { $component = RulesComponent::create($this->getExpression()); - $component->addContextDefinitionsForEvents([$this->getEvent()]); + $component->addContextDefinitionsForEvents($this->getEventNames()); return $component; } @@ -222,10 +227,30 @@ public function getTag() { } /** - * Returns the event on which this rule will trigger. + * Gets configuration of all events the rule is reacting on. + * + * @return array + * The events array. The array is numerically indexed and contains arrays + * with the following structure: + * - event_name: String with the event machine name. + * - configuration: An array containing the event configuration. + */ + public function getEvents() { + return $this->events; + } + + /** + * Gets fully qualified names of all events the rule is reacting on. + * + * @return string[] + * The array of fully qualified event names of the rule. */ - public function getEvent() { - return $this->event; + public function getEventNames() { + $names = []; + foreach ($this->events as $event) { + $names[] = $event['event_name']; + } + return $names; } /** diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index e03cab6d..1fabfbbf 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -95,10 +95,11 @@ public static function createInstance(ContainerInterface $container, EntityTypeI protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { - $event = $rules_config->getEvent(); - $event = $this->eventManager->getEventBaseName($event); - if ($event && !isset($events[$event])) { - $events[$event] = $event; + foreach ($rules_config->getEventNames() as $event_name) { + $event_name = $this->eventManager->getEventBaseName($event_name); + if (!isset($events[$event_name])) { + $events[$event_name] = $event_name; + } } } return $events; @@ -119,10 +120,13 @@ public function save(EntityInterface $entity) { // After the reaction rule is saved, we need to rebuild the container, // otherwise the reaction rule will not fire. However, we can do an - // optimization: if the event was already registered before, we do not have - // to rebuild the container. - if (empty($events_before[$entity->getEvent()])) { - $this->drupalKernel->rebuildContainer(); + // optimization: if every event was already registered before, we do not + // have to rebuild the container. + foreach ($entity->getEventNames() as $event_name) { + if (empty($events_before[$event_name])) { + $this->drupalKernel->rebuildContainer(); + break; + } } return $return; diff --git a/src/EventSubscriber/GenericEventSubscriber.php b/src/EventSubscriber/GenericEventSubscriber.php index ce3edd81..4fa13f64 100644 --- a/src/EventSubscriber/GenericEventSubscriber.php +++ b/src/EventSubscriber/GenericEventSubscriber.php @@ -125,7 +125,7 @@ public function onRulesEvent(Event $event, $event_name) { // another rule. foreach ($triggered_events as $triggered_event) { // @todo Only load active reaction rules here. - $configs = $storage->loadByProperties(['event' => $triggered_event]); + $configs = $storage->loadByProperties(['events.*.event_name' => $triggered_event]); // Loop over all rules and execute them. foreach ($configs as $config) { diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index 2fb521bd..ab26b69b 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -65,7 +65,8 @@ public function form(array $form, FormStateInterface $form_state) { } } - $form['event'] = [ + $form['events']['#tree'] = TRUE; + $form['events'][]['event_name'] = [ '#type' => 'select', '#title' => $this->t('React on event'), '#options' => $options, diff --git a/src/Form/ReactionRuleEditForm.php b/src/Form/ReactionRuleEditForm.php index 1a876c44..b1f3f7fc 100644 --- a/src/Form/ReactionRuleEditForm.php +++ b/src/Form/ReactionRuleEditForm.php @@ -71,12 +71,17 @@ protected function prepareEntity() { * {@inheritdoc} */ public function form(array $form, FormStateInterface $form_state) { - $event_name = $this->entity->getEvent(); - $event_definition = $this->eventManager->getDefinition($event_name); - $form['event']['#markup'] = $this->t('Event: @label (@name)', [ - '@label' => $event_definition['label'], - '@name' => $event_name, - ]); + foreach ($this->entity->getEventNames() as $key => $event_name) { + $event_definition = $this->eventManager->getDefinition($event_name); + $form['event'][$key] = [ + '#type' => 'item', + '#title' => $this->t('Events:'), + '#markup' => $this->t('@label (@name)', [ + '@label' => $event_definition['label'], + '@name' => $event_name, + ]), + ]; + } $form = $this->rulesUiHandler->getForm()->buildForm($form, $form_state); return parent::form($form, $form_state); } diff --git a/tests/src/Kernel/ConfigurableEventHandlerTest.php b/tests/src/Kernel/ConfigurableEventHandlerTest.php index 28c2bfa3..57eb1534 100644 --- a/tests/src/Kernel/ConfigurableEventHandlerTest.php +++ b/tests/src/Kernel/ConfigurableEventHandlerTest.php @@ -93,9 +93,11 @@ public function testConfigurableEventHandler() { ); $config_entity1 = $this->storage->create([ 'id' => 'test_rule1', - 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node--page', - ])->setExpression($rule1); + ]); + $config_entity1->set('events', [ + ['event_name' => 'rules_entity_presave:node--page'], + ]); + $config_entity1->set('expression', $rule1->getConfiguration()); $config_entity1->save(); // Create rule2 with the 'rules_entity_presave:node' event. @@ -106,9 +108,11 @@ public function testConfigurableEventHandler() { ); $config_entity2 = $this->storage->create([ 'id' => 'test_rule2', - 'expression_id' => 'rules_rule', - 'event' => 'rules_entity_presave:node', - ])->setExpression($rule2); + ]); + $config_entity2->set('events', [ + ['event_name' => 'rules_entity_presave:node'], + ]); + $config_entity2->set('expression', $rule2->getConfiguration()); $config_entity2->save(); // The logger instance has changed, refresh it. diff --git a/tests/src/Kernel/EventIntegrationTest.php b/tests/src/Kernel/EventIntegrationTest.php index d1dff4f4..7bc7e5d3 100644 --- a/tests/src/Kernel/EventIntegrationTest.php +++ b/tests/src/Kernel/EventIntegrationTest.php @@ -53,7 +53,7 @@ public function testUserLoginEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'event' => 'rules_user_login', + 'events' => [['event_name' => 'rules_user_login']], 'expression' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -79,7 +79,7 @@ public function testUserLogoutEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'event' => 'rules_user_logout', + 'events' => [['event_name' => 'rules_user_logout']], 'expression' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -105,7 +105,7 @@ public function testCronEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'event' => 'rules_system_cron', + 'events' => [['event_name' => 'rules_system_cron']], 'expression' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -130,7 +130,7 @@ public function testSystemLoggerEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'event' => 'rules_system_logger_event', + 'events' => [['event_name' => 'rules_system_logger_event']], 'expression' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -156,7 +156,7 @@ public function testInitEvent() { $config_entity = $this->storage->create([ 'id' => 'test_rule', - 'event' => KernelEvents::REQUEST, + 'events' => [['event_name' => KernelEvents::REQUEST]], 'expression' => $rule->getConfiguration(), ]); $config_entity->save(); @@ -180,4 +180,37 @@ public function testInitEvent() { $this->assertRulesLogEntryExists('action called'); } + /** + * Test that rules config supports multiple events. + */ + public function testMultipleEvents() { + $rule = $this->expressionManager->createRule(); + $rule->addCondition('rules_test_true'); + $rule->addAction('rules_test_log'); + + $config_entity = $this->storage->create([ + 'id' => 'test_rule', + ]); + $config_entity->set('events', [ + ['event_name' => 'rules_user_login'], + ['event_name' => 'rules_user_logout'], + ]); + $config_entity->set('expression', $rule->getConfiguration()); + $config_entity->save(); + + // The logger instance has changed, refresh it. + $this->logger = $this->container->get('logger.channel.rules'); + + $account = User::create(['name' => 'test_user']); + // Invoke the hook manually which should trigger the rules_user_login event. + rules_user_login($account); + // Invoke the hook manually which should trigger the rules_user_logout + // event. + rules_user_logout($account); + + // Test that the action in the rule logged something. + $this->assertRulesLogEntryExists('action called'); + $this->assertRulesLogEntryExists('action called', 1); + } + } From a341b281be9783d9e537a4b014ff0f82322c752e Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 18:21:06 +0100 Subject: [PATCH 29/57] Refactoring. --- ...RulesConfigurableEventHandlerInterface.php | 3 ++- .../ConfigurableEventHandlerEntityBundle.php | 4 ++-- src/Form/ReactionRuleAddForm.php | 24 ++++++++++++++----- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/Core/RulesConfigurableEventHandlerInterface.php b/src/Core/RulesConfigurableEventHandlerInterface.php index 10e6d7d1..e65e55d9 100644 --- a/src/Core/RulesConfigurableEventHandlerInterface.php +++ b/src/Core/RulesConfigurableEventHandlerInterface.php @@ -90,6 +90,7 @@ public function extractConfigurationFormValues(array &$form, FormStateInterface public function validate(); /** + * todo * Provides the event name suffix based upon the plugin configuration. * * If the event is configured and a suffix is provided, the event name Rules @@ -98,7 +99,7 @@ public function validate(); * @return string|false * The suffix string, for FALSE if no suffix should be appended. */ - public function getEventNameSuffix(); + public function getEventNamePrefix(); /** * Refines provided context definitions based upon plugin configuration. diff --git a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php index b8bbcb6c..5be9f575 100644 --- a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php +++ b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php @@ -41,7 +41,7 @@ class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase */ public function __construct(array $configuration, $plugin_id, $plugin_definition) { parent::__construct($configuration, $plugin_id, $plugin_definition); - $this->entityType = $this->getEventNameSuffix(); + $this->entityType = $this->getEventNamePrefix(); $this->entityInfo = \Drupal::entityTypeManager()->getDefinition($this->entityType); $this->bundlesInfo = \Drupal::entityManager()->getBundleInfo($this->entityType); if (!$this->bundlesInfo) { @@ -104,7 +104,7 @@ public function validate() { /** * {@inheritdoc} */ - public function getEventNameSuffix() { + public function getEventNamePrefix() { $parts = explode(':', $this->pluginId); return $parts[1]; } diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index 46783868..a255fb67 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -74,6 +74,7 @@ public function form(array $form, FormStateInterface $form_state) { '#required' => TRUE, '#ajax' => $this->getDefaultAjax(), '#description' => $this->t('Whenever the event occurs, rule evaluation is triggered.'), + '#submit' => array('::submitForm'), ]; $form['event_configuration'] = array(); @@ -90,13 +91,22 @@ public function form(array $form, FormStateInterface $form_state) { /** * {@inheritdoc} */ - public function save(array $form, FormStateInterface $form_state) { - $event_name = $form_state->getValue('events')[0]['event_name']; - if ($handler = $this->getEventHandler($event_name)) { - $handler->extractConfigurationFormValues($form['event_configuration'], $form_state); - $this->entity->set('configuration', $handler->getConfiguration()); - $this->entity->set('events', [['event_name' => $event_name . '--' . $handler->getConfiguration()['bundle']]]); + public function buildEntity(array $form, FormStateInterface $form_state) { + $entity = parent::buildEntity($form, $form_state); + foreach ($entity->getEventBaseNames() as $event_name) { + if ($handler = $this->getEventHandler($event_name)) { + $handler->extractConfigurationFormValues($form['event_configuration'], $form_state); + $entity->set('configuration', $handler->getConfiguration()); + $entity->set('events', [['event_name' => $event_name . '--' . $handler->getConfiguration()['bundle']]]); + } } + return $entity; + } + + /** + * {@inheritdoc} + */ + public function save(array $form, FormStateInterface $form_state) { parent::save($form, $form_state); drupal_set_message($this->t('Reaction rule %label has been created.', ['%label' => $this->entity->label()])); @@ -106,6 +116,8 @@ public function save(array $form, FormStateInterface $form_state) { /** * Gets event handler class. * + * Currently event handler is available only when the event is configurable. + * * @param $event_name * The event base name. * @param array $configuration From fa7d831827189da6a65f47e78e500bc5f05b72d1 Mon Sep 17 00:00:00 2001 From: milkovsky Date: Fri, 26 Feb 2016 18:24:07 +0100 Subject: [PATCH 30/57] Refactoring. --- src/Core/RulesEventManager.php | 2 +- src/Form/ReactionRuleAddForm.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/RulesEventManager.php b/src/Core/RulesEventManager.php index cc58d20e..160f7325 100644 --- a/src/Core/RulesEventManager.php +++ b/src/Core/RulesEventManager.php @@ -64,7 +64,7 @@ public function processDefinition(&$definition, $plugin_id) { * Gets the base name of a configured event name. * * For a configured event name like {EVENT_NAME}--{SUFFIX}, the base event - * name {EVENT_NAME} is returned.s + * name {EVENT_NAME} is returned. * * @return string * The event base name. diff --git a/src/Form/ReactionRuleAddForm.php b/src/Form/ReactionRuleAddForm.php index a255fb67..7b1869e7 100644 --- a/src/Form/ReactionRuleAddForm.php +++ b/src/Form/ReactionRuleAddForm.php @@ -74,7 +74,7 @@ public function form(array $form, FormStateInterface $form_state) { '#required' => TRUE, '#ajax' => $this->getDefaultAjax(), '#description' => $this->t('Whenever the event occurs, rule evaluation is triggered.'), - '#submit' => array('::submitForm'), + '#executes_submit_callback' => array('::submitForm'), ]; $form['event_configuration'] = array(); From d44f0265212659acb362735b88bf090637a57326 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 12:59:39 +0100 Subject: [PATCH 31/57] Issue #2677018 by fago: Condition and Action expression plugins have unused context methods. Also improve the documenation of those plugins a bit. --- src/Core/ConditionManager.php | 10 ++++++++ src/Core/RulesActionManagerInterface.php | 8 +++++++ src/Engine/IntegrityCheckTrait.php | 2 +- src/Plugin/RulesExpression/RulesAction.php | 24 +------------------ src/Plugin/RulesExpression/RulesCondition.php | 22 ++++------------- .../Integration/Engine/IntegrityCheckTest.php | 4 ++-- tests/src/Unit/RulesConditionTest.php | 20 ++-------------- 7 files changed, 28 insertions(+), 62 deletions(-) diff --git a/src/Core/ConditionManager.php b/src/Core/ConditionManager.php index 27b0e4e1..c3bbd9bf 100644 --- a/src/Core/ConditionManager.php +++ b/src/Core/ConditionManager.php @@ -16,6 +16,16 @@ */ class ConditionManager extends CoreConditionManager { + /** + * {@inheritdoc} + * + * @return \Drupal\rules\Core\RulesConditionInterface|\Drupal\Core\Condition\ConditionInterface + * A fully configured plugin instance. + */ + public function createInstance($plugin_id, array $configuration = []) { + return parent::createInstance($plugin_id, $configuration); + } + /** * {@inheritdoc} */ diff --git a/src/Core/RulesActionManagerInterface.php b/src/Core/RulesActionManagerInterface.php index 36f4a304..cb3e9ecd 100644 --- a/src/Core/RulesActionManagerInterface.php +++ b/src/Core/RulesActionManagerInterface.php @@ -20,4 +20,12 @@ */ interface RulesActionManagerInterface extends CategorizingPluginManagerInterface, ContextAwarePluginManagerInterface { + /** + * {@inheritdoc} + * + * @return \Drupal\rules\Core\RulesActionInterface + * A fully configured plugin instance. + */ + public function createInstance($plugin_id, array $configuration = []); + } diff --git a/src/Engine/IntegrityCheckTrait.php b/src/Engine/IntegrityCheckTrait.php index 19b473b6..e5385973 100644 --- a/src/Engine/IntegrityCheckTrait.php +++ b/src/Engine/IntegrityCheckTrait.php @@ -171,7 +171,7 @@ protected function checkDataTypeCompatible(ContextDefinitionInterface $context_d * @param CoreContextAwarePluginInterface $plugin * The action or condition plugin that may provide variables. * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state - * The excution metadata state to add variables to. + * The execution metadata state to add variables to. */ public function addProvidedVariablesToExecutionMetadataState(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { if ($plugin instanceof ContextProviderInterface) { diff --git a/src/Plugin/RulesExpression/RulesAction.php b/src/Plugin/RulesExpression/RulesAction.php index 8ea59fc1..07a31707 100644 --- a/src/Plugin/RulesExpression/RulesAction.php +++ b/src/Plugin/RulesExpression/RulesAction.php @@ -7,7 +7,6 @@ namespace Drupal\rules\Plugin\RulesExpression; -use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\rules\Context\ContextHandlerTrait; use Drupal\rules\Context\DataProcessorManager; @@ -101,7 +100,7 @@ public function executeWithState(ExecutionStateInterface $state) { // action plugin. $this->mapContext($action, $state); - $action->refineContextdefinitions(); + $action->refineContextDefinitions(); // Send the context value through configured data processor before executing // the action. @@ -120,27 +119,6 @@ public function executeWithState(ExecutionStateInterface $state) { $this->mapProvidedContext($action, $state); } - /** - * {@inheritdoc} - */ - public function getContextDefinitions() { - // Pass up the context definitions from the action plugin. - $definition = $this->actionManager->getDefinition($this->configuration['action_id']); - return !empty($definition['context']) ? $definition['context'] : []; - } - - /** - * {@inheritdoc} - */ - public function getContextDefinition($name) { - // Pass up the context definitions from the action plugin. - $definition = $this->actionManager->getDefinition($this->configuration['action_id']); - if (empty($definition['context'][$name])) { - throw new ContextException(sprintf("The %s context is not a valid context.", $name)); - } - return $definition['context'][$name]; - } - /** * {@inheritdoc} */ diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index c0046538..4d39e971 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -7,10 +7,10 @@ namespace Drupal\rules\Plugin\RulesExpression; -use Drupal\Core\Condition\ConditionManager; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\rules\Context\ContextHandlerTrait; use Drupal\rules\Context\DataProcessorManager; +use Drupal\rules\Core\ConditionManager; use Drupal\rules\Engine\ConditionExpressionInterface; use Drupal\rules\Engine\ExecutionMetadataStateInterface; use Drupal\rules\Engine\ExecutionStateInterface; @@ -40,7 +40,7 @@ class RulesCondition extends ExpressionBase implements ConditionExpressionInterf /** * The condition manager used to instantiate the condition plugin. * - * @var \Drupal\Core\Condition\ConditionManager + * @var \Drupal\rules\Core\ConditionManager */ protected $conditionManager; @@ -55,7 +55,7 @@ class RulesCondition extends ExpressionBase implements ConditionExpressionInterf * The plugin ID for the plugin instance. * @param mixed $plugin_definition * The plugin implementation definition. - * @param \Drupal\Core\Condition\ConditionManager $condition_manager + * @param \Drupal\rules\Core\ConditionManager $condition_manager * The condition manager. * @param \Drupal\rules\Context\DataProcessorManager $processor_manager * The data processor plugin manager. @@ -116,7 +116,7 @@ public function executeWithState(ExecutionStateInterface $state) { // condition plugin. $this->mapContext($condition, $state); - $condition->refineContextdefinitions(); + $condition->refineContextDefinitions(); // Send the context values through configured data processors before // evaluating the condition. @@ -142,20 +142,6 @@ public function isNegated() { return !empty($this->configuration['negate']); } - /** - * {@inheritdoc} - */ - public function getContextDefinitions() { - if (!isset($this->contextDefinitions)) { - // Pass up the context definitions from the condition plugin. - // @todo do not always create plugin instances here, the instance should - // be reused. Maybe that is what plugin bags are for? - $condition = $this->conditionManager->createInstance($this->configuration['condition_id']); - $this->contextDefinitions = $condition->getContextDefinitions(); - } - return $this->contextDefinitions; - } - /** * {@inheritdoc} */ diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index 4466834a..697633c5 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -166,7 +166,7 @@ public function testInvalidProvidedName() { } /** - * Tests the input restrction on contexts. + * Tests the input restriction on contexts. */ public function testInputRestriction() { $rule = $this->rulesExpressionManager->createRule(); @@ -299,7 +299,7 @@ public function testComplexTypeViolation() { public function testMissingRequiredContext() { $rule = $this->rulesExpressionManager->createRule(); - // The condition is completely unconfigured, missing 2 required contexts. + // The condition is completely un-configured, missing 2 required contexts. $condition = $this->rulesExpressionManager->createCondition('rules_node_is_of_type'); $rule->addExpressionObject($condition); diff --git a/tests/src/Unit/RulesConditionTest.php b/tests/src/Unit/RulesConditionTest.php index 262b8fb6..3e214884 100644 --- a/tests/src/Unit/RulesConditionTest.php +++ b/tests/src/Unit/RulesConditionTest.php @@ -7,7 +7,7 @@ namespace Drupal\Tests\rules\Unit; -use Drupal\Core\Condition\ConditionManager; +use Drupal\rules\Core\ConditionManager; use Drupal\Core\Plugin\Context\ContextDefinitionInterface; use Drupal\rules\Context\DataProcessorInterface; use Drupal\rules\Context\ContextConfig; @@ -28,7 +28,7 @@ class RulesConditionTest extends UnitTestCase { /** * The mocked condition manager. * - * @var \Drupal\Core\Condition\ConditionManager|\Prophecy\Prophecy\ProphecyInterface + * @var \Drupal\rules\Core\ConditionManager|\Prophecy\Prophecy\ProphecyInterface */ protected $conditionManager; @@ -73,22 +73,6 @@ public function setUp() { $this->conditionManager->reveal(), $this->processorManager->reveal()); } - /** - * Tests that context definitions are retrieved form the plugin. - */ - public function testContextDefinitions() { - $context_definition = $this->prophesize(ContextDefinitionInterface::class); - $this->trueCondition->getContextDefinitions() - ->willReturn(['test' => $context_definition->reveal()]) - ->shouldBeCalledTimes(1); - - $this->conditionManager->createInstance('test_condition') - ->willReturn($this->trueCondition->reveal()) - ->shouldBeCalledTimes(1); - - $this->assertSame($this->conditionExpression->getContextDefinitions(), ['test' => $context_definition->reveal()]); - } - /** * Tests that context values get data processed with processor mappings. */ From 660a5057fda2bea31582e8ca41275b84c18e84c3 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 16:20:02 +0100 Subject: [PATCH 32/57] Issue #2677042 by fago: Test that using provided variables passes integrity check. --- .../Integration/Engine/IntegrityCheckTest.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index 697633c5..041bf695 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -339,4 +339,24 @@ public function testNestedExpressionUuids() { $this->assertEquals($action->getUuid(), $violation_list[0]->getUuid()); } + /** + * Tests using provided variables in sub-sequent actions passes checks. + */ + public function testUsingProvidedVariables() { + $rule = $this->rulesExpressionManager->createRule(); + + $rule->addAction('rules_variable_add', ContextConfig::create() + ->setValue('type', 'any') + ->setValue('value', 'foo') + ); + $rule->addAction('rules_variable_add', ContextConfig::create() + ->setValue('type', 'any') + ->map('value', 'variable_added') + ); + + $violation_list = RulesComponent::create($rule) + ->checkIntegrity(); + $this->assertEquals(0, iterator_count($violation_list)); + } + } From 7f51cd4c1bb9c08a9129a34f1658ec1e1e62b902 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 13:50:27 +0100 Subject: [PATCH 33/57] Issue #2677034: Add a test case to ensure context is refined when checking integrity. --- src/Plugin/RulesAction/VariableAdd.php | 1 + .../Integration/Engine/IntegrityCheckTest.php | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/Plugin/RulesAction/VariableAdd.php b/src/Plugin/RulesAction/VariableAdd.php index 3b5e8ff7..a4bb0c13 100644 --- a/src/Plugin/RulesAction/VariableAdd.php +++ b/src/Plugin/RulesAction/VariableAdd.php @@ -51,6 +51,7 @@ protected function doExecute($type, $value) { */ public function refineContextDefinitions() { if ($type = $this->getContextValue('type')) { + $this->pluginDefinition['context']['value']->setDataType($type); $this->pluginDefinition['provides']['variable_added']->setDataType($type); } } diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index 041bf695..0ba22903 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -216,6 +216,25 @@ public function testSelectorRestriction() { $this->assertEquals($action->getUuid(), $violation_list[0]->getUuid()); } + /** + * Tests that refined context is respected when checking context. + */ + public function testRefinedContextViolation() { + $rule = $this->rulesExpressionManager->createRule(); + + $action = $this->rulesExpressionManager->createAction('rules_variable_add', ContextConfig::create() + ->setValue('type', 'integer') + ->map('value', 'text') + ->toArray() + ); + $rule->addExpressionObject($action); + + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('text', ContextDefinition::create('string')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + } + /** * Tests that a primitive context is assigned something that matches. */ From f3234cf657c38b95f0ea0f805951bb636fc1de4d Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 16:10:42 +0100 Subject: [PATCH 34/57] Issue #2677034: Add test case to verify provided variables use refined context when checking integrity. --- .../Integration/Engine/IntegrityCheckTest.php | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index 0ba22903..1611096b 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -216,25 +216,6 @@ public function testSelectorRestriction() { $this->assertEquals($action->getUuid(), $violation_list[0]->getUuid()); } - /** - * Tests that refined context is respected when checking context. - */ - public function testRefinedContextViolation() { - $rule = $this->rulesExpressionManager->createRule(); - - $action = $this->rulesExpressionManager->createAction('rules_variable_add', ContextConfig::create() - ->setValue('type', 'integer') - ->map('value', 'text') - ->toArray() - ); - $rule->addExpressionObject($action); - - $violation_list = RulesComponent::create($rule) - ->addContextDefinition('text', ContextDefinition::create('string')) - ->checkIntegrity(); - $this->assertEquals(1, iterator_count($violation_list)); - } - /** * Tests that a primitive context is assigned something that matches. */ @@ -378,4 +359,42 @@ public function testUsingProvidedVariables() { $this->assertEquals(0, iterator_count($violation_list)); } + /** + * Tests that refined context is respected when checking context. + */ + public function testRefinedContextViolation() { + $rule = $this->rulesExpressionManager->createRule(); + $rule->addAction('rules_variable_add', ContextConfig::create() + ->setValue('type', 'integer') + ->map('value', 'text') + ); + + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('text', ContextDefinition::create('string')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + } + + /** + * Tests using provided variables with refined context. + */ + public function testUsingRefinedProvidedVariables() { + $rule = $this->rulesExpressionManager->createRule(); + + $rule->addAction('rules_variable_add', ContextConfig::create() + ->setValue('type', 'string') + ->setValue('value', 'foo') + ); + $rule->addAction('rules_system_message', ContextConfig::create() + ->map('message', 'variable_added') + ->setValue('type', 'status') + ); + // The message action requires a string, thus if the context is not refined + // it will end up as "any" and integrity check would fail. + + $violation_list = RulesComponent::create($rule) + ->checkIntegrity(); + $this->assertEquals(0, iterator_count($violation_list)); + } + } From ef0aa79e7830099ac165b4328f93563b00bae681 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 17:07:22 +0100 Subject: [PATCH 35/57] Re-factored to move context-related integrity checks below Context namespace. --- .../ContextIntegrityCheckTrait.php} | 14 +++++++++----- src/Plugin/RulesExpression/RulesAction.php | 6 +++--- src/Plugin/RulesExpression/RulesCondition.php | 6 +++--- 3 files changed, 15 insertions(+), 11 deletions(-) rename src/{Engine/IntegrityCheckTrait.php => Context/ContextIntegrityCheckTrait.php} (94%) diff --git a/src/Engine/IntegrityCheckTrait.php b/src/Context/ContextIntegrityCheckTrait.php similarity index 94% rename from src/Engine/IntegrityCheckTrait.php rename to src/Context/ContextIntegrityCheckTrait.php index e5385973..d8d69d35 100644 --- a/src/Engine/IntegrityCheckTrait.php +++ b/src/Context/ContextIntegrityCheckTrait.php @@ -2,10 +2,10 @@ /** * @file - * Contains \Drupal\rules\Engine\IntegrityCheckTrait. + * Contains \Drupal\rules\Engine\ContextIntegrityCheckTrait. */ -namespace Drupal\rules\Engine; +namespace Drupal\rules\Context; use Drupal\Core\Plugin\Context\ContextDefinitionInterface; use Drupal\Core\Plugin\ContextAwarePluginInterface as CoreContextAwarePluginInterface; @@ -14,13 +14,17 @@ use Drupal\Core\TypedData\ListInterface; use Drupal\Core\TypedData\PrimitiveInterface; use Drupal\rules\Context\ContextDefinitionInterface as RulesContextDefinitionInterface; -use Drupal\rules\Context\ContextProviderInterface; +use Drupal\rules\Engine\ExecutionMetadataStateInterface; +use Drupal\rules\Engine\IntegrityViolation; +use Drupal\rules\Engine\IntegrityViolationList; use Drupal\rules\Exception\RulesIntegrityException; /** * Provides shared integrity checking methods for conditions and actions. */ -trait IntegrityCheckTrait { +trait ContextIntegrityCheckTrait { + + use ContextHandlerTrait; /** * Performs the integrity check. @@ -34,7 +38,7 @@ trait IntegrityCheckTrait { * @return \Drupal\rules\Engine\IntegrityViolationList * The list of integrity violations. */ - protected function doCheckIntegrity(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { + protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { $violation_list = new IntegrityViolationList(); $context_definitions = $plugin->getContextDefinitions(); diff --git a/src/Plugin/RulesExpression/RulesAction.php b/src/Plugin/RulesExpression/RulesAction.php index 07a31707..cdafc9e3 100644 --- a/src/Plugin/RulesExpression/RulesAction.php +++ b/src/Plugin/RulesExpression/RulesAction.php @@ -16,7 +16,7 @@ use Drupal\rules\Engine\ExecutionStateInterface; use Drupal\rules\Engine\ExpressionBase; use Drupal\rules\Engine\ExpressionInterface; -use Drupal\rules\Engine\IntegrityCheckTrait; +use Drupal\rules\Context\ContextIntegrityCheckTrait; use Drupal\rules\Engine\IntegrityViolationList; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -35,7 +35,7 @@ class RulesAction extends ExpressionBase implements ContainerFactoryPluginInterface, ActionExpressionInterface { use ContextHandlerTrait; - use IntegrityCheckTrait; + use \Drupal\rules\Context\ContextIntegrityCheckTrait; /** * The action manager used to instantiate the action plugin. @@ -158,7 +158,7 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) $action = $this->actionManager->createInstance($this->configuration['action_id']); - return $this->doCheckIntegrity($action, $metadata_state); + return $this->checkContextConfigIntegrity($action, $metadata_state); } /** diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index 4d39e971..8d31dd22 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -16,7 +16,7 @@ use Drupal\rules\Engine\ExecutionStateInterface; use Drupal\rules\Engine\ExpressionBase; use Drupal\rules\Engine\ExpressionInterface; -use Drupal\rules\Engine\IntegrityCheckTrait; +use Drupal\rules\Context\ContextIntegrityCheckTrait; use Drupal\rules\Engine\IntegrityViolationList; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -35,7 +35,7 @@ class RulesCondition extends ExpressionBase implements ConditionExpressionInterface, ContainerFactoryPluginInterface { use ContextHandlerTrait; - use IntegrityCheckTrait; + use \Drupal\rules\Context\ContextIntegrityCheckTrait; /** * The condition manager used to instantiate the condition plugin. @@ -183,7 +183,7 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) 'negate' => $this->configuration['negate'], ]); - return $this->doCheckIntegrity($condition, $metadata_state); + return $this->checkContextConfigIntegrity($condition, $metadata_state); } /** From 0632db54c98d6ae408d2d960ea69a19a59949f25 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 18:20:20 +0100 Subject: [PATCH 36/57] Issue #2677034 by fago: Simplify preparation of metadata state logic and prepare its usage for integrity checks. --- ...t.php => ContextHandlerIntegrityTrait.php} | 48 +++---------- src/Context/ContextHandlerTrait.php | 67 +++++++++++++++++-- src/Context/ContextProviderTrait.php | 3 + src/Engine/ActionExpressionContainer.php | 31 ++++----- src/Engine/ConditionExpressionContainer.php | 33 +++++---- .../ExecutionMetadataStateInterface.php | 3 +- src/Engine/ExpressionInterface.php | 32 +++++---- src/Plugin/RulesExpression/Rule.php | 13 ++-- src/Plugin/RulesExpression/RulesAction.php | 17 ++--- src/Plugin/RulesExpression/RulesCondition.php | 17 ++--- src/Plugin/RulesExpression/RulesLoop.php | 21 ++---- .../PrepareExecutionMetadataStateTest.php | 4 +- 12 files changed, 149 insertions(+), 140 deletions(-) rename src/Context/{ContextIntegrityCheckTrait.php => ContextHandlerIntegrityTrait.php} (80%) diff --git a/src/Context/ContextIntegrityCheckTrait.php b/src/Context/ContextHandlerIntegrityTrait.php similarity index 80% rename from src/Context/ContextIntegrityCheckTrait.php rename to src/Context/ContextHandlerIntegrityTrait.php index d8d69d35..6a2e4bb8 100644 --- a/src/Context/ContextIntegrityCheckTrait.php +++ b/src/Context/ContextHandlerIntegrityTrait.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\rules\Engine\ContextIntegrityCheckTrait. + * Contains \Drupal\rules\Engine\ContextHandlerIntegrityTrait. */ namespace Drupal\rules\Context; @@ -20,9 +20,9 @@ use Drupal\rules\Exception\RulesIntegrityException; /** - * Provides shared integrity checking methods for conditions and actions. + * Extends the context handler trait with support for checking integrity. */ -trait ContextIntegrityCheckTrait { +trait ContextHandlerIntegrityTrait { use ContextHandlerTrait; @@ -42,12 +42,18 @@ protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $ $violation_list = new IntegrityViolationList(); $context_definitions = $plugin->getContextDefinitions(); + // @todo: First step, ensure context is refined and metadata is prepared. + + + // Make sure that all provided variables by this plugin are added to the + // execution metadata state. + $this->addProvidedContextDefinitions($plugin, $metadata_state); + foreach ($context_definitions as $name => $context_definition) { // Check if a data selector is configured that maps to the state. if (isset($this->configuration['context_mapping'][$name])) { try { - $data_definition = $metadata_state->fetchDefinitionByPropertyPath($this->configuration['context_mapping'][$name]); - + $data_definition = $this->getMappedDefinition($name, $metadata_state); $this->checkDataTypeCompatible($context_definition, $data_definition, $name, $violation_list); } catch (RulesIntegrityException $e) { @@ -116,10 +122,6 @@ protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $ } } - // Make sure that all provided variables by this plugin are added to the - // execution metadata state. - $this->addProvidedVariablesToExecutionMetadataState($plugin, $metadata_state); - return $violation_list; } @@ -169,33 +171,5 @@ protected function checkDataTypeCompatible(ContextDefinitionInterface $context_d } } - /** - * Adds provided variables to the execution metadata state. - * - * @param CoreContextAwarePluginInterface $plugin - * The action or condition plugin that may provide variables. - * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state - * The execution metadata state to add variables to. - */ - public function addProvidedVariablesToExecutionMetadataState(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { - if ($plugin instanceof ContextProviderInterface) { - $provided_context_definitions = $plugin->getProvidedContextDefinitions(); - - foreach ($provided_context_definitions as $name => $context_definition) { - if (isset($this->configuration['provides_mapping'][$name])) { - // Populate the state with the new variable that is provided by this - // plugin. That is necessary so that the integrity check in subsequent - // actions knows about the variable and does not throw violations. - $metadata_state->setDataDefinition( - $this->configuration['provides_mapping'][$name], - $context_definition->getDataDefinition() - ); - } - else { - $metadata_state->setDataDefinition($name, $context_definition->getDataDefinition()); - } - } - } - } } diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index d9d77664..01ab072e 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -9,6 +9,7 @@ use Drupal\Core\Plugin\Context\Context; use Drupal\Core\Plugin\ContextAwarePluginInterface as CoreContextAwarePluginInterface; +use Drupal\rules\Engine\ExecutionMetadataStateInterface; use Drupal\rules\Engine\ExecutionStateInterface; use Drupal\rules\Exception\RulesEvaluationException; @@ -30,7 +31,7 @@ trait ContextHandlerTrait { protected $processorManager; /** - * Maps variables from rules state into the plugin context. + * Maps variables from the execution state into the plugin context. * * @param \Drupal\Core\Plugin\ContextAwarePluginInterface $plugin * The plugin that is populated with context values. @@ -77,14 +78,40 @@ protected function mapContext(CoreContextAwarePluginInterface $plugin, Execution } /** - * Maps provided context values from the plugin to the Rules state. + * Gets the definition of the data that is mapped to the given context. * - * @param ContextProviderInterface $plugin - * The plugin where the context values are extracted. + * @param string $context_name + * The name of the context. + * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state + * The metadata state containing metadata about available variables. + * + * @return \Drupal\Core\TypedData\DataDefinitionInterface|null + * A data definition if the property path could be applied, or NULL if the + * context is not mapped. + * + * @throws \Drupal\rules\Exception\RulesIntegrityException + * Thrown if the data selector that is configured for the context is + * invalid. + */ + protected function getMappedDefinition($context_name, ExecutionMetadataStateInterface $metadata_state) { + if (isset($this->configuration['context_mapping'][$context_name])) { + return $metadata_state->fetchDefinitionByPropertyPath($this->configuration['context_mapping'][$context_name]); + } + } + + /** + * Adds provided context values from the plugin to the execution state. + * + * @param CoreContextAwarePluginInterface $plugin + * The context aware plugin of which to add provided context. * @param \Drupal\rules\Engine\ExecutionStateInterface $state * The Rules state where the context variables are added. */ - protected function mapProvidedContext(ContextProviderInterface $plugin, ExecutionStateInterface $state) { + protected function addProvidedContext(CoreContextAwarePluginInterface $plugin, ExecutionStateInterface $state) { + // If the plugin does not support providing context, there is nothing to do. + if (!$plugin instanceof ContextProviderInterface) { + return; + } $provides = $plugin->getProvidedContextDefinitions(); foreach ($provides as $name => $provided_definition) { // Avoid name collisions in the rules state: provided variables can be @@ -98,6 +125,36 @@ protected function mapProvidedContext(ContextProviderInterface $plugin, Executio } } + /** + * Adds the definitions of provided context to the execution metadata state. + * + * @param CoreContextAwarePluginInterface $plugin + * The context aware plugin of which to add provided context. + * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state + * The execution metadata state to add variables to. + */ + protected function addProvidedContextDefinitions(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { + // If the plugin does not support providing context, there is nothing to do. + if (!$plugin instanceof ContextProviderInterface) { + return; + } + + foreach ($plugin->getProvidedContextDefinitions() as $name => $context_definition) { + if (isset($this->configuration['provides_mapping'][$name])) { + // Populate the state with the new variable that is provided by this + // plugin. That is necessary so that the integrity check in subsequent + // actions knows about the variable and does not throw violations. + $metadata_state->setDataDefinition( + $this->configuration['provides_mapping'][$name], + $context_definition->getDataDefinition() + ); + } + else { + $metadata_state->setDataDefinition($name, $context_definition->getDataDefinition()); + } + } + } + /** * Process data context on the plugin, usually before it gets executed. * diff --git a/src/Context/ContextProviderTrait.php b/src/Context/ContextProviderTrait.php index e0cd0847..7f4a6974 100644 --- a/src/Context/ContextProviderTrait.php +++ b/src/Context/ContextProviderTrait.php @@ -13,6 +13,9 @@ /** * A trait implementing the ContextProviderInterface. * + * This trait is intended for context aware plugins that want to provide + * context. + * * The trait requires the plugin to use configuration as defined by the * ContextConfig class. * diff --git a/src/Engine/ActionExpressionContainer.php b/src/Engine/ActionExpressionContainer.php index c2677d90..7e2f8233 100644 --- a/src/Engine/ActionExpressionContainer.php +++ b/src/Engine/ActionExpressionContainer.php @@ -24,6 +24,13 @@ abstract class ActionExpressionContainer extends ExpressionBase implements Actio */ protected $actions = []; + /** + * The expression manager. + * + * @var \Drupal\rules\Engine\ExpressionManagerInterface + */ + protected $expressionManager; + /** * Constructor. * @@ -176,26 +183,16 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) * {@inheritdoc} */ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - if ($until) { - if ($this->getUuid() === $until->getUuid()) { - return TRUE; - } - foreach ($this->actions as $action) { - if ($action->getUuid() === $until->getUuid()) { - return TRUE; - } - $found = $action->prepareExecutionMetadataState($metadata_state, $until); - if ($found) { - return TRUE; - } - } - return FALSE; + if ($until && $this->getUuid() === $until->getUuid()) { + return TRUE; } - foreach ($this->actions as $action) { - $action->prepareExecutionMetadataState($metadata_state); + $found = $action->prepareExecutionMetadataState($metadata_state, $until); + // If the expression was found, we need to stop. + if ($found) { + return TRUE; + } } - return TRUE; } } diff --git a/src/Engine/ConditionExpressionContainer.php b/src/Engine/ConditionExpressionContainer.php index 10452187..7e1dc8af 100644 --- a/src/Engine/ConditionExpressionContainer.php +++ b/src/Engine/ConditionExpressionContainer.php @@ -20,10 +20,17 @@ abstract class ConditionExpressionContainer extends ExpressionBase implements Co /** * List of conditions that are evaluated. * - * @var \Drupal\rules\Core\RulesConditionInterface[] + * @var \Drupal\rules\Engine\ConditionExpressionInterface[] */ protected $conditions = []; + /** + * The expression manager. + * + * @var \Drupal\rules\Engine\ExpressionManagerInterface + */ + protected $expressionManager; + /** * Constructs a new class instance. * @@ -204,26 +211,16 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) * {@inheritdoc} */ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - if ($until) { - if ($this->getUuid() === $until->getUuid()) { - return TRUE; - } - foreach ($this->conditions as $condition) { - if ($condition->getUuid() === $until->getUuid()) { - return TRUE; - } - $found = $condition->prepareExecutionMetadataState($metadata_state, $until); - if ($found) { - return TRUE; - } - } - return FALSE; + if ($until && $this->getUuid() === $until->getUuid()) { + return TRUE; } - foreach ($this->conditions as $condition) { - $condition->prepareExecutionMetadataState($metadata_state); + $found = $condition->prepareExecutionMetadataState($metadata_state, $until); + // If the expression was found, we need to stop. + if ($found) { + return TRUE; + } } - return TRUE; } } diff --git a/src/Engine/ExecutionMetadataStateInterface.php b/src/Engine/ExecutionMetadataStateInterface.php index 4215393a..fdf637f3 100644 --- a/src/Engine/ExecutionMetadataStateInterface.php +++ b/src/Engine/ExecutionMetadataStateInterface.php @@ -79,12 +79,13 @@ public function removeDataDefinition($name); * @param string $property_path * The property path, example: "node:title:value". * @param string $langcode - * The langauge code. + * The language code. * * @return \Drupal\Core\TypedData\DataDefinitionInterface * A data definition if the property path could be applied. * * @throws \Drupal\rules\Exception\RulesIntegrityException + * Thrown if the property path is invalid. */ public function fetchDefinitionByPropertyPath($property_path, $langcode = LanguageInterface::LANGCODE_NOT_SPECIFIED); diff --git a/src/Engine/ExpressionInterface.php b/src/Engine/ExpressionInterface.php index a910972d..71616c28 100644 --- a/src/Engine/ExpressionInterface.php +++ b/src/Engine/ExpressionInterface.php @@ -99,28 +99,32 @@ public function getUuid(); public function setUuid($uuid); /** - * Prepares the execution metadata state by adding variables to it. + * Prepares the execution metadata state by adding metadata to it. * * If this expression contains other expressions then the metadata state is * set up recursively. If a $until expression is specified then the setup will - * stop right before that expression. This is useful for inspecting the state - * at a certain point in the expression tree, for example to do autocompletion - * of available variables in the state. - * - * The difference to fully preparing the state is that not all variables are - * available in the middle of the expression tree. Preparing with + * stop right before that expression to calculate the state at this execution + * point. + * This is useful for inspecting the state at a certain point in the + * expression tree as needed during configuration, for example to do + * autocompletion of available variables in the state. + * + * The difference to fully preparing the state is that not necessarily all + * variables are available in the middle of the expression tree, as for + * example variables being added later are not added yet. Preparing with * $until = NULL reflects the execution metadata state at the end of the - * expression. + * expression execution. * * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state - * The execution metadata state to populate variables in. + * The execution metadata state. * @param \Drupal\rules\Engine\ExpressionInterface $until - * (optional) A nested expression if this expression is a container. - * Preparation of the sate will happen right before that expression. + * (optional) The expression at which metadata preparation should be + * stopped. The preparation of the state will be stopped right before that + * expression. * - * @return bool - * TRUE if $until is NULL or the nested expression was found in the tree, - * FALSE otherwise. + * @return true|null + * True if the metadata has been prepared and the $until expression was + * found in the tree. Null otherwise. */ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL); diff --git a/src/Plugin/RulesExpression/Rule.php b/src/Plugin/RulesExpression/Rule.php index b5a5b913..51f1db5b 100644 --- a/src/Plugin/RulesExpression/Rule.php +++ b/src/Plugin/RulesExpression/Rule.php @@ -225,16 +225,11 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) * {@inheritdoc} */ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - if ($until) { - $found = $this->conditions->prepareExecutionMetadataState($metadata_state, $until); - if (!$found) { - $found = $this->actions->prepareExecutionMetadataState($metadata_state, $until); - } - return $found; + $found = $this->conditions->prepareExecutionMetadataState($metadata_state, $until); + if ($found) { + return TRUE; } - $this->conditions->prepareExecutionMetadataState($metadata_state); - $this->actions->prepareExecutionMetadataState($metadata_state); - return TRUE; + return $this->actions->prepareExecutionMetadataState($metadata_state, $until); } /** diff --git a/src/Plugin/RulesExpression/RulesAction.php b/src/Plugin/RulesExpression/RulesAction.php index cdafc9e3..99d76bf1 100644 --- a/src/Plugin/RulesExpression/RulesAction.php +++ b/src/Plugin/RulesExpression/RulesAction.php @@ -8,7 +8,6 @@ namespace Drupal\rules\Plugin\RulesExpression; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; -use Drupal\rules\Context\ContextHandlerTrait; use Drupal\rules\Context\DataProcessorManager; use Drupal\rules\Core\RulesActionManagerInterface; use Drupal\rules\Engine\ActionExpressionInterface; @@ -16,7 +15,7 @@ use Drupal\rules\Engine\ExecutionStateInterface; use Drupal\rules\Engine\ExpressionBase; use Drupal\rules\Engine\ExpressionInterface; -use Drupal\rules\Context\ContextIntegrityCheckTrait; +use Drupal\rules\Context\ContextHandlerIntegrityTrait; use Drupal\rules\Engine\IntegrityViolationList; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -34,8 +33,7 @@ */ class RulesAction extends ExpressionBase implements ContainerFactoryPluginInterface, ActionExpressionInterface { - use ContextHandlerTrait; - use \Drupal\rules\Context\ContextIntegrityCheckTrait; + use ContextHandlerIntegrityTrait; /** * The action manager used to instantiate the action plugin. @@ -116,7 +114,7 @@ public function executeWithState(ExecutionStateInterface $state) { // Now that the action has been executed it can provide additional // context which we will have to pass back in the evaluation state. - $this->mapProvidedContext($action, $state); + $this->addProvidedContext($action, $state); } /** @@ -165,12 +163,11 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) * {@inheritdoc} */ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - $action = $this->actionManager->createInstance($this->configuration['action_id']); - $this->addProvidedVariablesToExecutionMetadataState($action, $metadata_state); - if ($until) { - return FALSE; + if ($until && $this->getUuid() === $until->getUuid()) { + return TRUE; } - return TRUE; + $action = $this->actionManager->createInstance($this->configuration['action_id']); + $this->addProvidedContextDefinitions($action, $metadata_state); } } diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index 8d31dd22..082a4a86 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -8,7 +8,6 @@ namespace Drupal\rules\Plugin\RulesExpression; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; -use Drupal\rules\Context\ContextHandlerTrait; use Drupal\rules\Context\DataProcessorManager; use Drupal\rules\Core\ConditionManager; use Drupal\rules\Engine\ConditionExpressionInterface; @@ -16,7 +15,7 @@ use Drupal\rules\Engine\ExecutionStateInterface; use Drupal\rules\Engine\ExpressionBase; use Drupal\rules\Engine\ExpressionInterface; -use Drupal\rules\Context\ContextIntegrityCheckTrait; +use Drupal\rules\Context\ContextHandlerIntegrityTrait; use Drupal\rules\Engine\IntegrityViolationList; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -34,8 +33,7 @@ */ class RulesCondition extends ExpressionBase implements ConditionExpressionInterface, ContainerFactoryPluginInterface { - use ContextHandlerTrait; - use \Drupal\rules\Context\ContextIntegrityCheckTrait; + use ContextHandlerIntegrityTrait; /** * The condition manager used to instantiate the condition plugin. @@ -126,7 +124,7 @@ public function executeWithState(ExecutionStateInterface $state) { // Now that the condition has been executed it can provide additional // context which we will have to pass back in the evaluation state. - $this->mapProvidedContext($condition, $state); + $this->addProvidedContext($condition, $state); if ($this->isNegated()) { $result = !$result; @@ -190,12 +188,11 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) * {@inheritdoc} */ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - $condition = $this->actionManager->createInstance($this->configuration['condition_id']); - $this->addProvidedVariablesToExecutionMetadataState($condition, $metadata_state); - if ($until) { - return FALSE; + if ($until && $this->getUuid() === $until->getUuid()) { + return TRUE; } - return TRUE; + $condition = $this->conditionManager->createInstance($this->configuration['condition_id']); + $this->addProvidedContextDefinitions($condition, $metadata_state); } } diff --git a/src/Plugin/RulesExpression/RulesLoop.php b/src/Plugin/RulesExpression/RulesLoop.php index 6e1efe96..fde0eb18 100644 --- a/src/Plugin/RulesExpression/RulesLoop.php +++ b/src/Plugin/RulesExpression/RulesLoop.php @@ -111,27 +111,14 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m // item definition to the state. } - if ($until) { - foreach ($this->actions as $action) { - if ($action->getUuid() === $until->getUuid()) { - return TRUE; - } - $found = $action->prepareExecutionMetadataState($metadata_state, $until); - if ($found) { - return TRUE; - } - } - // Remove the list item variable after the loop, it is out of scope now. - $metadata_state->removeDataDefinition($list_item_name); - return FALSE; - } - foreach ($this->actions as $action) { - $action->prepareExecutionMetadataState($metadata_state); + $found = $action->prepareExecutionMetadataState($metadata_state, $until); + if ($found) { + return TRUE; + } } // Remove the list item variable after the loop, it is out of scope now. $metadata_state->removeDataDefinition($list_item_name); - return TRUE; } } diff --git a/tests/src/Integration/Engine/PrepareExecutionMetadataStateTest.php b/tests/src/Integration/Engine/PrepareExecutionMetadataStateTest.php index a415c67d..de34d5bb 100644 --- a/tests/src/Integration/Engine/PrepareExecutionMetadataStateTest.php +++ b/tests/src/Integration/Engine/PrepareExecutionMetadataStateTest.php @@ -34,7 +34,7 @@ public function testAddingVariable() { $state = ExecutionMetadataState::create(); $found = $rule->prepareExecutionMetadataState($state); $this->assertTrue($state->hasDataDefinition('result')); - $this->assertTrue($found); + $this->assertNull($found); } /** @@ -113,7 +113,7 @@ public function testPrepareAfterLoop() { $found = $rule->prepareExecutionMetadataState($state); $this->assertFalse($state->hasDataDefinition('list_item')); - $this->assertTrue($found); + $this->assertNull($found); } } From 3c17965a51208a0b1ef8a2eebb68a1c712314720 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 20:36:46 +0100 Subject: [PATCH 37/57] Issue #2677034 by fago: Make checking integrity work with preparing metadata and fix refined context to be respected. --- src/Context/ContextHandlerIntegrityTrait.php | 7 +- src/Context/ContextHandlerTrait.php | 86 ++++++++++++------- src/Core/RulesActionBase.php | 15 ++++ src/Core/RulesConditionBase.php | 14 +++ src/Engine/ActionExpressionContainer.php | 24 ++++++ src/Engine/ConditionExpressionContainer.php | 23 +++++ src/Engine/ExpressionInterface.php | 3 +- src/Plugin/RulesAction/EntityFetchById.php | 3 +- src/Plugin/RulesExpression/RulesAction.php | 21 +++-- src/Plugin/RulesExpression/RulesCondition.php | 26 +++--- src/Plugin/RulesExpression/RulesLoop.php | 56 ++++++------ 11 files changed, 188 insertions(+), 90 deletions(-) diff --git a/src/Context/ContextHandlerIntegrityTrait.php b/src/Context/ContextHandlerIntegrityTrait.php index 6a2e4bb8..c504c769 100644 --- a/src/Context/ContextHandlerIntegrityTrait.php +++ b/src/Context/ContextHandlerIntegrityTrait.php @@ -7,7 +7,7 @@ namespace Drupal\rules\Context; -use Drupal\Core\Plugin\Context\ContextDefinitionInterface; +use Drupal\Core\Plugin\Context\ContextDefinitionInterface as CoreContextDefinitionInterface; use Drupal\Core\Plugin\ContextAwarePluginInterface as CoreContextAwarePluginInterface; use Drupal\Core\TypedData\ComplexDataInterface; use Drupal\Core\TypedData\DataDefinitionInterface; @@ -42,9 +42,6 @@ protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $ $violation_list = new IntegrityViolationList(); $context_definitions = $plugin->getContextDefinitions(); - // @todo: First step, ensure context is refined and metadata is prepared. - - // Make sure that all provided variables by this plugin are added to the // execution metadata state. $this->addProvidedContextDefinitions($plugin, $metadata_state); @@ -137,7 +134,7 @@ protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $ * @param \Drupal\rules\Engine\IntegrityViolationList $violation_list * The list of violations where new ones will be added. */ - protected function checkDataTypeCompatible(ContextDefinitionInterface $context_definition, DataDefinitionInterface $provided, $context_name, IntegrityViolationList $violation_list) { + protected function checkDataTypeCompatible(CoreContextDefinitionInterface $context_definition, DataDefinitionInterface $provided, $context_name, IntegrityViolationList $violation_list) { $expected_class = $context_definition->getDataDefinition()->getClass(); $provided_class = $provided->getClass(); $expected_type_problem = NULL; diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index 01ab072e..4cf02b6d 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -7,7 +7,7 @@ namespace Drupal\rules\Context; -use Drupal\Core\Plugin\Context\Context; +use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Core\Plugin\ContextAwarePluginInterface as CoreContextAwarePluginInterface; use Drupal\rules\Engine\ExecutionMetadataStateInterface; use Drupal\rules\Engine\ExecutionStateInterface; @@ -31,7 +31,18 @@ trait ContextHandlerTrait { protected $processorManager; /** - * Maps variables from the execution state into the plugin context. + * Prepares plugin context based upon the set context configuration. + * + * If no execution state is given, the configuration is applied as far as + * possible. That means, the configured context values are set and context is + * refined. + * If an execution state is available, the plugin is prepared for execution + * by mapping the variables from the execution state into the plugin context + * and applying data processors. + * In addition, it is ensured that all required context is basically + * available as defined. This include the following checks: + * - Required context must have a value set. + * - Context may not have NULL values unless the plugin allows it. * * @param \Drupal\Core\Plugin\ContextAwarePluginInterface $plugin * The plugin that is populated with context values. @@ -39,40 +50,55 @@ trait ContextHandlerTrait { * The Rules state containing available variables. * * @throws \Drupal\rules\Exception\RulesEvaluationException - * In case a required context is missing for the plugin. + * Thrown if an execution state is given, but some context is not satisfied; + * e.g. a required context is missing. */ - protected function mapContext(CoreContextAwarePluginInterface $plugin, ExecutionStateInterface $state) { - $context_definitions = $plugin->getContextDefinitions(); - foreach ($context_definitions as $name => $definition) { - // Check if a data selector is configured that maps to the state. - if (isset($this->configuration['context_mapping'][$name])) { - $typed_data = $state->fetchDataByPropertyPath($this->configuration['context_mapping'][$name]); + protected function prepareContext(CoreContextAwarePluginInterface $plugin, ExecutionStateInterface $state = NULL) { + if (isset($this->configuration['context_values'])) { + foreach ($this->configuration['context_values'] as $name => $value) { + $plugin->setContextValue($name, $value); + } + } + if ($plugin instanceof ContextAwarePluginInterface) { + // Getting context values may lead to undocumented exceptions if context + // is not set right now. So catch those exceptions. + // @todo: Remove ones https://www.drupal.org/node/2677162 got fixed. + try { + $plugin->refineContextDefinitions(); + } + catch (ContextException $e) { + } + } - if ($typed_data->getValue() === NULL && !$definition->isAllowedNull()) { - throw new RulesEvaluationException('The value of data selector ' - . $this->configuration['context_mapping'][$name] . " is NULL, but the context $name in " - . $plugin->getPluginId() . ' requires a value.'); - } - $context = $plugin->getContext($name); - $new_context = Context::createFromContext($context, $typed_data); - $plugin->setContext($name, $new_context); + // If no execution state has been provided, we are done now. + if (!$state) { + return; + } + + // Map context by apply data selectors. + if (isset($this->configuration['context_mapping'])) { + foreach ($this->configuration['context_mapping'] as $name => $selector) { + $typed_data = $state->fetchDataByPropertyPath($selector); + $plugin->setContextValue($name, $typed_data); } - elseif (isset($this->configuration['context_values']) - && array_key_exists($name, $this->configuration['context_values']) - ) { + } + // Apply data processors. + $this->processData($plugin, $state); - if ($this->configuration['context_values'][$name] === NULL && !$definition->isAllowedNull()) { - throw new RulesEvaluationException("The context value for $name is NULL, but the context $name in " + // Finally, ensure all contexts are set as expected now. + foreach ($plugin->getContextDefinitions() as $name => $definition) { + if ($plugin->getContextValue($name) === NULL && $definition->isRequired()) { + // If a context mapping has been specified, the value might end up NULL + // but valid (e.g. a reference on an empty property). In that case + // isAllowedNull determines whether the context is conform. + if (!isset($this->configuration['context_mapping'][$name])) { + throw new RulesEvaluationException("Required context $name is missing for plugin " + . $plugin->getPluginId() . '.'); + } + elseif (!$definition->isAllowedNull()) { + throw new RulesEvaluationException("The context for $name is NULL, but the context $name in " . $plugin->getPluginId() . ' requires a value.'); } - - $context = $plugin->getContext($name); - $new_context = Context::createFromContext($context, $this->configuration['context_values'][$name]); - $plugin->setContext($name, $new_context); - } - elseif ($definition->isRequired()) { - throw new RulesEvaluationException("Required context $name is missing for plugin " - . $plugin->getPluginId() . '.'); } } } diff --git a/src/Core/RulesActionBase.php b/src/Core/RulesActionBase.php index 1559c58f..1502b4b5 100644 --- a/src/Core/RulesActionBase.php +++ b/src/Core/RulesActionBase.php @@ -7,6 +7,7 @@ namespace Drupal\rules\Core; +use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Core\Access\AccessResult; use Drupal\Core\Plugin\ContextAwarePluginBase; use Drupal\Core\Session\AccountInterface; @@ -28,6 +29,20 @@ abstract class RulesActionBase extends ContextAwarePluginBase implements RulesAc */ protected $configuration; + /** + * {@inheritdoc} + */ + public function getContextValue($name) { + try { + return parent::getContextValue($name); + } + catch (ContextException $e) { + // Catch the undocumented exception thrown when no context value is set + // for a required context. + // @todo: Remove once https://www.drupal.org/node/2677162 is fixed. + } + } + /** * {@inheritdoc} */ diff --git a/src/Core/RulesConditionBase.php b/src/Core/RulesConditionBase.php index a7b5e3e8..e594099b 100644 --- a/src/Core/RulesConditionBase.php +++ b/src/Core/RulesConditionBase.php @@ -29,6 +29,20 @@ public function refineContextDefinitions() { // Do not refine anything by default. } + /** + * {@inheritdoc} + */ + public function getContextValue($name) { + try { + return parent::getContextValue($name); + } + catch (ContextException $e) { + // Catch the undocumented exception thrown when no context value is set + // for a required context. + // @todo: Remove once https://www.drupal.org/node/2677162 is fixed. + } + } + /** * {@inheritdoc} */ diff --git a/src/Engine/ActionExpressionContainer.php b/src/Engine/ActionExpressionContainer.php index 7e2f8233..afe903fb 100644 --- a/src/Engine/ActionExpressionContainer.php +++ b/src/Engine/ActionExpressionContainer.php @@ -172,10 +172,12 @@ public function deleteExpression($uuid) { */ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { $violation_list = new IntegrityViolationList(); + $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); foreach ($this->actions as $action) { $action_violations = $action->checkIntegrity($metadata_state); $violation_list->addAll($action_violations); } + $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); return $violation_list; } @@ -186,6 +188,7 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m if ($until && $this->getUuid() === $until->getUuid()) { return TRUE; } + $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); foreach ($this->actions as $action) { $found = $action->prepareExecutionMetadataState($metadata_state, $until); // If the expression was found, we need to stop. @@ -193,6 +196,27 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m return TRUE; } } + $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); + } + + /** + * Prepares execution metadata state before traversing through children. + * + * @see ::prepareExecutionMetadataState() + * @see ::checkIntegrity() + */ + protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) { + // Any pre-traversal preparations need to be added here. + } + + /** + * Prepares execution metadata state after traversing through children. + * + * @see ::prepareExecutionMetadataState() + * @see ::checkIntegrity() + */ + protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { + // Any post-traversal preparations need to be added here. } } diff --git a/src/Engine/ConditionExpressionContainer.php b/src/Engine/ConditionExpressionContainer.php index 7e1dc8af..322bdfa4 100644 --- a/src/Engine/ConditionExpressionContainer.php +++ b/src/Engine/ConditionExpressionContainer.php @@ -200,10 +200,12 @@ public function deleteExpression($uuid) { */ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { $violation_list = new IntegrityViolationList(); + $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); foreach ($this->conditions as $condition) { $condition_violations = $condition->checkIntegrity($metadata_state); $violation_list->addAll($condition_violations); } + $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); return $violation_list; } @@ -214,6 +216,7 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m if ($until && $this->getUuid() === $until->getUuid()) { return TRUE; } + $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); foreach ($this->conditions as $condition) { $found = $condition->prepareExecutionMetadataState($metadata_state, $until); // If the expression was found, we need to stop. @@ -221,6 +224,26 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m return TRUE; } } + $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); } + /** + * Prepares execution metadata state before traversing through children. + * + * @see ::prepareExecutionMetadataState() + * @see ::checkIntegrity() + */ + protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) { + // Any pre-traversal preparations need to be added here. + } + + /** + * Prepares execution metadata state after traversing through children. + * + * @see ::prepareExecutionMetadataState() + * @see ::checkIntegrity() + */ + protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { + // Any post-traversal preparations need to be added here. + } } diff --git a/src/Engine/ExpressionInterface.php b/src/Engine/ExpressionInterface.php index 71616c28..1ed1bcc0 100644 --- a/src/Engine/ExpressionInterface.php +++ b/src/Engine/ExpressionInterface.php @@ -116,7 +116,8 @@ public function setUuid($uuid); * expression execution. * * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state - * The execution metadata state. + * The execution metadata state, prepared until right before this + * expression. * @param \Drupal\rules\Engine\ExpressionInterface $until * (optional) The expression at which metadata preparation should be * stopped. The preparation of the state will be stopped right before that diff --git a/src/Plugin/RulesAction/EntityFetchById.php b/src/Plugin/RulesAction/EntityFetchById.php index 9815ad5a..0f196d05 100644 --- a/src/Plugin/RulesAction/EntityFetchById.php +++ b/src/Plugin/RulesAction/EntityFetchById.php @@ -82,7 +82,8 @@ public static function create(ContainerInterface $container, array $configuratio * {@inheritdoc} */ public function refineContextDefinitions() { - if ($type = $this->getContextValue('type')) { + // @todo: Simplify ones https://www.drupal.org/node/2677162 got fixed. + if ($this->getContext('type')->hasContextValue() && $type = $this->getContextValue('type')) { $this->pluginDefinition['provides']['entity_fetched']->setDataType("entity:$type"); } } diff --git a/src/Plugin/RulesExpression/RulesAction.php b/src/Plugin/RulesExpression/RulesAction.php index 99d76bf1..fc5db0d5 100644 --- a/src/Plugin/RulesExpression/RulesAction.php +++ b/src/Plugin/RulesExpression/RulesAction.php @@ -94,16 +94,7 @@ public function setConfiguration(array $configuration) { public function executeWithState(ExecutionStateInterface $state) { $action = $this->actionManager->createInstance($this->configuration['action_id']); - // We have to forward the context values from our configuration to the - // action plugin. - $this->mapContext($action, $state); - - $action->refineContextDefinitions(); - - // Send the context value through configured data processor before executing - // the action. - $this->processData($action, $state); - + $this->prepareContext($action, $state); $action->execute(); $auto_saves = $action->autoSaveContext(); @@ -156,7 +147,12 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) $action = $this->actionManager->createInstance($this->configuration['action_id']); - return $this->checkContextConfigIntegrity($action, $metadata_state); + // Prepare and refine the context before checking integrity, such that any + // context definition changes are respected while checking. + $this->prepareContext($action); + $result = $this->checkContextConfigIntegrity($action, $metadata_state); + $this->prepareExecutionMetadataState($metadata_state); + return $result; } /** @@ -167,6 +163,9 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m return TRUE; } $action = $this->actionManager->createInstance($this->configuration['action_id']); + // Make sure to refine context first, such that possibly refined definitions + // of provided context are respected. + $this->prepareContext($action); $this->addProvidedContextDefinitions($action, $metadata_state); } diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index 082a4a86..662513aa 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -110,16 +110,7 @@ public function executeWithState(ExecutionStateInterface $state) { 'negate' => $this->configuration['negate'], ]); - // We have to forward the context values from our configuration to the - // condition plugin. - $this->mapContext($condition, $state); - - $condition->refineContextDefinitions(); - - // Send the context values through configured data processors before - // evaluating the condition. - $this->processData($condition, $state); - + $this->prepareContext($condition, $state); $result = $condition->evaluate(); // Now that the condition has been executed it can provide additional @@ -180,8 +171,12 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) $condition = $this->conditionManager->createInstance($this->configuration['condition_id'], [ 'negate' => $this->configuration['negate'], ]); - - return $this->checkContextConfigIntegrity($condition, $metadata_state); + // Prepare and refine the context before checking integrity, such that any + // context definition changes are respected while checking. + $this->prepareContext($condition); + $result = $this->checkContextConfigIntegrity($condition, $metadata_state); + $this->prepareExecutionMetadataState($metadata_state); + return $result; } /** @@ -191,7 +186,12 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m if ($until && $this->getUuid() === $until->getUuid()) { return TRUE; } - $condition = $this->conditionManager->createInstance($this->configuration['condition_id']); + $condition = $this->conditionManager->createInstance($this->configuration['condition_id'], [ + 'negate' => $this->configuration['negate'], + ]); + // Make sure to refine context first, such that possibly refined definitions + // of provided context are respected. + $this->prepareContext($condition); $this->addProvidedContextDefinitions($condition, $metadata_state); } diff --git a/src/Plugin/RulesExpression/RulesLoop.php b/src/Plugin/RulesExpression/RulesLoop.php index fde0eb18..84e299d5 100644 --- a/src/Plugin/RulesExpression/RulesLoop.php +++ b/src/Plugin/RulesExpression/RulesLoop.php @@ -25,14 +25,22 @@ */ class RulesLoop extends ActionExpressionContainer { + /** + * {@inheritdoc} + */ + public function defaultConfiguration() { + return [ + // Default to 'list_item' as variable name for the list item. + 'list_item' => 'list_item', + ]; + } + /** * {@inheritdoc} */ public function executeWithState(ExecutionStateInterface $state) { $list_data = $state->fetchDataByPropertyPath($this->configuration['list']); - // Use a configured list item variable name, otherwise fall back to just - // 'list_item' as variable name. - $list_item_name = isset($this->configuration['list_item']) ? $this->configuration['list_item'] : 'list_item'; + $list_item_name = $this->configuration['list_item']; foreach ($list_data as $item) { $state->setVariableData($list_item_name, $item); @@ -75,50 +83,40 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) return $violation_list; } - if ($list_definition instanceof ListDataDefinitionInterface) { - $list_item_definition = $list_definition->getItemDefinition(); - $metadata_state->setDataDefinition($list_item_name, $list_item_definition); - - $violation_list = parent::checkIntegrity($metadata_state); - - // Remove the list item variable after the loop, it is out of scope now. - $metadata_state->removeDataDefinition($list_item_name); + if (!$list_definition instanceof ListDataDefinitionInterface) { + $violation_list->addViolationWithMessage($this->t('The data type of list variable %list is not a list.', [ + '%list' => $this->configuration['list'], + ])); return $violation_list; } - $violation_list->addViolationWithMessage($this->t('The data type of list variable %list is not a list.', [ - '%list' => $this->configuration['list'], - ])); + // So far all ok, so continue with checking integrity in contained actions. + // The parent implementation will take care of invoking pre/post traversal + // metadata state preparations. + $violation_list = parent::checkIntegrity($metadata_state); return $violation_list; } /** * {@inheritdoc} */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - if ($until && $this->getUuid() === $until->getUuid()) { - return TRUE; - } - - $list_item_name = isset($this->configuration['list_item']) ? $this->configuration['list_item'] : 'list_item'; + protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) { try { $list_definition = $metadata_state->fetchDefinitionByPropertyPath($this->configuration['list']); $list_item_definition = $list_definition->getItemDefinition(); - $metadata_state->setDataDefinition($list_item_name, $list_item_definition); + $metadata_state->setDataDefinition($this->configuration['list_item'], $list_item_definition); } catch (RulesIntegrityException $e) { // Silently eat the exception: we just continue without adding the list // item definition to the state. } + } - foreach ($this->actions as $action) { - $found = $action->prepareExecutionMetadataState($metadata_state, $until); - if ($found) { - return TRUE; - } - } + /** + * {@inheritdoc} + */ + protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { // Remove the list item variable after the loop, it is out of scope now. - $metadata_state->removeDataDefinition($list_item_name); + $metadata_state->removeDataDefinition($this->configuration['list_item']); } - } From 44d188057d7206d90ed576d321b0f2bd48f21d41 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 21:06:16 +0100 Subject: [PATCH 38/57] Issue #2677034: Update ContextHandlerTraitTest to recent changes. --- src/Context/ContextHandlerIntegrityTrait.php | 1 - src/Engine/ConditionExpressionContainer.php | 1 + src/Plugin/RulesExpression/RulesLoop.php | 2 +- tests/src/Integration/Engine/IntegrityCheckTest.php | 1 - tests/src/Unit/ContextHandlerTraitTest.php | 7 +++++-- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Context/ContextHandlerIntegrityTrait.php b/src/Context/ContextHandlerIntegrityTrait.php index c504c769..6fc0542d 100644 --- a/src/Context/ContextHandlerIntegrityTrait.php +++ b/src/Context/ContextHandlerIntegrityTrait.php @@ -168,5 +168,4 @@ protected function checkDataTypeCompatible(CoreContextDefinitionInterface $conte } } - } diff --git a/src/Engine/ConditionExpressionContainer.php b/src/Engine/ConditionExpressionContainer.php index 322bdfa4..2c7ab340 100644 --- a/src/Engine/ConditionExpressionContainer.php +++ b/src/Engine/ConditionExpressionContainer.php @@ -246,4 +246,5 @@ protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { // Any post-traversal preparations need to be added here. } + } diff --git a/src/Plugin/RulesExpression/RulesLoop.php b/src/Plugin/RulesExpression/RulesLoop.php index 84e299d5..e80ce3f5 100644 --- a/src/Plugin/RulesExpression/RulesLoop.php +++ b/src/Plugin/RulesExpression/RulesLoop.php @@ -11,7 +11,6 @@ use Drupal\rules\Engine\ActionExpressionContainer; use Drupal\rules\Engine\ExecutionMetadataStateInterface; use Drupal\rules\Engine\ExecutionStateInterface; -use Drupal\rules\Engine\ExpressionInterface; use Drupal\rules\Engine\IntegrityViolationList; use Drupal\rules\Exception\RulesIntegrityException; @@ -119,4 +118,5 @@ protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) // Remove the list item variable after the loop, it is out of scope now. $metadata_state->removeDataDefinition($this->configuration['list_item']); } + } diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index 1611096b..a05ae1cc 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -391,7 +391,6 @@ public function testUsingRefinedProvidedVariables() { ); // The message action requires a string, thus if the context is not refined // it will end up as "any" and integrity check would fail. - $violation_list = RulesComponent::create($rule) ->checkIntegrity(); $this->assertEquals(0, iterator_count($violation_list)); diff --git a/tests/src/Unit/ContextHandlerTraitTest.php b/tests/src/Unit/ContextHandlerTraitTest.php index 442c6c36..f45f50d4 100644 --- a/tests/src/Unit/ContextHandlerTraitTest.php +++ b/tests/src/Unit/ContextHandlerTraitTest.php @@ -22,7 +22,7 @@ class ContextHandlerTraitTest extends RulesUnitTestBase { /** * Tests that a missing required context triggers an exception. * - * @covers ::mapContext + * @covers ::prepareContext * * @expectedException \Drupal\rules\Exception\RulesEvaluationException * @@ -43,13 +43,16 @@ public function testMissingContext() { $plugin->getContextDefinitions() ->willReturn(['test' => $context_definition->reveal()]) ->shouldBeCalled(1); + $plugin->getContextValue('test') + ->willReturn(NULL) + ->shouldBeCalled(1); $plugin->getPluginId()->willReturn('testplugin')->shouldBeCalledTimes(1); $state = $this->prophesize(ExecutionStateInterface::class); // Make the 'mapContext' method visible. $reflection = new \ReflectionClass($trait); - $method = $reflection->getMethod('mapContext'); + $method = $reflection->getMethod('prepareContext'); $method->setAccessible(TRUE); $method->invokeArgs($trait, [$plugin->reveal(), $state->reveal()]); } From 3a240535330e96a5611e59d0d4589fc012e21c53 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 23:42:58 +0100 Subject: [PATCH 39/57] Issue #2677034 by fago: Make type checking strict while there is no type inheritance support. --- src/Context/ContextHandlerIntegrityTrait.php | 29 ++++--------------- .../Integration/Engine/IntegrityCheckTest.php | 4 +-- tests/src/Unit/RulesConditionTest.php | 2 +- 3 files changed, 8 insertions(+), 27 deletions(-) diff --git a/src/Context/ContextHandlerIntegrityTrait.php b/src/Context/ContextHandlerIntegrityTrait.php index 6fc0542d..d50faa4c 100644 --- a/src/Context/ContextHandlerIntegrityTrait.php +++ b/src/Context/ContextHandlerIntegrityTrait.php @@ -9,10 +9,7 @@ use Drupal\Core\Plugin\Context\ContextDefinitionInterface as CoreContextDefinitionInterface; use Drupal\Core\Plugin\ContextAwarePluginInterface as CoreContextAwarePluginInterface; -use Drupal\Core\TypedData\ComplexDataInterface; use Drupal\Core\TypedData\DataDefinitionInterface; -use Drupal\Core\TypedData\ListInterface; -use Drupal\Core\TypedData\PrimitiveInterface; use Drupal\rules\Context\ContextDefinitionInterface as RulesContextDefinitionInterface; use Drupal\rules\Engine\ExecutionMetadataStateInterface; use Drupal\rules\Engine\IntegrityViolation; @@ -135,27 +132,11 @@ protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $ * The list of violations where new ones will be added. */ protected function checkDataTypeCompatible(CoreContextDefinitionInterface $context_definition, DataDefinitionInterface $provided, $context_name, IntegrityViolationList $violation_list) { - $expected_class = $context_definition->getDataDefinition()->getClass(); - $provided_class = $provided->getClass(); - $expected_type_problem = NULL; - - if (is_subclass_of($expected_class, PrimitiveInterface::class) - && !is_subclass_of($provided_class, PrimitiveInterface::class) - ) { - $expected_type_problem = $this->t('primitive'); - } - elseif (is_subclass_of($expected_class, ListInterface::class) - && !is_subclass_of($provided_class, ListInterface::class) - ) { - $expected_type_problem = $this->t('list'); - } - elseif (is_subclass_of($expected_class, ComplexDataInterface::class) - && !is_subclass_of($provided_class, ComplexDataInterface::class) - ) { - $expected_type_problem = $this->t('complex'); - } - - if ($expected_type_problem) { + // Compare data types. For now, fail if they are not equal. + // @todo: Add support for matching based upon type-inheritance. + $target_type = $context_definition->getDataDefinition()->getDataType(); + if ($target_type != 'any' && $target_type != $provided->getDataType()) { + $expected_type_problem = $context_definition->getDataDefinition()->getDataType(); $violation = new IntegrityViolation(); $violation->setMessage($this->t('Expected a @expected_type data type for context %context_name but got a @provided_type data type instead.', [ '@expected_type' => $expected_type_problem, diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index a05ae1cc..396367d3 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -235,7 +235,7 @@ public function testPrimitiveTypeViolation() { ->checkIntegrity(); $this->assertEquals(1, iterator_count($violation_list)); $this->assertEquals( - 'Expected a primitive data type for context Text to compare but got a list data type instead.', + 'Expected a string data type for context Text to compare but got a list data type instead.', (string) $violation_list[0]->getMessage() ); $this->assertEquals($condition->getUuid(), $violation_list[0]->getUuid()); @@ -287,7 +287,7 @@ public function testComplexTypeViolation() { ->checkIntegrity(); $this->assertEquals(1, iterator_count($violation_list)); $this->assertEquals( - 'Expected a complex data type for context Node but got a list data type instead.', + 'Expected a entity:node data type for context Node but got a list data type instead.', (string) $violation_list[0]->getMessage() ); $this->assertEquals($condition->getUuid(), $violation_list[0]->getUuid()); diff --git a/tests/src/Unit/RulesConditionTest.php b/tests/src/Unit/RulesConditionTest.php index 3e214884..cf688ebf 100644 --- a/tests/src/Unit/RulesConditionTest.php +++ b/tests/src/Unit/RulesConditionTest.php @@ -101,7 +101,7 @@ public function testDataProcessor() { // Mock some original old value that will be replaced by the data processor. $this->trueCondition->getContextValue('test') ->willReturn('old_value') - ->shouldBeCalledTimes(1); + ->shouldBeCalled(); // The outcome of the data processor needs to get set on the condition. $this->trueCondition->setContextValue('test', 'new_value')->shouldBeCalledTimes(1); From fe2186eda8d2ed248a54c89621cf3cd17e43bf07 Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 00:02:00 +0100 Subject: [PATCH 40/57] Issue #2677034 by fago: Document how integrity checks and exeuction metadata state preparations relate. --- src/Engine/ExpressionInterface.php | 37 +++++++++++++++++++----------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/Engine/ExpressionInterface.php b/src/Engine/ExpressionInterface.php index 1ed1bcc0..43577a5d 100644 --- a/src/Engine/ExpressionInterface.php +++ b/src/Engine/ExpressionInterface.php @@ -68,20 +68,6 @@ public function setRoot(ExpressionInterface $root); */ public function getLabel(); - /** - * Verifies that this expression is configured correctly. - * - * Example: all variable names used in the expression are available. - * - * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state - * The configuration state used to hold available data definitions of - * variables. - * - * @return \Drupal\rules\Engine\IntegrityViolationList - * A list object containing \Drupal\rules\Engine\IntegrityViolation objects. - */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state); - /** * Returns the UUID of this expression if it is nested in another expression. * @@ -98,6 +84,29 @@ public function getUuid(); */ public function setUuid($uuid); + /** + * Verifies that this expression is configured correctly. + * + * Example: All configured data selectors must be valid. + * + * Note that for checking integrity the execution metadata state must be + * passed prepared as achieved by ::prepareExecutionMetadataState() and the + * expression must apply all metadata state preparations during its integrity + * check as it does in ::prepareExecutionMetadataState(). + * This allows for efficient integrity checks of expression trees; e.g. see + * \Drupal\rules\Engine\ActionExpressionContainer::checkIntegrity(). + * + * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state + * The execution metadata state, prepared until right before this + * expression. + * + * @return \Drupal\rules\Engine\IntegrityViolationList + * A list object containing \Drupal\rules\Engine\IntegrityViolation objects. + * + * @see ::prepareExecutionMetadataState() + */ + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state); + /** * Prepares the execution metadata state by adding metadata to it. * From c5c0c068099b5805e1c668b35d34eaaed1c2e13a Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 27 Feb 2016 13:30:07 +0100 Subject: [PATCH 41/57] Issue 2677028 by fago: Make VariableAddTest properly refine context. --- tests/src/Integration/Action/VariableAddTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/Integration/Action/VariableAddTest.php b/tests/src/Integration/Action/VariableAddTest.php index f96c55ec..6db4fd7f 100644 --- a/tests/src/Integration/Action/VariableAddTest.php +++ b/tests/src/Integration/Action/VariableAddTest.php @@ -34,10 +34,12 @@ public function testExecute() { $action = $this->actionManager->createInstance('rules_variable_add'); $action->setContextValue('type', 'string'); $action->setContextValue('value', $variable); + $action->refineContextDefinitions(); $action->execute(); $result = $action->getProvidedContext('variable_added'); $this->assertEquals($variable, $result->getContextValue()); + $this->assertEquals('string', $result->getContextDefinition()->getDataType()); } } From acf0fc67b8354bc9e502b7bc1f0b693052e5fc68 Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 14:33:13 +0100 Subject: [PATCH 42/57] Issue #2676996 by fago: Allow contextual plugins to fetch metadata of configured context. --- src/Context/ContextAwarePluginInterface.php | 6 +- src/Context/ContextHandlerTrait.php | 98 ++++++++++++++----- src/Core/RulesActionBase.php | 2 +- src/Core/RulesConditionBase.php | 2 +- src/Plugin/RulesAction/EntityCreate.php | 2 +- src/Plugin/RulesAction/EntityFetchByField.php | 2 +- src/Plugin/RulesAction/EntityFetchById.php | 5 +- src/Plugin/RulesAction/VariableAdd.php | 2 +- src/Plugin/RulesExpression/RulesAction.php | 4 +- src/Plugin/RulesExpression/RulesCondition.php | 4 +- .../Integration/Action/EntityCreateTest.php | 2 +- .../Action/EntityFetchByFieldTest.php | 2 +- .../Action/EntityFetchByIdTest.php | 2 +- .../Integration/Action/VariableAddTest.php | 2 +- tests/src/Unit/RulesConditionTest.php | 4 +- 15 files changed, 97 insertions(+), 42 deletions(-) diff --git a/src/Context/ContextAwarePluginInterface.php b/src/Context/ContextAwarePluginInterface.php index dd5c37a3..c9a5bb19 100644 --- a/src/Context/ContextAwarePluginInterface.php +++ b/src/Context/ContextAwarePluginInterface.php @@ -20,7 +20,11 @@ interface ContextAwarePluginInterface extends CoreContextAwarePluginInterface { * When a plugin is configured half-way or even fully, some context values are * already available upon which the definition of subsequent or provided * context can be refined. + * + * @param \Drupal\Core\TypedData\DataDefinitionInterface[] $selected_data + * An array of data definitions for context that is mapped using a data + * selector, keyed by context name. */ - public function refineContextDefinitions(); + public function refineContextDefinitions(array $selected_data); } diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index 4cf02b6d..be3adf1e 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -12,6 +12,7 @@ use Drupal\rules\Engine\ExecutionMetadataStateInterface; use Drupal\rules\Engine\ExecutionStateInterface; use Drupal\rules\Exception\RulesEvaluationException; +use Drupal\rules\Exception\RulesIntegrityException; /** * Provides methods for handling context based on the plugin configuration. @@ -33,12 +34,8 @@ trait ContextHandlerTrait { /** * Prepares plugin context based upon the set context configuration. * - * If no execution state is given, the configuration is applied as far as - * possible. That means, the configured context values are set and context is - * refined. - * If an execution state is available, the plugin is prepared for execution - * by mapping the variables from the execution state into the plugin context - * and applying data processors. + * The plugin is prepared for execution by mapping the variables from the + * execution state into the plugin context and applying data processors. * In addition, it is ensured that all required context is basically * available as defined. This include the following checks: * - Required context must have a value set. @@ -47,41 +44,45 @@ trait ContextHandlerTrait { * @param \Drupal\Core\Plugin\ContextAwarePluginInterface $plugin * The plugin that is populated with context values. * @param \Drupal\rules\Engine\ExecutionStateInterface $state - * The Rules state containing available variables. + * The execution state containing available variables. * * @throws \Drupal\rules\Exception\RulesEvaluationException - * Thrown if an execution state is given, but some context is not satisfied; - * e.g. a required context is missing. + * Thrown if some context is not satisfied; e.g. a required context is + * missing. + * + * @see ::prepareContextWithMetadata() */ - protected function prepareContext(CoreContextAwarePluginInterface $plugin, ExecutionStateInterface $state = NULL) { + protected function prepareContext(CoreContextAwarePluginInterface $plugin, ExecutionStateInterface $state) { if (isset($this->configuration['context_values'])) { foreach ($this->configuration['context_values'] as $name => $value) { $plugin->setContextValue($name, $value); } } + + $selected_data = []; + // Map context by applying data selectors and collected the definitions of + // selected data for refining context definitions later. Note, that we must + // refine context definitions on execution time also, such that provided + // context gets the right metadata attached. + if (isset($this->configuration['context_mapping'])) { + foreach ($this->configuration['context_mapping'] as $name => $selector) { + $typed_data = $state->fetchDataByPropertyPath($selector); + $plugin->setContextValue($name, $typed_data); + $selected_data[$name] = $typed_data->getDataDefinition(); + } + } + if ($plugin instanceof ContextAwarePluginInterface) { // Getting context values may lead to undocumented exceptions if context // is not set right now. So catch those exceptions. // @todo: Remove ones https://www.drupal.org/node/2677162 got fixed. try { - $plugin->refineContextDefinitions(); + $plugin->refineContextDefinitions($selected_data); } catch (ContextException $e) { } } - // If no execution state has been provided, we are done now. - if (!$state) { - return; - } - - // Map context by apply data selectors. - if (isset($this->configuration['context_mapping'])) { - foreach ($this->configuration['context_mapping'] as $name => $selector) { - $typed_data = $state->fetchDataByPropertyPath($selector); - $plugin->setContextValue($name, $typed_data); - } - } // Apply data processors. $this->processData($plugin, $state); @@ -103,6 +104,57 @@ protected function prepareContext(CoreContextAwarePluginInterface $plugin, Execu } } + /** + * Prepares plugin context based upon the set context configuration. + * + * The configuration is applied as far as possible without having execution + * time data. That means, the configured context values are set and context is + * refined while leveraging the definitions of selected data. + * + * @param \Drupal\Core\Plugin\ContextAwarePluginInterface $plugin + * The plugin that is prepared. + * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state + * The metadata state, prepared for the current expression. + * + * @see ::prepareContext() + */ + protected function prepareContextWithMetadata(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { + if (isset($this->configuration['context_values'])) { + foreach ($this->configuration['context_values'] as $name => $value) { + $plugin->setContextValue($name, $value); + } + } + + $selected_data = []; + // Collected the definitions of selected data for refining context + // definitions. + if (isset($this->configuration['context_mapping'])) { + // If no state is available, we need to fetch at least the definitions of + // selected data for refining context. + foreach ($this->configuration['context_mapping'] as $name => $selector) { + try { + $selected_data[$name] = $this->getMappedDefinition($name, $metadata_state); + } + catch (RulesIntegrityException $e) { + // Ignore invalid data selectors here, such that context gets refined + // as far as possible still and can be respected by the UI when fixing + // broken selectors. + } + } + } + + if ($plugin instanceof ContextAwarePluginInterface) { + // Getting context values may lead to undocumented exceptions if context + // is not set right now. So catch those exceptions. + // @todo: Remove ones https://www.drupal.org/node/2677162 got fixed. + try { + $plugin->refineContextDefinitions($selected_data); + } + catch (ContextException $e) { + } + } + } + /** * Gets the definition of the data that is mapped to the given context. * diff --git a/src/Core/RulesActionBase.php b/src/Core/RulesActionBase.php index 1502b4b5..198bd887 100644 --- a/src/Core/RulesActionBase.php +++ b/src/Core/RulesActionBase.php @@ -46,7 +46,7 @@ public function getContextValue($name) { /** * {@inheritdoc} */ - public function refineContextDefinitions() { + public function refineContextDefinitions(array $selected_data) { // Do not refine anything by default. } diff --git a/src/Core/RulesConditionBase.php b/src/Core/RulesConditionBase.php index e594099b..98bda4c8 100644 --- a/src/Core/RulesConditionBase.php +++ b/src/Core/RulesConditionBase.php @@ -25,7 +25,7 @@ abstract class RulesConditionBase extends ConditionPluginBase implements RulesCo /** * {@inheritdoc} */ - public function refineContextDefinitions() { + public function refineContextDefinitions(array $selected_data) { // Do not refine anything by default. } diff --git a/src/Plugin/RulesAction/EntityCreate.php b/src/Plugin/RulesAction/EntityCreate.php index fa733b22..d68905cf 100644 --- a/src/Plugin/RulesAction/EntityCreate.php +++ b/src/Plugin/RulesAction/EntityCreate.php @@ -71,7 +71,7 @@ public static function create(ContainerInterface $container, array $configuratio /** * {@inheritdoc} */ - public function refineContextDefinitions() { + public function refineContextDefinitions(array $selected_data) { if ($type = $this->entityTypeId) { $data_type = "entity:$type"; diff --git a/src/Plugin/RulesAction/EntityFetchByField.php b/src/Plugin/RulesAction/EntityFetchByField.php index 281726fe..8392938d 100644 --- a/src/Plugin/RulesAction/EntityFetchByField.php +++ b/src/Plugin/RulesAction/EntityFetchByField.php @@ -90,7 +90,7 @@ public static function create(ContainerInterface $container, array $configuratio /** * {@inheritdoc} */ - public function refineContextDefinitions() { + public function refineContextDefinitions(array $selected_data) { if ($type = $this->getContextValue('type')) { $this->pluginDefinition['provides']['entity_fetched']->setDataType("entity:$type"); } diff --git a/src/Plugin/RulesAction/EntityFetchById.php b/src/Plugin/RulesAction/EntityFetchById.php index 0f196d05..61a23020 100644 --- a/src/Plugin/RulesAction/EntityFetchById.php +++ b/src/Plugin/RulesAction/EntityFetchById.php @@ -81,9 +81,8 @@ public static function create(ContainerInterface $container, array $configuratio /** * {@inheritdoc} */ - public function refineContextDefinitions() { - // @todo: Simplify ones https://www.drupal.org/node/2677162 got fixed. - if ($this->getContext('type')->hasContextValue() && $type = $this->getContextValue('type')) { + public function refineContextDefinitions(array $selected_data) { + if ($type = $this->getContextValue('type')) { $this->pluginDefinition['provides']['entity_fetched']->setDataType("entity:$type"); } } diff --git a/src/Plugin/RulesAction/VariableAdd.php b/src/Plugin/RulesAction/VariableAdd.php index a4bb0c13..94d9be9c 100644 --- a/src/Plugin/RulesAction/VariableAdd.php +++ b/src/Plugin/RulesAction/VariableAdd.php @@ -49,7 +49,7 @@ protected function doExecute($type, $value) { /** * {@inheritdoc} */ - public function refineContextDefinitions() { + public function refineContextDefinitions(array $selected_data) { if ($type = $this->getContextValue('type')) { $this->pluginDefinition['context']['value']->setDataType($type); $this->pluginDefinition['provides']['variable_added']->setDataType($type); diff --git a/src/Plugin/RulesExpression/RulesAction.php b/src/Plugin/RulesExpression/RulesAction.php index fc5db0d5..bc0896c7 100644 --- a/src/Plugin/RulesExpression/RulesAction.php +++ b/src/Plugin/RulesExpression/RulesAction.php @@ -149,7 +149,7 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) // Prepare and refine the context before checking integrity, such that any // context definition changes are respected while checking. - $this->prepareContext($action); + $this->prepareContextWithMetadata($action, $metadata_state); $result = $this->checkContextConfigIntegrity($action, $metadata_state); $this->prepareExecutionMetadataState($metadata_state); return $result; @@ -165,7 +165,7 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m $action = $this->actionManager->createInstance($this->configuration['action_id']); // Make sure to refine context first, such that possibly refined definitions // of provided context are respected. - $this->prepareContext($action); + $this->prepareContextWithMetadata($action, $metadata_state); $this->addProvidedContextDefinitions($action, $metadata_state); } diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index 662513aa..7ff78d02 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -173,7 +173,7 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) ]); // Prepare and refine the context before checking integrity, such that any // context definition changes are respected while checking. - $this->prepareContext($condition); + $this->prepareContextWithMetadata($condition, $metadata_state); $result = $this->checkContextConfigIntegrity($condition, $metadata_state); $this->prepareExecutionMetadataState($metadata_state); return $result; @@ -191,7 +191,7 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m ]); // Make sure to refine context first, such that possibly refined definitions // of provided context are respected. - $this->prepareContext($condition); + $this->prepareContextWithMetadata($condition, $metadata_state); $this->addProvidedContextDefinitions($condition, $metadata_state); } diff --git a/tests/src/Integration/Action/EntityCreateTest.php b/tests/src/Integration/Action/EntityCreateTest.php index cbb8a75f..ef101874 100644 --- a/tests/src/Integration/Action/EntityCreateTest.php +++ b/tests/src/Integration/Action/EntityCreateTest.php @@ -140,7 +140,7 @@ public function testRequiredContexts() { */ public function testRefiningContextDefinitions() { $this->action->setContextValue('bundle', 'bundle_test'); - $this->action->refineContextDefinitions(); + $this->action->refineContextDefinitions([]); $this->assertEquals( $this->action->getProvidedContextDefinition('entity') ->getDataType(), 'entity:test:bundle_test' diff --git a/tests/src/Integration/Action/EntityFetchByFieldTest.php b/tests/src/Integration/Action/EntityFetchByFieldTest.php index 4e0aa43f..e101b5ce 100644 --- a/tests/src/Integration/Action/EntityFetchByFieldTest.php +++ b/tests/src/Integration/Action/EntityFetchByFieldTest.php @@ -172,7 +172,7 @@ public function testActionExecutionProvidedContextEntityType() { */ public function testRefiningContextDefinitions() { $this->action->setContextValue('type', 'entity_test'); - $this->action->refineContextDefinitions(); + $this->action->refineContextDefinitions([]); $this->assertEquals( $this->action->getProvidedContextDefinition('entity_fetched') ->getDataType(), 'entity:entity_test' diff --git a/tests/src/Integration/Action/EntityFetchByIdTest.php b/tests/src/Integration/Action/EntityFetchByIdTest.php index bd040717..bf633e1a 100644 --- a/tests/src/Integration/Action/EntityFetchByIdTest.php +++ b/tests/src/Integration/Action/EntityFetchByIdTest.php @@ -74,7 +74,7 @@ public function testActionExecution() { */ public function testRefiningContextDefinitions() { $this->action->setContextValue('type', 'entity_test'); - $this->action->refineContextDefinitions(); + $this->action->refineContextDefinitions([]); $this->assertEquals( $this->action->getProvidedContextDefinition('entity_fetched') ->getDataType(), 'entity:entity_test' diff --git a/tests/src/Integration/Action/VariableAddTest.php b/tests/src/Integration/Action/VariableAddTest.php index 6db4fd7f..32c71825 100644 --- a/tests/src/Integration/Action/VariableAddTest.php +++ b/tests/src/Integration/Action/VariableAddTest.php @@ -34,7 +34,7 @@ public function testExecute() { $action = $this->actionManager->createInstance('rules_variable_add'); $action->setContextValue('type', 'string'); $action->setContextValue('value', $variable); - $action->refineContextDefinitions(); + $action->refineContextDefinitions([]); $action->execute(); $result = $action->getProvidedContext('variable_added'); diff --git a/tests/src/Unit/RulesConditionTest.php b/tests/src/Unit/RulesConditionTest.php index cf688ebf..ec1d1533 100644 --- a/tests/src/Unit/RulesConditionTest.php +++ b/tests/src/Unit/RulesConditionTest.php @@ -105,7 +105,7 @@ public function testDataProcessor() { // The outcome of the data processor needs to get set on the condition. $this->trueCondition->setContextValue('test', 'new_value')->shouldBeCalledTimes(1); - $this->trueCondition->refineContextDefinitions()->shouldBeCalledTimes(1); + $this->trueCondition->refineContextDefinitions([])->shouldBeCalledTimes(1); $data_processor = $this->prophesize(DataProcessorInterface::class); $data_processor->process('old_value', Argument::any()) @@ -130,7 +130,7 @@ public function testDataProcessor() { */ public function testNegation() { $this->trueCondition->getContextDefinitions()->willReturn([]); - $this->trueCondition->refineContextDefinitions()->shouldBeCalledTimes(1); + $this->trueCondition->refineContextDefinitions([])->shouldBeCalledTimes(1); $this->trueCondition->getProvidedContextDefinitions() ->willReturn([]) ->shouldBeCalledTimes(1); From 48785689fadc12f9fc611ca1868975c9f7a70ebe Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 15:14:19 +0100 Subject: [PATCH 43/57] Fix integrity checks of required context to respect default values. --- src/Context/ContextHandlerIntegrityTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Context/ContextHandlerIntegrityTrait.php b/src/Context/ContextHandlerIntegrityTrait.php index d50faa4c..bf206a54 100644 --- a/src/Context/ContextHandlerIntegrityTrait.php +++ b/src/Context/ContextHandlerIntegrityTrait.php @@ -87,7 +87,7 @@ protected function checkContextConfigIntegrity(CoreContextAwarePluginInterface $ $violation_list->add($violation); } } - elseif ($context_definition->isRequired()) { + elseif ($context_definition->isRequired() && $context_definition->getDefaultValue() === NULL) { $violation = new IntegrityViolation(); $violation->setMessage($this->t('The required context %context_name is missing.', [ '%context_name' => $context_definition->getLabel(), From 8093abba69cca9fd5f914f95b0cf35f640108d60 Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 15:15:09 +0100 Subject: [PATCH 44/57] Improve work-around for #2677162 to only silently catch exception for required context. --- src/Context/ContextHandlerTrait.php | 3 +++ src/Core/RulesActionBase.php | 3 +++ src/Core/RulesConditionBase.php | 3 +++ 3 files changed, 9 insertions(+) diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index be3adf1e..93c3dfc3 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -151,6 +151,9 @@ protected function prepareContextWithMetadata(CoreContextAwarePluginInterface $p $plugin->refineContextDefinitions($selected_data); } catch (ContextException $e) { + if (strpos($e->getMessage(), 'context is required') === FALSE) { + throw $e; + } } } } diff --git a/src/Core/RulesActionBase.php b/src/Core/RulesActionBase.php index 198bd887..acb0e0fd 100644 --- a/src/Core/RulesActionBase.php +++ b/src/Core/RulesActionBase.php @@ -40,6 +40,9 @@ public function getContextValue($name) { // Catch the undocumented exception thrown when no context value is set // for a required context. // @todo: Remove once https://www.drupal.org/node/2677162 is fixed. + if (strpos($e->getMessage(), 'context is required') === FALSE) { + throw $e; + } } } diff --git a/src/Core/RulesConditionBase.php b/src/Core/RulesConditionBase.php index 98bda4c8..5bf1d577 100644 --- a/src/Core/RulesConditionBase.php +++ b/src/Core/RulesConditionBase.php @@ -40,6 +40,9 @@ public function getContextValue($name) { // Catch the undocumented exception thrown when no context value is set // for a required context. // @todo: Remove once https://www.drupal.org/node/2677162 is fixed. + if (strpos($e->getMessage(), 'context is required') === FALSE) { + throw $e; + } } } From bd51c6e84fdae7b0599f064044eca2723186d360 Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 15:17:15 +0100 Subject: [PATCH 45/57] Re-name data comparison operator context to operation and make it required. --- src/Plugin/Condition/DataComparison.php | 15 +++-- .../Condition/DataComparisonTest.php | 60 +++++++++---------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/Plugin/Condition/DataComparison.php b/src/Plugin/Condition/DataComparison.php index 27f6cdda..0dd7c680 100644 --- a/src/Plugin/Condition/DataComparison.php +++ b/src/Plugin/Condition/DataComparison.php @@ -21,11 +21,10 @@ * label = @Translation("Data to compare"), * description = @Translation("The data to be checked to be empty, specified by using a data selector, e.g. 'node:uid:entity:name:value'.") * ), - * "operator" = @ContextDefinition("string", + * "operation" = @ContextDefinition("string", * label = @Translation("Operator"), - * description = @Translation("The comparison operator."), + * description = @Translation("The comparison operation."), * default_value = "==", - * required = FALSE * ), * "value" = @ContextDefinition("any", * label = @Translation("Data value"), @@ -44,8 +43,8 @@ class DataComparison extends RulesConditionBase { * * @param mixed $data * Supplied data to test. - * @param string $operator - * Data comparison operator. Typically one of: + * @param string $operation + * Data comparison operation. Typically one of: * - "==" * - "<" * - ">" @@ -57,9 +56,9 @@ class DataComparison extends RulesConditionBase { * @return bool * The evaluation of the condition. */ - protected function doEvaluate($data, $operator, $value) { - $operator = $operator ? $operator : '=='; - switch ($operator) { + protected function doEvaluate($data, $operation, $value) { + $operation = $operation ? $operation : '=='; + switch ($operation) { case '<': return $data < $value; diff --git a/tests/src/Integration/Condition/DataComparisonTest.php b/tests/src/Integration/Condition/DataComparisonTest.php index 6310ba0d..e5cbb4b2 100644 --- a/tests/src/Integration/Condition/DataComparisonTest.php +++ b/tests/src/Integration/Condition/DataComparisonTest.php @@ -55,76 +55,76 @@ public function testConditionEvaluationOperatorEquals() { // is '==', TRUE is returned. $this->condition ->setContextValue('data', 'Llama') - ->setContextValue('operator', '==') + ->setContextValue('operation', '==') ->setContextValue('value', 'Llama'); $this->assertTrue($this->condition->evaluate()); // Test that when the data string does not equal the value string and the - // operator is '==', FALSE is returned. + // operation is '==', FALSE is returned. $this->condition ->setContextValue('data', 'Kitten') - ->setContextValue('operator', '==') + ->setContextValue('operation', '==') ->setContextValue('value', 'Llama'); $this->assertFalse($this->condition->evaluate()); - // Test that when both data and value are false booleans and the operator + // Test that when both data and value are false booleans and the operation // is '==', TRUE is returned. $this->condition ->setContextValue('data', FALSE) - ->setContextValue('operator', '==') + ->setContextValue('operation', '==') ->setContextValue('value', FALSE); $this->assertTrue($this->condition->evaluate()); // Test that when a boolean data does not equal a boolean value - // and the operator is '==', FALSE is returned. + // and the operation is '==', FALSE is returned. $this->condition ->setContextValue('data', TRUE) - ->setContextValue('operator', '==') + ->setContextValue('operation', '==') ->setContextValue('value', FALSE); $this->assertFalse($this->condition->evaluate()); } /** - * Tests evaluating the condition with the "contains" operator. + * Tests evaluating the condition with the "contains" operation. * * @covers ::evaluate */ public function testConditionEvaluationOperatorContains() { // Test that when the data string contains the value string, and the - // operator is 'CONTAINS', TRUE is returned. + // operation is 'CONTAINS', TRUE is returned. $this->condition ->setContextValue('data', 'Big Llama') - ->setContextValue('operator', 'contains') + ->setContextValue('operation', 'contains') ->setContextValue('value', 'Llama'); $this->assertTrue($this->condition->evaluate()); // Test that when the data string does not contain the value string, and - // the operator is 'contains', TRUE is returned. + // the operation is 'contains', TRUE is returned. $this->condition ->setContextValue('data', 'Big Kitten') - ->setContextValue('operator', 'contains') + ->setContextValue('operation', 'contains') ->setContextValue('value', 'Big Kitten'); $this->assertTrue($this->condition->evaluate()); - // Test that when a data array contains the value string, and the operator + // Test that when a data array contains the value string, and the operation // is 'CONTAINS', TRUE is returned. $this->condition ->setContextValue('data', ['Llama', 'Kitten']) - ->setContextValue('operator', 'contains') + ->setContextValue('operation', 'contains') ->setContextValue('value', 'Llama'); $this->assertTrue($this->condition->evaluate()); // Test that when a data array does not contain the value array, and the - // operator is 'CONTAINS', TRUE is returned. + // operation is 'CONTAINS', TRUE is returned. $this->condition ->setContextValue('data', ['Kitten']) - ->setContextValue('operator', 'contains') + ->setContextValue('operation', 'contains') ->setContextValue('value', ['Llama']); $this->assertFalse($this->condition->evaluate()); } /** - * Tests evaluating the condition with the "IN" operator. + * Tests evaluating the condition with the "IN" operation. * * @covers ::evaluate */ @@ -132,61 +132,61 @@ public function testConditionEvaluationOperatorIn() { // Test that when the data string is 'IN' the value array, TRUE is returned. $this->condition ->setContextValue('data', 'Llama') - ->setContextValue('operator', 'IN') + ->setContextValue('operation', 'IN') ->setContextValue('value', ['Llama', 'Kitten']); $this->assertTrue($this->condition->evaluate()); - // Test that when the data array is not in the value array, and the operator + // Test that when the data array is not in the value array, and the operation // is 'IN', FALSE is returned. $this->condition ->setContextValue('data', ['Llama']) - ->setContextValue('operator', 'IN') + ->setContextValue('operation', 'IN') ->setContextValue('value', ['Kitten']); $this->assertFalse($this->condition->evaluate()); } /** - * Tests evaluating the condition with the "is less than" operator. + * Tests evaluating the condition with the "is less than" operation. * * @covers ::evaluate */ public function testConditionEvaluationOperatorLessThan() { - // Test that when data is less than value and operator is '<', + // Test that when data is less than value and operation is '<', // TRUE is returned. $this->condition ->setContextValue('data', 1) - ->setContextValue('operator', '<') + ->setContextValue('operation', '<') ->setContextValue('value', 2); $this->assertTrue($this->condition->evaluate()); - // Test that when data is greater than value and operator is '<', + // Test that when data is greater than value and operation is '<', // FALSE is returned. $this->condition ->setContextValue('data', 2) - ->setContextValue('operator', '<') + ->setContextValue('operation', '<') ->setContextValue('value', 1); $this->assertFalse($this->condition->evaluate()); } /** - * Tests evaluating the condition with the "is greater than" operator. + * Tests evaluating the condition with the "is greater than" operation. * * @covers ::evaluate */ public function testConditionEvaluationOperatorGreaterThan() { - // Test that when data is greater than value and operator is '>', + // Test that when data is greater than value and operation is '>', // TRUE is returned. $this->condition ->setContextValue('data', 2) - ->setContextValue('operator', '>') + ->setContextValue('operation', '>') ->setContextValue('value', 1); $this->assertTrue($this->condition->evaluate()); - // Test that when data is less than value and operator is '>', + // Test that when data is less than value and operation is '>', // FALSE is returned. $this->condition ->setContextValue('data', 1) - ->setContextValue('operator', '>') + ->setContextValue('operation', '>') ->setContextValue('value', 2); $this->assertFalse($this->condition->evaluate()); } From 7f0d1a7f9f64cb79c2883c7fee7722f57064bd9c Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 15:18:15 +0100 Subject: [PATCH 46/57] Issue #2676996 by fago: Add test coverage for checking integrity based upon selected context. --- src/Plugin/Condition/DataComparison.php | 12 +++++++++ .../Integration/Engine/IntegrityCheckTest.php | 26 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/Plugin/Condition/DataComparison.php b/src/Plugin/Condition/DataComparison.php index 0dd7c680..77d0c7db 100644 --- a/src/Plugin/Condition/DataComparison.php +++ b/src/Plugin/Condition/DataComparison.php @@ -81,4 +81,16 @@ protected function doEvaluate($data, $operation, $value) { } } + /** + * {@inheritdoc} + */ + public function refineContextDefinitions(array $selected_data) { + if (isset($selected_data['data'])) { + $this->pluginDefinition['context']['value']->setDataType($selected_data['data']->getDataType()); + if ($this->getContextValue('operation') == 'IN') { + $this->pluginDefinition['context']['value']->setMultiple(); + } + } + } + } diff --git a/tests/src/Integration/Engine/IntegrityCheckTest.php b/tests/src/Integration/Engine/IntegrityCheckTest.php index 396367d3..ebb731ae 100644 --- a/tests/src/Integration/Engine/IntegrityCheckTest.php +++ b/tests/src/Integration/Engine/IntegrityCheckTest.php @@ -375,6 +375,32 @@ public function testRefinedContextViolation() { $this->assertEquals(1, iterator_count($violation_list)); } + /** + * Tests context can be refined based upon mapped context. + */ + public function testRefiningContextBasedonMappedContext() { + // DataComparision condition refines context based on selected data. Thus + // it for the test and ensure checking integrity passes when the comparison + // value is of a compatible type and fails else. + $rule = $this->rulesExpressionManager->createRule(); + $rule->addCondition('rules_data_comparison', ContextConfig::create() + ->map('data', 'text') + ->map('value', 'text2') + ); + + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('text', ContextDefinition::create('string')) + ->addContextDefinition('text2', ContextDefinition::create('string')) + ->checkIntegrity(); + $this->assertEquals(0, iterator_count($violation_list)); + + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('text', ContextDefinition::create('string')) + ->addContextDefinition('text2', ContextDefinition::create('integer')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + } + /** * Tests using provided variables with refined context. */ From eee4fc655788b650db8cf3ed96f8e05e5e32393a Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 15:28:17 +0100 Subject: [PATCH 47/57] Issue #2281079 by fago: Add test-coverage for refining context definitions of data comparisons. --- .../Condition/DataComparisonTest.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/src/Integration/Condition/DataComparisonTest.php b/tests/src/Integration/Condition/DataComparisonTest.php index e5cbb4b2..5811b0c8 100644 --- a/tests/src/Integration/Condition/DataComparisonTest.php +++ b/tests/src/Integration/Condition/DataComparisonTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\rules\Integration\Condition; +use Drupal\Core\TypedData\DataDefinition; use Drupal\Tests\rules\Integration\RulesIntegrationTestBase; /** @@ -200,4 +201,23 @@ public function testSummary() { $this->assertEquals('Data comparison', $this->condition->summary()); } + /** + * @covers ::refineContextDefinitions + */ + public function testRefineContextDefinitions() { + // When a string is selected for comparison, the value must be string also. + $this->condition->refineContextDefinitions([ + 'data' => DataDefinition::create('string'), + ]); + $this->assertEquals('string', $this->condition->getContextDefinition('value')->getDataType()); + + // IN operation requires a list of strings as value. + $this->condition->setContextValue('operation', 'IN'); + $this->condition->refineContextDefinitions([ + 'data' => DataDefinition::create('string'), + ]); + $this->assertEquals('string', $this->condition->getContextDefinition('value')->getDataType()); + $this->assertTrue($this->condition->getContextDefinition('value')->isMultiple()); + } + } From 67aea83f6aee4835dbbc1cd14ff0af547e6ea0f8 Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 28 Feb 2016 15:39:55 +0100 Subject: [PATCH 48/57] Improve prepareContextWithMetadata() documentation to mention possibly thrown context exceptions. --- src/Context/ContextHandlerTrait.php | 7 +++++++ tests/src/Integration/Condition/DataComparisonTest.php | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index 93c3dfc3..764f6776 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -80,6 +80,9 @@ protected function prepareContext(CoreContextAwarePluginInterface $plugin, Execu $plugin->refineContextDefinitions($selected_data); } catch (ContextException $e) { + if (strpos($e->getMessage(), 'context is required') === FALSE) { + throw new RulesEvaluationException($e->getMessage()); + } } } @@ -116,6 +119,10 @@ protected function prepareContext(CoreContextAwarePluginInterface $plugin, Execu * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state * The metadata state, prepared for the current expression. * + * @throws \Drupal\Component\Plugin\Exception\ContextException + * Thrown if the plugin tries to access some not-defined context. As this is + * a developer error, this should not be caught. + * * @see ::prepareContext() */ protected function prepareContextWithMetadata(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { diff --git a/tests/src/Integration/Condition/DataComparisonTest.php b/tests/src/Integration/Condition/DataComparisonTest.php index 5811b0c8..faf1711f 100644 --- a/tests/src/Integration/Condition/DataComparisonTest.php +++ b/tests/src/Integration/Condition/DataComparisonTest.php @@ -137,8 +137,8 @@ public function testConditionEvaluationOperatorIn() { ->setContextValue('value', ['Llama', 'Kitten']); $this->assertTrue($this->condition->evaluate()); - // Test that when the data array is not in the value array, and the operation - // is 'IN', FALSE is returned. + // Test that when the data array is not in the value array, and the + // operation is 'IN', FALSE is returned. $this->condition ->setContextValue('data', ['Llama']) ->setContextValue('operation', 'IN') From cc04db67a5c12446b8d8ab77d69619a596d12a9a Mon Sep 17 00:00:00 2001 From: Klaus Purer Date: Sat, 5 Mar 2016 09:50:45 +0100 Subject: [PATCH 49/57] Enabled Drupal 8.2.x dev in testing --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 498f2dc8..80eb8f54 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,6 +14,7 @@ php: env: - DRUPAL_CORE=8.0.x - DRUPAL_CORE=8.1.x + - DRUPAL_CORE=8.2.x matrix: allow_failures: @@ -59,7 +60,9 @@ before_script: # Run composer install for Drupal 8.1. We need an up-to-date composer when # installing Drupal 8.1. - - if [ "$DRUPAL_CORE" = "8.1.x" ]; then composer self-update && composer install; fi + # Disabled for now because core still includes the vendor directory, see + # https://www.drupal.org/node/1475510 + # - if [ "$DRUPAL_CORE" = "8.1.x" ]; then composer self-update && composer install; fi # Start a web server on port 8888, run in the background. - php -S localhost:8888 & From 691dbebf9023fff088bddb2dc089bea90bb74202 Mon Sep 17 00:00:00 2001 From: Klaus Purer Date: Mon, 29 Feb 2016 15:22:25 +0100 Subject: [PATCH 50/57] Removed duplicate config schema definition --- config/schema/rules.context.schema.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/config/schema/rules.context.schema.yml b/config/schema/rules.context.schema.yml index 45ee7071..3f6126e0 100644 --- a/config/schema/rules.context.schema.yml +++ b/config/schema/rules.context.schema.yml @@ -25,12 +25,6 @@ rules.context.values: # The entries depend on the plugin here. Plugins have to extend this and # provide a suiting schema. -rules.context.mapping: - type: sequence - label: 'Context mapping' - sequence: - - type: string - rules.context.processors: type: sequence label: 'Context processors' From 3a0d2303f8473b7eaa134c12e86a2f1e5a6e1d88 Mon Sep 17 00:00:00 2001 From: Klaus Purer Date: Sun, 6 Mar 2016 13:40:18 +0100 Subject: [PATCH 51/57] Issue #2664700: Expose Rules components as action plugin --- config/schema/rules.action.schema.yml | 7 + config/schema/rules.data_types.schema.yml | 6 +- src/Engine/ExecutionState.php | 19 +- src/Engine/ExecutionStateInterface.php | 9 + src/Engine/RulesComponent.php | 6 +- src/Entity/RulesComponentConfig.php | 35 ++-- src/Plugin/RulesAction/EntityCreate.php | 10 +- .../RulesAction/EntityCreateDeriver.php | 18 +- src/Plugin/RulesAction/EntitySave.php | 2 +- .../RulesAction/RulesComponentAction.php | 129 ++++++++++++ .../RulesComponentActionDeriver.php | 82 ++++++++ ...component.rules_test_default_component.yml | 7 +- .../Integration/Action/EntityCreateTest.php | 21 +- .../Action/RulesComponentActionTest.php | 198 ++++++++++++++++++ .../Integration/RulesIntegrationTestBase.php | 6 + tests/src/Kernel/CoreIntegrationTest.php | 41 ++++ 16 files changed, 560 insertions(+), 36 deletions(-) create mode 100644 src/Plugin/RulesAction/RulesComponentAction.php create mode 100644 src/Plugin/RulesAction/RulesComponentActionDeriver.php create mode 100644 tests/src/Integration/Action/RulesComponentActionTest.php diff --git a/config/schema/rules.action.schema.yml b/config/schema/rules.action.schema.yml index ac585450..bbefa6aa 100644 --- a/config/schema/rules.action.schema.yml +++ b/config/schema/rules.action.schema.yml @@ -1,3 +1,10 @@ +# Per default the schema of arbitrary context values of an action cannot be +# typed. Actions that need translatability or other features of the config +# system must specify their context value schema explicitly, see examples below. +rules.action.context_values.*: + type: ignore + label: Context values + rules.action.context_values.rules_system_message: type: mapping label: Message action context values diff --git a/config/schema/rules.data_types.schema.yml b/config/schema/rules.data_types.schema.yml index c40fee14..bbc9c351 100644 --- a/config/schema/rules.data_types.schema.yml +++ b/config/schema/rules.data_types.schema.yml @@ -7,11 +7,11 @@ rules_component: label: 'Context definitions' sequence: - type: rules.context.definition - provided_context: + provided_context_definitions: type: sequence - label: 'Names of provided context' + label: 'Provided context definitions' sequence: - - type: string + - type: rules.context.definition expression: type: rules_expression.[id] label: 'Expression configuration' diff --git a/src/Engine/ExecutionState.php b/src/Engine/ExecutionState.php index 18eba781..21d8d972 100644 --- a/src/Engine/ExecutionState.php +++ b/src/Engine/ExecutionState.php @@ -178,6 +178,13 @@ public function saveChangesLater($selector) { return $this; } + /** + * {@inheritdoc} + */ + public function getAutoSaveSelectors() { + return array_keys($this->saveLater); + } + /** * {@inheritdoc} */ @@ -185,14 +192,10 @@ public function autoSave() { // Make changes permanent. foreach ($this->saveLater as $selector => $flag) { $typed_data = $this->fetchDataByPropertyPath($selector); - // The returned data can be NULL, only save it if we actually have - // something here. - if ($typed_data) { - // Things that can be saved must have a save() method, right? - // Saving is always done at the root of the typed data tree, for example - // on the entity level. - $typed_data->getRoot()->getValue()->save(); - } + // Things that can be saved must have a save() method, right? + // Saving is always done at the root of the typed data tree, for example + // on the entity level. + $typed_data->getRoot()->getValue()->save(); } return $this; } diff --git a/src/Engine/ExecutionStateInterface.php b/src/Engine/ExecutionStateInterface.php index c75833fc..65464fb4 100644 --- a/src/Engine/ExecutionStateInterface.php +++ b/src/Engine/ExecutionStateInterface.php @@ -121,6 +121,15 @@ public function fetchDataByPropertyPath($property_path, $langcode = NULL); */ public function saveChangesLater($selector); + /** + * Returns the list of variables that should be auto-saved after execution. + * + * @return string[] + * The list of data selectors that specify the target object to be saved. + * Example: node.uid.entity. + */ + public function getAutoSaveSelectors(); + /** * Saves all variables that have been marked for auto saving. * diff --git a/src/Engine/RulesComponent.php b/src/Engine/RulesComponent.php index 1e500c50..27049e64 100644 --- a/src/Engine/RulesComponent.php +++ b/src/Engine/RulesComponent.php @@ -69,7 +69,7 @@ public static function create(ExpressionInterface $expression) { public static function createFromConfiguration(array $configuration) { $configuration += [ 'context_definitions' => [], - 'provided_context' => [], + 'provided_context_definitions' => [], ]; // @todo: Can we improve this use dependency injection somehow? $expression_manager = \Drupal::service('plugin.manager.rules_expression'); @@ -78,7 +78,7 @@ public static function createFromConfiguration(array $configuration) { foreach ($configuration['context_definitions'] as $name => $definition) { $component->addContextDefinition($name, ContextDefinition::createFromArray($definition)); } - foreach ($configuration['provided_context'] as $name) { + foreach ($configuration['provided_context_definitions'] as $name => $definition) { $component->provideContext($name); } return $component; @@ -122,7 +122,7 @@ public function getConfiguration() { 'context_definitions' => array_map(function (ContextDefinitionInterface $definition) { return $definition->toArray(); }, $this->contextDefinitions), - 'provided_context' => $this->providedContext, + 'provided_context_definitions' => $this->providedContext, ]; } diff --git a/src/Entity/RulesComponentConfig.php b/src/Entity/RulesComponentConfig.php index 3c638cf5..e4962881 100644 --- a/src/Entity/RulesComponentConfig.php +++ b/src/Entity/RulesComponentConfig.php @@ -164,8 +164,10 @@ public function updateFromComponent(RulesComponent $component) { */ public function getContextDefinitions() { $definitions = []; - foreach ($this->component['context_definitions'] as $name => $definition) { - $definitions[$name] = ContextDefinition::createFromArray($definition); + if (!empty($this->component['context_definitions'])) { + foreach ($this->component['context_definitions'] as $name => $definition) { + $definitions[$name] = ContextDefinition::createFromArray($definition); + } } return $definitions; } @@ -187,25 +189,34 @@ public function setContextDefinitions($definitions) { } /** - * Returns the names of context that is provided back to the caller. + * Gets the definitions of the provided context. * - * @return string[] - * The names of the context that is provided back. + * @return \Drupal\rules\Context\ContextDefinitionInterface[] + * The array of context definition, keyed by context name. */ - public function getProvidedContext() { - return $this->component['provided_context']; + public function getProvidedContextDefinitions() { + $definitions = []; + if (!empty($this->component['provided_context_definitions'])) { + foreach ($this->component['provided_context_definitions'] as $name => $definition) { + $definitions[$name] = ContextDefinition::createFromArray($definition); + } + } + return $definitions; } /** - * Sets the names of the context that is provided back to the caller. + * Sets the definitions of the provided context. * - * @param string[] $names - * The names of the context that is provided back. + * @param \Drupal\rules\Context\ContextDefinitionInterface[] $definitions + * The array of context definitions, keyed by context name. * * @return $this */ - public function setProvidedContext($names) { - $this->component['provided_context'] = $names; + public function setProvidedContextDefinitions($definitions) { + $this->component['provided_context_definitions'] = []; + foreach ($definitions as $name => $definition) { + $this->component['provided_context_definitions'][$name] = $definition->toArray(); + } return $this; } diff --git a/src/Plugin/RulesAction/EntityCreate.php b/src/Plugin/RulesAction/EntityCreate.php index d68905cf..aafd465f 100644 --- a/src/Plugin/RulesAction/EntityCreate.php +++ b/src/Plugin/RulesAction/EntityCreate.php @@ -38,6 +38,13 @@ class EntityCreate extends RulesActionBase implements ContainerFactoryPluginInte */ protected $entityTypeId; + /** + * The entity bundle key used for the entity type. + * + * @var string + */ + protected $bundleKey; + /** * Constructs an EntityCreate object. * @@ -54,6 +61,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition parent::__construct($configuration, $plugin_id, $plugin_definition); $this->storage = $storage; $this->entityTypeId = $plugin_definition['entity_type_id']; + $this->bundleKey = $plugin_definition['bundle_key']; } /** @@ -75,7 +83,7 @@ public function refineContextDefinitions(array $selected_data) { if ($type = $this->entityTypeId) { $data_type = "entity:$type"; - if ($bundle = $this->getContextValue('bundle')) { + if ($this->bundleKey && $bundle = $this->getContextValue($this->bundleKey)) { $data_type .= ":$bundle"; } diff --git a/src/Plugin/RulesAction/EntityCreateDeriver.php b/src/Plugin/RulesAction/EntityCreateDeriver.php index f40c1ab0..18f88425 100644 --- a/src/Plugin/RulesAction/EntityCreateDeriver.php +++ b/src/Plugin/RulesAction/EntityCreateDeriver.php @@ -9,11 +9,12 @@ use Drupal\Component\Plugin\Derivative\DeriverBase; use Drupal\Core\Entity\ContentEntityTypeInterface; -use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Entity\EntityFieldManagerInterface; +use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Plugin\Discovery\ContainerDeriverInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\Core\TypedData\DataReferenceDefinitionInterface; use Drupal\rules\Context\ContextDefinition; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -83,15 +84,28 @@ public function getDerivativeDefinitions($base_plugin_definition) { // other required base fields. This matches the storage create() behavior, // where only the bundle requirement is enforced. $bundle_key = $entity_type->getKey('bundle'); + $this->derivatives[$entity_type_id]['bundle_key'] = $bundle_key; + $base_field_definitions = $this->entityFieldManager->getBaseFieldDefinitions($entity_type_id); foreach ($base_field_definitions as $field_name => $definition) { if ($field_name != $bundle_key && !$definition->isRequired()) { continue; } + $item_definition = $definition->getItemDefinition(); + $type_definition = $item_definition->getPropertyDefinition($item_definition->getMainPropertyName()); + + // If this is an entity reference then we expect the target type as + // context. + if ($type_definition instanceof DataReferenceDefinitionInterface) { + $type_definition->getTargetDefinition(); + } + $type = $type_definition->getDataType(); + $is_bundle = ($field_name == $bundle_key); $multiple = ($definition->getCardinality() === 1) ? FALSE : TRUE; - $context_definition = ContextDefinition::create($definition->getType()) + + $context_definition = ContextDefinition::create($type) ->setLabel($definition->getLabel()) ->setRequired($is_bundle) ->setMultiple($multiple) diff --git a/src/Plugin/RulesAction/EntitySave.php b/src/Plugin/RulesAction/EntitySave.php index e2743959..1209ad14 100644 --- a/src/Plugin/RulesAction/EntitySave.php +++ b/src/Plugin/RulesAction/EntitySave.php @@ -25,7 +25,7 @@ * "immediate" = @ContextDefinition("boolean", * label = @Translation("Force saving immediately"), * description = @Translation("Usually saving is postponed till the end of the evaluation, so that multiple saves can be fold into one. If this set, saving is forced to happen immediately."), - * default_value = NULL, + * default_value = FALSE, * required = FALSE * ) * } diff --git a/src/Plugin/RulesAction/RulesComponentAction.php b/src/Plugin/RulesAction/RulesComponentAction.php new file mode 100644 index 00000000..491e688b --- /dev/null +++ b/src/Plugin/RulesAction/RulesComponentAction.php @@ -0,0 +1,129 @@ +storage = $storage; + $this->componentId = $plugin_definition['component_id']; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { + return new static( + $configuration, + $plugin_id, + $plugin_definition, + $container->get('entity_type.manager')->getStorage('rules_component') + ); + } + + /** + * {@inheritdoc} + */ + public function execute() { + $rules_config = $this->storage->load($this->componentId); + + // Setup an isolated execution state for this expression and pass on the + // necessary context. + $rules_component = $rules_config->getComponent(); + foreach ($this->getContexts() as $context_name => $context) { + // Pass through the already existing typed data objects to avoid creating + // them from scratch. + $rules_component->getState()->setVariableData($context_name, $context->getContextData()); + } + + // We don't use RulesComponent::execute() here since we don't want to + // auto-save immediately. + $state = $rules_component->getState(); + $expression = $rules_component->getExpression(); + $expression->executeWithState($state); + + // Postpone auto-saving to the parent expression triggering this action. + foreach ($state->getAutoSaveSelectors() as $selector) { + $parts = explode('.', $selector); + $context_name = reset($parts); + if (array_key_exists($context_name, $this->context)) { + $this->saveLater[] = $context_name; + } + else { + // Otherwise we need to save here since it will not happen in the parent + // execution. + $typed_data = $state->fetchDataByPropertyPath($selector); + // Things that can be saved must have a save() method, right? + // Saving is always done at the root of the typed data tree, for example + // on the entity level. + $typed_data->getRoot()->getValue()->save(); + } + } + + foreach ($this->getProvidedContextDefinitions() as $name => $definition) { + $this->setProvidedValue($name, $state->getVariable($name)); + } + } + + /** + * {@inheritdoc} + */ + public function autoSaveContext() { + return $this->saveLater; + } + +} diff --git a/src/Plugin/RulesAction/RulesComponentActionDeriver.php b/src/Plugin/RulesAction/RulesComponentActionDeriver.php new file mode 100644 index 00000000..85329f77 --- /dev/null +++ b/src/Plugin/RulesAction/RulesComponentActionDeriver.php @@ -0,0 +1,82 @@ +storage = $storage; + $this->expressionManager = $expression_manager; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container, $base_plugin_id) { + return new static( + $container->get('entity_type.manager')->getStorage('rules_component'), + $container->get('plugin.manager.rules_expression') + ); + } + + /** + * {@inheritdoc} + */ + public function getDerivativeDefinitions($base_plugin_definition) { + $rules_components = $this->storage->loadMultiple(); + foreach ($rules_components as $rules_component) { + + $component_config = $rules_component->get('component'); + $expression_definition = $this->expressionManager->getDefinition($component_config['expression']['id']); + + $this->derivatives[$rules_component->id()] = [ + 'label' => $this->t('@expression_type: @label', [ + '@expression_type' => $expression_definition['label'], + '@label' => $rules_component->label(), + ]), + 'category' => $this->t('Components'), + 'component_id' => $rules_component->id(), + 'context' => $rules_component->getContextDefinitions(), + 'provides' => $rules_component->getProvidedContextDefinitions(), + ] + $base_plugin_definition; + } + + return $this->derivatives; + } + +} diff --git a/tests/modules/rules_test_default_component/config/install/rules.component.rules_test_default_component.yml b/tests/modules/rules_test_default_component/config/install/rules.component.rules_test_default_component.yml index e0c9ca2d..91bff11f 100644 --- a/tests/modules/rules_test_default_component/config/install/rules.component.rules_test_default_component.yml +++ b/tests/modules/rules_test_default_component/config/install/rules.component.rules_test_default_component.yml @@ -13,8 +13,11 @@ component: type: 'entity:user' label: User description: 'The user whose mail address to print.' - provided_context: - - concatenated + provided_context_definitions: + concatenated: + type: 'string' + label: Concatenated text + description: 'The concatenated text.' expression: id: rules_rule conditions: diff --git a/tests/src/Integration/Action/EntityCreateTest.php b/tests/src/Integration/Action/EntityCreateTest.php index ef101874..41a435bf 100644 --- a/tests/src/Integration/Action/EntityCreateTest.php +++ b/tests/src/Integration/Action/EntityCreateTest.php @@ -9,8 +9,11 @@ use Drupal\Core\Entity\EntityStorageBase; use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\Core\Field\TypedData\FieldItemDataDefinition; +use Drupal\Core\TypedData\DataDefinitionInterface; use Drupal\rules\Context\ContextDefinition; use Drupal\Tests\rules\Integration\RulesEntityIntegrationTestBase; +use Prophecy\Argument; /** * @coversDefaultClass \Drupal\rules\Plugin\RulesAction\EntityCreate @@ -43,23 +46,33 @@ public function setUp() { $bundle_field_definition_optional = $this->prophesize(BaseFieldDefinition::class); $bundle_field_definition_required = $this->prophesize(BaseFieldDefinition::class); + $property_definition = $this->prophesize(DataDefinitionInterface::class); + $property_definition->getDataType()->willReturn('string'); + + $item_definition = $this->prophesize(FieldItemDataDefinition::class); + $item_definition->getPropertyDefinition(Argument::any()) + ->willReturn($property_definition->reveal()); + $item_definition->getMainPropertyName()->willReturn('value'); + // The next methods are mocked because EntityCreateDeriver executes them, // and the mocked field definition is instantiated without the necessary // information. + $bundle_field_definition->getItemDefinition() + ->willReturn($item_definition->reveal()); $bundle_field_definition->getCardinality()->willReturn(1) ->shouldBeCalledTimes(1); - $bundle_field_definition->getType()->willReturn('string') - ->shouldBeCalledTimes(1); + $bundle_field_definition->getType()->willReturn('string'); $bundle_field_definition->getLabel()->willReturn('Bundle') ->shouldBeCalledTimes(1); $bundle_field_definition->getDescription() ->willReturn('Bundle mock description') ->shouldBeCalledTimes(1); + $bundle_field_definition_required->getItemDefinition() + ->willReturn($item_definition->reveal()); $bundle_field_definition_required->getCardinality()->willReturn(1) ->shouldBeCalledTimes(1); - $bundle_field_definition_required->getType()->willReturn('string') - ->shouldBeCalledTimes(1); + $bundle_field_definition_required->getType()->willReturn('string'); $bundle_field_definition_required->getLabel()->willReturn('Required field') ->shouldBeCalledTimes(1); $bundle_field_definition_required->getDescription() diff --git a/tests/src/Integration/Action/RulesComponentActionTest.php b/tests/src/Integration/Action/RulesComponentActionTest.php new file mode 100644 index 00000000..67a7f1c8 --- /dev/null +++ b/tests/src/Integration/Action/RulesComponentActionTest.php @@ -0,0 +1,198 @@ +rulesExpressionManager->createRule(); + + $rules_config = new RulesComponentConfig([ + 'id' => 'test_rule', + 'label' => 'Test rule', + ], 'rules_component'); + $rules_config->setExpression($rule); + + $this->prophesizeStorage([$rules_config]); + + $definition = $this->actionManager->getDefinition('rules_component:test_rule'); + $this->assertEquals('Components', $definition['category']); + $this->assertEquals('Rule: Test rule', (string) $definition['label']); + } + + /** + * Tests that the execution of the action invokes the Rules component. + */ + public function testExecute() { + // Set up a rules component that will just save an entity. + $nested_rule = $this->rulesExpressionManager->createRule(); + $nested_rule->addAction('rules_entity_save', ContextConfig::create() + ->map('entity', 'entity') + ); + + $rules_config = new RulesComponentConfig([ + 'id' => 'test_rule', + 'label' => 'Test rule', + ], 'rules_component'); + $rules_config->setExpression($nested_rule); + $rules_config->setContextDefinitions(['entity' => ContextDefinition::create('entity')]); + + $this->prophesizeStorage([$rules_config]); + + // Invoke the rules component in another rule. + $rule = $this->rulesExpressionManager->createRule(); + $rule->addAction('rules_component:test_rule', ContextConfig::create() + ->map('entity', 'entity') + ); + + // The call to save the entity means that the action was executed. + $entity = $this->prophesizeEntity(EntityInterface::class); + $entity->save()->shouldBeCalledTimes(1); + + RulesComponent::create($rule) + ->addContextDefinition('entity', ContextDefinition::create('entity')) + ->setContextValue('entity', $entity->reveal()) + ->execute(); + } + + /** + * Tests that context definitions are available on the derived action. + */ + public function testContextDefinitions() { + $rule = $this->rulesExpressionManager->createRule(); + $rule + ->addAction('rules_entity_save', ContextConfig::create() + ->map('entity', 'entity') + ) + ->addAction('rules_test_string', ContextConfig::create() + ->setValue('text', 'x') + ); + + $rules_config = new RulesComponentConfig([ + 'id' => 'test_rule', + 'label' => 'Test rule', + ], 'rules_component'); + $rules_config->setExpression($rule); + + $context_definitions = ['entity' => ContextDefinition::create('entity')]; + $rules_config->setContextDefinitions($context_definitions); + $provided_definitions = ['concatenated' => ContextDefinition::create('string')]; + $rules_config->setProvidedContextDefinitions($provided_definitions); + + $this->prophesizeStorage([$rules_config]); + + $definition = $this->actionManager->getDefinition('rules_component:test_rule'); + $this->assertEquals($context_definitions, $definition['context']); + $this->assertEquals($provided_definitions, $definition['provides']); + } + + /** + * Tests that a rules component in an action can also provide variables. + */ + public function testExecutionProvidedVariables() { + // Create a rule that produces a provided string variable. + $nested_rule = $this->rulesExpressionManager->createRule(); + $nested_rule->addAction('rules_test_string', ContextConfig::create() + ->setValue('text', 'x') + ); + + $rules_config = new RulesComponentConfig([ + 'id' => 'test_rule', + 'label' => 'Test rule', + ], 'rules_component'); + $rules_config->setExpression($nested_rule); + $rules_config->setProvidedContextDefinitions(['concatenated' => ContextDefinition::create('string')]); + + $this->prophesizeStorage([$rules_config]); + + // Invoke the rules component in another rule. + $rule = $this->rulesExpressionManager->createRule(); + $rule->addAction('rules_component:test_rule'); + + $result = RulesComponent::create($rule) + ->provideContext('concatenated') + ->execute(); + + $this->assertEquals('xx', $result['concatenated']); + } + + /** + * Tests that auto saving is only triggered once with nested components. + */ + public function testAutosaveOnlyOnce() { + $entity = $this->prophesizeEntity(EntityInterface::class); + + $nested_rule = $this->rulesExpressionManager->createRule(); + $nested_rule->addAction('rules_entity_save', ContextConfig::create() + ->map('entity', 'entity') + ); + + $rules_config = new RulesComponentConfig([ + 'id' => 'test_rule', + 'label' => 'Test rule', + ], 'rules_component'); + $rules_config->setExpression($nested_rule); + $rules_config->setContextDefinitions(['entity' => ContextDefinition::create('entity')]); + + $this->prophesizeStorage([$rules_config]); + + // Create a rule with a nested rule. Overall there are 2 actions to set the + // entity then. + $rule = $this->rulesExpressionManager->createRule(); + $rule->addAction('rules_component:test_rule', ContextConfig::create() + ->map('entity', 'entity') + ); + $rule->addAction('rules_entity_save', ContextConfig::create() + ->map('entity', 'entity') + ); + + // Auto-saving should only be triggered once on the entity. + $entity->save()->shouldBeCalledTimes(1); + + RulesComponent::create($rule) + ->addContextDefinition('entity', ContextDefinition::create('entity')) + ->setContextValue('entity', $entity->reveal()) + ->execute(); + } + + /** + * Prepares a mocked entity storage that returns the provided Rules configs. + * + * @param RulesComponentConfig[] $rules_configs + * The Rules componentn config entities that should be returned. + */ + protected function prophesizeStorage($rules_configs) { + $storage = $this->prophesize(ConfigEntityStorageInterface::class); + $keyed_configs = []; + + foreach ($rules_configs as $rules_config) { + $keyed_configs[$rules_config->id()] = $rules_config; + $storage->load($rules_config->id())->willReturn($rules_config); + } + + $storage->loadMultiple(NULL)->willReturn($keyed_configs); + $this->entityTypeManager->getStorage('rules_component')->willReturn($storage->reveal()); + } + +} diff --git a/tests/src/Integration/RulesIntegrationTestBase.php b/tests/src/Integration/RulesIntegrationTestBase.php index dccd21c7..b31b72ee 100644 --- a/tests/src/Integration/RulesIntegrationTestBase.php +++ b/tests/src/Integration/RulesIntegrationTestBase.php @@ -11,6 +11,7 @@ use Drupal\Core\Cache\NullBackend; use Drupal\Core\DependencyInjection\ClassResolverInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\Config\Entity\ConfigEntityStorageInterface; use Drupal\Core\Entity\EntityFieldManagerInterface; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Entity\EntityTypeBundleInfoInterface; @@ -204,6 +205,11 @@ public function setUp() { $this->entityTypeManager = $this->prophesize(EntityTypeManagerInterface::class); $this->entityTypeManager->getDefinitions()->willReturn([]); + // Setup a rules_component storage mock which returns nothing by default. + $storage = $this->prophesize(ConfigEntityStorageInterface::class); + $storage->loadMultiple(NULL)->willReturn([]); + $this->entityTypeManager->getStorage('rules_component')->willReturn($storage->reveal()); + $this->entityFieldManager = $this->prophesize(EntityFieldManagerInterface::class); $this->entityFieldManager->getBaseFieldDefinitions()->willReturn([]); diff --git a/tests/src/Kernel/CoreIntegrationTest.php b/tests/src/Kernel/CoreIntegrationTest.php index 64293ab3..bb9c3365 100644 --- a/tests/src/Kernel/CoreIntegrationTest.php +++ b/tests/src/Kernel/CoreIntegrationTest.php @@ -7,9 +7,11 @@ namespace Drupal\Tests\rules\Kernel; +use Drupal\node\Entity\Node; use Drupal\rules\Context\ContextConfig; use Drupal\rules\Context\ContextDefinition; use Drupal\rules\Engine\RulesComponent; +use Drupal\rules\Entity\RulesComponentConfig; use Drupal\user\Entity\User; /** @@ -244,6 +246,45 @@ public function testDataSetEntities() { $this->assertNotNull($node->id(), 'Node ID is set, which means that the node has been auto-saved.'); } + /** + * Tests that auto saving in a component executed as action works. + */ + public function testComponentActionAutoSave() { + $entity_type_manager = $this->container->get('entity_type.manager'); + $entity_type_manager->getStorage('node_type') + ->create(['type' => 'page']) + ->save(); + + $nested_rule = $this->expressionManager->createRule(); + // Create a node entity with the action. + $nested_rule->addAction('rules_entity_create:node', ContextConfig::create() + ->setValue('type', 'page') + ); + // Set the title of the new node so that it is marked for auto-saving. + $nested_rule->addAction('rules_data_set', ContextConfig::create() + ->map('data', 'entity.title') + ->setValue('value', 'new title') + ); + + $rules_config = new RulesComponentConfig([ + 'id' => 'test_rule', + 'label' => 'Test rule', + ], 'rules_component'); + $rules_config->setExpression($nested_rule); + $rules_config->save(); + + // Invoke the rules component in another rule. + $rule = $this->expressionManager->createRule(); + $rule->addAction('rules_component:test_rule'); + + RulesComponent::create($rule)->execute(); + + $nodes = Node::loadMultiple(); + $node = reset($nodes); + $this->assertEquals('new title', $node->getTitle()); + $this->assertNotNull($node->id(), 'Node ID is set, which means that the node has been auto-saved.'); + } + /** * Tests using global context. */ From 86476dfe92f6a84709ce649768bda0f807018609 Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 5 Mar 2016 20:29:05 +0100 Subject: [PATCH 52/57] Issue #2681733 by fago: Allow context-aware plugins to assert metadata. --- src/Context/ContextAwarePluginInterface.php | 22 +++++++++++++++++++++ src/Core/RulesActionBase.php | 7 +++++++ src/Core/RulesConditionBase.php | 7 +++++++ src/Plugin/Condition/EntityIsOfBundle.php | 13 +++++++++++- 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/Context/ContextAwarePluginInterface.php b/src/Context/ContextAwarePluginInterface.php index c9a5bb19..c70bbb3a 100644 --- a/src/Context/ContextAwarePluginInterface.php +++ b/src/Context/ContextAwarePluginInterface.php @@ -21,10 +21,32 @@ interface ContextAwarePluginInterface extends CoreContextAwarePluginInterface { * already available upon which the definition of subsequent or provided * context can be refined. * + * Note that context gets refined at configuration and execution time of the + * plugin. + * * @param \Drupal\Core\TypedData\DataDefinitionInterface[] $selected_data * An array of data definitions for context that is mapped using a data * selector, keyed by context name. */ public function refineContextDefinitions(array $selected_data); + /** + * Asserts additional metadata for the selected data. + * + * Allows the plugin to assert additional metadata that is in place when the + * plugin has been successfully executed. A typical use-case would be + * asserting the node type for a "Node is of type" condition. By doing so, + * sub-sequent executed plugins are aware of the metadata and can build upon + * it. + * + * Note that metadata is only asserted on configuration time. The plugin has + * to ensure that the run-time data matches the asserted configuration if it + * has been executed successfully. + * + * @param \Drupal\Core\TypedData\DataDefinitionInterface[] $selected_data + * An array of data definitions for context that is mapped using a data + * selector, keyed by context name. + */ + public function assertMetadata(array $selected_data); + } diff --git a/src/Core/RulesActionBase.php b/src/Core/RulesActionBase.php index acb0e0fd..83aaff73 100644 --- a/src/Core/RulesActionBase.php +++ b/src/Core/RulesActionBase.php @@ -53,6 +53,13 @@ public function refineContextDefinitions(array $selected_data) { // Do not refine anything by default. } + /** + * {@inheritdoc} + */ + public function assertMetadata(array $selected_data) { + // Nothing to assert by default. + } + /** * {@inheritdoc} */ diff --git a/src/Core/RulesConditionBase.php b/src/Core/RulesConditionBase.php index 5bf1d577..bdda099b 100644 --- a/src/Core/RulesConditionBase.php +++ b/src/Core/RulesConditionBase.php @@ -29,6 +29,13 @@ public function refineContextDefinitions(array $selected_data) { // Do not refine anything by default. } + /** + * {@inheritdoc} + */ + public function assertMetadata(array $selected_data) { + // Nothing to assert by default. + } + /** * {@inheritdoc} */ diff --git a/src/Plugin/Condition/EntityIsOfBundle.php b/src/Plugin/Condition/EntityIsOfBundle.php index a8393e02..3bf52fe4 100644 --- a/src/Plugin/Condition/EntityIsOfBundle.php +++ b/src/Plugin/Condition/EntityIsOfBundle.php @@ -20,7 +20,8 @@ * context = { * "entity" = @ContextDefinition("entity", * label = @Translation("Entity"), - * description = @Translation("Specifies the entity for which to evaluate the condition.") + * description = @Translation("Specifies the entity for which to evaluate the condition."), + * assignment_restriction = "selector", * ), * "type" = @ContextDefinition("string", * label = @Translation("Type"), @@ -59,4 +60,14 @@ protected function doEvaluate(EntityInterface $entity, $type, $bundle) { return $entity_bundle == $bundle && $entity_type == $type; } + /** + * {@inheritdoc} + */ + public function assertMetadata(array $selected_data) { + // Assert the checked bundle. + if (isset($selected_data['entity']) && $bundle = $this->getContextValue('bundle')) { + $selected_data['entity']->setBundles($this->getContextValue('bundle')); + } + } + } From 7d22771f11416e30e48a81f2d815e336832397dd Mon Sep 17 00:00:00 2001 From: fago Date: Sat, 5 Mar 2016 21:32:38 +0100 Subject: [PATCH 53/57] Issue #2681733 by fago: Add metadata assertion support to expressions. --- src/Context/ContextHandlerTrait.php | 58 ++++++--- src/Engine/ActionExpressionContainer.php | 83 +------------ src/Engine/ConditionExpressionContainer.php | 83 +------------ src/Engine/ExpressionContainerBase.php | 110 ++++++++++++++++++ src/Engine/ExpressionInterface.php | 14 ++- src/Plugin/RulesExpression/ActionSet.php | 7 ++ src/Plugin/RulesExpression/Rule.php | 15 ++- src/Plugin/RulesExpression/RulesAction.php | 9 +- src/Plugin/RulesExpression/RulesAnd.php | 9 ++ src/Plugin/RulesExpression/RulesCondition.php | 9 +- src/Plugin/RulesExpression/RulesLoop.php | 17 ++- src/Plugin/RulesExpression/RulesOr.php | 9 ++ 12 files changed, 227 insertions(+), 196 deletions(-) create mode 100644 src/Engine/ExpressionContainerBase.php diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index 764f6776..c6268faa 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -132,6 +132,33 @@ protected function prepareContextWithMetadata(CoreContextAwarePluginInterface $p } } + if ($plugin instanceof ContextAwarePluginInterface) { + $selected_data = $this->getSelectedData($metadata_state); + // Getting context values may lead to undocumented exceptions if context + // is not set right now. So catch those exceptions. + // @todo: Remove ones https://www.drupal.org/node/2677162 got fixed. + try { + $plugin->refineContextDefinitions($selected_data); + } + catch (ContextException $e) { + if (strpos($e->getMessage(), 'context is required') === FALSE) { + throw $e; + } + } + } + } + + /** + * Gets definitions of all selected data at configuration time. + * + * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state + * The metadata state. + * + * @return \Drupal\Core\TypedData\DataDefinitionInterface[] + * An array of data definitions for context that is mapped using a data + * selector, keyed by context name. + */ + protected function getSelectedData(ExecutionMetadataStateInterface $metadata_state) { $selected_data = []; // Collected the definitions of selected data for refining context // definitions. @@ -149,20 +176,7 @@ protected function prepareContextWithMetadata(CoreContextAwarePluginInterface $p } } } - - if ($plugin instanceof ContextAwarePluginInterface) { - // Getting context values may lead to undocumented exceptions if context - // is not set right now. So catch those exceptions. - // @todo: Remove ones https://www.drupal.org/node/2677162 got fixed. - try { - $plugin->refineContextDefinitions($selected_data); - } - catch (ContextException $e) { - if (strpos($e->getMessage(), 'context is required') === FALSE) { - throw $e; - } - } - } + return $selected_data; } /** @@ -243,6 +257,22 @@ protected function addProvidedContextDefinitions(CoreContextAwarePluginInterface } } + /** + * Asserts additional metadata. + * + * @param CoreContextAwarePluginInterface $plugin + * The context aware plugin. + * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state + * The execution metadata state. + */ + protected function assertMetadata(CoreContextAwarePluginInterface $plugin, ExecutionMetadataStateInterface $metadata_state) { + // If the plugin does not implement the Rules-enhanced interface, skip this. + if (!$plugin instanceof ContextAwarePluginInterface) { + return; + } + $plugin->assertMetadata($this->getSelectedData($metadata_state)); + } + /** * Process data context on the plugin, usually before it gets executed. * diff --git a/src/Engine/ActionExpressionContainer.php b/src/Engine/ActionExpressionContainer.php index afe903fb..fac597fe 100644 --- a/src/Engine/ActionExpressionContainer.php +++ b/src/Engine/ActionExpressionContainer.php @@ -10,12 +10,11 @@ use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\rules\Context\ContextConfig; use Drupal\rules\Exception\InvalidExpressionException; -use Symfony\Component\DependencyInjection\ContainerInterface; /** * Container for actions. */ -abstract class ActionExpressionContainer extends ExpressionBase implements ActionExpressionContainerInterface, ContainerFactoryPluginInterface { +abstract class ActionExpressionContainer extends ExpressionContainerBase implements ActionExpressionContainerInterface, ContainerFactoryPluginInterface { /** * List of actions that will be executed. @@ -24,13 +23,6 @@ abstract class ActionExpressionContainer extends ExpressionBase implements Actio */ protected $actions = []; - /** - * The expression manager. - * - * @var \Drupal\rules\Engine\ExpressionManagerInterface - */ - protected $expressionManager; - /** * Constructor. * @@ -54,18 +46,6 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition } } - /** - * {@inheritdoc} - */ - public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { - return new static( - $configuration, - $plugin_id, - $plugin_definition, - $container->get('plugin.manager.rules_expression') - ); - } - /** * {@inheritdoc} */ @@ -80,15 +60,6 @@ public function addExpressionObject(ExpressionInterface $expression) { return $this; } - /** - * {@inheritdoc} - */ - public function addExpression($plugin_id, ContextConfig $config = NULL) { - return $this->addExpressionObject( - $this->expressionManager->createInstance($plugin_id, $config ? $config->toArray() : []) - ); - } - /** * {@inheritdoc} */ @@ -167,56 +138,4 @@ public function deleteExpression($uuid) { return FALSE; } - /** - * {@inheritdoc} - */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { - $violation_list = new IntegrityViolationList(); - $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); - foreach ($this->actions as $action) { - $action_violations = $action->checkIntegrity($metadata_state); - $violation_list->addAll($action_violations); - } - $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); - return $violation_list; - } - - /** - * {@inheritdoc} - */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - if ($until && $this->getUuid() === $until->getUuid()) { - return TRUE; - } - $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); - foreach ($this->actions as $action) { - $found = $action->prepareExecutionMetadataState($metadata_state, $until); - // If the expression was found, we need to stop. - if ($found) { - return TRUE; - } - } - $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); - } - - /** - * Prepares execution metadata state before traversing through children. - * - * @see ::prepareExecutionMetadataState() - * @see ::checkIntegrity() - */ - protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) { - // Any pre-traversal preparations need to be added here. - } - - /** - * Prepares execution metadata state after traversing through children. - * - * @see ::prepareExecutionMetadataState() - * @see ::checkIntegrity() - */ - protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { - // Any post-traversal preparations need to be added here. - } - } diff --git a/src/Engine/ConditionExpressionContainer.php b/src/Engine/ConditionExpressionContainer.php index 2c7ab340..f23ac2da 100644 --- a/src/Engine/ConditionExpressionContainer.php +++ b/src/Engine/ConditionExpressionContainer.php @@ -10,12 +10,11 @@ use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\rules\Context\ContextConfig; use Drupal\rules\Exception\InvalidExpressionException; -use Symfony\Component\DependencyInjection\ContainerInterface; /** * Container for conditions. */ -abstract class ConditionExpressionContainer extends ExpressionBase implements ConditionExpressionContainerInterface, ContainerFactoryPluginInterface { +abstract class ConditionExpressionContainer extends ExpressionContainerBase implements ConditionExpressionContainerInterface, ContainerFactoryPluginInterface { /** * List of conditions that are evaluated. @@ -24,13 +23,6 @@ abstract class ConditionExpressionContainer extends ExpressionBase implements Co */ protected $conditions = []; - /** - * The expression manager. - * - * @var \Drupal\rules\Engine\ExpressionManagerInterface - */ - protected $expressionManager; - /** * Constructs a new class instance. * @@ -54,18 +46,6 @@ public function __construct(array $configuration, $plugin_id, array $plugin_defi } } - /** - * {@inheritdoc} - */ - public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { - return new static( - $configuration, - $plugin_id, - $plugin_definition, - $container->get('plugin.manager.rules_expression') - ); - } - /** * {@inheritdoc} */ @@ -80,15 +60,6 @@ public function addExpressionObject(ExpressionInterface $expression) { return $this; } - /** - * {@inheritdoc} - */ - public function addExpression($plugin_id, ContextConfig $config = NULL) { - return $this->addExpressionObject( - $this->expressionManager->createInstance($plugin_id, $config ? $config->toArray() : []) - ); - } - /** * {@inheritdoc} */ @@ -195,56 +166,4 @@ public function deleteExpression($uuid) { return FALSE; } - /** - * {@inheritdoc} - */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { - $violation_list = new IntegrityViolationList(); - $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); - foreach ($this->conditions as $condition) { - $condition_violations = $condition->checkIntegrity($metadata_state); - $violation_list->addAll($condition_violations); - } - $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); - return $violation_list; - } - - /** - * {@inheritdoc} - */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - if ($until && $this->getUuid() === $until->getUuid()) { - return TRUE; - } - $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); - foreach ($this->conditions as $condition) { - $found = $condition->prepareExecutionMetadataState($metadata_state, $until); - // If the expression was found, we need to stop. - if ($found) { - return TRUE; - } - } - $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); - } - - /** - * Prepares execution metadata state before traversing through children. - * - * @see ::prepareExecutionMetadataState() - * @see ::checkIntegrity() - */ - protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) { - // Any pre-traversal preparations need to be added here. - } - - /** - * Prepares execution metadata state after traversing through children. - * - * @see ::prepareExecutionMetadataState() - * @see ::checkIntegrity() - */ - protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { - // Any post-traversal preparations need to be added here. - } - } diff --git a/src/Engine/ExpressionContainerBase.php b/src/Engine/ExpressionContainerBase.php new file mode 100644 index 00000000..f652e2b3 --- /dev/null +++ b/src/Engine/ExpressionContainerBase.php @@ -0,0 +1,110 @@ +get('plugin.manager.rules_expression') + ); + } + + /** + * {@inheritdoc} + */ + public function addExpression($plugin_id, ContextConfig $config = NULL) { + return $this->addExpressionObject( + $this->expressionManager->createInstance($plugin_id, $config ? $config->toArray() : []) + ); + } + + /** + * Determines whether child-expressions are allowed to assert metadata. + * + * @return bool + * Whether child-expressions are allowed to assert metadata. + * + * @see \Drupal\rules\Engine\ExpressionInterface::prepareExecutionMetadataState() + */ + abstract protected function allowsMetadataAssertions(); + + /** + * {@inheritdoc} + */ + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state, $apply_assertions = TRUE) { + $violation_list = new IntegrityViolationList(); + $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); + $apply_assertions = $apply_assertions && $this->allowsMetadataAssertions(); + foreach ($this as $child_expression) { + $child_violations = $child_expression->checkIntegrity($metadata_state, $apply_assertions); + $violation_list->addAll($child_violations); + } + $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); + return $violation_list; + } + + /** + * {@inheritdoc} + */ + public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL, $apply_assertions = TRUE) { + if ($until && $this->getUuid() === $until->getUuid()) { + return TRUE; + } + $this->prepareExecutionMetadataStateBeforeTraversal($metadata_state); + $apply_assertions = $apply_assertions && $this->allowsMetadataAssertions(); + foreach ($this as $child_expression) { + $found = $child_expression->prepareExecutionMetadataState($metadata_state, $until, $apply_assertions); + // If the expression was found, we need to stop. + if ($found) { + return TRUE; + } + } + $this->prepareExecutionMetadataStateAfterTraversal($metadata_state); + } + + /** + * Prepares execution metadata state before traversing through children. + * + * @see ::prepareExecutionMetadataState() + * @see ::checkIntegrity() + */ + protected function prepareExecutionMetadataStateBeforeTraversal(ExecutionMetadataStateInterface $metadata_state) { + // Any pre-traversal preparations need to be added here. + } + + /** + * Prepares execution metadata state after traversing through children. + * + * @see ::prepareExecutionMetadataState() + * @see ::checkIntegrity() + */ + protected function prepareExecutionMetadataStateAfterTraversal(ExecutionMetadataStateInterface $metadata_state) { + // Any post-traversal preparations need to be added here. + } + +} diff --git a/src/Engine/ExpressionInterface.php b/src/Engine/ExpressionInterface.php index 43577a5d..7c2be02b 100644 --- a/src/Engine/ExpressionInterface.php +++ b/src/Engine/ExpressionInterface.php @@ -99,13 +99,16 @@ public function setUuid($uuid); * @param \Drupal\rules\Engine\ExecutionMetadataStateInterface $metadata_state * The execution metadata state, prepared until right before this * expression. + * @param bool $apply_assertions + * (optional) Whether to apply metadata assertions while preparing the + * execution metadata state. Defaults to TRUE. * * @return \Drupal\rules\Engine\IntegrityViolationList * A list object containing \Drupal\rules\Engine\IntegrityViolation objects. * * @see ::prepareExecutionMetadataState() */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state); + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state, $apply_assertions = TRUE); /** * Prepares the execution metadata state by adding metadata to it. @@ -131,11 +134,18 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state); * (optional) The expression at which metadata preparation should be * stopped. The preparation of the state will be stopped right before that * expression. + * @param bool $apply_assertions + * (optional) Whether to apply metadata assertions while preparing the + * execution metadata state. Defaults to TRUE. Metadata assertions should + * be only applied if the expression's execution is required for sub-sequent + * expressions being executed. For example, if a condition is optional as + * it is part of a logical OR expression, its assertions may not be applied. + * Defaults to TRUE. * * @return true|null * True if the metadata has been prepared and the $until expression was * found in the tree. Null otherwise. */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL); + public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL, $apply_assertions = TRUE); } diff --git a/src/Plugin/RulesExpression/ActionSet.php b/src/Plugin/RulesExpression/ActionSet.php index a8470d56..17fef135 100644 --- a/src/Plugin/RulesExpression/ActionSet.php +++ b/src/Plugin/RulesExpression/ActionSet.php @@ -21,6 +21,13 @@ */ class ActionSet extends ActionExpressionContainer { + /** + * {@inheritdoc} + */ + protected function allowsMetadataAssertions() { + return TRUE; + } + /** * {@inheritdoc} */ diff --git a/src/Plugin/RulesExpression/Rule.php b/src/Plugin/RulesExpression/Rule.php index 51f1db5b..746ea054 100644 --- a/src/Plugin/RulesExpression/Rule.php +++ b/src/Plugin/RulesExpression/Rule.php @@ -215,21 +215,24 @@ public function deleteExpression($uuid) { /** * {@inheritdoc} */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { - $violation_list = $this->conditions->checkIntegrity($metadata_state); - $violation_list->addAll($this->actions->checkIntegrity($metadata_state)); + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state, $apply_assertions = TRUE) { + $violation_list = $this->conditions->checkIntegrity($metadata_state, $apply_assertions); + $violation_list->addAll($this->actions->checkIntegrity($metadata_state, $apply_assertions)); return $violation_list; } /** * {@inheritdoc} */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { - $found = $this->conditions->prepareExecutionMetadataState($metadata_state, $until); + public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL, $apply_assertions = TRUE) { + // @todo: If the rule is nested, we may not pass assertions to following + // expressions as we do not know whether the rule fires at all. Should we + // clone the metadata state to ensure modifications stay local? + $found = $this->conditions->prepareExecutionMetadataState($metadata_state, $until, $apply_assertions); if ($found) { return TRUE; } - return $this->actions->prepareExecutionMetadataState($metadata_state, $until); + return $this->actions->prepareExecutionMetadataState($metadata_state, $until, $apply_assertions); } /** diff --git a/src/Plugin/RulesExpression/RulesAction.php b/src/Plugin/RulesExpression/RulesAction.php index bc0896c7..563f179f 100644 --- a/src/Plugin/RulesExpression/RulesAction.php +++ b/src/Plugin/RulesExpression/RulesAction.php @@ -132,7 +132,7 @@ public function getFormHandler() { /** * {@inheritdoc} */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state, $apply_assertions = TRUE) { $violation_list = new IntegrityViolationList(); if (empty($this->configuration['action_id'])) { $violation_list->addViolationWithMessage($this->t('Action plugin ID is missing'), $this->getUuid()); @@ -151,14 +151,14 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) // context definition changes are respected while checking. $this->prepareContextWithMetadata($action, $metadata_state); $result = $this->checkContextConfigIntegrity($action, $metadata_state); - $this->prepareExecutionMetadataState($metadata_state); + $this->prepareExecutionMetadataState($metadata_state, NULL, $apply_assertions); return $result; } /** * {@inheritdoc} */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { + public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL, $apply_assertions = TRUE) { if ($until && $this->getUuid() === $until->getUuid()) { return TRUE; } @@ -167,6 +167,9 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m // of provided context are respected. $this->prepareContextWithMetadata($action, $metadata_state); $this->addProvidedContextDefinitions($action, $metadata_state); + if ($apply_assertions) { + $this->assertMetadata($action, $metadata_state); + } } } diff --git a/src/Plugin/RulesExpression/RulesAnd.php b/src/Plugin/RulesExpression/RulesAnd.php index 06444105..82efd8f5 100644 --- a/src/Plugin/RulesExpression/RulesAnd.php +++ b/src/Plugin/RulesExpression/RulesAnd.php @@ -47,4 +47,13 @@ public function evaluate(ExecutionStateInterface $state) { return !empty($this->conditions); } + /** + * {@inheritdoc} + */ + protected function allowsMetadataAssertions() { + // If the AND is not negated, all child-expressions must be executed - thus + // assertions can be added it. + return !$this->isNegated(); + } + } diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index 7ff78d02..a4100956 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -155,7 +155,7 @@ public function getFormHandler() { /** * {@inheritdoc} */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state, $apply_assertions = TRUE) { $violation_list = new IntegrityViolationList(); if (empty($this->configuration['condition_id'])) { $violation_list->addViolationWithMessage($this->t('Condition plugin ID is missing'), $this->getUuid()); @@ -175,14 +175,14 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) // context definition changes are respected while checking. $this->prepareContextWithMetadata($condition, $metadata_state); $result = $this->checkContextConfigIntegrity($condition, $metadata_state); - $this->prepareExecutionMetadataState($metadata_state); + $this->prepareExecutionMetadataState($metadata_state, NULL, $apply_assertions); return $result; } /** * {@inheritdoc} */ - public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL) { + public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $metadata_state, ExpressionInterface $until = NULL, $apply_assertions = TRUE) { if ($until && $this->getUuid() === $until->getUuid()) { return TRUE; } @@ -193,6 +193,9 @@ public function prepareExecutionMetadataState(ExecutionMetadataStateInterface $m // of provided context are respected. $this->prepareContextWithMetadata($condition, $metadata_state); $this->addProvidedContextDefinitions($condition, $metadata_state); + if ($apply_assertions && !$this->isNegated()) { + $this->assertMetadata($condition, $metadata_state); + } } } diff --git a/src/Plugin/RulesExpression/RulesLoop.php b/src/Plugin/RulesExpression/RulesLoop.php index e80ce3f5..01b275d6 100644 --- a/src/Plugin/RulesExpression/RulesLoop.php +++ b/src/Plugin/RulesExpression/RulesLoop.php @@ -55,7 +55,7 @@ public function executeWithState(ExecutionStateInterface $state) { /** * {@inheritdoc} */ - public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) { + public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state, $apply_assertions = TRUE) { $violation_list = new IntegrityViolationList(); if (empty($this->configuration['list'])) { @@ -92,14 +92,23 @@ public function checkIntegrity(ExecutionMetadataStateInterface $metadata_state) // So far all ok, so continue with checking integrity in contained actions. // The parent implementation will take care of invoking pre/post traversal // metadata state preparations. - $violation_list = parent::checkIntegrity($metadata_state); + $violation_list = parent::checkIntegrity($metadata_state, $apply_assertions); return $violation_list; } /** * {@inheritdoc} */ - protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) { + protected function allowsMetadataAssertions() { + // As the list can be empty, we cannot ensure child expressions are + // executed at all - thus no assertions can be added. + return FALSE; + } + + /** + * {@inheritdoc} + */ + protected function prepareExecutionMetadataStateBeforeTraversal(ExecutionMetadataStateInterface $metadata_state) { try { $list_definition = $metadata_state->fetchDefinitionByPropertyPath($this->configuration['list']); $list_item_definition = $list_definition->getItemDefinition(); @@ -114,7 +123,7 @@ protected function prepareExecutionMetadataStateBeforeTraversal($metadata_state) /** * {@inheritdoc} */ - protected function prepareExecutionMetadataStateAfterTraversal($metadata_state) { + protected function prepareExecutionMetadataStateAfterTraversal(ExecutionMetadataStateInterface $metadata_state) { // Remove the list item variable after the loop, it is out of scope now. $metadata_state->removeDataDefinition($this->configuration['list_item']); } diff --git a/src/Plugin/RulesExpression/RulesOr.php b/src/Plugin/RulesExpression/RulesOr.php index bc740109..bacc639f 100644 --- a/src/Plugin/RulesExpression/RulesOr.php +++ b/src/Plugin/RulesExpression/RulesOr.php @@ -34,4 +34,13 @@ public function evaluate(ExecutionStateInterface $state) { return empty($this->conditions); } + /** + * {@inheritdoc} + */ + protected function allowsMetadataAssertions() { + // We cannot garantuee child expressions are executed, thus we cannot allow + // metadata assertions. + return FALSE; + } + } From e0ad5062f29e7662bc5acb8e66e2923171f5f682 Mon Sep 17 00:00:00 2001 From: fago Date: Sun, 6 Mar 2016 12:25:07 +0100 Subject: [PATCH 54/57] Issue #2681733 by fago: Add test coverage for metadata assertions. --- src/Context/ContextHandlerIntegrityTrait.php | 7 +- src/Engine/IntegrityViolationList.php | 29 ++++++ src/Plugin/Condition/EntityIsOfBundle.php | 3 +- .../Kernel/Engine/MetadataAssertionTest.php | 90 +++++++++++++++++++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tests/src/Kernel/Engine/MetadataAssertionTest.php diff --git a/src/Context/ContextHandlerIntegrityTrait.php b/src/Context/ContextHandlerIntegrityTrait.php index bf206a54..0f5135d6 100644 --- a/src/Context/ContextHandlerIntegrityTrait.php +++ b/src/Context/ContextHandlerIntegrityTrait.php @@ -135,7 +135,12 @@ protected function checkDataTypeCompatible(CoreContextDefinitionInterface $conte // Compare data types. For now, fail if they are not equal. // @todo: Add support for matching based upon type-inheritance. $target_type = $context_definition->getDataDefinition()->getDataType(); - if ($target_type != 'any' && $target_type != $provided->getDataType()) { + + // Special case any and entity target types for now. + if ($target_type == 'any' || ($target_type == 'entity' && strpos($provided->getDataType(), 'entity:') !== FALSE)) { + return; + } + if ($target_type != $provided->getDataType()) { $expected_type_problem = $context_definition->getDataDefinition()->getDataType(); $violation = new IntegrityViolation(); $violation->setMessage($this->t('Expected a @expected_type data type for context %context_name but got a @provided_type data type instead.', [ diff --git a/src/Engine/IntegrityViolationList.php b/src/Engine/IntegrityViolationList.php index d48ab532..c62a40d2 100644 --- a/src/Engine/IntegrityViolationList.php +++ b/src/Engine/IntegrityViolationList.php @@ -30,6 +30,35 @@ public function addAll(IntegrityViolationList $other_list) { } } + /** + * Returns the violation at a given offset. + * + * @param int $offset + * The offset of the violation. + * + * @return \Drupal\rules\Engine\IntegrityViolationInterface + * The violation. + * + * @throws \OutOfBoundsException + * Thrown if the offset does not exist. + */ + public function get($offset) { + return $this->offsetGet($offset); + } + + /** + * Returns whether the given offset exists. + * + * @param int $offset + * The violation offset. + * + * @return bool + * Whether the offset exists. + */ + public function has($offset) { + return $this->offsetExists($offset); + } + /** * Creates a new violation with the message and adds it to this list. * diff --git a/src/Plugin/Condition/EntityIsOfBundle.php b/src/Plugin/Condition/EntityIsOfBundle.php index 3bf52fe4..43187c85 100644 --- a/src/Plugin/Condition/EntityIsOfBundle.php +++ b/src/Plugin/Condition/EntityIsOfBundle.php @@ -66,7 +66,8 @@ protected function doEvaluate(EntityInterface $entity, $type, $bundle) { public function assertMetadata(array $selected_data) { // Assert the checked bundle. if (isset($selected_data['entity']) && $bundle = $this->getContextValue('bundle')) { - $selected_data['entity']->setBundles($this->getContextValue('bundle')); + $bundles = is_array($bundle) ? $bundle : [$bundle]; + $selected_data['entity']->setBundles($bundles); } } diff --git a/tests/src/Kernel/Engine/MetadataAssertionTest.php b/tests/src/Kernel/Engine/MetadataAssertionTest.php new file mode 100644 index 00000000..9764e1b7 --- /dev/null +++ b/tests/src/Kernel/Engine/MetadataAssertionTest.php @@ -0,0 +1,90 @@ +installSchema('system', ['sequences']); + $this->installEntitySchema('user'); + $this->installEntitySchema('node'); + $this->installConfig(['field']); + + $entity_type_manager = $this->container->get('entity_type.manager'); + $entity_type_manager->getStorage('node_type') + ->create(['type' => 'page']) + ->save(); + + FieldStorageConfig::create([ + 'field_name' => 'field_text', + 'type' => 'string', + 'entity_type' => 'node', + 'cardinality' => 1, + ])->save(); + FieldConfig::create([ + 'field_name' => 'field_text', + 'entity_type' => 'node', + 'bundle' => 'page', + ])->save(); + } + + /** + * Tests asserting metadata using the EntityIfOfBundle condition. + */ + public function testAssertingEntityBundle() { + // When trying to use the field_text field without knowledge of the bundle, + // the field is not available. + $rule = $this->expressionManager->createRule(); + $rule->addAction('rules_system_message', ContextConfig::create() + ->map('message', 'node.field_text.value') + ->setValue('type', 'status') + ); + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('node', ContextDefinition::create('entity:node')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + $this->assertEquals( + 'Data selector %selector for context %context_name is invalid. @message', + $violation_list->get(0)->getMessage()->getUntranslatedString() + ); + + // Now add the EntityIsOfBundle condition and try again. + $rule->addCondition('rules_entity_is_of_bundle', ContextConfig::create() + ->map('entity', 'node') + ->setValue('type', 'node') + ->setValue('bundle', 'page') + ); + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('node', ContextDefinition::create('entity:node')) + ->checkIntegrity(); + $this->assertEquals(0, iterator_count($violation_list)); + } + +} From 2f4d041a42cc40950f79a9004cb146ba2d276315 Mon Sep 17 00:00:00 2001 From: Wolfgang Ziegler // fago Date: Mon, 7 Mar 2016 11:46:16 +0100 Subject: [PATCH 55/57] Issue #2681733 by fago: Improve docs to clarify the difference between refining context definitions and asserting metadata. --- src/Context/ContextAwarePluginInterface.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Context/ContextAwarePluginInterface.php b/src/Context/ContextAwarePluginInterface.php index c70bbb3a..2b5b3fc9 100644 --- a/src/Context/ContextAwarePluginInterface.php +++ b/src/Context/ContextAwarePluginInterface.php @@ -21,6 +21,10 @@ interface ContextAwarePluginInterface extends CoreContextAwarePluginInterface { * already available upon which the definition of subsequent or provided * context can be refined. * + * Implement this method, when the plugin's context definitions need to be + * refined. When the selected data definitions should be refined, implement + * ::assertMetadata() instead. + * * Note that context gets refined at configuration and execution time of the * plugin. * @@ -39,6 +43,10 @@ public function refineContextDefinitions(array $selected_data); * sub-sequent executed plugins are aware of the metadata and can build upon * it. * + * Implement this method, when the selected data definitions need to be + * refined. When the plugin's context definitions should be refined, implement + * ::refineContextDefinitions() instead. + * * Note that metadata is only asserted on configuration time. The plugin has * to ensure that the run-time data matches the asserted configuration if it * has been executed successfully. From d95464cb8b685eac0699939e127a975500b63a02 Mon Sep 17 00:00:00 2001 From: Wolfgang Ziegler // fago Date: Mon, 7 Mar 2016 12:39:35 +0100 Subject: [PATCH 56/57] Issue #2681733 by fago: Improve metadata assertions test coverage. --- src/Context/ContextAwarePluginInterface.php | 5 ++ src/Context/ContextHandlerTrait.php | 11 ++- src/Core/RulesActionBase.php | 1 + src/Core/RulesConditionBase.php | 1 + src/Engine/ConditionExpressionInterface.php | 11 +++ src/Plugin/Condition/EntityIsOfBundle.php | 5 +- src/Plugin/RulesExpression/Rule.php | 6 +- src/Plugin/RulesExpression/RulesCondition.php | 8 ++ .../Kernel/Engine/MetadataAssertionTest.php | 81 +++++++++++++++++++ 9 files changed, 123 insertions(+), 6 deletions(-) diff --git a/src/Context/ContextAwarePluginInterface.php b/src/Context/ContextAwarePluginInterface.php index 2b5b3fc9..97806c18 100644 --- a/src/Context/ContextAwarePluginInterface.php +++ b/src/Context/ContextAwarePluginInterface.php @@ -54,6 +54,11 @@ public function refineContextDefinitions(array $selected_data); * @param \Drupal\Core\TypedData\DataDefinitionInterface[] $selected_data * An array of data definitions for context that is mapped using a data * selector, keyed by context name. + * + * @return \Drupal\Core\TypedData\DataDefinitionInterface[] + * An array of modified data definitions, keyed as the passed array. Note + * data definitions need to be cloned *before* they are modified, such that + * the changes do not propagate unintentionally. */ public function assertMetadata(array $selected_data); diff --git a/src/Context/ContextHandlerTrait.php b/src/Context/ContextHandlerTrait.php index c6268faa..462e27e2 100644 --- a/src/Context/ContextHandlerTrait.php +++ b/src/Context/ContextHandlerTrait.php @@ -270,7 +270,16 @@ protected function assertMetadata(CoreContextAwarePluginInterface $plugin, Execu if (!$plugin instanceof ContextAwarePluginInterface) { return; } - $plugin->assertMetadata($this->getSelectedData($metadata_state)); + $changed_definitions = $plugin->assertMetadata($this->getSelectedData($metadata_state)); + + // Reverse the mapping and apply the changes. + foreach ($changed_definitions as $context_name => $definition) { + $selector = $this->configuration['context_mapping'][$context_name]; + // @todo: Deal with selectors matching not a context name. + if (strpos($selector, '.') === FALSE) { + $metadata_state->setDataDefinition($selector, $definition); + } + } } /** diff --git a/src/Core/RulesActionBase.php b/src/Core/RulesActionBase.php index 83aaff73..431a797b 100644 --- a/src/Core/RulesActionBase.php +++ b/src/Core/RulesActionBase.php @@ -58,6 +58,7 @@ public function refineContextDefinitions(array $selected_data) { */ public function assertMetadata(array $selected_data) { // Nothing to assert by default. + return []; } /** diff --git a/src/Core/RulesConditionBase.php b/src/Core/RulesConditionBase.php index bdda099b..9ac5d8ef 100644 --- a/src/Core/RulesConditionBase.php +++ b/src/Core/RulesConditionBase.php @@ -34,6 +34,7 @@ public function refineContextDefinitions(array $selected_data) { */ public function assertMetadata(array $selected_data) { // Nothing to assert by default. + return []; } /** diff --git a/src/Engine/ConditionExpressionInterface.php b/src/Engine/ConditionExpressionInterface.php index b10705bc..9f058e5b 100644 --- a/src/Engine/ConditionExpressionInterface.php +++ b/src/Engine/ConditionExpressionInterface.php @@ -12,6 +12,17 @@ */ interface ConditionExpressionInterface extends ExpressionInterface { + + /** + * Negates the result after evaluating this condition. + * + * @param bool $negate + * TRUE to indicate that the condition should be negated, FALSE otherwise. + * + * @return $this + */ + public function negate($negate = TRUE); + /** * Determines whether condition result will be negated. * diff --git a/src/Plugin/Condition/EntityIsOfBundle.php b/src/Plugin/Condition/EntityIsOfBundle.php index 43187c85..78de8c35 100644 --- a/src/Plugin/Condition/EntityIsOfBundle.php +++ b/src/Plugin/Condition/EntityIsOfBundle.php @@ -65,10 +65,13 @@ protected function doEvaluate(EntityInterface $entity, $type, $bundle) { */ public function assertMetadata(array $selected_data) { // Assert the checked bundle. + $changed_definitions = []; if (isset($selected_data['entity']) && $bundle = $this->getContextValue('bundle')) { + $changed_definitions['entity'] = clone $selected_data['entity']; $bundles = is_array($bundle) ? $bundle : [$bundle]; - $selected_data['entity']->setBundles($bundles); + $changed_definitions['entity']->setBundles($bundles); } + return $changed_definitions; } } diff --git a/src/Plugin/RulesExpression/Rule.php b/src/Plugin/RulesExpression/Rule.php index 746ea054..10fadcbe 100644 --- a/src/Plugin/RulesExpression/Rule.php +++ b/src/Plugin/RulesExpression/Rule.php @@ -103,8 +103,7 @@ public function executeWithState(ExecutionStateInterface $state) { * {@inheritdoc} */ public function addCondition($condition_id, ContextConfig $config = NULL) { - $this->conditions->addCondition($condition_id, $config); - return $this; + return $this->conditions->addCondition($condition_id, $config); } /** @@ -126,8 +125,7 @@ public function setConditions(ConditionExpressionContainerInterface $conditions) * {@inheritdoc} */ public function addAction($action_id, ContextConfig $config = NULL) { - $this->actions->addAction($action_id, $config); - return $this; + return $this->actions->addAction($action_id, $config); } /** diff --git a/src/Plugin/RulesExpression/RulesCondition.php b/src/Plugin/RulesExpression/RulesCondition.php index a4100956..d0637ac7 100644 --- a/src/Plugin/RulesExpression/RulesCondition.php +++ b/src/Plugin/RulesExpression/RulesCondition.php @@ -124,6 +124,14 @@ public function executeWithState(ExecutionStateInterface $state) { return $result; } + /** + * {@inheritdoc} + */ + public function negate($negate = TRUE) { + $this->configuration['negate'] = $negate; + return $this; + } + /** * {@inheritdoc} */ diff --git a/tests/src/Kernel/Engine/MetadataAssertionTest.php b/tests/src/Kernel/Engine/MetadataAssertionTest.php index 9764e1b7..6cea3a66 100644 --- a/tests/src/Kernel/Engine/MetadataAssertionTest.php +++ b/tests/src/Kernel/Engine/MetadataAssertionTest.php @@ -87,4 +87,85 @@ public function testAssertingEntityBundle() { $this->assertEquals(0, iterator_count($violation_list)); } + /** + * Tests asserted metadata is handled correctly in OR and AND containers. + */ + public function testAssertingWithLogicalOperations() { + // Add an nested AND and make sure it keeps working. + $rule = $this->expressionManager->createRule(); + $and = $this->expressionManager->createAnd(); + $and->addCondition('rules_entity_is_of_bundle', ContextConfig::create() + ->map('entity', 'node') + ->setValue('type', 'node') + ->setValue('bundle', 'page') + ); + $rule->addExpressionObject($and); + $rule->addAction('rules_system_message', ContextConfig::create() + ->map('message', 'node.field_text.value') + ->setValue('type', 'status') + ); + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('node', ContextDefinition::create('entity:node')) + ->checkIntegrity(); + $this->assertEquals(0, iterator_count($violation_list)); + + // Add an nested OR and make sure it is ignored. + $rule = $this->expressionManager->createRule(); + $or = $this->expressionManager->createOr(); + $or->addCondition('rules_entity_is_of_bundle', ContextConfig::create() + ->map('entity', 'node') + ->setValue('type', 'node') + ->setValue('bundle', 'page') + ); + $rule->addExpressionObject($or); + $rule->addAction('rules_system_message', ContextConfig::create() + ->map('message', 'node.field_text.value') + ->setValue('type', 'status') + ); + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('node', ContextDefinition::create('entity:node')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + } + + /** + * Tests asserted metadata of negated conditions is ignored. + */ + public function testAssertingOfNegatedConditions() { + // Negate the condition only and make sure it is ignored. + $rule = $this->expressionManager->createRule(); + $rule->addCondition('rules_entity_is_of_bundle', ContextConfig::create() + ->map('entity', 'node') + ->setValue('type', 'node') + ->setValue('bundle', 'page') + )->negate(TRUE); + $rule->addAction('rules_system_message', ContextConfig::create() + ->map('message', 'node.field_text.value') + ->setValue('type', 'status') + ); + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('node', ContextDefinition::create('entity:node')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + + // Add an negated AND and make sure it is ignored. + $rule = $this->expressionManager->createRule(); + $and = $this->expressionManager->createAnd(); + $and->addCondition('rules_entity_is_of_bundle', ContextConfig::create() + ->map('entity', 'node') + ->setValue('type', 'node') + ->setValue('bundle', 'page') + ); + $and->negate(TRUE); + $rule->addExpressionObject($and); + $rule->addAction('rules_system_message', ContextConfig::create() + ->map('message', 'node.field_text.value') + ->setValue('type', 'status') + ); + $violation_list = RulesComponent::create($rule) + ->addContextDefinition('node', ContextDefinition::create('entity:node')) + ->checkIntegrity(); + $this->assertEquals(1, iterator_count($violation_list)); + } + } From 7fecba2b2b2c18360c8d8fd00e3db9e047071048 Mon Sep 17 00:00:00 2001 From: Wolfgang Ziegler // fago Date: Mon, 7 Mar 2016 13:49:11 +0100 Subject: [PATCH 57/57] Worked over bundle configurable event. --- src/Core/RulesDefaultEventHandler.php | 1 + src/Entity/ReactionRuleStorage.php | 2 +- .../ConfigurableEventHandlerEntityBundle.php | 21 +++++++++++-------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Core/RulesDefaultEventHandler.php b/src/Core/RulesDefaultEventHandler.php index f2297bae..1af70b63 100644 --- a/src/Core/RulesDefaultEventHandler.php +++ b/src/Core/RulesDefaultEventHandler.php @@ -22,6 +22,7 @@ public function getContextDefinitions() { $definition = $this->getPluginDefinition(); if ($this instanceof RulesConfigurableEventHandlerInterface) { $this->refineContextDefinitions(); + $definition = $this->getPluginDefinition(); } return !empty($definition['context']) ? $definition['context'] : []; } diff --git a/src/Entity/ReactionRuleStorage.php b/src/Entity/ReactionRuleStorage.php index 29c4713c..50baa766 100644 --- a/src/Entity/ReactionRuleStorage.php +++ b/src/Entity/ReactionRuleStorage.php @@ -95,7 +95,7 @@ public static function createInstance(ContainerInterface $container, EntityTypeI protected function getRegisteredEvents() { $events = []; foreach ($this->loadMultiple() as $rules_config) { - foreach ($rules_config->getBaseEventNames() as $event_name) { + foreach ($rules_config->getEventBaseNames() as $event_name) { if (!isset($events[$event_name])) { $events[$event_name] = $event_name; } diff --git a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php index b8bbcb6c..58362796 100644 --- a/src/EventHandler/ConfigurableEventHandlerEntityBundle.php +++ b/src/EventHandler/ConfigurableEventHandlerEntityBundle.php @@ -34,16 +34,18 @@ class ConfigurableEventHandlerEntityBundle extends ConfigurableEventHandlerBase * * @var string */ - protected $entityType; + protected $entityTypeId; /** * {@inheritdoc} */ public function __construct(array $configuration, $plugin_id, $plugin_definition) { parent::__construct($configuration, $plugin_id, $plugin_definition); - $this->entityType = $this->getEventNameSuffix(); - $this->entityInfo = \Drupal::entityTypeManager()->getDefinition($this->entityType); - $this->bundlesInfo = \Drupal::entityManager()->getBundleInfo($this->entityType); + $this->entityTypeId = $plugin_definition['entity_type_id']; + // @todo: This needs to use dependency injection. + $this->entityInfo = \Drupal::entityTypeManager()->getDefinition($this->entityTypeId); + // @tdo: use EntityTypeBundleInfo service. + $this->bundlesInfo = \Drupal::entityManager()->getBundleInfo($this->entityTypeId); if (!$this->bundlesInfo) { throw new \InvalidArgumentException('Unsupported event name passed.'); } @@ -98,29 +100,30 @@ public function extractConfigurationFormValues(array &$form, FormStateInterface * {@inheritdoc} */ public function validate() { - // Nothing to check by default. + // Nothing to validate. } /** * {@inheritdoc} */ public function getEventNameSuffix() { - $parts = explode(':', $this->pluginId); - return $parts[1]; + return isset($this->configuration['bundle']) ? $this->configuration['bundle'] : FALSE; } /** * {@inheritdoc} */ public function refineContextDefinitions() { - // Nothing to refine by default. + if ($bundle = $this->getEventNameSuffix()) { + $this->pluginDefinition['context']['entity']->setBundles([$bundle]); + } } /** * {@inheritdoc} */ public function calculateDependencies() { - // Nothing to calculate by default. + // @todo: Implement. } }