2011/11/10 Mark Thomas <ma...@apache.org>:
> On 08/11/2011 22:49, Mark Thomas wrote:
>> I found some time today to look at this. Summarising the issues so far:
>>
>> 1. I have separately applied a couple of minor fixes that were included
>> in the larger patch.
>>
>> 2. Renaming watchedResources -> reloadResoucres
>> Agree with reverting this change
>>
>> 3. Adding JavaDoc to FailedContext
>> Agree with doing this. I tweaked the wording a little bit as it might
>> not be a StandardContext instance that we failed to create
>>
>> 3. Formatting of FailedContext
>> Given that the majority of the code is NO-OP and the rest is pretty much
>> getters/setters the current formatting is deliberate to reduce the
>> length of the file. I did something similar for WebXml.
>>
>> 4. s/absolue/absolute/
>> Drat. Missed this first time around. Fixed now.
>>
>> 5. redeployResource has been implemented for FailedContext
>>
>> 6. Unexpected deletion of resources due to adding conf/context.xml to
>> redeployResources
>> - removed redeployResources from Context
>> - ensure global resources are never deleted
>> - add global resources at end of redeploy list
>>
>>
>> Particularly with the changes for 6, the patch should be easier to read.
>>
>> The patch is here:
>> http://people.apache.org/~markt/patches/2011-11-08-redeploy-trunk-v4.patch
>>
>> There are currently a few open TC7 bugs so fixing those should provide
>> enough time to review this patch before the 7.0.23 tag.
>
> It has been a couple of days and no objections so I plan to apply this
> patch in the next day or so and tag 7.0.23 early next week. That should
> give me time to fix the remaining open bug and any unit test and/or TCK
> failures that appear.
>
1) s/Redploy/Redeploy/ in several comments.
2) In HostConfig.checkResources() there are three occurrences of the
following code:
if (...
|| (current.getAbsolutePath().startsWith(
configBase().getAbsolutePath()))) {
if (log.isDebugEnabled())
log.debug("Delete " + current);
ExpandWar.delete(current);
}
Thus the following two files will be deleted:
- CATALINA_BASE/conf/<service>/<host>/<appname>.xml
- CATALINA_BASE/conf/<service>/<host>/context.xml.default
Actually there is the following code block that protects
"context.xml.default", but it is only in 2 places out of 3:
// Never delete per host context.xml defaults
if (Constants.HostContextXml.equals(
current.getName())) {
continue;
}
It is easy to fix.
3) Testing redeployment.
Using the following configuration:
- Default tomcat build, plus make two copies of the default context.xml file:
cp context.xml Catalina/localhost/context.xml.default
cp context.xml Catalina/localhost/examples.xml
Tests:
1. touch context.xml -> OK, all redeployed
2. touch context.xml.default -> OK, all redeployed
3. touch examples.xml -> FAIL, redeployed examples app, but it deleted
"context.xml.default" (causing redeployment of all other webapps)
Now shutdown Tomcat, restore "context.xml.default" and start Tomcat
4. delete examples.xml -> Examples app is undeployed, but is instantly
deployed again as "web application directory". It is strange. Is it
expected?
5. rename "context.xml.default" -> OK, all redeployed
6. rename "context.xml" -> OK, all redeployed
I have not tested what happens if I deploy a war file.
4) There is StandardContext#setDefaultContextXml() method.
I do not see it being called by Tomcat code, nor I see it documented
in config/context.html.
The code itself is old, starting with the following commit (Sept 2004,
7 years ago).
http://svn.apache.org/viewvc?view=revision&revision=303172
Do not know why it is needed. Maybe for some embedding scenarios.
It provides an alternative for conf/context.xml.
I mean that HostConfig#addGlobalRedeployResources() could use that
file, but it is unclear how to prevent its deletion on undeploy. It is
a minor and rare issue though (and it is not so important what file
you are going to touch to cause redeployment. Let's allow to touch
conf/context.xml regardless of this configuration).
5) FailedContext is used in HostConfig class only.
Actually HostConfig#digester just preparses the context.xml file: it
processes only the root <Context> element and its attributes.
Main parsing is done in ContextConfig#init() ->
#contextConfig(Digester) -> calls #processContextConfig(Digester, URL)
x 3 times.
There is ContextConfig#ok flag that is reset to "false" if parsing fails.
At end of ContextConfig#configureStart() it does:
if (ok) {
context.setConfigured(true);
} else {
log.error(sm.getString("contextConfig.unavailable"));
context.setConfigured(false);
}
I mean:
- FailedContext class solves a very limited task.
- I think that if a StandardContext instance were used instead of
FailedContext, it will result in a failure when parsing one of 3
context files. It covers more wide range of failures.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
