Skip to content

Commit

Permalink
Changes to default webapp configuration from JNDI
Browse files Browse the repository at this point in the history
I was previously using JNDI path mappings to map
"jndi.config file" to "webapp.config.file" and
"jndi.config directory" to "webapp.config.directory".

There are two problems with this:

1) I was thinking this made sense because the JNDI variable should be
   as straightforward as possible; the JNDI values don't get merged
   with the global config namespace, so they don't need to be prefixed
   to avoid name clashes.

   Upon further consideration, I don't think it's worth having
   essentially two names than mean the same thing. So in the new
   setup, the JNDI variables are also called "webapp.config._" just
   like this config paths. This should be easier to explain and
   remember.

2) In the previous setup, setting the JNDI value "config file" would
   override the config value "webapp.config.file", so if you set both,
   only the one specified by JNDI would get loaded.

   I think this was a counterintuitive choice because it's antithetical
   to the Config library's philosophy of merging configs. As further
   evidence that I was wrong, note that I described it incorrectly in
   the documentation. What I had written in the README and in the
   javadoc on WebappConfigs.webappConfigFactory(ServletContext)
   actually describes the new behavior after this commit.
  • Loading branch information
chris-martin committed Oct 1, 2013
1 parent e5ff97c commit 8cc4ae6
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 19 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ ConfigFactory factory = webappConfigFactory(servletContext);
This factory loads these configuration sources, in order from highest to lowest precedence:

* System properties
* File: `JNDI(config directory)/[servlet context path]`
* File: `JNDI(webapp.config.directory)/[servlet context path]`
* File: `${webapp.config.directory}/[servlet context path]`
* File: `JNDI(config file)`
* File: `JNDI(webapp.config.file)`
* File: `${webapp.config file}`
* Classpath resource: `application.conf`
* Classpath resource: `resource.conf`
Expand All @@ -111,7 +111,7 @@ You might set up `server.xml` in a Tomcat installation like this, for example:

```
<Context path="/myApplication/apiServer" docBase="/users/chris/myAppApiServer">
<Environment name="config directory" type="java.lang.String" value="${CATALINA_BASE}/conf"/>
<Environment name="webapp.config.directory" type="java.lang.String" value="${CATALINA_BASE}/conf"/>
</Context>
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

import javax.servlet.ServletContext;

import static edu.gatech.gtri.typesafeconfigextensions.forwebapps.JndiConfigSource.PathMapping.jndiPathMapping;
import static edu.gatech.gtri.typesafeconfigextensions.forwebapps.JndiConfigSourceImpl.defaultJndiConfigSource;
import static edu.gatech.gtri.typesafeconfigextensions.internal.Check.checkNotNull;

Expand All @@ -45,11 +44,11 @@ private WebappConfigs() { }
* <ul>
* <li>System properties</li>
* <li>File:
* {@code JNDI(config directory)/[servlet context path]}</li>
* {@code JNDI(webapp.config.directory)/[servlet context path]}</li>
* <li>File:
* {@code ${webapp.config.directory}/[servlet context path]}</li>
* <li>File: {@code JNDI(config file)}</li>
* <li>File: {@code ${webapp.config file}}</li>
* <li>File: {@code JNDI(webapp.config.file)}</li>
* <li>File: {@code ${webapp.config.file}}</li>
* <li>Classpath resource: {@code application.conf}</li>
* <li>Classpath resource: {@code resource.conf}</li>
* </ul>
Expand Down Expand Up @@ -87,18 +86,11 @@ public static ConfigFactory webappConfigFactory() {
return ConfigFactory.emptyConfigFactory()
.bindDefaults()
.withSources(
jndi().withPathMappings(
jndiPathMapping(
"config directory",
"webapp.config.directory"
),
jndiPathMapping(
"config file",
"webapp.config.file"
)
),
jndi(),
ConfigFactory.systemProperties(),
servletContextDirectory().byKey("jndi.webapp.config.directory"),
servletContextDirectory().byKey("webapp.config.directory"),
ConfigFactory.configFile().byKey("jndi.webapp.config.file"),
ConfigFactory.configFile().byKey("webapp.config.file"),
ConfigFactory.classpathResource("application"),
ConfigFactory.classpathResource("reference")
Expand Down
4 changes: 2 additions & 2 deletions for-webapps/src/test/webapp/WEB-INF/jetty-env.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@

<New class="org.eclipse.jetty.plus.jndi.EnvEntry">
<Arg/>
<Arg>config file</Arg>
<Arg>webapp.config.file</Arg>
<Arg type="java.lang.String">for-webapps/src/test/webapp/conf/jetty-webapp-test.conf</Arg>
<Arg type="boolean">true</Arg>
</New>

<New class="org.eclipse.jetty.plus.jndi.EnvEntry">
<Arg/>
<Arg>config directory</Arg>
<Arg>webapp.config.directory</Arg>
<Arg type="java.lang.String">${jetty conf dir}</Arg>
<Arg type="boolean">true</Arg>
</New>
Expand Down

0 comments on commit 8cc4ae6

Please sign in to comment.