Skip to content

Commit

Permalink
Merge pull request #1075 from robfrawley/feature-remove-configurable-…
Browse files Browse the repository at this point in the history 
…post-processor-interface

[Filters] [Post Processors] Remove configurable post processor interface for one interface and cleanup filter manager
  • Loading branch information
@robfrawley
robfrawley committed Mar 11, 2018
2 parents 8458724 + fa082eb commit 988d235
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 171 deletions.
187 changes: 120 additions & 67 deletions Imagine/Filter/FilterManager.php
View file
Expand Up @@ -11,12 +11,12 @@

namespace Liip\ImagineBundle\Imagine\Filter;

use Imagine\Image\ImageInterface;
use Imagine\Image\ImagineInterface;
use Liip\ImagineBundle\Binary\BinaryInterface;
use Liip\ImagineBundle\Binary\FileBinaryInterface;
use Liip\ImagineBundle\Binary\MimeTypeGuesserInterface;
use Liip\ImagineBundle\Imagine\Filter\Loader\LoaderInterface;
use Liip\ImagineBundle\Imagine\Filter\PostProcessor\ConfigurablePostProcessorInterface;
use Liip\ImagineBundle\Imagine\Filter\PostProcessor\PostProcessorInterface;
use Liip\ImagineBundle\Model\Binary;

