Allow to configure the imagine class

[Toflar]

Right now you cannot configure the desired Imagine class. This has two major drawbacks:

1. If e.g. Imagick is installed on your system but it's broken somehow, you are lost. You cannot switch to Gd without adding another compiler pass.
2. If you add your own Imagine implementation you cannot use that without adding another compiler pass.

This PR allows you to configure the class very easily:

contao:
    image:
        imagine_service: 'contao.image.imagine.gd'

[ausi]

I’m not sure if this is the correct approach. How about keeping the existing compiler pass, removing the class from the services.yml in https://github.com/contao/core-bundle/blob/864fd03cd899864b5268be879ad1ffa91d983eaa/src/Resources/config/services.yml#L148 and checking for ->getClass() === null?

public function process(ContainerBuilder $container): void
{
    $definition = $container->getDefinition('contao.image.imagine');

    if (null === $definition->getClass()) {
        $definition->setClass($this->getImagineImplementation());
    }
}

This way you can simply overwrite the used class in the service definition in your installation.

What do you think?

[aschempp]

@ausi 's suggestion looks reasonable to me

[Toflar]

Nope, I disagree here. The compiler pass was wrong from day one that's why I changed it. The class you would like to use is a configuration value. It's exactly the right place to do so and it doesn't need any compiler pass at all. But yes, I can change the key to contao.image.imagine_class.

[ausi]

But what if the custom Imagine class you want to use needs a constructor parameter or other injections?

IMO the services definition ist the right place for replacing services in your application.

[aschempp]

But what if the custom Imagine class you want to use needs a constructor parameter or other injections?

That means you would need dependency injection and not just set the class as an argument. A completely new (maybe valid) case. Maybe we should generate a service on the fly and assign it as argument to that method?

[Toflar]

I have a better idea, give me a few minutes :)

[Toflar]

Okay, so now you can configure any service you like. This will allow to define your own service in the container (that has other arguments) and configure it here. If you don't configure anything at all, the best solution is chosen for you and aliased. For convenience, I've added gd, imagick and gmagick so it's easy to configure if you want to use one of these three, see the updated initial example.

[leofeyer]

Ok. Let's see what @ausi says and then merge this.

[ausi]

I still think that the services definition is the right place for replacing services in your application. We do that in Contao for example with the translator service. Every developer that is familiar with Symfony knows how to do that:

services:
    contao.image.imagine:
        class: Imagine\Gd\Imagine

To get knowledge of the configuration value you need to read the documentation first.

My suggestion from https://github.com/contao/core-bundle/pull/1615#issuecomment-404230609 would solve your issues too, right?

[Toflar]

I don't like that. I don't want to configure things in my services.yml. I already configure the imagine options in contao.image. So I would need to do things in 2 places.

[ausi]

But how do you configure which translator class should be used be the Symfony translator for example, or any other service?

[Toflar]

No idea, it's not part of my problem at the moment 😄 The way this PR implements the service you choose is the same as it's done in thousands of bundles. I don't see the issue here. There's always many ways to achieve the same thing but this one is the one I like best.

[aschempp]

That is not the same to me. Overriding the application is one thing, setting configuration is a different thing.

Manually defining your Imagine class is a configuration that makes sense, because people might prefer one over the other. Therefore, there should be a configuration so set it. It's like settings the JPEG quality, you could do that using a compiler pass and change the service argument, but that's just unnecessarily cumbersome. We want people to configure their Imagine service, if they know which one they can use on their server.

Same as with the Doctrine server version. You should configure it, but if you don't the system tries to detect the current version.

[leofeyer]

Thank you @Toflar.