URLHelper::replaceUrl() - Code Metrics - Inspection of "Merge pull request #2678 from acrobat/improve-url-..." - Kunstmaan/KunstmaanBundlesCMS - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 6dc195...257818 )

by Jeroen

created 2020-06-25 11:25 UTC

URLHelper::replaceUrl() C

↳ Parent: URLHelper

Complexity

Conditions	16
Paths	18

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	30
CRAP Score	16.4163

Importance

Changes

Metric	Value
dl	0
loc	63
ccs	30
cts	34
cp	0.8824
rs	5.5666
c	0
b	0
f	0
cc	16
nc	18
nop	1
crap	16.4163

How to fix Long Method Complexity

<?php

namespace Kunstmaan\NodeBundle\Helper;

use Doctrine\ORM\EntityManager;
use Kunstmaan\AdminBundle\Helper\DomainConfigurationInterface;
use Kunstmaan\NodeBundle\Validation\URLValidator;
use Psr\Log\LoggerInterface;
use Symfony\Component\Routing\RouterInterface;

/**
 * A helper for replacing url's
 */
class URLHelper
{
    use URLValidator;

    /**
     * @var EntityManager
     */
    private $em;

    /**
     * @var RouterInterface
     */
    private $router;

    /**
     * @var LoggerInterface
     */
    private $logger;

    /** @var array */
    private $nodeTranslationCache = [];

    /** @var array */
    private $mediaCache = [];

    /**
     * @var DomainConfigurationInterface
     */
    private $domainConfiguration;

    /**
     * @param EntityManager                $em
     * @param RouterInterface              $router
     * @param LoggerInterface              $logger
     * @param DomainConfigurationInterface $domainConfiguration
     */
    public function __construct(EntityManager $em, RouterInterface $router, LoggerInterface $logger, DomainConfigurationInterface $domainConfiguration)
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;
    }
}
    {
        $this->em = $em;
        $this->router = $router;
        $this->logger = $logger;
        $this->domainConfiguration = $domainConfiguration;
    }

    /**
     * Replace a given text, according to the node translation id and the multidomain site id.
     *
     * @param string $text
     *
     * @return string
     */
    public function replaceUrl($text)
    {
        if ($this->isEmailAddress($text)) {
            $text = sprintf('%s:%s', 'mailto', $text);
        }

        if ($this->isInternalLink($text)) {
            preg_match_all("/\[(([a-z_A-Z\.]+):)?NT([0-9]+)\]/", $text, $matches, PREG_SET_ORDER);

            if (\count($matches) > 0) {
                foreach ($matches as $match) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
                    $fullTag = $match[0];
                    $hostId = $match[2];

                    $hostConfig = !empty($hostId) ? $this->domainConfiguration->getFullHostById($hostId) : null;
                    $host = null !== $hostConfig && array_key_exists('host', $hostConfig) ? $hostConfig['host'] : null;
                    $hostBaseUrl = $this->domainConfiguration->getHostBaseUrl($host);

                    $nodeTranslationId = $match[3];
                    $nodeTranslation = $this->getNodeTranslation($nodeTranslationId);

                    if ($nodeTranslation) {

                        $urlParams = ['url' => $nodeTranslation['url']];
                        // Only add locale if multilingual site
                        if ($this->domainConfiguration->isMultiLanguage($host)) {
                            $urlParams['_locale'] = $nodeTranslation['lang'];
                        }

                        // Only add other site, when having this.
                        if ($hostId) {
                            $urlParams['otherSite'] = $hostId;
                        }

                        $url = $this->router->generate('_slug', $urlParams);

                        $text = str_replace($fullTag, $hostId ? $hostBaseUrl . $url : $url, $text);
                    } else {
                        $this->logger->error('No NodeTranslation found in the database when replacing url tag ' . $fullTag);
                    }
                }
            }
        }

        if ($this->isInternalMediaLink($text)) {
            preg_match_all("/\[(([a-z_A-Z]+):)?M([0-9]+)\]/", $text, $matches, PREG_SET_ORDER);

            if (\count($matches) > 0) {
                foreach ($matches as $match) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
                    $fullTag = $match[0];
                    $mediaId = $match[3];

                    $mediaItem = $this->getMedia($mediaId);
                    if ($mediaItem) {

                        $text = str_replace($fullTag, $mediaItem['url'], $text);
                    } else {
                        $this->logger->error('No Media found in the database when replacing url tag ' . $fullTag);
                    }
                }
            }
        }

        return $text;
    }

    private function getNodeTranslation($nodeTranslationId): array
    {
        if (isset($this->nodeTranslationCache[$nodeTranslationId])) {
            return $this->nodeTranslationCache[$nodeTranslationId];
        }

        $stmt = $this->em->getConnection()->executeQuery(
            'SELECT url, lang FROM kuma_node_translations WHERE id = :nodeTranslationId',
            ['nodeTranslationId' => $nodeTranslationId]
        );
        $nodeTranslation = $stmt->fetch();

        $this->nodeTranslationCache[$nodeTranslationId] = $nodeTranslation;

        return $nodeTranslation;
    }

    private function getMedia($mediaId): array
    {
        if (isset($this->mediaCache[$mediaId])) {
            return $this->mediaCache[$mediaId];
        }

        $stmt = $this->em->getConnection()->executeQuery(
            'SELECT url FROM kuma_media WHERE id = :mediaId',
            ['mediaId' => $mediaId]
        );
        $media = $stmt->fetch();

        $this->mediaCache[$mediaId] = $media;

        return $media;
    }
}


