Skip to content

Commit 5aa6a94

Browse files
author
MateuszKolankowski
committed
Deprecated filename-less download route in favor of filename-validated variant
1 parent c62638d commit 5aa6a94

6 files changed

Lines changed: 152 additions & 7 deletions

File tree

‎src/bundle/Core/Resources/config/routing/internal.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,21 @@ ibexa.content.preview.default:
3333
ibexa.user_hash:
3434
path: /_fos_user_context_hash
3535

36+
# Must be defined before ibexa.content.download, so that numeric {fieldId} takes precedence over {fieldIdentifier}
37+
ibexa.content.download.field_id.filename:
38+
path: /content/download/{contentId}/{fieldId}/{filename}
39+
defaults: { _controller: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction }
40+
requirements:
41+
contentId: '\d+'
42+
fieldId: '\d+'
43+
3644
ibexa.content.download:
3745
path: /content/download/{contentId}/{fieldIdentifier}/{filename}
3846
defaults: { _controller: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileAction }
3947
requirements:
4048
contentId: '\d+'
4149

50+
# Deprecated since 5.0, will be removed in 6.0. Use ibexa.content.download.field_id.filename instead.
4251
ibexa.content.download.field_id:
4352
path: /content/download/{contentId}/{fieldId}
4453
defaults: { _controller: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction }

‎src/bundle/Core/Resources/config/services.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ services:
130130

131131
Ibexa\Core\MVC\Symfony\Controller\Content\DownloadRedirectionController:
132132
class: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadRedirectionController
133+
deprecated:
134+
package: 'ibexa/core'
135+
version: '5.0'
136+
message: 'Since ibexa/core 5.0: The "%service_id%" service is deprecated and will be removed in 6.0. No route references it, use Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction instead'
133137
arguments:
134138
$contentService: '@ibexa.api.service.content'
135139
$router: "@router"

