Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-85: Fixed Content cache containing invalid "uri" for file field types #3096

Merged
merged 7 commits into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions eZ/Publish/Core/FieldType/BinaryBase/BinaryBaseStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
namespace eZ\Publish\Core\FieldType\BinaryBase;

use eZ\Publish\SPI\FieldType\BinaryBase\RouteAwarePathGenerator;
use eZ\Publish\SPI\FieldType\GatewayBasedStorage;
use eZ\Publish\Core\IO\IOServiceInterface;
use eZ\Publish\SPI\FieldType\BinaryBase\PathGenerator;
Expand All @@ -32,7 +33,7 @@ class BinaryBaseStorage extends GatewayBasedStorage
/** @var \eZ\Publish\SPI\IO\MimeTypeDetector */
protected $mimeTypeDetector;

/** @var PathGenerator */
/** @var \eZ\Publish\SPI\FieldType\BinaryBase\PathGenerator */
protected $downloadUrlGenerator;

/** @var \eZ\Publish\Core\FieldType\BinaryBase\BinaryBaseStorage\Gateway */
Expand All @@ -59,7 +60,7 @@ public function __construct(
}

/**
* @param PathGenerator $downloadUrlGenerator
* @param \eZ\Publish\SPI\FieldType\BinaryBase\PathGenerator $downloadUrlGenerator
*/
public function setDownloadUrlGenerator(PathGenerator $downloadUrlGenerator)
{
Expand Down Expand Up @@ -144,9 +145,21 @@ public function getFieldData(VersionInfo $versionInfo, Field $field, array $cont
if ($field->value->externalData !== null) {
$binaryFile = $this->ioService->loadBinaryFile($field->value->externalData['id']);
$field->value->externalData['fileSize'] = $binaryFile->size;
$field->value->externalData['uri'] = isset($this->downloadUrlGenerator) ?
$this->downloadUrlGenerator->getStoragePathForField($field, $versionInfo) :
$binaryFile->uri;

$uri = $binaryFile->uri;
if (isset($this->downloadUrlGenerator)) {
$uri = $this->downloadUrlGenerator->getStoragePathForField($field, $versionInfo);

if ($this->downloadUrlGenerator instanceof RouteAwarePathGenerator) {
$field->value->externalData['route'] = $this->downloadUrlGenerator->getRoute($field, $versionInfo);
$field->value->externalData['route_parameters'] = $this->downloadUrlGenerator->getParameters(
$field,
$versionInfo
);
}
}

$field->value->externalData['uri'] = $uri;
}
}

Expand Down
60 changes: 38 additions & 22 deletions eZ/Publish/Core/FieldType/BinaryBase/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

use eZ\Publish\Core\FieldType\FieldType;
use eZ\Publish\Core\Base\Exceptions\InvalidArgumentValue;
use eZ\Publish\Core\FieldType\Media\Value;
use eZ\Publish\Core\FieldType\ValidationError;
use eZ\Publish\API\Repository\Values\ContentType\FieldDefinition;
use eZ\Publish\SPI\FieldType\BinaryBase\RouteAwarePathGenerator;
use eZ\Publish\SPI\FieldType\Value as SPIValue;
use eZ\Publish\SPI\Persistence\Content\FieldValue as PersistenceValue;
use eZ\Publish\Core\FieldType\Value as BaseValue;
Expand All @@ -20,7 +22,7 @@
abstract class Type extends FieldType
{
/**
* @see eZ\Publish\Core\FieldType::$validatorConfigurationSchema
* @see \eZ\Publish\Core\FieldType\FieldType::$validatorConfigurationSchema
*/
protected $validatorConfigurationSchema = [
'FileSizeValidator' => [
Expand All @@ -34,12 +36,16 @@ abstract class Type extends FieldType
/** @var \eZ\Publish\Core\FieldType\Validator[] */
private $validators;

/** @var \eZ\Publish\SPI\FieldType\BinaryBase\RouteAwarePathGenerator|null */
protected $routeAwarePathGenerator;

/**
* @param \eZ\Publish\Core\FieldType\Validator[] $validators
*/
public function __construct(array $validators)
public function __construct(array $validators, ?RouteAwarePathGenerator $routeAwarePathGenerator = null)
{
$this->validators = $validators;
$this->routeAwarePathGenerator = $routeAwarePathGenerator;
}

/**
Expand All @@ -51,6 +57,20 @@ public function __construct(array $validators)
*/
abstract protected function createValue(array $inputValue);

final protected function regenerateUri(array $inputValue): array
{
if (isset($this->routeAwarePathGenerator, $inputValue['route'])) {
$inputValue['uri'] = $this->routeAwarePathGenerator->generate(
$inputValue['route'],
$inputValue['route_parameters'] ?? []
);

unset($inputValue['route'], $inputValue['route_parameters']);
}

return $inputValue;
}

/**
* Returns the name of the given field value.
*
Expand Down Expand Up @@ -252,27 +272,23 @@ public function fromPersistenceValue(PersistenceValue $fieldValue)
{
// Restored data comes in $data, since it has already been processed
// there might be more data in the persistence value than needed here
$result = $this->fromHash(
[
'id' => (isset($fieldValue->externalData['id'])
? $fieldValue->externalData['id']
: null),
'fileName' => (isset($fieldValue->externalData['fileName'])
? $fieldValue->externalData['fileName']
: null),
'fileSize' => (isset($fieldValue->externalData['fileSize'])
? $fieldValue->externalData['fileSize']
: null),
'mimeType' => (isset($fieldValue->externalData['mimeType'])
? $fieldValue->externalData['mimeType']
: null),
'uri' => (isset($fieldValue->externalData['uri'])
? $fieldValue->externalData['uri']
: null),
]
);
$hash = [
'id' => $fieldValue->externalData['id'] ?? null,
'fileName' => $fieldValue->externalData['fileName'] ?? null,
'fileSize' => $fieldValue->externalData['fileSize'] ?? null,
'mimeType' => $fieldValue->externalData['mimeType'] ?? null,
'uri' => $fieldValue->externalData['uri'] ?? null,
];

if (isset($fieldValue->externalData['route'])) {
$hash['route'] = $fieldValue->externalData['route'];
}

if (isset($fieldValue->externalData['route_parameters'])) {
$hash['route_parameters'] = $fieldValue->externalData['route_parameters'];
}

return $result;
return $this->fromHash($hash);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion eZ/Publish/Core/FieldType/BinaryFile/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ public function getEmptyValue()
*
* @param array $inputValue
*
* @return Value
* @return \eZ\Publish\Core\FieldType\BinaryFile\Value
*/
protected function createValue(array $inputValue)
{
$inputValue = $this->regenerateUri($inputValue);

return new Value($inputValue);
}

Expand Down
4 changes: 3 additions & 1 deletion eZ/Publish/Core/FieldType/Media/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,12 @@ public function validateFieldSettings($fieldSettings)
*
* @param array $inputValue
*
* @return Value
* @return \eZ\Publish\Core\FieldType\Media\Value
*/
protected function createValue(array $inputValue)
{
$inputValue = $this->regenerateUri($inputValue);

return new Value($inputValue);
}

Expand Down
77 changes: 74 additions & 3 deletions eZ/Publish/Core/FieldType/Tests/BinaryFileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
use eZ\Publish\Core\FieldType\BinaryFile\Type as BinaryFileType;
use eZ\Publish\Core\FieldType\BinaryFile\Value as BinaryFileValue;
use eZ\Publish\Core\Base\Exceptions\InvalidArgumentValue;
use eZ\Publish\Core\FieldType\FieldType;
use eZ\Publish\Core\FieldType\ValidationError;
use eZ\Publish\SPI\FieldType\BinaryBase\RouteAwarePathGenerator;

/**
* @group fieldType
* @group ezbinaryfile
*
* @covers \eZ\Publish\Core\FieldType\BinaryFile\Type
*/
class BinaryFileTest extends BinaryBaseTest
{
Expand All @@ -26,11 +30,14 @@ class BinaryFileTest extends BinaryBaseTest
* NOT take care for test case wide caching of the field type, just return
* a new instance from this method!
*
* @return FieldType
* @return \eZ\Publish\Core\FieldType\FieldType
*/
protected function createFieldTypeUnderTest()
protected function createFieldTypeUnderTest(): FieldType
{
$fieldType = new BinaryFileType([$this->getBlackListValidatorMock()]);
$fieldType = new BinaryFileType(
[$this->getBlackListValidatorMock()],
$this->getRouteAwarePathGenerator()
);
$fieldType->setTransformationProcessor($this->getTransformationProcessorMock());

return $fieldType;
Expand Down Expand Up @@ -445,6 +452,55 @@ public function provideInputForFromHash()
]
),
],
[
[
'id' => __FILE__,
'fileName' => 'sindelfingen.jpg',
'fileSize' => 2342,
'downloadCount' => 0,
'mimeType' => 'image/jpeg',
'uri' => 'some_uri_acquired_from_SPI',
'route' => 'some_route',
],
new BinaryFileValue(
[
'id' => null,
'inputUri' => __FILE__,
'path' => __FILE__,
'fileName' => 'sindelfingen.jpg',
'fileSize' => 2342,
'downloadCount' => 0,
'mimeType' => 'image/jpeg',
'uri' => '__GENERATED_URI__',
]
),
],
[
[
'id' => __FILE__,
'fileName' => 'sindelfingen.jpg',
'fileSize' => 2342,
'downloadCount' => 0,
'mimeType' => 'image/jpeg',
'uri' => 'some_uri_acquired_from_SPI',
'route' => 'some_route',
'route_parameters' => [
'any_param' => true,
],
],
new BinaryFileValue(
[
'id' => null,
'inputUri' => __FILE__,
'path' => __FILE__,
'fileName' => 'sindelfingen.jpg',
'fileSize' => 2342,
'downloadCount' => 0,
'mimeType' => 'image/jpeg',
'uri' => '__GENERATED_URI_WITH_PARAMS__',
]
),
],
// @todo: Provide upload struct (via REST)!
];
}
Expand Down Expand Up @@ -582,4 +638,19 @@ public function provideInvalidDataForValidate()
],
];
}

