Skip to content

Commit

Permalink
IBX-7502: Added file size validation for image asset field type (#320)
Browse files Browse the repository at this point in the history
* IBX-7502: Added file size validation for image asset field type

https://issues.ibexa.co/browse/IBX-7502

* [Tests] Adjusted unit tests

* [PHPStan] Regenerated baseline

* Dropped contentTypeService argument

* Added early return from validation when content is not an asset

* Update tests/lib/FieldType/ImageAssetTest.php

Co-authored-by: Konrad Oboza <[email protected]>

* [PHPStan] Regenerated baseline

---------

Co-authored-by: Konrad Oboza <[email protected]>
  • Loading branch information
ciastektk and konradoboza authored Jan 16, 2024
1 parent c249ab4 commit ed512f1
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 62 deletions.
6 changes: 1 addition & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -22689,6 +22689,7 @@ parameters:
message: "#^Method Ibexa\\\\Core\\\\Search\\\\Legacy\\\\Content\\\\Gateway\\\\CriterionHandler\\\\Ancestor\\:\\:handle\\(\\) has parameter \\$languageSettings with no value type specified in iterable type array\\.$#"
count: 1
path: src/lib/Search/Legacy/Content/Gateway/CriterionHandler/Ancestor.php

-
message: "#^Method Ibexa\\\\Core\\\\Search\\\\Legacy\\\\Content\\\\Gateway\\\\CriterionHandler\\\\LocationId\\:\\:handle\\(\\) has parameter \\$languageSettings with no value type specified in iterable type array\\.$#"
count: 1
Expand Down Expand Up @@ -43584,11 +43585,6 @@ parameters:
count: 1
path: tests/lib/FieldType/ImageAssetTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\FieldType\\\\ImageAssetTest\\:\\:testValidateValidNonEmptyAssetValue\\(\\) has no return type specified\\.$#"
count: 1
path: tests/lib/FieldType/ImageAssetTest.php

-
message: "#^Access to an undefined property Ibexa\\\\Tests\\\\Core\\\\FieldType\\\\ImageTest\\:\\:\\$mimeTypeDetectorMock\\.$#"
count: 1
Expand Down
76 changes: 53 additions & 23 deletions src/lib/FieldType/ImageAsset/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
use Ibexa\Contracts\Core\FieldType\Value as SPIValue;
use Ibexa\Contracts\Core\Persistence\Content\Handler as SPIContentHandler;
use Ibexa\Contracts\Core\Repository\ContentService;
use Ibexa\Contracts\Core\Repository\ContentTypeService;
use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException;
use Ibexa\Contracts\Core\Repository\Values\Content\Content;
use Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo;
use Ibexa\Contracts\Core\Repository\Values\Content\Relation;
use Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition;
Expand All @@ -30,29 +30,18 @@ class Type extends FieldType implements TranslationContainerInterface
/** @var \Ibexa\Contracts\Core\Repository\ContentService */
private $contentService;

/** @var \Ibexa\Contracts\Core\Repository\ContentTypeService */
private $contentTypeService;

/** @var \Ibexa\Core\FieldType\ImageAsset\AssetMapper */
private $assetMapper;

/** @var \Ibexa\Contracts\Core\Persistence\Content\Handler */
private $handler;

/**
* @param \Ibexa\Contracts\Core\Repository\ContentService $contentService
* @param \Ibexa\Contracts\Core\Repository\ContentTypeService $contentTypeService
* @param \Ibexa\Core\FieldType\ImageAsset\AssetMapper $mapper
* @param \Ibexa\Contracts\Core\Persistence\Content\Handler $handler
*/
public function __construct(
ContentService $contentService,
ContentTypeService $contentTypeService,
AssetMapper $mapper,
SPIContentHandler $handler
) {
$this->contentService = $contentService;
$this->contentTypeService = $contentTypeService;
$this->assetMapper = $mapper;
$this->handler = $handler;
}
Expand All @@ -64,6 +53,10 @@ public function __construct(
* @param \Ibexa\Core\FieldType\ImageAsset\Value $fieldValue The field value for which an action is performed
*
* @return \Ibexa\Contracts\Core\FieldType\ValidationError[]
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException
*/
public function validate(FieldDefinition $fieldDefinition, SPIValue $fieldValue): array
{
Expand All @@ -78,18 +71,21 @@ public function validate(FieldDefinition $fieldDefinition, SPIValue $fieldValue)
);

if (!$this->assetMapper->isAsset($content)) {
$currentContentType = $this->contentTypeService->loadContentType(
(int)$content->contentInfo->contentTypeId
);
return [
new ValidationError(
'Content %type% is not a valid asset target',
null,
[
'%type%' => $content->getContentType()->identifier,
],
'destinationContentId'
),
];
}

$errors[] = new ValidationError(
'Content %type% is not a valid asset target',
null,
[
'%type%' => $currentContentType->identifier,
],
'destinationContentId'
);
$validationError = $this->validateMaxFileSize($content);
if (null !== $validationError) {
$errors[] = $validationError;
}

return $errors;
Expand Down Expand Up @@ -296,6 +292,40 @@ public static function getTranslationMessages(): array
Message::create('ezimageasset.name', 'ibexa_fieldtypes')->setDesc('Image Asset'),
];
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
*/
private function validateMaxFileSize(Content $content): ?ValidationError
{
$fileSize = $this->assetMapper
->getAssetValue($content)
->getFileSize();

$assetValidatorConfiguration = $this->assetMapper
->getAssetFieldDefinition()
->getValidatorConfiguration();

$maxFileSizeMB = $assetValidatorConfiguration['FileSizeValidator']['maxFileSize'];
$maxFileSizeKB = $maxFileSizeMB * 1024 * 1024;

if (
$maxFileSizeKB > 0
&& $fileSize > $maxFileSizeKB
) {
return new ValidationError(
'The file size cannot exceed %size% megabyte.',
'The file size cannot exceed %size% megabytes.',
[
'%size%' => $maxFileSizeMB,
],
'fileSize'
);
}

return null;
}
}

