yeah, your patch would work, but i think we agree that the
session approach is better and cleaner (both functionally and
conceptually) for the long term even if it could at least
/conceivably/ break some especially odd code (which it
seems likely does not even exist).
the semantics are actually quite nice in the session version.
messages added directly to the session would still have a
null reporter and persist until rendered, as they currently
do (no changes there).
messages reported by components would all be cleared at
the end of a request (not the beginning of the next one).
this is conceptually clearer and simpler and it actually
reduces memory load (as does the removal of all aspects
of feedback message storage from Page).
so i can't actually see any important disadvantage to the
session approach. and there seem to be several nice
little wins. probably less code too.
Eelco Hillenius wrote:
>
>> We chose that, yes. But would it be better to simply have all feedback
>> messages stored in the session? That would solve all our scoping
>> problems
>> wouldn't it? Every feedback message would go into the session and then
>> feedback panels could pull them out when they're ready to. If a feedback
>> message is in the session for more than one request cycle and it is not
>> cleared
>> by a feedback panel, we would discard it (and maybe warn in debug mode).
>
> I think I'm fine with a session based solution. The thing that
> wouldn't work anymore is when you assign messages to components/ pages
> which are not rendered this request but a next request. That might be
> too far-fetched to support though. However, the solution which I'll
> post as a patch on the end of this message will support anything it
> does now. The thing I like about putting them in the session is that
> it is clean and will save a little bit of memory in the page. WDYT?
>
> Eelco
>
> Index:
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/Component.java
> ===================================================================
> ---
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/Component.java
> (revision
> 524461)
> +++
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/Component.java
> (working
> copy)
> @@ -662,7 +662,7 @@
> */
> public final void debug(final String message)
> {
> - getPage().getFeedbackMessages().debug(this, message);
> + Page.addMessage(new FeedbackMessage(this, message,
> FeedbackMessage.DEBUG));
> }
>
> /**
> @@ -685,7 +685,7 @@
> */
> public final void error(final Serializable message)
> {
> - getPage().getFeedbackMessages().error(this, message);
> + Page.addMessage(new FeedbackMessage(this, message,
> FeedbackMessage.ERROR));
> }
>
> /**
> @@ -696,7 +696,7 @@
> */
> public final void fatal(final String message)
> {
> - getPage().getFeedbackMessages().fatal(this, message);
> + Page.addMessage(new FeedbackMessage(this, message,
> FeedbackMessage.FATAL));
> }
>
> /**
> @@ -1235,7 +1235,7 @@
> */
> public final void info(final String message)
> {
> - getPage().getFeedbackMessages().info(this, message);
> + Page.addMessage(new FeedbackMessage(this, message,
> FeedbackMessage.INFO));
> }
>
> /**
> @@ -2376,7 +2376,7 @@
> */
> public final void warn(final String message)
> {
> - getPage().getFeedbackMessages().warn(this, message);
> + Page.addMessage(new FeedbackMessage(this, message,
> FeedbackMessage.WARNING));
> }
>
> /**
> Index:
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/feedback/FeedbackMessages.java
> ===================================================================
> ---
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/feedback/FeedbackMessages.java
> (revision
> 524461)
> +++
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/feedback/FeedbackMessages.java
> (working
> copy)
> @@ -301,7 +301,7 @@
> * @param message
> * @param level
> */
> - public final void add(Component reporter, String message, int level) {
> + public final void add(Component reporter, Serializable message, int
> level) {
> add(new FeedbackMessage(reporter, message, level));
> }
>
> @@ -311,7 +311,7 @@
> * @param message
> * the message
> */
> - final void add(FeedbackMessage message)
> + public final void add(FeedbackMessage message)
> {
> if (log.isDebugEnabled())
> {
> Index:
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/Page.java
> ===================================================================
> ---
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/Page.java
> (revision
> 524461)
> +++
> /Users/eelcohillenius/Documents/workspace_wicket/wicket-1.x/jdk-1.4/wicket/src/main/java/wicket/Page.java
> (working
> copy)
> @@ -18,6 +18,7 @@
>
> import java.util.ArrayList;
> import java.util.HashSet;
> +import java.util.Iterator;
> import java.util.List;
> import java.util.Set;
>
> @@ -25,6 +26,7 @@
> import org.apache.commons.logging.LogFactory;
>
> import wicket.authorization.UnauthorizedActionException;
> +import wicket.feedback.FeedbackMessage;
> import wicket.feedback.FeedbackMessages;
> import wicket.feedback.IFeedback;
> import wicket.markup.MarkupException;
> @@ -288,23 +290,24 @@
> */
> public void detachModels()
> {
> -// // visit all this page's children to detach the models
> -// visitChildren(new IVisitor()
> -// {
> -// public Object component(Component component)
> -// {
> -// try
> -// {
> -// // detach any models of the component
> -// component.detachModels();
> -// }
> -// catch (Exception e) // catch anything; we MUST
> detach all models
> -// {
> -// log.error("detaching models of
> component " + component + "
> failed:", e);
> -// }
> -// return IVisitor.CONTINUE_TRAVERSAL;
> -// }
> -// });
> + // // visit all this page's children to detach the models
> + // visitChildren(new IVisitor()
> + // {
> + // public Object component(Component component)
> + // {
> + // try
> + // {
> + // // detach any models of the component
> + // component.detachModels();
> + // }
> + // catch (Exception e) // catch anything; we MUST detach all
> models
> + // {
> + // log.error("detaching models of component " + component + "
> failed:",
> + // e);
> + // }
> + // return IVisitor.CONTINUE_TRAVERSAL;
> + // }
> + // });
>
> super.detachModels();
> }
> @@ -317,6 +320,84 @@
> Session.get().dirtyPage(this);
> }
>
> + private static final ThreadLocal tempFeedbackMessages = new
> ThreadLocal();
> +
> + /**
> + * Adds message for thread
> + *
> + * @param message
> + * feedback message
> + */
> + static void addMessage(FeedbackMessage message)
> + {
> + synchronized (tempFeedbackMessages)
> + {
> + FeedbackMessages messages =
> (FeedbackMessages)tempFeedbackMessages.get();
> + if (messages == null)
> + {
> + messages = new FeedbackMessages();
> + tempFeedbackMessages.set(messages);
> + }
> + messages.add(message);
> + }
> + }
> +
> + private static void assignFeedbackMessagesToPages()
> + {
> + FeedbackMessages messages =
> (FeedbackMessages)tempFeedbackMessages.get();
> + if (messages != null)
> + {
> + for (Iterator i = messages.iterator(); i.hasNext();)
> + {
> + FeedbackMessage msg = (FeedbackMessage)i.next();
> + Component reporter = msg.getReporter();
> + if (reporter != null)
> + {
> + // it's a page specific message
> + Page page = reporter.findPage();
> + if (page != null)
> + {
> + if (page.feedbackMessages ==
> null)
> + {
> + page.feedbackMessages =
> new FeedbackMessages();
> + }
> + page.feedbackMessages.add(msg);
> + }
> + // remove, regardles of whether we were
> actually able to
> + // distribute it to a page
> + i.remove();
> + }
> + }
> + }
> + tempFeedbackMessages.remove();
> + }
> +
> + private void assignFeedbackMessagesToPage()
> + {
> + FeedbackMessages messages =
> (FeedbackMessages)tempFeedbackMessages.get();
> + if (messages != null)
> + {
> + for (Iterator i = messages.iterator(); i.hasNext();)
> + {
> + FeedbackMessage msg = (FeedbackMessage)i.next();
> + Component reporter = msg.getReporter();
> + if (reporter != null)
> + {
> + Page page = reporter.findPage();
> + if (page != null && page.equals(this))
> + {
> + if (feedbackMessages == null)
> + {
> + feedbackMessages = new
> FeedbackMessages();
> + }
> + feedbackMessages.add(msg);
> + i.remove();
> + }
> + }
> + }
> + }
> + }
> +
> /**
> * THIS METHOD IS NOT PART OF THE WICKET PUBLIC API. DO NOT CALL IT.
> */
> @@ -341,6 +422,8 @@
> // Set form component values from cookies
> setFormComponentValuesFromCookies();
>
> + assignFeedbackMessagesToPages();
> +
> // First, give priority to IFeedback instances, as they have to
> // collect their messages before components like ListViews
> // remove any child components
> @@ -455,7 +538,7 @@
> }
>
> /**
> - * @return The current ajax version number of this page.
> + * @return The current ajax version number of this page.
> */
> public final int getAjaxVersionNumber()
> {
> @@ -461,7 +544,7 @@
> {
> return versionManager == null ? 0 :
> versionManager.getAjaxVersionNumber();
> }
> -
> +
> /**
> * This returns a page instance that is rollbacked the number of
> versions
> * that is specified compared to the current page.
> @@ -466,9 +549,10 @@
> * This returns a page instance that is rollbacked the number of
> versions
> * that is specified compared to the current page.
> *
> - * This is a rollback including ajax versions.
> + * This is a rollback including ajax versions.
> *
> - * @param numberOfVersions to rollback
> + * @param numberOfVersions
> + * to rollback
> * @return
> */
> public final Page rollbackPage(int numberOfVersions)
> @@ -473,10 +557,11 @@
> */
> public final Page rollbackPage(int numberOfVersions)
> {
> - Page page = versionManager == null? this :
> versionManager.rollbackPage(numberOfVersions);
> + Page page = versionManager == null ? this :
> versionManager.rollbackPage(numberOfVersions);
> getSession().touch(page);
> return page;
> }
> +
> /**
> * @return Returns feedback messages from all components in this page
> * (including the page itself).
> @@ -487,6 +572,7 @@
> {
> feedbackMessages = new FeedbackMessages();
> }
> + assignFeedbackMessagesToPage();
> return feedbackMessages;
> }
>
> @@ -610,7 +696,8 @@
> }
>
> // If we went all the way back to the original
> page
> - if (page != null &&
> page.getCurrentVersionNumber() == 0 &&
> page.getAjaxVersionNumber() == 0)
> + if (page != null &&
> page.getCurrentVersionNumber() == 0
> + && page.getAjaxVersionNumber()
> == 0)
> {
> // remove version info
> page.versionManager = null;
> @@ -861,15 +948,16 @@
> */
> public String toString()
> {
> - if(versionManager != null)
> + if (versionManager != null)
> {
> - return "[Page class = " + getClass().getName() + ", id
> = " + getId() +
> - ", version = " +
> versionManager.getCurrentVersionNumber() + ", ajax
> = " +
> - versionManager.getAjaxVersionNumber() + "]";
> + return "[Page class = " + getClass().getName() + ", id
> = " +
> getId() + ", version = "
> + +
> versionManager.getCurrentVersionNumber() + ", ajax = "
> + + versionManager.getAjaxVersionNumber()
> + "]";
> }
> else
> {
> - return "[Page class = " + getClass().getName() + ", id
> = " +
> getId() + ", version = " + 0 + "]";
> + return "[Page class = " + getClass().getName() + ", id
> = " +
> getId() + ", version = "
> + + 0 + "]";
> }
> }
>
> @@ -928,7 +1016,7 @@
> }
>
> endVersion();
> -
> +
> super.onDetach();
> }
>
> @@ -1253,7 +1341,7 @@
> // this effectively means that change tracking begins after the
> // first request to a page completes.
> setFlag(FLAG_TRACK_CHANGES, true);
> -
> +
> // If a new version was created
> if (getFlag(FLAG_NEW_VERSION))
> {
> @@ -1362,14 +1450,14 @@
> }
>
> /**
> - * Call this method when the current (ajax) request shouldn't merge
> - * the changes that are happening to the page with the previous version.
> + * Call this method when the current (ajax) request shouldn't merge the
> + * changes that are happening to the page with the previous version.
> *
> - * This is for example needed when you want to redirect to this
> - * page in an ajax request and then you do want to version normally..
> + * This is for example needed when you want to redirect to this page in
> an
> + * ajax request and then you do want to version normally..
> *
> - * This method doesn't do anything if the getRequest().mergeVersion
> - * doesn't return true.
> + * This method doesn't do anything if the getRequest().mergeVersion
> doesn't
> + * return true.
> */
> public final void ignoreVersionMerge()
> {
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share
> your
> opinions on IT & business topics through brief surveys-and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> _______________________________________________
> Wicket-user mailing list
> Wicket-user@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/wicket-user
>
>
--
View this message in context:
http://www.nabble.com/error%28...%29-No-page-found-for-component-tf3497125.html#a9774829
Sent from the Wicket - User mailing list archive at Nabble.com.
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Wicket-user mailing list
Wicket-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-user
