Passed
Push — master ( fb1eb0...02cee8 )
by Guido
08:09
created

CreateOrAssignUserRoleActionCommand::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 6

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 1
nc 1
nop 3
dl 0
loc 6
rs 10
c 0
b 0
f 0
1
<?php
2
3
namespace Gvera\Commands;
4
5
use Gvera\Exceptions\NotFoundException;
6
use Gvera\Helpers\entities\GvEntityManager;
7
use Gvera\Models\UserRole;
8
use Gvera\Models\UserRoleAction;
9
10
class CreateOrAssignUserRoleActionCommand implements CommandInterface
11
{
12
    /**
13
     * @var string
14
     */
15
    private $actionName;
16
    /**
17
     * @var string
18
     */
19
    private $userRoleName;
20
    /**
21
     * @var GvEntityManager
22
     */
23
    private $entityManager;
24
25
26
    /**
27
     * CreateOrAssignUserRoleActionCommand constructor.
28
     * @param string $actionName
29
     * @param string $userRoleName
30
     * @param GvEntityManager $entityManager
31
     */
32
    public function __construct(string $actionName, string $userRoleName, GvEntityManager $entityManager)
0 ignored issues
show
Bug introduced by
You have injected the EntityManager via parameter $entityManager. This is generally not recommended as it might get closed and become unusable. Instead, it is recommended to inject the ManagerRegistry and retrieve the EntityManager via getManager() each time you need it.

The EntityManager might become unusable for example if a transaction is rolled back and it gets closed. Let’s assume that somewhere in your application, or in a third-party library, there is code such as the following:

function someFunction(ManagerRegistry $registry) {
    $em = $registry->getManager();
    $em->getConnection()->beginTransaction();
    try {
        // Do something.
        $em->getConnection()->commit();
    } catch (\Exception $ex) {
        $em->getConnection()->rollback();
        $em->close();

        throw $ex;
    }
}

If that code throws an exception and the EntityManager is closed. Any other code which depends on the same instance of the EntityManager during this request will fail.

On the other hand, if you instead inject the ManagerRegistry, the getManager() method guarantees that you will always get a usable manager instance.

Loading history...
33
    {
34
        $this->actionName = $actionName;
35
        $this->userRoleName = $userRoleName;
36
        $this->entityManager = $entityManager;
37
    }
38
39
    /**
40
     * @throws NotFoundException
41
     * @throws \Doctrine\ORM\ORMException
42
     * @throws \Doctrine\ORM\OptimisticLockException
43
     */
44
    public function execute():void
45
    {
46
        $exists = $this->entityManager->getRepository(UserRoleAction::class)
47
            ->findOneBy(['name' => $this->actionName]);
48
        $action = $exists ?? new UserRoleAction();
49
        $action->setActionName($this->actionName);
50
51
        $userRole = $this->entityManager->getRepository(UserRole::class)
0 ignored issues
show
Bug introduced by
Are you sure the assignment to $userRole is correct as $this->entityManager->ge...> $this->userRoleName)) (which targets Doctrine\Persistence\ObjectRepository::findOneBy()) seems to always return null.

This check looks for function or method calls that always return null and whose return value is assigned to a variable.

class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
$object = $a->getObject();

The method getObject() can return nothing but null, so it makes no sense to assign that value to a variable.

The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.

Loading history...
52
            ->findOneBy(['name' => $this->userRoleName]);
53
54
        if (null === $userRole) {
55
            throw new NotFoundException('user role was not found', ['userRoleName' => $this->userRoleName]);
56
        }
57
58
        $action->addUserRole($userRole);
59
        $this->entityManager->persist($action);
60
        $this->entityManager->flush();
61
    }
62
}
63