Dynamically adding a callback to an event within another event listener

[NotionCommotion]

I have a listener which executes a HTTP request to a remote API before the User entity is persisted and uses the response to set one of the entity's properties.  It will also listen for update and remove and will make the appropriate HTTP request to the API but will not modify the entity.  All works as desired... Almost.  If when persisting the entity, I have some error, the remote API and my application become out of sync.  I wish to change my application to perform a second call to the API if an error occurs and reverse the previous call.  My thoughts on how to implement are:

1. Place a try/catch block when executing the query.  Don't like this approach.
2. Add an ExceptionListener which somehow retrieves the entity and makes the applicable changes.  Maybe part of the solution, but too complicated to be the full solution.
3. When adding, updating, or deleting a user from the remote API under UserListener's three methods, adding a callback which gets executed upon a PDOException.  I think this is the best approach and expanded my thoughts below.

 

<?php

namespace App\EventListener;

use Doctrine\Persistence\Event\LifecycleEventArgs;
use App\Service\HelpDeskClient;
use App\Entity\AbstractUser;

final class UserListner
{
    private $helpDeskClient;

    public function __construct(HelpDeskClient $helpDeskClient)
    {
        $this->helpDeskClient = $helpDeskClient;
    }

    public function prePersist(AbstractUser $user, LifecycleEventArgs $event)
    {
        $this->helpDeskClient->addUser($user);   //$user will be updated with the HTTP response
    }

    public function preUpdate(AbstractUser $user, LifecycleEventArgs $event)
    {
        $this->helpDeskClient->updateUser($user);
    }

    public function preRemove (AbstractUser $user, LifecycleEventArgs $event)
    {
        $this->helpDeskClient->deleteUser($user);
    }
}

Okay, how do I actually do this?  Was thinking of modifying UserListner as follows:

//...
use Symfony\Component\HttpKernel\KernelEvents;

final class UserListner
{
    // ...
    
    public function prePersist(AbstractUser $user, LifecycleEventArgs $event)
    {
        $this->helpDeskClient->addUser($user);
        
        $event->getObjectManager()->getEventManager()->addEventListener(KernelEvents::EXCEPTION, function($something) use($user) {
            // Use $this->helpDeskClient to reverse the changes
        });
    }
    
    // Similar for update and remove
}

 

But when trying this approach, I get a PDOException, but my callback never gets excecated.  I've also tried replacing KernelEvents::EXCEPTION with '\PDOException' (note the quotes) with no success.

Any ideas what I should be doing differently?

Maybe some totally different approach?  I suppose I could make the request to the API after the DB query is complete for updating and deleting, but not for adding.

[requinix]

Why would the update query fail?

[NotionCommotion]

1 hour ago, requinix said:

Why would the update query fail?

Because validation was not fully implemented.

Off topic, but what is your position on utilizing the database to enforce some validation rules?  I know that some will disagree, but I feel there are benefits to allowing the database to solely validate unique constraints.

[requinix]

37 minutes ago, NotionCommotion said:

Because validation was not fully implemented.

Will this whole problem go away once validation is implemented?

 

37 minutes ago, NotionCommotion said:

Off topic, but what is your position on utilizing the database to enforce some validation rules?  I know that some will disagree, but I feel there are benefits to allowing the database to solely validate unique constraints.

My position?

Database rules to enforce data integrity, mostly with uniqueness and foreign keys. No code should ever be able to leave the database in an invalid state - incomplete, perhaps, but never invalid.
Application rules to enforce business decisions. Opinions change quickly and it's better to deal with that in code. Databases should not be modified unless the structural nature of its data has to change.

[NotionCommotion]

2 hours ago, requinix said:

Will this whole problem go away once validation is implemented?

Not after getting your perspective

Instead of validating the data for duplicates, I will make a POST request to the API, get the response, attempt to make an insert in the DB, call it good if the query goes through, but if on the rare chance it doesn't, need make another request to the API to reverse the action.

[requinix]

Prepare for the most likely scenario. If something were to break, would it probably be the API or the database query? I would think the API. That suggests you do it before the query; if the query fails then you undo the API call, and if that call fails (perhaps you lost network connectivity between the API call and query) then you log what happened and let a human deal with it.

[kicken]

10 hours ago, NotionCommotion said:

but my callback never gets excecated.

Doctrine uses a different events system than Symfony, that's why your code doesn't get executed.  You would need to inject Symfony's event dispatcher and tie into it that way.  You would also need some way to remove your event listener when the query is completed successfully, otherwise you might have it trigger for some other unrelated exception that gets throw later on in your script.  Something like this would be the code then:

final class UserListener {
    private $helpDeskClient;
    private $dispatcher;

    public function __construct(HelpDeskClient $helpDeskClient, EventDispatcherInterface $dispatcher){
        $this->helpDeskClient = $helpDeskClient;
        $this->dispatcher = $dispatcher;
    }

    public function prePersist(AbstractUser $user, LifecycleEventArgs $event){
        $this->helpDeskClient->addUser($user);
        $this->dispatcher->addListener(KernelEvents::EXCEPTION, [$this, 'exceptionHandler']);
    }

    public function postFlush(){
        $this->dispatcher->removeListener(KernelEvents::EXCEPTION, [$this, 'exceptionHandler']);
    }

    public function exceptionHandler(ExceptionEvent $event){
        $this->helpDeskClient->rollback(); //or whatever.
        $this->dispatcher->removeListener(KernelEvents::EXCEPTION, [$this, 'exceptionHandler']);
    }
}

 

That might work out ok.  If for some reason it doesn't, my alternative solution would probably be to create a custom event that gets fired when the user update fails and listen for that.  Something like:

final class HelpdeskRollbackListener implements \Symfony\Component\EventDispatcher\EventSubscriberInterface {
    private $helpDeskClient;

    public function __construct(HelpDeskClient $helpDeskClient){
        $this->helpDeskClient = $helpDeskClient;
    }

    public static function getSubscribedEvents(){
        return [
            UserUpdateFailed::class => 'rollback'
        ];
    }

    public function rollback(UserEvent $event){
        $this->helpDeskClient->rollback($event->getUser());
    }
}

final class UserUpdateFailed extends Symfony\Contracts\EventDispatcher\Event {
    private $user;

    public function __construct(AbstractUser $user){
        $this->user = $user;
    }

    public function getUser(){
        return $this->user;
    }
}

final class YourControllerOrWhatever extends Symfony\Bundle\FrameworkBundle\Controller\AbstractController {
    private $em;
    private $dispatcher;

    public function __construct(EntityManagerInterface $em, Symfony\Component\EventDispatcher\EventDispatcherInterface $dispatcher){
        $this->em = $em;
        $this->dispatcher = $dispatcher;
    }

    public function indexAction(){
        $user = new AbstractUser(/*...*/);

        try {
            $this->em->transactional(function() use ($user){
                $this->em->persist($user);
            });
        } catch (PDOException $ex){
            $this->dispatcher->dispatch(new UserUpdateFailed($user));
            throw $ex;
        }
    }
}

 

[NotionCommotion]

Thanks kicken, I would first like to try your first approach.

13 hours ago, kicken said:

Doctrine uses a different events system than Symfony, that's why your code doesn't get executed.

I've always been confused when to use Doctrine events or Symfony events when used in a Symfony project.  Obviously, Doctrine events should be used for a standalone (non-Symfony) project, but both Doctrine events and Symfony events are described in the Symfony documentation.  I think I asked this before and you said that Doctrine events are only used for events related to the database and Symfony events are more encompassing.  Do I have this right?  Does this mean that I could always just use just Symfony events if I wanted (other I expect that Symfony events related to database operations use Doctrine events so maybe Doctrine events would be more performant).

13 hours ago, kicken said:

You would need to inject Symfony's event dispatcher and tie into it that way.

Makes sense, but somehow I would also need to get $user to be used by $this->helpDeskClient->rollback().  I tried $this->dispatcher->addListener(KernelEvents::EXCEPTION, [$this, $user, 'exceptionHandler']);, but no luck.  I guess I could create a class which is injected with both UserListner and AbstractOwer, but seems like there should be a better way.  Any thoughts?

13 hours ago, kicken said:

You would also need some way to remove your event listener when the query is completed successfully, otherwise you might have it trigger for some other unrelated exception that gets throw later on in your script.

The postFlush is not being fired.  Is its use appropriate? According to the Doctrine docs,  postFlush - The postFlush event occurs at the end of a flush operation.  this event is not a lifecycle callback.  If not a lifecycle callback, what is it?

    App\EventListener\UserListner:
        tags:
            -
                name: 'doctrine.orm.entity_listener'
                event: 'prePersist'
                entity: 'App\Entity\AbstractUser'
            -
                name: 'doctrine.orm.entity_listener'
                event: 'postFlush'
                entity: 'App\Entity\AbstractUser'

 

