Completed
Push — master ( 6dc195...257818 )
by Jeroen
04:37 queued 04:31
created

URLHelper::replaceUrl()   C

Complexity

Conditions 16
Paths 18

Size

Total Lines 63

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 30
CRAP Score 16.4163

Importance

Changes 0
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   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

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 by Derek Stephen McLean
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 by Sander Goossens
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.

  1. 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) { /** ... */ }
    
  2. 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) { /** .. */ }
    
  3. 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 by Jeroen Thora
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 by Sander Goossens
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.

  1. 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) { /** ... */ }
    
  2. 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) { /** .. */ }
    
  3. 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 by Jeroen Thora
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