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

Encode using media type instead of extension #410

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
{
"name": "Jonathan Reinink",
"email": "[email protected]",
"homepage": "http://reinink.ca"
"homepage": "https://reinink.ca/"
},
{
"name": "Titouan Galopin",
Expand Down
14 changes: 4 additions & 10 deletions src/Api/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class Api implements ApiInterface
/**
* Create API instance.
*
* @param ImageManager $imageManager Intervention image manager.
* @param array $manipulators Collection of manipulators.
* @param ImageManager $imageManager Intervention image manager.
* @param list<ManipulatorInterface|null> $manipulators Collection of manipulators.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think |null is necessary

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, its been fixed

*/
public function __construct(ImageManager $imageManager, array $manipulators)
{
Expand Down Expand Up @@ -58,17 +58,11 @@ public function getImageManager(): ImageManager
/**
* Set the manipulators.
*
* @param ManipulatorInterface[] $manipulators Collection of manipulators.
* @param array $manipulators Collection of manipulators.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*/
* @throws \InvalidArgumentException
*/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ADmad good catch, I added the missing phpdoc

public function setManipulators(array $manipulators): void
{
foreach ($manipulators as $manipulator) {
if (!($manipulator instanceof ManipulatorInterface)) {
throw new \InvalidArgumentException('Not a valid manipulator.');
}
}

$this->manipulators = $manipulators;
$this->manipulators = array_filter($manipulators, fn ($manipulator) => $manipulator instanceof ManipulatorInterface || throw new \InvalidArgumentException('Not a valid manipulator.'));
}

/**
Expand Down
129 changes: 90 additions & 39 deletions src/Api/Encoder.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Intervention\Image\Interfaces\EncodedImageInterface;
use Intervention\Image\Interfaces\ImageInterface;
use Intervention\Image\MediaType;

/**
* Encoder Api class to convert a given image to a specific format.
Expand Down Expand Up @@ -60,36 +61,42 @@ public function getParam(string $name): mixed
*/
public function run(ImageInterface $image): EncodedImageInterface
{
$format = $this->getFormat($image);
$quality = $this->getQuality();
$shouldInterlace = filter_var($this->getParam('interlace'), FILTER_VALIDATE_BOOLEAN);
$encoderOptions = [];
$mediaType = $this->getMediaType($image);

if ('pjpg' === $format) {
$shouldInterlace = true;
$format = 'jpg';
if (MediaType::IMAGE_PJPEG === $mediaType) {
$encoderOptions['progressive'] = true;
}

$encoderOptions = [];
switch ($format) {
case 'avif':
case 'heic':
case 'tiff':
case 'webp':
$encoderOptions['quality'] = $quality;
break;
case 'jpg':
$encoderOptions['quality'] = $quality;
$encoderOptions['progressive'] = $shouldInterlace;
break;
case 'gif':
case 'png':
$encoderOptions['interlaced'] = $shouldInterlace;
break;
default:
throw new \Exception("Invalid format provided: {$format}");
if ($this->allowsQuality($mediaType)) {
$encoderOptions['quality'] = $this->getQuality();
}

return $image->encodeByExtension($format, ...$encoderOptions);
if ($this->allowsInterlaced($mediaType)) {
$encoderOptions['interlaced'] = filter_var($this->getParam('interlace'), FILTER_VALIDATE_BOOLEAN);
}

return $mediaType->format()->encoder(...array_filter($encoderOptions))->encode($image);
}

/**
* Resolve media type.
*
* @throws \Exception
*/
public function getMediaType(ImageInterface $image): MediaType
{
$fm = (string) $this->getParam('fm');

if ('' !== $fm) {
return self::supportedMediaTypes()[$fm] ?? throw new \Exception("Invalid format provided: {$fm}");
}

try {
return MediaType::from($image->origin()->mediaType());
} catch (\ValueError) {
return MediaType::IMAGE_JPEG;
}
}

/**
Expand All @@ -98,17 +105,18 @@ public function run(ImageInterface $image): EncodedImageInterface
* @param ImageInterface $image The source image.
*
* @return string The resolved format.
*
* @psalm-suppress RiskyTruthyFalsyComparison
*/
public function getFormat(ImageInterface $image): string
{
$fm = (string) $this->getParam('fm');
ADmad marked this conversation as resolved.
Show resolved Hide resolved
try {
$mediaType = $this->getMediaType($image);

if ($fm && array_key_exists($fm, static::supportedFormats())) {
return $fm;
return array_search($mediaType->value, self::supportedFormats(), true) ?: 'jpg';
} catch (\Exception) {
return 'jpg';
}

/** @psalm-suppress RiskyTruthyFalsyComparison */
return array_search($image->origin()->mediaType(), static::supportedFormats(), true) ?: 'jpg';
}

/**
Expand All @@ -117,19 +125,62 @@ public function getFormat(ImageInterface $image): string
* @return array<string,string>
*/
public static function supportedFormats(): array
{
return array_map(fn (MediaType $mediaType) => $mediaType->value, self::supportedMediaTypes());
}

/**
* Get a list of supported image formats and media types.
*
* @return array<string,MediaType>
*/
public static function supportedMediaTypes(): array
{
return [
'avif' => 'image/avif',
'gif' => 'image/gif',
'jpg' => 'image/jpeg',
'pjpg' => 'image/jpeg',
'png' => 'image/png',
'webp' => 'image/webp',
'tiff' => 'image/tiff',
'heic' => 'image/heic',
'avif' => MediaType::IMAGE_AVIF,
'bmp' => MediaType::IMAGE_BMP,
'gif' => MediaType::IMAGE_GIF,
'heic' => MediaType::IMAGE_HEIC,
'jpg' => MediaType::IMAGE_JPEG,
'pjpg' => MediaType::IMAGE_PJPEG,
'png' => MediaType::IMAGE_PNG,
'tiff' => MediaType::IMAGE_TIFF,
'webp' => MediaType::IMAGE_WEBP,
];
}

/**
* Checks if we can pass the quality parameter to the encoder.
*/
public function allowsQuality(MediaType $mediaType): bool
{
return !in_array($mediaType, [
MediaType::IMAGE_GIF,
MediaType::IMAGE_PNG,
MediaType::IMAGE_X_PNG,
MediaType::IMAGE_BMP,
MediaType::IMAGE_MS_BMP,
MediaType::IMAGE_X_BITMAP,
MediaType::IMAGE_X_BMP,
MediaType::IMAGE_X_MS_BMP,
MediaType::IMAGE_X_XBITMAP,
MediaType::IMAGE_X_WINDOWS_BMP,
MediaType::IMAGE_X_WIN_BITMAP,
]);
}

/**
* hecks if we can pass the interlaced parameter to the encoder.
*/
public function allowsInterlaced(MediaType $mediaType): bool
{
return in_array($mediaType, [
MediaType::IMAGE_PNG,
MediaType::IMAGE_X_PNG,
MediaType::IMAGE_GIF,
]);
}

/**
* Resolve quality.
*
Expand Down
3 changes: 2 additions & 1 deletion src/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use League\Glide\Manipulators\Filter;
use League\Glide\Manipulators\Flip;
use League\Glide\Manipulators\Gamma;
use League\Glide\Manipulators\ManipulatorInterface;
use League\Glide\Manipulators\Orientation;
use League\Glide\Manipulators\Pixelate;
use League\Glide\Manipulators\Sharpen;
Expand Down Expand Up @@ -237,7 +238,7 @@ public function getImageManager(): ImageManager
/**
* Get image manipulators.
*
* @return array Image manipulators.
* @return list<ManipulatorInterface> Image manipulators.
*/
public function getManipulators(): array
{
Expand Down
28 changes: 17 additions & 11 deletions tests/Api/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
namespace League\Glide\Api;

use Intervention\Image\ImageManager;
use Intervention\Image\Interfaces\DriverInterface;
use Intervention\Image\Interfaces\EncodedImageInterface;
use Intervention\Image\Interfaces\ImageInterface;
use Intervention\Image\Origin;
use League\Glide\Manipulators\ManipulatorInterface;
use PHPUnit\Framework\TestCase;

class ApiTest extends TestCase
{
private $api;
private Api $api;

public function setUp(): void
{
Expand All @@ -24,50 +26,54 @@ public function tearDown(): void
\Mockery::close();
}

public function testCreateInstance()
public function testCreateInstance(): void
{
$this->assertInstanceOf(Api::class, $this->api);
}

public function testSetImageManager()
public function testSetImageManager(): void
{
$this->api->setImageManager(ImageManager::gd());
$this->assertInstanceOf(ImageManager::class, $this->api->getImageManager());
}

public function testGetImageManager()
public function testGetImageManager(): void
{
$this->assertInstanceOf(ImageManager::class, $this->api->getImageManager());
}

public function testSetManipulators()
public function testSetManipulators(): void
{
$this->api->setManipulators([\Mockery::mock(ManipulatorInterface::class)]);
$manipulators = $this->api->getManipulators();
$this->assertInstanceOf(ManipulatorInterface::class, $manipulators[0]);
}

public function testSetInvalidManipulator()
public function testSetInvalidManipulator(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Not a valid manipulator.');

$this->api->setManipulators([new \stdClass()]);
}

public function testGetManipulators()
public function testGetManipulators(): void
{
$this->assertEquals([], $this->api->getManipulators());
}

public function testRun()
public function testRun(): void
{
$image = \Mockery::mock(ImageInterface::class, function ($mock) {
$mock->shouldReceive('origin')->andReturn(\Mockery::mock('\Intervention\Image\Origin', function ($mock) {
$mock->shouldReceive('origin')->andReturn(\Mockery::mock(Origin::class, function ($mock) {
$mock->shouldReceive('mediaType')->andReturn('image/png');
}));

$mock->shouldReceive('encodeByExtension')->with('png')->andReturn(\Mockery::mock(EncodedImageInterface::class, function ($mock) {
$mock->shouldReceive('driver')->andReturn(\Mockery::mock(DriverInterface::class, function ($mock) {
$mock->shouldReceive('supports');
}));

$mock->shouldReceive('encode')->andReturn(\Mockery::mock(EncodedImageInterface::class, function ($mock) {
$mock->shouldReceive('toString')->andReturn('encoded');
}));
});
Expand All @@ -82,7 +88,7 @@ public function testRun()
$api = new Api($manager, [$manipulator]);

$this->assertEquals('encoded', $api->run(
file_get_contents(dirname(__FILE__, 2).'/files/red-pixel.png'),
(string) file_get_contents(dirname(__FILE__, 2).'/files/red-pixel.png'),
[]
));
}
Expand Down
Loading