Re: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
A few more updates on this: - I have uploaded a proposed fix to MYFACES-2500. - I have checked in the corresponding fix to Mojarra. - I have logged TRINIDAD-1709 to track some Trinidad clean up that we can do once we've got the fix available in both MyFaces and Mojarra. Andy
Re: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
Andy, thanks for the 2500 patch; just applied it to our trunk. -Matthias On Thu, Feb 4, 2010 at 10:31 PM, Andy Schwartz andy.g.schwa...@gmail.com wrote: A few more updates on this: - I have uploaded a proposed fix to MYFACES-2500. - I have checked in the corresponding fix to Mojarra. - I have logged TRINIDAD-1709 to track some Trinidad clean up that we can do once we've got the fix available in both MyFaces and Mojarra. Andy -- Matthias Wessendorf blog: http://matthiaswessendorf.wordpress.com/ sessions: http://www.slideshare.net/mwessendorf twitter: http://twitter.com/mwessendorf
Re: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
Just wanted to mention that I have logged this issue here: https://issues.apache.org/jira/browse/MYFACES-2500 Issue 2500. Pretty cool! :-) Andy
Fwd: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
MyFaces Gang - I haven't had a chance to check to see whether this same issue exists in MyFaces 2.0. Thought I would pass this along in case anyone has thoughts on the existing Facelets behavior (ie. can anyone think of a reason why this behavior might be intentional?) and proposed fix. Thanks, Andy -- Forwarded message -- From: Andy Schwartz andy.g.schwa...@gmail.com Date: Wed, Jan 13, 2010 at 10:25 AM Subject: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters To: d...@javaserverfaces.dev.java.net Gang - While looking into Trinidad/JSF 2 Ajax integration, I came across the following very old comment in Trinidad: // Facelets - as of version 1.1.11 - does something // very strange with its ResponseWriter stack. // It starts with an ordinary stack (which will have // a PPRResponseWriter around an HtmlResponseWriter). // Then it treats that *as an ordinary Writer*, and // wraps it in a StateWriter class, which merely // is used to switch between passing output through // and buffering it. Then it takes that StateWriter // and uses it to cloneWithWriter() a new ResponseWriter // stack! As a result, we have the following stack // outermost to innermost: // PPRResponseWriter // HtmlResponseWriter // StateWriter // PPRResponseWriter // HtmlResponseWriter // ServletResponse's Writer // In the end, we have to get that inner PPRResponseWriter // to just cut it out and pass everything through. Hence, // this hack! The behavior described above still exists in Mojarra 2.0, in FaceletViewHandlingStrategy.renderView(): stateWriter = new WriteBehindStateWriter(origWriter, ctx, responseBufferSize); ResponseWriter writer = origWriter.cloneWithWriter(stateWriter); ctx.setResponseWriter(writer); My guess is that the intention was not to duplicate the entire response writer stack (origWriter above), but to insert the WriteBehindStateWriter into the stack around the original Writer. This is easily accomplished. Instead of re-purposing the current ResponseWriter as the Writer to use in the clone, we can simply grab the Writer itself from the ExternalContext: ExternalContext extContext = ctx.getExternalContext(); Writer outputWriter = extContext.getResponseOutputWriter() stateWriter = new WriteBehindStateWriter(outputWriter, ctx, responseBufferSize); ResponseWriter writer = origWriter.cloneWithWriter(stateWriter); Note that FaceletViewHandlingStrategy uses this technique elsewhere - in createResponseWriter() - which makes me think that the renderView() approach was a simple oversight. On the other hand, if this was as simple as I think, seems like this would have been addressed sooner (like when the Trinidad issue was originally uncovered in 2006), so I would appreciate any thoughts on whether my analysis is off. BTW, while this behavior impacts Trinidad in a noticeable way (and thus forced the hack), it also has a more subtle impact on Mojarra itself. As far as I can tell, Mojarra's own HtmlResponseWriter ends up in the response writer stack twice. That is, before the clone, the response writer stack for Mojarra looks like: HtmlResponseWriter PECoyoteResponse$PECoyoteWriter After the clone, the stack is: HtmlResponseWriter WriteBehindStateWriter HtmlResponseWriter PECoyoteResponse$PECoyoteWriter With my proposed fix above, the stack ends up as: HtmlResponseWriter WriteBehindStateWriter PECoyoteResponse$PECoyoteWriter Pretty sure this was the original intention. It certainly seems cleaner to avoid duplicating the HtmlResponseWriter. Thoughts? Andy
Re: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
Hi
The code in myfaces looks like this:
public PartialResponseWriter getPartialResponseWriter() {
assertNotReleased();
if (_partialResponseWriter == null)
{
ResponseWriter responseWriter =
_facesContext.getResponseWriter();
if (responseWriter == null)
{
// This case happens when getPartialResponseWriter() is
called before
// render phase, like in ExternalContext.redirect(). We have
to create a
// ResponseWriter from the RenderKit and then wrap if
necessary.
try
{
responseWriter =
_facesContext.getRenderKit().createResponseWriter(
_facesContext.getExternalContext().getResponseOutputWriter(), text/xml,
_facesContext.getExternalContext().getRequestCharacterEncoding());
}
catch (IOException e)
{
throw new IllegalStateException(Cannot create Partial
Response Writer,e);
}
}
// It is possible that the RenderKit return a
PartialResponseWriter instance when
// createResponseWriter, so we should cast here for it and
prevent double wrapping.
if (responseWriter instanceof PartialResponseWriter)
{
_partialResponseWriter = (PartialResponseWriter)
responseWriter;
}
else
{
_partialResponseWriter = new
PartialResponseWriter(responseWriter);
}
}
return _partialResponseWriter;
}
I'm not tested trinidad 2.0 alpha + myfaces core 2.0 yet, but I hope test it
soon and dig into this issue.
regards,
Leonardo Uribe
2010/1/13 Andy Schwartz andy.g.schwa...@gmail.com
MyFaces Gang -
I haven't had a chance to check to see whether this same issue exists
in MyFaces 2.0. Thought I would pass this along in case anyone has
thoughts on the existing Facelets behavior (ie. can anyone think of a
reason why this behavior might be intentional?) and proposed fix.
Thanks,
Andy
-- Forwarded message --
From: Andy Schwartz andy.g.schwa...@gmail.com
Date: Wed, Jan 13, 2010 at 10:25 AM
Subject: FaceletViewHandlingStrategy: cloneWithWriter() results in
duplicate ResponseWriters
To: d...@javaserverfaces.dev.java.net
Gang -
While looking into Trinidad/JSF 2 Ajax integration, I came across the
following very old comment in Trinidad:
// Facelets - as of version 1.1.11 - does something
// very strange with its ResponseWriter stack.
// It starts with an ordinary stack (which will have
// a PPRResponseWriter around an HtmlResponseWriter).
// Then it treats that *as an ordinary Writer*, and
// wraps it in a StateWriter class, which merely
// is used to switch between passing output through
// and buffering it. Then it takes that StateWriter
// and uses it to cloneWithWriter() a new ResponseWriter
// stack! As a result, we have the following stack
// outermost to innermost:
// PPRResponseWriter
// HtmlResponseWriter
// StateWriter
// PPRResponseWriter
// HtmlResponseWriter
// ServletResponse's Writer
// In the end, we have to get that inner PPRResponseWriter
// to just cut it out and pass everything through. Hence,
// this hack!
The behavior described above still exists in Mojarra 2.0, in
FaceletViewHandlingStrategy.renderView():
stateWriter = new WriteBehindStateWriter(origWriter,
ctx,
responseBufferSize);
ResponseWriter writer = origWriter.cloneWithWriter(stateWriter);
ctx.setResponseWriter(writer);
My guess is that the intention was not to duplicate the entire
response writer stack (origWriter above), but to insert the
WriteBehindStateWriter into the stack around the original Writer.
This is easily accomplished. Instead of re-purposing the current
ResponseWriter as the Writer to use in the clone, we can simply grab
the Writer itself from the ExternalContext:
ExternalContext extContext = ctx.getExternalContext();
Writer outputWriter = extContext.getResponseOutputWriter()
stateWriter = new WriteBehindStateWriter(outputWriter,
ctx,
responseBufferSize);
ResponseWriter writer = origWriter.cloneWithWriter(stateWriter);
Note that FaceletViewHandlingStrategy uses this technique elsewhere -
in createResponseWriter() - which makes me think that the renderView()
approach was a simple oversight. On the other hand, if this was as
simple as I think, seems like this would have been addressed sooner
(like when the Trinidad issue was originally uncovered in 2006), so I
would appreciate any thoughts on whether my analysis is off.
BTW, while this behavior
Re: FaceletViewHandlingStrategy: cloneWithWriter() results in duplicate ResponseWriters
Hey, great, thanks for looking at this Leonardo! Note that it is possible that this does not impact the Ajax/PartialResponseWriter case, since that might use a different code path for response writer creation. Might want to take a closer look at the renderView() implementations in: - core/impl/src/main/java/org/apache/myfaces/view/facelets/FaceletViewHandler.java - core/impl/src/main/java/org/apache/myfaces/view/facelets/FaceletViewDeclarationLanguage.java Both of these look like they are doing the same sort of response writer cloning (using the original response writer as the Writer) that I noticed in Mojarra. Andy
