[MAGNOLIA-3094] Also use contextPath when resolving magnolia.properties files at startup Created: 19/Feb/10  Updated: 25/Jul/13  Resolved: 23/Mar/12

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

Type: Improvement Priority: Major
Reporter: Magnolia International Assignee: Tobias Mattsson
Resolution: Fixed Votes: 0
Labels: todocument
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File contextPath_in_PropertiesInitializer.patch     Text File use-contextPath.patch    
Issue Links:
dependency
is depended upon by MAGNOLIA-5155 Remove redundancy of magnolia-test-pu... Closed
is depended upon by MGNLEE-294 Remove redundancy of magnolia-ee-test... Closed
relation
is related to MAGNOLIA-2644 Allow property initializer to use als... Closed
is related to MAGNOLIA-3303 Upgrade to Servlet API 2.5 Closed
is related to DOCU-264 Document new possibility of using con... Closed
is related to MAGNOLIA-3516 Allow PropertyInitializer to also use... Closed
is related to MAGNOLIA-3521 PropertyInitializer should be pluggable. 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   

When looking up magnolia.properties [1] files, we're using the current host name, as well as the webapp folder name (i.e the last element of the path in which the webapp is deployed) - which is not necessarily the servlet context path for the webapp.

Using the webapp's context path might help in a bunch of situations (while developing I often have a context path differing from the webapp's name. same goes when deploying multiple apps under different vhosts of a single tomcat, for instance; some appservers deploy their webapps in folder whose name doesn't correspond to the contextPath either)

Unfortunately, javax.servlet.ServletContext#getContextPath is only available since the 2.5 version of the Servlet API. Can we update ?

(marking this as fix-for 4.3, only in the case we can quickly decide on upgrading the servlet api dependency, in which case adding this should be trivial)

[1] http://documentation.magnolia-cms.com/cookbook/using-a-single-war-file-with-multiple-configurations.html



 Comments   
Comment by Fabrizio Giustina [ 19/Feb/10 ]

Unfortunately, javax.servlet.ServletContext#getContextPath is only available since the 2.5 version of the Servlet API. Can we update ?

well, I am pretty against upgrading only for this reason... is there anything can be really useful? Keep in mind that this would also mean Magnolia will not run anymore on tomcat 5.5, it will require tomcat 6.

Anyway, apart that during development any good production configuration usually have an empty context path, I don't consider this feature so useful...

Comment by Magnolia International [ 19/Feb/10 ]

Have a look at this patch - it's missing the tests (i have some locally), the log statement that logs

{contextPath}

next to the other variables, and the same change in the default web.xml, but that's pretty much all it does. So, unless I'm missing something, it would not require users to upgrade anything. It would just provide the extra feature for those who have.

Granted, it might not be very useful in production, when you often have an empty contextPath - but not always, and when you do, using the "real" contextPath is probably a better solution than the directory name ({webapp}) which can be somewhat random, depending on your container.

Let me know what you think.

Comment by Fabrizio Giustina [ 19/Feb/10 ]

well, the patch would be ok if changed to use reflection instead of calling servletContext.getServletPath() directly, which would require compiling all the code with servlet api 2.5.

Granted, it might not be very useful in production, when you often have an empty contextPath - but not always, and when you do, using the "real" contextPath is probably a better solution than the directory name (webapp) which can be somewhat random, depending on your container.

FYI we are now mostly using a context parameter instead of the webapp name for production configuration. That also allow you to start an author and public instance starting from the same deployed webapp.
This a sample configuration with two instances mapped to the root context, assigned to two different hosts and with the same webapp folder.

