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

[DI] [Controller] Add controller redirect response code configuration option #1220

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 37 additions & 0 deletions Config/Controller/ControllerConfig.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Config\Controller;

use Liip\ImagineBundle\Exception\InvalidArgumentException;

final class ControllerConfig
{
public const REDIRECT_RESPONSE_CODES = [201, 301, 302, 303, 307, 308];

private $redirectResponseCode;

public function __construct(int $redirectResponseCode)
{
if (!in_array($redirectResponseCode, self::REDIRECT_RESPONSE_CODES, true)) {
throw new InvalidArgumentException(sprintf(
'Invalid redirect response code "%s" (must be 201, 301, 302, 303, 307, or 308).', $redirectResponseCode
));
}

$this->redirectResponseCode = $redirectResponseCode;
}

public function getRedirectResponseCode(): int
{
return $this->redirectResponseCode;
}
}
61 changes: 39 additions & 22 deletions Controller/ImagineController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Liip\ImagineBundle\Controller;

use Imagine\Exception\RuntimeException;
use Liip\ImagineBundle\Config\Controller\ControllerConfig;
use Liip\ImagineBundle\Exception\Binary\Loader\NotLoadableException;
use Liip\ImagineBundle\Exception\Imagine\Filter\NonExistingFilterException;
use Liip\ImagineBundle\Imagine\Cache\Helper\PathHelper;
Expand Down Expand Up @@ -41,15 +42,29 @@ class ImagineController
private $signer;

/**
* @param FilterService $filterService
* @param DataManager $dataManager
* @param SignerInterface $signer
* @var ControllerConfig
*/
public function __construct(FilterService $filterService, DataManager $dataManager, SignerInterface $signer)
private $controllerConfig;

/**
* @param FilterService $filterService
* @param DataManager $dataManager
* @param SignerInterface $signer
* @param ControllerConfig|null $controllerConfig
*/
public function __construct(FilterService $filterService, DataManager $dataManager, SignerInterface $signer, ?ControllerConfig $controllerConfig = null)
{
$this->filterService = $filterService;
$this->dataManager = $dataManager;
$this->signer = $signer;

if (null === $controllerConfig) {
@trigger_error(sprintf(
'Instantiating "%s" without a forth argument of type "%s" is deprecated since 2.2.0 and will be required in 3.0.', self::class, ControllerConfig::class
), E_USER_DEPRECATED);
}

$this->controllerConfig = $controllerConfig ?? new ControllerConfig(301);
}

