Update picture helper, move anilist oauth calls to the settings controller

app/appConf/routes.php

@@ -191,12 +191,12 @@ $routes = [
 	'anilist-redirect' => [
 		'path' => '/anilist-redirect',
 		'action' => 'anilistRedirect',
-		'controller' => 'user',
+		'controller' => 'settings',
 	],
 	'anilist-callback' => [
 		'path' => '/anilist-oauth',
 		'action' => 'anilistCallback',
-		'controller' => 'user',
+		'controller' => 'settings',
 	],
 	'image_proxy' => [
 		'path' => '/public/images/{type}/{file}',

src/Controller/Settings.php

@@ -84,4 +84,66 @@ final class Settings extends BaseController {
 
 		$this->redirect($this->url->generate('settings'), 303);
 	}
+
+	/**
+	 * Redirect to Anilist to start Oauth flow
+	 */
+	public function anilistRedirect()
+	{
+		$redirectUrl = 'https://anilist.co/api/v2/oauth/authorize?' .
+			http_build_query([
+				'client_id' => $this->config->get(['anilist', 'client_id']),
+				'redirect_uri' => $this->urlGenerator->url('/anilist-oauth'),
+				'response_type' => 'code',
+			]);
+
+		$this->redirect($redirectUrl, 303);
+	}
+
+	/**
+	 * Oauth callback for Anilist API
+	 */
+	public function anilistCallback()
+	{
+		$query = $this->request->getQueryParams();
+		$authCode = $query['code'];
+		$uri = $this->urlGenerator->url('/anilist-oauth');
+
+		$authData = $this->anilistModel->authenticate($authCode, $uri);
+		$settings = $this->settingsModel->getSettings();
+
+		if (array_key_exists('error', $authData))
+		{
+			$this->errorPage(400, 'Error Linking Account', $authData['hint']);
+			return;
+		}
+
+		// Update the override config file
+		$anilistSettings = [
+			'access_token' => $authData['access_token'],
+			'access_token_expires' => (time() - 10) + $authData['expires_in'],
+			'refresh_token' => $authData['refresh_token'],
+		];
+
+		$newSettings = $settings;
+		$newSettings['anilist'] = array_merge($settings['anilist'], $anilistSettings);
+
+		foreach ($newSettings['config'] as $key => $value)
+		{
+			$newSettings[$key] = $value;
+		}
+		unset($newSettings['config']);
+
+		$saved = $this->settingsModel->saveSettingsFile($newSettings);
+
+		if ($saved)
+		{
+			$this->setFlashMessage('Linked Anilist Account', 'success');
+		} else
+		{
+			$this->setFlashMessage('Error Linking Anilist Account', 'error');
+		}
+
+		$this->redirect($this->url->generate('settings'), 303);
+	}
 }

src/Controller/User.php

@@ -91,70 +91,6 @@ final class User extends BaseController {
 		]);
 	}
 
-	
-	/**
-	 * Redirect to Anilist to start Oauth flow
-	 */
-	public function anilistRedirect()
-	{
-		$redirectUrl = 'https://anilist.co/api/v2/oauth/authorize?' .
-			http_build_query([
-				'client_id' => $this->config->get(['anilist', 'client_id']),
-				'redirect_uri' => $this->urlGenerator->url('/anilist-oauth'),
-				'response_type' => 'code',
-			]);
-
-		$this->redirect($redirectUrl, 303);
-	}
-
-	/**
-	 * Oauth callback for Anilist API
-	 */
-	public function anilistCallback()
-	{
-		$query = $this->request->getQueryParams();
-		$authCode = $query['code'];
-		$uri = $this->urlGenerator->url('/anilist-oauth');
-
-		$authData = $this->anilistModel->authenticate($authCode, $uri);
-		$settings = $this->settingsModel->getSettings();
-
-		if (array_key_exists('error', $authData))
-		{
-			$this->errorPage(400, 'Error Linking Account', $authData['hint']);
-			return;
-		}
-
-		// Update the override config file
-		$anilistSettings = [
-			'access_token' => $authData['access_token'],
-			'access_token_expires' => (time() - 10) + $authData['expires_in'],
-			'refresh_token' => $authData['refresh_token'],
-		];
-
-		$newSettings = $settings;
-		$newSettings['anilist'] = array_merge($settings['anilist'], $anilistSettings);
-
-		foreach($newSettings['config'] as $key => $value)
-		{
-			$newSettings[$key] = $value;
-		}
-		unset($newSettings['config']);
-
-		$saved = $this->settingsModel->saveSettingsFile($newSettings);
-
-		if ($saved)
-		{
-			$this->setFlashMessage('Linked Anilist Account', 'success');
-		}
-		else
-		{
-			$this->setFlashMessage('Error Linking Anilist Account', 'error');
-		}
-
-		$this->redirect($this->url->generate('settings'), 303);
-	}
-
 	/**
 	 * Reorganize favorites data to be more useful
 	 *

src/Helper/Picture.php

@@ -25,49 +25,86 @@ final class Picture {
 
 	use ContainerAware;
 
+	private const MIME_MAP = [
+		'apng' => 'image/vnd.mozilla.apng',
+		'bmp' => 'image/bmp',
+		'gif' => 'image/gif',
+		'ico' => 'image/x-icon',
+		'jpeg' => 'image/jpeg',
+		'jpf' => 'image/jpx',
+		'jpg' => 'image/jpeg',
+		'jpx' => 'image/jpx',
+		'png' => 'image/png',
+		'svg' => 'image/svg+xml',
+		'tif' => 'image/tiff',
+		'tiff' => 'image/tiff',
+		'webp' => 'image/webp',
+	];
+
+	private const SIMPLE_IMAGE_TYPES = [
+		'gif',
+		'jpeg',
+		'jpg',
+		'png',
+	];
+
 	/**
-	 * Create the html f
+	 * Create the html for an html picture element
 	 *
-	 * @param string $webp
+	 * @param string $uri
 	 * @param string $fallbackExt
 	 * @param array $picAttrs
 	 * @param array $imgAttrs
 	 * @return string
 	 */
