Hi guys, let me weigh in on the filter question: please, no filter!
@prefix suffix mappings: you can get the registered mappings in previous servlet versions: parsing the web.xml in servlet 3.0: via the API http://download.oracle.com/javaee/6/api/javax/servlet/ServletRegistration.html the resource url should then always be generated with the prefix mapping - how can this lead to inconsistencies? best regards, Martin On Thu, Jun 23, 2011 at 11:54 PM, Leonardo Uribe <lu4...@gmail.com> wrote: > Hi > > In the last days this enhancements were commited: > > ------------------------------------------------------------------------------ > > Added GZIP compression to ExtendedResourceHandler and these params: > > /** > * Enable or disable gzip compressions for resources served by > this extended resource handler. By default is disabled (false). > */ > @JSFWebConfigParam(defaultValue="false") > public static final String INIT_PARAM_GZIP_RESOURCES_ENABLED = > "org.apache.myfaces.commons.GZIP_RESOURCES_ENABLED"; > > /** > * Indicate the suffix used to recognize resources that should be > compressed. By default is ".css .js". > */ > @JSFWebConfigParam(defaultValue=".css, .js") > public static final String INIT_PARAM_GZIP_RESOURCES_SUFFIX = > "org.apache.myfaces.commons.GZIP_RESOURCES_SUFFIX"; > public static final String > INIT_PARAM_GZIP_RESOURCES_EXTENSIONS_DEFAULT = ".css .js"; > > /** > * Indicate if gzipped files are stored on a temporal directory to > serve them later. By default is true. If this is > * disable, the files are compressed when they are served. > */ > @JSFWebConfigParam(defaultValue="true") > public static final String INIT_PARAM_CACHE_DISK_GZIP_RESOURCES = > "org.apache.myfaces.commons.CACHE_DISK_GZIP_RESOURCES"; > > by default compression is set to false. It could be good to enable > compression only on files bigger than some specified lenght, to allow > finer tuning. > > ------------------------------------------------------------------------------ > > > and these enhancements: > > > ------------------------------------------------------------------------------ > > Added new scanning and parsing of myfaces-resources-config.xml files. > It was added this param: > > /** > * This param allow to override the default strategy to locate > myfaces-resources-config.xml files, that will be parsed later. In this > way > * it is possible to include new source locations or handle cases > like OSGi specific setup. > */ > @JSFWebConfigParam > public static final String > INIT_PARAM_EXTENDED_RESOURCE_HANDLER_CONFIG_URL_PROVIDER = > "org.apache.myfaces.commons.EXTENDED_RESOURCE_HANDLER_CONFIG_URL_PROVIDER"; > > I think just a param that instantiate a class implementing > MyFacesResourceHandlerUrlProvider is enough. The default algorithm > loook on classpath for META-INF/myfaces-resources-config.xml and on > servlet context for WEB-INF/myfaces-resources-config.xml files. > > myfaces-resources-config.xml files can be used with these options: > > <?xml version="1.0" encoding="UTF-8"?> > <myfaces-resources-config> > <!-- Mark this library to be handled by Extended Resource Handler --> > <library> > <library-name>libraryA</library-name> > </library> > > <!-- Indicate this library has another name, so if libraryC is used, > resources should be redirected to libraryC1 --> > <library> > <library-name>libraryC</library-name> > <redirect-name>libraryC1</redirect-name> > </library> > > <!-- Allow to customize the request path generated, to do things like > take library resources from a Content Delivery Network (CDN) or just > take it directly from an specified location. Note it is responsibility > of the developer to configure it properly, and the resources should > exists locally under the library name selected. --> > <library> > <library-name>libraryB</library-name> > > <request-path>http://someaddress.com/alternatePath/#{resourceName}</request-path> > <!-- This example shows the variables that can be called > inside the expression to construct the request map > <request-path>#{extensionMapping ? '' : > mapping}/javax.faces.resource/$/#{localePrefix}/#{libraryName}/#{resourceName}#{extensionMapping > ? mapping : ''}</request-path> > --> > </library> > > </myfaces-resources-config> > > All libraries referenced here will be handled by the extended > ResourceHandler. Additionally, there is an option to redirect a > library name into another, to deal with possible conflicts between > resources loaded, specially javascript libraries. And finally there is > an option to override the request-path with an EL expression, so if > you have a library with some static resources it is easy to construct > an url to load them from a Content Delivery Network (CDN) or just from > some specified path. The only thing you should note is the library > should exists locally under the library name, to detect when a > resource can be resolved or not. > > ------------------------------------------------------------------------------ > > I have not tested it fully, but in my opinion it looks good. I has the > best of the previous AdvancedResourceHandler with some new valuable > features proposed. > > If no objections I'll remove the previous code, since it was > integrated on the alternate solution. > > Suggestions and tomatoes are welcome > > Leonardo Uribe > > 2011/6/14 Leonardo Uribe <lu4...@gmail.com>: >> Hi Jakob >> >> 2011/6/14 Jakob Korherr <jakob.korh...@gmail.com>: >>> Hi Leonardo, >>> >>>>Because set prefix and suffix mapping for the same webapp could lead >>>>to inconsistencies. >>> >>> Which inconsistencies exactly? Please give an example, I can't really >>> think of any! >>> >> >> Let's take a look to AdvanceResource.getRequestPath: >> >> public String getRequestPath() >> { >> FacesContext facesContext = FacesContext.getCurrentInstance(); >> StringBuilder path = new StringBuilder(); >> path.append(ResourceUtils.getFacesServletPrefix(facesContext)); >> ..... >> >> Now look on getFacesServletPrefix: >> >> public static String getFacesServletPrefix(FacesContext facesContext) >> { >> ExternalContext externalContext = facesContext.getExternalContext(); >> Map<String, Object> applicationMap = >> externalContext.getApplicationMap(); >> >> // check if already cached >> String prefix = (String) applicationMap.get(FACES_SERVLET_PREFIX_KEY); >> if (prefix == null) >> { >> // try to extract it from current request >> prefix = getFacesServletPrefixMapping(facesContext); >> .... >> >> public static String getFacesServletPrefixMapping(FacesContext >> facesContext) >> { >> ExternalContext externalContext = facesContext.getExternalContext(); >> >> String pathInfo = externalContext.getRequestPathInfo(); >> String servletPath = externalContext.getRequestServletPath(); >> >> if (pathInfo != null) >> { >> return servletPath; >> } >> else >> { >> // In the case of extension mapping, no "extra path" is available. >> // Still it's possible that prefix-based mapping has been used. >> // Actually, if there was an exact match no "extra path" >> // is available (e.g. if the url-pattern is "/faces/*" >> // and the request-uri is "/context/faces"). >> int slashPos = servletPath.lastIndexOf('/'); >> int extensionPos = servletPath.lastIndexOf('.'); >> if (extensionPos > -1 && extensionPos > slashPos) >> { >> // we are only interested in the prefix mapping >> return null; >> } >> else >> { >> // There is no extension in the given servletPath and >> therefore >> // we assume that it's an exact match using >> prefix-based mapping. >> return servletPath; >> } >> } >> } >> >> The code takes pathInfo/servletPath information and prepend it to the >> beggining. The first bug is the code prepend the extension when suffix >> mapping is used!. But look the mapping is saved on the application >> map. So on further request, the mapping is retrieved from application >> map, so if the first request is suffix mapping, all later resource >> request paths will be generated wrong, even if prefix mapping is used. >> >> The problem is to know if prefix mapping is used you should parse >> web.xml file, but that's wrong, because in servlet 3.0 spec you don't >> necessary have that file (web fragment?). In conclusion there is no >> way to "detect" and generate the mapping correctly. >> >> The nice part about the filter is you can put some code to detect >> automatically if the filter is registered or not and act according. >> This is the param: >> >> /** >> * Indicate if this filter is being used to process request. It >> works in three modes: >> * >> * <ul> >> * <li>true: assume the filter is correctly setup.</li> >> * <li>check: check if the filter has been setup and if that so, >> use it. Otherwise, it uses FacesServlet (use prefix mapping to make >> all features work).</li> >> * <li>false: filter is not used at all.</li> >> * </ul> >> */ >> @JSFWebConfigParam(defaultValue="check", expectedValues="true, >> false, check") >> public static final String INIT_PARAM_USE_EXTENDED_RESOURCE_FILTER >> = "org.apache.myfaces.commons.USE_EXTENDED_RESOURCE_FILTER"; >> public static final String >> INIT_PARAM_USE_EXTENDED_RESOURCE_FILTER_DEFAULT = "check"; >> >> In this way, there will not be inconsistencies, because we have the >> three options: >> >> - If prefix mapping is used -> prepend the prefix >> - If suffix mapping is used and no filter setup -> use suffix mapping >> like always >> - If suffix mapping is used and filter setup -> use filter prefix mapping >> >>>>[...] If a page is rendered using suffix mapping, >>>>resource paths will use that and not prefix mapping, because faces >>>>mapping is derived from the request path. >>> >>> Nope. That's the whole point of the AdvancedResourceHandler. It always >>> uses prefix mapping, regardless of what the current page is using!! >>> Just check the code (before your commit) ;) >>> >> >> As you can see, I have found many bugs in the previous code. I usually >> take my time to check this stuff. In fact, I implemented all >> ResourceHandler implementation in MyFaces, and other alternate >> implementations on tomahawk and sandbox for different use cases, so I >> know step by step what says the spec and how the code works. >> >>> I have to say I am not a real fan of this filter. It's like in the old >>> days.. with tomahawk... >>> >> >> Note every JSF library uses a filter! Trinidad, RichFaces, PrimeFaces, >> IceFaces. It could be good to find a solution without use a filter but >> based on the previous discussion I don't see any. I don't get the >> point. If you have a better idea please send your comments. >> >> I think the strategy proposed is an advance, because you only use it >> when it is necessary. The other alternative is tell users don't use >> suffix mapping. >> >>>> I think the opposite in this case, because the previous syntax is >>>> ambiguous, so you can't decide how to get the libraryName and >>>> resourceName from the resourceBasePath, and the spec requires describe >>>> that in a explicit way. Think about a resource on: >>>> >>>> /de/mydir/myresource.js (resourceName="de/mydir/myresource.js") >>>> >>>> will produce this request path: >>>> >>>> http://{server}[:port]/{appPath}/javax.faces.resource/de_AT/mydir/myresource.js >>>> >>>> The algorithm will detect de as a locale prefix, mydir as a library >>>> and myresource.js as a resource name, but that's wrong because the >>>> resource name is de/mydir/myresource.js. >>> >>> I am sorry, but this is wrong, Leo. >>> >>> At first a resourceName of "de/mydir/myresource.js" should not be >>> used. It should rather be resourceName="myresource.js" and >>> libraryName="de/mydir". I know the spec does not explicitly tell us >>> that the resourceName must not be a path, but it is the only way it >>> really makes sence, if you think about it. Otherwise separation of >>> libraryName and resourceName would not be necessary! >>> >> >> The problem is "should not be used" is not an option. I'm saying here >> that the same url could be handled by both the default and the >> proposed method. Assume that a developer will do everything you >> imagine is not very realistic. >> >>> Furthermore, a resourceName of "de/mydir/myresource.js" would produce >>> the following path (you did skip "de" and "faces"): >>> >>> http://{server}[:port]/{appPath}/faces/javax.faces.resource/de_AT/de/mydir/myresource.js >>> >>> ..thus producing a resource with libraryName="de/mydir" and >>> resourceName="myresource.js". And this is exactly what is expected of >>> it!! >> >> No, because "de" is a valid locale!. >> >> I think that the relationship between Resource instances and request >> paths generated should be 1:1 and should be symmetric. That means, if >> I call this code from a renderer: >> >> ResourceHandler.createResource("","","de/mydir/myresource.js"); >> >> Later the ResourceHandler implementation, when >> handleResourceRequest(FacesContext) is called should call the same >> method, but instead it will call: >> >> ResourceHandler.createResource("de","mydir","myresource.js"); >> >> Who should attend the request? the extended resource handler or the >> default one. The first call expect the default one, but the second?. >> >> In conclusion, if the example does not fulfit the two conditions (be >> 1:1 and symmetric), for any imaginable Resource instance, it will not >> be correctly specified. >> >>> >>>> Anyway we need something to "diferentiate" between the old and the >>>> alternate syntax, so use '$/' is as good as any other we can imagine. >>> >>> I don't think we need to do this differentiation in the first place. I >>> see no reason for it. My code in MyFaces commons (before you committed >>> your stuff) did not use it either and it worked well! Of course, I did >>> not have this filter, but I don't like that anyway (see above). >>> >> >> Why don't you like it? do you have something better in mind?. If you >> want I change of opinion, please provide me with arguments to think >> the opposite. I'm always open to any suggestions or critics. >> >>>> My interest is put this as a module for JSF 2.0, because there is >>>> nothing that prevent us doing it, and this is the "base stone" to make >>>> components with libraries like dojo, that requires load modules from >>>> derived base paths. After that, we can push this on the spec for JSF >>>> 2.2 and the EG will decide. >>> >>> That's the general idea. And note that I am the guy working on the >>> resource handler stuff in the JSF 2.2 EG ;) >>> >>> >>> One more note at the end: actually I am not very happy that you >>> committed your code directly into the svn without providing it as >>> patch before. You did not do any work on the AdvancedResourceHandler >>> before (it was all my code) and it was a pretty big commit (even took >>> 2 commit-mails). Thus you gave me no choice to take a look at it and >>> discuss the changes with you. If I did something like this, the first >>> thing you would do is reverting my commit and providing it as patch so >>> that we can discuss it. I won't do that, but actually it's kinda >>> annoying... >>> >> >> I commited the code instead create a patch, because the code commited >> does not override the previous code. So you can put the two solutions >> side by side and compare them in a easier way. If something doesn't >> like us, we can remove the added files and that's it, there is no harm >> or you don't have to do something difficult to revert the code, >> right?. Note the code has not released yet, so we don't have to >> preserve the package or the class name or any structure. >> >> Things are different when you have already code and you need to >> "override" something, to include something new. A patch is better in >> that case. But in this case, I'm working on a completely different >> solution from scratch. >> >> regards, >> >> Leonardo Uribe >> >>> Regards, >>> Jakob >>> >>> 2011/6/14 Leonardo Uribe <lu4...@gmail.com>: >>>> Hi Jakob >>>> >>>> 2011/6/13 Jakob Korherr <jakob.korh...@gmail.com>: >>>>> Hi Leo, >>>>> >>>>> Overall this seems nice, thanks! >>>>> >>>>> However, I have some comments on your solution: >>>>> >>>>> 1) If I have to configure a Filter in web.xml I can just as good >>>>> define a prefix mapping for the FacesServlet. I don't see why an >>>>> additional Filter is better than an additional servlet-mapping. So why >>>>> exactly? >>>>> >>>> >>>> Because set prefix and suffix mapping for the same webapp could lead >>>> to inconsistencies. If a page is rendered using suffix mapping, >>>> resource paths will use that and not prefix mapping, because faces >>>> mapping is derived from the request path. >>>> >>>> We can't change FacesServlet to only handle resource request for a >>>> specific mapping, but with the filter this is done by default. Note >>>> the filter will be used only when suffix mapping is used. I tried it >>>> using FacesServlet but it is useless, because you should do changes on >>>> jsf impl, so at the end it will only work on myfaces, and the >>>> intention is provide it as a module for any jsf implementation. >>>> >>>>> 2) The locale in the resource path really is essential, please do NOT >>>>> remove it. I did a lot of tests with different browsers about this and >>>>> you just cannot verify that every user will get the right (localized) >>>>> resource, if the user's locale is not on the request path. The two >>>>> main problems here are: a) the user changes the locale, but the >>>>> browser uses the cached resource (with the old locale), because it >>>>> cannot know that it has changed (some browsers will not even start a >>>>> request for it) - however, if the locale is in the path, it will >>>>> change and thus the browser will trigger a new request for the >>>>> resource. b) you cannot really know if there are multiple versions of >>>>> a resource for different locales, because you should not scan all jar >>>>> files for them (--> remember the performance-issue we had with this >>>>> stuff) and furthermore the classpath might change! >>>>> >>>> >>>> Ok, good to know that. The current code works "forcing" output the >>>> locale, so we can just let things as is. >>>> >>>>> 3) >>>>>> http://{server}[:port]/{appPath}/javax.faces.resource/{locale}/{libraryName}/[resourceName] >>>>>> >>>>>> Unfortunately, this syntax is ambiguous, because it is not possible to >>>>>> identify if the request should be handled by the default algorithm or >>>>>> by the "extended" ResourceHandler. So I tried this one on >>>>>> ExtendedResourceHandler: >>>>>> >>>>>> http://{server}[:port]/{appPath}/javax.faces.resource/$/{locale}/{libraryName}/[resourceName] >>>>> >>>>> This is a nice idea, but I guess this will not be an option for the >>>>> JSF 2.2 resource handler (which will most likely be a modified version >>>>> of the AdvancedResourceHandler). >>>>> >>>> >>>> I think the opposite in this case, because the previous syntax is >>>> ambiguous, so you can't decide how to get the libraryName and >>>> resourceName from the resourceBasePath, and the spec requires describe >>>> that in a explicit way. Think about a resource on: >>>> >>>> /de/mydir/myresource.js (resourceName="de/mydir/myresource.js") >>>> >>>> will produce this request path: >>>> >>>> http://{server}[:port]/{appPath}/javax.faces.resource/de_AT/mydir/myresource.js >>>> >>>> The algorithm will detect de as a locale prefix, mydir as a library >>>> and myresource.js as a resource name, but that's wrong because the >>>> resource name is de/mydir/myresource.js. >>>> >>>> Anyway we need something to "diferentiate" between the old and the >>>> alternate syntax, so use '$/' is as good as any other we can imagine. >>>> My interest is put this as a module for JSF 2.0, because there is >>>> nothing that prevent us doing it, and this is the "base stone" to make >>>> components with libraries like dojo, that requires load modules from >>>> derived base paths. After that, we can push this on the spec for JSF >>>> 2.2 and the EG will decide. >>>> >>>> regards, >>>> >>>> Leonardo Uribe >>>> >>>>> >>>>> Please take this stuff into account - thanks! >>>>> >>>>> Regards, >>>>> Jakob >>>>> >>>>> 2011/6/14 Leonardo Uribe <lu4...@gmail.com>: >>>>>> Hi >>>>>> >>>>>> I committed on myfaces-commons-resourcehandler module on trunk an >>>>>> alternative solution for this issue. It is still not complete, so the >>>>>> idea is discuss it. See: >>>>>> >>>>>> https://issues.apache.org/jira/browse/MFCOMMONS-33 >>>>>> >>>>>> From previous discussion, on AdvancedResource handler we have: >>>>>> >>>>>> a. relative paths between resources (css files referencing images >>>>>> without using #resource['..']) >>>>>> b. caching resources in the client (disabled if ProjectStage == >>>>>> Development) >>>>>> c. GZIP compression and local cache in tmp dir (disabled if >>>>>> ProjectStage == Development) >>>>>> d. i18n (supporting country code and language). >>>>>> >>>>>> We had the following proposals: >>>>>> >>>>>> 1. reutilize resource information to prevent unnecessary calls to >>>>>> getResource() (shared ResourceCache). >>>>>> 2. Alternate xml file >>>>>> 3. Make it work with suffix mapping. >>>>>> 4. Add a SPI interface to delegate .xml resource scanning. >>>>>> 5. Use content delivery network (CDN) to load known javascript or other >>>>>> resource files like jQuery or prototype. >>>>>> >>>>>> The objective is provide a solution for all those wanted features. >>>>>> >>>>>> The most important one is number 3. (make it work with suffix >>>>>> mapping), because it limits the scope where a. (relative paths between >>>>>> resources) could be applied. Use a parse on some files it is not a >>>>>> very good solution, so I tried to found an alternative. The most >>>>>> simple one is use a filter that just do the "resource handling" part, >>>>>> just like FacesServlet does. So with suffix mapping you only need to >>>>>> add this on web.xml file: >>>>>> >>>>>> <filter> >>>>>> <filter-name>Faces Filter</filter-name> >>>>>> >>>>>> <filter-class>org.apache.myfaces.commons.resourcehandler.filter.ResourceHandlerFilter</filter-class> >>>>>> </filter> >>>>>> >>>>>> <filter-mapping> >>>>>> <filter-name>Faces Filter</filter-name> >>>>>> <url-pattern>/javax.faces.resource/*</url-pattern> >>>>>> </filter-mapping> >>>>>> >>>>>> and that's it. In this way, there is no need to any parser, just put >>>>>> the files on a library, register it on the xml file. If you are using >>>>>> prefix mapping for Faces Servlet, you will not need that entry, >>>>>> because everything will be handled from Faces Servlet. >>>>>> >>>>>> With this solution, javascript libraries like dojo that loads files or >>>>>> have css resources with url(...) entries will work without any >>>>>> changes. >>>>>> >>>>>> I have seen this issue: >>>>>> >>>>>> https://issues.apache.org/jira/browse/MFCOMMONS-30 >>>>>> Change URL management of Advanced JSF 2 ResourceHandler >>>>>> >>>>>> The idea was use this >>>>>> >>>>>> http://{server}[:port]/{appPath}/javax.faces.resource/{locale}/{libraryName}/[resourceName] >>>>>> >>>>>> Unfortunately, this syntax is ambiguous, because it is not possible to >>>>>> identify if the request should be handled by the default algorithm or >>>>>> by the "extended" ResourceHandler. So I tried this one on >>>>>> ExtendedResourceHandler: >>>>>> >>>>>> http://{server}[:port]/{appPath}/javax.faces.resource/$/{locale}/{libraryName}/[resourceName] >>>>>> >>>>>> The first $ caracter says this extension should be handled by the >>>>>> ExtendedResourceHandler. We can go further and allow this notation: >>>>>> >>>>>> http://{server}[:port]/{appPath}/javax.faces.resource/$$/{libraryName}/[resourceName] >>>>>> >>>>>> In this way there is no ambiguity, and we don't need to force locale >>>>>> to be output. This could be possible too: >>>>>> >>>>>> http://{server}[:port]/{appPath}/javax.faces.resource/$$$/[resourceName] >>>>>> >>>>>> But that it is not really necessary at all. >>>>>> >>>>>> The proposed code still does not contains the options for GZIP >>>>>> compression, because the previous algorithm does not take into account >>>>>> what happen on concurrent requests (two threads modifying the same >>>>>> file at the same time). I did an algorithm for sandbox for JSF 2.0 >>>>>> s:roundedPanel. It uses an application scope map and some synchronized >>>>>> blocks to ensure only one thread writes the file. Exactly the same >>>>>> pattern works in this case, so the only thing we need to do is >>>>>> refactor that code and put it here. >>>>>> >>>>>> Does that sounds good? if no objections commit the proposals here soon. >>>>>> >>>>>> regards, >>>>>> >>>>>> Leonardo Uribe >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Jakob Korherr >>>>> >>>>> blog: http://www.jakobk.com >>>>> twitter: http://twitter.com/jakobkorherr >>>>> work: http://www.irian.at >>>>> >>>> >>> >>> >>> >>> -- >>> Jakob Korherr >>> >>> blog: http://www.jakobk.com >>> twitter: http://twitter.com/jakobkorherr >>> work: http://www.irian.at >>> >> > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces
