Skip to content

Commit 81b4661

Browse files
christianlupuschristianlupus
authored
Merge 68a3171 into 7b66a85 
2 parents 7b66a85 + 68a3171 commit 81b4661

File tree

8 files changed

+260
-9
lines changed

8 files changed

+260
-9
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
### Added
44
- Add IDE configuration to codebase to prevent small issues
55
[#978](https://github.com/nextcloud/cookbook/pull/978) @christianlupus
6+
- Allow client to specify accepted image types
7+
[#982](https://github.com/nextcloud/cookbook/pull/982) @christianlupus
68

79
### Fixed
810
- Refactor the code for image handling to make it testable

‎docs/dev/api/0.0.2/index.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
window.onload = function() {
4040
// Begin Swagger UI call region
4141
const ui = SwaggerUIBundle({
42-
url: "https://nextcloud.github.io/cookbook/dev/api/0.0.2/openapi-cookbook.yaml",
42+
url: "openapi-cookbook.yaml",
4343
dom_id: '#swagger-ui',
4444
deepLinking: true,
4545
presets: [

‎docs/dev/api/0.0.3/index.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
window.onload = function() {
4040
// Begin Swagger UI call region
4141
const ui = SwaggerUIBundle({
42-
url: "https://nextcloud.github.io/cookbook/dev/api/0.0.3/openapi-cookbook.yaml",
42+
url: "openapi-cookbook.yaml",
4343
dom_id: '#swagger-ui',
4444
deepLinking: true,
4545
presets: [

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

+12-1
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,18 @@ paths:
297297
type: integer
298298
responses:
299299
200:
300-
description: Image was obtained and will be inresponse either as image/jpeg or image/svg+xml
300+
description: Image was obtained and will be in response either as image/jpeg or image/svg+xml
301+
content:
302+
image/jpeg:
303+
schema:
304+
type: string
305+
format: binary
306+
image/svg+xml:
307+
schema:
308+
type: string
309+
format: binary
310+
406:
311+
description: The recipe has no image whose MIME type matches the Accept header
301312
/api/search/{query}:
302313
parameters:
303314
- in: path

‎lib/Controller/RecipeController.php

+39-4
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
use OCP\IURLGenerator;
1515
use OCA\Cookbook\Service\DbCacheService;
1616
use OCA\Cookbook\Exception\RecipeExistsException;
17+
use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
1718
use OCA\Cookbook\Helper\RestParameterParser;
1819
use OCP\AppFramework\Http\JSONResponse;
20+
use OCP\IL10N;
1921

2022
class RecipeController extends Controller {
2123
/**
@@ -37,13 +39,34 @@ class RecipeController extends Controller {
3739
*/
3840
private $restParser;
3941

40-
public function __construct($AppName, IRequest $request, IURLGenerator $urlGenerator, RecipeService $recipeService, DbCacheService $dbCacheService, RestParameterParser $restParser) {
42+
/**
43+
* @var AcceptHeaderParsingHelper
44+
*/
45+
private $acceptHeaderParser;
46+
47+
/**
48+
* @var IL10N
49+
*/
50+
private $l;
51+
52+
public function __construct(
53+
$AppName,
54+
IRequest $request,
55+
IURLGenerator $urlGenerator,
56+
RecipeService $recipeService,
57+
DbCacheService $dbCacheService,
58+
RestParameterParser $restParser,
59+
AcceptHeaderParsingHelper $acceptHeaderParser,
60+
IL10N $l
61+
) {
4162
parent::__construct($AppName, $request);
4263

4364
$this->service = $recipeService;
4465
$this->urlGenerator = $urlGenerator;
4566
$this->dbCacheService = $dbCacheService;
4667
$this->restParser = $restParser;
68+
$this->acceptHeaderParser = $acceptHeaderParser;
69+
$this->l = $l;
4770
}
4871

4972
/**
@@ -178,16 +201,28 @@ public function destroy($id) {
178201
public function image($id) {
179202
$this->dbCacheService->triggerCheck();
180203

204+
$acceptHeader = $this->request->getHeader('Accept');
205+
$acceptedExtensions = $this->acceptHeaderParser->parseHeader($acceptHeader);
206+
181207
$size = isset($_GET['size']) ? $_GET['size'] : null;
182208

183209
try {
184210
$file = $this->service->getRecipeImageFileByFolderId($id, $size);
185211

186212
return new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/jpeg', 'Cache-Control' => 'public, max-age=604800']);
187213
} catch (\Exception $e) {
188-
$file = file_get_contents(dirname(__FILE__) . '/../../img/recipe.svg');
189-
190-
return new DataDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
214+
if (array_search('svg', $acceptedExtensions, true) === false) {
215+
// We may not serve a SVG image. Tell the client about the missing image.
216+
$json = [
217+
'msg' => $this->l->t('No image with the matching mime type was found on the server.'),
218+
];
219+
return new JSONResponse($json, Http::STATUS_NOT_ACCEPTABLE);
220+
} else {
221+
// The client accepts the SVG file. Send it.
222+
$file = file_get_contents(dirname(__FILE__) . '/../../img/recipe.svg');
223+
224+
return new DataDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
225+
}
191226
}
192227
}
193228
}

‎lib/Helper/AcceptHeaderParsingHelper.php

+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
<?php
2+
3+
namespace OCA\Cookbook\Helper;
4+
5+
/**
6+
* This class parses the Accepts header of an HTTP request and returns an array of accepted file extensions.
7+
*
8+
* The return value is a list of extensions that the client is willing to accept.
9+
* Higher priorities are sorted first in the array.
10+
*/
11+
class AcceptHeaderParsingHelper {
12+
13+
/**
14+
* Parse the content of a header and generate the list of valid file extensions the client will accept.
15+
*
16+
* The entries in the return value will be sorted according to the priority given by the sender.
17+
* Higher priority entries are sorted first.
18+
*
19+
* @param string $header The value of the Accept header to be parsed
20+
* @return array The sorted list of file extensions that are valid
21+
*/
22+
public function parseHeader(string $header): array {
23+
$parts = explode(',', $header);
24+
$parts = array_map(function ($x) {
25+
return trim($x);
26+
}, $parts);
27+
28+
// $this->sortParts($parts);
29+
$weightedParts = $this->sortAndWeightParts($parts);
30+
31+
$extensions = [];
32+
33+
foreach ($weightedParts as $wp) {
34+
$ex = $this->getFileTypes($wp['type']);
35+
36+
foreach ($ex as $e) {
37+
if (array_search($e, $extensions) === false) {
38+
$extensions[] = $e;
39+
}
40+
}
41+
}
42+
return $extensions;
43+
}
44+
45+
/**
46+
* Return the list of all supported file extensions by the app.
47+
* The return value in the same format as with the parseHeader function
48+
*
49+
* @return array The list of supported file extensions by the app
50+
*/
51+
public function getDefaultExtensions(): array {
52+
return ['jpg'];
53+
}
54+
55+
private function sortAndWeightParts(array $parts): array {
56+
$weightedParts = array_map(function ($x) {
57+
return $this->parsePart($x);
58+
}, $parts);
59+
60+
usort($weightedParts, function ($a, $b) {
61+
$tmp = $a['weight'] - $b['weight'];
62+
if ($tmp < - 0.001) {
63+
return -1;
64+
} elseif ($tmp > 0.001) {
65+
return 1;
66+
} else {
67+
return 0;
68+
}
69+
});
70+
$weightedParts = array_reverse($weightedParts);
71+
72+
return $weightedParts;
73+
}
74+
75+
private function parsePart($part): array {
76+
if (preg_match('/\s*(.+?)\s*;q=([0-9.]+)\s*$/', $part, $matches) === 0) {
77+
// No qualifier was found
78+
$mime = trim($part);
79+
$weight = 1;
80+
} else {
81+
// Separate qualifier and part
82+
$mime = trim($matches[1]);
83+
$weight = $matches[2];
84+
}
85+
86+
return [
87+
'type' => $mime,
88+
'weight' => $weight,
89+
];
90+
}
91+
92+
private function getFileTypes(string $mime): array {
93+
$parts = explode(';', $mime, 2);
94+
switch ($parts[0]) {
95+
case 'image/jpeg':
96+
case 'image/jpg':
97+
return ['jpg'];
98+
case 'image/png':
99+
return ['png'];
100+
case 'image/svg+xml':
101+
return ['svg'];
102+
case 'image/*':
103+
return ['jpg', 'png', 'svg'];
104+
case '*/*':
105+
return ['jpg', 'png', 'svg'];
106+
}
107+
return [];
108+
}
109+
}

‎tests/Unit/Controller/RecipeControllerTest.php

+47-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
use OCA\Cookbook\Exception\NoRecipeNameGivenException;
2020
use OCP\AppFramework\Http\FileDisplayResponse;
2121
use OCA\Cookbook\Exception\RecipeExistsException;
22+
use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
23+
use OCP\IL10N;
24+
use PHPUnit\Framework\MockObject\Stub;
2225

2326
/**
2427
* @covers \OCA\Cookbook\Controller\RecipeController
@@ -47,16 +50,33 @@ class RecipeControllerTest extends TestCase {
4750
*/
4851
private $sut;
4952

53+
/**
54+
* @var IRequest|MockObject
55+
*/
56+
private $request;
57+
58+
/**
59+
* @var AcceptHeaderParsingHelper|Stub
60+
*/
61+
private $acceptHeaderParser;
62+
5063
public function setUp(): void {
5164
parent::setUp();
5265

5366
$this->recipeService = $this->createMock(RecipeService::class);
5467
$this->urlGenerator = $this->createMock(IURLGenerator::class);
5568
$this->dbCacheService = $this->createMock(DbCacheService::class);
5669
$this->restParser = $this->createMock(RestParameterParser::class);
57-
$request = $this->createStub(IRequest::class);
70+
$this->request = $this->createMock(IRequest::class);
71+
$this->acceptHeaderParser = $this->createStub(AcceptHeaderParsingHelper::class);
5872

59-
$this->sut = new RecipeController('cookbook', $request, $this->urlGenerator, $this->recipeService, $this->dbCacheService, $this->restParser);
73+
/**
74+
* @var Stub|IL10N $l
75+
*/
76+
$l = $this->createStub(IL10N::class);
77+
$l->method('t')->willReturnArgument(0);
78+
79+
$this->sut = new RecipeController('cookbook', $this->request, $this->urlGenerator, $this->recipeService, $this->dbCacheService, $this->restParser, $this->acceptHeaderParser, $l);
6080
}
6181

6282
public function testConstructor(): void {
@@ -300,6 +320,31 @@ public function dataProviderImage(): array {
300320
];
301321
}
302322

323+
public function dpImageNotFound() {
324+
yield [['jpg', 'png'], 406];
325+
yield [['jpg', 'png', 'svg'], 200];
326+
}
327+
328+
/**
329+
* @dataProvider dpImageNotFound
330+
*/
331+
public function testImageNotFound($accept, $expectedStatus) {
332+
$id = 123;
333+
334+
$ex = new Exception();
335+
$this->recipeService->method('getRecipeImageFileByFolderId')->willThrowException($ex);
336+
337+
$headerContent = 'The content of the header as supposed by teh framework';
338+
$this->request->method('getHeader')->with('Accept')->willReturn($headerContent);
339+
$this->acceptHeaderParser->method('parseHeader')->willReturnMap([
340+
[$headerContent, $accept],
341+
]);
342+
343+
$ret = $this->sut->image($id);
344+
345+
$this->assertEquals($expectedStatus, $ret->getStatus());
346+
}
347+
303348
/**
304349
* @dataProvider dataProviderIndex
305350
* @todo no work on controller

‎tests/Unit/Helper/AcceptHeaderParsingHelperTest.php

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
namespace OCA\Cookbook\tests\Unit\Helper;
4+
5+
use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
6+
use PHPUnit\Framework\TestCase;
7+
8+
class AcceptHeaderParsingHelperTest extends TestCase {
9+
10+
/**
11+
* @var AcceptHeaderParsingHelper
12+
*/
13+
private $dut;
14+
15+
protected function setUp(): void {
16+
parent::setUp();
17+
18+
$this->dut = new AcceptHeaderParsingHelper();
19+
}
20+
21+
public function testDefaultExtensions() {
22+
$ret = $this->dut->getDefaultExtensions();
23+
24+
$this->assertEquals(['jpg'], $ret);
25+
}
26+
27+
public function dataProvider() {
28+
yield ['image/jpeg', ['jpg']];
29+
yield ['image/jpg', ['jpg']];
30+
yield ['image/png', ['png']];
31+
yield ['image/svg+xml', ['svg']];
32+
yield ['image/*', ['jpg', 'png', 'svg']];
33+
yield ['*/*', ['jpg', 'png', 'svg']];
34+
yield ['image/jpeg, image/*', ['jpg', 'png', 'svg']];
35+
yield ['image/jpeg;q=0.9, image/*;q=0.5', ['jpg', 'png', 'svg']];
36+
yield ['image/webn, image/jpeg', ['jpg']];
37+
yield ['image/webn', []];
38+
yield ['image/png;q=0.5, image/jpg', ['jpg', 'png']];
39+
yield ['image/png;q=0.5, image/jpeg;q=0.3, image/svg+xml', ['svg', 'png', 'jpg']];
40+
yield ['image/png, image/jpeg;q=0.3, image/svg+xml;q=0.9', ['png', 'svg', 'jpg']];
41+
}
42+
43+
/**
44+
* @dataProvider dataProvider
45+
*/
46+
public function testParseHeader($header, $expected) {
47+
$this->assertEquals($expected, $this->dut->parseHeader($header));
48+
}
49+
}

0 commit comments

Comments
 (0)