Rules event dispatch does not propagate context from GenericEvent

Looks like Klausi just created a createReactionRule method, so I'll remove that from my patch.

I'm adding a quick diff file to show the change, sans test, which I'm working on now.

When I have the test set up, I'll create a PR.

The problem is actually worse than I thought. In my opinion, the RulesState is needlessly isolated from Action and Configuration plugins.

Here's the problem.

Look at ContextHandlerTrait::mapContext(). Here's the problem code:

    $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->applyDataSelector($this->configuration['context_mapping'][$name]);
      
     //Lots of stuff omitted here....
    }

The problem here is that unless a plugin asks for a piece of data by name via its annotation, Event data will never be seen by the plugin. Even if manipulating $this->configuration['context_mapping'] wasn't a PITA (and yeah, there's no good API access right now)., there is absolutely no flexibility for data access in the plugins.

I wrote a number of Rules 7.x-2.x actions and conditions that were only possible because the RulesState information was available to the call back. IT REALLY NEEDS TO BE THERE IN 8.x-3.x AS WELL.

In particular, the following test code CANNOT WORK in the current implementation:

  public function execute() {
    $context_name = $this->getContextValue('context_name');
    $log_missing  = $this->getContextValue('log_missing');
    $context = $this->getContexts();
    $type = 'ABSENT'; 
    if (isset($context[$context_name])) {
      $type = $context[$context_name]->getDataType();
    }
    else if (!$log_missing) {
      return;
    }
    $this->logger->info("$context_name: $type");
  }

What I'm trying to do here is very simple, and is something that people often need to do: manipulate the data passed in via the event, or test items in it. 'context_name' and 'log_missing' are defined in the action plugin's annotation. And the ReactionRules event, rules_user_login, passes a user object as "account". But the event data will NEVER get to this point.

I'm not sure what mapContext() is intended to do, but if at all possible, I'd love to see it just go away.

Is there any reason why the RuleState cannot be directly available to the executing plugins, without the additional complexity of this mapping process?

This is, in my opinion, a serious problem in the Rules processing pipeline.

Raising the priority to get your collective attention. RulesState needs to get passed where plugin in code can actually get at it. Current implementation makes that impossible.

Is there any way to make the RulesState object accessible to plugins during the process?

mapConext() must stay in Rules because we need to map variables names from the Rules state to variable names in the action plugin.

Certainly, we need to map variable names. But right now, the process also excludes data. It seems to me that if the event supplies data, it should be visible to conditions and to actions, at least so we can introspect the objects. But with the way we map context right now, if something is not explicitly named, it's not in the context at all. This is different from Drupal 7, where the callbacks got the RuleState as an argument.

Could you describe the use case of your action/condition so that we can understand the need for the RulesState better?

Here's 3 use cases where it's useful to have direct access to RulesState.

• Even in Drupal 7, mapping of information to conditions and actions was very unpredictable and frustrating to work with, since if you had multiple actions, reaction rules would frequently fail because attributes of the triggering event were invisible to one of the conditions or actions, causing Rules to throw exceptions. This is definitely an issue with the wider community; it made Rules 2.x frustrating to deal with, and required some strange hacks (phantom conditions that referenced data I wanted downstream was one of my favorites). At least in 7.x-2.x you could get at the state from your callbacks (it was appended to the $args in call_func_array). Currently, that workaround is not available in 8.x-3.x. If mapping is going to be this strict, that workaround is going to be needed.
• Debugging rules conditions and actions for developers. Problems with context are very challenging for developers to fix. Letting the developers introspect what Rules is doing w/o doing fancy things in debuggers would deal with a significant pain point.
• Partly because of the non-transparency of the mapping process (from a developer perspective), writing data processors for rules can be difficult. For example, it was very difficult in Rules 7.x-2.x to "tokenize" a variable generated in by a previous action. For some purposes, simply modifying the data out of RulesState (as you might do in an alter hook) helped to handle these kinds of cases.

Part of what I'm looking at in the 3.x source base is how these kinds of things can be done. Right now, it looks like it's going to be at least as hard in 3.x as currently written as it was in 2.x.

I'd like to turn the question around, as well. You say

An action or condition should not know anything about state. They receive all their necessary data through context definitions.

Why is this? Is there some particular reason that actions and conditions need to be isolated from the rules that call them? In a lot of Drupal code, we go around this kind of isolation. Why should Rules be different in this regard?

Also:

Additional API functions such as autoSaveContext() and refineContext() are used to cover that cases.

Some additional documentation would help a lot here. I don't see a refineContext() -- there is, however, a refineContextDefintions(), which is called w/o arguments -- although I've seen autoSaveContext(). But cannot tell from the very short doc comments what they are supposed to do:

• autoSaveContext() has a doc comment in RulesActionInterface.php, but it is unclear just what "saves" means. If it means "save to RulesState", it probably should actually say that in the doc comments.
• refineContextDefintions() has no doc comment at all that I could find; everything says "{@inheritdoc}". Note also that since refineContextDefinitions() takes no arguments, that the RulesState is in any case unavailable.

Selectors can be used for some things as well, I know, but they are pretty complex and not all that well documented either.

If we can do something that is as simple as possible to understand, document and debug, I think we're helping ourselves and we're helping the community.

fago,

Yeah, that would likely work. It's not as transparent as simply writing over the state, but yeah, it would be possible at least to see it.