<Server port="9063" shutdown="SHUTDOWN">

  <Service name="Catalina">

    <Connector port="8063" enableLookups="false" protocol="AJP/1.3" URIEncoding="UTF-8"/>

    <Engine name="Catalina" defaultHost="">
      <Realm className="org.apache.catalina.realm.MemoryRealm" />

      <Host name="www.mysite.com" appBase="/www/tomcat/empty"
            unpackWARs="true" autoDeploy="false" xmlValidation="false" xmlNamespaceAware="false">
               <Context path="" docBase="/www/webapps/mysite" debug="0" >
                <Parameter name="instance" value="mysite-public" override="false"/>
            </Context>
      </Host>

      <Host name="author.mysite.com" appBase="/www/tomcatempty"
            unpackWARs="true" autoDeploy="false" xmlValidation="false" xmlNamespaceAware="false">
               <Context path="" docBase="/www/webapps/mysite" debug="0" >
                <Parameter name="instance" value="mysite-author" override="false"/>
                <Manager className="org.apache.catalina.session.PersistentManager" debug="0" saveOnRestart="false">
                        <Store className="org.apache.catalina.session.FileStore"/>
                </Manager>
            </Context>
      </Host>
    </Engine>
  </Service>
</Server>
Comment by Magnolia International [ 19/Feb/10 ]

That's very interesting, and i'd love if this ended up on the wiki !

Re:patch - what's the concern about compiling against servlet-api-2.5 ? It's just a bunch of interfaces, afaik it shouldn't cause any runtime problems (as long as we don't use the new methods obviously) ? If you're concerned we could accidentally use some 2.5-only APIs: I've been thinking about using animal sniffer for a while, which could help preventing that.

Comment by Magnolia International [ 19/Feb/10 ]

here's a more complete patch - including tests and web.xml changes. I've set the updated dependency in the core pom so as to avoid impacting all modules, but if I can manage to setup Animal Sniffer (and if we agree that's the way to go), I'd rather set the servlet-api version in the depMgt of the parent pom.

Comment by Fabrizio Giustina [ 19/Feb/10 ]

Yes, I am concerned about using 2.5 api by mistake and I don't think that setting up a tool like animal sniffer is a good solution.
We similary have the tools that analyze binary incompatibility between releases and we are still breaking things without noticing...

Why are you so concerned about using reflection anyway? I think it's a lot easier, simple and safe... just replace the servletContext.getContextPath call with PropertyUtils.getProperty(servletContext, "contextPath") and save the change in the pom.xml dependency...

Comment by Magnolia International [ 19/Feb/10 ]

I'm not concerned, just curious. I wouldn't go as far as saying that using reflection is simpler and safer, but I get your point.

Comment by Danilo Ghirardelli [ 22/Feb/10 ]

As I said in the other issue, wouldn't be better if we make this property initialization configurable just like the other parts of Magnolia? I mean, everybody seems to have his own way to "stretch" property initialization, depending on customer configuration and habits, and none seems to be better or more flexible than others. So 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 = MyOwnPropertyInitializer. So Magnolia will delegate to the given class the property init, with an obvious fallback to the default one (the existing one). This shuold cover also most of the corner cases, API updates and so on.

Comment by Magnolia International [ 03/Mar/10 ]

Agreed; now that I know how the feature of the other issue works, I'll hold this off until we refactor the whole startup api.

Note that instead of setting a Context/Parameter in the context descriptor, one could also directly set the magnolia.initialization.file property in there!

Comment by Tobias Mattsson [ 20/Mar/12 ]

Implemented. If the contextPath is empty, as it is for default (root) contexts we look for the file using 'ROOT' instead.

The default locations to search for the property file is now:

WEB-INF/config/${servername}/${contextPath}/magnolia.properties
WEB-INF/config/${servername}/${webapp}/magnolia.properties
WEB-INF/config/${servername}/magnolia.properties
WEB-INF/config/${contextPath}/magnolia.properties
WEB-INF/config/${webapp}/magnolia.properties
WEB-INF/config/default/magnolia.properties
WEB-INF/config/magnolia.properties
Comment by Magnolia International [ 20/Mar/12 ]

Looks good, but:

  • how about a test that validates the ROOT ctxPath substitution?
  • didn't we already upgrade to servlet 2.5, making the reflection duscussion and code obsolete?
Comment by Tobias Mattsson [ 21/Mar/12 ]

That's correct, as of 4.5 we require a 2.5 compatible servlet container. See MAGNOLIA-3303.

testFileResolutionWithRootContextPath() in DefaultMagnoliaPropertiesResolverTest tests that a default (root) context becomes ROOT.

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