+1 for Leonardo solution. I really cannot find another way to accomplish this as well.
On Thu, Aug 14, 2008 at 2:19 AM, Leonardo Uribe <[EMAIL PROTECTED]> wrote: > > > On Wed, Aug 13, 2008 at 3:48 PM, Leonardo Uribe <[EMAIL PROTECTED]> wrote: > >> >> >> On Wed, Aug 13, 2008 at 4:12 AM, [EMAIL PROTECTED] < >> [EMAIL PROTECTED]> wrote: >> >>> Matthias Wessendorf schrieb: >>> >>> Simon, >>>> >>>> On Wed, Aug 13, 2008 at 10:53 AM, Simon Kitching <[EMAIL PROTECTED]> >>>> wrote: >>>> >>>> >>>>> Hi, >>>>> >>>>> Just a reminder of the email I posted a few weeks ago: I'm still very >>>>> concerned about the recent ExtensionsFilter refactoring. >>>>> >>>>> The new code now parses the web.xml to extract some configuration >>>>> information. This just feels wrong to me. >>>>> The new code also is based around a custom FacesContext wrapper. I'm >>>>> worried >>>>> about possible interactions between this and other libraries that also >>>>> wrap >>>>> FacesContext. And I'm worried about the portability of this code to the >>>>> upcoming JSF2.0. >>>>> >>>>> I know this stuff is needed to get tomahawk useable for portlets. But >>>>> it's >>>>> very important not to break existing users. >>>>> >>>>> What I would like to see is: >>>>> * separate out the resource-serving stuff into a separate utilities >>>>> module, >>>>> that can be called from either a filter or a FacesContext. >>>>> And call the common code from both ExtensionsFilter and >>>>> TomahawkFacesContextWrapper. >>>>> * have the TomahawkFacesContextFactory create a FacesContext wrapper >>>>> only in >>>>> the case where the filter is not configured, ie >>>>> look in the request-scope for a flag placed there by the filter. That >>>>> means >>>>> there is no chance of breaking existing setups where >>>>> the filter is defined explicitly. >>>>> * ideally, also have some way of turning off the faces-context wrapping >>>>> even >>>>> when the filter is not present. >>>>> * don't parse the web.xml at all. I know you said earlier that it is >>>>> needed, >>>>> but I just don't see why. In any case, I think we *must* >>>>> find some other solution; the current approach is really hard to >>>>> maintain. >>>>> >>>>> Until this is fixed, I would definitely vote -1 on any tomahawk >>>>> release. >>>>> It's a major issue. >>>>> >>>>> >>>> >>>> was there a jira issue for it ? >>>> or can you point me to the svn rev? >>>> >>>> >>> The best thing to do is to look at these classes: >>> org.apache.myfaces.webapp.filter.ExtensionsFilter >>> org.apache.myfaces.webapp.filter.TomahawkFacesContextFactory >>> org.apache.myfaces.webapp.filter.TomahawkFacesContextWrapper >>> org.apache.myfaces.webapp.filter.ExtensionsFilterConfig [1] >>> >>> The svn history of these files references MYFACES-434. >>> There is no specific jira issue about my concerns; it is really an >>> architecture/design disagreement rather than a specific bug. >>> >>> [1] ExtensionsFilterConfig.getExtensionsFilterConfig is invoked from >>> TomahawkFacesContextWrapper. >>> >>> Regards, Simon >>> >>> >> Hi >> >> I'll do a simple review (just to make clear the discussion). >> ExtensionsFilter has the following responsibilities (all we know this but >> sometimes is not very clear): >> >> 1. Wrap a multipart request (used for t:inputFileUpload), so the request >> could be correctly decoded. >> >> 1.1 For do this, it receives some params from web.xml like this: >> >> <filter> >> <filter-name>extensionsFilter</filter-name> >> >> <filter-class>org.apache.myfaces.webapp.filter.ExtensionsFilter</filter-class> >> <init-param> >> <param-name>uploadMaxFileSize</param-name> >> <param-value>100m</param-value> >> </init-param> >> <init-param> >> <param-name>uploadThresholdSize</param-name> >> <param-value>100k</param-value> >> </init-param> >> </filter> >> >> 1.2 So, ExtensionsFilter must wrap the request that goes to faces pages >> using a filter-mapping like this: >> >> <filter-mapping> >> <filter-name>extensionsFilter</filter-name> >> <url-pattern>*.jsf</url-pattern> >> </filter-mapping> >> <filter-mapping> >> <filter-name>extensionsFilter</filter-name> >> <url-pattern>/faces/*</url-pattern> >> </filter-mapping> >> >> 2. ExtensionsFilter must serve resources (javascript, css...) of some >> tomahawk components. >> >> 3. If a buffered instance of AddResource is configured, ExtensionsFilter >> must buffer and add the resource reference >> to the head of jsf pages (for example when it is used DefaultAddResource) >> >> The changes proposed on MYFACES-434 was the following: >> >> a. Use a TomahawkFacesContextWrapper to do tasks 1 and 3 of >> ExtensionsFilter. It must be read the config params to know >> how to wrap the request from web.xml, because this params are on the >> filter init-param part. >> >> b. Add a ServeResourcePhaseListener for do the task 2 of ExtensionsFilter >> (In portlets we could not be ExtensionsFilter working). >> >> c. Wrap the portlet request properly when it is multipart content, so >> t:inputFileUpload could work on portlets >> (to be done, Scott told me to wait some time). >> >> Actually, ExtensionsFilter only does task 2 (ServeResourcePhaseListener >> was added on faces-config.xml) and tasks 1 and 3 >> are done by TomahawkFacesContextWrapper. >> >> Now the problem. >> >> It could be some problems when you mix 1.1 libraries with 1.2 code that >> wraps FacesContext (I don't remember the myfaces >> issue, but the local example of this problem is when you mix >> orchestra(1.1) myfaces core(1.2) with tomahawk(1.1.7-SNAPSHOT)). >> >> In order to anticipate the events, It could be good to remain the old >> feature (if ExtensionsFilter is configured and >> is working, do not use TomahawkFacesContextWrapper). Sounds good the >> suggestions, but in my opinion, there are not >> blocking issues for a release. >> > > I have added the param > org.apache.myfaces.DISABLE_TOMAHAWK_FACES_CONTEXT_WRAPPER, and if > ExtensionsFilter is used before, do not use TomahawkFacesContextWrapper. If > someone wants to implement the other suggestions go ahead, but remember do > not break anything or reduce the features available right now. It takes a > lot of effort and time make things work. > > >> >> But I don't see a solution for get the params for initialize the >> multipart request wrapper different than parse the web.xml file >> I'm open to suggestions, but any solution must remain the actual >> behavior(it works and it is easier to users, since >> the params has been configured there for a long time). >> >> regards >> >> Leonardo Uribe >> > >
