Skip to content

Allow Translator to use CacheInterface instead of ConfigCacheInterface #45991

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

Closed
freiondrej-lmc opened this issue Apr 11, 2022 · 14 comments
Closed

Comments

@freiondrej-lmc
Copy link
Contributor

Description

Hello,
we would like to allow translation catalogues to update even without a new deploy of the app. To achieve that, we'd write a custom DatabaseLoader (or use some bundle for that). However, currently the Translator uses ConfigCache, which is always file-based - we would like to use e.g. a RedisAdapter cache for this, so that the cache could be invalidated and rebuilt centrally for multiple containers.

Example

No response

@freiondrej-lmc
Copy link
Contributor Author

I found an existing thread, where Nicolas says "This is especially true when considering the current way: dumping catalogs to the filesystem means leveraging opcache.". Does that mean this is a bad idea altogether and we should always execute a new deploy when translations change?

@nicolas-grekas
Copy link
Member

I think so yes: translations should be superfast to load, and nothing is faster than opcache for this. You might be able to dump translations without doing a full redeploy. It depends on your hosting environement (read-only fs, etc.)

@freiondrej-lmc
Copy link
Contributor Author

freiondrej-lmc commented Apr 11, 2022

So does this mean this is not the way Symfony wants to go and such feature is not likely to be merged, since it is considered risky from a performance standpoint? I'm asking so that I could close this issue in that case.

You might be able to dump translations without doing a full redeploy

by "dump translations", you mean the ConfigCache files, if I understand correctly?

Thanks :)

@nicolas-grekas
Copy link
Member

You might be able to dump translations without doing a full redeploy

I might have been too quick on this one, I don't know the process well enough 😬
The doc says cache:clear is needed...

Maybe @welcoMattic has some input here?

@wouterj
Copy link
Member

wouterj commented Apr 11, 2022

See also symfony/symfony-docs#5136 (comment) . It's sadly an undocumented feature, but this use-case seems to be already possible

@freiondrej-lmc
Copy link
Contributor Author

Thanks @wouterj, that seems to describe the very use case we have! Do I understand correctly that the cookbook has not been written yet? Which means I should think about ConfigCacheFactoryInterface in a more file-less kind of way and try to achieve what the comment describes, but that there are no code samples to start with? :)

@wouterj
Copy link
Member

wouterj commented Apr 11, 2022

Yes, the feature is quite advanced and we never found time to test this in a demo app to write an article. So you'll need to do some digging on your own.

  • The feature was introduced in this PR: [Config] Delegate creation of ConfigCache instances to a factory. #14178 While it doesn't show you code examples, it does show you the discussion and changes, which can help finding the correct files to look at.
  • From a quick look, I think you have to create a class/service implementing ConfigCacheFactoryInterface. Then, you'll need to modify the translator service definition (in a compiler pass) to add a setConfigCacheFactory() method call.

If you manage to get it working, it would be cool if you can share some (basic) code examples as a first start with the documentation :)

@freiondrej-lmc
Copy link
Contributor Author

@wouterj thank you so much for the information, this will really help me start. I'll try to provide code examples if I manage to get it working. When thinking about it, I think I could actually wrap the Cache component (to leverage RedisAdapter etc) into the ConfigCacheFactoryInterface to combine the two worlds.

Do you think it makes sense to keep this issue open? Or should it be changed to a docs-altering one?

@wouterj
Copy link
Member

wouterj commented Apr 11, 2022

Let's close this one for the moment, as it seems like the feature request is already possible.

If you discover things don't work as expected, feel free to leave a comment here. We can always reopen it :)

@wouterj wouterj closed this as completed Apr 11, 2022
@freiondrej-lmc
Copy link
Contributor Author

freiondrej-lmc commented May 19, 2022

Just a note for anyone trying to solve the same thing:

If you stumble upon the problem that the Translator wants to include a real file (https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Translation/Translator.php#L292), but the file does not exist (because the file path is just a key in your cache storage, not a real file), the solution is a few lines above: https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Translation/Translator.php#L286 . I implemented it so that the custom ConfigCacheFactory dispatches an event which I called ConfigCacheReadyEvent just before returning and there is a ConfigCacheReadySubscriber, which evals the cache content (I did not find any other way than eval). The eval result then is a MessageCatalogue instance, which I pass to the Translator using a rather nasty Closure::bind like this:

\Closure::bind(
            static function (TranslatorInterface $translator, MessageCatalogue $catalogue) {
                if ($translator instanceof DataCollectorTranslator) {
                    $translator = \Closure::bind(
                        static function (DataCollectorTranslator $translator) {
                            return $translator->translator;
                        },
                        null,
                        DataCollectorTranslator::class
                    )($translator);
                }

                $translator->catalogues[$catalogue->getLocale()] = $catalogue;
            },
            null,
            Translator::class
        )($this->translator, $evalResult);

That way, when Translator reaches the isset, the catalogue is already known and it does not proceed to the include call. Hope this helps someone :)

EDIT: the event and the subscriber are used in an attempt to maintain somehow clean code. But of course the whole eval thing could be done directly in the custom ConfigCacheFactory.

@faizanakram99
Copy link
Contributor

faizanakram99 commented Jun 30, 2022

@wouterj

If it helps, we are using custom ConfigCacheFactory in production for two years now, the use case was to use tenant specific translation cache files (we allow our tenants to override default translations)

Here is the gist https://gist.github.com/faizanakram99/9e90c794c5c3708d597d05daaab9f3ea

@mpdude
Copy link
Contributor

mpdude commented Jun 30, 2022

Subscribing since I did the initial ConfigCacheFactory implementation.

Don’t know if I can be of any help atm and about to go on vacation for three weeks, but we’re using ConfigCacheFactory to change cached files at runtime, both for translations and routes.

Basically the idea is to change cache file names/suffixes when new data needs to be loaded. Works also when Opcahce is configured not to check timestamps.

@mpdude
Copy link
Contributor

mpdude commented Jun 30, 2022

Oh, and we have open-sourced https://github.com/webfactory/WebfactoryWfdMetaBundle.

It’s poorly documented and nothing you’d want to use directly, but maybe there are some ideas in the code that might help you.

@faizanakram99
Copy link
Contributor

faizanakram99 commented Jul 1, 2022

@freiondrej-lmc

I just found it today if you add resource to MessageCatalogue (where resource is the dummy translation file used to invoke database translation loader), translations are re-loaded from its loader (i.e. database) whenever the file is modified (a simple touch(file) whenever new or existing translations are modified will do the trick. No need to fiddle with ConfigCache.

From our db translation loader (which extends ArrayLoader)

public function load($resource, $locale, $domain = 'messages'): MessageCatalogue
{
    try {
        $translationsResource = $this->getTranslationRepository()->findTranslationsByLocaleAndDomain($locale, $domain);
    } catch (DBALException) {
        $translationsResource = [];
    }

    $catalogue = parent::load($translationsResource, $locale, $domain);

    // This does the trick (of notifying symfony to look for freshness of cache based on timestamp of resource file)
    $catalogue->addResource(new FileResource($resource));

    return $catalogue;
}

EDIT: Symfony\Component\Config\Resource\SelfCheckingResourceChecker is added to "resource checkers" of ConfigCache only in debug mode, hence the example given above will not invalidate cache automatically in production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants