You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2014/02/25 13:01:26 UTC
Re: git commit: WICKET-3335 refactor/cleanup component queueing - a
clean way to ignore outer tags which is necessary for proper fragment and
border support
On Tue, Feb 25, 2014 at 8:28 AM, <iv...@apache.org> wrote:
> 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 <ig...@gmail.com>
> Authored: Mon Feb 24 22:28:03 2014 -0800
> Committer: Igor Vaynberg <ig...@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
>
This class is not used at the moment
Do we need it ?
> +{
> +
> +}
>
>
> 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()
>
>