Skip to content

Commit 385f048

Browse files
PowerKiKiPowerKiKi
committed
On upload most images are converted to WebP and have a max size of 3500px #9623
In production data we found many images that are very excessively too large, such as some 68 MB JPG and/or that uses a very poor choice of format, such as BMP, resulting in too big files. This is a problem because dynamic generation of thumbnails will be remarkably slow for the site visitor. And it is also a problem for our storage that need to be over-dimensioned.
1 parent 3f71901 commit 385f048

File tree

13 files changed

+144
-31
lines changed

13 files changed

+144
-31
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,4 @@ specific purpose. It is not meant to be generic.
2626
If code is becoming too complex to adapt to all use-cases, then we should instead
2727
consider implementing it in the application itself instead ot this library.
2828

29-
![Felix](logo.svg)
29+
![Felix](./tests/data/images/logo.svg)

‎src/Model/File.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace Ecodev\Felix\Model;
66

7+
use Psr\Http\Message\UploadedFileInterface;
8+
79
interface File
810
{
911
/**
@@ -20,4 +22,9 @@ public function getFilename(): string;
2022
* Set filename (without path).
2123
*/
2224
public function setFilename(string $filename): void;
25+
26+
/**
27+
* Set the file.
28+
*/
29+
public function setFile(UploadedFileInterface $file): void;
2330
}

‎src/Model/Traits/AbstractFile.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ private function generateUniqueFilename(string $originalFilename): void
111111
/**
112112
* Delete file and throw exception if MIME type is invalid.
113113
*/
114-
private function validateMimeType(): void
114+
final protected function validateMimeType(): void
115115
{
116116
$path = $this->getPath();
117117
$mime = mime_content_type($path);

‎src/Model/Traits/Image.php‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Doctrine\ORM\Mapping as ORM;
88
use GraphQL\Doctrine\Attribute as API;
99
use Imagine\Filter\Basic\Autorotate;
10+
use Imagine\Image\Box;
1011
use Imagine\Image\ImagineInterface;
1112
use Psr\Http\Message\UploadedFileInterface;
1213

@@ -99,14 +100,42 @@ private function readFileInfo(): void
99100
$imagine = $container->get(ImagineInterface::class);
100101
$image = $imagine->open($path);
101102

102-
// Auto-rotate image if EXIF says it's rotated, but only JPG, otherwise it might deteriorate other format (SVG)
103-
if ($this->getMime() === 'image/jpeg') {
104-
$autorotate = new Autorotate();
105-
$autorotate->apply($image);
106-
$image->save($path);
107-
}
108-
109103
$size = $image->getSize();
104+
$maxSize = 3500; // Maximum size ever of an image is a bit less than 4K
105+
$tooBig = $size->getWidth() > $maxSize || $size->getHeight() > $maxSize;
106+
107+
// Pretty much only SVG is better than WebP
108+
$worseThanWebp = in_array($this->getMime(), [
109+
'image/bmp',
110+
'image/x-ms-bmp',
111+
'image/gif',
112+
'image/jpeg',
113+
'image/pjpeg',
114+
'image/png', // We lose animation, even though WebP supports it, but we assume we never use animated PNG anyway
115+
], true);
116+
117+
if ($tooBig || $worseThanWebp) {
118+
// Auto-rotate image if EXIF says it's rotated, but only JPG, otherwise it might deteriorate other format (SVG)
119+
if ($this->getMime() === 'image/jpeg') {
120+
$autorotate = new Autorotate();
121+
$autorotate->apply($image);
122+
}
123+
124+
$image = $image->thumbnail(new Box($maxSize, $maxSize));
125+
$size = $image->getSize();
126+
127+
// Replace extension
128+
if ($worseThanWebp) {
129+
$this->setFilename(preg_replace('~((\\.[^.]+)?$)~', '', $this->getFilename()) . '.webp');
130+
$newPath = $this->getPath();
131+
$image->save($newPath);
132+
unlink($path);
133+
} else {
134+
$image->save($path);
135+
}
136+
137+
$this->validateMimeType();
138+
}
110139

111140
$this->setWidth($size->getWidth());
112141
$this->setHeight($size->getHeight());

‎src/Service/ImageResizer.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function __construct(private readonly ImagineInterface $imagine)
2020
}
2121

