[Translator] Revert #1965, which break cache warmup for Symfony applications

[Kocal]

Q	A
Bug fix?	yes
New feature?	no
Issues	Fix #2056
License	MIT

Hi everyone!

It looks like we were too fast on #1965, and our tests base didn't see the issue.

With 2.19.0, people started to have issues with the CacheWarmer from UX Translator, which was... simply never called, and so the folder var/translations was not generated anymore.

In private, we've decided to revert the feature to fix the issue, and to re-open the discussion if necessary.

[SanderVerkuil]

Yikes, this use case should've been found I recon. The interesting thing now is, that there are essentially two things (that appear to be conflicting). On the one hand, we have the issue mentioned in #2056 being caused by, what appears to be, people not having the translator service defined, and on the other hand #1962 where the translator is defined, but is being defined as the Identity translator, which causes the crash mentioned.

Wouldn't it be better to write a separate test-case that reproduces #2056 and fix that, while also still having the test passing for #1962? I'll see whether I can get a MR ready.

[smnandre]

It appears no one has the translator service anymore, as @MatTheCat reported (and we reproduced the same thing with @kbond & @Kocal )

So we need to revert asap (this could impact apps in production)

We also realize we need to improve testing in the CI as we did not saw this coming.

One solution we talked about would be to pass a simple TranslatorInterface to the cachewarmer and exit early if this is not a TranslatorBagInterface

[SanderVerkuil]

@smnandre I thought that might work as well, though the TranslatorBag does not extend/implement the TranslatorInterface. So when injecting the TranslatorInterface, it could be that it won't extend the TranslatorBag.

I'm currently trying to see whether I can remove the compiler pass, and make the TranslatorBag nullable/optional.

@smnandre But if the translator service is not available, how does the service('translator') method in the services.php find the correct dependency?

[smnandre]

But if the translator service is not available, how does the service('translator') method in the services.php find the correct dependency?

That's what @MatTheCat explained in #2056

The culprit seems to be #1965: our translator service is an alias of translation.default, so TranslatorCompilerPass::hasValidTranslator returns false because ContainerBuilder::hasDefinition does not check aliases.

--

So when injecting the TranslatorInterface, it could be that it won't extend the TranslatorBag.

That would be the point. Warmer would only dump files if given a TranslatorInterface&TranslatorBagInterface

[SanderVerkuil]

Ah alright, fair enought. I've created a separate MR that should fix it, by making the Translator nullable. See #2061, and please let me know whether this would be an appropriate fix.

[kbond]

Thanks Hugo.