class_alias(Type::class, 'eZ\Publish\Core\FieldType\ImageAsset\Type');
1 change: 0 additions & 1 deletion src/lib/Resources/settings/fieldtypes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,6 @@ services:
parent: Ibexa\Core\FieldType\FieldType
arguments:
- '@ibexa.api.service.content'
- '@ibexa.api.service.content_type'
- '@Ibexa\Core\FieldType\ImageAsset\AssetMapper'
- '@Ibexa\Core\Persistence\Cache\ContentHandler'
tags:
Expand Down
105 changes: 72 additions & 33 deletions tests/lib/FieldType/ImageAssetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
use Ibexa\Contracts\Core\Persistence\Content\Handler as SPIContentHandler;
use Ibexa\Contracts\Core\Persistence\Content\VersionInfo;
use Ibexa\Contracts\Core\Repository\ContentService;
use Ibexa\Contracts\Core\Repository\ContentTypeService;
use Ibexa\Contracts\Core\Repository\Values\Content\Content;
use Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo;
use Ibexa\Contracts\Core\Repository\Values\Content\Relation;
use Ibexa\Contracts\Core\Repository\Values\ContentType\ContentType;
use Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition;
use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
use Ibexa\Core\FieldType\Image\Value;
use Ibexa\Core\FieldType\ImageAsset;
use Ibexa\Core\FieldType\ValidationError;

Expand All @@ -33,9 +33,6 @@ class ImageAssetTest extends FieldTypeTest
/** @var \Ibexa\Contracts\Core\Repository\ContentService|\PHPUnit\Framework\MockObject\MockObject */
private $contentServiceMock;

/** @var \Ibexa\Contracts\Core\Repository\ContentTypeService|\PHPUnit\Framework\MockObject\MockObject */
private $contentTypeServiceMock;

/** @var \Ibexa\Core\FieldType\ImageAsset\AssetMapper|\PHPUnit\Framework\MockObject\MockObject */
private $assetMapperMock;

Expand All @@ -50,7 +47,6 @@ protected function setUp(): void
parent::setUp();

