[TASK] Reduce logic in render methods of YouTube and Vimeo Renderer 41/57341/9
authorDaniel Goerz <ervaude@gmail.com>
Sat, 23 Jun 2018 12:45:01 +0000 (14:45 +0200)
committerStefan Neufeind <typo3.neufeind@speedpartner.de>
Fri, 29 Jun 2018 06:42:23 +0000 (08:42 +0200)
This patch moves the logic from the render method of the
YouTube and Vimeo Renderer to dedicated methods to keep
the render methods themselves as short, clean and simple as
possible.

Also $options is initialized as an empty array as stated
in the interface.

Functionality, Output and Behavior stay unchanged.

Resolves: #85362
Releases: master, 8.7
Change-Id: I00bfc9d7e3bbf97d8a68fbf825a6cf3dd8b2d3aa
Reviewed-on: https://review.typo3.org/57341
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Frans Saris <franssaris@gmail.com>
Tested-by: Frans Saris <franssaris@gmail.com>
Reviewed-by: Helmut Hummel <typo3@helhum.io>
Reviewed-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Tested-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
typo3/sysext/core/Classes/Resource/Rendering/VimeoRenderer.php
typo3/sysext/core/Classes/Resource/Rendering/YouTubeRenderer.php
typo3/sysext/core/Tests/Unit/Resource/Rendering/YouTubeRendererTest.php

index 0042fc5..56ea7da 100644 (file)
@@ -88,7 +88,25 @@ class VimeoRenderer implements FileRendererInterface
      * @param bool $usedPathsRelativeToCurrentScript See $file->getPublicUrl()
      * @return string
      */
-    public function render(FileInterface $file, $width, $height, array $options = null, $usedPathsRelativeToCurrentScript = false)
+    public function render(FileInterface $file, $width, $height, array $options = [], $usedPathsRelativeToCurrentScript = false)
+    {
+        $options = $this->collectOptions($options, $file);
+        $src = $this->createVimeoUrl($options, $file);
+        $attributes = $this->collectIframeAttributes($width, $height, $options);
+
+        return sprintf(
+            '<iframe src="%s"%s></iframe>',
+            $src,
+            empty($attributes) ? '' : ' ' . implode(' ', $attributes)
+        );
+    }
+
+    /**
+     * @param array $options
+     * @param FileInterface $file
+     * @return array
+     */
+    protected function collectOptions(array $options, FileInterface $file)
     {
         // Check for an autoplay option at the file reference itself, if not overridden yet.
         if (!isset($options['autoplay']) && $file instanceof FileReference) {
@@ -98,6 +116,25 @@ class VimeoRenderer implements FileRendererInterface
             }
         }
 
+        if (!isset($options['allow'])) {
+            $options['allow'] = 'fullscreen';
+            if (!empty($options['autoplay'])) {
+                $options['allow'] = 'autoplay; fullscreen';
+            }
+        }
+
+        return $options;
+    }
+
+    /**
+     * @param array $options
+     * @param FileInterface $file
+     * @return string
+     */
+    protected function createVimeoUrl(array $options, FileInterface $file)
+    {
+        $videoId = $this->getVideoIdFromFile($file);
+
         $urlParams = [];
         if (!empty($options['autoplay'])) {
             $urlParams[] = 'autoplay=1';
@@ -109,22 +146,33 @@ class VimeoRenderer implements FileRendererInterface
         $urlParams[] = 'byline=' . (int)!empty($options['showinfo']);
         $urlParams[] = 'portrait=0';
 
+        return sprintf('https://player.vimeo.com/video/%s?%s', $videoId, implode('&amp;', $urlParams));
+    }
+
+    /**
+     * @param FileInterface $file
+     * @return string
+     */
+    protected function getVideoIdFromFile(FileInterface $file)
+    {
         if ($file instanceof FileReference) {
             $orgFile = $file->getOriginalFile();
         } else {
             $orgFile = $file;
         }
 
-        $videoId = $this->getOnlineMediaHelper($file)->getOnlineMediaId($orgFile);
-        $src = sprintf('https://player.vimeo.com/video/%s?%s', $videoId, implode('&amp;', $urlParams));
+        return $this->getOnlineMediaHelper($file)->getOnlineMediaId($orgFile);
+    }
 
+    /**
+     * @param int|string $width
+     * @param int|string $height
+     * @param array $options
+     * @return array
+     */
+    protected function collectIframeAttributes($width, $height, array $options)
+    {
         $attributes = ['allowfullscreen'];
-        if (!isset($options['allow'])) {
-            $options['allow'] = 'fullscreen';
-            if (!empty($options['autoplay'])) {
-                $options['allow'] = 'autoplay; fullscreen';
-            }
-        }
         if (isset($options['additionalAttributes']) && is_array($options['additionalAttributes'])) {
             $attributes[] = GeneralUtility::implodeAttributes($options['additionalAttributes'], true, true);
         }
@@ -148,11 +196,6 @@ class VimeoRenderer implements FileRendererInterface
                 $attributes[] = $key . '="' . htmlspecialchars($options[$key]) . '"';
             }
         }
-
-        return sprintf(
-            '<iframe src="%s"%s></iframe>',
-            $src,
-            empty($attributes) ? '' : ' ' . implode(' ', $attributes)
-        );
+        return $attributes;
     }
 }
index 6540bd2..3652ad9 100644 (file)
@@ -88,7 +88,25 @@ class YouTubeRenderer implements FileRendererInterface
      * @param bool $usedPathsRelativeToCurrentScript See $file->getPublicUrl()
      * @return string
      */
-    public function render(FileInterface $file, $width, $height, array $options = null, $usedPathsRelativeToCurrentScript = false)
+    public function render(FileInterface $file, $width, $height, array $options = [], $usedPathsRelativeToCurrentScript = false)
+    {
+        $options = $this->collectOptions($options, $file);
+        $src = $this->createYouTubeUrl($options, $file);
+        $attributes = $this->collectIframeAttributes($width, $height, $options);
+
+        return sprintf(
+            '<iframe src="%s"%s></iframe>',
+            $src,
+            empty($attributes) ? '' : ' ' . implode(' ', $attributes)
+        );
+    }
+
+    /**
+     * @param array $options
+     * @param FileInterface $file
+     * @return array
+     */
+    protected function collectOptions(array $options, FileInterface $file)
     {
         // Check for an autoplay option at the file reference itself, if not overridden yet.
         if (!isset($options['autoplay']) && $file instanceof FileReference) {
@@ -98,18 +116,28 @@ class YouTubeRenderer implements FileRendererInterface
             }
         }
 
-        if ($file instanceof FileReference) {
-            $orgFile = $file->getOriginalFile();
-        } else {
-            $orgFile = $file;
+        $options['controls'] = $options['controls'] ?? 2;
+        $options['controls'] = MathUtility::canBeInterpretedAsInteger($options['controls']) ? MathUtility::forceIntegerInRange($options['controls'], 0, 2) : 2;
+
+        if (!isset($options['allow'])) {
+            $options['allow'] = 'fullscreen';
+            if (!empty($options['autoplay'])) {
+                $options['allow'] = 'autoplay; fullscreen';
+            }
         }
+        return $options;
+    }
 
-        $videoId = $this->getOnlineMediaHelper($file)->getOnlineMediaId($orgFile);
+    /**
+     * @param array $options
+     * @param FileInterface $file
+     * @return string
+     */
+    protected function createYouTubeUrl(array $options, FileInterface $file)
+    {
+        $videoId = $this->getVideoIdFromFile($file);
 
         $urlParams = ['autohide=1'];
-
-        $options['controls'] = MathUtility::canBeInterpretedAsInteger($options['controls']) ? (int)$options['controls'] : 2;
-        $options['controls'] = MathUtility::forceIntegerInRange($options['controls'], 0, 2);
         $urlParams[] = 'controls=' . $options['controls'];
         if (!empty($options['autoplay'])) {
             $urlParams[] = 'autoplay=1';
@@ -128,20 +156,41 @@ class YouTubeRenderer implements FileRendererInterface
         }
         $urlParams[] = 'showinfo=' . (int)!empty($options['showinfo']);
 
-        $src = sprintf(
+        $youTubeUrl = sprintf(
             'https://www.youtube%s.com/embed/%s?%s',
             !isset($options['no-cookie']) || !empty($options['no-cookie']) ? '-nocookie' : '',
             $videoId,
             implode('&amp;', $urlParams)
         );
 
-        $attributes = ['allowfullscreen'];
-        if (!isset($options['allow'])) {
-            $options['allow'] = 'fullscreen';
-            if (!empty($options['autoplay'])) {
-                $options['allow'] = 'autoplay; fullscreen';
-            }
+        return $youTubeUrl;
+    }
+
+    /**
+     * @param FileInterface $file
+     * @return string
+     */
+    protected function getVideoIdFromFile(FileInterface $file)
+    {
+        if ($file instanceof FileReference) {
+            $orgFile = $file->getOriginalFile();
+        } else {
+            $orgFile = $file;
         }
+
+        return $this->getOnlineMediaHelper($file)->getOnlineMediaId($orgFile);
+    }
+
+    /**
+     * @param int|string $width
+     * @param int|string $height
+     * @param array $options
+     * @return array
+     */
+    protected function collectIframeAttributes($width, $height, array $options)
+    {
+        $attributes = ['allowfullscreen'];
+
         if (isset($options['additionalAttributes']) && is_array($options['additionalAttributes'])) {
             $attributes[] = GeneralUtility::implodeAttributes($options['additionalAttributes'], true, true);
         }
@@ -166,10 +215,6 @@ class YouTubeRenderer implements FileRendererInterface
             }
         }
 
-        return sprintf(
-            '<iframe src="%s"%s></iframe>',
-            $src,
-            empty($attributes) ? '' : ' ' . implode(' ', $attributes)
-        );
+        return $attributes;
     }
 }
index 9c345ca..0ac4c8e 100644 (file)
@@ -152,7 +152,7 @@ class YouTubeRendererTest extends UnitTestCase
         return [
             'no options given' => [
                 '<iframe src="https://www.youtube-nocookie.com/embed/7331?autohide=1&amp;controls=2&amp;enablejsapi=1&amp;origin=http%3A%2F%2Ftest.server.org&amp;showinfo=0" allowfullscreen width="300" height="200" allow="fullscreen"></iframe>',
-                null
+                []
             ],
             'with option controls = foo as invalid string' => [
                 '<iframe src="https://www.youtube-nocookie.com/embed/7331?autohide=1&amp;controls=2&amp;enablejsapi=1&amp;origin=http%3A%2F%2Ftest.server.org&amp;showinfo=0" allowfullscreen width="300" height="200" allow="fullscreen"></iframe>',