Test case

I changed Component#urlFor() in 6.9.2 with good success. This eliminates PageExpiredException for bookmarkable pages.

There are many cosmetic test failures due to a different expectation of the target URL in the generated responses: Instead of "wicket/page?0-1.IBehaviorListener.0-test" we should expect a more specific value "wicket/bookmarkable/org.apache.wicket.ajax.DomReadyOrderPage?0-1.IBehaviorListener.0-test", or a mounted path.

I have made real pages in a quickstart out of some test cases - they work fine. Test cases that expect PageExpiredException should no longer do so because of the improvement.

We can expect a performance improvement because Page#isPageStateless(), called for every generated URL on the page is very expensive compared with Page#isBookmarkable().

I've pushed a branch (wicket-4997) against 6.x with a proposed fix for this issue. A problem I'm facing is that urls for mounted pages with required page parameters are rendered as bookmarkable urls (wicket/bookmarkable) when recreateMountedPagesAfterExpiry is disabled. This can be prevented by overriding isBookmarkable on the page (see PageWithLink), but I'm not sure if that's a good thing or not.

Thanks Emond.

Yes, I agree with your concerns. Have you tried Page#wasCreatedBookmarkable instead? I have just discoverd this on line 384 in AbstractBookmarkableMapper#mapHandler

if (checkPageInstance(page) &&
(!pageMustHaveBeenCreatedBookmarkable() || page.wasCreatedBookmarkable()))
{

I was getting second thoughts about using Page#isBookmarkable() because it creates a bookmarkable URL for an unmounted page that was in fact created by a non-bookmarkable constructor i.e. with IModel parameter as provided with the test case. Sorry if I first suggested the wrong approach.

On the other hand expiry in the non-bookmarkable constructor case can be handled nicely in the constructor with a customised response so Page#isBookmarkable() is more flexible and less disrupting to the user. We just don't want PageExpiredException. That weakens the point in my last comment.

In other words if the developer codes bookmarkable constructors then these constructors must be supported properly i.e. validated.

This is the kind of scenario that makes me I think that Wicket should call a no-args page constructor instead of creating bogus PageParameters, if no PageParameters are provided.

Also, regarding your concern, there is still an outstanding issue WICKET-5068. When we get the PageParameters back, then there is less motivation to prevent the page from being re-created.

BTW this fixes WICKET-5043 which is essentially a duplicate of this issue.

Some source files in a zip archive for consideration.

I think that WebResponseExceptionsTest#expirePage must fail with a bookmarkable TestExpirePage because clicking the link should re-create the page. That is why I changed it to be non-bookmarkable.

We still need a trivial additional test that positively verifies that a bookmarkable page does NOT expire with PageExpiredException but with the same page re-created after session expiry.

Please provide code changes in unified diff format and in this case against the wicket-4997 branch. The easiest way to create a diff is using git diff.

I can see a lot of remote branches but can't see that 4997 remote branch in my git client. I don't have much experience with git. What else can I do?

The branch is there (you can also see it on the github mirror). Perhaps you need to fetch it.

I have fetched the branch. What is the diff command please?

Please see the attached 6 files. I changed only 4 main source files in addition to the obvious Component. MountedMapper#mapHandler now additionally handles BookmarkableListenerInterfaceRequestHandler via a new interface IListenerInterfaceRequestHandler which is implemented by BookmarkableListenerInterfaceRequestHandler and ListenerInterfaceRequestHandler.

Wicket needs this to add the bookmarkable URL component to link URLs to avoid PageExpiredException for bookmarkable pages.

Hi Emond,

Many thanks again. The results of your changes look good to me. The tests look fine, too. I may have overlooked a couple of things or looked at an older version.

We now use the following condition to render bookmarkable urls:
getApplication().getPageSettings().getRecreateMountedPagesAfterExpiry() && ((page.isBookmarkable() && page.wasCreatedBookmarkable()) || page.isPageStateless())

This results in bookmarkable urls for pages that are bookmarkable and were created bookmarkable. These pages can be recreated after session expiry.

Emond, my big thanks for that!

Emond Papegaaij sorry for disturbing you, but, but I'd like to get your opinion on few things related with commit bf43914fd7f0ed5ff57a56a82420cda76d3cbaee
Thus,

• once you amend
  Component.java

  if (page.isBookmarkable())
  {
      handler = new BookmarkableListenerInterfaceRequestHandler(provider, listener);
  }
  else
  {
     handler = new ListenerInterfaceRequestHandler(provider, listener);
  }
  

  ,
  does "if conditon"

  MountedMapper.java

  //public Url mapHandler(IRequestHandler requestHandler)
  if (url == null && requestHandler instanceof ListenerInterfaceRequestHandler &&
  			getRecreateMountedPagesAfterExpiry())
  

  can ever trigger?

• if I dealing with stateless bookmarkable pages and buld link for the next page, for example, I have now something like /url?pageIdx=0&pageIdx=1, does it meat that you expect me switch off recreateMountedPagesAfterExpiry option? But it will means that my statefull bookmarkable pages won't be able to restore state if session expired.

Thanks in advance.

Please use users@wicket.apache.org for questions.
The description above is not very clear about your use case and the problem you face.