1		<?php
2
3		namespace Kunstmaan\NodeBundle\Helper;
4
5		use Doctrine\ORM\EntityManager;
6		use Kunstmaan\AdminBundle\Helper\DomainConfigurationInterface;
7		use Kunstmaan\NodeBundle\Validation\URLValidator;
8		use Psr\Log\LoggerInterface;
9		use Symfony\Component\Routing\RouterInterface;
10
11		/**
12		* A helper for replacing url's
13		*/
14		class URLHelper
15		{
16		use URLValidator;
17
18		/**
19		* @var EntityManager
20		*/
21		private $em;
22
23		/**
24		* @var RouterInterface
25		*/
26		private $router;
27
28		/**
29		* @var LoggerInterface
30		*/
31		private $logger;
32
33		/** @var array */
34		private $nodeTranslationCache = [];
35
36		/** @var array */
37		private $mediaCache = [];
38
39		/**
40		* @var DomainConfigurationInterface
41		*/
42		private $domainConfiguration;
43
44		/**
45		* @param EntityManager $em
46		* @param RouterInterface $router
47		* @param LoggerInterface $logger
48		* @param DomainConfigurationInterface $domainConfiguration
49		*/
50	3	public function __construct(EntityManager $em, RouterInterface $router, LoggerInterface $logger, DomainConfigurationInterface $domainConfiguration)
		0 ignored issues – show Bug introduced 2018-04-18 13:05 UTC by Report Bug Copy Issue Report You have injected the EntityManager via parameter `$em`. 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...
51		{
52	3	$this->em = $em;
53	3	$this->router = $router;
54	3	$this->logger = $logger;
55	3	$this->domainConfiguration = $domainConfiguration;
56	3	}
57
58		/**
59		* Replace a given text, according to the node translation id and the multidomain site id.
60		*
61		* @param string $text
62		*
63		* @return string
64		*/
65	3	public function replaceUrl($text)
66		{
67	3	if ($this->isEmailAddress($text)) {
68	1	$text = sprintf('%s:%s', 'mailto', $text);
69		}
70
71	3	if ($this->isInternalLink($text)) {
72	1	preg_match_all("/\[(([a-z_A-Z\.]+):)?NT([0-9]+)\]/", $text, $matches, PREG_SET_ORDER);
73
74	1	if (\count($matches) > 0) {
75	1	foreach ($matches as $match) {
		0 ignored issues – show Bug introduced 2020-05-17 14:09 UTC by Report Bug Copy Issue Report The expression `$matches` of type `null\|array<integer,array<integer,string>>` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } If you are sure that the expression is traversable, you might want to add a doc comment cast* to improve IDE auto-completion and static analysis: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
76	1	$fullTag = $match[0];
77	1	$hostId = $match[2];
78
79	1	$hostConfig = !empty($hostId) ? $this->domainConfiguration->getFullHostById($hostId) : null;
80	1	$host = null !== $hostConfig && array_key_exists('host', $hostConfig) ? $hostConfig['host'] : null;
81	1	$hostBaseUrl = $this->domainConfiguration->getHostBaseUrl($host);
82
83	1	$nodeTranslationId = $match[3];
84	1	$nodeTranslation = $this->getNodeTranslation($nodeTranslationId);
85
86	1	if ($nodeTranslation) {
		0 ignored issues – show Bug Best Practice introduced 2020-05-17 14:09 UTC by Report Bug Copy Issue Report The expression `$nodeTranslation` 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...
87	1	$urlParams = ['url' => $nodeTranslation['url']];
88		// Only add locale if multilingual site
89	1	if ($this->domainConfiguration->isMultiLanguage($host)) {
90		$urlParams['_locale'] = $nodeTranslation['lang'];
91		}
92
93		// Only add other site, when having this.
94	1	if ($hostId) {
95		$urlParams['otherSite'] = $hostId;
96		}
97
98	1	$url = $this->router->generate('_slug', $urlParams);
99
100	1	$text = str_replace($fullTag, $hostId ? $hostBaseUrl . $url : $url, $text);
101		} else {
102		$this->logger->error('No NodeTranslation found in the database when replacing url tag ' . $fullTag);
103		}
104		}
105		}
106		}
107
108	3	if ($this->isInternalMediaLink($text)) {
109	1	preg_match_all("/\[(([a-z_A-Z]+):)?M([0-9]+)\]/", $text, $matches, PREG_SET_ORDER);
110
111	1	if (\count($matches) > 0) {
112	1	foreach ($matches as $match) {
		0 ignored issues – show Bug introduced 2020-05-17 14:09 UTC by Report Bug Copy Issue Report The expression `$matches` of type `null\|array<integer,array<integer,string>>` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } If you are sure that the expression is traversable, you might want to add a doc comment cast* to improve IDE auto-completion and static analysis: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
113	1	$fullTag = $match[0];
114	1	$mediaId = $match[3];
115
116	1	$mediaItem = $this->getMedia($mediaId);
117	1	if ($mediaItem) {
		0 ignored issues – show Bug Best Practice introduced 2020-05-17 14:09 UTC by Report Bug Copy Issue Report The expression `$mediaItem` 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...
118	1	$text = str_replace($fullTag, $mediaItem['url'], $text);
119		} else {
120		$this->logger->error('No Media found in the database when replacing url tag ' . $fullTag);
121		}
122		}
123		}
124		}
125
126	3	return $text;
127		}
128
129	1	private function getNodeTranslation($nodeTranslationId): array
130		{
131	1	if (isset($this->nodeTranslationCache[$nodeTranslationId])) {
132	1	return $this->nodeTranslationCache[$nodeTranslationId];
133		}
134
135	1	$stmt = $this->em->getConnection()->executeQuery(
136	1	'SELECT url, lang FROM kuma_node_translations WHERE id = :nodeTranslationId',
137	1	['nodeTranslationId' => $nodeTranslationId]
138		);
139	1	$nodeTranslation = $stmt->fetch();
140
141	1	$this->nodeTranslationCache[$nodeTranslationId] = $nodeTranslation;
142
143	1	return $nodeTranslation;
144		}
145
146	1	private function getMedia($mediaId): array
147		{
148	1	if (isset($this->mediaCache[$mediaId])) {
149	1	return $this->mediaCache[$mediaId];
150		}
151
152	1	$stmt = $this->em->getConnection()->executeQuery(
153	1	'SELECT url FROM kuma_media WHERE id = :mediaId',
154	1	['mediaId' => $mediaId]
155		);
156	1	$media = $stmt->fetch();
157
158	1	$this->mediaCache[$mediaId] = $media;
159
160	1	return $media;
161		}
162		}
163

Kunstmaan / KunstmaanBundlesCMS

Push — master ( 6dc195...257818 )

URLHelper::replaceUrl() C

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like