[
https://issues.apache.org/jira/browse/WICKET-4201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15856856#comment-15856856
]
ASF GitHub Bot commented on WICKET-4201:
----------------------------------------
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/210#discussion_r99936172
--- Diff:
wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageProvider.java
---
@@ -197,26 +194,49 @@ else if (isNewPageInstance() == false)
}
/**
- * The page instance is new only if there is no cached instance or the
data stores doesn't have
- * a page with that id with the same {@linkplain #pageClass}.
- *
- * @see IPageProvider#isNewPageInstance()
+ * @return negates {@link PageProvider#hasPageInstance()}
+ * @deprecated use {@link PageProvider#hasPageInstance()} negation
instead
*/
@Override
public boolean isNewPageInstance()
{
- boolean isNew = pageInstance == null;
- if (isNew && pageId != null)
+ return !hasPageInstance();
+ }
+
+ /**
+ * 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 (provision != null || pageId != null)
{
- IRequestablePage storedPageInstance =
getStoredPage(pageId);
- if (storedPageInstance != null)
- {
- pageInstance = storedPageInstance;
- isNew = false;
- }
+ return getProvision().didResolveToPage();
}
+ else
+ return false;
+ }
- return isNew;
+ /**
+ * 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 doesProvideNewPage()
+ {
+ if (provision == null)
+ {
+ throw new IllegalStateException("Page instance not yet
resolved");
--- End diff --
This doesn't look nice.
At
https://issues.apache.org/jira/browse/WICKET-4201?focusedCommentId=15853108&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15853108
you say
` removed the PageExpiredException from PageProvider since it breaks the
class coherence. It's designed to provide pages and related metadata (such as
if the page was expired included), not to change the application flow on
application exceptions.` but this runtime exception does exactly this.
IMO it would be better to return a three state result: `NEW`, `OLD` and
`UNKNOWN`
> IPageProvider and its implementations need to be improved
> ---------------------------------------------------------
>
> Key: WICKET-4201
> URL: https://issues.apache.org/jira/browse/WICKET-4201
> Project: Wicket
> Issue Type: Task
> Components: wicket
> Affects Versions: 1.5.0, 1.5.1, 1.5.2
> Reporter: Emond Papegaaij
> Assignee: Pedro Santos
>
> During the development op 1.5, IPageProvider and its implementations have
> become a bit of a mess. The interface is not clearly defined. One of the
> biggest problems is that several methods can throw exceptions and there is no
> way of knowing which method will throw which exception and when. It should
> always be clear what exceptions to expect. For example, getPage can throw a
> PageExpiredException, but getPageClass cannot, it should return null if no
> page class is set. Perhaps, it's even better to never throw exceptions at
> all. Also, the various introspection methods are not very well defined and
> make it almost impossible to come up with an alternative implementation of
> the interface (which, IMHO is a sign of a broken API).
> Changing this interface is not an option for 1.5, but looking at the number
> of subtle bugs that came from this part of the code, it should really be
> considered for wicket.next.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
