[MAGNOLIA-3521] PropertyInitializer should be pluggable. Created: 20/Jan/11  Updated: 14/Mar/12  Resolved: 14/Mar/12

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 4.3.8, 4.4.1
Fix Version/s: None

Type: Improvement Priority: Neutral
Reporter: Danilo Ghirardelli Assignee: Unassigned
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
dependency
is depended upon by MAGNOLIA-3541 Inconsistency in properties naming (a... Closed
relation
is related to MAGNOLIA-3530 Deprecate SystemProperties and Proper... Closed
is related to MAGNOLIA-2644 Allow property initializer to use als... Closed
is related to MAGNOLIA-3094 Also use contextPath when resolving m... Closed
is related to MAGNOLIA-3516 Allow PropertyInitializer to also use... Closed
is related to MAGNOLIA-3530 Deprecate SystemProperties and Proper... Closed
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   

I don't know if it's un der refactoring right now, but just in case, I'm adding this.
Most of the patch I submitted on this topic would have been simply avoided if the PropertyInizializer class could be specified somehow. It would be nice to have the PropertyInitializer class set (optionally) in the web.xml just like the magnolia.initialization.file. Something like magnolia.property.initialization.class = my.package.MyOwnPropertyInitializer. So Magnolia will delegate to the given class the property init, maybe with a fallback to the default one (more or less the current one). This shuold cover also most of the corner cases in configuration without overriding the PropertyInitializer each time.
Maybe it could even be restored the simpler PropertyInitializer as the default one, leaving the contextpath/attribute/system extension to another class.



 Comments   
Comment by Magnolia International [ 26/Jan/11 ]

MAGNOLIA-3530 should cover most of this. The main difference with the current implementation is that the resolving of the path specified in web.xml is now the responsibility of info.magnolia.init.MagnoliaPropertiesResolver, while the properties themselves are now provided by info.magnolia.init.MagnoliaConfigurationProperties.

So far, there is no configuration mechanism for the replacement of such components, but we could think about it.
http://wiki.magnolia-cms.com/display/DEV/Concept+IOC+in+Magnolia#ConceptIOCinMagnolia-Properties and http://wiki.magnolia-cms.com/display/DEV/PicoContainer-specific+notes attempts to outline the current situation. (see "Containers" in particular)

Let me know what you think.

Comment by Danilo Ghirardelli [ 10/Feb/11 ]

Property initialization is always "special" in the various ioc framework, always developed with some "border line" implementation...

I don't know much about pico best practices about properties initialization, but my opinion is that this new implementation is a bit overkill but misses the central point to be pluggable.
Honestly I would have kept a single class to handle both the property file resolution and the properties loading. Or at least have one class delegating those two parts, to keep the context listener clean. This would cover another corner case, i.e. when the properties are stored in a database. It never happened to me to do such a thing but I don't think that's something uheard of. With your new implementation you may have to plug/override both classes (or worse, the entire hierarcy of those classes) just to ignore the file resolution.

Unfortunately pluggability for property resolver is a real and pressing problem (at least for me), and I honestly don't see the point of having the current hierarchy of classes if I couldn't choose which use or to plug an extension...

Given that the properties initialization is somewhat "external" for the container, I would have kept a simpler version that just uses a class declared in the context params (with fallback to the default initializer), without involving Pico. At least this is my opinion.

Comment by Jan Haderka [ 14/Mar/12 ]

Most of the methods of the class are gone as of Magnolia 4.5 and it will be removed completely.
DefaultMagnoliaPropertiesResolver is the class to use instead (which is indeed pluggable).

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