2012/10/16 Mark Thomas <ma...@apache.org>:
> On 09/10/2012 22:27, Mark Thomas wrote:
>> I believe from the various threads on the Resources implementation for
>> Tomcat 8 there is agreement that:
>>
>> - DirContext is not the right basis for the Resources API
>> - Refactoring is required to simplify the multiple 'add-ons' and to
>> provide a sound basis for overlays (which may slip to Servlet 3.2)
>> - Any Resources implementation must provide in-memory caching
>>
>> The sandbox Resources implementation addresses all of the above and
>> passes the Servlet and JSP TCKs as well as all the unit tests.
>>
>> Given the above, is there any objection to merging the changes made in
>> the sandbox back to trunk?
>
> It has been a week and no objections so I will go ahead and do this.
> However, I am currently working on the new HTTP Upgrade API, the new
> WebSocket API and converting trunk to use both. I plan to focus on that
> for the next few days and and merge the resources changes afterwards.
>
> Mark
>
>> Note that the following TODOs remain:
>> - Provide an option to exclude selected resources from the cache
>> - Remove the DirContext code
>> - Review the use of trailing '/'
>> - Determine if a custom URL scheme is required (and implement if it is)
>> - Review the interaction of WebappClassLoader and Resources to see if
>> there current work-arounds are a) still required b) implemented
>> efficiently
>>
1. Are <PreResource> and <PostResource> able to inject individual files?
2. I am -1 on removal of VirtualWebappLoader.
The feature of injecting additional classpath entries into
WebappClassLoader.super.addURL() is an important one and I use it a
lot to inject external dependent libraries into web applications.
I do not like your approach as a replacement, because classpath
entries handling (jars handling) in URLClassLoader and in
WebappClassLoader is based on different requirements. I think that
URLClassLoader serves better for external libraries
See also #1. above.
3. Regarding the API.
In the tests I see
[[[
tomcat.addContext("", System.getProperty("java.io.tmpdir"));
File lib = new File("webapps/examples/WEB-INF/lib");
- ctx.setAliases("/WEB-INF/lib=" + lib.getCanonicalPath());
+ ctx.setResources(new StandardRoot(ctx));
+ ctx.getResources().createWebResourceSet(
+ WebResourceRoot.ResourceSetType.POST, lib.getAbsolutePath(),
+ "/WEB-INF/lib", "");
]]]
That "setResources" call seems to be excessive.
Maybe getResources() should create a StandardRoot instance implicitly,
or Tomcat.addContext() could create one, or StandardContext could
create one.
4. org.apache.catalina.webresources.Cache
+ private ConcurrentMap<String,CachedResource> resourceCache =
+ new ConcurrentHashMap<>();
It think it'd be better with "final" there.
If you resolve #2., then I do not mind the merge.
I do not see much problems with the new APIs. I think we can go with them.
For reference:
The last merge from trunk to the branch was in r1397302, so I did a
svn diff between trunk and trunk-resources branch state after that
commit.
The command is
svn diff --old=https://svn.apache.org/repos/asf/tomcat/trunk@1397302
--new=https://svn.apache.org/repos/asf/tomcat/sandbox/trunk-resources@1397302
I uploaded the diff file here (its size is ~400KB):
http://people.apache.org/~kkolinko/patches/2012-10-17_tc8_resources-vs-trunk_r1397302.diff
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
