Updated Branches: refs/heads/master f430bf75a -> 6185761df
WICKET-4888 introduce FencedFeedbackPanel Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6185761d Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6185761d Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6185761d Branch: refs/heads/master Commit: 6185761dfbe141fb990c4ea8fc8d941b287df80a Parents: f430bf7 Author: Igor Vaynberg <igor.vaynb...@gmail.com> Authored: Sun Nov 25 21:11:54 2012 -0800 Committer: Igor Vaynberg <igor.vaynb...@gmail.com> Committed: Sun Nov 25 21:11:54 2012 -0800 ---------------------------------------------------------------------- .../apache/wicket/feedback/FeedbackCollector.java | 29 ++- .../wicket/feedback/FeedbackMessagesModel.java | 15 +- .../wicket/feedback/FencedFeedbackPanel.java | 202 ++++++++++++++ .../markup/html/panel/FencedFeedbackPanelTest.java | 204 +++++++++++++++ 4 files changed, 444 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/6185761d/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackCollector.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackCollector.java b/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackCollector.java index b6b7967..e537df3 100755 --- a/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackCollector.java +++ b/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackCollector.java @@ -30,7 +30,7 @@ import org.apache.wicket.util.visit.IVisitor; * * @author igor */ -public final class FeedbackCollector +public class FeedbackCollector { private final Component component; private boolean includeSession = true; @@ -65,7 +65,7 @@ public final class FeedbackCollector * @param value * @return {@code this} for chaining */ - public FeedbackCollector setIncludeSession(boolean value) + public final FeedbackCollector setIncludeSession(boolean value) { includeSession = value; return this; @@ -78,7 +78,7 @@ public final class FeedbackCollector * @param value * @return {@code this} for chaining */ - public FeedbackCollector setRecursive(boolean value) + public final FeedbackCollector setRecursive(boolean value) { recursive = value; return this; @@ -89,7 +89,7 @@ public final class FeedbackCollector * * @return a {@link List} of collected messages */ - public List<FeedbackMessage> collect() + public final List<FeedbackMessage> collect() { return collect(IFeedbackMessageFilter.ALL); } @@ -100,7 +100,7 @@ public final class FeedbackCollector * @param filter * @return a {@link List} of collected messages */ - public List<FeedbackMessage> collect(final IFeedbackMessageFilter filter) + public final List<FeedbackMessage> collect(final IFeedbackMessageFilter filter) { final List<FeedbackMessage> messages = new ArrayList<FeedbackMessage>(); @@ -122,6 +122,12 @@ public final class FeedbackCollector @Override public void component(Component object, IVisit<Void> visit) { + if (!shouldRecurseInto(object)) + { + visit.dontGoDeeper(); + return; + } + if (object.hasFeedbackMessage()) { messages.addAll(object.getFeedbackMessages().messages(filter)); @@ -132,4 +138,17 @@ public final class FeedbackCollector return messages; } + + /** + * Determines whether or not recursive message collection should continue into the specified + * component. If returning {@code false} feedback messages from the specified component nor any + * of its children will be included. + * + * @param component + * @return + */ + protected boolean shouldRecurseInto(Component component) + { + return true; + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/6185761d/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessagesModel.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessagesModel.java b/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessagesModel.java index 2cf51b9..a4e7dbd 100644 --- a/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessagesModel.java +++ b/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessagesModel.java @@ -103,7 +103,7 @@ public class FeedbackMessagesModel implements IModel<List<FeedbackMessage>> if (messages == null) { // Get filtered messages from page where component lives - messages = new FeedbackCollector(pageResolvingComponent.getPage()).collect(filter); + messages = collectMessages(pageResolvingComponent, filter); // Sort the list before returning it if (sortingComparator != null) @@ -122,6 +122,19 @@ public class FeedbackMessagesModel implements IModel<List<FeedbackMessage>> } /** + * Collects feedback messages + * + * @param pageResolvingComponent + * @param filter + * @return list of feedback messages + */ + protected List<FeedbackMessage> collectMessages(Component pageResolvingComponent, + IFeedbackMessageFilter filter) + { + return new FeedbackCollector(pageResolvingComponent.getPage()).collect(filter); + } + + /** * @param filter * Filter to apply to model * @return this http://git-wip-us.apache.org/repos/asf/wicket/blob/6185761d/wicket-core/src/main/java/org/apache/wicket/feedback/FencedFeedbackPanel.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/feedback/FencedFeedbackPanel.java b/wicket-core/src/main/java/org/apache/wicket/feedback/FencedFeedbackPanel.java new file mode 100644 index 0000000..c39250a --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/feedback/FencedFeedbackPanel.java @@ -0,0 +1,202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.feedback; + +import java.util.List; + +import org.apache.wicket.Component; +import org.apache.wicket.MetaDataKey; +import org.apache.wicket.Session; +import org.apache.wicket.markup.html.panel.FeedbackPanel; + +/** + * A specialized feedback panel that only displays messages from inside a fence defined by a + * container component. Instances will not show messages coming from inside a nested fence, allowing + * the nesting of these panels to work correctly without displaying the same feedback message twice. + * A constructor that does not takes a fencing component creates a catch-all panel that shows + * messages that do not come from inside any fence or from the {@link Session}. + * + * <h2>IN DEPTH EXPLANATION</h2> + * <p> + * It is often very useful to have feedback panels that show feedback that comes from inside a + * certain container only. For example given a page with the following structure: + * </p> + * + * <pre> + * Page + * Form1 + * Feedback1 + * Input1 + * Form2 + * Feedback2 + * Input2 + * </pre> + * <p> + * we want Feedback2 to show messages originating only from inside Form2 and Feedback1 to show + * messages only originating from Form1 but not Form2 (because messages originating from Form2 are + * already shown by Feedback2). + * </p> + * <p> + * It is fairly simple to configure Feedback2 - a {@link ContainerFeedbackMessageFilter} added to + * the regular {@link FeedbackPanel} will do the trick. The hard part is configuring Feedback1. We + * can add a {@link ContainerFeedbackMessageFilter} to it, but since Form2 is inside Form1 the + * container filter will allow messages from both Form1 and Form2 to be added to FeedbackPanel1. + * </p> + * <p> + * This is where the {@link FencedFeedbackPanel} comes in. All we have to do is to make + * FeedbackPanel2 a {@link FencedFeedbackPanel} with the fencing component defined as Form2 and + * Feedback1 a {@link FencedFeedbackPanel} with the fencing component defiend as Form1. + * {@link FencedFeedbackPanel} will only show messages that original from inside its fencing + * component and not from inside any descendant component that acts as a fence for another + * {@link FencedFeedbackPanel}. + * </p> + * <p> + * When created with a {@code null} fencing component or using a constructor that does not take one + * the panel will only display messages that do not come from inside a fence. It will also display + * messages that come from {@link Session}. This acts as a catch-all panels showing messages that + * would not be shown using any other instance of the {@link FencedFeedbackPanel} created witha + * fencing component. There is usually one instance of such a panel at the top of the page to + * display notifications of success. + * </p> + * + * @author igor + */ +public class FencedFeedbackPanel extends FeedbackPanel +{ + private static final long serialVersionUID = 1L; + + private static MetaDataKey<Integer> FENCE_KEY = new MetaDataKey<Integer>() + { + private static final long serialVersionUID = 1L; + }; + + private final Component fence; + + /** + * Creates a catch-all feedback panel that will show messages not coming from any fence, + * including messages coming from {@link Session} + * + * @param id + */ + public FencedFeedbackPanel(String id) + { + this(id, (Component)null); + } + + /** + * Creates a feedback panel that will only show messages if they original from, or inside of, + * the {@code fence} component and not from any inner fence. + * + * @param id + * @param fence + */ + public FencedFeedbackPanel(String id, Component fence) + { + this(id, fence, null); + } + + /** + * Creates a catch-all instance witha filter. + * + * @see #FencedFeedbackPanel(String) + * + * @param id + * @param filter + */ + public FencedFeedbackPanel(String id, IFeedbackMessageFilter filter) + { + this(id, null, filter); + } + + /** + * Creates a fenced feedback panel with a filter. + * + * @see #FencedFeedbackPanel(String, Component) + * + * @param id + * @param fence + * @param filter + */ + public FencedFeedbackPanel(String id, Component fence, IFeedbackMessageFilter filter) + { + super(id, filter); + this.fence = fence; + if (fence != null) + { + Integer count = fence.getMetaData(FENCE_KEY); + count = count == null ? 1 : count + 1; + fence.setMetaData(FENCE_KEY, count); + } + } + + @Override + protected void onRemove() + { + super.onRemove(); + if (fence != null) + { + // decrement the fence count + + Integer count = fence.getMetaData(FENCE_KEY); + count = count == 1 ? null : count - 1; + fence.setMetaData(FENCE_KEY, count); + } + } + + @Override + protected FeedbackMessagesModel newFeedbackMessagesModel() + { + return new FeedbackMessagesModel(this) + { + private static final long serialVersionUID = 1L; + + @Override + protected List<FeedbackMessage> collectMessages(Component panel, + IFeedbackMessageFilter filter) + { + if (fence == null) + { + // this is the catch-all panel + + return new FeedbackCollector(panel.getPage()) + { + @Override + protected boolean shouldRecurseInto(Component component) + { + return component.getMetaData(FENCE_KEY) == null; + } + }.collect(filter); + } + else + { + // this is a fenced panel + + return new FeedbackCollector(fence) + { + @Override + protected boolean shouldRecurseInto(Component component) + { + // only recurse into components that are not fences + + return component.getMetaData(FENCE_KEY) == null; + } + }.setIncludeSession(false).collect(filter); + } + } + }; + } +} http://git-wip-us.apache.org/repos/asf/wicket/blob/6185761d/wicket-core/src/test/java/org/apache/wicket/markup/html/panel/FencedFeedbackPanelTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/panel/FencedFeedbackPanelTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/panel/FencedFeedbackPanelTest.java new file mode 100644 index 0000000..af6b10a --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/panel/FencedFeedbackPanelTest.java @@ -0,0 +1,204 @@ +package org.apache.wicket.markup.html.panel; + +import static org.junit.Assert.*; + +import org.apache.wicket.Component; +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.feedback.ErrorLevelFeedbackMessageFilter; +import org.apache.wicket.feedback.FeedbackMessage; +import org.apache.wicket.feedback.FencedFeedbackPanel; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebMarkupContainer; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.basic.Label; +import org.apache.wicket.markup.html.form.Form; +import org.apache.wicket.markup.html.form.TextField; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTesterScope; +import org.junit.Rule; +import org.junit.Test; + +/** + * Tests {@link FencedFeedbackPanel} + * + * @author igor + */ +public class FencedFeedbackPanelTest +{ + @Rule + public WicketTesterScope scope = new WicketTesterScope(); + + @Test + public void fencing() + { + TestPage page = scope.getTester().startPage(TestPage.class); + page.containerInput.error("error"); + + // container messages should be visible to container feedbacks but not outside + + assertTrue(page.containerFeedback.anyMessage()); + assertTrue(page.containerFeedback2.anyMessage()); + assertFalse(page.formFeedback.anyMessage()); + assertFalse(page.externalFeedback.anyMessage()); + + page = scope.getTester().startPage(TestPage.class); + page.formInput.error("error"); + + // form messages should be visible only to the form feedbacks + + assertFalse(page.containerFeedback.anyMessage()); + assertFalse(page.containerFeedback2.anyMessage()); + assertTrue(page.formFeedback.anyMessage()); + assertFalse(page.externalFeedback.anyMessage()); + + page = scope.getTester().startPage(TestPage.class); + page.externalLabel.error("error"); + + // external messages should be picked up only by catch-all feedbacks + + assertFalse(page.containerFeedback.anyMessage()); + assertFalse(page.containerFeedback2.anyMessage()); + assertFalse(page.formFeedback.anyMessage()); + assertTrue(page.externalFeedback.anyMessage()); + + page = scope.getTester().startPage(TestPage.class); + page.getSession().error("error"); + + // session scoped errors should only be picked up by catch-all feedbacks + + assertFalse(page.containerFeedback.anyMessage()); + assertFalse(page.containerFeedback2.anyMessage()); + assertFalse(page.formFeedback.anyMessage()); + assertTrue(page.externalFeedback.anyMessage()); + } + + @Test + public void filtering() + { + TestPage page = scope.getTester().startPage(TestPage.class); + + // set a filter that will only allow errors or higher + + page.containerFeedback.setFilter(new ErrorLevelFeedbackMessageFilter(FeedbackMessage.ERROR)); + + // report an info message - should be filtered out + + page.containerInput.info("info"); + + // check info message was filtered out + + assertFalse(page.containerFeedback.anyMessage()); + assertTrue(page.containerFeedback2.anyMessage()); + + // ensure filtered out messages dont leak + + assertFalse(page.formFeedback.anyMessage()); + assertFalse(page.externalFeedback.anyMessage()); + + // same setup + + page = scope.getTester().startPage(TestPage.class); + + page.containerFeedback.setFilter(new ErrorLevelFeedbackMessageFilter(FeedbackMessage.ERROR)); + + // but now with an error message that should not be filtered out + + page.containerInput.error("info"); + + // check message was not filtered out + + assertTrue(page.containerFeedback.anyMessage()); + assertTrue(page.containerFeedback2.anyMessage()); + + // and that it should not leak + + assertFalse(page.formFeedback.anyMessage()); + assertFalse(page.externalFeedback.anyMessage()); + + } + + @Test + public void moving() + { + TestPage page = scope.getTester().startPage(TestPage.class); + page.containerInput.error("error"); + + assertTrue(page.containerFeedback.anyMessage()); + assertTrue(page.containerFeedback2.anyMessage()); + + // does not propagate out of container + assertFalse(page.formFeedback.anyMessage()); + + // remove one of two fencing feedback panels + + page = scope.getTester().startPage(TestPage.class); + page.containerFeedback.remove(); + + page.containerInput.error("error"); + + assertTrue(page.containerFeedback2.anyMessage()); + + // still does not propagate out of container because there is still a fencing panel + assertFalse(page.formFeedback.anyMessage()); + + // remove the last fencing feedback panel + + page = scope.getTester().startPage(TestPage.class); + page.containerFeedback.remove(); + page.containerFeedback2.remove(); + + page.containerInput.error("error"); + + // now propagates out of container + assertTrue(page.formFeedback.anyMessage()); + + } + + public static class TestPage extends WebPage implements IMarkupResourceStreamProvider + { + FencedFeedbackPanel externalFeedback, formFeedback, containerFeedback, containerFeedback2; + Component externalLabel, formInput, containerInput; + + public TestPage() + { + externalFeedback = new FencedFeedbackPanel("feedback"); + externalLabel = new Label("externalLabel"); + add(externalFeedback, externalLabel); + + Form<?> form = new Form<Void>("form"); + formFeedback = new FencedFeedbackPanel("formFeedback", form); + form.add(formFeedback); + formInput = new TextField<String>("formInput"); + form.add(formInput); + WebMarkupContainer container = new WebMarkupContainer("container"); + containerFeedback = new FencedFeedbackPanel("containerFeedback", container); + containerFeedback2 = new FencedFeedbackPanel("containerFeedback2", container); + container.add(containerFeedback, containerFeedback2); + containerInput = new TextField<String>("containerInput"); + container.add(containerInput); + form.add(container); + add(form); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream(// + " <body>" + // + " <div wicket:id='feedback'/>" + // + " <div wicket:id='externalLabel'/>" + // + " <form wicket:id='form'>" + // + " <div wicket:id='formFeedback'/>" + // + " <input wicket:id='formInput' type='text'/>" + // + " <div wicket:id='container'>" + // + " <div wicket:id='containerFeedback'/>" + // + " <input wicket:id='containerInput' type='text'/>" + // + " <div wicket:id='containerFeedback2'/>" + // + " </div>" + // + " </form>" + // + "</body>"); + } + } +}
