admin@glassfish.java.net

Re: Shale Issue #360

From: Craig McClanahan <Craig.McClanahan_at_Sun.COM>
Date: Fri, 13 Apr 2007 12:46:43 -0700

Ken Paulsen wrote:
>
> Hi Craig,
>
> Good news / bad news. :)
>
> Good news: this fixed the problem reported in issue #360.
>
> Bad news: I found another issue.
>
> This one has nothing to do w/ our ViewHandler. In order to increase
> startup performance, we took Ryan's suggestion to wrap the
> FacesServlet. Shale Remoting looks for the FacesServlet, can't find
> it and throws a NPE on line #429 of MappingsHelper.java:
>
> if (name.equals(servletName)) {
>
> I see that I can work-a-round this issue by supplying a context-param,
> I tested this... it works. However, since the SWDP may impose shale
> on all apps, shale should not require apps that do this sort of thing
> to implement work-a-rounds. So I tested a fix that avoids the NPE, it
> also works. I also modified an existing log message in the
> Bundle.properies file to make it more clear (which interestingly did
> not seem to be used anywhere).
>
> I'll file an issue for this, can you review and commit these changes?
>
Your fix looks reasonable ... go ahead and create an issue, and I'll get
this in today.

Ed also showed me the changes he made in the copy of Shale Remoting that
is included with Dynamic Faces, which basically turn Remoting off by
default. We definitely need some sort of strategy for this, to cover
the case where the app server provides the library in a shared parent
classloader ... the interesting part of the challenge is figuring out
how to do the defaulting. Maybe default to "on" if we are loaded from
the app's class loader directly, or "off" if loaded from a parent class
loader, or something like that.

At any rate, I'd like to get a solution for that into the standard
codebase as well, so Ed doesn't have to fork either.
> Thanks!
>
> Ken
>
> Index: MappingsHelper.java
> ===================================================================
> --- MappingsHelper.java (revision 528551)
> +++ MappingsHelper.java (working copy)
> @@ -408,6 +408,12 @@
> name = servletName;
> }
> }
> + if (null == name) {
> + if (log().isInfoEnabled()) {
> +
> log().info(bundle.getString("xhtml.noServletMapping"));
> + }
> + return new String[0];
> + }
> }
>
> // Identify the URL patterns to which this servlet is mapped
>
>
>
> Index: Bundle_no.properties
> ===================================================================
> --- Bundle_no.properties (revision 528618)
> +++ Bundle_no.properties (working copy)
> @@ -31,4 +31,4 @@
> xhtml.noMappings = "Mappings" instansen for denne applikasjonen
> er ikke konfigurert enn\u00E5 - sjekk tjener loggene for \u00E5
> forsikre deg om at det ikke var oppstarts unntak.
> xhtml.noMechanism = Ressurs mekanismen du har spesifisert er tom
> xhtml.noResourceId = Ressurs identifikatoren du spesifiserte er tom
> -xhtml.noServletMapping = "Mappings" instansen er ikke konfigurert for
> servlet tilknyttning(er) for javax.facesl.webapp.FacesServlet -- sjekk
> web.xml filen for \u00E5 forsikre deg om at denne servlet er deklarert
> og tilknyttet.
> +xhtml.noServletMapping = "Mappings" instansen er ikke konfigurert for
> servlet tilknyttning(er) for javax.faces.webapp.FacesServlet -- sjekk
> web.xml filen for \u00E5 forsikre deg om at denne servlet er deklarert
> og tilknyttet.
> Index: Bundle.properties
> ===================================================================
> --- Bundle.properties (revision 528618)
> +++ Bundle.properties (working copy)
> @@ -25,4 +25,4 @@
> xhtml.noMappings=The "Mappings" instance for this application has not
> yet been configured -- check your server logs to ensure there were no
> startup exceptions
> xhtml.noMechanism=The resource mechanism you have specified is null
> xhtml.noResourceId=The resource identifier you have specified is null
> -xhtml.noServletMapping=The "Mappings" instance is not configured with
> the servlet mapping(s) for javax.facesl.webapp.FacesServlet -- check
> your web.xml file to make sure this servlet is declared and mapped
> +xhtml.noServletMapping=The "Mappings" instance is not configured with
> the servlet mapping(s) for javax.faces.webapp.FacesServlet -- check
> your web.xml file to make sure this Servlet is declared and mapped, or
> that you have identified it with the
> "org.apache.shale.remoting.FACES_SERVLET_NAME" context-param.
>
>
>
>
> Craig McClanahan wrote:
>> Ken Paulsen wrote:
>>>
>>> Hi Craig,
>>>
>>> I believe this issue still isn't resolved. I also think the fix is
>>> very simple, shale needs to check to see if the response is complete
>>> before doing it's processing. Here is a proposed fix:
>>>
>> Fixed in the trunk (thanks for the reminder). Can you verify with
>> tonight's nightly build (or just build it yourself if you're using
>> source)?
>>
>> Craig
>>> Index: RemotingPhaseListener.java
>>> ===================================================================
>>> --- RemotingPhaseListener.java (revision 528221)
>>> +++ RemotingPhaseListener.java (working copy)
>>> @@ -94,6 +94,12 @@
>>>
>>> // Acquire a reference to the FacesContext for this request
>>> FacesContext context = event.getFacesContext();
>>> +
>>> + // Make sure the request isn't already complete
>>> + if (context.getResponseComplete()) {
>>> + return;
>>> + }
>>> +
>>> if (log().isDebugEnabled()) {
>>> log().debug("Checking view identifier '" +
>>> context.getViewRoot().getViewId() + "'");
>>> }
>>>
>>>
>>>
>>> Can you please add this to RemotingPhaseListener.java?
>>>
>>> Thanks!
>>>
>>> Ken
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>