[MAGNOLIA-6203] Inject the Context to ContextLocaleProvider Created: 08/May/15 Updated: 21/Apr/16 Resolved: 11/May/15 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.4 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Espen Jervidalo | Assignee: | Magnolia International |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||
| Description |
|
The current implementation of ContextLocaleProvider retrieves the Locale by calling the static MgnlContext.getLocale() method. This can cause issues when e.g translating notifications sent from a thread without MgnlContext set, e.g. triggered by JCR-Observation. (currently fails with IllegalStateException in MgnlContext.getInstance) Anything that gets a SimpleTranslator through injection (i.e which injects a LocaleProvider indirectly) will currently be using the Locale of whatever context is bound to thread where it tries to do the translation. If we injected the Context in ContextLocaleProvider instead, it'd be using the Locale corresponding to the context/thread where it was instantiated. |
| Comments |
| Comment by Magnolia International [ 11/May/15 ] |
|
Done on fix/ |
| Comment by Mikaël Geljić [ 11/May/15 ] |
|
Not too fond about making the LocaleProvider an abstract class, but whatever, really. |
| Comment by Magnolia International [ 11/May/15 ] |
|
Agreed, but noise created by abstract class, without its lack of useful made me go the other way (i.e all tests used a concrete impl, which was a copy of what's now been added as FixedLocaleProvider) |