Expand Down Expand Up @@ -52,11 +52,8 @@ class FilterManager
* @param ImagineInterface $imagine
* @param MimeTypeGuesserInterface $mimeTypeGuesser
*/
public function __construct(
FilterConfiguration $filterConfig,
ImagineInterface $imagine,
MimeTypeGuesserInterface $mimeTypeGuesser
) {
public function __construct(FilterConfiguration $filterConfig, ImagineInterface $imagine, MimeTypeGuesserInterface $mimeTypeGuesser)
{
$this->filterConfig = $filterConfig;
$this->imagine = $imagine;
$this->mimeTypeGuesser = $mimeTypeGuesser;
Expand All @@ -68,7 +65,7 @@ public function __construct(
* @param string $filter
* @param LoaderInterface $loader
*/
public function addLoader($filter, LoaderInterface $loader)
public function addLoader(string $filter, LoaderInterface $loader): void
{
$this->loaders[$filter] = $loader;
}
Expand All @@ -79,15 +76,15 @@ public function addLoader($filter, LoaderInterface $loader)
* @param string $name
* @param PostProcessorInterface $postProcessor
*/
public function addPostProcessor($name, PostProcessorInterface $postProcessor)
public function addPostProcessor(string $name, PostProcessorInterface $postProcessor): void
{
$this->postProcessors[$name] = $postProcessor;
}

/**
* @return FilterConfiguration
*/
public function getFilterConfiguration()
public function getFilterConfiguration(): FilterConfiguration
{
return $this->filterConfig;
}
Expand All @@ -100,37 +97,89 @@ public function getFilterConfiguration()
*
* @return BinaryInterface
*/
public function apply(BinaryInterface $binary, array $config)
public function apply(BinaryInterface $binary, array $config): BinaryInterface
{
$config = array_replace([
'filters' => [],
$config += [
'quality' => 100,
'animated' => false,
], $config);
];

return $this->applyPostProcessors($this->applyFilters($binary, $config), $config);
}

/**
* @param BinaryInterface $binary
* @param array $config
*
* @return BinaryInterface
*/
public function applyFilters(BinaryInterface $binary, array $config): BinaryInterface
{
if ($binary instanceof FileBinaryInterface) {
$image = $this->imagine->open($binary->getPath());
} else {
$image = $this->imagine->load($binary->getContent());
}

foreach ($config['filters'] as $eachFilter => $eachOptions) {
if (!isset($this->loaders[$eachFilter])) {
throw new \InvalidArgumentException(sprintf(
'Could not find filter loader for "%s" filter type', $eachFilter
));
foreach ($this->sanitizeFilters($config['filters'] ?? []) as $name => $options) {
$prior = $image;
$image = $this->loaders[$name]->load($image, $options);

if ($prior !== $image) {
$this->destroyImage($prior);
}
}

$prevImage = $image;
$image = $this->loaders[$eachFilter]->load($image, $eachOptions);
return $this->exportConfiguredImageBinary($binary, $image, $config);
}

// If the filter returns a different image object destruct the old one because imagick keeps consuming memory if we don't
// See https://github.com/liip/LiipImagineBundle/pull/682
if ($prevImage !== $image && method_exists($prevImage, '__destruct')) {
$prevImage->__destruct();
}
/**
* Apply the provided filter set on the given binary.
*
* @param BinaryInterface $binary
* @param string $filter
* @param array $runtimeConfig
*
* @throws \InvalidArgumentException
*
* @return BinaryInterface
*/
public function applyFilter(BinaryInterface $binary, $filter, array $runtimeConfig = [])
{
$config = array_replace_recursive(
$this->getFilterConfiguration()->get($filter),
$runtimeConfig
);

return $this->apply($binary, $config);
}

/**
* @param BinaryInterface $binary
* @param array $config
*
* @throws \InvalidArgumentException
*
* @return BinaryInterface
*/
public function applyPostProcessors(BinaryInterface $binary, array $config): BinaryInterface
{
foreach ($this->sanitizePostProcessors($config['post_processors'] ?? []) as $name => $options) {
$binary = $this->postProcessors[$name]->process($binary, $options);
}

return $binary;
}

/**
* @param BinaryInterface $binary
* @param ImageInterface $image
* @param array $config
*
* @return BinaryInterface
*/
private function exportConfiguredImageBinary(BinaryInterface $binary, ImageInterface $image, array $config): BinaryInterface
{
$options = [
'quality' => $config['quality'],
];
Expand All @@ -149,64 +198,68 @@ public function apply(BinaryInterface $binary, array $config)
$options['animated'] = $config['animated'];
}

$filteredFormat = isset($config['format']) ? $config['format'] : $binary->getFormat();
$filteredContent = $image->get($filteredFormat, $options);
$filteredMimeType = $filteredFormat === $binary->getFormat() ? $binary->getMimeType() : $this->mimeTypeGuesser->guess($filteredContent);
$filteredFormat = $config['format'] ?? $binary->getFormat();
$filteredString = $image->get($filteredFormat, $options);

// We are done with the image object so we can destruct the this because imagick keeps consuming memory if we don't
// See https://github.com/liip/LiipImagineBundle/pull/682
if (method_exists($image, '__destruct')) {
$image->__destruct();
}
$this->destroyImage($image);

return $this->applyPostProcessors(new Binary($filteredContent, $filteredMimeType, $filteredFormat), $config);
return new Binary(
$filteredString,
$filteredFormat === $binary->getFormat() ? $binary->getMimeType() : $this->mimeTypeGuesser->guess($filteredString),
$filteredFormat
);
}

/**
* @param BinaryInterface $binary
* @param array $config
*
* @throws \InvalidArgumentException
* @param array $filters
*
* @return BinaryInterface
* @return array
*/
public function applyPostProcessors(BinaryInterface $binary, $config)
private function sanitizeFilters(array $filters): array
{
$config += ['post_processors' => []];
foreach ($config['post_processors'] as $postProcessorName => $postProcessorOptions) {
if (!isset($this->postProcessors[$postProcessorName])) {
throw new \InvalidArgumentException(sprintf(
'Could not find post processor "%s"', $postProcessorName
));
}
if ($this->postProcessors[$postProcessorName] instanceof ConfigurablePostProcessorInterface) {
$binary = $this->postProcessors[$postProcessorName]->processWithConfiguration($binary, $postProcessorOptions);
} else {
$binary = $this->postProcessors[$postProcessorName]->process($binary);
}
$sanitized = array_filter($filters, function (string $name): bool {
return isset($this->loaders[$name]);
}, ARRAY_FILTER_USE_KEY);

if (count($filters) !== count($sanitized)) {
throw new \InvalidArgumentException(sprintf('Could not find filter(s): %s', implode(', ', array_map(function (string $name): string {
return sprintf('"%s"', $name);
}, array_diff(array_keys($filters), array_keys($sanitized))))));
}

return $binary;
return $sanitized;
}

/**
* Apply the provided filter set on the given binary.
*
* @param BinaryInterface $binary
* @param string $filter
* @param array $runtimeConfig
* @param array $processors
*
* @throws \InvalidArgumentException
*
* @return BinaryInterface
* @return array
*/
public function applyFilter(BinaryInterface $binary, $filter, array $runtimeConfig = [])
private function sanitizePostProcessors(array $processors): array
{
$config = array_replace_recursive(
$this->getFilterConfiguration()->get($filter),
$runtimeConfig
);
$sanitized = array_filter($processors, function (string $name): bool {
return isset($this->postProcessors[$name]);
}, ARRAY_FILTER_USE_KEY);

return $this->apply($binary, $config);
if (count($processors) !== count($sanitized)) {
throw new \InvalidArgumentException(sprintf('Could not find post processor(s): %s', implode(', ', array_map(function (string $name): string {
return sprintf('"%s"', $name);
}, array_diff(array_keys($processors), array_keys($sanitized))))));
}

return $sanitized;
}

/**
* We are done with the image object so we can destruct the this because imagick keeps consuming memory if we don't.
* See https://github.com/liip/LiipImagineBundle/pull/682
*
* @param ImageInterface $image
*/
private function destroyImage(ImageInterface $image): void
{
if (method_exists($image, '__destruct')) {
$image->__destruct();
}
}
}
34 changes: 0 additions & 34 deletions Imagine/Filter/PostProcessor/ConfigurablePostProcessorInterface.php
View file

This file was deleted.

20 changes: 5 additions & 15 deletions Imagine/Filter/PostProcessor/JpegOptimPostProcessor.php
View file
Expand Up @@ -17,9 +17,11 @@
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Process;

class JpegOptimPostProcessor implements PostProcessorInterface, ConfigurablePostProcessorInterface
class JpegOptimPostProcessor implements PostProcessorInterface
{
/** @var string Path to jpegoptim binary */
/**
* @var string Path to jpegoptim binary
*/
protected $jpegoptimBin;

/**
Expand Down Expand Up @@ -109,18 +111,6 @@ public function setStripAll($stripAll)
return $this;
}

/**
* @param BinaryInterface $binary
*
* @throws ProcessFailedException
*
* @return BinaryInterface
*/
public function process(BinaryInterface $binary)
{
return $this->processWithConfiguration($binary, []);
}

/**
* @param BinaryInterface $binary
* @param array $options
Expand All @@ -129,7 +119,7 @@ public function process(BinaryInterface $binary)
*
* @return BinaryInterface
*/
public function processWithConfiguration(BinaryInterface $binary, array $options)
public function process(BinaryInterface $binary, array $options = []): BinaryInterface
{
$type = mb_strtolower($binary->getMimeType());
if (!in_array($type, ['image/jpeg', 'image/jpg'], true)) {
Expand Down
24 changes: 8 additions & 16 deletions Imagine/Filter/PostProcessor/MozJpegPostProcessor.php
View file
Expand Up @@ -24,12 +24,16 @@
*
* @author Alex Wilson <a@ax.gy>
*/
class MozJpegPostProcessor implements PostProcessorInterface, ConfigurablePostProcessorInterface
class MozJpegPostProcessor implements PostProcessorInterface
{
/** @var string Path to the mozjpeg cjpeg binary */
/**
* @var string Path to the mozjpeg cjpeg binary
*/
protected $mozjpegBin;

/** @var null|int Quality factor */
/**
* @var null|int Quality factor
*/
protected $quality;

/**
Expand Down Expand Up @@ -58,18 +62,6 @@ public function setQuality($quality)
return $this;
}

/**
* @param BinaryInterface $binary
*
* @throws ProcessFailedException
*
* @return BinaryInterface
*/
public function process(BinaryInterface $binary)
{
return $this->processWithConfiguration($binary, []);
}

/**
* @param BinaryInterface $binary
* @param array $options
Expand All @@ -78,7 +70,7 @@ public function process(BinaryInterface $binary)
*
* @return BinaryInterface
*/
public function processWithConfiguration(BinaryInterface $binary, array $options)
public function process(BinaryInterface $binary, array $options = []): BinaryInterface
{
$type = mb_strtolower($binary->getMimeType());
if (!in_array($type, ['image/jpeg', 'image/jpg'], true)) {
Expand Down

0 comments on commit 988d235

Please sign in to comment.