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

added ability to inject custom drivers #1105

Merged
merged 5 commits into from Jun 26, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions Binary/Locator/FileSystemInsecureLocator.php
Expand Up @@ -21,9 +21,9 @@ class FileSystemInsecureLocator extends FileSystemLocator
*/
protected function generateAbsolutePath(string $root, string $path): ?string
{
if (false === mb_strpos($path, '..'.DIRECTORY_SEPARATOR) &&
false === mb_strpos($path, DIRECTORY_SEPARATOR.'..') &&
false !== file_exists($absolute = $root.DIRECTORY_SEPARATOR.$path)
if (false === mb_strpos($path, '..'.\DIRECTORY_SEPARATOR) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

why the backslashes here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is global constant and php-cs doesn't like it without backslash

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm maybe better to disable the php-cs rule? I have never added blackslashes to such global constants and don't really see the point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was fixed in 1107.
will remove backslashes.

false === mb_strpos($path, \DIRECTORY_SEPARATOR.'..') &&
false !== file_exists($absolute = $root.\DIRECTORY_SEPARATOR.$path)
) {
return $absolute;
}
Expand Down
2 changes: 1 addition & 1 deletion Binary/Locator/FileSystemLocator.php
Expand Up @@ -60,7 +60,7 @@ public function locate(string $path): string
*/
protected function generateAbsolutePath(string $root, string $path): ?string
{
if (false !== $absolute = realpath($root.DIRECTORY_SEPARATOR.$path)) {
if (false !== $absolute = realpath($root.\DIRECTORY_SEPARATOR.$path)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why the backslashes here?

return $absolute;
}

Expand Down
37 changes: 37 additions & 0 deletions DependencyInjection/Compiler/DriverCompilerPass.php
@@ -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\DependencyInjection\Compiler;

use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* Give a helpful exception message in case the imagine driver does not exist.
*
* Third parties can provide a driver, and thus we can only validate after the container has been built.
*/
class DriverCompilerPass extends AbstractCompilerPass
{
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
{
$liipImagineDriver = $container->getParameter('liip_imagine.driver_service');

if (!$container->hasDefinition($liipImagineDriver)) {
throw new InvalidConfigurationException(sprintf(
"Specified driver '%s' is not defined.", $liipImagineDriver
));
}
}
}
9 changes: 1 addition & 8 deletions DependencyInjection/Configuration.php
Expand Up @@ -104,14 +104,7 @@ public function getConfigTreeBuilder()
$rootNode
->fixXmlConfig('filter_set', 'filter_sets')
->children()
->scalarNode('driver')->defaultValue('gd')
->validate()
->ifTrue(function ($v) {
return !in_array($v, ['gd', 'imagick', 'gmagick'], true);
})
->thenInvalid('Invalid imagine driver specified: %s')
->end()
->end()
->scalarNode('driver')->defaultValue('gd')->end()
->scalarNode('cache')->defaultValue('default')->end()
->scalarNode('cache_base_path')->defaultValue('')->end()
->scalarNode('data_loader')->defaultValue('default')->end()
Expand Down
Expand Up @@ -127,7 +127,7 @@ private function getBundleResourcePaths(ContainerBuilder $container)
}

return array_map(function ($path) {
return $path.DIRECTORY_SEPARATOR.'Resources'.DIRECTORY_SEPARATOR.'public';
return $path.\DIRECTORY_SEPARATOR.'Resources'.\DIRECTORY_SEPARATOR.'public';
Copy link
Contributor

Choose a reason for hiding this comment

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

why the backslashes here?

}, $paths);
}

Expand Down
5 changes: 1 addition & 4 deletions DependencyInjection/LiipImagineExtension.php
Expand Up @@ -18,7 +18,6 @@
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;

class LiipImagineExtension extends Extension
Expand Down Expand Up @@ -81,9 +80,7 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('enqueue.xml');
}

$container->getDefinition('liip_imagine.'.$config['driver'])->addMethodCall('setMetadataReader', [
new Reference('liip_imagine.meta_data.reader'),
]);
$container->setParameter('liip_imagine.driver_service', 'liip_imagine.'.$config['driver']);

$container->setAlias('liip_imagine', new Alias('liip_imagine.'.$config['driver']));
$container->setAlias(CacheManager::class, new Alias('liip_imagine.cache.manager', false));
Expand Down
2 changes: 2 additions & 0 deletions LiipImagineBundle.php
Expand Up @@ -13,6 +13,7 @@

use Enqueue\Bundle\DependencyInjection\Compiler\AddTopicMetaPass;
use Liip\ImagineBundle\Async\Topics;
use Liip\ImagineBundle\DependencyInjection\Compiler\DriverCompilerPass;
use Liip\ImagineBundle\DependencyInjection\Compiler\FiltersCompilerPass;
use Liip\ImagineBundle\DependencyInjection\Compiler\LoadersCompilerPass;
use Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass;
Expand All @@ -38,6 +39,7 @@ public function build(ContainerBuilder $container)
{
parent::build($container);

$container->addCompilerPass(new DriverCompilerPass());
$container->addCompilerPass(new LoadersCompilerPass());
$container->addCompilerPass(new FiltersCompilerPass());
$container->addCompilerPass(new PostProcessorsCompilerPass());
Expand Down
18 changes: 15 additions & 3 deletions Resources/config/imagine.xml
Expand Up @@ -83,11 +83,23 @@

<service id="liip_imagine" alias="liip_imagine.gd" />

<service id="liip_imagine.gd" class="Imagine\Gd\Imagine" public="false" />
<service id="liip_imagine.gd" class="Imagine\Gd\Imagine" public="false">
<call method="setMetadataReader">
<argument type="service" id="liip_imagine.meta_data.reader" />
</call>
</service>

<service id="liip_imagine.imagick" class="Imagine\Imagick\Imagine" public="false" />
<service id="liip_imagine.imagick" class="Imagine\Imagick\Imagine" public="false">
<call method="setMetadataReader">
<argument type="service" id="liip_imagine.meta_data.reader" />
</call>
</service>

<service id="liip_imagine.gmagick" class="Imagine\Gmagick\Imagine" public="false" />
<service id="liip_imagine.gmagick" class="Imagine\Gmagick\Imagine" public="false">
<call method="setMetadataReader">
<argument type="service" id="liip_imagine.meta_data.reader" />
</call>
</service>

<!-- Templating helpers and extensions -->

Expand Down
4 changes: 2 additions & 2 deletions Tests/AbstractTest.php
Expand Up @@ -50,8 +50,8 @@ abstract class AbstractTest extends TestCase

protected function setUp()
{
$this->fixturesPath = realpath(__DIR__.DIRECTORY_SEPARATOR.'Fixtures');
$this->temporaryPath = sys_get_temp_dir().DIRECTORY_SEPARATOR.'liip_imagine_test';
$this->fixturesPath = realpath(__DIR__.\DIRECTORY_SEPARATOR.'Fixtures');
Copy link
Contributor

Choose a reason for hiding this comment

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

why the backslashes here?

$this->temporaryPath = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'liip_imagine_test';
$this->filesystem = new Filesystem();

if ($this->filesystem->exists($this->temporaryPath)) {
Expand Down
8 changes: 4 additions & 4 deletions Tests/LiipImagineBundleTest.php
Expand Up @@ -46,7 +46,7 @@ public function testAddLoadersCompilerPassOnBuild()
->with('liip_imagine')
->will($this->returnValue($this->createLiipImagineExtensionMock()));
$containerMock
->expects($this->at(0))
->expects($this->at(1))
->method('addCompilerPass')
->with($this->isInstanceOf(LoadersCompilerPass::class));

Expand All @@ -63,7 +63,7 @@ public function testAddFiltersCompilerPassOnBuild()
->with('liip_imagine')
->will($this->returnValue($this->createLiipImagineExtensionMock()));
$containerMock
->expects($this->at(1))
->expects($this->at(2))
->method('addCompilerPass')
->with($this->isInstanceOf(FiltersCompilerPass::class));

Expand All @@ -80,7 +80,7 @@ public function testAddPostProcessorsCompilerPassOnBuild()
->with('liip_imagine')
->will($this->returnValue($this->createLiipImagineExtensionMock()));
$containerMock
->expects($this->at(2))
->expects($this->at(3))
->method('addCompilerPass')
->with($this->isInstanceOf(PostProcessorsCompilerPass::class));

Expand All @@ -97,7 +97,7 @@ public function testAddResolversCompilerPassOnBuild()
->with('liip_imagine')
->will($this->returnValue($this->createLiipImagineExtensionMock()));
$containerMock
->expects($this->at(3))
->expects($this->at(4))
->method('addCompilerPass')
->with($this->isInstanceOf(ResolversCompilerPass::class));

Expand Down