isPostback patch for Trinidad 1.2 state manager

[Rogers Reilly <ro...@s-rod.com>]

Hi,

I'm porting a Trinidad webapp over to 1.2 (with the help of Adam's great 
new 1.2 build- thnx!), and was having trouble using Shale remoting.  
Specifically, a Shale remoting request with parameters was causing 
Trinidad 1.2 to bomb out, throw an NPE when finding the ViewRoot.   I 
tracked down the source of the issue:  Trinidad's 
CoreResponseStateManager was telling RestoreView that the request was a 
postback, and then when it looked for the state parameter 
("org.apache.myfaces.trinidad.faces.STATE"), it couldn't find it.  The 
ViewRoot is then NULL.

(I actually tried to pass the state along as a param in the Remoting 
request, but it didn't quite work- the restored viewId corresponds to 
the base page URL, so the Shale PhaseListener passes, doesn't realize 
it's supposed to take it.)

Anyway, looking into the code, I found that CoreResponseStateManger 
doesn't implement .isPostback(FacesContext); it just lets its parent's 
implementation stand- a default implementation that just returns TRUE if 
the request has any parameters at all, meant for old applications.  
Here's how the spec says this method should be implemented:

"The implementation if this method for the Standard HTML RenderKit must 
consult the javax.faces.context.ExternalContext's requestParameterMap 
and return true if and only if there is a key equal to the value of the 
symbolic constant VIEW_STATE_PARAM."

Which is easy enough to do.  I just grabbed the implementation from RI's 
ResponseStateManagerImpl, tweaked the constant, dropped it into 
CoreReponseStateManager, and rebuilt.  + this code:

 @Override
  public boolean isPostback(FacesContext context) {

        return context.getExternalContext().getRequestParameterMap().
              containsKey(_STATE_FIELD_NAME);

  }


Not sure what the other effects of implementing this function will be, 
but as it seems to agree with the spec, I thought I'd pass it along.

Rogers

Re: isPostback patch for Trinidad 1.2 state manager

[Adam Winer <aw...@gmail.com>]

Thanks!  I'll get that in ASAP.  And I also need to change
our state parameter from org.apache.myfaces.trinidad.faces.STATE
to the spec-standardized parameter.

-- Adam


On 10/9/06, Rogers Reilly <ro...@s-rod.com> wrote:
> Hi,
>
> I'm porting a Trinidad webapp over to 1.2 (with the help of Adam's great
> new 1.2 build- thnx!), and was having trouble using Shale remoting.
> Specifically, a Shale remoting request with parameters was causing
> Trinidad 1.2 to bomb out, throw an NPE when finding the ViewRoot.   I
> tracked down the source of the issue:  Trinidad's
> CoreResponseStateManager was telling RestoreView that the request was a
> postback, and then when it looked for the state parameter
> ("org.apache.myfaces.trinidad.faces.STATE"), it couldn't find it.  The
> ViewRoot is then NULL.
>
> (I actually tried to pass the state along as a param in the Remoting
> request, but it didn't quite work- the restored viewId corresponds to
> the base page URL, so the Shale PhaseListener passes, doesn't realize
> it's supposed to take it.)
>
> Anyway, looking into the code, I found that CoreResponseStateManger
> doesn't implement .isPostback(FacesContext); it just lets its parent's
> implementation stand- a default implementation that just returns TRUE if
> the request has any parameters at all, meant for old applications.
> Here's how the spec says this method should be implemented:
>
> "The implementation if this method for the Standard HTML RenderKit must
> consult the javax.faces.context.ExternalContext's requestParameterMap
> and return true if and only if there is a key equal to the value of the
> symbolic constant VIEW_STATE_PARAM."
>
> Which is easy enough to do.  I just grabbed the implementation from RI's
> ResponseStateManagerImpl, tweaked the constant, dropped it into
> CoreReponseStateManager, and rebuilt.  + this code:
>
>  @Override
>   public boolean isPostback(FacesContext context) {
>
>         return context.getExternalContext().getRequestParameterMap().
>               containsKey(_STATE_FIELD_NAME);
>
>   }
>
>
> Not sure what the other effects of implementing this function will be,
> but as it seems to agree with the spec, I thought I'd pass it along.
>
> Rogers
>
>