Spring Web Flow
  1. Spring Web Flow
  2. SWF-939

Locating of flow definitions broken when base-path not being used

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 2.0.5
    • Component/s: Core: Flow Executor
    • Labels:
      None

      Description

      A previously functional SWF 2.0.3 based application has stopped working after upgrading to SWF 2.0.4
      Flow definitions are no longer being found due to the change re: using flow definition path as the flow id, rather than the flow definition file name.

      <webflow:flow-executor id="flowExecutor" flow-registry="flowRegistry">
      <webflow:flow-execution-repository max-executions="1" max-execution-snapshots="30" />
      <webflow:flow-execution-listeners>
      <webflow:listener ref="securityFlowExecutionListener" />
      </webflow:flow-execution-listeners>
      </webflow:flow-executor>

      <!-- The registry of executable flow definitions -->
      <webflow:flow-registry id="flowRegistry" flow-builder-services="facesFlowBuilderServices">
      <webflow:flow-location-pattern value="/WEB-INF/flow/*/.xml"/>
      </webflow:flow-registry>

      <!-- Configures the Spring Web Flow JSF integration -->
      <faces:flow-builder-services id="facesFlowBuilderServices" />

      The flow definition located at /WEB-INF/flow/login/login.xml gets stored in the flowDefinitionRegistry
      with an id of "login" in both SWF 2.0.3 and 2.0.4.

      However, when attempting to request the flow - SWF 2.0.3 looks for "login" in the registry,
      where as SWF 2.0.4 looks for "flow/login" and fails to find it.

      I believe this is due to a shift in approach to wishing to use the flow definition's path as the flow id, rather than the flow name.
      The piece of code for determining the flow id when a request comes in is DefaultFlowUrlHandler.getFlowId().

      SWF 2.0.3 implementation returns "login" as the id -

      public String getFlowId(HttpServletRequest request) {
      String pathInfo = request.getPathInfo();
      if (pathInfo != null)

      { return pathInfo.substring(1); }

      else

      { . . . }

      }

      SWF 2.0.4 implementation returns "flow/login" as the id -

      public String getFlowId(HttpServletRequest request)

      { return WebUtils.extractFilenameFromUrlPath(urlPathHelper.getLookupPathForRequest(request)); }

      As SWF 2.0.4 is a point release hopefully it shouldn't be breaking existing applications.

        Activity

        Hide
        Nick Rountree added a comment -

        Note that the SWF code excerpts above are back-to-front.

        Show
        Nick Rountree added a comment - Note that the SWF code excerpts above are back-to-front.
        Hide
        Keith Donald added a comment -

        Nick. This was a tough call for us, and one I went back and forth on. I would definitely prefer not to introduce such changes in point releases, but we felt that the existing behavior was not ideal . We ultimately considered the previous 2.0.x behavior a bug / gap since it could simply not support nested flow paths, for example: /air/booking and /hotel/booking would simply both map to flow id 'booking' then, which obviously breaks down.

        Separately, mapping flows to URLs manually resulted in too much redundancy in configuration. We sought to address this in the 2.0.x scope from the outset--we introduced FlowHandlerAdapter in 2.0.0 with the goal to address this but unfortunately didn't complete the job until FlowHandlerMapping was introduced in 2.0.4 [which is now recommended as a superior alternative to the old FlowController].

        We also considered going to 2.1 with this change, but in the end decided it was best to complete 2.0.x and classify this as "closing the last gap" in Web Flow 2.0.x.

        We were expecting the production using nested URL paths to their flows to be fairly small at this point, versus the number invoking flows at top-level URLs e.g. /login, which would not be effected by this change. We may have underestimated that.

        How are you working around this issue? More insight and suggestions from your side is appreciated.

        Show
        Keith Donald added a comment - Nick. This was a tough call for us, and one I went back and forth on. I would definitely prefer not to introduce such changes in point releases, but we felt that the existing behavior was not ideal . We ultimately considered the previous 2.0.x behavior a bug / gap since it could simply not support nested flow paths, for example: /air/booking and /hotel/booking would simply both map to flow id 'booking' then, which obviously breaks down. Separately, mapping flows to URLs manually resulted in too much redundancy in configuration. We sought to address this in the 2.0.x scope from the outset--we introduced FlowHandlerAdapter in 2.0.0 with the goal to address this but unfortunately didn't complete the job until FlowHandlerMapping was introduced in 2.0.4 [which is now recommended as a superior alternative to the old FlowController] . We also considered going to 2.1 with this change, but in the end decided it was best to complete 2.0.x and classify this as "closing the last gap" in Web Flow 2.0.x. We were expecting the production using nested URL paths to their flows to be fairly small at this point, versus the number invoking flows at top-level URLs e.g. /login, which would not be effected by this change. We may have underestimated that. How are you working around this issue? More insight and suggestions from your side is appreciated.
        Hide
        Keith Donald added a comment -

        Will document compatibility issue in 2.0.5 and note how to restore earlier 2.0.x behavior.

        Show
        Keith Donald added a comment - Will document compatibility issue in 2.0.5 and note how to restore earlier 2.0.x behavior.
        Hide
        Keith Donald added a comment -

        Considering adding "useFullRequestPathAsFlowId" property to DefaultFlowUrlHandler, with value=false when used with FlowController and value=true for FlowHandlerAdapter/FlowHandlerMapping. Since it is expected most existing web flow applications use the FlowController, this will have the positive benefit of not breaking them. Those using the handlermapping/adapter can set the flag to false explicitly if they need the old behavior. Sound acceptable?

        Show
        Keith Donald added a comment - Considering adding "useFullRequestPathAsFlowId" property to DefaultFlowUrlHandler, with value=false when used with FlowController and value=true for FlowHandlerAdapter/FlowHandlerMapping. Since it is expected most existing web flow applications use the FlowController, this will have the positive benefit of not breaking them. Those using the handlermapping/adapter can set the flag to false explicitly if they need the old behavior. Sound acceptable?
        Hide
        Agim Emruli added a comment -

        Introduced org.springframework.webflow.context.servlet.FilenameFlowUrlHandler that is the default implementation of the FlowController. This implementation handles all the URL like Web Flow 2.0.3 does.

        Web Flow 2.0.3 users will not be affected by the URL change if using the FlowController.

        Show
        Agim Emruli added a comment - Introduced org.springframework.webflow.context.servlet.FilenameFlowUrlHandler that is the default implementation of the FlowController. This implementation handles all the URL like Web Flow 2.0.3 does. Web Flow 2.0.3 users will not be affected by the URL change if using the FlowController.

          People

          • Assignee:
            Agim Emruli
            Reporter:
            Nick Rountree
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1d
              1d
              Remaining:
              Remaining Estimate - 1d
              1d
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development