AnalyticsEventConsumer::handleArticleImpressions() - Code Metrics - Inspection of "impressions performance improvements (#861)" - superdesk/web-publisher - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 1.5 ( 293f07...bc7cd6 )

by Rafał

created 2019-06-14 12:43 UTC

AnalyticsEventConsumer::handleArticleImpressions() C

↳ Parent: AnalyticsEventConsumer

Complexity

Conditions	11
Paths	199

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	67
rs	5.9132
c	0
b	0
f	0
cc	11
nc	199
nop	1

How to fix Long Method Complexity

<?php

declare(strict_types=1);

/*
 * This file is part of the Superdesk Web Publisher Core Bundle.
 *
 * Copyright 2017 Sourcefabric z.ú. and contributors.
 *
 * For the full copyright and license information, please see the
 * AUTHORS and LICENSE files distributed with this source code.
 *
 * @copyright 2017 Sourcefabric z.ú
 * @license http://www.superdesk.org/license
 */

namespace SWP\Bundle\CoreBundle\Consumer;

use Doctrine\Common\Cache\CacheProvider;
use Doctrine\Common\Persistence\ObjectManager;
use OldSound\RabbitMqBundle\RabbitMq\ConsumerInterface;
use PhpAmqpLib\Message\AMQPMessage;
use SWP\Bundle\AnalyticsBundle\Model\ArticleEventInterface;
use SWP\Bundle\AnalyticsBundle\Services\ArticleStatisticsServiceInterface;
use SWP\Bundle\ContentBundle\Model\RouteInterface;
use SWP\Bundle\CoreBundle\Model\ArticleInterface;
use SWP\Bundle\CoreBundle\Model\ArticleStatistics;
use SWP\Bundle\CoreBundle\Resolver\ArticleResolverInterface;
use SWP\Component\MultiTenancy\Context\TenantContextInterface;
use SWP\Component\MultiTenancy\Exception\TenantNotFoundException;
use SWP\Component\MultiTenancy\Resolver\TenantResolver;
use SWP\Component\TemplatesSystem\Gimme\Meta\Meta;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\Matcher\UrlMatcherInterface;

/**
 * Class AnalyticsEventConsumer.
 */
final class AnalyticsEventConsumer implements ConsumerInterface
{
    /**
     * @var ArticleStatisticsServiceInterface
     */
    private $articleStatisticsService;

    /**
     * @var TenantResolver
     */
    private $tenantResolver;

    /**
     * @var TenantContextInterface
     */
    private $tenantContext;

    /**
     * @var UrlMatcherInterface
     */
    private $matcher;

    /**
     * @var ArticleResolverInterface
     */
    private $articleResolver;

    private $articleStatisticsObjectManager;

    private $cacheProvider;

    public function __construct(
        ArticleStatisticsServiceInterface $articleStatisticsService,
        TenantResolver $tenantResolver,
        TenantContextInterface $tenantContext,
        UrlMatcherInterface $matcher,
        ArticleResolverInterface $articleResolver,
        ObjectManager $articleStatisticsObjectManager,
        CacheProvider $cacheProvider
    ) {
        $this->articleStatisticsService = $articleStatisticsService;
        $this->tenantResolver = $tenantResolver;
        $this->tenantContext = $tenantContext;
        $this->matcher = $matcher;
        $this->articleResolver = $articleResolver;
        $this->articleStatisticsObjectManager = $articleStatisticsObjectManager;
        $this->cacheProvider = $cacheProvider;
    }

    /**
     * @param AMQPMessage $message
     *
     * @return bool|mixed
     */
    public function execute(AMQPMessage $message)
    {
        /** @var Request $request */
        $request = unserialize($message->getBody());
        if (!$request instanceof Request) {
            return ConsumerInterface::MSG_REJECT;
        }

        try {
            $this->setTenant($request);
        } catch (TenantNotFoundException $e) {
            echo $e->getMessage()."\n";

            return ConsumerInterface::MSG_REJECT;
        }
        echo 'Set tenant: '.$this->tenantContext->getTenant()->getCode()."\n";

        if ($request->query->has('articleId')) {
            $this->handleArticlePageViews($request);

            echo 'Pageview for article '.$request->query->get('articleId')." was processed \n";
// for HTML
$sanitized = htmlentities($tainted, ENT_QUOTES);

// for URLs
$sanitized = urlencode($tainted);
        }

        if ($request->attributes->has('data') && ArticleEventInterface::ACTION_IMPRESSION === $request->query->get('type')) {
            $this->handleArticleImpressions($request);

            echo "Article impressions were processed \n";
        }

        return ConsumerInterface::MSG_ACK;
    }