‎src/lib/MVC/Symfony/Controller/Content/DownloadController.php‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,29 @@ public function __construct(
4545
/**
4646
* Download binary file identified by field ID.
4747
*
48+
* @param string|null $filename
49+
*
4850
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException If the field $fieldId can't be found, or the translation can't be found.
4951
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException If the content is trashed, or can't be found.
5052
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException If the user has no access to read content and in case of un-published content: read versions.
5153
*/
52-
public function downloadBinaryFileByIdAction(Request $request, int $contentId, int $fieldId): BinaryStreamResponse
53-
{
54+
public function downloadBinaryFileByIdAction(
55+
Request $request,
56+
int $contentId,
57+
int $fieldId,
58+
?string $filename = null
59+
): BinaryStreamResponse {
60+
if ($filename === null) {
61+
trigger_deprecation(
62+
'ibexa/core',
63+
'5.0',
64+
'The "ibexa.content.download.field_id" route (/content/download/{contentId}/{fieldId}) is deprecated'
65+
. ' and will be removed in 6.0.'
66+
. ' Use the "ibexa.content.download.field_id.filename" route'
67+
. ' (/content/download/{contentId}/{fieldId}/{filename}) instead.'
68+
);
69+
}
70+
5471
$versionNo = $request->query->has('version') ? $request->query->getInt('version') : null;
5572
$language = $request->query->has('inLanguage') ? $request->query->get('inLanguage') : null;
5673

@@ -65,6 +82,10 @@ public function downloadBinaryFileByIdAction(Request $request, int $contentId, i
6582
throw new NotFoundException('File', $fieldId);
6683
}
6784

85+
if ($filename !== null && $field->value->fileName !== $filename) {
86+
throw new NotFoundException('File', $filename);
87+
}
88+
6889
return $this->downloadBinaryFileAction($contentId, $field->fieldDefIdentifier, $field->value->fileName, $request);
6990
}
7091

‎src/lib/MVC/Symfony/Controller/Content/DownloadRedirectionController.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@
2020
use Symfony\Component\HttpFoundation\Response;
2121
use Symfony\Component\Routing\RouterInterface;
2222

23+
/**
24+
* @deprecated since ibexa/core 5.0, will be removed in 6.0. No route has referenced this controller since the
25+
* "ibexa.content.download.field_id" route started serving files directly.
26+
* Use {@see \Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction} instead.
27+
*/
2328
class DownloadRedirectionController extends Controller
2429
{
2530
private ContentService $contentService;

‎src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,22 @@
1414

1515
class ContentDownloadUrlGenerator implements RouteAwarePathGenerator
1616
{
17+
private const ROUTE = 'ibexa.content.download.field_id.filename';
18+
1719
/** @var \Symfony\Component\Routing\RouterInterface */
1820
private $router;
1921

20-
/** @var string */
21-
private $route = 'ibexa.content.download.field_id';
22-
2322
public function __construct(RouterInterface $router)
2423
{
2524
$this->router = $router;
2625
}
2726

2827
public function getStoragePathForField(Field $field, VersionInfo $versionInfo): string
2928
{
30-
return $this->generate($this->route, $this->getParameters($field, $versionInfo));
29+
return $this->generate(
30+
$this->getRoute($field, $versionInfo),
31+
$this->getParameters($field, $versionInfo)
32+
);
3133
}
3234

3335
public function generate(string $route, ?array $parameters = []): string
@@ -37,7 +39,7 @@ public function generate(string $route, ?array $parameters = []): string
3739

3840
public function getRoute(Field $field, VersionInfo $versionInfo): string
3941
{
40-
return $this->route;
42+
return self::ROUTE;
4143
}
4244

4345
public function getParameters(Field $field, VersionInfo $versionInfo): array
@@ -46,6 +48,7 @@ public function getParameters(Field $field, VersionInfo $versionInfo): array
4648
'contentId' => $versionInfo->contentInfo->id,
4749
'fieldId' => $field->id,
4850
'version' => $versionInfo->versionNo,
51+
'filename' => $field->value->externalData['fileName'] ?? '',
4952
];
5053
}
5154
}

‎tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace Ibexa\Tests\Core\MVC\Symfony\FieldType\BinaryBase;
10+
11+
use Ibexa\Contracts\Core\Persistence\Content\ContentInfo;
12+
use Ibexa\Contracts\Core\Persistence\Content\Field;
13+
use Ibexa\Contracts\Core\Persistence\Content\FieldValue;
14+
use Ibexa\Contracts\Core\Persistence\Content\VersionInfo;
15+
use Ibexa\Core\MVC\Symfony\FieldType\BinaryBase\ContentDownloadUrlGenerator;
16+
use PHPUnit\Framework\TestCase;
17+
use Symfony\Component\Routing\RouterInterface;
18+
19+
/**
20+
* @covers \Ibexa\Core\MVC\Symfony\FieldType\BinaryBase\ContentDownloadUrlGenerator
21+
*/
22+
final class ContentDownloadUrlGeneratorTest extends TestCase
23+
{
24+
private const ROUTE = 'ibexa.content.download.field_id.filename';
25+
26+
/** @var \Symfony\Component\Routing\RouterInterface&\PHPUnit\Framework\MockObject\MockObject */
27+
private RouterInterface $router;
28+
29+
private ContentDownloadUrlGenerator $generator;
30+
31+
protected function setUp(): void
32+
{
33+
parent::setUp();
34+
35+
$this->router = $this->createMock(RouterInterface::class);
36+
$this->generator = new ContentDownloadUrlGenerator($this->router);
37+
}
38+
39+
public function testGetRouteReturnsFilenameAwareRoute(): void
40+
{
41+
self::assertSame(
42+
self::ROUTE,
43+
$this->generator->getRoute($this->createField(), $this->createVersionInfo())
44+
);
45+
}
46+
47+
public function testGetParametersContainFilename(): void
48+
{
49+
self::assertSame(
50+
[
51+
'contentId' => 42,
52+
'fieldId' => 7,
53+
'version' => 2,
54+
'filename' => 'Test-file.pdf',
55+
],
56+
$this->generator->getParameters($this->createField(), $this->createVersionInfo())
57+
);
58+
}
59+
60+
public function testGetStoragePathForFieldGeneratesFilenameAwareUrl(): void
61+
{
62+
$this->router
63+
->expects(self::once())
64+
->method('generate')
65+
->with(
66+
self::ROUTE,
67+
[
68+
'contentId' => 42,
69+
'fieldId' => 7,
70+
'version' => 2,
71+
'filename' => 'Test-file.pdf',
72+
]
73+
)
74+
->willReturn('/content/download/42/7/Test-file.pdf?version=2');
75+
76+
self::assertSame(
77+
'/content/download/42/7/Test-file.pdf?version=2',
78+
$this->generator->getStoragePathForField($this->createField(), $this->createVersionInfo())
79+
);
80+
}
81+
82+
private function createField(): Field
83+
{
84+
return new Field([
85+
'id' => 7,
86+
'value' => new FieldValue([
87+
'externalData' => [
88+
'fileName' => 'Test-file.pdf',
89+
],
90+
]),
91+
]);
92+
}
93+
94+
private function createVersionInfo(): VersionInfo
95+
{
96+
return new VersionInfo([
97+
'versionNo' => 2,
98+
'contentInfo' => new ContentInfo([
99+
'id' => 42,
100+
]),
101+
]);
102+
}
103+
}

0 commit comments

Comments
 (0)