Repository: wicket Updated Branches: refs/heads/master cc5d56a50 -> 2008dfb70
WICKET-3335 refactor/cleanup component queueing - a clean way to ignore outer tags which is necessary for proper fragment and border support Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/2008dfb7 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/2008dfb7 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/2008dfb7 Branch: refs/heads/master Commit: 2008dfb7044f544d7b112cf4666dfacf42406b89 Parents: cc5d56a Author: Igor Vaynberg <igor.vaynb...@gmail.com> Authored: Mon Feb 24 22:28:03 2014 -0800 Committer: Igor Vaynberg <igor.vaynb...@gmail.com> Committed: Mon Feb 24 22:28:03 2014 -0800 ---------------------------------------------------------------------- .../java/org/apache/wicket/DequeueContext.java | 90 ++++++++++++++++++-- .../org/apache/wicket/DequeueTagAction.java | 11 +++ .../java/org/apache/wicket/IQueueRegion.java | 8 +- .../java/org/apache/wicket/MarkupContainer.java | 59 +++++++------ .../wicket/markup/html/border/Border.java | 59 ++----------- .../html/panel/DequeueMarkupFragment.java | 6 ++ .../wicket/markup/html/panel/Fragment.java | 16 ++-- .../wicket/queueing/ComponentQueueingTest.java | 24 +++++- 8 files changed, 168 insertions(+), 105 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java b/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java index a5bb925..2717e8c 100644 --- a/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java +++ b/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java @@ -34,6 +34,8 @@ public final class DequeueContext private int index; private ComponentTag next; private ArrayListStack<ComponentTag> tags = new ArrayListStack<>(); + private final boolean skipFirst; + private ComponentTag first; private ArrayListStack<MarkupContainer> containers = new ArrayListStack<>(); @@ -62,9 +64,10 @@ public final class DequeueContext } } - public DequeueContext(IMarkupFragment markup, MarkupContainer root) + public DequeueContext(IMarkupFragment markup, MarkupContainer root, boolean skipFirst) { this.markup = markup; + this.skipFirst = skipFirst; containers.push(root); next=nextTag(); } @@ -105,6 +108,12 @@ public final class DequeueContext public ComponentTag takeTag() { ComponentTag taken=next; + + if (taken == null) + { + return null; + } + if (taken.isOpen() && !taken.hasNoCloseTag()) { tags.push(taken); @@ -130,27 +139,89 @@ public final class DequeueContext private ComponentTag nextTag() { + if (skipFirst && first == null) + { + for (; index < markup.size(); index++) + { + MarkupElement element = markup.get(index); + if (element instanceof ComponentTag) + { + first = (ComponentTag)element; + index++; + break; + } + } + } + for (; index < markup.size(); index++) { MarkupElement element = markup.get(index); if (element instanceof ComponentTag) { ComponentTag tag = (ComponentTag)element; - ComponentTag open = tag.isClose() ? tag.getOpenTag() : tag; - if (open != null && canDequeueTag(open)) + + if (tag.isOpen() || tag.isOpenClose()) { - index++; - return tag; + DequeueTagAction action = canDequeueTag(tag); + switch (action) + { + case IGNORE : + continue; + case DEQUEUE : + index++; + return tag; + case SKIP : // skip to close tag + boolean found = false; + for (; index < markup.size(); index++) + { + if ((markup.get(index) instanceof ComponentTag) + && ((ComponentTag)markup.get(index)).closes(tag)) + { + found = true; + break; + } + } + if (!found) + { + throw new IllegalStateException( + "Could not find close tag for tag: " + tag); + } + + } + } + else + { + // closed tag + ComponentTag open = tag.isClose() ? tag.getOpenTag() : tag; + + if (skipFirst && first != null && open == first) + { + continue; + } + + switch (canDequeueTag(open)) + { + case DEQUEUE : + index++; + return tag; + case IGNORE : + continue; + case SKIP : + throw new IllegalStateException( + "Should not see closed tag of skipped open tag: " + tag); + } } } } return null; } - private boolean canDequeueTag(ComponentTag open) + private DequeueTagAction canDequeueTag(ComponentTag open) { Args.notNull(open, "open"); + DequeueTagAction action = null; + if (containers.size() < 1) { // TODO queueing message: called too early @@ -158,12 +229,13 @@ public final class DequeueContext } for (int i = containers.size() - 1; i >= 0; i--) { - if (containers.get(i).canDequeueTag((open))) + action = containers.get(i).canDequeueTag((open)); + if (action != null) { - return true; + return action; } } - return false; + return DequeueTagAction.IGNORE; } /** http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/DequeueTagAction.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/DequeueTagAction.java b/wicket-core/src/main/java/org/apache/wicket/DequeueTagAction.java new file mode 100644 index 0000000..5981cb1 --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/DequeueTagAction.java @@ -0,0 +1,11 @@ +package org.apache.wicket; + +public enum DequeueTagAction +{ + /** dequeue the tag */ + DEQUEUE, + /** skip this tag and all its children */ + SKIP, + /** ignore this tag, skip it but do not skip its children */ + IGNORE; +} http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java b/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java index c0e431a..4bad6f7 100644 --- a/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java +++ b/wicket-core/src/main/java/org/apache/wicket/IQueueRegion.java @@ -16,7 +16,6 @@ */ package org.apache.wicket; -import org.apache.wicket.markup.IMarkupFragment; /** * Demarcates components that act as a root can dequeue children. These are usually components with @@ -33,13 +32,14 @@ import org.apache.wicket.markup.IMarkupFragment; public interface IQueueRegion { /** - * Gets the markup that will be used to dequeue components in this container. Usually containers - * will return their associated markup by simply delegating to + * Creates a new {@link DequeueContext} that will be used to dequeue children of this region. + * + * Usually containers will create a context with their associated markup by getting it via * {@link MarkupContainer#getAssociatedMarkup()}, but components that do not render markup in a * standard way (such as repeaters and borders) may choose to override this method to implement * custom behavior for the dequeueing process. */ - public IMarkupFragment getDequeueMarkup(); + public DequeueContext newDequeueContext(); /** * Starts component dequeueing on this {@link IQueueRegion}. This is the entry point into the http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java index 1aba865..616654f 100644 --- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java +++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java @@ -1525,21 +1525,14 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp private void dequeueAutoComponents() { // dequeue auto components - IMarkupFragment markup = getDequeueMarkup(); - if (markup != null) + DequeueContext context = newDequeueContext(); + if (context != null && context.peekTag() != null) { - // make sure we have markup, when running inside tests we wont - for (int i = 0; i < markup.size(); i++) + for (ComponentTag tag = context.takeTag(); tag != null; tag = context.takeTag()) { - MarkupElement element = markup.get(i); - if (element instanceof ComponentTag) + if (tag.getAutoComponentFactory() != null) { - ComponentTag tag = (ComponentTag)element; - if (tag.getAutoComponentFactory() != null) - { - Component auto = tag.getAutoComponentFactory().newComponent(tag); - queue(auto); - } + queue(tag.getAutoComponentFactory().newComponent(tag)); } } } @@ -2039,16 +2032,13 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp setRequestFlag(RFLAG_CONTAINER_DEQUEING, true); try { - IMarkupFragment markup = getDequeueMarkup(); - if (markup == null) + DequeueContext dequeue = newDequeueContext(); + if (dequeue == null) { - // markup not found, skip dequeuing - // this sometimes happens when we are in a unit test + // not ready to dequeue yet return; } - DequeueContext dequeue = new DequeueContext(markup, this); - if (dequeue.peekTag() != null) { dequeue(dequeue); @@ -2083,9 +2073,8 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp // see if child is already added to parent - Component child = get(tag.getId()); // TODO queueing add this into findInQueue and - // rename it to dequeue - + Component child = get(tag.getId()); + if (child == null) { // the container does not yet have a child with this id, see if we can @@ -2147,12 +2136,17 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp } - /** @see IQueueRegion#getDequeueMarkup() */ - public IMarkupFragment getDequeueMarkup() + /** @see IQueueRegion#newDequeueContext() */ + public DequeueContext newDequeueContext() { - return getAssociatedMarkup(); + Markup markup = getAssociatedMarkup(); + if (markup == null) + { + return null; + } + return new DequeueContext(markup, this, false); } - + /** * Checks if this container can dequeue a child represented by the specified tag. This method * should be overridden when containers can dequeue components represented by non-standard tags. @@ -2164,25 +2158,30 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp * * @param tag */ - protected boolean canDequeueTag(ComponentTag tag) + protected DequeueTagAction canDequeueTag(ComponentTag tag) { if (tag instanceof WicketTag) { WicketTag wicketTag = (WicketTag)tag; if (wicketTag.isContainerTag()) { - return true; + return DequeueTagAction.DEQUEUE; } + else if (wicketTag.getAutoComponentFactory() != null) { - return true; + return DequeueTagAction.DEQUEUE; + } + else if (wicketTag.isFragmentTag()) + { + return DequeueTagAction.SKIP; } else { - return false; + return null; // dont know } } - return true; + return DequeueTagAction.DEQUEUE; } /** http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java index a53b780..f60fc8a 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java @@ -17,6 +17,8 @@ package org.apache.wicket.markup.html.border; import org.apache.wicket.Component; +import org.apache.wicket.DequeueContext; +import org.apache.wicket.DequeueTagAction; import org.apache.wicket.IQueueRegion; import org.apache.wicket.MarkupContainer; import org.apache.wicket.markup.ComponentTag; @@ -538,7 +540,7 @@ public abstract class Border extends WebMarkupContainer implements IComponentRes } @Override - public IMarkupFragment getDequeueMarkup() + public DequeueContext newDequeueContext() { Border border=findParent(Border.class); IMarkupFragment fragment = border.getMarkup(); @@ -548,56 +550,7 @@ public abstract class Border extends WebMarkupContainer implements IComponentRes return null; } - /* - * we want to get the contents of the border here (the markup that - * is represented by the body tag) to do this we need to strip the - * tag that the border is attached to (usually the first tag) - */ - - int i = 0; - while (i < fragment.size()) - { - // TODO queueing Use fragment.find(border.getId()); instead ?! - MarkupElement element = fragment.get(i); - if (element instanceof ComponentTag - && ((ComponentTag)element).getId().equals(border.getId())) - { - break; - } - i++; - } - - if (i >= fragment.size()) - { - throw new IllegalStateException("Could not find starting border tag for border: " - + border.getId() + " in markup: " + fragment); - } - - - /* - * (i) is now at the border tag, find the next component tag (the tag that will belong - * to the first direct child - */ - - i++; - while (i < fragment.size()) - { - MarkupElement element = fragment.get(i); - if (element instanceof ComponentTag) - { - break; - } - i++; - } - - ComponentTag tag = (ComponentTag)fragment.get(i); - if (tag.isClose()) - { - // this closes the border tag, border only has raw markup - return null; - } - - return new MarkupFragment(fragment, i); + return new DequeueContext(fragment, this, true); } @Override @@ -636,11 +589,11 @@ public abstract class Border extends WebMarkupContainer implements IComponentRes } @Override - protected boolean canDequeueTag(ComponentTag tag) + protected DequeueTagAction canDequeueTag(ComponentTag tag) { if ((tag instanceof WicketTag) && ((WicketTag)tag).isBodyTag()) { - return true; + return DequeueTagAction.DEQUEUE; } return super.canDequeueTag(tag); http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/DequeueMarkupFragment.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/DequeueMarkupFragment.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/DequeueMarkupFragment.java new file mode 100644 index 0000000..5e0a874 --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/DequeueMarkupFragment.java @@ -0,0 +1,6 @@ +package org.apache.wicket.markup.html.panel; + +public class DequeueMarkupFragment +{ + +} http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/Fragment.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/Fragment.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/Fragment.java index 676cca5..e915714 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/Fragment.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/Fragment.java @@ -17,12 +17,10 @@ package org.apache.wicket.markup.html.panel; import org.apache.wicket.Component; +import org.apache.wicket.DequeueContext; import org.apache.wicket.IQueueRegion; import org.apache.wicket.MarkupContainer; import org.apache.wicket.markup.IMarkupFragment; -import org.apache.wicket.markup.MarkupElement; -import org.apache.wicket.markup.MarkupFragment; -import org.apache.wicket.markup.WicketTag; import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.model.IModel; import org.apache.wicket.util.lang.Args; @@ -134,8 +132,16 @@ public class Fragment extends WebMarkupContainer implements IQueueRegion return associatedMarkupId; } + @Override - public IMarkupFragment getDequeueMarkup() { - return newMarkupSourcingStrategy().getMarkup(this, null); + public DequeueContext newDequeueContext() + { + IMarkupFragment markup = newMarkupSourcingStrategy().getMarkup(this, null); + if (markup == null) + { + return null; + } + + return new DequeueContext(markup, this, true); } } http://git-wip-us.apache.org/repos/asf/wicket/blob/2008dfb7/wicket-core/src/test/java/org/apache/wicket/queueing/ComponentQueueingTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/queueing/ComponentQueueingTest.java b/wicket-core/src/test/java/org/apache/wicket/queueing/ComponentQueueingTest.java index 96f0c77..ed76db5 100644 --- a/wicket-core/src/test/java/org/apache/wicket/queueing/ComponentQueueingTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/queueing/ComponentQueueingTest.java @@ -18,6 +18,7 @@ package org.apache.wicket.queueing; import static org.apache.wicket.queueing.WicketMatchers.hasPath; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import java.util.ArrayList; @@ -56,10 +57,8 @@ public class ComponentQueueingTest extends WicketTestCase MarkupContainer a = new A(), b = new B(), c = new C(); p.queue(b, c, a); - - tester.startPage(p); - assertThat(p, hasPath(a, b, c)); + tester.startPage(p); } /** {@code [a[b,c]] -> [a[b[c]]] } */ @@ -600,7 +599,7 @@ public class ComponentQueueingTest extends WicketTestCase @Test - public void nestedBorders() + public void border_nested() { MarkupContainer a = new A(), b = new B(), c= new C(), d = new D(), r = new R(), s = new S(); @@ -644,6 +643,23 @@ public class ComponentQueueingTest extends WicketTestCase assertThat(page, hasPath(new Path(fragment, r))); assertThat(page, hasPath(new Path(fragment, s))); } + + @Test + public void fragment_doesNotDequeueAcrossRegion() + { + MarkupContainer a = new A(); + + TestPage page = new TestPage(); + page.setPageMarkup("<f wicket:id='fragment'></f><wicket:fragment wicket:id='f'><a wicket:id='a'></a></wicket:fragment>"); + + Fragment fragment = new Fragment("fragment", "f", page); + + page.queue(a, fragment); + + assertThat(page, hasPath(new Path(fragment))); + assertThat(a.getParent(), is(nullValue())); + } + @Test public void containerTag1()
