admin@glassfish.java.net

Re: Shale Issue #360

From: Ken Paulsen <Ken.Paulsen_at_Sun.COM>
Date: Fri, 13 Apr 2007 13:21:55 -0700

This is true...

Thanks!

Ken

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