2222
/**
23-
* Resize image as JPG or WEBP and return the path to the resized version.
23+
* Resize image as JPG or WebP and return the path to the resized version.
2424
*/
2525
public function resize(Image $image, int $maxHeight, bool $useWebp): string
2626
{
@@ -44,7 +44,7 @@ public function resize(Image $image, int $maxHeight, bool $useWebp): string
4444
}
4545

4646
/**
47-
* Assumes the image is WEBP, converts it to JPG, and return path to JPG version.
47+
* Assumes the image is WebP, converts it to JPG, and return path to JPG version.
4848
*/
4949
public function webpToJpg(Image $image): string
5050
{

‎tests/Handler/ImageHandlerTest.php‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ protected function setUp(): void
2222
// Minimal binary headers to cheat mime detection
2323
$virtualFileSystem = [
2424
'image.png' => '',
25-
'image-100.jpg' => "\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x01\x00\x60",
26-
'image-100.webp' => 'RIFF4<..WEBPVP8',
25+
'image.jpg' => "\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x01\x00\x60",
26+
'image.webp' => 'RIFF4<..WEBPVP8',
2727
];
2828

2929
vfsStream::setup('felix', null, $virtualFileSystem);
@@ -39,7 +39,7 @@ public function testWillServeThumbnailJpgByDefault(): void
3939
$imageResizer->expects(self::once())
4040
->method('resize')
4141
->with($image, $maxHeight, false)
42-
->willReturn('vfs://felix/image-100.jpg');
42+
->willReturn('vfs://felix/image.jpg');
4343

4444
// A request without accept header
4545
$request = new ServerRequest();
@@ -62,7 +62,7 @@ public function testWillServeThumbnailWebpIfAccepted(): void
6262
$imageResizer->expects(self::once())
6363
->method('resize')
6464
->with($image, $maxHeight, true)
65-
->willReturn('vfs://felix/image-100.webp');
65+
->willReturn('vfs://felix/image.webp');
6666

6767
// A request specifically accepting webp images
6868
$request = new ServerRequest();
@@ -78,7 +78,7 @@ public function testWillServeThumbnailWebpIfAccepted(): void
7878

7979
public function testWillServeOriginalWebpIfAccepted(): void
8080
{
81-
$image = $this->createImageMock('vfs://felix/image-100.webp');
81+
$image = $this->createImageMock('vfs://felix/image.webp');
8282
$repository = $this->createRepositoryMock($image);
8383

8484
$imageResizer = $this->createMock(ImageResizer::class);
@@ -97,14 +97,14 @@ public function testWillServeOriginalWebpIfAccepted(): void
9797

9898
public function testWillServeOriginalJpgIfWebpNotAccepted(): void
9999
{
100-
$image = $this->createImageMock('vfs://felix/image-100.webp');
100+
$image = $this->createImageMock('vfs://felix/image.webp');
101101
$repository = $this->createRepositoryMock($image);
102102

103103
$imageResizer = $this->createMock(ImageResizer::class);
104104
$imageResizer->expects(self::once())
105105
->method('webpToJpg')
106106
->with($image)
107-
->willReturn('vfs://felix/image-100.jpg');
107+
->willReturn('vfs://felix/image.jpg');
108108

109109
// A request specifically accepting webp images
110110
$request = new ServerRequest();
@@ -158,8 +158,8 @@ private function createImageMock(string $path = 'vfs://felix/image.png'): Image
158158
$image->expects(self::once())->method('getPath')->willReturn($path);
159159
$image->expects(self::atMost(1))->method('getMime')->willReturn(match ($path) {
160160
'vfs://felix/image.png' => 'image/png',
161-
'vfs://felix/image-100.jpg' => 'image/jpeg',
162-
'vfs://felix/image-100.webp' => 'image/webp',
161+
'vfs://felix/image.jpg' => 'image/jpeg',
162+
'vfs://felix/image.webp' => 'image/webp',
163163
'vfs://felix/totally-non-existing-path' => '',
164164
default => throw new Exception('Unsupported :' . $path),
165165
});

‎tests/Model/Traits/ImageTest.php‎

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,35 +5,112 @@
55
namespace EcodevTests\Felix\Model\Traits;
66

77
use Ecodev\Felix\Model\Traits\Image;
8+
use EcodevTests\Felix\Traits\TestWithContainer;
89
use PHPUnit\Framework\TestCase;
10+
use Psr\Http\Message\UploadedFileInterface;
911

1012
final class ImageTest extends TestCase
1113
{
12-
private \Ecodev\Felix\Model\Image $image;
14+
private const TEMP = '/tmp/felix';
15+
16+
use TestWithContainer {
17+
tearDown as tearDownWithContainer;
18+
}
1319

1420
protected function setUp(): void
1521
{
16-
$this->image = new class() implements \Ecodev\Felix\Model\Image {
17-
use Image;
18-
};
22+
@mkdir(self::TEMP, recursive: true);
23+
}
24+
25+
protected function tearDown(): void
26+
{
27+
$this->tearDownWithContainer();
28+
shell_exec('rm -rf ' . self::TEMP);
1929
}
2030

2131
public function testGetPath(): void
2232
{
23-
$this->image->setFilename('photo.jpg');
33+
$image = $this->createImage();
34+
$image->setFilename('photo.jpg');
2435

25-
self::assertSame('photo.jpg', $this->image->getFilename());
36+
self::assertSame('photo.jpg', $image->getFilename());
2637
$appPath = realpath('.');
2738
$expected = $appPath . '/data/images/photo.jpg';
28-
self::assertSame($expected, $this->image->getPath());
39+
self::assertSame($expected, $image->getPath());
2940
}
3041

3142
public function testDimension(): void
3243
{
33-
$this->image->setWidth(123);
34-
$this->image->setHeight(456);
44+
$image = $this->createImage();
45+
$image->setWidth(123);
46+
$image->setHeight(456);
47+
48+
self::assertSame(123, $image->getWidth());
49+
self::assertSame(456, $image->getHeight());
50+
}
51+
52+
/**
53+
* @dataProvider providerSetFile
54+
*/
55+
public function testSetFile(string $filename, int $width, int $height, bool $isSvg = false): void
56+
{
57+
$this->createDefaultFelixContainer();
58+
59+
$file = $this->createFileToUpload($filename);
60+
$image = $this->createImageForUpload();
61+
62+
$image->setFile($file);
63+
self::assertSame($width, $image->getWidth());
64+
self::assertSame($height, $image->getHeight());
65+
self::assertSame($isSvg ? 'image/svg+xml' : 'image/webp', $image->getMime());
66+
self::assertStringEndsWith($isSvg ? '.svg' : '.webp', $image->getPath());
67+
}
68+
69+
public function providerSetFile(): iterable
70+
{
71+
yield 'jpg is converted to webp' => ['image.jpg', 400, 400];
72+
yield 'png is converted to webp' => ['image.png', 400, 300];
73+
yield 'svg is untouched' => ['logo.svg', 445, 488, true];
74+
yield 'webp is untouched' => ['image.webp', 400, 400];
75+
yield 'huge jpg is resized to webp' => ['huge.jpg', 3500, 19];
76+
yield 'huge webp is resized' => ['huge.webp', 3500, 19];
77+
}
78+
79+
private function createImage(): \Ecodev\Felix\Model\Image
80+
{
81+
return new class() implements \Ecodev\Felix\Model\Image {
82+
use Image;
83+
};
84+
}
85+
86+
private function createImageForUpload(): \Ecodev\Felix\Model\Image
87+
{
88+
return new class() implements \Ecodev\Felix\Model\Image {
89+
use Image;
90+
91+
public function getBasePath(): string
92+
{
93+
return '/tmp/felix/';
94+
}
95+
96+
public function getPath(): string
97+
{
98+
return $this->getBasePath() . $this->getFilename();
99+
}
100+
};
101+
}
102+
103+
private function createFileToUpload(string $filename): UploadedFileInterface
104+
{
105+
$file = $this->createMock(UploadedFileInterface::class);
106+
$file->expects(self::once())
107+
->method('getClientFilename')
108+
->willReturn("useless-prefix-$filename");
109+
110+
$file->expects(self::once())
111+
->method('moveTo')
112+
->willReturnCallback(fn ($dest) => copy("tests/data/images/$filename", $dest));
35113

36-
self::assertSame(123, $this->image->getWidth());
37-
self::assertSame(456, $this->image->getHeight());
114+
return $file;
38115
}
39116
}

‎tests/data/images/huge.jpg‎

798 Bytes
Loading

‎tests/data/images/huge.webp‎

36 Bytes
Loading

‎tests/data/images/image.jpg‎

10.3 KB
Loading

0 commit comments

Comments
 (0)