[MAGNOLIA-6241] The messages are not replaced by new one when they are duplicated Created: 04/Jun/15 Updated: 15/Apr/16 Resolved: 23/Jul/15 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core, i18n |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major |
| Reporter: | Sang Ngo Huu | Assignee: | Trang Truong |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | 541backlog, i18n | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 2d 3h | ||
| Original Estimate: | 2.5h | ||
| 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)
|
||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||
| Date of First Response: | |||||||||
| Sprint: | Sprint 2 (Vietnam) | ||||||||
| Story Points: | 0 | ||||||||
| Description |
|
In info.magnolia.i18nsystem.DefaultMessageBundlesLoader,
log.warn("Found duplicate key [{}] at file [{}]. The new key will overwrite the existing one. Please set the log level of this class to DEBUG to find out more.", key, propertiesFile);
But actually, the logic in the code below, it keeps the old messages: final Properties properties = new Properties(); properties.load(inStream); Properties existingProperties = messages.get(locale); if (existingProperties != null) { checkForDuplicates(existingProperties, properties, propertiesFile); // this locale already exists in our bundle, add the new ones log.debug("Adding properties to already existing ones under {} locale", locale); properties.putAll(existingProperties); } messages.put(locale, properties); In the case user want to modify the core messages:
So, the logic should be changed: |
| Comments |
| Comment by Sang Ngo Huu [ 04/Jun/15 ] |
|
Hi @gjoseph, |
| Comment by Magnolia International [ 09/Jun/15 ] |
|
Hmm you're right; to fix, please add a test that first demonstrates the issue. But I think you'll realize that there is no order guarantee in how message files are loaded; I'm not sure where you got the idea that the custom module should extend another one to override its messages, but by looking at the code I don't see how that'd work (hopefully I'm wrong) - perhaps there's even a jira issue about this. In any case, we also need to emphasize this is not intended behavior; this feature ("checkForDuplicates") was mostly intended as a debugging help to avoid duplicate keys, which somewhat - what's your use-case for replacing existing messages ? |
| Comment by Sang Ngo Huu [ 16/Jun/15 ] |
|
Sometimes, user want to customize the messages in depended modules for his purpose. The order of loading module, you can configure it in Module Descriptor file: Which module will be loaded before your module load. It is not high priority but i think it should be fixed. |
| Comment by Trang Truong [ 24/Jun/15 ] |
|
It's fixed to allow new keys can overwrite the existing ones. |
| Comment by Roman Kovařík [ 07/Jul/15 ] |
|
Reopened: |
| Comment by Trang Truong [ 08/Jul/15 ] |
|
rkovarik yes, I agree that I don't guarantee this fix will solve the order of loading message i18n. The fix is adjust the order of putting message into a map existingProperties.putAll(properties); which contains messages by locale as keys. As discussed in scrum, we move this to 5.4.x to low priority and will get back if having a good solution to support this. |
| Comment by Trang Truong [ 23/Jul/15 ] |
|
This isn't supported to replace new messages with old existing ones. |