Repository: wicket Updated Branches: refs/heads/WICKET-4201-improved-page-provider 32259ca91 -> e1fa342ab
WICKET-4201 removing the page expired exception from page provider and using its API to determine such state throughout the code Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e1fa342a Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e1fa342a Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e1fa342a Branch: refs/heads/WICKET-4201-improved-page-provider Commit: e1fa342ab24d22748a44df4acbbf4e65a18a5f34 Parents: 32259ca Author: Pedro Henrique Oliveira dos Santos <[email protected]> Authored: Sun Feb 5 01:36:23 2017 -0200 Committer: Pedro Henrique Oliveira dos Santos <[email protected]> Committed: Sun Feb 5 01:36:23 2017 -0200 ---------------------------------------------------------------------- .../request/handler/ListenerRequestHandler.java | 19 ++- .../core/request/handler/PageProvider.java | 141 +++++++++++-------- .../mapper/AbstractBookmarkableMapper.java | 2 +- 3 files changed, 98 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/e1fa342a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java index 400feb2..f9b3bbc 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java @@ -23,6 +23,7 @@ import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.behavior.Behavior; import org.apache.wicket.core.request.handler.RenderPageRequestHandler.RedirectPolicy; import org.apache.wicket.core.request.handler.logger.ListenerLogData; +import org.apache.wicket.protocol.http.PageExpiredException; import org.apache.wicket.request.ILoggableRequestHandler; import org.apache.wicket.request.IRequestCycle; import org.apache.wicket.request.component.IRequestableComponent; @@ -94,7 +95,23 @@ public class ListenerRequestHandler @Override public IRequestablePage getPage() { - return pageComponentProvider.getPageInstance(); + try + { + return pageComponentProvider.getPageInstance(); + } + catch (IllegalStateException e) + { + if (pageComponentProvider.wasExpired()) + { + + throw new PageExpiredException( + "Page with id '" + pageComponentProvider.getPageId() + "' has expired."); + } + else + { + throw e;// bubbles up + } + } } @Override http://git-wip-us.apache.org/repos/asf/wicket/blob/e1fa342a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java index cda486a..2934cf3 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java @@ -20,7 +20,6 @@ import org.apache.wicket.Application; import org.apache.wicket.core.request.mapper.IPageSource; import org.apache.wicket.core.request.mapper.StalePageException; import org.apache.wicket.page.IPageManager; -import org.apache.wicket.protocol.http.PageExpiredException; import org.apache.wicket.request.IRequestHandler; import org.apache.wicket.request.IRequestMapper; import org.apache.wicket.request.component.IRequestablePage; @@ -59,7 +58,7 @@ public class PageProvider implements IPageProvider, IClusterable private PageParameters pageParameters; - private Provision provision; + private Provision provision = new Provision(); /** * Creates a new page provider object. Upon calling of {@link #getPageInstance()} this provider @@ -151,18 +150,17 @@ public class PageProvider implements IPageProvider, IClusterable { Args.notNull(page, "page"); - provision = new Provision(page); + provision = new Provision().resolve(page); pageId = page.getPageId(); renderCount = page.getRenderCount(); } - private Provision getProvision() + private Provision getResolvedProvision() { - if (provision == null) - { - provision = new Provision(getPageSource()).resolvePageInstance(pageId, pageClass, - pageParameters, renderCount); - } + if (!provision.wasResolved()) + + provision.resolve(pageId, pageClass, pageParameters, renderCount); + return provision; } @@ -172,11 +170,13 @@ public class PageProvider implements IPageProvider, IClusterable @Override public IRequestablePage getPageInstance() { - if (!getProvision().didResolvePage() && !getProvision().doesProvideNewPage()) + Provision resolvedProvision = getResolvedProvision(); + + if (!resolvedProvision.didResolveToPage() && !resolvedProvision.doesProvideNewPage()) { - throw new PageExpiredException("Page with id '" + pageId + "' has expired."); + throw new IllegalStateException("The configured page provider can't resolve a page."); } - return getProvision().get(); + return resolvedProvision.getPage(); } /** @@ -189,9 +189,9 @@ public class PageProvider implements IPageProvider, IClusterable { return pageParameters; } - else if (isNewPageInstance() == false) + else if (hasPageInstance()) { - return getProvision().get().getPageParameters(); + return getPageInstance().getPageParameters(); } else { @@ -200,7 +200,8 @@ public class PageProvider implements IPageProvider, IClusterable } /** - * @return don't resolve a page, but can create a new one + * @return negates {@link PageProvider#hasPageInstance()} + * @deprecated use {@link PageProvider#hasPageInstance()} negation instead */ @Override public boolean isNewPageInstance() @@ -209,24 +210,48 @@ public class PageProvider implements IPageProvider, IClusterable } /** - * @return if provides an identified page + * If this provider returns existing page, regardless if it was already created by PageProvider + * itself or is or can be found in the data store. The only guarantee is that by calling + * {@link PageProvider#getPageInstance()} this provider will return an existing instance and no + * page will be created. + * + * @return if provides an existing page */ @Override public final boolean hasPageInstance() { - if (pageId != null || provision != null) - return getProvision().didResolvePage(); + if (provision.wasResolved() || pageId != null) + { + return getResolvedProvision().didResolveToPage(); + } else return false; } /** + * Returns whether or not the page instance held by this provider has been instantiated by the + * provider. + * + * @return {@code true} iff the page instance held by this provider was instantiated by the + * provider + */ + @Override + public final boolean isPageInstanceFresh() + { + if (!this.provision.wasResolved()) + { + throw new IllegalStateException("Page instance not yet resolved"); + } + return getResolvedProvision().doesProvideNewPage(); + } + + /** * @see org.apache.wicket.core.request.handler.IPageProvider#wasExpired() */ @Override public boolean wasExpired() { - return pageId != null && isPageInstanceFresh(); + return pageId != null && getResolvedProvision().didFailToFindStoredPage(); } /** @@ -271,11 +296,7 @@ public class PageProvider implements IPageProvider, IClusterable @Override public void detach() { - if (provision != null) - { - provision.detach(); - provision = null; - } + provision.detach(); } /** @@ -287,7 +308,7 @@ public class PageProvider implements IPageProvider, IClusterable */ public void setPageSource(IPageSource pageSource) { - if (this.provision != null) + if (this.provision.wasResolved()) { throw new IllegalStateException( "A provision was already been done. The provider can be forcefully detached or a new one needs to be used to provide using this page source."); @@ -331,71 +352,64 @@ public class PageProvider implements IPageProvider, IClusterable return renderCount; } - /** - * Returns whether or not the page instance held by this provider has been instantiated by the - * provider. - * - * @return {@code true} iff the page instance held by this provider was instantiated by the - * provider - */ - @Override - public final boolean isPageInstanceFresh() - { - if (this.provision == null) - { - throw new IllegalStateException("Page instance not yet resolved"); - } - return getProvision().doesProvideNewPage(); - } - @Override public String toString() { return "PageProvider{" + "renderCount=" + renderCount + ", pageId=" + pageId - + ", pageClass=" + pageClass + ", pageParameters=" + pageParameters + ", provision=" - + provision + '}'; + + ", pageClass=" + pageClass + ", pageParameters=" + pageParameters + '}'; } private class Provision { - private transient IRequestablePage page; - private boolean failedToFindStoreddPage; - private IPageSource pageSource; + transient IRequestablePage page; + boolean resolved; + boolean failedToFindStoredPage; - Provision(IRequestablePage page) + IRequestablePage getPage() { - this.page = page; + if (page == null && doesProvideNewPage()) + + page = getPageSource().newPageInstance(pageClass, pageParameters); + + return page; } - Provision(IPageSource pageSource) + boolean wasResolved() { - this.pageSource = pageSource; + return resolved; } - IRequestablePage get() + boolean didResolveToPage() { - if (page == null && doesProvideNewPage()) - page = pageSource.newPageInstance(pageClass, pageParameters); - return page; + return page != null; } - boolean didResolvePage() + boolean doesProvideNewPage() { - return page != null; + return (pageId == null || failedToFindStoredPage) && pageClass != null; + } + + boolean didFailToFindStoredPage() + { + return failedToFindStoredPage; } - private boolean doesProvideNewPage() + Provision resolve(IRequestablePage page) { - return failedToFindStoreddPage && pageClass != null; + this.page = page; + + resolved = true; + + return this; } - Provision resolvePageInstance(Integer pageId, Class<? extends IRequestablePage> pageClass, + Provision resolve(Integer pageId, Class<? extends IRequestablePage> pageClass, PageParameters pageParameters, Integer renderCount) { if (pageId != null) { - IRequestablePage stored = pageSource.getPageInstance(pageId); + IRequestablePage stored = getPageSource().getPageInstance(pageId); if (stored != null && (pageClass == null || pageClass.equals(stored.getClass()))) { @@ -404,9 +418,12 @@ public class PageProvider implements IPageProvider, IClusterable if (renderCount != null && page.getRenderCount() != renderCount) throw new StalePageException(page); } + + failedToFindStoredPage = page == null; } - failedToFindStoreddPage = page == null; + + resolved = true; return this; } http://git-wip-us.apache.org/repos/asf/wicket/blob/e1fa342a/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 afafcee..f12aaec8 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 @@ -309,7 +309,7 @@ public abstract class AbstractBookmarkableMapper extends AbstractComponentMapper private void checkExpiration(PageProvider provider, PageInfo pageInfo) { - if (provider.isNewPageInstance() && !getRecreateMountedPagesAfterExpiry()) + if (provider.wasExpired() && !getRecreateMountedPagesAfterExpiry()) { throw new PageExpiredException(String.format("Bookmarkable page with id '%d' has expired.", pageInfo.getPageId()));
