On 20/09/2013 06:11, Jeremy Boynes wrote: > On Sep 19, 2013, at 8:31 AM, Jeremy Boynes <jboy...@apache.org> wrote: > >> On Sep 19, 2013, at 6:07 AM, ma...@apache.org wrote: >> >>> Author: markt >>> Date: Thu Sep 19 13:07:02 2013 >>> New Revision: 1524727 >>> >>> URL: http://svn.apache.org/r1524727 >>> Log: >>> Always load container SCIs first >> >> This seems at odds with the requirement that "the order in which these >> services are discovered MUST follow the application’s class loading >> delegation model" which is independent of orderedLibs. If we base this on >> ServiceLoader, which seems to be the intent, the algorithm would be: >> >> if (orderedLibs != null) >> excludedJars = URLs of all jars in WEB-INF/lib that are not listed in >> orderedLibs >> for (URL configFile : webappLoader.getResources()) { >> if (!excludedJars.contains(jar containing configFile)) { >> load SCIs from entries in configFile >> } >> } >> >> I originally shied away from trying to compare the URLs returned by the >> classloader and with those from ServletContext.getResource() as I was was >> not sure they would be the same. If they are, then the implementation can be >> simplified along the lines above. If not, we would also need to pass in the >> delegation order of the classloader to know whether to add the "application" >> SCIs before or after the "container" ones. >> >> I'll go ahead and modify this based on the assumption the URLs can be >> compared. > > Patch that changes this to load in the order returned by the ClassLoader
-1 a) The container defined SCIs need to be loaded first. Any other solution places an unnecessary burden on application developers. It is reasonable for application developers to expect that if a container advertises support for WebSocket, JSP, etc. then those features are available when the application's SCI's are executed. b) The URls are not (yet) comparable. Further refactoring of the new resources implementation is required before this is true in all cases. c) The Servlet specification currently makes no statement on a required execution order - therefore any ordering scheme is specification compliant. From a specification point of view, both the current code and the proposed patch are acceptable. d) No-one on the EG has yet raised any objections to the proposal to require container defined SCIs to be loaded first. I have plans to address b) as it should allow the simplification of the resources handling in the WebappClassLoader but even with that issue addressed the other points above stand. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