private function getRouteAwarePathGenerator(): RouteAwarePathGenerator
{
$mock = $this->createMock(RouteAwarePathGenerator::class);
$mock->method('generate')
->willReturnCallback(static function (string $route, array $routeParameters = []): string {
if ($routeParameters) {
return '__GENERATED_URI_WITH_PARAMS__';
}

return '__GENERATED_URI__';
});

return $mock;
}
}
2 changes: 1 addition & 1 deletion eZ/Publish/Core/FieldType/Tests/FieldTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ public function testToHash($inputValue, $expectedResult)
}

/**
* @param mixed $inputValue
* @param mixed $inputHash
* @param array $expectedResult
*
* @dataProvider provideInputForFromHash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,45 @@
namespace eZ\Publish\Core\MVC\Symfony\FieldType\BinaryBase;

use eZ\Publish\SPI\FieldType\BinaryBase\PathGenerator;
use eZ\Publish\SPI\FieldType\BinaryBase\RouteAwarePathGenerator;
use eZ\Publish\SPI\Persistence\Content\Field;
use eZ\Publish\SPI\Persistence\Content\VersionInfo;
use Symfony\Component\Routing\RouterInterface;

class ContentDownloadUrlGenerator extends PathGenerator
class ContentDownloadUrlGenerator extends PathGenerator implements RouteAwarePathGenerator
{
/** @var \Symfony\Component\Routing\RouterInterface */
private $router;

/** @var string */
private $route = 'ez_content_download_field_id';

public function __construct(RouterInterface $router)
{
$this->router = $router;
}

public function getStoragePathForField(Field $field, VersionInfo $versionInfo)
{
return $this->router->generate(
'ez_content_download_field_id',
[
'contentId' => $versionInfo->contentInfo->id,
'fieldId' => $field->id,
'version' => $versionInfo->versionNo,
]
);
return $this->generate($this->route, $this->getParameters($field, $versionInfo));
}

public function generate(string $route, ?array $parameters = []): string
{
return $this->router->generate($route, $parameters ?? []);
}

public function getRoute(Field $field, VersionInfo $versionInfo): string
{
return $this->route;
}

public function getParameters(Field $field, VersionInfo $versionInfo): array
{
return [
'contentId' => $versionInfo->contentInfo->id,
'fieldId' => $field->id,
'version' => $versionInfo->versionNo,
];
}
}
1 change: 1 addition & 0 deletions eZ/Publish/Core/settings/fieldtypes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ services:
class: "%ezpublish.fieldType.ezbinaryfile.class%"
arguments:
- ['@ezpublish.fieldType.validator.black_list']
- "@?ezpublish.fieldType.ezbinarybase.download_url_generator"
parent: ezpublish.fieldType
tags:
- {name: ezpublish.fieldType, alias: ezbinaryfile}
Expand Down
5 changes: 4 additions & 1 deletion eZ/Publish/SPI/FieldType/BinaryBase/PathGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
use eZ\Publish\SPI\Persistence\Content\VersionInfo;
use eZ\Publish\SPI\Persistence\Content\Field;

abstract class PathGenerator
/**
* @deprecated use \eZ\Publish\SPI\FieldType\BinaryBase\PathGeneratorInterface instead.
*/
abstract class PathGenerator implements PathGeneratorInterface
{
abstract public function getStoragePathForField(Field $field, VersionInfo $versionInfo);
}
17 changes: 17 additions & 0 deletions eZ/Publish/SPI/FieldType/BinaryBase/PathGeneratorInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace eZ\Publish\SPI\FieldType\BinaryBase;

use eZ\Publish\SPI\Persistence\Content\Field;
use eZ\Publish\SPI\Persistence\Content\VersionInfo;

interface PathGeneratorInterface
{
public function getStoragePathForField(Field $field, VersionInfo $versionInfo);
}
Loading