Wicket
  1. Wicket
  2. WICKET-5424

Page.isPageStateless() returning true in regular run but false in WicketTester

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.8.0, 6.12.0
    • Fix Version/s: 6.13.0
    • Component/s: None
    • Labels:
      None

      Description

      Motivation:
      Healthcheck/heartbeat pages must always be stateless to prevent significant amounts of session creation and storage.
      Also each anonymously accessed page in a public site should be stateless due to the same reason (otherwise the site could easily be DoSsed down).
      It would be nice to verify these requirements by tests.

      If I create an ought-to-be-stateless page with an AjaxLink which is hidden by a behavior:

      public class MyPage extends WebPage {
      public MyPage() {
      add(new AjaxLink<Void>("link") {
      @Override
      public void onClick(AjaxRequestTarget target)

      { // }

      }.add(new Behavior() {
      @Override
      public void onConfigure(Component c)

      { c.setVisible(false); }

      }));
      add(new Label("isPageStateless", new AbstractReadOnlyModel<Boolean>() {
      @Override
      public Boolean getObject()

      { return MyPage.this.isPageStateless(); }

      }));
      }
      }

      then checking through a web server the page correctly prints "true", and no HttpSessions are created.

      However, when I try to verify statelessness through WicketTester, the following test passes:

      @Test
      public void testName() throws Exception {
      WicketTester tester = new WicketTester(new WebApplication() {
      @Override
      public Class<? extends Page> getHomePage()

      { return MyPage.class; }

      });
      tester.startPage(MyPage.class);
      tester.assertLabel("isPageStateless", "false");
      assertFalse(tester.getLastRenderedPage().isPageStateless());
      }

      It seems that somehow due to WicketTester, isPageStateless() is being invoked before any behaviors are run (and thus the AjaxLink is still visible), and since stateless-flag for the page is cached, it remains false.

      If it's by design that isPageStateless should always return the same result during each request, then I guess that the statelessness resolution process must not depend on anything happening after the page constructor? I assume it's not by design.

      Suggestions:

      A) Obvious fix would be to remove stateless-flag caching, since apparently it is causing problems, as also suggested by a hackish comment in Page.init(). In general, caching should always be used sparingly.

      B) Or maybe whoever is invoking isPageStateless() at an early stage should actually be using Page.peekPageStateless()? But this doesn't really seem like a real fix, more like a temporary hack.

      C) All caching could also be disabled during test runs, but this would not be a good thing since tests should reproduce the actual behavior as closely as possible.

      D) In case this is a known issue without a proper fix, how then could I verify page statelessness through WicketTester? Currently I'm clearing the stateless-cache by reflection, which feels kind of bad...

      1. WICKET-5424-tester.patch
        2 kB
        Sven Meier
      2. wicket-bug-test.zip
        5 kB
        Jyri-Matti Lähteenmäki

        Issue Links

          Activity

          Hide
          Jyri-Matti Lähteenmäki added a comment -

          Thanks a lot, you are quick =)

          ...but I'm a bit worried of two things:

          1) Caching should be used with great caution, unless it's clear that the result will never change. If this approach is used widely in Wicket, it might be causing bugs which are really hard to catch. At least it's making Wicket more fragile. Are you sure removing the cache would cause a noticeable hit? It looks like the method is just invoking isVisible/EnabledInHierarchy for the whole component tree a couple of times. Since the implementation is "going down" with a visitor and on each component coming back up with the "hierarchy", it's doing the same stuff multiple times, so maybe the whole check could be performed in one method so that the visibility of each component is checked only once? Would it still be too slow?

          2) I probably don't understand much about statelessness in Wicket, but doesn't it feel odd that statelessness can change during a request? How about turning the process "upside down": instead of checking for statelessness, make each component determine its statelessness just before rendering. Thus this flag wouldn't even be available before the render phase and it could not change once it's determined (there's no point in changing it during rendering or afterwards, right?).

          I'm kind of waving my hands in the air here, so feel free to ignore my suggestions =)

          Show
          Jyri-Matti Lähteenmäki added a comment - Thanks a lot, you are quick =) ...but I'm a bit worried of two things: 1) Caching should be used with great caution, unless it's clear that the result will never change. If this approach is used widely in Wicket, it might be causing bugs which are really hard to catch. At least it's making Wicket more fragile. Are you sure removing the cache would cause a noticeable hit? It looks like the method is just invoking isVisible/EnabledInHierarchy for the whole component tree a couple of times. Since the implementation is "going down" with a visitor and on each component coming back up with the "hierarchy", it's doing the same stuff multiple times, so maybe the whole check could be performed in one method so that the visibility of each component is checked only once? Would it still be too slow? 2) I probably don't understand much about statelessness in Wicket, but doesn't it feel odd that statelessness can change during a request? How about turning the process "upside down": instead of checking for statelessness, make each component determine its statelessness just before rendering. Thus this flag wouldn't even be available before the render phase and it could not change once it's determined (there's no point in changing it during rendering or afterwards, right?). I'm kind of waving my hands in the air here, so feel free to ignore my suggestions =)
          Hide
          Sven Meier added a comment -

          Thanks Martin, I'll use WicketTesterLazyIsPageStatelessBase and subclasses in WICKET-5426.

          Show
          Sven Meier added a comment - Thanks Martin, I'll use WicketTesterLazyIsPageStatelessBase and subclasses in WICKET-5426 .
          Hide
          Sven Meier added a comment -

          The difference between test and in-browser run is present in 6.x only, this issue is now fixed with 6.13.0.

          WICKET-5426 will track improvements on statelessness handling when stateful components are hidden in the render phase.

          Show
          Sven Meier added a comment - The difference between test and in-browser run is present in 6.x only, this issue is now fixed with 6.13.0. WICKET-5426 will track improvements on statelessness handling when stateful components are hidden in the render phase.
          Hide
          Martin Grigorov added a comment -

          The issue is still existing in master branch - http://ci.apache.org/builders/wicket-master/builds/1522/steps/compile/logs/stdio.

          I've used the attached quickstart as a test case.

          Show
          Martin Grigorov added a comment - The issue is still existing in master branch - http://ci.apache.org/builders/wicket-master/builds/1522/steps/compile/logs/stdio . I've used the attached quickstart as a test case.
          Hide
          Sven Meier added a comment -

          BaseWicketTester now takes care not to interfere with the wrapped PageRenderer. With the configured render strategy ONE_PASS_RENDER the page is reported as stateless in the testcase and when rendered in the browser.

          Show
          Sven Meier added a comment - BaseWicketTester now takes care not to interfere with the wrapped PageRenderer. With the configured render strategy ONE_PASS_RENDER the page is reported as stateless in the testcase and when rendered in the browser.
          Hide
          Martin Grigorov added a comment -

          Good catch!
          The patch looks good to me.

          Show
          Martin Grigorov added a comment - Good catch! The patch looks good to me.
          Hide
          Sven Meier added a comment -

          Actually there are two different problems:

          1) BaseWicketTester uses a special PageRenderer that records the last rendered page. The current implementation pulls the page out of the pageProvider before actual rendering.
          This results in the observed difference between test and non-test (only with ONE_PASS_RENDER though).

          IMHO we should fix this with the attached patch and close this ticket.

          2) Then reconsider the separate issue of changing statelessness in #onConfigure().

          Show
          Sven Meier added a comment - Actually there are two different problems: 1) BaseWicketTester uses a special PageRenderer that records the last rendered page. The current implementation pulls the page out of the pageProvider before actual rendering. This results in the observed difference between test and non-test (only with ONE_PASS_RENDER though). IMHO we should fix this with the attached patch and close this ticket. 2) Then reconsider the separate issue of changing statelessness in #onConfigure().
          Hide
          Sven Meier added a comment -

          I'll take a look at it tomorrow.

          Show
          Sven Meier added a comment - I'll take a look at it tomorrow.
          Hide
          Martin Grigorov added a comment -

          The quickstart has no Start.java so I cannot run it normally (i.e. w/o WicketTester). I already deleted maven jetty plugin - it was using 6.1.22... I didn't want to download something that old. Sorry
          Next time use http://wicket.apache.org/start/quickstart.html.

          But I think it should behave the same way - #isPageStateless() is called early in org.apache.wicket.request.handler.render.WebPageRenderer#respond().
          Behavior#onConfigure() is called once the page rendering process starts. So it is late to hide the stateful component and make the page stateless again.

          The problem is deeper.
          org.apache.wicket.Page#stateless is transient but the page is deserialized only if it is loaded from the disk. Before loading from disk Wicket checks for the level 1 cache - the http session keeps the last used page. So if the page is loaded from http session the 'stateless' cache is still there and even if you remove/replace the solely stateful component with a stateless one the page will still be assumed as stateful due to the cached.
          This can be improved by null-ifying the cache in #detach().

          I am not sure that removing the cache will be a good thing. This may make the page processing considerably slower.
          Maybe we can introduce protected non-final #getStatelessCache() that returns org.apache.wicket.Page#stateless. If page like yours needs to disable the cache then you can override this method and return null always.

          Sven Meier, Igor Vaynberg WDYT about this problem ?

          Show
          Martin Grigorov added a comment - The quickstart has no Start.java so I cannot run it normally (i.e. w/o WicketTester). I already deleted maven jetty plugin - it was using 6.1.22... I didn't want to download something that old. Sorry Next time use http://wicket.apache.org/start/quickstart.html . But I think it should behave the same way - #isPageStateless() is called early in org.apache.wicket.request.handler.render.WebPageRenderer#respond(). Behavior#onConfigure() is called once the page rendering process starts. So it is late to hide the stateful component and make the page stateless again. The problem is deeper. org.apache.wicket.Page#stateless is transient but the page is deserialized only if it is loaded from the disk. Before loading from disk Wicket checks for the level 1 cache - the http session keeps the last used page. So if the page is loaded from http session the 'stateless' cache is still there and even if you remove/replace the solely stateful component with a stateless one the page will still be assumed as stateful due to the cached. This can be improved by null-ifying the cache in #detach(). I am not sure that removing the cache will be a good thing. This may make the page processing considerably slower. Maybe we can introduce protected non-final #getStatelessCache() that returns org.apache.wicket.Page#stateless. If page like yours needs to disable the cache then you can override this method and return null always. Sven Meier , Igor Vaynberg WDYT about this problem ?
          Hide
          Jyri-Matti Lähteenmäki added a comment -

          I organized the test so that the two tests pass meaning that the page is considered stateful. Should have made two failing tests, sorry, I'll learn

          Anyway, the test now asserts that isPageStateless returns false, but if you start jetty and load the frontpage it displays true. So the behaviors in the test and actual run differ.

          Show
          Jyri-Matti Lähteenmäki added a comment - I organized the test so that the two tests pass meaning that the page is considered stateful. Should have made two failing tests, sorry, I'll learn Anyway, the test now asserts that isPageStateless returns false , but if you start jetty and load the frontpage it displays true . So the behaviors in the test and actual run differ.
          Hide
          Martin Grigorov added a comment -

          The test case is not failing.
          Let me see deeper.

          Show
          Martin Grigorov added a comment - The test case is not failing. Let me see deeper.
          Hide
          Jyri-Matti Lähteenmäki added a comment -

          Added test case.
          Seems that this has something to do with RenderStrategy.ONE_PASS_RENDER

          Show
          Jyri-Matti Lähteenmäki added a comment - Added test case. Seems that this has something to do with RenderStrategy.ONE_PASS_RENDER
          Hide
          Martin Grigorov added a comment -

          Could you please attach a failing test case ?
          Thank you!

          Show
          Martin Grigorov added a comment - Could you please attach a failing test case ? Thank you!

            People

            • Assignee:
              Sven Meier
              Reporter:
              Jyri-Matti Lähteenmäki
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development