Hi Emond,

On Mon, Aug 19, 2013 at 12:10 PM, <[email protected]> wrote:

> WICKET-4997: render bookmarkable urls for bookmarkable pages (not
> stateless)
>
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e99bf147
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e99bf147
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e99bf147
>
> Branch: refs/heads/wicket-4997
> Commit: e99bf1476403ff13e409ba28331a8291a03ca25f
> Parents: 7d8313f
> Author: Emond Papegaaij <[email protected]>
> Authored: Mon Aug 19 11:07:41 2013 +0200
> Committer: Emond Papegaaij <[email protected]>
> Committed: Mon Aug 19 11:10:02 2013 +0200
>
> ----------------------------------------------------------------------
>  .../main/java/org/apache/wicket/Component.java  |  4 +-
>  .../mapper/AbstractBookmarkableMapper.java      | 13 +++++--
>  .../core/request/mapper/MountedMapper.java      | 41 ++++++++------------
>  3 files changed, 29 insertions(+), 29 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/e99bf147/wicket-core/src/main/java/org/apache/wicket/Component.java
> ----------------------------------------------------------------------
> diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java
> b/wicket-core/src/main/java/org/apache/wicket/Component.java
> index 8fa63a3..116eb86 100644
> --- a/wicket-core/src/main/java/org/apache/wicket/Component.java
> +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java
> @@ -3336,7 +3336,7 @@ public abstract class Component
>                 Page page = getPage();
>                 PageAndComponentProvider provider = new
> PageAndComponentProvider(page, this, parameters);
>                 IRequestHandler handler;
> -               if (page.isPageStateless())
> +               if (page.isBookmarkable())
>                 {
>                         handler = new
> BookmarkableListenerInterfaceRequestHandler(provider, listener, id);
>                 }
> @@ -3379,7 +3379,7 @@ public abstract class Component
>                 Page page = getPage();
>                 PageAndComponentProvider provider = new
> PageAndComponentProvider(page, this, parameters);
>                 IRequestHandler handler;
> -               if (page.isPageStateless())
> +               if (page.isBookmarkable())
>

I think this change is OK.
Maybe we can improve it a bit by using Application.get().getPageSettings().
getRecreateMountedPagesAfterExpiry() in the checks above ?

With the new check as you can see the produced urls contain the class name.
Some users don't like this.
Application.get().getPageSettings().getRecreateMountedPagesAfterExpiry()
return true by default. If someone doesn't like the extra info in the
produced urls then she can disable it this way.


