Symfony2: Injecting @security.context to get the current user. How to avoid a "ServiceCircularReferenceException"? Inject the whole container? [duplicate]

Question

Coming from this post and after fix things I'm in another issue/security question/problem.

As yours may see in the other post I'm trying to inject security context in the listener but if I leave the code intact without touch I got this error:

ServiceCircularReferenceException: Circular reference detected for service "doctrine.orm.default_entity_manager"

So, reading and researching I found a solution but is not clear to me if is the right or if it's secure for my application. So this is what I did:

Instead of inject [@security.context] I did this:

services:
    orderhascomment.listener:
        class: PL\OrderBundle\Listener\OrderHasCommentListener
        arguments: [@service_container]
        tags:
            - { name: doctrine.event_listener, event: prePersist, method: onPrePersist }

And my listener OrderHasCommentListener.php is as follow:

namespace PL\OrderBundle\Listener;

use Doctrine\ORM\Event\LifecycleEventArgs;
use Symfony\Component\DependencyInjection\ContainerInterface;

class OrderHasCommentListener {

    protected $container;

    public function __construct(ContainerInterface $container = null) {
        $this->container = $container;
    }

    /**
     *
     * @param LifecycleEventArgs $args 
     */
    public function onPrePersist(LifecycleEventArgs $args) {

        $entity = $args->getEntity();
        $user = $this->container->get('security.context')->getToken()->getUser();
        $entity->setUser($user);
    }

}

Is that the right way to do this? Or exists another one? I read it's a bad idea to inject the whole container since I need just the security context, what's the solution then? (https://insight.sensiolabs.com/what-we-analyse)

Trying to convert UserCallable in a service

I'm trying to convert UserCallable in a service by following instructions here and taking a look at DoctrineBehaviors orm-services.yml file and also seeing how them do it at BlameableListener but I can not get it to work since I get this error:

ContextErrorException: Catchable Fatal Error: Argument 1 passed to PL\OrderBundle\Listener\OrderHasCommentListener::__construct() must be callable, string given

This is how my definition looks like at app/config/config.yml:

services:
    orderhascomment.listener:
        class: PL\OrderBundle\Listener\OrderHasCommentListener
        arguments: 
            - user_callable
        tags:
            - { name: doctrine.event_listener, event: prePersist, method: onPrePersist }
    user_callable:
        class: PL\OrderBundle\Util\UserCallable
        arguments:
            - "@service_container"
        public:  false

And this is how I passing to __construct() function in OrderHasCommentListener.php file:

/**
 * @param UserCallableInterface $user_callable 
 * */
public function __construct(callable $user_callable = null) {
    $this->userCallable = $user_callable;
}

What is wrong?

Answer 1

Injecting the whole container directly into the lister may be a working solution ... but we can do better :)

Inject a UserCallable that returns the current user instead.

This way you express the real purpose of the depedency more clearly without introducing a hard dependency between your listener and the container(-interface). An example would be ...

Knp\DoctrineBehaviors\ORM\Blameable\UserCallable

This particular example can be improved further by creating an interface and using that for type-hinting in your listener instead. That allows easier exchangeability if you plan to re-use the listener.

The interfaces:

namespace Acme\Common;

interface UserCallableInterface
{
    /**
     * @return \Symfony\Component\Security\Core\User\UserInterface
     */
    public function getCurrentUser();
}

---

namespace Acme\Common;

use Symfony\Component\Security\Core\User\UserInterface;

interface TrackableInterface
{
    /**
     * @param UserInterface $user
     */
    public function setUser(UserInterface $user);
}

The UserCallable:

namespace Acme\Util;

use Acme\Common\UserCallableInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

class UserCallable implements UserCallableInterface
{
   /** @var ContainerInterface **/
   protected $container;

   /** 
    * @param ContainerInterface $container
    */
   public function __construct(ContainerInterface $container)
   {
      $this->container = $container;
   }

   /**
    * @{inheritdoc}
    */
   public function getCurrentUser()
   {
      return $this->container->get('security.context')->getToken()->getUser() ?: false;
   }

The listener:

use Acme\Common\UserCallableInterface;
use Acme\Common\TrackableInterface;
use Doctrine\Common\EventArgs;

class Listener
{
    /** @var UserCallableInterface **/
    protected $userCallable;

    /** 
     * @param UserCallableInterface $user_callable 
     **/    
    public function __construct(UserCallableInterface $user_callable)
    {
       $this->userCallable = $user_callable;
    }

    /** 
     * @param EventArgs $args 
     **/
    public function onPrePersist(EventArgs $args)
    {
       $entity = $args->getEntity();

       if ( !($entity instanceof TrackableInterface) ) {
           return;
       }

       if ( !($user = $this->userCallable->getCurrentUser())) {
           return;
       }

       $entity->setUser($user);
    }      
}