MyFaces Tomahawk
  1. MyFaces Tomahawk
  2. TOMAHAWK-1331

ExternalContextUtils.getRequestType() doesn't work in a portlet environment

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.7
    • Fix Version/s: 1.1.8
    • Component/s: Portlet_Support
    • Labels:
      None
    • Environment:
      WebSphere Portal 6.0
      MyFaces 1.1.6
      Tomahawk 1.1.7
      Portals Bridges 1.0.4

      Description

      ExternalContextUtils.getRequestType(Object config, Object request) signature doesn't correspond to what's used in TomahawkFacesContextFactory.getFacesContext(): The called method requires a config and a request while it's invoked using a context and a request.
      ==> This always leads to a ClassCastException at TomahawkFacesContextFactory.getFacesContext()#64.

      The invoked method seems to have to be fixed something like this:
      class ExternalContextUtils:
      public static final RequestType getRequestType(Object context, Object request) {
      if(_PORTLET_CONTEXT_CLASS != null)
      {
      if (_PORTLET_CONTEXT_CLASS.isInstance(context))
      {
      // blablabla
      }

      1. TOMAHAWK-1331-v2.patch
        3 kB
        Leonardo Uribe
      2. TOMAHAWK-1331.patch
        2 kB
        Leonardo Uribe
      3. TOMAHAWK-1331.log
        38 kB
        Romain Seguy

        Issue Links

          Activity

          Hide
          Leonardo Uribe added a comment -

          Could you provide the full stack trace, so we can see in more deep what is the problem?

          Show
          Leonardo Uribe added a comment - Could you provide the full stack trace, so we can see in more deep what is the problem?
          Hide
          Scott O'Bryan added a comment -

          BTW- Is there any reason NOT to use commons for this?

          Show
          Scott O'Bryan added a comment - BTW- Is there any reason NOT to use commons for this?
          Hide
          Leonardo Uribe added a comment -

          commons 11 is jdk 1.5 compatible. Tomahawk is jdk 1.4, so I cannot add this dependency (and I did some modifications to this code to make it work on jdk 1.4).

          Show
          Leonardo Uribe added a comment - commons 11 is jdk 1.5 compatible. Tomahawk is jdk 1.4, so I cannot add this dependency (and I did some modifications to this code to make it work on jdk 1.4).
          Hide
          Leonardo Uribe added a comment -

          I'm testing it with pluto, myfaces portlet bridge and myfaces core 1.2 and works as expected.

          The code is fine. In this part of the code, we don't have an ExternalContext instance, so we have to test is based on the request and context object.

          It is supposed that on context var it receives a javax.portlet.PortletConfig instance. This method was added to tomahawk ExternalContextUtils (not present in commons 1.1 ExternalContextUtils version)

          Show
          Leonardo Uribe added a comment - I'm testing it with pluto, myfaces portlet bridge and myfaces core 1.2 and works as expected. The code is fine. In this part of the code, we don't have an ExternalContext instance, so we have to test is based on the request and context object. It is supposed that on context var it receives a javax.portlet.PortletConfig instance. This method was added to tomahawk ExternalContextUtils (not present in commons 1.1 ExternalContextUtils version)
          Hide
          Romain Seguy added a comment - - edited

          TOMAHAWK-1331.log: Log generated when the first render is performed on the portlet for the initial display.

          Tech env:

          • WebSphere Portal 6.0 (Pluto-based) on Java 1.4
          • MyFaces 1.1.6
          • Tomahawk 1.1.7
          • Portals Bridges 1.0.4

          web.xml extract:
          <context-param>
          <param-name>javax.faces.STATE_SAVING_METHOD</param-name>
          <param-value>server</param-value>
          </context-param>
          <context-param>
          <param-name>javax.faces.CONFIG_FILES</param-name>
          <param-value></param-value>
          </context-param>
          <context-param>
          <param-name>javax.faces.DEFAULT_SUFFIX</param-name>
          <param-value>.jsp</param-value>
          </context-param>
          <context-param>
          <param-name>javax.faces.PARTIAL_STATE_SAVING_METHOD</param-name>
          <param-value>false</param-value>
          </context-param>
          <context-param>
          <param-name>org.apache.myfaces.ALLOW_JAVASCRIPT</param-name>
          <param-value>true</param-value>
          </context-param>
          <context-param>
          <param-name>org.apache.myfaces.DETECT_JAVASCRIPT</param-name>
          <param-value>false</param-value>
          </context-param>
          <context-param>
          <param-name>org.apache.myfaces.PRETTY_HTML</param-name>
          <param-value>true</param-value>
          </context-param>
          <context-param>
          <param-name>org.apache.myfaces.AUTO_SCROLL</param-name>
          <param-value>true</param-value>
          </context-param>
          <context-param>
          <param-name>org.apache.myfaces.ADD_RESOURCE_CLASS</param-name>
          <param-value>org.apache.myfaces.renderkit.html.util.NonBufferingAddResource</param-value>
          </context-param>
          <context-param>
          <param-name>org.apache.myfaces.DISABLE_TOMAHAWK_FACES_CONTEXT_WRAPPER</param-name>
          <param-value>true</param-value>
          </context-param>
          <filter>
          <description></description>
          <display-name>extensionsFilter</display-name>
          <filter-name>extensionsFilter</filter-name>
          <filter-class>org.apache.myfaces.webapp.filter.ExtensionsFilter</filter-class>
          <init-param>
          <description></description>
          <param-name>uploadMaxFileSize</param-name>
          <param-value>100m</param-value>
          </init-param>
          <init-param>
          <description></description>
          <param-name>uploadThresholdSize</param-name>
          <param-value>100k</param-value>
          </init-param>
          </filter>
          <listener>
          <listener-class>org.apache.myfaces.webapp.StartupServletContextListener</listener-class>
          </listener>
          <servlet>
          <description></description>
          <display-name>Faces Servlet</display-name>
          <servlet-name>Faces Servlet</servlet-name>
          <servlet-class>javax.faces.webapp.FacesServlet</servlet-class>
          </servlet>

          faces-config.xml extract:
          <lifecycle>
          <phase-listener>org.apache.myfaces.webapp.filter.ServeResourcePhaseListener</phase-listener>
          </lifecycle>

          <factory>
          <faces-context-factory>org.apache.myfaces.webapp.filter.TomahawkFacesContextFactory</faces-context-factory>
          </factory>

          Show
          Romain Seguy added a comment - - edited TOMAHAWK-1331 .log: Log generated when the first render is performed on the portlet for the initial display. Tech env: WebSphere Portal 6.0 (Pluto-based) on Java 1.4 MyFaces 1.1.6 Tomahawk 1.1.7 Portals Bridges 1.0.4 web.xml extract: <context-param> <param-name>javax.faces.STATE_SAVING_METHOD</param-name> <param-value>server</param-value> </context-param> <context-param> <param-name>javax.faces.CONFIG_FILES</param-name> <param-value></param-value> </context-param> <context-param> <param-name>javax.faces.DEFAULT_SUFFIX</param-name> <param-value>.jsp</param-value> </context-param> <context-param> <param-name>javax.faces.PARTIAL_STATE_SAVING_METHOD</param-name> <param-value>false</param-value> </context-param> <context-param> <param-name>org.apache.myfaces.ALLOW_JAVASCRIPT</param-name> <param-value>true</param-value> </context-param> <context-param> <param-name>org.apache.myfaces.DETECT_JAVASCRIPT</param-name> <param-value>false</param-value> </context-param> <context-param> <param-name>org.apache.myfaces.PRETTY_HTML</param-name> <param-value>true</param-value> </context-param> <context-param> <param-name>org.apache.myfaces.AUTO_SCROLL</param-name> <param-value>true</param-value> </context-param> <context-param> <param-name>org.apache.myfaces.ADD_RESOURCE_CLASS</param-name> <param-value>org.apache.myfaces.renderkit.html.util.NonBufferingAddResource</param-value> </context-param> <context-param> <param-name>org.apache.myfaces.DISABLE_TOMAHAWK_FACES_CONTEXT_WRAPPER</param-name> <param-value>true</param-value> </context-param> <filter> <description></description> <display-name>extensionsFilter</display-name> <filter-name>extensionsFilter</filter-name> <filter-class>org.apache.myfaces.webapp.filter.ExtensionsFilter</filter-class> <init-param> <description></description> <param-name>uploadMaxFileSize</param-name> <param-value>100m</param-value> </init-param> <init-param> <description></description> <param-name>uploadThresholdSize</param-name> <param-value>100k</param-value> </init-param> </filter> <listener> <listener-class>org.apache.myfaces.webapp.StartupServletContextListener</listener-class> </listener> <servlet> <description></description> <display-name>Faces Servlet</display-name> <servlet-name>Faces Servlet</servlet-name> <servlet-class>javax.faces.webapp.FacesServlet</servlet-class> </servlet> faces-config.xml extract: <lifecycle> <phase-listener>org.apache.myfaces.webapp.filter.ServeResourcePhaseListener</phase-listener> </lifecycle> <factory> <faces-context-factory>org.apache.myfaces.webapp.filter.TomahawkFacesContextFactory</faces-context-factory> </factory>
          Hide
          Leonardo Uribe added a comment -

          Thanks a lot for the log, it helps a lot to understand it.

          After looking in deep the code, it seem that TomahawkFacesContextFactory method:

          public FacesContext getFacesContext(Object context, Object request, Object response, Lifecycle lifecycle) throws FacesException

          The context variable is in some configuration an instance of javax.portlet.PortletConfig and in others (I checked jsf 1.1 MyfacesGenericPortlet and the stack trace provided show this) javax.portlet.PortletContext.

          The more user friendly is assume to receive any of the two classes in that var (checking if this var is instance of javax.portlet.PortletConfig or javax.portlet.PortletContext). In any case ExternalContext.getContext() only retrieves a javax.portlet.PortletContext instance, but in this case we don't have a instance of this class yet, so anyway we need an alternative to detect if a request is in portlets or not.

          I have attached a patch. It could be good to have feedback about it. If not, I'll commit the solution and close the issue soon

          Show
          Leonardo Uribe added a comment - Thanks a lot for the log, it helps a lot to understand it. After looking in deep the code, it seem that TomahawkFacesContextFactory method: public FacesContext getFacesContext(Object context, Object request, Object response, Lifecycle lifecycle) throws FacesException The context variable is in some configuration an instance of javax.portlet.PortletConfig and in others (I checked jsf 1.1 MyfacesGenericPortlet and the stack trace provided show this) javax.portlet.PortletContext. The more user friendly is assume to receive any of the two classes in that var (checking if this var is instance of javax.portlet.PortletConfig or javax.portlet.PortletContext). In any case ExternalContext.getContext() only retrieves a javax.portlet.PortletContext instance, but in this case we don't have a instance of this class yet, so anyway we need an alternative to detect if a request is in portlets or not. I have attached a patch. It could be good to have feedback about it. If not, I'll commit the solution and close the issue soon
          Hide
          Romain Seguy added a comment -

          I've sucessfully tested the patch on my env. Works fine. Thanks for it.

          Show
          Romain Seguy added a comment - I've sucessfully tested the patch on my env. Works fine. Thanks for it.
          Hide
          Scott O'Bryan added a comment -

          FYI, I think the 1.1 bridge is in error. getContext should ALWAYS return a PortletContext. If it returns a PortletConfig then we should probably fix this at the bridge level. I know JSR-301 always returns the PortletContext which is why you are not seeing this issue.

          Scott

          Show
          Scott O'Bryan added a comment - FYI, I think the 1.1 bridge is in error. getContext should ALWAYS return a PortletContext. If it returns a PortletConfig then we should probably fix this at the bridge level. I know JSR-301 always returns the PortletContext which is why you are not seeing this issue. Scott
          Hide
          Leonardo Uribe added a comment -

          The method getContext is ok. The behavior that causes problem is this:

          On MyfacesGenericPortlet (myfaces-impl 1.1) and the portlet bridge used by Romain, the FacesContext instance for portlets is created in this way

          protected FacesContext facesContext(PortletRequest request,
          PortletResponse response)

          { return facesContextFactory.getFacesContext(portletContext, request, response, lifecycle); }

          But on myfaces portlet bridge (for jsf 1.2) do this (org.apache.myfaces.portlet.faces.bridge.BridgeImpl line 240):

          try
          {
          // Get the FacesContext instance for this request
          context =
          getFacesContextFactory().getFacesContext(mPortletConfig, request, response, getLifecycle());

          both return on getContext() correctly, but since the changes was tested against portlet bridge, the first case was not covered.

          On TomahawkFacesContextFactory, we need a way to know when the request comes from portlets or not, but we don't have an instance of ExternalContext yet, so the original method ExternalContextUtils.getRequestType(ExternalContext externalContext) can't be used.

          Show
          Leonardo Uribe added a comment - The method getContext is ok. The behavior that causes problem is this: On MyfacesGenericPortlet (myfaces-impl 1.1) and the portlet bridge used by Romain, the FacesContext instance for portlets is created in this way protected FacesContext facesContext(PortletRequest request, PortletResponse response) { return facesContextFactory.getFacesContext(portletContext, request, response, lifecycle); } But on myfaces portlet bridge (for jsf 1.2) do this (org.apache.myfaces.portlet.faces.bridge.BridgeImpl line 240): try { // Get the FacesContext instance for this request context = getFacesContextFactory().getFacesContext(mPortletConfig, request, response, getLifecycle()); both return on getContext() correctly, but since the changes was tested against portlet bridge, the first case was not covered. On TomahawkFacesContextFactory, we need a way to know when the request comes from portlets or not, but we don't have an instance of ExternalContext yet, so the original method ExternalContextUtils.getRequestType(ExternalContext externalContext) can't be used.
          Hide
          Scott O'Bryan added a comment -

          This is the bridge currently included with the MyFaces core 1.2, not the JSR-301 bridge from the Apache MyFaces Portlet-Bridge project, correct? If so, this bridge is totally unsupported (and IMO should be ripped out). The Portal solution for 1.2 going forward should be the 301 bridge. If you are coding to the proprietary bridge in 1.2, then you are neither testing a proper 1.1 bridge environment OR a proper 1.2 bridge environment.

          In either case. The FacesContextFactory.getFacesContext needs to be given a PortletContext, never a portlet config. Whoever did this is violating a JSF contract for the Factory.

          Show
          Scott O'Bryan added a comment - This is the bridge currently included with the MyFaces core 1.2, not the JSR-301 bridge from the Apache MyFaces Portlet-Bridge project, correct? If so, this bridge is totally unsupported (and IMO should be ripped out). The Portal solution for 1.2 going forward should be the 301 bridge. If you are coding to the proprietary bridge in 1.2, then you are neither testing a proper 1.1 bridge environment OR a proper 1.2 bridge environment. In either case. The FacesContextFactory.getFacesContext needs to be given a PortletContext, never a portlet config. Whoever did this is violating a JSF contract for the Factory.
          Hide
          Leonardo Uribe added a comment -

          It is the JSR-301 bridge from the Apache MyFaces Portlet-Bridge project. Yes, I know that the MyfacesGenericPortlet inside myfaces core 1.2 is unsupported (in fact does not work because some changes in ViewHandler from jsf 1.1 to jsf 1.2 became this code obsolete) and should be removed in jsf 2.0 branch (part of the cleanup planed in that field).

          So the code of the JSR-301 Apache MyFaces Portlet-Bridge project is the wrong one. Scott, should we create an issue for that one in portlet bridge jira?

          Show
          Leonardo Uribe added a comment - It is the JSR-301 bridge from the Apache MyFaces Portlet-Bridge project. Yes, I know that the MyfacesGenericPortlet inside myfaces core 1.2 is unsupported (in fact does not work because some changes in ViewHandler from jsf 1.1 to jsf 1.2 became this code obsolete) and should be removed in jsf 2.0 branch (part of the cleanup planed in that field). So the code of the JSR-301 Apache MyFaces Portlet-Bridge project is the wrong one. Scott, should we create an issue for that one in portlet bridge jira?
          Hide
          Scott O'Bryan added a comment -

          Well pppttt. Yeah, that's a problem. Right now there are a number of them. But I agree, the context should be a context and AFAIK, we didn't spec that any differently. If you could write up a jira for that and mark it as high priority so that it get's fixed in alpha-3, I would be much obliged.

          Thanks Leonardo, and sorry for the mixup, I have like 20 pokers in the fire ATM.

          Show
          Scott O'Bryan added a comment - Well pppttt. Yeah, that's a problem. Right now there are a number of them. But I agree, the context should be a context and AFAIK, we didn't spec that any differently. If you could write up a jira for that and mark it as high priority so that it get's fixed in alpha-3, I would be much obliged. Thanks Leonardo, and sorry for the mixup, I have like 20 pokers in the fire ATM.
          Hide
          Leonardo Uribe added a comment -

          This patch is the variant according to Scott information about it, but before commit this stuff it could be good to be PORTLETBRIDGE-47 solved.

          Show
          Leonardo Uribe added a comment - This patch is the variant according to Scott information about it, but before commit this stuff it could be good to be PORTLETBRIDGE-47 solved.
          Hide
          Leonardo Uribe added a comment -

          I have committed the solution 1 provisionally as compatibility to current portlet bridge code, but this issue will be closed with option 2 as soon as PORTLETBRIDGE-47 is solved.

          Show
          Leonardo Uribe added a comment - I have committed the solution 1 provisionally as compatibility to current portlet bridge code, but this issue will be closed with option 2 as soon as PORTLETBRIDGE-47 is solved.
          Hide
          Leonardo Uribe added a comment -

          PortletBridge issue was solved, but since this one does not harm or change any expected behavior, I'll close this one as fixed on version 1.1.8

          Show
          Leonardo Uribe added a comment - PortletBridge issue was solved, but since this one does not harm or change any expected behavior, I'll close this one as fixed on version 1.1.8

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Romain Seguy
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development