Merge pull request #982 from nextcloud/fix/954-image-file-types

Allow the client to specify the image type using Accept header

CHANGELOG.md

@@ -3,6 +3,8 @@
 ### Added
 - Add IDE configuration to codebase to prevent small issues
   [#978](https://github.com/nextcloud/cookbook/pull/978) @christianlupus
+- Allow client to specify accepted image types
+  [#982](https://github.com/nextcloud/cookbook/pull/982) @christianlupus
 
 ### Fixed
 - Refactor the code for image handling to make it testable

docs/dev/api/0.0.2/index.html

@@ -39,7 +39,7 @@
     window.onload = function() {
       // Begin Swagger UI call region
       const ui = SwaggerUIBundle({
-        url: "https://nextcloud.github.io/cookbook/dev/api/0.0.2/openapi-cookbook.yaml",
+        url: "openapi-cookbook.yaml",
         dom_id: '#swagger-ui',
         deepLinking: true,
         presets: [

docs/dev/api/0.0.3/index.html

@@ -39,7 +39,7 @@
     window.onload = function() {
       // Begin Swagger UI call region
       const ui = SwaggerUIBundle({
-        url: "https://nextcloud.github.io/cookbook/dev/api/0.0.3/openapi-cookbook.yaml",
+        url: "openapi-cookbook.yaml",
         dom_id: '#swagger-ui',
         deepLinking: true,
         presets: [

docs/dev/api/0.0.3/openapi-cookbook.yaml

@@ -297,7 +297,18 @@ paths:
             type: integer
       responses:
         200:
-          description: Image was obtained and will be inresponse either as image/jpeg or image/svg+xml
+          description: Image was obtained and will be in response either as image/jpeg or image/svg+xml
+          content:
+            image/jpeg:
+              schema:
+                type: string
+                format: binary
+            image/svg+xml:
+              schema:
+                type: string
+                format: binary
+        406:
+          description: The recipe has no image whose MIME type matches the Accept header
   /api/search/{query}:
     parameters:
       - in: path

lib/Controller/RecipeController.php

@@ -14,8 +14,10 @@ use OCA\Cookbook\Service\RecipeService;
 use OCP\IURLGenerator;
 use OCA\Cookbook\Service\DbCacheService;
 use OCA\Cookbook\Exception\RecipeExistsException;
+use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
 use OCA\Cookbook\Helper\RestParameterParser;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\IL10N;
 
 class RecipeController extends Controller {
 	/**
@@ -37,13 +39,34 @@ class RecipeController extends Controller {
 	 */
 	private $restParser;
 
-	public function __construct($AppName, IRequest $request, IURLGenerator $urlGenerator, RecipeService $recipeService, DbCacheService $dbCacheService, RestParameterParser $restParser) {
+	/**
+	 * @var AcceptHeaderParsingHelper
+	 */
+	private $acceptHeaderParser;
+
+	/**
+	 * @var IL10N
+	 */
+	private $l;
+
+	public function __construct(
+			$AppName,
+			IRequest $request,
+			IURLGenerator $urlGenerator,
+			RecipeService $recipeService,
+			DbCacheService $dbCacheService,
+			RestParameterParser $restParser,
+			AcceptHeaderParsingHelper $acceptHeaderParser,
+			IL10N $l
+			) {
 		parent::__construct($AppName, $request);
 
 		$this->service = $recipeService;
 		$this->urlGenerator = $urlGenerator;
 		$this->dbCacheService = $dbCacheService;
 		$this->restParser = $restParser;
+		$this->acceptHeaderParser = $acceptHeaderParser;
+		$this->l = $l;
 	}
 
 	/**
@@ -178,6 +201,9 @@ class RecipeController extends Controller {
 	public function image($id) {
 		$this->dbCacheService->triggerCheck();
 		
+		$acceptHeader = $this->request->getHeader('Accept');
+		$acceptedExtensions = $this->acceptHeaderParser->parseHeader($acceptHeader);
+
 		$size = isset($_GET['size']) ? $_GET['size'] : null;
 
 		try {
@@ -185,9 +211,18 @@ class RecipeController extends Controller {
 
 			return new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/jpeg', 'Cache-Control' => 'public, max-age=604800']);
 		} catch (\Exception $e) {
-			$file = file_get_contents(dirname(__FILE__) . '/../../img/recipe.svg');
-
-			return new DataDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
+			if (array_search('svg', $acceptedExtensions, true) === false) {
+				// We may not serve a SVG image. Tell the client about the missing image.
+				$json = [
+					'msg' => $this->l->t('No image with the matching mime type was found on the server.'),
+				];
+				return new JSONResponse($json, Http::STATUS_NOT_ACCEPTABLE);
+			} else {
+				// The client accepts the SVG file. Send it.
+				$file = file_get_contents(dirname(__FILE__) . '/../../img/recipe.svg');
+	
+				return new DataDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
+			}
 		}
 	}
 }

lib/Helper/AcceptHeaderParsingHelper.php

@@ -0,0 +1,109 @@
+<?php
+
+namespace OCA\Cookbook\Helper;
+
+/**
+ * This class parses the Accepts header of an HTTP request and returns an array of accepted file extensions.
+ *
+ * The return value is a list of extensions that the client is willing to accept.
+ * Higher priorities are sorted first in the array.
+ */
+class AcceptHeaderParsingHelper {
+
+	/**
+	 * Parse the content of a header and generate the list of valid file extensions the client will accept.
+	 *
+	 * The entries in the return value will be sorted according to the priority given by the sender.
+	 * Higher priority entries are sorted first.
+	 *
+	 * @param string $header The value of the Accept header to be parsed
+	 * @return array The sorted list of file extensions that are valid
+	 */
+	public function parseHeader(string $header): array {
+		$parts = explode(',', $header);
+		$parts = array_map(function ($x) {
+			return trim($x);
+		}, $parts);
+
+		// $this->sortParts($parts);
+		$weightedParts = $this->sortAndWeightParts($parts);
+
+		$extensions = [];
+
+		foreach ($weightedParts as $wp) {
+			$ex = $this->getFileTypes($wp['type']);
+
+			foreach ($ex as $e) {
+				if (array_search($e, $extensions) === false) {
+					$extensions[] = $e;
+				}
+			}
+		}
+		return $extensions;
+	}
+
+	/**
+	 * Return the list of all supported file extensions by the app.
+	 * The return value in the same format as with the parseHeader function
+	 *
+	 * @return array The list of supported file extensions by the app
+	 */
+	public function getDefaultExtensions(): array {
+		return ['jpg'];
+	}
+
+	private function sortAndWeightParts(array $parts): array {
+		$weightedParts = array_map(function ($x) {
+			return $this->parsePart($x);
+		}, $parts);
+
+		usort($weightedParts, function ($a, $b) {
+			$tmp = $a['weight'] - $b['weight'];
+			if ($tmp < - 0.001) {
+				return -1;
+			} elseif ($tmp > 0.001) {
+				return 1;
+			} else {
+				return 0;
+			}
+		});
+		$weightedParts = array_reverse($weightedParts);
+
+		return $weightedParts;
+	}
+
+	private function parsePart($part): array {
+		if (preg_match('/\s*(.+?)\s*;q=([0-9.]+)\s*$/', $part, $matches) === 0) {
+			// No qualifier was found
+			$mime = trim($part);
+			$weight = 1;
+		} else {
+			// Separate qualifier and part
+			$mime = trim($matches[1]);
+			$weight = $matches[2];
+		}
+
+		return [
+			'type' => $mime,
+			'weight' => $weight,
+		];
+	}
+
+	private function getFileTypes(string $mime): array {
+		$parts = explode(';', $mime, 2);
+		switch ($parts[0]) {
+			case 'image/jpeg':
+			case 'image/jpg':
+				return ['jpg'];
+			case 'image/png':
+				return ['png'];
+			case 'image/svg+xml':
+				return ['svg'];
+			case 'image/*':
+				return ['jpg', 'png', 'svg'];
+			case '*/*':
+				return ['jpg', 'png', 'svg'];
+		}
+		return [];
+	}
+}

tests/Unit/Controller/RecipeControllerTest.php

@@ -19,6 +19,9 @@ use OCA\Cookbook\Controller\RecipeController;
 use OCA\Cookbook\Exception\NoRecipeNameGivenException;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCA\Cookbook\Exception\RecipeExistsException;
+use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
+use OCP\IL10N;
+use PHPUnit\Framework\MockObject\Stub;
 
 /**
  * @covers \OCA\Cookbook\Controller\RecipeController
@@ -47,6 +50,16 @@ class RecipeControllerTest extends TestCase {
 	 */
 	private $sut;
 
+	/**
+	 * @var IRequest|MockObject
+	 */
+	private $request;
+
+	/**
+	 * @var AcceptHeaderParsingHelper|Stub
+	 */
+	private $acceptHeaderParser;
+
 	public function setUp(): void {
 		parent::setUp();
 
@@ -54,9 +67,16 @@ class RecipeControllerTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->dbCacheService = $this->createMock(DbCacheService::class);
 		$this->restParser = $this->createMock(RestParameterParser::class);
-		$request = $this->createStub(IRequest::class);
+		$this->request = $this->createMock(IRequest::class);
+		$this->acceptHeaderParser = $this->createStub(AcceptHeaderParsingHelper::class);
 
-		$this->sut = new RecipeController('cookbook', $request, $this->urlGenerator, $this->recipeService, $this->dbCacheService, $this->restParser);
+		/**
+		 * @var Stub|IL10N $l
+		 */
+		$l = $this->createStub(IL10N::class);
+		$l->method('t')->willReturnArgument(0);
+
+		$this->sut = new RecipeController('cookbook', $this->request, $this->urlGenerator, $this->recipeService, $this->dbCacheService, $this->restParser, $this->acceptHeaderParser, $l);
 	}
 
 	public function testConstructor(): void {
@@ -300,6 +320,31 @@ class RecipeControllerTest extends TestCase {
 		];
 	}
 
+	public function dpImageNotFound() {
+		yield [['jpg', 'png'], 406];
+		yield [['jpg', 'png', 'svg'], 200];
+	}
+
+	/**
+	 * @dataProvider dpImageNotFound
+	 */
+	public function testImageNotFound($accept, $expectedStatus) {
+		$id = 123;
+
+		$ex = new Exception();
+		$this->recipeService->method('getRecipeImageFileByFolderId')->willThrowException($ex);
+
+		$headerContent = 'The content of the header as supposed by teh framework';
+		$this->request->method('getHeader')->with('Accept')->willReturn($headerContent);
+		$this->acceptHeaderParser->method('parseHeader')->willReturnMap([
+			[$headerContent, $accept],
+		]);
+
+		$ret = $this->sut->image($id);
+
+		$this->assertEquals($expectedStatus, $ret->getStatus());
+	}
+
 	/**
 	 * @dataProvider dataProviderIndex
 	 * @todo no work on controller

tests/Unit/Helper/AcceptHeaderParsingHelperTest.php

@@ -0,0 +1,49 @@
+<?php
+
+namespace OCA\Cookbook\tests\Unit\Helper;
+
+use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
+use PHPUnit\Framework\TestCase;
+
+class AcceptHeaderParsingHelperTest extends TestCase {
+
+	/**
+	 * @var AcceptHeaderParsingHelper
+	 */
+	private $dut;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->dut = new AcceptHeaderParsingHelper();
+	}
+
+	public function testDefaultExtensions() {
+		$ret = $this->dut->getDefaultExtensions();
+
+		$this->assertEquals(['jpg'], $ret);
+	}
+
+	public function dataProvider() {
+		yield ['image/jpeg', ['jpg']];
+		yield ['image/jpg', ['jpg']];
+		yield ['image/png', ['png']];
+		yield ['image/svg+xml', ['svg']];
+		yield ['image/*', ['jpg', 'png', 'svg']];
+		yield ['*/*', ['jpg', 'png', 'svg']];
+		yield ['image/jpeg, image/*', ['jpg', 'png', 'svg']];
+		yield ['image/jpeg;q=0.9, image/*;q=0.5', ['jpg', 'png', 'svg']];
+		yield ['image/webn, image/jpeg', ['jpg']];
+		yield ['image/webn', []];
+		yield ['image/png;q=0.5, image/jpg', ['jpg', 'png']];
+		yield ['image/png;q=0.5, image/jpeg;q=0.3, image/svg+xml', ['svg', 'png', 'jpg']];
+		yield ['image/png, image/jpeg;q=0.3, image/svg+xml;q=0.9', ['png', 'svg', 'jpg']];
+	}
+
+	/**
+	 * @dataProvider dataProvider
+	 */
+	public function testParseHeader($header, $expected) {
+		$this->assertEquals($expected, $this->dut->parseHeader($header));
+	}
+}