LiipImagineBundle icon indicating copy to clipboard operation
LiipImagineBundle copied to clipboard

Allow extra keys for certain configuration

Open bobvandevijver opened this issue 2 years ago • 1 comments

This makes it possible to configure the bundle using the Symfony way

Q A
Branch? 2.x
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc

Symfony generates a configuration class for bundle configuration, which can be used to configure the bundle with php and IDE autocomplete magic. For this bundle, that main class would be Symfony\Config\LiipImagineConfig, which is located in your cache directory.

However, currently it is not possible to configure the filter properties, as extra keys are not explicitly allowed. This MR aims to solve this.

Before this MR, the generated class for the filter configuration would look like:

<?php

namespace Symfony\Config\LiipImagine\DefaultFilterSetSettings;

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

/**
 * This class is automatically generated to help in creating a config.
 */
class FiltersConfig 
{
    public function __construct(array $value = [])
    {
        if ([] !== $value) {
            throw new InvalidConfigurationException(sprintf('The following keys are not supported by "%s": ', __CLASS__).implode(', ', array_keys($value)));
        }
    }

    public function toArray(): array
    {
        $output = [];

        return $output;
    }
}

After this change, it will look like:

<?php

namespace Symfony\Config\LiipImagine\FilterSetConfig;

use Symfony\Component\Config\Loader\ParamConfigurator;

/**
 * This class is automatically generated to help in creating a config.
 */
class FilterConfig
{
    private $_extraKeys;

    public function __construct(array $value = [])
    {
        $this->_extraKeys = $value;
    }

    public function toArray(): array
    {
        $output = [];

        return $output + $this->_extraKeys;
    }

    /**
     * @param ParamConfigurator|mixed $value
     * @return $this
     */
    public function set(string $key, $value): self
    {
        $this->_extraKeys[$key] = $value;

        return $this;
    }
}

This ultimately allows configuration like this:

<?php

use Symfony\Config\LiipImagineConfig;

return static function (LiipImagineConfig $liipImagine): void {
  $profileImageIcon = $liipImagine->filterSet('profile_image_icon')->quality(60);
  $profileImageIcon->filter('auto_rotate');
  $profileImageIcon->filter('fixed', ['width' => 25, 'height' => 25]);
  // or, depending on preference
  $profileImageIcon->filter('fixed')
    ->set('width', 25) 
    ->set('height', 25);
};

Without this change, you would get an error like:

The following keys are not supported by "Symfony\Config\LiipImagine\FilterSetConfig\FilterConfig": width, height

bobvandevijver avatar Sep 06 '22 10:09 bobvandevijver

Coverage Status

Coverage decreased (-2.09%) to 79.388% when pulling 283b37ffe544fa684d64e2292db3777fbf81b170 on bobvandevijver:patch-1 into 747966c4a8b74d20115afeb1e88c4399fff2f185 on liip:2.x.

coveralls avatar Sep 06 '22 10:09 coveralls

hi, thanks for this PR. can you please rebase on 2.x to get rid of the codestyle errors?

and can you please add an entry to the changelog, for the next minor version?

i am not too familiar with the extra keys thing - are we sure this will not be a BC break for existing users? i think it won't break anything existing, but could you confirm?

dbu avatar Sep 26 '22 14:09 dbu

@dbu Requested changes have been done!

About impact: it shouldn't impact existing users, or at least, not that I know of. I have tested this change with my project which has about 10 filters configured. The output of bin/console debug:config liip_imagine is identical with both versions.

Only the order of the keys becomes different when you start using the new php configuration that this PR enables, so when you use $profileImageIcon->filter('fixed', ['width' => 25, 'height' => 25]); instead of $container->extension('liip_imagine', ['filter_sets' => ['profile_image_icon' => ['filters' => ['fixed' => ['width' => 25, 'height' => 25]]]]]).

bobvandevijver avatar Sep 26 '22 15:09 bobvandevijver

great, thanks a lot for this improvement!

dbu avatar Sep 26 '22 15:09 dbu