These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more
1 | <?php |
||
2 | |||
3 | namespace Kunstmaan\SeoBundle\Twig; |
||
4 | |||
5 | use Doctrine\ORM\EntityManager; |
||
6 | use Kunstmaan\AdminBundle\Entity\AbstractEntity; |
||
7 | use Kunstmaan\NodeBundle\Entity\AbstractPage; |
||
8 | use Kunstmaan\SeoBundle\Entity\Seo; |
||
9 | use Psr\Cache\CacheItemPoolInterface; |
||
10 | use Twig\Environment; |
||
11 | use Twig\Extension\AbstractExtension; |
||
12 | use Twig\TwigFunction; |
||
13 | |||
14 | /** |
||
15 | * Twig extensions for Seo |
||
16 | * |
||
17 | * @final since 5.4 |
||
18 | */ |
||
19 | class SeoTwigExtension extends AbstractExtension |
||
20 | { |
||
21 | /** |
||
22 | * @var EntityManager |
||
23 | */ |
||
24 | protected $em; |
||
25 | |||
26 | /** |
||
27 | * Website title defined in your parameters |
||
28 | * |
||
29 | * @var string |
||
30 | */ |
||
31 | private $websiteTitle; |
||
32 | |||
33 | /** |
||
34 | * Saves querying the db multiple times, if you happen to use any of the defined |
||
35 | * functions more than once in your templates |
||
36 | * |
||
37 | * @var array |
||
38 | */ |
||
39 | private $seoCache = []; |
||
40 | |||
41 | /** |
||
42 | * @var CacheItemPoolInterface |
||
43 | */ |
||
44 | private $requestCache; |
||
45 | |||
46 | /** |
||
47 | * @param EntityManager $em |
||
48 | */ |
||
49 | public function __construct(EntityManager $em) |
||
0 ignored issues
–
show
|
|||
50 | { |
||
51 | $this->em = $em; |
||
52 | } |
||
53 | |||
54 | /** |
||
55 | * Returns a list of functions to add to the existing list. |
||
56 | * |
||
57 | * @return array An array of functions |
||
0 ignored issues
–
show
|
|||
58 | */ |
||
59 | public function getFunctions() |
||
60 | { |
||
61 | return array( |
||
62 | new TwigFunction('render_seo_metadata_for', array($this, 'renderSeoMetadataFor'), array('is_safe' => array('html'), 'needs_environment' => true)), |
||
63 | new TwigFunction('get_seo_for', array($this, 'getSeoFor')), |
||
64 | new TwigFunction('get_title_for', array($this, 'getTitleFor')), |
||
65 | new TwigFunction('get_title_for_page_or_default', array($this, 'getTitleForPageOrDefault')), |
||
66 | new TwigFunction('get_absolute_url', array($this, 'getAbsoluteUrl')), |
||
67 | new TwigFunction('get_image_dimensions', array($this, 'getImageDimensions')), |
||
68 | ); |
||
69 | } |
||
70 | |||
71 | /** |
||
72 | * Validates the $url value as URL (according to » http://www.faqs.org/rfcs/rfc2396), optionally with required components. |
||
73 | * It will just return the url if it's valid. If it starts with '/', the $host will be prepended. |
||
74 | * |
||
75 | * @param string $url |
||
76 | * @param string $host |
||
0 ignored issues
–
show
Should the type for parameter
$host not be string|null ?
This check looks for It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. ![]() |
|||
77 | * |
||
78 | * @return string |
||
0 ignored issues
–
show
|
|||
79 | */ |
||
80 | public function getAbsoluteUrl($url, $host = null) |
||
81 | { |
||
82 | $validUrl = filter_var($url, FILTER_VALIDATE_URL); |
||
83 | $host = rtrim($host, '/'); |
||
84 | |||
85 | if (!$validUrl === false) { |
||
86 | // The url is valid |
||
87 | return $url; |
||
88 | } |
||
89 | |||
90 | // Prepend with $host if $url starts with "/" |
||
91 | if (strpos($url, '/') === 0) { |
||
92 | return $url = $host.$url; |
||
0 ignored issues
–
show
$url is not used, you could remove the assignment.
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value';
$higher = false;
if (rand(1, 6) > 3) {
$higher = true;
} else {
$higher = false;
}
Both the ![]() |
|||
93 | } |
||
94 | |||
95 | return false; |
||
96 | } |
||
97 | |||
98 | /** |
||
99 | * @param AbstractPage $entity |
||
100 | * |
||
101 | * @return Seo |
||
102 | */ |
||
103 | public function getSeoFor(AbstractPage $entity) |
||
104 | { |
||
105 | $key = md5(\get_class($entity).$entity->getId()); |
||
106 | |||
107 | if (!\array_key_exists($key, $this->seoCache)) { |
||
108 | $seo = $this->em->getRepository(Seo::class)->findOrCreateFor($entity); |
||
0 ignored issues
–
show
It seems like you code against a concrete implementation and not the interface
Doctrine\Persistence\ObjectRepository as the method findOrCreateFor() does only exist in the following implementations of said interface: Kunstmaan\SeoBundle\Repository\SeoRepository .
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
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
![]() |
|||
109 | $this->seoCache[$key] = $seo; |
||
110 | } |
||
111 | |||
112 | return $this->seoCache[$key]; |
||
113 | } |
||
114 | |||
115 | /** |
||
116 | * The first value that is not null or empty will be returned. |
||
117 | * |
||
118 | * @param AbstractPage $entity the entity for which you want the page title |
||
119 | * |
||
120 | * @return string The page title. Will look in the SEO meta first, then the NodeTranslation, then the page. |
||
121 | */ |
||
122 | public function getTitleFor(AbstractPage $entity) |
||
123 | { |
||
124 | $arr = array(); |
||
125 | |||
126 | $arr[] = $this->getSeoTitle($entity); |
||
127 | |||
128 | $arr[] = $entity->getTitle(); |
||
129 | |||
130 | return $this->getPreferredValue($arr); |
||
131 | } |
||
132 | |||
133 | /** |
||
134 | * @param AbstractPage $entity |
||
0 ignored issues
–
show
Should the type for parameter
$entity not be null|AbstractPage ?
This check looks for It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. ![]() |
|||
135 | * @param null|string $default if given we'll return this text if no SEO title was found |
||
136 | * |
||
137 | * @return string |
||
0 ignored issues
–
show
|
|||
138 | */ |
||
139 | public function getTitleForPageOrDefault(AbstractPage $entity = null, $default = null) |
||
140 | { |
||
141 | if (\is_null($entity)) { |
||
142 | return $default; |
||
143 | } |
||
144 | |||
145 | $arr = array(); |
||
146 | |||
147 | $arr[] = $this->getSeoTitle($entity); |
||
148 | |||
149 | $arr[] = $default; |
||
150 | |||
151 | $arr[] = $entity->getTitle(); |
||
152 | |||
153 | return $this->getPreferredValue($arr); |
||
154 | } |
||
155 | |||
156 | /** |
||
157 | * @param Environment $environment |
||
158 | * @param AbstractEntity $entity The entity |
||
159 | * @param mixed $currentNode The current node |
||
160 | * @param string $template The template |
||
161 | * |
||
162 | * @return string |
||
163 | */ |
||
164 | public function renderSeoMetadataFor(Environment $environment, AbstractEntity $entity, $currentNode = null, $template = '@KunstmaanSeo/SeoTwigExtension/metadata.html.twig') |
||
165 | { |
||
166 | $seo = $this->getSeoFor($entity); |
||
0 ignored issues
–
show
$entity of type object<Kunstmaan\AdminBu...\Entity\AbstractEntity> is not a sub-type of object<Kunstmaan\NodeBundle\Entity\AbstractPage> . It seems like you assume a child class of the class Kunstmaan\AdminBundle\Entity\AbstractEntity to be always present.
This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass. Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type. ![]() |
|||
167 | $template = $environment->load($template); |
||
168 | |||
169 | return $template->render( |
||
170 | array( |
||
171 | 'seo' => $seo, |
||
172 | 'entity' => $entity, |
||
173 | 'currentNode' => $currentNode, |
||
174 | ) |
||
175 | ); |
||
176 | } |
||
177 | |||
178 | /** |
||
179 | * @param array $values |
||
180 | * |
||
181 | * @return string |
||
182 | */ |
||
183 | protected function getPreferredValue(array $values) |
||
184 | { |
||
185 | foreach ($values as $v) { |
||
186 | if (!\is_null($v) && !empty($v)) { |
||
187 | return $v; |
||
188 | } |
||
189 | } |
||
190 | |||
191 | return ''; |
||
192 | } |
||
193 | |||
194 | /** |
||
195 | * @param AbstractPage $entity |
||
0 ignored issues
–
show
Should the type for parameter
$entity not be null|AbstractPage ?
This check looks for It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. ![]() |
|||
196 | * |
||
197 | * @return null|string |
||
198 | */ |
||
199 | private function getSeoTitle(AbstractPage $entity = null) |
||
200 | { |
||
201 | if (\is_null($entity)) { |
||
202 | return null; |
||
203 | } |
||
204 | |||
205 | $seo = $this->getSeoFor($entity); |
||
206 | if (!\is_null($seo)) { |
||
207 | $title = $seo->getMetaTitle(); |
||
208 | if (!empty($title)) { |
||
209 | return str_replace('%websitetitle%', $this->getWebsiteTitle(), $title); |
||
210 | } |
||
211 | } |
||
212 | |||
213 | return null; |
||
214 | } |
||
215 | |||
216 | /** |
||
217 | * Gets the Website title defined in your parameters. |
||
218 | * |
||
219 | * @return string |
||
220 | */ |
||
221 | public function getWebsiteTitle() |
||
222 | { |
||
223 | return $this->websiteTitle; |
||
224 | } |
||
225 | |||
226 | /** |
||
227 | * Sets the Website title defined in your parameters. |
||
228 | * |
||
229 | * @param string $websiteTitle the website title |
||
230 | * |
||
231 | * @return self |
||
232 | */ |
||
233 | public function setWebsiteTitle($websiteTitle) |
||
234 | { |
||
235 | $this->websiteTitle = $websiteTitle; |
||
236 | |||
237 | return $this; |
||
238 | } |
||
239 | |||
240 | /** |
||
241 | * @param $src |
||
242 | * |
||
243 | * @return array |
||
244 | */ |
||
245 | public function getImageDimensions($src) |
||
246 | { |
||
247 | list($width, $height) = $this->getImageSize($src); |
||
248 | |||
249 | return ['width' => $width, 'height' => $height]; |
||
250 | } |
||
251 | |||
252 | public function setRequestCache(CacheItemPoolInterface $cacheService) |
||
253 | { |
||
254 | $this->requestCache = $cacheService; |
||
255 | } |
||
256 | |||
257 | /** |
||
258 | * @return CacheItemPoolInterface |
||
259 | */ |
||
260 | public function getRequestCache() |
||
261 | { |
||
262 | return $this->requestCache; |
||
263 | } |
||
264 | |||
265 | private function getImageSize($src) |
||
0 ignored issues
–
show
The return type could not be reliably inferred; please add a
@return annotation.
Our type inference engine in quite powerful, but sometimes the code does not
provide enough clues to go by. In these cases we request you to add a ![]() |
|||
266 | { |
||
267 | try { |
||
268 | $cache = $this->getRequestCache(); |
||
269 | if (null === $cache) { |
||
270 | return getimagesize($src); |
||
271 | } |
||
272 | |||
273 | $cachedImageSizes = $cache->getItem(md5($src)); |
||
274 | if (!$cachedImageSizes->isHit()) { |
||
275 | $sizes = getimagesize($src); |
||
276 | |||
277 | $cachedImageSizes->set($sizes); |
||
278 | $cache->save($cachedImageSizes); |
||
279 | } |
||
280 | |||
281 | return $cachedImageSizes->get(); |
||
282 | } catch (\Exception $e) { |
||
283 | return [null, null]; |
||
284 | } |
||
285 | } |
||
286 | } |
||
287 |
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:If that code throws an exception and the
EntityManager
is closed. Any other code which depends on the same instance of theEntityManager
during this request will fail.On the other hand, if you instead inject the
ManagerRegistry
, thegetManager()
method guarantees that you will always get a usable manager instance.