/**
Expand All @@ -73,19 +88,9 @@ public function filterAction(Request $request, $path, $filter)
$path = PathHelper::urlPathToFilePath($path);
$resolver = $request->get('resolver');

try {
return new RedirectResponse($this->filterService->getUrlOfFilteredImage($path, $filter, $resolver), 301);
} catch (NotLoadableException $e) {
if (null !== $this->dataManager->getDefaultImageUrl($filter)) {
return new RedirectResponse($this->dataManager->getDefaultImageUrl($filter));
}

throw new NotFoundHttpException(sprintf('Source image for path "%s" could not be found', $path));
} catch (NonExistingFilterException $e) {
throw new NotFoundHttpException(sprintf('Requested non-existing filter "%s"', $filter));
} catch (RuntimeException $e) {
throw new \RuntimeException(sprintf('Unable to create image for path "%s" and filter "%s". Message was "%s"', $path, $filter, $e->getMessage()), 0, $e);
}
return $this->createRedirectResponse(function () use ($path, $filter, $resolver) {
return $this->filterService->getUrlOfFilteredImage($path, $filter, $resolver);
}, $path, $filter);
}

/**
Expand All @@ -109,6 +114,7 @@ public function filterAction(Request $request, $path, $filter)
public function filterRuntimeAction(Request $request, $hash, $path, $filter)
{
$resolver = $request->get('resolver');
$path = PathHelper::urlPathToFilePath($path);
$runtimeConfig = $request->query->get('filters', []);

if (!\is_array($runtimeConfig)) {
Expand All @@ -124,18 +130,29 @@ public function filterRuntimeAction(Request $request, $hash, $path, $filter)
));
}

return $this->createRedirectResponse(function () use ($path, $filter, $runtimeConfig, $resolver) {
return $this->filterService->getUrlOfFilteredImageWithRuntimeFilters($path, $filter, $runtimeConfig, $resolver);
}, $path, $filter, $hash);
}

private function createRedirectResponse(\Closure $url, string $path, string $filter, ?string $hash = null): RedirectResponse
{
try {
return new RedirectResponse($this->filterService->getUrlOfFilteredImageWithRuntimeFilters($path, $filter, $runtimeConfig, $resolver), 301);
} catch (NotLoadableException $e) {
return new RedirectResponse($url(), $this->controllerConfig->getRedirectResponseCode());
} catch (NotLoadableException $exception) {
if (null !== $this->dataManager->getDefaultImageUrl($filter)) {
return new RedirectResponse($this->dataManager->getDefaultImageUrl($filter));
}

throw new NotFoundHttpException(sprintf('Source image for path "%s" could not be found', $path));
} catch (NonExistingFilterException $e) {
} catch (NonExistingFilterException $exception) {
throw new NotFoundHttpException(sprintf('Requested non-existing filter "%s"', $filter));
} catch (RuntimeException $e) {
throw new \RuntimeException(sprintf('Unable to create image for path "%s" and filter "%s". Message was "%s"', $hash.'/'.$path, $filter, $e->getMessage()), 0, $e);
} catch (RuntimeException $exception) {
throw new \RuntimeException(vsprintf('Unable to create image for path "%s" and filter "%s". Message was "%s"', [
$hash ? sprintf('%s/%s', $hash, $path) : $path,
$filter,
$exception->getMessage(),
]), 0, $exception);
}
}
}
9 changes: 9 additions & 0 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Liip\ImagineBundle\DependencyInjection;

use Liip\ImagineBundle\Config\Controller\ControllerConfig;
use Liip\ImagineBundle\Controller\ImagineController;
use Liip\ImagineBundle\DependencyInjection\Factory\Loader\LoaderFactoryInterface;
use Liip\ImagineBundle\DependencyInjection\Factory\Resolver\ResolverFactoryInterface;
Expand Down Expand Up @@ -123,6 +124,14 @@ public function getConfigTreeBuilder()
->children()
->scalarNode('filter_action')->defaultValue(sprintf('%s::filterAction', ImagineController::class))->end()
->scalarNode('filter_runtime_action')->defaultValue(sprintf('%s::filterRuntimeAction', ImagineController::class))->end()
->integerNode('redirect_response_code')->defaultValue(301)
->validate()
->ifTrue(function ($redirectResponseCode) {
return !\in_array($redirectResponseCode, ControllerConfig::REDIRECT_RESPONSE_CODES, true);
})
->thenInvalid('Invalid redirect response code "%s" (must be 201, 301, 302, 303, 307, or 308).')
->end()
->end()
->end()
->end()
->arrayNode('filter_sets')
Expand Down
4 changes: 4 additions & 0 deletions DependencyInjection/LiipImagineExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ public function load(array $configs, ContainerBuilder $container)

$container->setParameter('liip_imagine.driver_service', 'liip_imagine.'.$config['driver']);

$container
->getDefinition('liip_imagine.controller.config')
->replaceArgument(0, $config['controller']['redirect_response_code']);

$container->setAlias('liip_imagine', new Alias('liip_imagine.'.$config['driver']));
$container->setAlias(CacheManager::class, new Alias('liip_imagine.cache.manager', false));
$container->setAlias(DataManager::class, new Alias('liip_imagine.data.manager', false));
Expand Down
7 changes: 7 additions & 0 deletions Resources/config/imagine.xml
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,19 @@
<argument type="service" id="logger" on-invalid="ignore" />
</service>

<!-- Config -->

<service id="liip_imagine.controller.config" class="Liip\ImagineBundle\Config\Controller\ControllerConfig" public="false">
<argument><!-- will be injected by LiipImagineExtension --></argument>
</service>

<!-- Controller -->

<service id="Liip\ImagineBundle\Controller\ImagineController" public="true">
<argument type="service" id="liip_imagine.service.filter" />
<argument type="service" id="liip_imagine.data.manager" />
<argument type="service" id="liip_imagine.cache.signer" />
<argument type="service" id="liip_imagine.controller.config" />
</service>

<service id="liip_imagine.controller" alias="Liip\ImagineBundle\Controller\ImagineController" public="true" />
Expand Down
7 changes: 5 additions & 2 deletions Resources/doc/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ The default configuration for the bundle looks like this:
data_loader: default
default_image: null
controller:
filter_action: liip_imagine.controller:filterAction
filter_runtime_action: liip_imagine.controller:filterRuntimeAction
filter_action: liip_imagine.controller:filterAction
filter_runtime_action: liip_imagine.controller:filterRuntimeAction
redirect_response_code: 301
filter_sets:

# Prototype
Expand Down Expand Up @@ -62,6 +63,8 @@ There are several configuration options available:
Default value: ``liip_imagine.controller:filterAction``
* ``filter_runtime_action`` - name of the controller action to use in the route
loader for runtimeconfig images. Default value: ``liip_imagine.controller:filterRuntimeAction``
* ``redirect_response_code`` - The HTTP redirect response code to return from the imagine controller,
one of ``201``, ``301``, ``302``, ``303``, ``307``, or ``308``. Default value: ``301``
* ``driver`` - one of the three drivers: ``gd``, ``imagick``, ``gmagick``.
Default value: ``gd``
* ``filter_sets`` - specify the filter sets that you want to define and use.
Expand Down
6 changes: 6 additions & 0 deletions Tests/AbstractTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Imagine\Image\Metadata\MetadataBag;
use Liip\ImagineBundle\Binary\Loader\LoaderInterface;
use Liip\ImagineBundle\Binary\MimeTypeGuesserInterface;
use Liip\ImagineBundle\Config\Controller\ControllerConfig;
use Liip\ImagineBundle\Imagine\Cache\CacheManager;
use Liip\ImagineBundle\Imagine\Cache\Resolver\ResolverInterface;
use Liip\ImagineBundle\Imagine\Cache\SignerInterface;
Expand Down Expand Up @@ -242,6 +243,11 @@ protected function createDataManagerMock()
return $this->createObjectMock(DataManager::class, [], false);
}

protected function createControllerConfigInstance(int $redirectResponseCode = null): ControllerConfig
{
return new ControllerConfig($redirectResponseCode ?? 301);
}

/**
* @param string $object
* @param string[] $methods
Expand Down
56 changes: 56 additions & 0 deletions Tests/Config/Controller/ControllerConfigTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Tests\Config\Controller;

use Liip\ImagineBundle\Config\Controller\ControllerConfig;
use Liip\ImagineBundle\Exception\InvalidArgumentException;
use Liip\ImagineBundle\Tests\AbstractTest;

/**
* @covers \Liip\ImagineBundle\Config\Controller\ControllerConfig
*/
class ControllerConfigTest extends AbstractTest
{
public static function provideRedirectResponseCodeData(): \Generator
{
foreach (ControllerConfig::REDIRECT_RESPONSE_CODES as $code) {
yield [$code];
}
}

/**
* @dataProvider provideRedirectResponseCodeData
*/
public function testRedirectResponseCode(int $redirectResponseCode): void
{
$this->assertSame($redirectResponseCode, (new ControllerConfig($redirectResponseCode))->getRedirectResponseCode());
}

public static function provideInvalidRedirectResponseCodeData(): \Generator
{
foreach ([200, 202, 304, 405, 406, 309, 310] as $code) {
yield [$code];
}
}

/**
* @dataProvider provideInvalidRedirectResponseCodeData
*/
public function testInvalidRedirectResponseCode(int $redirectResponseCode): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage(sprintf(
'Invalid redirect response code "%s" (must be 201, 301, 302, 303, 307, or 308).', $redirectResponseCode
));
$this->assertSame($redirectResponseCode, (new ControllerConfig($redirectResponseCode))->getRedirectResponseCode());
}
}
Loading