Thanks for the review. I fixed it. I implemented the calculation method of host's default config path to Host(StanderdHost). just like a Host#getAppBaseFile(). This fix is applied only to trunk.
If there is a different implementation idea, feel free to re-fix. 2012/5/25 Konstantin Kolinko <knst.koli...@gmail.com>: > 2012/5/22 <kfuj...@apache.org>: >> Author: kfujino >> Date: Tue May 22 09:27:00 2012 >> New Revision: 1341370 >> >> URL: http://svn.apache.org/viewvc?rev=1341370&view=rev >> Log: >> Enable host's xmlBase attribute in ContextConfig. >> >> Modified: >> tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java >> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml >> >> Modified: >> tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java >> URL: >> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1341370&r1=1341369&r2=1341370&view=diff >> ============================================================================== >> --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java >> (original) >> +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/startup/ContextConfig.java >> Tue May 22 09:27:00 2012 >> @@ -559,9 +559,8 @@ public class ContextConfig implements Li >> "contextConfig.badUrl", defaultContextFile), e); >> } >> } >> - >> - File hostContextFile = new File(getConfigBase(), >> - getHostConfigPath(Constants.HostContextXml)); >> + >> + File hostContextFile = new File(getHostConfigBase(), >> Constants.HostContextXml); >> if (hostContextFile.exists()) { >> try { >> URL hostContextUrl = hostContextFile.toURI().toURL(); >> @@ -1152,30 +1151,43 @@ public class ContextConfig implements Li >> return configBase; >> } >> >> - >> - protected String getHostConfigPath(String resourceName) { >> - StringBuilder result = new StringBuilder(); >> + protected File getHostConfigBase() { >> + File file = null; >> Container container = context; >> - Container host = null; >> - Container engine = null; >> + Host host = null; >> + Engine engine = null; >> while (container != null) { >> - if (container instanceof Host) >> - host = container; >> - if (container instanceof Engine) >> - engine = container; >> + if (container instanceof Host) { >> + host = (Host)container; >> + } >> + if (container instanceof Engine) { >> + engine = (Engine)container; >> + } >> container = container.getParent(); >> } >> - if (engine != null) { >> - result.append(engine.getName()).append('/'); >> + if (host != null && host.getXmlBase()!=null) { >> + String xmlBase = host.getXmlBase(); >> + file = new File(xmlBase); >> + if (!file.isAbsolute()) >> + file = new File(new >> File(System.getProperty(Globals.CATALINA_BASE_PROP)), >> + xmlBase); > > BTW, ContextConfig could call its own ContextConfig.getBaseDir() here > instead of direct look up of the system property. > > The same in 3 other its methods (antiLocking(), fixDocBase(), > getConfigBase()). > > >> + } else { >> + StringBuilder result = new StringBuilder(); >> + if (engine != null) { >> + result.append(engine.getName()).append('/'); >> + } >> + if (host != null) { >> + result.append(host.getName()).append('/'); >> + } >> + file = new File (getConfigBase(), result.toString()); >> } >> - if (host != null) { >> - result.append(host.getName()).append('/'); >> + try { >> + return file.getCanonicalFile(); >> + } catch (IOException e) { >> + return file; >> } >> - result.append(resourceName); >> - return result.toString(); >> } >> >> - >> /** >> * Scan the web.xml files that apply to the web application and merge >> them >> * using the rules defined in the spec. For the global web.xml files, >> @@ -1686,22 +1698,15 @@ public class ContextConfig implements Li >> * it. >> */ >> protected InputSource getHostWebXmlSource() { >> - String resourceName = getHostConfigPath(Constants.HostWebXml); >> - >> - // In an embedded environment, configBase might not be set >> - File configBase = getConfigBase(); >> - if (configBase == null) >> - return null; >> - > > The above "return null" case disappears without replacement. > >> String basePath = null; >> try { >> - basePath = configBase.getCanonicalPath(); >> + basePath = getHostConfigBase().getCanonicalPath(); > > getCanonicalFile() is already called inside of getHostConfigBase(), so > there is double work here. > >> } catch (IOException e) { >> log.error(sm.getString("contextConfig.baseError"), e); >> return null; >> } >> >> - return getWebXmlSource(resourceName, basePath); >> + return getWebXmlSource(Constants.HostWebXml, basePath); >> } >> >> /** >> > > There is code duplication between > HostConfig.configBase() and this new method ContextConfig.getHostConfigBase() > > I wonder whether it could be simplified. If HostConfig reference > cannot be reached from ContextConfig, it could be a static method > inside of HostConfig to perform the calculation. Calculation of host's > default config path belongs to the host. > > Best regards, > Konstantin Kolinko > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > -- Keiichi.Fujino --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