    private function handleArticleImpressions(Request $request): void
    {
        $articles = [];
        if (!\is_array($request->attributes->get('data'))) {
            return;
        }

        foreach ($request->attributes->get('data') as $articleId) {
            $cacheKey = md5('article_impressions_'.$articleId);

            if ($this->cacheProvider->contains($cacheKey)) {
                $articleId = $this->cacheProvider->fetch($cacheKey);
            } elseif (filter_var($articleId, FILTER_VALIDATE_URL)) {
                try {
                    $article = $this->articleResolver->resolve($articleId);
                    if (null === $article) {
                        continue;
                    }
                } catch (\Exception $e) {
                    continue;
                }

                $articleId = $article->getId();
                $this->cacheProvider->save($cacheKey, $articleId);
            }

            if (!\array_key_exists($articleId, $articles)) {
                $articles[] = $articleId;
            }
        }

        $impressionSource = $this->getImpressionSource($request);

        $this->articleStatisticsObjectManager->getConnection()->beginTransaction();
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.
    }
}

        try {
            $ids = [];
            foreach ($articles as $articleId) {
                $articleStatistics = $this->articleStatisticsService->addArticleEvent(
                    (int) $articleId,
                    ArticleEventInterface::ACTION_IMPRESSION,
                    $impressionSource
                );

                $ids[] = $articleStatistics->getId();
                echo 'Article '.$articleId." impression was added \n";
// for HTML
$sanitized = htmlentities($tainted, ENT_QUOTES);

// for URLs
$sanitized = urlencode($tainted);
            }

            try {
                $stmt = $this->articleStatisticsObjectManager->getConnection()->prepare('LOCK TABLE swp_article_statistics IN EXCLUSIVE MODE;');
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.
    }
}
                $stmt->execute();
            } catch (\Exception $e) {
                // ignore when lock not supported
            }

            $query = $this->articleStatisticsObjectManager->createQuery('UPDATE '.ArticleStatistics::class.' s SET s.impressionsNumber = s.impressionsNumber + 1 WHERE s.id IN (:ids)');
            $query->setParameter('ids', $ids);
            $query->execute();

            $this->articleStatisticsObjectManager->flush();
            $this->articleStatisticsObjectManager->getConnection()->commit();
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.
    }
}
        } catch (\Exception $e) {
            $this->articleStatisticsObjectManager->getConnection()->rollBack();
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.
    }
}

            throw $e;
        }
    }

    private function handleArticlePageViews(Request $request): void
    {
        $articleId = $request->query->get('articleId', null);
        if (null !== $articleId && 0 !== (int) $articleId) {
            $articleStatistics = $this->articleStatisticsService->addArticleEvent((int) $articleId, ArticleEventInterface::ACTION_PAGEVIEW, [
                'pageViewSource' => $this->getPageViewSource($request),
            ]);

            $query = $this->articleStatisticsObjectManager->createQuery('UPDATE '.ArticleStatistics::class.' s SET s.pageViewsNumber = s.pageViewsNumber + 1 WHERE s.id = :id');
            $query->setParameter('id', $articleStatistics->getId());
            $query->execute();
        }
    }

    private function getImpressionSource(Request $request): array
    {
        $source = [];
        $referrer = $request->server->get('HTTP_REFERER');

        if (null === $referrer) {
            return $source;
        }

        try {
            $route = $this->matcher->match($this->getFragmentFromUrl($referrer, 'path'));
        } catch (ResourceNotFoundException $e) {
            return $source;
        }

        if (isset($route['_article_meta']) && $route['_article_meta'] instanceof Meta && $route['_article_meta']->getValues() instanceof ArticleInterface) {
            $source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_TYPE] = 'article';
            $source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_SOURCE_ARTICLE] = $route['_article_meta']->getValues();
        } elseif (isset($route['_route_meta']) && $route['_route_meta'] instanceof Meta && $route['_route_meta']->getValues() instanceof RouteInterface) {
            $source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_TYPE] = 'route';
            $source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_SOURCE_ROUTE] = $route['_route_meta']->getValues();
        } elseif (isset($route['_route']) && 'homepage' === $route['_route']) {
            $source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_TYPE] = 'homepage';
        }

        return $source;
    }

    private function getPageViewSource(Request $request): string
    {
        $pageViewReferer = $request->query->get('ref', null);
        if (null !== $pageViewReferer) {
            $refererHost = $this->getFragmentFromUrl($pageViewReferer, 'host');
            if ($refererHost && $this->isHostMatchingTenant($refererHost)) {
                return ArticleEventInterface::PAGEVIEW_SOURCE_INTERNAL;
            }
        }

        return ArticleEventInterface::PAGEVIEW_SOURCE_EXTERNAL;
    }

    private function getFragmentFromUrl(string $url, string $fragment): ?string
    {
        $fragments = \parse_url($url);
        if (!\array_key_exists($fragment, $fragments)) {
            return null;
        }

        return str_replace('/app_dev.php', '', $fragments[$fragment]);
    }

    private function isHostMatchingTenant(string $host): bool
    {
        $tenant = $this->tenantContext->getTenant();
        $tenantHost = $tenant->getDomainName();
        if (null !== ($subdomain = $tenant->getSubdomain())) {
            $tenantHost = $subdomain.'.'.$tenantHost;
        }

        return $host === $tenantHost;
    }

    /**
     * @param Request $request
     */
    private function setTenant(Request $request): void
    {
        $this->tenantContext->setTenant(
            $this->tenantResolver->resolve(
                $request->server->get('HTTP_REFERER',
                    $request->query->get('host',
                        $request->getHost()
                    )
                )
            )
        );
    }
}


