diff --git a/CHANGELOG.md b/CHANGELOG.md index 22394a843..d869f53b7 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/dev/api/0.0.2/index.html b/docs/dev/api/0.0.2/index.html index c86a109cf..f22ad2818 100644 --- a/docs/dev/api/0.0.2/index.html +++ b/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: [ diff --git a/docs/dev/api/0.0.3/index.html b/docs/dev/api/0.0.3/index.html index f07215b01..f22ad2818 100644 --- a/docs/dev/api/0.0.3/index.html +++ b/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: [ diff --git a/docs/dev/api/0.0.3/openapi-cookbook.yaml b/docs/dev/api/0.0.3/openapi-cookbook.yaml index 94162ed0a..d7fb9e925 100644 --- a/docs/dev/api/0.0.3/openapi-cookbook.yaml +++ b/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 diff --git a/lib/Controller/RecipeController.php b/lib/Controller/RecipeController.php index 35e4995cc..abdd078b0 100755 --- a/lib/Controller/RecipeController.php +++ b/lib/Controller/RecipeController.php @@ -14,8 +14,10 @@ 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 @@ public function destroy($id) { 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 @@ public function image($id) { 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']); + } } } } diff --git a/lib/Helper/AcceptHeaderParsingHelper.php b/lib/Helper/AcceptHeaderParsingHelper.php new file mode 100644 index 000000000..c21330923 --- /dev/null +++ b/lib/Helper/AcceptHeaderParsingHelper.php @@ -0,0 +1,109 @@ +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 []; + } +} diff --git a/tests/Unit/Controller/RecipeControllerTest.php b/tests/Unit/Controller/RecipeControllerTest.php index 8808fb94b..182cd8900 100644 --- a/tests/Unit/Controller/RecipeControllerTest.php +++ b/tests/Unit/Controller/RecipeControllerTest.php @@ -19,6 +19,9 @@ 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 @@ public function setUp(): void { $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 @@ public function dataProviderImage(): array { ]; } + 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 diff --git a/tests/Unit/Helper/AcceptHeaderParsingHelperTest.php b/tests/Unit/Helper/AcceptHeaderParsingHelperTest.php new file mode 100644 index 000000000..98aac11a7 --- /dev/null +++ b/tests/Unit/Helper/AcceptHeaderParsingHelperTest.php @@ -0,0 +1,49 @@ +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)); + } +}