admin@glassfish.java.net

Re: Shale Issue #360

From: Craig McClanahan <Craig.McClanahan_at_Sun.COM>
Date: Fri, 13 Apr 2007 13:03:58 -0700

Ken Paulsen wrote:
>
> Yes, I agree changing the default depending on the ClassLoader would
> be a nice feature. Is there an easy cross-environment way to tell if
> you have a webapp classloader, though?
>
You always have a webapp classloader ... the container provides it as
the thread context class loader whenever it calls into application
code. So, something like attempting to load a class from the *parent*
of the webapp class loader ought to provide enough heuristic information
to make this determination.

> Ken
>
Craig

> Craig McClanahan wrote:
>> 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
>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>