Skip to content

Commit aa0cb9c

Browse files
konradobozakonradoboza
committed
IBX-10597: Fixed accessing uninitialized image variation properties while switching to the original variation
1 parent a56a539 commit aa0cb9c

File tree

9 files changed

+235
-141
lines changed

9 files changed

+235
-141
lines changed

‎phpstan-baseline.neon‎

Lines changed: 186 additions & 72 deletions
Large diffs are not rendered by default.

‎src/bundle/Core/Imagine/AliasGenerator.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ public function getVariation(Field $field, VersionInfo $versionInfo, string $var
134134
'imageId' => $imageValue->imageId,
135135
'width' => $variationWidth,
136136
'height' => $variationHeight,
137+
'fileSize' => $imageValue->getFileSize() ?? 0,
138+
'mimeType' => $imageValue->mime ?? '',
137139
]
138140
);
139141
}

‎src/lib/Repository/LocationService.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,8 @@ public function createLocation(ContentInfo $contentInfo, LocationCreateStruct $l
412412

413413
$contentType = $content->getContentType();
414414

415-
$locationCreateStruct->sortField = $locationCreateStruct->sortField
416-
?? ($contentType->defaultSortField ?? Location::SORT_FIELD_NAME);
417-
$locationCreateStruct->sortOrder = $locationCreateStruct->sortOrder
418-
?? ($contentType->defaultSortOrder ?? Location::SORT_ORDER_ASC);
415+
$locationCreateStruct->sortField = $locationCreateStruct->sortField ?? $contentType->defaultSortField;
416+
$locationCreateStruct->sortOrder = $locationCreateStruct->sortOrder ?? $contentType->defaultSortOrder;
419417

420418
$contentInfo = $content->contentInfo;
421419

‎tests/bundle/Core/Imagine/AliasGeneratorTest.php‎

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,19 @@
44
* @copyright Copyright (C) Ibexa AS. All rights reserved.
55
* @license For full copyright and license information view LICENSE file distributed with this source code.
66
*/
7+
declare(strict_types=1);
78

89
namespace Ibexa\Tests\Bundle\Core\Imagine;
910

1011
use DateTime;
1112
use Ibexa\Bundle\Core\Imagine\AliasGenerator;
1213
use Ibexa\Bundle\Core\Imagine\Variation\ImagineAwareAliasGenerator;
14+
use Ibexa\Contracts\Core\FieldType\Value;
1315
use Ibexa\Contracts\Core\FieldType\Value as FieldTypeValue;
1416
use Ibexa\Contracts\Core\Repository\Exceptions\InvalidVariationException;
1517
use Ibexa\Contracts\Core\Repository\Values\Content\Field;
1618
use Ibexa\Contracts\Core\Variation\Values\ImageVariation;
19+
use Ibexa\Contracts\Core\Variation\VariationHandler;
1720
use Ibexa\Contracts\Core\Variation\VariationPathGenerator;
1821
use Ibexa\Core\FieldType\Image\Value as ImageValue;
1922
use Ibexa\Core\FieldType\TextLine\Value as TextLineValue;
@@ -31,46 +34,35 @@
3134
use Liip\ImagineBundle\Imagine\Cache\Resolver\ResolverInterface;
3235
use Liip\ImagineBundle\Imagine\Filter\FilterConfiguration;
3336
use Liip\ImagineBundle\Imagine\Filter\FilterManager;
37+
use PHPUnit\Framework\MockObject\MockObject;
3438
use PHPUnit\Framework\TestCase;
3539
use Psr\Log\LoggerInterface;
3640

37-
class AliasGeneratorTest extends TestCase
41+
final class AliasGeneratorTest extends TestCase
3842
{
39-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Liip\ImagineBundle\Binary\Loader\LoaderInterface */
40-
private $dataLoader;
43+
private MockObject|LoaderInterface $dataLoader;
4144

42-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Liip\ImagineBundle\Imagine\Filter\FilterManager */
43-
private $filterManager;
45+
private MockObject|FilterManager $filterManager;
4446

45-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Liip\ImagineBundle\Imagine\Cache\Resolver\ResolverInterface */
46-
private $ioResolver;
47+
private MockObject|ResolverInterface $ioResolver;
4748

48-
/** @var \Liip\ImagineBundle\Imagine\Filter\FilterConfiguration */
49-
private $filterConfiguration;
49+
private MockObject|FilterConfiguration $filterConfiguration;
5050

51-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Psr\Log\LoggerInterface */
52-
private $logger;
51+
private MockObject|LoggerInterface $logger;
5352

54-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Imagine\Image\ImagineInterface */
55-
private $imagine;
53+
private MockObject|ImagineInterface $imagine;
5654

57-
/** @var \Ibexa\Bundle\Core\Imagine\AliasGenerator */
58-
private $aliasGenerator;
55+
private MockObject|AliasGenerator $aliasGenerator;
5956

60-
/** @var \Ibexa\Contracts\Core\Variation\VariationHandler */
61-
private $decoratedAliasGenerator;
57+
private MockObject|VariationHandler $decoratedAliasGenerator;
6258

63-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Imagine\Image\BoxInterface */
64-
private $box;
59+
private MockObject|BoxInterface $box;
6560

66-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Imagine\Image\ImageInterface */
67-
private $image;
61+
private MockObject|ImageInterface $image;
6862

69-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Core\IO\IOServiceInterface */
70-
private $ioService;
63+
private MockObject|IOServiceInterface $ioService;
7164

72-
/** @var \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Contracts\Core\Variation\VariationPathGenerator */
73-
private $variationPathGenerator;
65+
private MockObject|VariationPathGenerator $variationPathGenerator;
7466

7567
protected function setUp(): void
7668
{
@@ -105,11 +97,8 @@ protected function setUp(): void
10597

10698
/**
10799
* @dataProvider supportsValueProvider
108-
*
109-
* @param \Ibexa\Contracts\Core\FieldType\Value $value
110-
* @param bool $isSupported
111100
*/
112-
public function testSupportsValue($value, $isSupported)
101+
public function testSupportsValue(Value $value, bool $isSupported): void
113102
{
114103
self::assertSame($isSupported, $this->aliasGenerator->supportsValue($value));
115104
}
@@ -123,7 +112,7 @@ public function testSupportsValue($value, $isSupported)
123112
*
124113
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
125114
*/
126-
public function supportsValueProvider()
115+
public function supportsValueProvider(): array
127116
{
128117
return [
129118
[$this->createMock(FieldTypeValue::class), false],
@@ -133,7 +122,7 @@ public function supportsValueProvider()
133122
];
134123
}
135124

136-
public function testGetVariationWrongValue()
125+
public function testGetVariationWrongValue(): void
137126
{
138127
$this->expectException(\InvalidArgumentException::class);
139128

@@ -149,7 +138,7 @@ public function testGetVariationWrongValue()
149138
*
150139
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType
151140
*/
152-
public function testGetVariationNotStored()
141+
public function testGetVariationNotStored(): void
153142
{
154143
$originalPath = 'foo/bar/image.jpg';
155144
$variationName = 'my_variation';
@@ -195,7 +184,7 @@ public function testGetVariationNotStored()
195184
);
196185
}
197186

198-
public function testGetVariationOriginal()
187+
public function testGetVariationOriginal(): void
199188
{
200189
$originalPath = 'foo/bar/image.jpg';
201190
$variationName = 'original';
@@ -209,6 +198,8 @@ public function testGetVariationOriginal()
209198
'imageId' => $imageId,
210199
'width' => $imageWidth,
211200
'height' => $imageHeight,
201+
'fileSize' => 1024,
202+
'mime' => 'image/jpeg',
212203
]
213204
);
214205
$field = new Field([
@@ -242,17 +233,26 @@ public function testGetVariationOriginal()
242233
'imageId' => $imageId,
243234
'height' => $imageHeight,
244235
'width' => $imageWidth,
236+
'fileSize' => 1024,
237+
'mimeType' => 'image/jpeg',
245238
]
246239
);
247-
self::assertEquals($expected, $this->decoratedAliasGenerator->getVariation($field, new VersionInfo(), $variationName));
240+
self::assertEquals(
241+
$expected,
242+
$this->decoratedAliasGenerator->getVariation(
243+
$field,
244+
new VersionInfo(),
245+
$variationName
246+
)
247+
);
248248
}
249249

250250
/**
251251
* Test obtaining Image Variation that hasn't been stored yet and has multiple references.
252252
*
253253
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType
254254
*/
255-
public function testGetVariationNotStoredHavingReferences()
255+
public function testGetVariationNotStoredHavingReferences(): void
256256
{
257257
$originalPath = 'foo/bar/image.jpg';
258258
$variationName = 'my_variation';
@@ -323,7 +323,7 @@ public function testGetVariationNotStoredHavingReferences()
323323
*
324324
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType
325325
*/
326-
public function testGetVariationAlreadyStored()
326+
public function testGetVariationAlreadyStored(): void
327327
{
328328
$originalPath = 'foo/bar/image.jpg';
329329
$variationName = 'my_variation';
@@ -362,7 +362,7 @@ public function testGetVariationAlreadyStored()
362362
);
363363
}
364364

365-
public function testGetVariationOriginalNotFound()
365+
public function testGetVariationOriginalNotFound(): void
366366
{
367367
$this->expectException(SourceImageNotFoundException::class);
368368

@@ -378,7 +378,7 @@ public function testGetVariationOriginalNotFound()
378378
$this->aliasGenerator->getVariation($field, new VersionInfo(), 'foo');
379379
}
380380

381-
public function testGetVariationInvalidVariation()
381+
public function testGetVariationInvalidVariation(): void
382382
{
383383
$this->expectException(InvalidVariationException::class);
384384

‎tests/bundle/IO/.gitkeep‎

Whitespace-only changes.

‎tests/integration/Core/Repository/Regression/EZP21798Test.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class EZP21798Test extends BaseTestCase
2323
* This test will verify that anonymous users can access to a new section
2424
* that it's allowed to
2525
*/
26-
public function testRoleChanges()
26+
public function testRoleChanges(): void
2727
{
2828
$repository = $this->getRepository();
2929
$permissionResolver = $repository->getPermissionResolver();
@@ -62,7 +62,7 @@ public function testRoleChanges()
6262
$contentCreateStructArticle->setField('title', 'Article 1');
6363

6464
$newsLocation = $urlAliasService->lookup('/News');
65-
$locationNews = $locationService->loadLocation($newsLocation->destination);
65+
$locationNews = $locationService->loadLocation((int)$newsLocation->destination);
6666

6767
$locationCreateStructArticle = $locationService->newLocationCreateStruct($locationNews->id);
6868
$draftArticle = $contentService->createContent($contentCreateStructArticle, [$locationCreateStructArticle]);

‎tests/integration/Core/Repository/URLWildcardServiceTest.php‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,6 @@ public function testCreate()
4949
return $urlWildcard;
5050
}
5151

52-
/**
53-
* Test for the create() method.
54-
*
55-
* @param \Ibexa\Contracts\Core\Repository\Values\Content\URLWildcard $urlWildcard
56-
*
57-
* @covers \Ibexa\Contracts\Core\Repository\URLWildcardService::create()
58-
*
59-
* @depends testCreate
60-
*/
61-
public function testCreateSetsIdPropertyOnURLWildcard(URLWildcard $urlWildcard)
62-
{
63-
self::assertNotNull($urlWildcard->id);
64-
}
65-
6652
/**
6753
* Test for the create() method.
6854
*

‎tests/lib/Repository/Values/User/RoleTest.php‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class RoleTest extends TestCase
2323
/**
2424
* Test a new class and default values on properties.
2525
*/
26-
public function testNewClass()
26+
public function testNewClass(): void
2727
{
2828
$this->assertPropertiesCorrect(
2929
[
@@ -40,7 +40,7 @@ public function testNewClass()
4040
*
4141
* @covers \Ibexa\Core\Repository\Values\User\Role::__get
4242
*/
43-
public function testMissingProperty()
43+
public function testMissingProperty(): void
4444
{
4545
$this->expectException(PropertyNotFoundException::class);
4646

@@ -54,7 +54,7 @@ public function testMissingProperty()
5454
*
5555
* @covers \Ibexa\Core\Repository\Values\User\Role::__set
5656
*/
57-
public function testReadOnlyProperty()
57+
public function testReadOnlyProperty(): void
5858
{
5959
$this->expectException(PropertyReadOnlyException::class);
6060

@@ -66,22 +66,19 @@ public function testReadOnlyProperty()
6666
/**
6767
* Test if property exists.
6868
*/
69-
public function testIsPropertySet()
69+
public function testIsPropertySet(): void
7070
{
7171
$role = new Role();
7272
$value = isset($role->notDefined);
7373
self::assertFalse($value);
74-
75-
$value = isset($role->id);
76-
self::assertTrue($value);
7774
}
7875

7976
/**
8077
* Test unsetting a property.
8178
*
8279
* @covers \Ibexa\Core\Repository\Values\User\Role::__unset
8380
*/
84-
public function testUnsetProperty()
81+
public function testUnsetProperty(): void
8582
{
8683
$this->expectException(PropertyReadOnlyException::class);
8784

‎tests/lib/Repository/Values/User/UserTest.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,6 @@ public function testIsPropertySet(): void
7777
$user = new User();
7878
$value = isset($user->notDefined);
7979
self::assertFalse($value);
80-
81-
$value = isset($user->login);
82-
self::assertTrue($value);
8380
}
8481

8582
public function testUnsetProperty(): void

0 commit comments

Comments
 (0)