>                 {
>                         handler = new
> BookmarkableListenerInterfaceRequestHandler(provider, listener);
>                 }
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/e99bf147/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
> index 93c22d2..bbe2e1c 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/AbstractBookmarkableMapper.java
> @@ -210,8 +210,7 @@ public abstract class AbstractBookmarkableMapper
> extends AbstractComponentMapper
>                 PageProvider provider = new
> PageProvider(pageInfo.getPageId(), pageClass, pageParameters,
>                         renderCount);
>                 provider.setPageSource(getContext());
> -               if (provider.isNewPageInstance() &&
> -
> !WebApplication.get().getPageSettings().getRecreateMountedPagesAfterExpiry())
> +               if (provider.isNewPageInstance() &&
> !getRecreateMountedPagesAfterExpiry())
>                 {
>                         throw new
> PageExpiredException(String.format("Bookmarkable page id '%d' has expired.",
>                                 pageInfo.getPageId()));
> @@ -222,6 +221,11 @@ public abstract class AbstractBookmarkableMapper
> extends AbstractComponentMapper
>                 }
>         }
>
> +       boolean getRecreateMountedPagesAfterExpiry()
> +       {
> +               return
> WebApplication.get().getPageSettings().getRecreateMountedPagesAfterExpiry();
> +       }
> +
>         /**
>          * Creates a {@code IRequestHandler} that processes a listener
> request.
>          *
> @@ -420,8 +424,11 @@ public abstract class AbstractBookmarkableMapper
> extends AbstractComponentMapper
>
> requestListenerInterfaceToString(handler.getListenerInterface()),
>                                 handler.getComponentPath(),
> handler.getBehaviorIndex());
>
> +                       PageParameters parameters =
> getRecreateMountedPagesAfterExpiry() ? new PageParameters(
> +
> handler.getPage().getPageParameters()).mergeWith(handler.getPageParameters())
> +                               : handler.getPageParameters();
>                         UrlInfo urlInfo = new UrlInfo(new
> PageComponentInfo(pageInfo, componentInfo),
> -                               pageClass, handler.getPageParameters());
> +                               pageClass, parameters);
>                         return buildUrl(urlInfo);
>                 }
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/e99bf147/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
> index 9b1db28..19212b6 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/MountedMapper.java
> @@ -19,7 +19,6 @@ package org.apache.wicket.core.request.mapper;
>  import java.util.ArrayList;
>  import java.util.List;
>
> -import org.apache.wicket.Application;
>  import org.apache.wicket.RequestListenerInterface;
>  import
> org.apache.wicket.core.request.handler.ListenerInterfaceRequestHandler;
>  import org.apache.wicket.request.IRequestHandler;
> @@ -49,21 +48,21 @@ import org.apache.wicket.util.string.Strings;
>   * are matched before optional parameters, and optional parameters eager
> (from left to right).
>   * <p>
>   * Decodes and encodes the following URLs:
> - *
> + *
>   * <pre>
>   *  Page Class - Render (BookmarkablePageRequestHandler for mounted pages)
>   *  /mount/point
>   *  (these will redirect to hybrid alternative if page is not stateless)
> - *
> + *
>   *  IPage Instance - Render Hybrid (RenderPageRequestHandler for mounted
> pages)
>   *  /mount/point?2
> - *
> + *
>   *  IPage Instance - Bookmarkable Listener
> (BookmarkableListenerInterfaceRequestHandler for mounted pages)
>   *  /mount/point?2-click-foo-bar-baz
>   *  /mount/point?2-5.click.1-foo-bar-baz (1 is behavior index, 5 is
> render count)
>   *  (these will redirect to hybrid if page is not stateless)
>   * </pre>
> - *
> + *
>   * @author Matej Knopp
>   */
>  public class MountedMapper extends AbstractBookmarkableMapper
> @@ -143,7 +142,7 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>
>         /**
>          * Construct.
> -        *
> +        *
>          * @param mountPath
>          * @param pageClass
>          */
> @@ -154,32 +153,32 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>
>         /**
>          * Construct.
> -        *
> +        *
>          * @param mountPath
>          * @param pageClassProvider
>          */
>         @Deprecated
>         public MountedMapper(String mountPath,
> -                            ClassProvider<? extends IRequestablePage>
> pageClassProvider)
> +               ClassProvider<? extends IRequestablePage>
> pageClassProvider)
>         {
>                 this(mountPath, new
> ClassReference(pageClassProvider.get()), new PageParametersEncoder());
>         }
>
>         /**
>          * Construct.
> -        *
> +        *
>          * @param mountPath
>          * @param pageClassProvider
>          */
>         public MountedMapper(String mountPath,
> -                            IProvider<Class<? extends IRequestablePage>>
> pageClassProvider)
> +               IProvider<Class<? extends IRequestablePage>>
> pageClassProvider)
>         {
>                 this(mountPath, pageClassProvider, new
> PageParametersEncoder());
>         }
>
>         /**
>          * Construct.
> -        *
> +        *
>          * @param mountPath
>          * @param pageClass
>          * @param pageParametersEncoder
> @@ -192,7 +191,7 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>
>         /**
>          * Construct.
> -        *
> +        *
>          * @param mountPath
>          * @param pageClassProvider
>          * @param pageParametersEncoder
> @@ -202,20 +201,19 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>                 ClassProvider<? extends IRequestablePage>
> pageClassProvider,
>                 IPageParametersEncoder pageParametersEncoder)
>         {
> -               this(mountPath, new
> ClassReference(pageClassProvider.get()),
> -                               pageParametersEncoder);
> +               this(mountPath, new
> ClassReference(pageClassProvider.get()), pageParametersEncoder);
>         }
>
>         /**
>          * Construct.
> -        *
> +        *
>          * @param mountPath
>          * @param pageClassProvider
>          * @param pageParametersEncoder
>          */
>         public MountedMapper(String mountPath,
> -                            IProvider<Class<? extends IRequestablePage>>
> pageClassProvider,
> -                            IPageParametersEncoder pageParametersEncoder)
> +               IProvider<Class<? extends IRequestablePage>>
> pageClassProvider,
> +               IPageParametersEncoder pageParametersEncoder)
>         {
>                 Args.notEmpty(mountPath, "mountPath");
>                 Args.notNull(pageClassProvider, "pageClassProvider");
> @@ -427,11 +425,6 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>                 return url;
>         }
>
> -       boolean getRecreateMountedPagesAfterExpiry()
> -       {
> -               return
> Application.get().getPageSettings().getRecreateMountedPagesAfterExpiry();
> -       }
> -
>         /**
>          * @see
> AbstractBookmarkableMapper#buildUrl(AbstractBookmarkableMapper.UrlInfo)
>          */
> @@ -483,7 +476,7 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>         /**
>          * Check if the URL is for home page and the home page class match
> mounted class. If so,
>          * redirect to mounted URL.
> -        *
> +        *
>          * @param url
>          * @return request handler or <code>null</code>
>          */
> @@ -504,7 +497,7 @@ public class MountedMapper extends
> AbstractBookmarkableMapper
>          * If this method returns <code>true</code> and application home
> page class is same as the class
>          * mounted with this encoder, request to home page will result in
> a redirect to the mounted
>          * path.
> -        *
> +        *
>          * @return whether this encode should respond to home page request
> when home page class is same
>          *         as mounted class.
>          */
>
>

Reply via email to