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! >[...] 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) ;) I have to say I am not a real fan of this filter. It's like in the old days.. with tomahawk... > 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! 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!! > 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). > 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... 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
