contao / core-bundle

[READ-ONLY] Contao Core Bundle
GNU Lesser General Public License v3.0
123 stars 58 forks source link

Implement TranslatorBagInterface in Translator class #1674

Closed koertho closed 5 years ago

koertho commented 6 years ago

The decorated translator service, introduced in Contao 4.5, doesn't implement the TranslatorBagInterface which the original Symfony service implements. Cause some extension rely on this service and the implementation (so we are, and we are working with Contao 4.4, but got some errors from 4.5 users), this should be fixed (also for 4.5).

See Translator class and TranslatorBagInterface

dmolineus commented 6 years ago

The service uses only the TranslatorInterface. You shouldn't rely on the actual implementation but on the interface.

https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml#L23

If you need to access the catalogue you maybe use the translator.default service.

leofeyer commented 5 years ago

@ausi What do you think?

ausi commented 5 years ago

Because we decorate the original Symfony service, I think we should probably add the bag interface and forward the method call.
Should be as simple as adding:

+ use Symfony\Component\Translation\TranslatorBagInterface;

- class Translator implements TranslatorInterface
+ class Translator implements TranslatorInterface, TranslatorBagInterface

and:

+ public function getCatalogue($locale = null)
+ {
+     return $this->translator->getCatalogue($locale);
+ }
dmolineus commented 5 years ago

I don't mind if the TranslatorBagInterface is also implemented. But keep in mind that any third party bundle could replace the standard translator with a translator which only implements the TranslatorInterface.

ausi commented 5 years ago

The correct way to use it in a third party bundle could be something like:

if ($transaltor instanceof TranslatorBagInterface) { … }
dmolineus commented 5 years ago

Yes, and Contao could provide two decorator one implementing the TranslatorBagInterface and one without depending on the actual service.

It's basically what I do in my backport for Contao 4.4:

$definition      = $container->findDefinition('translator');
$translatorClass = $container->getParameterBag()->resolveValue($definition->getClass());
$decoratorClass  = is_subclass_of($translatorClass, TranslatorBag::class)
        ? LangArrayTranslatorBagTranslator::class
        : LangArrayTranslator::class;

$definition = new Definition(
    $decoratorClass,
    [
        new Reference('netzmacht.contao_toolkit.translation.translator.inner'),
        new Reference('contao.framework')
    ]
);

$definition->setDecoratedService('translator');
aschempp commented 5 years ago

We should totally implement that interface like @ausi mentioned in https://github.com/contao/core-bundle/issues/1674#issuecomment-414838525. Everything else seems wrong to me

leofeyer commented 5 years ago

Implemented in contao/contao@28404321f37587231fb1da978d55d741f7b8a158.