On Jan 11, 2011, at 12:28 AM, Konstantin Kolinko wrote:
> 2011/1/10 <ma...@apache.org>:
>> Author: markt
>> Date: Mon Jan 10 16:48:25 2011
>> New Revision: 1057275
>>
>> URL: http://svn.apache.org/viewvc?rev=1057275&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50205
>> Add deployIgnore to Host
>> Based on a patch by Jim Riggs
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/Host.java
>> tomcat/trunk/java/org/apache/catalina/core/StandardHost.java
>> tomcat/trunk/java/org/apache/catalina/core/mbeans-descriptors.xml
>> tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java
>> tomcat/trunk/java/org/apache/catalina/startup/LocalStrings.properties
>> tomcat/trunk/webapps/docs/config/host.xml
>>
>
> (..skipped)
> Looking at HostConfig#filterAppPaths()
>
>> /**
>> + * Filter the list of application file paths to remove those that match
>> + * the regular expression defined by {...@link Host#getDeployIgnore()}.
>> + *
>> + * @param unfilteredAppPaths The list of application paths to filtert
>> + *
>> + * @return The filtered list of application paths
>> + */
>> + protected String[] filterAppPaths(String[] unfilteredAppPaths) {
>> + if (host.getDeployIgnore() == null) {
>> + return unfilteredAppPaths;
>> + }
>> +
>> + Pattern filter = Pattern.compile(host.getDeployIgnore());
>
> 1) It would be better to store the value as Pattern inside the Host
> and implement Host.getDeployIgnorePattern() method that will return
> Pattern. This way a syntax error can be detected at assignment time.
> 2) null string and "" string should be treated equally
My original patch did both of these things, but the patch was written for the
old world of comma-separated list of REs. It looks like Mark's changes to use
just a single RE changed this behavior. I agree -- especially now that it is
just a single RE -- compiling it and storing just the compiled pattern is a
good idea. (My original patch stored both the comma-separated string and a
list of compiled patterns.)
>> +
>> + List<String> filteredList = new ArrayList<String>();
>> + for (String appPath : unfilteredAppPaths) {
>> + if (filter.matcher(appPath).matches()) {
>> + log.debug(sm.getString("hostConfig.ignorePath", appPath));
>
> Wrap with log.isDebugEnabled()
Again, this was in my patch. Just an oversight when applied, I guess.
Thanks for catching these, Konstantin.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
