Implement recursion prevention

We could create an account to be used as a bot and check if the bot or a normal user is updating/inserting/deleting entities, this method > [api link] probably work but I'm not sure why this approach is marked as "highly discouraged!"

Using AccountSwitcherInterface::switchTo || \Drupal::service('account_switcher'); work beautiful

Let's just follow what we have in d7 - that is proven to work as good enough.

Starting with a failing test https://github.com/fago/rules/pull/443

I think a beta should have that.

Can we please get an estimated time for fixing this one? This is making the Rules module virtually unusable for us!

Thanks,
Sukanya

I have temporary solution at

Is this functionality is available in any other module? If not I can try to implement it.

We had recursion prevention in D7 Rules. D7 Rules also has tests which demonstrate the conditions where we might encounter recursion and show what we expect to happen in those cases. It looks like @klausi started to port those tests above in #6. I don't know how far he got, but his work is there in the pull request.

There is no one working on this currently.

I think a good approach would be to update and finish what @klausi started - let's get the recursion tests ported to D8 first, that way we will have a way to evaluate any patch for recursion prevention and that way we will have an automated check to ensure recursion prevention continues to work.

I would prefer that all development be done here in the issue queue with patches, so I would like to see that updated and posted here as a patch. @fago hasn't been active in Rules for more than a year, so pull requests in github won't help.

@milindk: Do you intend to work on this?

@TR Yeah I will be working on this.

Thank you! If you have questions or problems just ask.

@milindk: Any progress?

Here's an update to and re-roll of @klausi's "failing test" from #6. The code was taken from the PR and changed only to apply to the current version of Rules. I expect the patch will apply and the test will fail.

This, and porting the other recursion tests from the d7-tests directory should be the first step.

Part of that fail is because I misspelled 'context_definitions' when I re-rolled the PR. But part is because we're no longer operating on config entities but on RulesComponents, which don't have a uuid() method. I'm not going to fix that because I'm not working on this issue right now, I just wanted to re-roll the patch in hopes that someone could pick this up and finish it off.

Not a beta blocker, since as far as I can tell this is NOT going to involve an API change.

But it should be a release blocker.

Added uuid() function.
The patch is rolled against 3.0-alpha6 tag.

Re-rolled to pass CS and sort out index uuid issue.

Please provide an interdiff.txt so we can see what you changed. And as I noted in #17 my previous patch had a spelling error that caused a test mail and needs to be corrected.

Updated the code to cope with tests.
Removed spelling error so test pass.

For those following this issue an update on why the test failed, as it is possible that you can apply the patch and because none of your rules are updating the entity that is being saved, then you may find that it works. In the failing test, the 'rules_entity_update:node' rule updates the title field and it is failing with the following message.

The website encountered an unexpected error. Please try again later.

Drupal\Core\Entity\EntityStorageException: Update existing 'node' entity revision while changing the revision ID is not supported. 2 1 in Drupal\Core\Entity\ContentEntityStorageBase->doPreSave() (line 711 of core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).

Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 837)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 395)
Drupal\Core\Entity\EntityBase->save() (Line: 246)
Drupal\rules\Context\ExecutionState->autoSave() (Line: 160)
Drupal\rules\EventSubscriber\GenericEventSubscriber->onRulesEvent(Object, 'rules_entity_update:node', Object)
call_user_func(Array, Object, 'rules_entity_update:node', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('rules_entity_update:node', Object) (Line: 183)
rules_entity_update(Object)
call_user_func_array('rules_entity_update', Array) (Line: 403)
Drupal\Core\Extension\ModuleHandler->invokeAll('entity_update', Array) (Line: 206)
Drupal\Core\Entity\EntityStorageBase->invokeHook('update', Object) (Line: 843)
Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('update', Object) (Line: 535)
Drupal\Core\Entity\EntityStorageBase->doPostSave(Object, 1) (Line: 728)
Drupal\Core\Entity\ContentEntityStorageBase->doPostSave(Object, 1) (Line: 460)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 837)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 395)
Drupal\Core\Entity\EntityBase->save() (Line: 294)
Drupal\node\NodeForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 114)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 52)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 593)
Drupal\Core\Form\FormBuilder->processForm('node_article_edit_form', Array, Object) (Line: 321)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 91)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Here is the changed from released code that produced that message

      if (!$entity->isNewRevision() && $entity->getRevisionId() != $entity->getLoadedRevisionId()) {
        throw new EntityStorageException("Update existing '{$this->entityTypeId}' entity revision while changing the revision ID is not supported. {$entity->getRevisionId()} {$entity->getLoadedRevisionId()}");
      }

Hence it is indicating that the revisionId has changed, from 1 to 2, but it is not flagged as a new revision.

So the patch prevents the issue title, but the test indicates that more work has to be done so that when the rule is updating self. Perhaps the use of 'rules_entity_presave:entity_type_id' is an acceptable workaround.

What does work is updating of other entity when an entity is being updated.

Would like to hear from others about sucesses and failures after applying the patch, to see if more test are needed to written. Perhaps someone knows what needs to change in Drupal\rules\Context\ExecutionState->autoSave() or elsewhere, to cope with updating self. Probably the testing need to be extended so that the calling variable is has the updated information as well or document that it does not.

  // Pseudo code.
  $node->setTitle('Foo');
  $node->save();
  // Check to see if the rule has changed the label from Foo to Bar.
  $this->assertEquals('Bar', $node->getTitle());

It was found that in some cases comparing $context_values failed with a PHP Error indicating it was out of stack space when there was large objects in the context. Now a hashes are compare.

The patch can be applied to 8.x-3.0-alpha7 released 3 December 2021 . The patch cannot apply to the -dev as it has moved on.

I triggered the tests on #28 so we can see what DrupalCI thinks about this patch.

Patch needs re-roll.

Rerolled patch #28, please review it.

Thanks!

Rerolled #31 as the executeWithState() was not protected.