Completed
Push — 1.9 ( d8eb28...5c3e2e )
by
unknown
61:52 queued 29s
created

RFMMetricStateManager::executeResetQuery()   A

Complexity

Conditions 3
Paths 3

Size

Total Lines 23
Code Lines 15

Duplication

Lines 0
Ratio 0 %
Metric Value
dl 0
loc 23
rs 9.0856
cc 3
eloc 15
nc 3
nop 2
1
<?php
2
3
namespace OroCRM\Bundle\AnalyticsBundle\Model;
4
5
use JMS\JobQueueBundle\Entity\Job;
6
7
use Oro\Bundle\EntityBundle\ORM\DoctrineHelper;
8
9
use OroCRM\Bundle\AnalyticsBundle\Command\CalculateAnalyticsCommand;
10
use OroCRM\Bundle\AnalyticsBundle\Entity\RFMMetricCategory;
11
use OroCRM\Bundle\ChannelBundle\Entity\Channel;
12
13
class RFMMetricStateManager extends StateManager
14
{
15
    /**
16
     * @var string
17
     */
18
    protected $interface;
19
20
    /**
21
     * @var string
22
     */
23
    protected $channelClass;
24
25
    /**
26
     * @param DoctrineHelper $doctrineHelper
27
     * @param string         $interface
28
     * @param string         $channelClass
29
     */
30
    public function __construct(DoctrineHelper $doctrineHelper, $interface, $channelClass)
31
    {
32
        parent::__construct($doctrineHelper);
33
34
        $this->interface    = $interface;
35
        $this->channelClass = $channelClass;
36
    }
37
38
    /**
39
     * @param Channel $channel
40
     */
41
    public function resetMetrics(Channel $channel = null)
42
    {
43
        $criteria = [];
44
45
        if ($channel) {
46
            $criteria = ['id' => $this->doctrineHelper->getSingleEntityIdentifier($channel)];
47
        }
48
49
        /** @var Channel[] $channels */
50
        $channels = $this->doctrineHelper->getEntityRepository($this->channelClass)->findBy($criteria);
51
52
        $channelsByCustomerIdentity = [];
53
        foreach ($channels as $channel) {
54
            $customerIdentity = $channel->getCustomerIdentity();
55
56
            if (!$customerIdentity) {
57
                continue;
58
            }
59
60
            if (!in_array($this->interface, class_implements($customerIdentity), true)) {
61
                continue;
62
            }
63
64
            $channelsByCustomerIdentity[$customerIdentity][] = $this->doctrineHelper
65
                ->getSingleEntityIdentifier($channel);
66
        }
67
68
        foreach ($channelsByCustomerIdentity as $className => $channelIds) {
69
            $this->executeResetQuery($className, $channelIds);
70
        }
71
    }
72
73
    /**
74
     * @param string $className
75
     * @param array  $ids
76
     */
77
    protected function executeResetQuery($className, $ids)
78
    {
79
        if (!$ids) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $ids of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
80
            return;
81
        }
82
83
        $qb = $this->doctrineHelper
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Doctrine\Common\Persistence\ObjectManager as the method createQueryBuilder() does only exist in the following implementations of said interface: Doctrine\ORM\Decorator\EntityManagerDecorator, Doctrine\ORM\EntityManager, Oro\Bundle\EntityBundle\ORM\OroEntityManager, Oro\Bundle\TestFramework...Mocks\EntityManagerMock, Oro\Component\TestUtils\...Mocks\EntityManagerMock.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
84
            ->getEntityManager($className)
85
            ->createQueryBuilder()
86
            ->update($className, 'e');
87
88
        foreach (RFMMetricCategory::$types as $type) {
89
            $qb
90
                ->set(sprintf('e.%s', $type), sprintf(':%s', $type))
91
                ->setParameter($type, null);
92
        }
93
94
        $qb
95
            ->where($qb->expr()->in('e.dataChannel', ':dataChannels'))
96
            ->setParameter('dataChannels', $ids);
97
98
        $qb->getQuery()->execute();
99
    }
100
101
    /**
102
     * @param Channel $channel
103
     */
104
    public function scheduleRecalculation(Channel $channel = null)
105
    {
106
        if ($channel) {
107
            $argument = sprintf('--channel=%s', $channel->getId());
108
109
            if ($this->isJobRunning($argument)) {
110
                return;
111
            }
112
113
            $isActiveChannel = $channel->getStatus() === Channel::STATUS_ACTIVE;
114
            $channelData = $channel->getData();
115
            $rfmEnabled = !empty($channelData[RFMAwareInterface::RFM_STATE_KEY]);
116
117
            if (!$isActiveChannel || !$rfmEnabled) {
118
                return;
119
            }
120
        }
121
122
        if ($this->getJob()) {
123
            return;
124
        }
125
126
        $args = [];
127
        if ($channel) {
128
            $argument = sprintf('--channel=%s', $channel->getId());
129
            $channelJob = $this->getJob($argument);
130
            if ($channelJob) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $channelJob of type JMS\JobQueueBundle\Entity\Job[] is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
131
                return;
132
            }
133
134
            $args = [$argument];
135
        }
136
137
        $job = new Job(CalculateAnalyticsCommand::COMMAND_NAME, $args);
138
        $em = $this->doctrineHelper->getEntityManager($job);
139
140
        if (!$channel) {
141
            $channelJobs = $this->getJob('--channel');
142
143
            if ($channelJobs) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $channelJobs of type JMS\JobQueueBundle\Entity\Job[] is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
144
                foreach ($channelJobs as $channelJob) {
145
                    $em->remove($channelJob);
146
                }
147
            }
148
        }
149
150
        $em->persist($job);
151
        $em->flush($job);
152
    }
153
}
154