Skip to content

[RFC] Factor loading and caching out of Translator #14622

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
wants to merge 4 commits into from
Closed

[RFC] Factor loading and caching out of Translator #14622

wants to merge 4 commits into from

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented May 12, 2015

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? not yet added to the code, but will follow
Tests pass? yes
Fixed tickets 14622
License MIT
Doc PR none

See #14530 for a discussion what this PR tries to achieve.

This is WIP, so for the time being please comment on the general approach and not on CS glitches.

Todo:

  • Move fallback locales from the Translator into the new DefaultProvider? That way the Providers would resemble (or could even implement) TranslatorBagInterface.
  • Discuss/finalize namings
  • Figure out how to deprecate protected methods appropriately
  • Come up with a plan how the final result in 3.0 should look like.
  • Can I preserve git history for code moved around?
  • Refactor existing and possibly add new tests
  • Make sure assertValidLocale calls are put in a few but "strategically right" places to prevent fixed problems from re-appearing (also when possibly using new components in isolation).

@aitboudad
Copy link
Contributor

I'm not sure if we need MessageCatalogueProviderInterface here only TranslatorBagInterface is enough, I already started working on this https://gist.github.com/aitboudad/6eed3ff1187bd8930520#file-delegatingloader-php but couldn't find time to complete it :( maybe I provide an initial work this week-end.

@mpdude
Copy link
Contributor Author

mpdude commented May 13, 2015

@aitboudad Yes, I am not sure yet how we need to design those interfaces.

Let's see what we're aiming at:

The long-term goal is to get a better encapsulation of MessageCatalogues inside the Translator, so that the Translator could use (for example) "inlined" fallback messages.

There are, however, use cases where clients need to access the full set of messages. Currently they can get those from Translator through the TranslatorBagInterface, but that would not work if the Translator only has the "optimized" catalogues available (or it would need to juggle with the "full" and "optimized" catalogues at the same time).

So it would be helpful if the Translator could handoff getCatalogue() calls to some other (internal/helper) class or if clients could query that class directly. Also note that in order to accomplish its main task (see TranslatorInterface), all the Translator would need to have is some implementation of TranslatorBagInterface.

That would also simplify Translator a lot, because most of its code (and the intricate call graph) actually results from loading and caching message catalogues (both on disk and in-memory), even allowing subclasses to override some protected methods.

If the fallback locales were moved to the Provider part as well, the Provider could extend/implement the TranslatorBagInterface. And once we've got rid of the protected methods in Translator (loadCatalogue, initCatalogue, computeFallbackLocales) and remove the "configuration" things (addResource, setFallbackLocales, addLoader...), we'd pretty much be set with Translator using a TranslatorBag and performing the translating part only.

So why do I currently need another interface?

Inside Translator, there were already two distinct parts of code. initializeCacheCatalogue, dumpCatalogue and getFallbackCatalogue are needed for caching only. They will eventually call initializeCatalogue and its sub-methods doLoadCatalogue, loadFallbackCatalogue to do the actual loading. loadCatalogue would dispatch to either part, depending on caching being desired or not.

This naturally led to a decorator-based approach for the Providers. But the cache has at least to be aware of the resources used to correctly separate cached data. Thus, we need MessageCatalogueProviderInterface as a more powerful way of configuring and composing the load-and-cache related parts.

Does that make sense? In what direction should we proceed from here?

@aitboudad
Copy link
Contributor

@mpdude see #14671

@mpdude mpdude changed the title [WIP] Factor loading and caching out of Translator [RFC] Factor loading and caching out of Translator Oct 1, 2015
@nicolas-grekas nicolas-grekas added this to the 3.x milestone Dec 6, 2016
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 14, 2017

I think we don't need this one: optimized caching is already dealt with on PHP7, where static arrays and static strings are far superior optimization strategies. Thus, no need to do anything more on our side to me.

@nicolas-grekas
Copy link
Member

Closing for the reason given above (and lack of contra-arguments :) )

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

Successfully merging this pull request may close these issues.

4 participants