-	public function __invoke(string $webp, string $fallbackExt = 'jpg', $picAttrs = [], $imgAttrs = []): string
+	public function __invoke(string $uri, string $fallbackExt = 'jpg', array $picAttrs = [], array $imgAttrs = []): string
 	{
 		$urlGenerator = $this->container->get('url-generator');
 		$helper = $this->container->get('html-helper');
 
 		// If it is a placeholder image, make the
 		// fallback a png, not a jpg
-		if (strpos($webp, 'placeholder') !== FALSE)
+		if (strpos($uri, 'placeholder') !== FALSE)
 		{
 			$fallbackExt = 'png';
 		}
 
-		if (strpos($webp, '//') === FALSE)
+		if (strpos($uri, '//') === FALSE)
 		{
-			$webp = $urlGenerator->assetUrl($webp);
+			$uri = $urlGenerator->assetUrl($uri);
 		}
 
-
+		$urlParts = explode('.', $uri);
-		$urlParts = explode('.', $webp);
 		$ext = array_pop($urlParts);
 		$path = implode('.', $urlParts);
 
-		$mime = $ext === 'jpg'
+		$mime = array_key_exists($ext, static::MIME_MAP)
-			? 'image/jpeg'
+			? static::MIME_MAP[$ext]
-			: "image/{$ext}";
+			: 'image/jpeg';
-		$fallbackMime = $fallbackExt === 'jpg'
+
-			? 'image/jpeg'
+		$fallbackMime = array_key_exists($fallbackExt, static::MIME_MAP)
-			: "image/{$fallbackExt}";
+			? static::MIME_MAP[$fallbackExt]
+			: 'image/jpeg';
+
+		// For image types that are well-established, just return a
+		// simple <img /> element instead
+		if (
+			$ext === $fallbackExt ||
+			\in_array($ext, static::SIMPLE_IMAGE_TYPES, TRUE)
+		)
+		{
+			$attrs = ( ! empty($imgAttrs))
+				? $imgAttrs
+				: $picAttrs;
+
+			return $helper->img($uri, $attrs);
+		}
 
 		$fallbackImg = "{$path}.{$fallbackExt}";
 
 		$pictureChildren = [
 			$helper->void('source', [
-				'srcset' => $webp,
+				'srcset' => $uri,
 				'type' => $mime,
 			]),
 			$helper->void('source', [

tests/AnimeClientTest.php

@@ -17,6 +17,7 @@
 namespace Aviat\AnimeClient\Tests;
 
 use function Aviat\AnimeClient\arrayToToml;
+use function Aviat\AnimeClient\isSequentialArray;
 use function Aviat\AnimeClient\tomlToArray;
 
 class AnimeClientTest extends AnimeClientTestCase
@@ -55,4 +56,12 @@ class AnimeClientTest extends AnimeClientTestCase
 
 		$this->assertEquals($arr, $parsedArray);
 	}
+
+	public function testIsSequentialArray()
+	{
+		$this->assertFalse(isSequentialArray(0));
+		$this->assertFalse(isSequentialArray([50 => 'foo']));
+		$this->assertTrue(isSequentialArray([]));
+		$this->assertTrue(isSequentialArray([1,2,3,4,5]));
+	}
 }

tests/Helper/PictureHelperTest.php

@@ -0,0 +1,151 @@
+<?php declare(strict_types=1);
+/**
+ * Hummingbird Anime List Client
+ *
+ * An API client for Kitsu to manage anime and manga watch lists
+ *
+ * PHP version 7.1
+ *
+ * @package     HummingbirdAnimeClient
+ * @author      Timothy J. Warren <tim@timshomepage.net>
+ * @copyright   2015 - 2018  Timothy J. Warren
+ * @license     http://www.opensource.org/licenses/mit-license.html  MIT License
+ * @version     4.1
+ * @link        https://git.timshomepage.net/timw4mail/HummingBirdAnimeClient
+ */
+
+namespace Aviat\AnimeClient\Tests\Helper;
+
+use Aviat\AnimeClient\Helper\Picture as PictureHelper;
+use Aviat\AnimeClient\Tests\AnimeClientTestCase;
+
+class PictureHelperTest extends AnimeClientTestCase {
+	/**
+	 * @dataProvider dataPictureCase
+	 */
+	public function testPictureHelper($params, $expected = NULL)
+	{
+		$helper = new PictureHelper();
+		$helper->setContainer($this->container);
+
+		$actual = $helper(...$params);
+
+		if ($expected === NULL)
+		{
+			$this->assertMatchesSnapshot($actual);
+		}
+		else
+		{
+			$this->assertEquals($expected, $actual);
+		}
+	}
+
+	/**
+	 * @dataProvider dataSimpleImageCase
+	 */
+	public function testSimpleImage(string $ext, bool $isSimple)
+	{
+		$helper = new PictureHelper();
+		$helper->setContainer($this->container);
+
+		$url = "https://example.com/image.{$ext}";
+		$actual = $helper($url);
+
+		$actuallySimple = strpos($actual, '<picture') === FALSE;
+
+		$this->assertEquals($isSimple, $actuallySimple);
+	}
+
+	public function testSimpleImageByFallback()
+	{
+		$helper = new PictureHelper();
+		$helper->setContainer($this->container);
+
+		$actual = $helper("foo.svg", 'svg');
+
+		$this->assertTrue(strpos($actual, '<picture') === FALSE);
+	}
+
+	public function dataPictureCase()
+	{
+		return [
+			'Full webp URL' => [
+				'params' => [
+					'https://www.example.com/image.webp',
+				],
+			],
+			'Partial webp URL' => [
+				'params' => [
+					'images/anime/15424.webp'
+				],
+			],
+			'bmp with gif fallback' => [
+				'params' => [
+					'images/avatar/25.bmp',
+					'gif',
+				],
+			],
+			'webp placeholder image' => [
+				'params' => [
+					'images/placeholder.webp',
+				]
+			],
+			'png placeholder image' => [
+				'params' => [
+					'images/placeholder.png',
+				]
+			],
+			'jpeg2000' => [
+				'params' => [
+					'images/foo.jpf',
+				]
+			],
+			'svg with png fallback and lots of attributes' => [
+				'params' => [
+					'images/example.svg',
+					'png',
+					[ 'width' => 200, 'height' => 300 ],
+					[ 'alt' => 'Example text' ]
+				]
+			],
+			'simple image with attributes' => [
+				'params' => [
+					'images/foo.jpg',
+					'jpg',
+					[ 'x' => 1, 'y' => 1 ],
+					['width' => 200, 'height' => 200, 'alt' => 'should exist'],
+				]
+			]
+		];
+	}
+
+	public function dataSimpleImageCase()
+	{
+		return [
+			'apng' => [
+				'ext' => 'apng',
+				'isSimple' => FALSE,
+			],
+			'gif' => [
+				'ext' => 'gif',
+				'isSimple' => TRUE,
+			],
+			'jpg' => [
+				'ext' => 'jpg',
+				'isSimple' => TRUE,
+			],
+			'jpeg' => [
+				'ext' => 'jpeg',
+				'isSimple' => TRUE,
+			],
+			'png' => [
+				'ext' => 'png',
+				'isSimple' => TRUE,
+			],
+			'webp' => [
+				'ext' => 'webp',
+				'isSimple' => FALSE,
+			],
+		];
+	}
+}

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set Full webp URL__1.php

@@ -0,0 +1 @@
+<?php return '<picture><source srcset="https://www.example.com/image.webp" type="image/webp" /><source srcset="https://www.example.com/image.jpg" type="image/jpeg" /><img src="https://www.example.com/image.jpg" alt="" /></picture>';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set Partial webp URL__1.php

@@ -0,0 +1 @@
+<?php return '<picture><source srcset="https://localhost/assets/images/anime/15424.webp" type="image/webp" /><source srcset="https://localhost/assets/images/anime/15424.jpg" type="image/jpeg" /><img src="https://localhost/assets/images/anime/15424.jpg" alt="" /></picture>';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set bmp with gif fallback__1.php

@@ -0,0 +1 @@
+<?php return '<picture><source srcset="https://localhost/assets/images/avatar/25.bmp" type="image/bmp" /><source srcset="https://localhost/assets/images/avatar/25.gif" type="image/gif" /><img src="https://localhost/assets/images/avatar/25.gif" alt="" /></picture>';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set jpeg2000__1.php

@@ -0,0 +1 @@
+<?php return '<picture><source srcset="https://localhost/assets/images/foo.jpf" type="image/jpx" /><source srcset="https://localhost/assets/images/foo.jpg" type="image/jpeg" /><img src="https://localhost/assets/images/foo.jpg" alt="" /></picture>';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set png placeholder image__1.php

@@ -0,0 +1 @@
+<?php return '<img src="https://localhost/assets/images/placeholder.png" alt="placeholder.png" />';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set simple image with attributes__1.php

@@ -0,0 +1 @@
+<?php return '<img src="https://localhost/assets/images/foo.jpg" alt="should exist" width="200" height="200" />';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set svg with png fallback and lots of attributes__1.php

@@ -0,0 +1 @@
+<?php return '<picture width="200" height="300"><source srcset="https://localhost/assets/images/example.svg" type="image/svg+xml" /><source srcset="https://localhost/assets/images/example.png" type="image/png" /><img src="https://localhost/assets/images/example.png" alt="Example text" /></picture>';

tests/Helper/__snapshots__/PictureHelperTest__testPictureHelper with data set webp placeholder image__1.php

@@ -0,0 +1 @@
+<?php return '<picture><source srcset="https://localhost/assets/images/placeholder.webp" type="image/webp" /><source srcset="https://localhost/assets/images/placeholder.png" type="image/png" /><img src="https://localhost/assets/images/placeholder.png" alt="" /></picture>';