-- Adam
On 12/27/06, Scott O'Bryan <[EMAIL PROTECTED]> wrote:
Hey Adam, In your last checkin to my branch you made some comments I would like to address. In the "DispatchResponseConfiguratorImpl" there is an "isApplied" function. You were asking why it was there. The reason for this is simple.. Backward compatibility. You mentioned to me in some previous emails that the filter was needed for backward compatibility and that any wrappers which WERE handled by the filter, needed to continue to be handled by the filter. Likewise, in many instances, the use of the filter is optional and until the JSR-301 is complete, Portal environments won't have access to pre-lifecycle requests and whatnot. So the isApplied simply tells whether the wrapper has been applied or not. If it has (in the case of the filter or other "wrapping" mechanisms like Portlet Filters (JSR-286) or Bridge listeners (JSR-301)) then it doesn't apply it a second time when the ExternalContext is retrieved. If this hasn't been pre-initialized then it does.
OK, I see that - but a much simpler way of going about this is to run the GlobalConfiguratorImpl *as a whole* from the Filter and move this "have I run?" stuff into just GlobalConfiguratorImpl.
This code is REALLY only needed for backward compatibility and any Configurators going forward would not be dependant on the filter and therefore would not need to worry about whether it was applied or not.
No, it's not just "backwards compatibility"; the point is that it is perfectly acceptable for a developer to write a servlet filter. Just because you or I may want to require portlet support doesn't mean everyone does, or that everyone on the planet should stop using servlet filters. So, we should support those that want to write servlet filters to the extent that it's feasible, or those that want to integrate pre-existing filters to the extent that it's feasible. Which it is, as best I can see.
In FileUploadConfiguratorImpl you added a fixme that we should clean up the handles to the files as soon as possible. I agree with this. The current implementation (ie. before configurators were added) applied these before execution of the filterchain and then was allowed to get GC'd after. I believe that doing the same logic inside of the beginRequest and endRequest has about the same lifespan. So my question is, how does this differ from what was provided by Trinidad before these enhancements? If it isn't any different, then maybe we can file a Jira ticket and handle this as part of another patch.
Yep, if it's endRequest(), that's good enough (as long as our configurator code has absolute guarantees that endRequest() will be called, which it does, I think?) -- Adam
Are these acceptable or are these something that need to be changed for this patch? Scott