Edited June 26, 2021 by NotionCommotion

[kicken]

1 hour ago, NotionCommotion said:

Do I have this right?

Pretty much.  The doctrine event system is specific to it and the related database operations.  The Symfony system is generic and can be used as needed.

1 hour ago, NotionCommotion said:

If not a lifecycle callback, what is it?

It's a lifecycle event, but not a callback.  Doctrine makes a distinction between the two things.  A lifecycle callback is a method defined directly on the entity that gets called.  Eg:

class AbstractUser {
   public function postUpdate(){
       //...
   }
}

A lifecycle event that is not a callback requires a listener class like you have, but it needs to be a lifecycle listener not an entity listener.  Those are tagged as doctrine.event_listener.  I'm not sure if you can just change the tag to make it work or if it will need to be split into two separate classes.

1 hour ago, NotionCommotion said:

somehow I would also need to get $user to be used by $this->helpDeskClient->rollback()

Assuming just changing the tag gets your flush event working, you could just store the user as a property on the class.

 

private $user;
public function prePersist(AbstractUser $user, LifecycleEventArgs $event){
    $this->user=$user;
    $this->helpDeskClient->addUser($user);
    $this->dispatcher->addListener(KernelEvents::EXCEPTION, [$this, 'exceptionHandler']);
}

Something to consider is how this would work if you update multiple AbstractUser's at the same time.

[NotionCommotion]

1 hour ago, NotionCommotion said:

I've always been confused when to use Doctrine events or Symfony events when used in a Symfony project.  Obviously, Doctrine events should be used for a standalone (non-Symfony) project, but both Doctrine events and Symfony events are described in the Symfony documentation.  I think I asked this before and you said that Doctrine events are only used for events related to the database and Symfony events are more encompassing.  Do I have this right?  Does this mean that I could always just use just Symfony events if I wanted (other I expect that Symfony events related to database operations use Doctrine events so maybe Doctrine events would be more performant).

I now think I was completely off base for this part

Quote

Symfony triggers several events related to the kernel while processing the HTTP Request. Third-party bundles may also dispatch events, and you can even dispatch custom events from your own code.

and should be thinking this way:

Symfony events:

• kernel.request
• kernel.controller
• kernel.controller_arguments
• kernel.view
• kernel.response
• kernel.finish_request
• kernel.terminate
• kernel.exception
• No others, right?

Third-party bundles

• Doctrine Events
• Etc...

[NotionCommotion]

27 minutes ago, kicken said:

It's a lifecycle event, but not a callback.  Doctrine makes a distinction between the two things.  A lifecycle callback is a method defined directly on the entity that gets called.  Eg:

Ah, of course!  Thank you!

 

27 minutes ago, kicken said:

 

private $user;
public function prePersist(AbstractUser $user, LifecycleEventArgs $event){
    $this->user=$user;
    $this->helpDeskClient->addUser($user);
    $this->dispatcher->addListener(KernelEvents::EXCEPTION, [$this, 'exceptionHandler']);
}

Something to consider is how this would work if you update multiple AbstractUser's at the same time.

As a matter of principle, would rather keep $this->helpDeskClient stateless and do it one of the following two ways if I am unable to pass both when adding the listener.  Think the first approach is more "proper"?  Off topic and sorry for asking all these questions, but what directory would a Symfony application typically locate the AbstractUserUpdater class?  One thought is the same directory as HelpDeskClient (which I have in the App/Service namespace which maybe isn't correct?).  Of maybe in the Entity namespace and just don't make it a Doctrine entity.    EDIT.  Actually, maybe it will not work as the postFlush() method can't be used.  EDIT2.  Or maybe it can and I just need to make make the function postFlush (AbstractUser $user, LifecycleEventArgs $event).  EDIT3.  No, that won't work either because I don't have the instance of AbstactUserUpdater, so maybe my second approach...

    public function prePersist(AbstractUser $user, LifecycleEventArgs $event)
    {
        $this->helpDeskClient->addUser($user);
        $this->dispatcher->addListener(KernelEvents::EXCEPTION, [new AbstractUserUpdater($user, $this), 'exceptionHandler']);
    }

 

    public function prePersist(AbstractUser $user, LifecycleEventArgs $event)
    {
        $this->helpDeskClient->addUser($user);
        $user->setHelpDeskClient($this);
        $this->dispatcher->addListener(KernelEvents::EXCEPTION, [$user, 'exceptionHandler']);
    }

 

Edited June 26, 2021 by NotionCommotion

[kicken]

3 hours ago, NotionCommotion said:

As a matter of principle, would rather keep $this->helpDeskClient stateless

My suggestion doesn't violate this.  The user is saved to the UserListener class not the HelpDeskClient.

 

[NotionCommotion]

5 hours ago, kicken said:

My suggestion doesn't violate this.  The user is saved to the UserListener class not the HelpDeskClient.

Sorry, my bad.  That being said, listeners smell a little like a service but not exactly like one.  Guess if they can be injected via Symphony's auto-wiring, they should be considered one, but don't know whether this is the case or not.

9 hours ago, kicken said:

Something to consider is how this would work if you update multiple AbstractUser's at the same time.

Sounds like you somewhat agree.  Don't envision working with multiple AbstractUsers at the same time, but then again I am often surprised when the impossible becomes the inevitable.

[NotionCommotion]

Thanks again for the help kicken,  End up going almost exactly with your approach with a couple small changes.  Instead of storing User in the listener class, stored LifecycleEventArgs so that I could get the before and after property values of User (needed to reverse an update to the API).  Also, used postPersist instead of postFlush to remove the listener (never totally understood why this is even necessary, but all good).

And thank you to requinix for your recommendation to perform that task which has the highest likelihood of failure first.

<?php

namespace App\EventListener;

use Doctrine\Persistence\Event\LifecycleEventArgs;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use App\Service\HelpDeskClient;
use App\Entity\AbstractUser;

final class UserListner
{
    private $helpDeskClient;
    private $dispatcher;
    private $initialEvent;

    public function __construct(HelpDeskClient $helpDeskClient, EventDispatcherInterface $dispatcher)
    {
        $this->helpDeskClient = $helpDeskClient;
        $this->dispatcher = $dispatcher;
    }

    public function prePersist(AbstractUser $entity, LifecycleEventArgs $event)
    {
        $this->sendHelpDeskRequest($event, 'persist');
    }
    public function preUpdate(AbstractUser $entity, LifecycleEventArgs $event)
    {
        if($this->getHelpDeskChanges($event)) {
            // Only make a HTTP request if specific data is changed in user.
            $this->sendHelpDeskRequest($event, 'update');
        }
    }
    public function preRemove (AbstractUser $entity, LifecycleEventArgs $event)
    {
        $this->sendHelpDeskRequest($event, 'remove');
    }

    public function postPersist(AbstractUser $entity, LifecycleEventArgs $event)
    {
        $this->removeExceptionListener('persist');
    }
    public function postUpdate(AbstractUser $entity, LifecycleEventArgs $event)
    {
        $this->removeExceptionListener('update');
    }
    public function postRemove(AbstractUser $entity, LifecycleEventArgs $event)
    {
        $this->removeExceptionListener('remove');
    }

    public function persistExceptionHandler(ExceptionEvent $event, $type, $dispatcher)
    {
        $this->helpDeskClient->removeUser($this->initialEvent->getEntity());
        $this->removeExceptionListener('persist');
    }
    public function updateExceptionHandler(ExceptionEvent $event, $type, $dispatcher)
    {
        $entity = $this->initialEvent->getEntity();
        foreach($this->getHelpDeskChanges($this->initialEvent) as $name=>$values) {
            $entity->{'set'.$name}($values[0]);            
        }
        $this->helpDeskClient->updateUser($entity);
        $this->removeExceptionListener('update');
    }
    public function removeExceptionHandler(ExceptionEvent $event, $type, $dispatcher)
    {
        $this->helpDeskClient->addUser($this->initialEvent->getEntity());
        $this->removeExceptionListener('remove');
    }

    private function sendHelpDeskRequest(LifecycleEventArgs $initialEvent, string $action):self
    {
        $this->initialEvent = $initialEvent;
        $this->helpDeskClient->{$action.'User'}($this->initialEvent->getEntity());
        $this->dispatcher->addListener(KernelEvents::EXCEPTION, [$this, $action.'ExceptionHandler']);
        return $this;
    }
    private function removeExceptionListener(string $action):self
    {
        $this->dispatcher->removeListener(KernelEvents::EXCEPTION, [$this, $action.'ExceptionHandler']);
        return $this;
    }
    private function getHelpDeskChanges(LifecycleEventArgs $initialEvent):array
    {
        return array_intersect_key($initialEvent->getEntityChangeSet(), array_flip($this->helpDeskClient->getUserProperties()));
    }
}