ICEfaces
  1. ICEfaces
  2. ICE-4942

ResourceRegistry should allow registering resources without pre-pending the request context path

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.1
    • Fix Version/s: 1.8.2
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      n/a

      Description

      D2DViewHandler, getResourceURL(FacesContext context, String path) checks to see if the path contains a leading '/'. If so, it blindly appends the request context path, whether or not the given path already contains the context root path. This conflicts with the behaviour of the ResourceRegistry, which returns URLs that contain the root context path, and makes it difficult to use a URL derived from the ResourceRegistry on any component that first calls getResourceURL(..) to transform a path. We should first check if the given path argument contains the context path before appending, if it does, we should simply return the path unchanged, as we do if the path does not contain a leading '/'.

      suggested fix:

      Index: C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/application/D2DViewHandler.java
      ===================================================================
      --- C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/application/D2DViewHandler.java (revision 19267)
      +++ C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/application/D2DViewHandler.java (working copy)
      @@ -308,9 +308,9 @@
           }
       
           public String getResourceURL(FacesContext context, String path) {
      - ExternalContext extContext = context.getExternalContext();
      - if (path.startsWith("/")) {
      - return (extContext.getRequestContextPath() + path);
      + String requestContextPath = context.getExternalContext().getRequestContextPath();
      + if (path.startsWith("/") && !path.startsWith(requestContextPath)) {
      + return (requestContextPath + path);
               } else {
                   return path;
               }

        Activity

        Hide
        Philip Breau added a comment -

        tested on lastest build, seems ok

        Show
        Philip Breau added a comment - tested on lastest build, seems ok
        Hide
        Mircea Toma added a comment -

        Add methods for registering resources that return a path that considers the web-application context as root.

        Show
        Mircea Toma added a comment - Add methods for registering resources that return a path that considers the web-application context as root.
        Hide
        Deryk Sinotte added a comment -

        After clarifying the issue, we've decided to modify the interface and implementation to include the ability for the ResourceRegistry to provide resource URIs that do not include the application context.

        The typical use case for the ResourceRegistry was to provide resource URIs for inclusion things like CSS files where it was necessary to add the context dynamically. However, if the developer wanted to use the ResourceRegistry from a backing bean and get the URI for a resource having the context pre-pended is problematic. Because a component that used the URI from the bean would then send that URI to the ViewHandler to be resolved, it would result in the app context being pre-pended again.

        Show
        Deryk Sinotte added a comment - After clarifying the issue, we've decided to modify the interface and implementation to include the ability for the ResourceRegistry to provide resource URIs that do not include the application context. The typical use case for the ResourceRegistry was to provide resource URIs for inclusion things like CSS files where it was necessary to add the context dynamically. However, if the developer wanted to use the ResourceRegistry from a backing bean and get the URI for a resource having the context pre-pended is problematic. Because a component that used the URI from the bean would then send that URI to the ViewHandler to be resolved, it would result in the app context being pre-pended again.
        Hide
        Mircea Toma added a comment -

        I don't think that an application developer has any good reason for having webapp context name hardcoded in the JSPX files. I am marking this invalid.

        Show
        Mircea Toma added a comment - I don't think that an application developer has any good reason for having webapp context name hardcoded in the JSPX files. I am marking this invalid.
        Hide
        Mircea Toma added a comment - - edited

        The reason why all URI are supposed to be transformed by ViewHandler.getResourceURL method is to allow the change of webapp's name or move the pages within the application under a different folder without having to change anything in the page files.

        Show
        Mircea Toma added a comment - - edited The reason why all URI are supposed to be transformed by ViewHandler.getResourceURL method is to allow the change of webapp's name or move the pages within the application under a different folder without having to change anything in the page files.
        Hide
        Mircea Toma added a comment - - edited

        The provided test case does not exercise at all the ResourceRegistry functionality. ice:comandButton (in ICEfacesPage1.jspx) does not use ResourceRegistry for referencing the image of the button. Dynamic resources have their URL generated, there is no input from the application developer nor the component on how the URL should look like. This rules out any API changes we would need to make in the ResourceRegistry.

        Also, ice:commandButton uses directly ViewHandler.getResourceURL method to generate an absolute path to where the image can be found. In fact all JSF and ICEfaces components are expected to receive paths that will be transformed according to the Javadoc of ViewHandler.getResourceURL: http://java.sun.com/javaee/javaserverfaces/1.2/docs/api/javax/faces/application/ViewHandler.html#getResourceURL(javax.faces.context.FacesContext,%20java.lang.String)

        Show
        Mircea Toma added a comment - - edited The provided test case does not exercise at all the ResourceRegistry functionality. ice:comandButton (in ICEfacesPage1.jspx) does not use ResourceRegistry for referencing the image of the button. Dynamic resources have their URL generated, there is no input from the application developer nor the component on how the URL should look like. This rules out any API changes we would need to make in the ResourceRegistry. Also, ice:commandButton uses directly ViewHandler.getResourceURL method to generate an absolute path to where the image can be found. In fact all JSF and ICEfaces components are expected to receive paths that will be transformed according to the Javadoc of ViewHandler.getResourceURL: http://java.sun.com/javaee/javaserverfaces/1.2/docs/api/javax/faces/application/ViewHandler.html#getResourceURL(javax.faces.context.FacesContext,%20java.lang.String )
        Hide
        Philip Breau added a comment -

        suggested fix:

        Index: C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/ResourceRegistry.java
        ===================================================================
        — C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/ResourceRegistry.java (revision 19267)
        +++ C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/ResourceRegistry.java (working copy)
        @@ -67,6 +67,8 @@

        • @return the URI of the resource
          */
          URI registerResource(Resource resource);
          +
          + URI registerResourceWithRelativePath(Resource resource);

        /**

        • Register resource to be served. The URI is encoded using

        Index: C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/BridgeFacesContext.java
        ===================================================================
        — C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/BridgeFacesContext.java (revision 19267)
        +++ C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/BridgeFacesContext.java (working copy)
        @@ -941,4 +941,8 @@
        }
        return false;
        }
        +
        + public URI registerResourceWithRelativePath(Resource resource)

        { + return URI.create("." + resourceDispatcher.registerResource(resource, null).toString()); + }

        }

        Show
        Philip Breau added a comment - suggested fix: Index: C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/ResourceRegistry.java =================================================================== — C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/ResourceRegistry.java (revision 19267) +++ C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/ResourceRegistry.java (working copy) @@ -67,6 +67,8 @@ @return the URI of the resource */ URI registerResource(Resource resource); + + URI registerResourceWithRelativePath(Resource resource); /** Register resource to be served. The URI is encoded using Index: C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/BridgeFacesContext.java =================================================================== — C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/BridgeFacesContext.java (revision 19267) +++ C:/work/workspace/icefaces-trunk-ossrepo/core/src/com/icesoft/faces/context/BridgeFacesContext.java (working copy) @@ -941,4 +941,8 @@ } return false; } + + public URI registerResourceWithRelativePath(Resource resource) { + return URI.create("." + resourceDispatcher.registerResource(resource, null).toString()); + } }
        Hide
        Deryk Sinotte added a comment -

        Something along these lines seems reasonable. Assigning to Mircea.

        Show
        Deryk Sinotte added a comment - Something along these lines seems reasonable. Assigning to Mircea.
        Hide
        Philip Breau added a comment -

        Another option is simply to provide another method in ResourceRegistry that does not call the private BridgeFacesContext.resolve() method, which is prepending the request context path.

        ResourceRegistry:

        URI registerResourceWithRelativePath(Resource resource);

        BridgeFacesContext:

        public URI registerResourceWithRelativePath(Resource resource)

        { return URI.create(resourceDispatcher.registerResource(resource, null).toString()); }
        Show
        Philip Breau added a comment - Another option is simply to provide another method in ResourceRegistry that does not call the private BridgeFacesContext.resolve() method, which is prepending the request context path. ResourceRegistry: URI registerResourceWithRelativePath(Resource resource); BridgeFacesContext: public URI registerResourceWithRelativePath(Resource resource) { return URI.create(resourceDispatcher.registerResource(resource, null).toString()); }
        Hide
        Philip Breau added a comment -

        Simple test case with commandButton having an image attribute whose value includes the request context path

        Show
        Philip Breau added a comment - Simple test case with commandButton having an image attribute whose value includes the request context path

          People

          • Assignee:
            Mircea Toma
            Reporter:
            Philip Breau
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: