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

ViewState.resume should check externalcontext.isResponseComplete before rendering or requesting redirect

    Details

      Description

      There are cases when a user action may want to handle the http response manually, for example write directly to the response or call methods such as externalContext.requestExternalRedirect(...)

      I assume that the user should then call externalContext.recordResponseComplete() to notify the SWF engine that the response was handled.

      In these cases the View state should take into account whether recordResponseComplete has been called so that it does not try to handle the response itself. Specifically the resume( ) method should do someting like:

      if (!stateExited && context.getExternalContext( ).isResponseAllowed( ) && context.getExternalContext( ).isResponseComplete()) {
      ....
      }

      What is missing is the last check: context.getExternalContext( ).isResponseComplete( )

      Also, I noticed that FlowHandlerAdapter executes the requested redirects without checking if response.isCommited(). Shouldn't this check be present as well, just in case?

        Issue Links

          Activity

          Hide
          Nazaret Kazarian added a comment -

          If I am not mistaken similary functionality was provided in SWF 1 with the ability to define a null view (ViewSelection.NULL_VIEW)

          Show
          Nazaret Kazarian added a comment - If I am not mistaken similary functionality was provided in SWF 1 with the ability to define a null view (ViewSelection.NULL_VIEW)
          Hide
          Keith Donald added a comment -

          Thanks for this report.

          ViewState and EndState now both test if they 'canSendResponse' before rendering their content:

          private boolean canSendResponse(ExternalContext context)

          { return context.isResponseAllowed() && !context.isResponseComplete(); }
          Show
          Keith Donald added a comment - Thanks for this report. ViewState and EndState now both test if they 'canSendResponse' before rendering their content: private boolean canSendResponse(ExternalContext context) { return context.isResponseAllowed() && !context.isResponseComplete(); }
          Hide
          Nazaret Kazarian added a comment -

          There is still a slight problem, the flashScope is not cleared when the user handles the response and this causes problems to the next request. What is the correct approach? Should the user explicitly clear the flash scope or should the framework clear it somewhere? What about the messageContext? Should the messages be cleared as well? Because I see ViewState clears them explicitly after rendering. (I don't know exactly why since they are part of the flash scope). Thanks!

          Show
          Nazaret Kazarian added a comment - There is still a slight problem, the flashScope is not cleared when the user handles the response and this causes problems to the next request. What is the correct approach? Should the user explicitly clear the flash scope or should the framework clear it somewhere? What about the messageContext? Should the messages be cleared as well? Because I see ViewState clears them explicitly after rendering. (I don't know exactly why since they are part of the flash scope). Thanks!
          Hide
          Keith Donald added a comment -

          Framework should clear flash scope when pausing, arguably always, regardless of whether a ViewState view renders or not. good catch. We'll look into this.

          Show
          Keith Donald added a comment - Framework should clear flash scope when pausing, arguably always, regardless of whether a ViewState view renders or not. good catch. We'll look into this.
          Hide
          Keith Donald added a comment -

          I have committed a fix to this in 2.0.x maintenance for 2.0.7. Would you mind testing a nightly snapshot?

          Show
          Keith Donald added a comment - I have committed a fix to this in 2.0.x maintenance for 2.0.7. Would you mind testing a nightly snapshot?
          Hide
          Nazaret Kazarian added a comment -

          Ok, tested. Actually, in my use case the problem remains. I am doing the following: I am requesting an external redirect, which is loaded in a new browser window. When the user goes back to the previous window and tries to execute any other action, the flash scope has not been cleared and is causing problems to that request. I see that with the current fix, ViewState.clearFlashIfNotRedirecting( ) prevents the flash scope from being cleared when any kind of redirect is requested. Wouldn't it make more sense to clear flash "if not flow execution redirect requested". I think we could argue that flash scope is valid only across a flow execution redirect, so it should be cleared before flow definition and external redirects.

          Show
          Nazaret Kazarian added a comment - Ok, tested. Actually, in my use case the problem remains. I am doing the following: I am requesting an external redirect, which is loaded in a new browser window. When the user goes back to the previous window and tries to execute any other action, the flash scope has not been cleared and is causing problems to that request. I see that with the current fix, ViewState.clearFlashIfNotRedirecting( ) prevents the flash scope from being cleared when any kind of redirect is requested. Wouldn't it make more sense to clear flash "if not flow execution redirect requested". I think we could argue that flash scope is valid only across a flow execution redirect, so it should be cleared before flow definition and external redirects.
          Hide
          Keith Donald added a comment -

          Good catch... You can watch for the commit using fisheye at https://fisheye.springsource.org/browse/spring-webflow

          Show
          Keith Donald added a comment - Good catch... You can watch for the commit using fisheye at https://fisheye.springsource.org/browse/spring-webflow
          Hide
          Keith Donald added a comment -

          Fix committed. Is it resolved for your use case now?

          Show
          Keith Donald added a comment - Fix committed. Is it resolved for your use case now?
          Hide
          Nazaret Kazarian added a comment -

          Yes it is. Thanks!

          Show
          Nazaret Kazarian added a comment - Yes it is. Thanks!

            People

            • Assignee:
              Keith Donald
              Reporter:
              Nazaret Kazarian
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development