Push — 1.5 ( 293f07...bc7cd6 )

AnalyticsEventConsumer::handleArticleImpressions() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

1 path for user data to reach this point

Preventing Cross-Site-Scripting Attacks

General Strategies to prevent injection

Available Fixes

1 path for user data to reach this point

Preventing Cross-Site-Scripting Attacks

General Strategies to prevent injection

Available Fixes

Available Fixes

Available Fixes

1			<?php
2
3			declare(strict_types=1);
4
5			/*
6			* This file is part of the Superdesk Web Publisher Core Bundle.
7			*
8			* Copyright 2017 Sourcefabric z.ú. and contributors.
9			*
10			* For the full copyright and license information, please see the
11			* AUTHORS and LICENSE files distributed with this source code.
12			*
13			* @copyright 2017 Sourcefabric z.ú
14			* @license http://www.superdesk.org/license
15			*/
16
17			namespace SWP\Bundle\CoreBundle\Consumer;
18
19			use Doctrine\Common\Cache\CacheProvider;
20			use Doctrine\Common\Persistence\ObjectManager;
21			use OldSound\RabbitMqBundle\RabbitMq\ConsumerInterface;
22			use PhpAmqpLib\Message\AMQPMessage;
23			use SWP\Bundle\AnalyticsBundle\Model\ArticleEventInterface;
24			use SWP\Bundle\AnalyticsBundle\Services\ArticleStatisticsServiceInterface;
25			use SWP\Bundle\ContentBundle\Model\RouteInterface;
26			use SWP\Bundle\CoreBundle\Model\ArticleInterface;
27			use SWP\Bundle\CoreBundle\Model\ArticleStatistics;
28			use SWP\Bundle\CoreBundle\Resolver\ArticleResolverInterface;
29			use SWP\Component\MultiTenancy\Context\TenantContextInterface;
30			use SWP\Component\MultiTenancy\Exception\TenantNotFoundException;
31			use SWP\Component\MultiTenancy\Resolver\TenantResolver;
32			use SWP\Component\TemplatesSystem\Gimme\Meta\Meta;
33			use Symfony\Component\HttpFoundation\Request;
34			use Symfony\Component\Routing\Exception\ResourceNotFoundException;
35			use Symfony\Component\Routing\Matcher\UrlMatcherInterface;
36
37			/**
38			* Class AnalyticsEventConsumer.
39			*/
40			final class AnalyticsEventConsumer implements ConsumerInterface
41			{
42			/**
43			* @var ArticleStatisticsServiceInterface
44			*/
45			private $articleStatisticsService;
46
47			/**
48			* @var TenantResolver
49			*/
50			private $tenantResolver;
51
52			/**
53			* @var TenantContextInterface
54			*/
55			private $tenantContext;
56
57			/**
58			* @var UrlMatcherInterface
59			*/
60			private $matcher;
61
62			/**
63			* @var ArticleResolverInterface
64			*/
65			private $articleResolver;
66
67			private $articleStatisticsObjectManager;
68
69			private $cacheProvider;
70
71			public function __construct(
72			ArticleStatisticsServiceInterface $articleStatisticsService,
73			TenantResolver $tenantResolver,
74			TenantContextInterface $tenantContext,
75			UrlMatcherInterface $matcher,
76			ArticleResolverInterface $articleResolver,
77			ObjectManager $articleStatisticsObjectManager,
78			CacheProvider $cacheProvider
79			) {
80			$this->articleStatisticsService = $articleStatisticsService;
81			$this->tenantResolver = $tenantResolver;
82			$this->tenantContext = $tenantContext;
83			$this->matcher = $matcher;
84			$this->articleResolver = $articleResolver;
85			$this->articleStatisticsObjectManager = $articleStatisticsObjectManager;
86			$this->cacheProvider = $cacheProvider;
87			}
88
89			/**
90			* @param AMQPMessage $message
91			*
92			* @return bool\|mixed
93			*/
94			public function execute(AMQPMessage $message)
95			{
96			/** @var Request $request */
97			$request = unserialize($message->getBody());
98			if (!$request instanceof Request) {
99			return ConsumerInterface::MSG_REJECT;
100			}
101
102			try {
103			$this->setTenant($request);
104			} catch (TenantNotFoundException $e) {
105			echo $e->getMessage()."\n";
106
107			return ConsumerInterface::MSG_REJECT;
108			}
109			echo 'Set tenant: '.$this->tenantContext->getTenant()->getCode()."\n";
110
111			if ($request->query->has('articleId')) {
112			$this->handleArticlePageViews($request);
113
114			echo 'Pageview for article '.$request->query->get('articleId')." was processed \n";
			0 ignored issues – show Security Cross-Site Scripting introduced 2018-09-06 10:15 UTC by Report Bug Copy Issue Report `'Pageview for article ' ...') . ' was processed '` can contain request data and is used in output context(s) leading to a potential security vulnerability. 1 path for user data to reach this point ParameterBag::get() returns tainted data in src/SWP/Bundle/CoreBundle/Consumer/AnalyticsEventConsumer.php on line 114 Preventing Cross-Site-Scripting Attacks Cross-Site-Scripting allows an attacker to inject malicious code into your website - in particular Javascript code, and have that code executed with the privileges of a visiting user. This can be used to obtain data, or perform actions on behalf of that visiting user. In order to prevent this, make sure to escape all user-provided data: // for HTML $sanitized = htmlentities($tainted, ENT_QUOTES); // for URLs $sanitized = urlencode($tainted); General Strategies to prevent injection In general, it is advisable to prevent any user-data to reach this point. This can be done by white-listing certain values: if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) { throw new \InvalidArgumentException('This input is not allowed.'); } For numeric data, we recommend to explicitly cast the data: $sanitized = (integer) $tainted; Loading history...
115			}
116
117			if ($request->attributes->has('data') && ArticleEventInterface::ACTION_IMPRESSION === $request->query->get('type')) {
118			$this->handleArticleImpressions($request);
119
120			echo "Article impressions were processed \n";
121			}
122
123			return ConsumerInterface::MSG_ACK;
124			}
125
126			private function handleArticleImpressions(Request $request): void
127			{
128			$articles = [];
129			if (!\is_array($request->attributes->get('data'))) {
130			return;
131			}
132
133			foreach ($request->attributes->get('data') as $articleId) {
134			$cacheKey = md5('article_impressions_'.$articleId);
135
136			if ($this->cacheProvider->contains($cacheKey)) {
137			$articleId = $this->cacheProvider->fetch($cacheKey);
138			} elseif (filter_var($articleId, FILTER_VALIDATE_URL)) {
139			try {
140			$article = $this->articleResolver->resolve($articleId);
141			if (null === $article) {
142			continue;
143			}
144			} catch (\Exception $e) {
145			continue;
146			}
147
148			$articleId = $article->getId();
149			$this->cacheProvider->save($cacheKey, $articleId);
150			}
151
152			if (!\array_key_exists($articleId, $articles)) {
153			$articles[] = $articleId;
154			}
155			}
156
157			$impressionSource = $this->getImpressionSource($request);
158
159			$this->articleStatisticsObjectManager->getConnection()->beginTransaction();
			0 ignored issues – show Bug introduced 2019-06-07 08:21 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Doctrine\Common\Persistence\ObjectManager` as the method `getConnection()` does only exist in the following implementations of said interface: `Doctrine\ORM\Decorator\EntityManagerDecorator`, `Doctrine\ORM\EntityManager`, `Pixers\DoctrineProfilerBundle\ORM\EntityManager`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
160
161			try {
162			$ids = [];
163			foreach ($articles as $articleId) {
164			$articleStatistics = $this->articleStatisticsService->addArticleEvent(
165			(int) $articleId,
166			ArticleEventInterface::ACTION_IMPRESSION,
167			$impressionSource
168			);
169
170			$ids[] = $articleStatistics->getId();
171			echo 'Article '.$articleId." impression was added \n";
			0 ignored issues – show Security Cross-Site Scripting introduced 2019-06-05 14:25 UTC by Report Bug Copy Issue Report `'Article ' . $articleId ...impression was added '` can contain request data and is used in output context(s) leading to a potential security vulnerability. 1 path for user data to reach this point ParameterBag::get() returns tainted data, and `$articleId` is assigned in src/SWP/Bundle/CoreBundle/Consumer/AnalyticsEventConsumer.php on line 133 `$articles` is assigned in src/SWP/Bundle/CoreBundle/Consumer/AnalyticsEventConsumer.php on line 153 `$articleId` is assigned in src/SWP/Bundle/CoreBundle/Consumer/AnalyticsEventConsumer.php on line 163 Preventing Cross-Site-Scripting Attacks Cross-Site-Scripting allows an attacker to inject malicious code into your website - in particular Javascript code, and have that code executed with the privileges of a visiting user. This can be used to obtain data, or perform actions on behalf of that visiting user. In order to prevent this, make sure to escape all user-provided data: // for HTML $sanitized = htmlentities($tainted, ENT_QUOTES); // for URLs $sanitized = urlencode($tainted); General Strategies to prevent injection In general, it is advisable to prevent any user-data to reach this point. This can be done by white-listing certain values: if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) { throw new \InvalidArgumentException('This input is not allowed.'); } For numeric data, we recommend to explicitly cast the data: $sanitized = (integer) $tainted; Loading history...
172			}
173
174			try {
175			$stmt = $this->articleStatisticsObjectManager->getConnection()->prepare('LOCK TABLE swp_article_statistics IN EXCLUSIVE MODE;');
			0 ignored issues – show Bug introduced 2019-06-07 08:21 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Doctrine\Common\Persistence\ObjectManager` as the method `getConnection()` does only exist in the following implementations of said interface: `Doctrine\ORM\Decorator\EntityManagerDecorator`, `Doctrine\ORM\EntityManager`, `Pixers\DoctrineProfilerBundle\ORM\EntityManager`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
176			$stmt->execute();
177			} catch (\Exception $e) {
178			// ignore when lock not supported
179			}
180
181			$query = $this->articleStatisticsObjectManager->createQuery('UPDATE '.ArticleStatistics::class.' s SET s.impressionsNumber = s.impressionsNumber + 1 WHERE s.id IN (:ids)');
182			$query->setParameter('ids', $ids);
183			$query->execute();
184
185			$this->articleStatisticsObjectManager->flush();
186			$this->articleStatisticsObjectManager->getConnection()->commit();
			0 ignored issues – show Bug introduced 2019-06-07 08:21 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Doctrine\Common\Persistence\ObjectManager` as the method `getConnection()` does only exist in the following implementations of said interface: `Doctrine\ORM\Decorator\EntityManagerDecorator`, `Doctrine\ORM\EntityManager`, `Pixers\DoctrineProfilerBundle\ORM\EntityManager`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
187			} catch (\Exception $e) {
188			$this->articleStatisticsObjectManager->getConnection()->rollBack();
			0 ignored issues – show Bug introduced 2019-06-07 08:21 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Doctrine\Common\Persistence\ObjectManager` as the method `getConnection()` does only exist in the following implementations of said interface: `Doctrine\ORM\Decorator\EntityManagerDecorator`, `Doctrine\ORM\EntityManager`, `Pixers\DoctrineProfilerBundle\ORM\EntityManager`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
189
190			throw $e;
191			}
192			}
193
194			private function handleArticlePageViews(Request $request): void
195			{
196			$articleId = $request->query->get('articleId', null);
197			if (null !== $articleId && 0 !== (int) $articleId) {
198			$articleStatistics = $this->articleStatisticsService->addArticleEvent((int) $articleId, ArticleEventInterface::ACTION_PAGEVIEW, [
199			'pageViewSource' => $this->getPageViewSource($request),
200			]);
201
202			$query = $this->articleStatisticsObjectManager->createQuery('UPDATE '.ArticleStatistics::class.' s SET s.pageViewsNumber = s.pageViewsNumber + 1 WHERE s.id = :id');
203			$query->setParameter('id', $articleStatistics->getId());
204			$query->execute();
205			}
206			}
207
208			private function getImpressionSource(Request $request): array
209			{
210			$source = [];
211			$referrer = $request->server->get('HTTP_REFERER');
212
213			if (null === $referrer) {
214			return $source;
215			}
216
217			try {
218			$route = $this->matcher->match($this->getFragmentFromUrl($referrer, 'path'));
219			} catch (ResourceNotFoundException $e) {
220			return $source;
221			}
222
223			if (isset($route['_article_meta']) && $route['_article_meta'] instanceof Meta && $route['_article_meta']->getValues() instanceof ArticleInterface) {
224			$source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_TYPE] = 'article';
225			$source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_SOURCE_ARTICLE] = $route['_article_meta']->getValues();
226			} elseif (isset($route['_route_meta']) && $route['_route_meta'] instanceof Meta && $route['_route_meta']->getValues() instanceof RouteInterface) {
227			$source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_TYPE] = 'route';
228			$source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_SOURCE_ROUTE] = $route['_route_meta']->getValues();
229			} elseif (isset($route['_route']) && 'homepage' === $route['_route']) {
230			$source[ArticleStatisticsServiceInterface::KEY_IMPRESSION_TYPE] = 'homepage';
231			}
232
233			return $source;
234			}
235
236			private function getPageViewSource(Request $request): string
237			{
238			$pageViewReferer = $request->query->get('ref', null);
239			if (null !== $pageViewReferer) {
240			$refererHost = $this->getFragmentFromUrl($pageViewReferer, 'host');
241			if ($refererHost && $this->isHostMatchingTenant($refererHost)) {
242			return ArticleEventInterface::PAGEVIEW_SOURCE_INTERNAL;
243			}
244			}
245
246			return ArticleEventInterface::PAGEVIEW_SOURCE_EXTERNAL;
247			}
248
249			private function getFragmentFromUrl(string $url, string $fragment): ?string
250			{
251			$fragments = \parse_url($url);
252			if (!\array_key_exists($fragment, $fragments)) {
253			return null;
254			}
255
256			return str_replace('/app_dev.php', '', $fragments[$fragment]);
257			}
258
259			private function isHostMatchingTenant(string $host): bool
260			{
261			$tenant = $this->tenantContext->getTenant();
262			$tenantHost = $tenant->getDomainName();
263			if (null !== ($subdomain = $tenant->getSubdomain())) {
264			$tenantHost = $subdomain.'.'.$tenantHost;
265			}
266
267			return $host === $tenantHost;
268			}
269
270			/**
271			* @param Request $request
272			*/
273			private function setTenant(Request $request): void
274			{
275			$this->tenantContext->setTenant(
276			$this->tenantResolver->resolve(
277			$request->server->get('HTTP_REFERER',
278			$request->query->get('host',
279			$request->getHost()
280			)
281			)
282			)
283			);
284			}
285			}
286

superdesk / web-publisher

Push — 1.5 ( 293f07...bc7cd6 )

AnalyticsEventConsumer::handleArticleImpressions() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

1 path for user data to reach this point

Preventing Cross-Site-Scripting Attacks

General Strategies to prevent injection

Available Fixes

1 path for user data to reach this point

Preventing Cross-Site-Scripting Attacks

General Strategies to prevent injection

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like