WICKET-3335 break up and refactor monolithing dequeuing process into delegatable chunks. all working but borders
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/b9ddb888 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/b9ddb888 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/b9ddb888 Branch: refs/heads/master Commit: b9ddb88862f15be28dd0a74869489a804e4cda43 Parents: e95c712 Author: Igor Vaynberg <igor.vaynb...@gmail.com> Authored: Fri Feb 14 21:38:34 2014 -0800 Committer: Igor Vaynberg <igor.vaynb...@gmail.com> Committed: Thu Feb 20 23:37:15 2014 -0800 ---------------------------------------------------------------------- .../java/org/apache/wicket/ComponentQueue2.java | 2 + .../java/org/apache/wicket/DequeueContext.java | 152 +++++++++++ .../java/org/apache/wicket/MarkupContainer.java | 254 +++++-------------- .../markup/repeater/AbstractRepeater.java | 21 ++ 4 files changed, 240 insertions(+), 189 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/b9ddb888/wicket-core/src/main/java/org/apache/wicket/ComponentQueue2.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/ComponentQueue2.java b/wicket-core/src/main/java/org/apache/wicket/ComponentQueue2.java index ec5251c..2ce3e8f 100644 --- a/wicket-core/src/main/java/org/apache/wicket/ComponentQueue2.java +++ b/wicket-core/src/main/java/org/apache/wicket/ComponentQueue2.java @@ -25,6 +25,8 @@ import org.apache.wicket.util.lang.Args; * Manages the component queue. * * @author igor + * + * @deprecated uses too much memory compared to ComponentQueue */ class ComponentQueue2 { http://git-wip-us.apache.org/repos/asf/wicket/blob/b9ddb888/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 new file mode 100644 index 0000000..8324161 --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java @@ -0,0 +1,152 @@ +package org.apache.wicket; + +import org.apache.wicket.markup.ComponentTag; +import org.apache.wicket.markup.Markup; +import org.apache.wicket.markup.MarkupElement; +import org.apache.wicket.util.collections.ArrayListStack; + +public class DequeueContext +{ + private final Markup markup; + private int index; + private ComponentTag next; + private ArrayListStack<ComponentTag> tags = new ArrayListStack<>(); + private ArrayListStack<MarkupContainer> containers = new ArrayListStack<>(); + + public static class Bookmark + { + private final int index; + private final ComponentTag next; + private final ArrayListStack<ComponentTag> tags; + private final ArrayListStack<MarkupContainer> containers; + + private Bookmark(DequeueContext parser) + { + this.index = parser.index; + this.next = parser.next; + this.tags = new ArrayListStack<>(parser.tags); + this.containers = new ArrayListStack<>(parser.containers); + } + + private void restore(DequeueContext parser) + { + parser.index = index; + parser.next = next; + parser.tags = new ArrayListStack<ComponentTag>(tags); + parser.containers = new ArrayListStack<MarkupContainer>(containers); + } + } + + public DequeueContext(Markup markup, MarkupContainer root) + { + this.markup = markup; + containers.push(root); + next=nextTag(); + } + + public Bookmark save() + { + return new Bookmark(this); + } + + public void restore(Bookmark bookmark) + { + bookmark.restore(this); + } + + public ComponentTag peekTag() + { + return next; + } + + public ComponentTag popTag() + { + ComponentTag taken=next; + tags.push(taken); + next=nextTag(); + return taken; + } + + public void skipToCloseTag() + { + if (tags.peek().isOpen()) + { + while (!next.closes(tags.peek())) + { + next = nextTag(); + } + tags.pop(); + } + } + + private ComponentTag nextTag() + { + 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 (canDequeue(open)) + { + index++; + return tag; + } + } + } + return null; + } + + private boolean canDequeue(ComponentTag open) + { + if (containers.size() < 1) + { + // TODO queueing message: called too early + throw new IllegalStateException(); + } + for (int i = containers.size() - 1; i >= 0; i--) + { + if (containers.get(i).supportsDequeueingFrom((open))) + { + return true; + } + } + return false; + } + + public boolean isAtOpenOrOpenCloseTag() + { + return peekTag() != null && (peekTag().isOpen() || peekTag().isOpenClose()); + } + + public MarkupContainer peekContainer() + { + return containers.peek(); + } + + public void pushContainer(MarkupContainer container) + { + containers.push(container); + } + + public MarkupContainer popContainer() + { + return containers.pop(); + } + + public Component dequeue(ComponentTag tag) + { + for (int j = containers.size() - 1; j >= 0; j--) + { + MarkupContainer container = containers.get(j); + Component child = container.findComponentToDequeue(tag); + if (child != null) + { + return child; + } + } + return null; + } + +} http://git-wip-us.apache.org/repos/asf/wicket/blob/b9ddb888/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 5005f29..72f7cc9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java +++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java @@ -2080,20 +2080,6 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp private void internalDequeue() { - class Repeater - { - int parentMarkupIndex; - Iterator<Component> renderIterator; - ComponentTag tag; - - Repeater(int parentMarkupIndex, Iterator<Component> renderIterator, ComponentTag tag) - { - this.parentMarkupIndex = parentMarkupIndex; - this.renderIterator = renderIterator; - this.tag = tag; - } - } - Markup markup = getAssociatedMarkup(); if (markup == null) { @@ -2101,207 +2087,97 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp // this sometimes happens when we are in a unit test return; } - // use arraydeque? - ArrayListStack<ComponentTag> tags = new ArrayListStack<>(); - ArrayListStack<MarkupContainer> containers = new ArrayListStack<>(); - ArrayListStack<Repeater> repeaters = new ArrayListStack<>(); - containers.push(this); + DequeueContext dequeue = new DequeueContext(markup, this); - for (int i = 0; i < markup.size(); i++) + if (dequeue.peekTag() != null) { - MarkupElement element = markup.get(i); + dequeue(dequeue); + } - if (!(element instanceof ComponentTag)) - { - continue; - } - ComponentTag tag = (ComponentTag)element; + } + public void dequeue(DequeueContext dequeue) + { + + + while (dequeue.isAtOpenOrOpenCloseTag()) + { + + ComponentTag tag = dequeue.popTag(); + + // see if child is already added to parent + - if (tag instanceof WicketTag) + Component child = get(tag.getId()); // TODO queueing add this into findInQueue and + // rename it to dequeue + + if (child == null) { - ComponentTag openTag = tag.getOpenTag() == null ? tag : tag.getOpenTag(); - if (((WicketTag) tag).isBorderTag()) + // the container does not yet have a child with this id, see if we can + // dequeue + + child = dequeue.dequeue(tag); + + if (child != null) { - if (tag.isOpen()) + add(child); + if (child instanceof IQueueRegion) { - tags.push(tag); + ((MarkupContainer)child).dequeue(); } - else if (tag.isClose()) - { - tags.pop(); - } - continue; - } - else if (openTag.getAutoComponentFactory() == null) - { - // wicket tags that do not produce auto components can be ignored - continue; } } - - if (tag.isClose()) + + if (child == null || tag.isOpenClose() || !(child instanceof MarkupContainer)) { - tags.pop(); - containers.pop(); - - if (containers.peek() instanceof AbstractRepeater) - { - Repeater repeater = repeaters.peek(); - if (repeater.renderIterator.hasNext()) - { - containers.push((MarkupContainer)repeater.renderIterator.next()); - tags.push(repeater.tag); - i = repeater.parentMarkupIndex; - continue; - } - else - { - // we rendered the last item, now time to close the repeater - repeaters.pop(); - tags.pop(); - containers.pop(); - } - } + // could not dequeue, or does not contain children + + dequeue.skipToCloseTag(); } else { - String id = tag.getId(); - Component child = containers.peek().get(id); - - // see if child is already added to parent - if (child == null) - { - // the container does not yet have a child with this id, see if we can - // dequeue - for (int j = containers.size() - 1; j >= 0; j--) - { - MarkupContainer container = containers.get(j); - child = container.queue != null - ? container.queue.remove(id) - : null; - if (child != null) - { - break; - } - } - - if (child != null) - { - MarkupContainer parentContainer = containers.peek(); - boolean isInBorder = isInBorder(tags, parentContainer); - if (isInBorder) - { - ((Border) parentContainer).addToBorder(child); - } - else - { - parentContainer.add(child); - } - - if (child instanceof IQueueRegion) - { - ((MarkupContainer)child).dequeue(); - } + MarkupContainer container = (MarkupContainer)child; + dequeue.pushContainer(container); + container.dequeue(dequeue); + dequeue.popContainer(); + } - } - } - if (child == null) + if (tag.isOpen()) + { + // pull the close tag off + ComponentTag close = dequeue.popTag(); + if (!close.closes(tag)) { - // could not dequeue, skip until closing tag - - if (tag.isOpen()) - { - for (i = i + 1; i < markup.size(); i++) - { - MarkupElement e = markup.get(i); - if (e instanceof ComponentTag && e.closes(tag)) - { - break; - } - } - } - - // if (dequeueSites == null) - // { - // dequeueSites = new HashSet<String>(); - // } - // dequeueSites.add(id); - + // sanity check + // FIXME queueing message + throw new IllegalStateException(); } - else - { - if (tag.isOpen()) - { - if (child instanceof MarkupContainer) - { - containers.push((MarkupContainer)child); - tags.push(tag); + } - if (child instanceof AbstractRepeater) - { - Repeater repeater = new Repeater(i, - ((AbstractRepeater)child).iterator(), tag); - if (repeater.renderIterator.hasNext()) - { - repeaters.push(repeater); - containers - .push((MarkupContainer)repeater.renderIterator.next()); - tags.push(tag); - } - else - { - // empty repeater, skip until closing tag - for (i = i + 1; i < markup.size(); i++) - { - MarkupElement e = markup.get(i); - if (e instanceof ComponentTag && e.closes(tag)) - { - break; - } - } - i--; - continue; - } - } - } - else - { - // web component, skip until closing tag - for (i = i + 1; i < markup.size(); i++) - { - MarkupElement e = markup.get(i); - if (e instanceof ComponentTag && e.closes(tag)) - { - break; - } - } - } - } - else - { - // openclose tag, nothing to do - } - } - } } - } - - private boolean isInBorder(ArrayListStack<ComponentTag> tags, MarkupContainer parentContainer) - { - boolean result = false; - if (parentContainer instanceof Border && tags.empty() == false) - { - ComponentTag parentsTag = tags.peek(); - if (parentsTag instanceof WicketTag && ((WicketTag)parentsTag).isBorderTag()) + protected boolean supportsDequeueingFrom(ComponentTag tag) { + if (tag instanceof WicketTag) { + WicketTag wicketTag=(WicketTag)tag; + if (wicketTag.getAutoComponentFactory() != null) { - result = true; + return true; + } else { + return false; } } - return result; + return true; + } + + protected Component findComponentToDequeue(ComponentTag tag) + { + return queue == null ? null : queue.remove(tag.getId()); + } + + protected void addDequeuedComponent(Component component, ComponentTag tag) { + add(component); } } http://git-wip-us.apache.org/repos/asf/wicket/blob/b9ddb888/wicket-core/src/main/java/org/apache/wicket/markup/repeater/AbstractRepeater.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/repeater/AbstractRepeater.java b/wicket-core/src/main/java/org/apache/wicket/markup/repeater/AbstractRepeater.java index 1698197..134e239 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/repeater/AbstractRepeater.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/repeater/AbstractRepeater.java @@ -21,6 +21,9 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.wicket.Component; +import org.apache.wicket.DequeueContext; +import org.apache.wicket.DequeueContext.Bookmark; +import org.apache.wicket.MarkupContainer; import org.apache.wicket.markup.IMarkupFragment; import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.model.IModel; @@ -150,4 +153,22 @@ public abstract class AbstractRepeater extends WebMarkupContainer * Callback to let the repeater know it should populate itself with its items. */ protected abstract void onPopulate(); + + @Override + public void dequeue(DequeueContext dequeue) { + if (size()>0) { + Bookmark bookmark=dequeue.save(); + + for (Component child:this) { + if (child instanceof MarkupContainer) { + dequeue.popContainer(); // pop the repeater + MarkupContainer container=(MarkupContainer) child; + dequeue.pushContainer(container); + container.dequeue(dequeue); + dequeue.restore(bookmark); + } + } + } + dequeue.skipToCloseTag(); + } }