Tapestry
  1. Tapestry
  2. TAPESTRY-1627

Start page redirect requests can return an empty response.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.5
    • Fix Version/s: 5.0.7
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      A Start page redirect request will produce an empty response if the request URL was root, e.g: http://localhost:8080/myapp/
      It will succeed only if the URL maps to the Start page, eg: http://localhost:8080/myapp/start

      It looks like just a couple of lines need to be (changed/added) in org.apache.tapestry.internal.services.RootPathDispatcher.dispatch()
      [Marked by -/+ below.]

      public boolean dispatch(Request request, final Response response) throws IOException
      {
      // Only match the root path

      if (!request.getPath().equals("/")) return false;

      if (_componentClassResolver.isPageName(_startPageName))

      { - _handler.handle(_startPageName, _emptyContext); + ActionResponseGenerator responseGenerator = _handler.handle(pageName, context); + if (responseGenerator != null) responseGenerator.sendClientResponse(response); return true; }

      return false;
      }

      Cheers,
      Nick.

        Issue Links

          Activity

          Hide
          Nick Westgate added a comment -

          Just to clarify, by redirect request I mean via onActivate() in Start.java, e.g:

          Object onActivate()

          { return "MyPage"; }

          Cheers,
          Nick.

          Show
          Nick Westgate added a comment - Just to clarify, by redirect request I mean via onActivate() in Start.java, e.g: Object onActivate() { return "MyPage"; } Cheers, Nick.
          Hide
          Nick Westgate added a comment -

          I've tested this fix now and it seems to work ok.
          Fixing copy & pasted variable names above, it is:

          • _handler.handle(_startPageName, _emptyContext);
            + ActionResponseGenerator responseGenerator = _handler.handle(_startPageName, _emptyContext);

          + if (responseGenerator != null) responseGenerator.sendClientResponse(response);

          Cheers,
          Nick.

          Show
          Nick Westgate added a comment - I've tested this fix now and it seems to work ok. Fixing copy & pasted variable names above, it is: _handler.handle(_startPageName, _emptyContext); + ActionResponseGenerator responseGenerator = _handler.handle(_startPageName, _emptyContext); + if (responseGenerator != null) responseGenerator.sendClientResponse(response); Cheers, Nick.
          Hide
          Nick Westgate added a comment -

          Well, here's a 3 line patch!

          I had a go at adding a test for this, but the redirect makes it tough.
          You can't use a test with a context, because the RootPathDispatcher won't catch that.
          It might be easier in app2, or require another test app entirely.
          It also might not be worth testing it at all!

          For the record, I tried to use an ASO in the Start page of app1 as follows:

          import org.apache.tapestry.integration.app1.SimpleASO;

          /**

          • Have to start somewhere!
            */
            public class Start
            {
            @ApplicationState
            private SimpleASO _aso;
            private boolean _asoExists;

          Object onActivate()
          {
          if (!_asoExists)

          { // test redirect handling from root page _aso = new SimpleASO(); return this; }

          return null;
          }
          }

          The result if broken:
          FAILURE!
          com.thoughtworks.selenium.SeleniumException: ERROR: Element link=Expansion Page not found

          The result if fixed:
          FAILURE!
          crushing_number_of_threads_at_startup(org.apache.tapestry.integration.LaunchCrusherTest)

          Cheers,
          Nick.

          Show
          Nick Westgate added a comment - Well, here's a 3 line patch! I had a go at adding a test for this, but the redirect makes it tough. You can't use a test with a context, because the RootPathDispatcher won't catch that. It might be easier in app2, or require another test app entirely. It also might not be worth testing it at all! For the record, I tried to use an ASO in the Start page of app1 as follows: import org.apache.tapestry.integration.app1.SimpleASO; /** Have to start somewhere! */ public class Start { @ApplicationState private SimpleASO _aso; private boolean _asoExists; Object onActivate() { if (!_asoExists) { // test redirect handling from root page _aso = new SimpleASO(); return this; } return null; } } The result if broken: FAILURE! com.thoughtworks.selenium.SeleniumException: ERROR: Element link=Expansion Page not found The result if fixed: FAILURE! crushing_number_of_threads_at_startup(org.apache.tapestry.integration.LaunchCrusherTest) Cheers, Nick.
          Hide
          Nick Westgate added a comment -

          I added a runtime patch for this here:
          http://wiki.apache.org/tapestry/Tapestry5RootPathEmptyResponse

          Cheers,
          Nick.

          Show
          Nick Westgate added a comment - I added a runtime patch for this here: http://wiki.apache.org/tapestry/Tapestry5RootPathEmptyResponse Cheers, Nick.
          Hide
          Howard M. Lewis Ship added a comment -

          This may already be fixed, as a consequence of the changes that eliminated ActionResponseGenerator.

          I'll be creating a app3 for testing this!

          Show
          Howard M. Lewis Ship added a comment - This may already be fixed, as a consequence of the changes that eliminated ActionResponseGenerator. I'll be creating a app3 for testing this!
          Hide
          Howard M. Lewis Ship added a comment -

          Again, I think this was inadvertently fixed along with the other simplifications when stripping out ActionResponseGenerator. I've added a new integration test to ensure that it is, in fact, fixed.

          Show
          Howard M. Lewis Ship added a comment - Again, I think this was inadvertently fixed along with the other simplifications when stripping out ActionResponseGenerator. I've added a new integration test to ensure that it is, in fact, fixed.

            People

            • Assignee:
              Howard M. Lewis Ship
              Reporter:
              Nick Westgate
            • Votes:
              8 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development