$this->contentServiceMock = $this->createMock(ContentService::class);
$this->contentTypeServiceMock = $this->createMock(ContentTypeService::class);
$this->assetMapperMock = $this->createMock(ImageAsset\AssetMapper::class);
$this->contentHandlerMock = $this->createMock(SPIContentHandler::class);
$versionInfo = new VersionInfo([
Expand Down Expand Up @@ -95,7 +91,6 @@ protected function createFieldTypeUnderTest()
{
return new ImageAsset\Type(
$this->contentServiceMock,
$this->contentTypeServiceMock,
$this->assetMapperMock,
$this->contentHandlerMock
);
Expand Down Expand Up @@ -241,23 +236,17 @@ public function testValidateNonAsset()
{
$destinationContentId = 7;
$destinationContent = $this->createMock(Content::class);
$invalidContentTypeId = 789;
$invalidContentTypeIdentifier = 'article';
$invalidContentType = $this->createMock(ContentType::class);

$destinationContentInfo = $this->createMock(ContentInfo::class);

$destinationContentInfo
$invalidContentType
->expects($this->once())
->method('__get')
->with('contentTypeId')
->willReturn($invalidContentTypeId);
->with('identifier')
->willReturn($invalidContentTypeIdentifier);

$destinationContent
->expects($this->once())
->method('__get')
->with('contentInfo')
->willReturn($destinationContentInfo);
->method('getContentType')
->willReturn($invalidContentType);

$this->contentServiceMock
->expects($this->once())
Expand All @@ -271,18 +260,6 @@ public function testValidateNonAsset()
->with($destinationContent)
->willReturn(false);

$this->contentTypeServiceMock
->expects($this->once())
->method('loadContentType')
->with($invalidContentTypeId)
->willReturn($invalidContentType);

$invalidContentType
->expects($this->once())
->method('__get')
->with('identifier')
->willReturn($invalidContentTypeIdentifier);

$validationErrors = $this->doValidate([], new ImageAsset\Value($destinationContentId));

$this->assertIsArray($validationErrors);
Expand Down Expand Up @@ -311,8 +288,15 @@ public function provideValidDataForValidate(): array
];
}

public function testValidateValidNonEmptyAssetValue()
{
/**
* @dataProvider provideDataForTestValidateValidNonEmptyAssetValue
*
* @param array<\Ibexa\Core\FieldType\ValidationError> $expectedValidationErrors
*/
public function testValidateValidNonEmptyAssetValue(
int $fileSize,
array $expectedValidationErrors
): void {
$destinationContentId = 7;
$destinationContent = $this->createMock(Content::class);

Expand All @@ -328,10 +312,65 @@ public function testValidateValidNonEmptyAssetValue()
->with($destinationContent)
->willReturn(true);

$assetValueMock = $this->createMock(Value::class);
$assetValueMock
->method('getFileSize')
->willReturn($fileSize);

$this->assetMapperMock
->expects($this->once())
->method('getAssetValue')
->with($destinationContent)
->willReturn($assetValueMock);

$fieldDefinitionMock = $this->createMock(FieldDefinition::class);
$fieldDefinitionMock
->method('getValidatorConfiguration')
->willReturn(
[
'FileSizeValidator' => [
'maxFileSize' => 1.4,
],
]
);

$this->assetMapperMock
->method('getAssetFieldDefinition')
->willReturn($fieldDefinitionMock);

$validationErrors = $this->doValidate([], new ImageAsset\Value($destinationContentId));
$this->assertEquals(
$expectedValidationErrors,
$validationErrors
);
}

$this->assertIsArray($validationErrors);
$this->assertEmpty($validationErrors, "Got value:\n" . var_export($validationErrors, true));
/**
* @return iterable<array{
* int,
* array<\Ibexa\Core\FieldType\ValidationError>,
* }>
*/
public function provideDataForTestValidateValidNonEmptyAssetValue(): iterable
{
yield 'No validation errors' => [
123456,
[],
];

yield 'Maximum file size exceeded' => [
12345678912356,
[
new ValidationError(
'The file size cannot exceed %size% megabyte.',
'The file size cannot exceed %size% megabytes.',
[
'%size%' => 1.4,
],
'fileSize'
),
],
];
}

/**
Expand Down

0 comments on commit ed512f1

Please sign in to comment.