[MAGNOLIA-2621] Extract interface from MessagesManager Created: 13/Feb/09  Updated: 23/Jan/13  Resolved: 15/Feb/09

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 4.0
Fix Version/s: 4.0

Type: Bug Priority: Major
Reporter: Manuel Molaschi Assignee: Fabrizio Giustina
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File patchMessagesManager.txt    
Issue Links:
relation
is related to MAGNOLIA-2273 Make MessagesManager easier to extend... Closed
Template:
Patch included:
Yes
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:

 Description   

Hi.
We need to extends MessagesManager. I see (MAGNOLIA-2273) you've introduces the getInstance() by using FactoryUtils, but the problem is that the resulting object is casted to MessagesManager itself, which is final, no allowing subclassing.
I attach a patch in which i create a MessagesProvider interface, apply that to MessagesManager and use it for the MessagesManager.getInstance()

I think that MessagesManager could be improved by allowing, via configuration, to add Messages implementations to be (on startup or on configuration changes) chained in newMessages method.



 Comments   
Comment by manuel.molaschi [ 13/Feb/09 ]

Wait! I found an error
I attach the new patch

Comment by manuel.molaschi [ 13/Feb/09 ]

the new patch

Comment by manuel.molaschi [ 13/Feb/09 ]

Modified MessagesManager to make it abstract (all static methods, getInstance, and base abstract methods)
Added DefaultMessagesManager in which i move all implementation before in MessagesManager
Added in mgnl-beans DefaultMessagesManager as default implementation for MessagesManager

Comment by Fabrizio Giustina [ 15/Feb/09 ]

Patch committed for 4.0-rc4.

I assume this is ok for being committed in RCs: although the diff looks big it's just because of the creation of the base abstract class. All the signatures of public static methods in message manager are unchanged, and there is nothing changed also in the implementation of all the methods in the default message manager, just moved for easier subclassing.
This is required in order to make a translation module available for 4.0 and can be considered a leftover/fix for MAGNOLIA-2273 , declared fixed for 4.0.

Comment by Magnolia International [ 16/Feb/09 ]

I avoided doing this for MAGNOLIA-2273 for the simple reason that to provide a clean api for a MessagesManager/Provider, we'd have to split the static util methods out of MessagesManager and add a cleaner interface; issues linked to MAGNOLIA-2273 point to that direction ("cleanup").

I'm pretty sure simply removing the final keyword from the MessagesManager class would have been enough for your purpose, pending a more complete cleanup in the next release; moreover [tests are now broken|http://hudson.magnolia-cms.com/job/magnolia_main-trunk-forked-tests/info.magnolia$magnolia-core/105/testReport/6. I'd rather avoid having to deal with this in the future, but I'm hoping to quickly find the reason why these tests are now failing and fix it. If not, I'll have to revert this - and will of course remove the final from MessagesManager, that was an obvious mistake.

Generated at Mon Feb 12 03:38:29 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.