WICKET-5732 Improve component queuing and auto component (code review with MArtin Grigorov)
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/9274ee24 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/9274ee24 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/9274ee24 Branch: refs/heads/master Commit: 9274ee2493f277dd7081991cd1558f57321443ec Parents: ff5aa63 Author: Andrea Del Bene <[email protected]> Authored: Wed Oct 22 18:53:16 2014 +0200 Committer: Andrea Del Bene <[email protected]> Committed: Fri Oct 31 17:40:26 2014 +0100 ---------------------------------------------------------------------- .../java/org/apache/wicket/DequeueContext.java | 2 +- .../java/org/apache/wicket/MarkupContainer.java | 141 ++++++++++--------- .../org/apache/wicket/markup/ComponentTag.java | 23 ++- .../markup/parser/filter/HtmlHandler.java | 26 ++-- .../filter/RelativePathPrefixHandler.java | 69 +++++---- 5 files changed, 129 insertions(+), 132 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/9274ee24/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 0f0b403..b3071e9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java +++ b/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java @@ -279,7 +279,7 @@ public final class DequeueContext } /** - * Searches the container stack for a component that can be dequeued + * Searches the container stack for a component that can be dequeude * * @param tag * @return http://git-wip-us.apache.org/repos/asf/wicket/blob/9274ee24/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 8a15571..13ffb0a 100644 --- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java +++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java @@ -941,9 +941,8 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp /* * dequeue both normal and auto components * - * https://issues.apache.org/jira/browse/WICKET-5724 */ - childContainer.dequeue(); + childContainer.dequeue(); } } @@ -1515,18 +1514,19 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp { for (ComponentTag tag = context.takeTag(); tag != null; tag = context.takeTag()) { - ComponentTag.IAutoComponentFactory autoComponentFactory = tag.getAutoComponentFactory(); - if (autoComponentFactory != null) - { - queue(autoComponentFactory.newComponent(this, tag)); - } - - // Every component is responsible just for its own auto components - // so skip to the close tag. - if (tag.isOpen() && !tag.hasNoCloseTag()) - { - context.skipToCloseTag(); - } + ComponentTag.IAutoComponentFactory autoComponentFactory = tag + .getAutoComponentFactory(); + if (autoComponentFactory != null) + { + queue(autoComponentFactory.newComponent(this, tag)); + } + + // Every component is responsible just for its own auto components + // so skip to the close tag. + if (tag.isOpen() && !tag.hasNoCloseTag()) + { + context.skipToCloseTag(); + } } } } @@ -1977,30 +1977,30 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp */ public void dequeue() { - if (this instanceof IQueueRegion) - { - DequeueContext dequeue = newDequeueContext(); - dequeuePreamble(dequeue); - } - else - { - MarkupContainer queueRegion = (MarkupContainer)findParent(IQueueRegion.class); - - if (!queueRegion.getRequestFlag(RFLAG_CONTAINER_DEQUEING)) - { - queueRegion.dequeue(); - } - } + if (this instanceof IQueueRegion) + { + DequeueContext dequeue = newDequeueContext(); + dequeuePreamble(dequeue); + } + else + { + MarkupContainer queueRegion = (MarkupContainer)findParent(IQueueRegion.class); + + if (queueRegion != null && !queueRegion.getRequestFlag(RFLAG_CONTAINER_DEQUEING)) + { + queueRegion.dequeue(); + } + } } - + /** - * Run preliminary operations before running {@link #dequeue(DequeueContext)}. More in detail - * it throws an exception if the container is already dequeuing, and it also takes care of - * setting flag {@code RFLAG_CONTAINER_DEQUEING} to true before running {@link #dequeue(DequeueContext)} + * Run preliminary operations before running {@link #dequeue(DequeueContext)}. More in detail it + * throws an exception if the container is already dequeuing, and it also takes care of setting + * flag {@code RFLAG_CONTAINER_DEQUEING} to true before running {@link #dequeue(DequeueContext)} * and setting it back to false after dequeuing is completed. * * @param dequeue - * the dequeue context to use + * the dequeue context to use */ protected void dequeuePreamble(DequeueContext dequeue) { @@ -2061,7 +2061,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp addDequeuedComponent(child, tag); } } - + if (tag.isOpen() && !tag.hasNoCloseTag()) { dequeueChild(child, tag, dequeue); @@ -2071,53 +2071,54 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp } /** - * Propagates dequeuing to children components. + * Propagates dequeuing to child component. * * @param child - * the children component + * the child component * @param tag - * the children tag + * the child tag * @param dequeue * the dequeue context to use */ private void dequeueChild(Component child, ComponentTag tag, DequeueContext dequeue) - { - if (child == null || child instanceof IQueueRegion) - { - // could not dequeue, or is a dequeue container - dequeue.skipToCloseTag(); - - } - else if (child instanceof MarkupContainer) - { - //propagate dequeuing to containers - MarkupContainer childContainer = (MarkupContainer)child; - - dequeue.pushContainer(childContainer); - childContainer.dequeue(dequeue); - dequeue.popContainer(); - } - - // pull the close tag off - ComponentTag close = dequeue.takeTag(); - if (!close.closes(tag)) - { - // sanity check - throw new IllegalStateException(String.format("Tag '%s' should be the closing one for '%s'", close, tag)); - } - - } + { + if (child == null || child instanceof IQueueRegion) + { + // could not dequeue, or is a dequeue container + dequeue.skipToCloseTag(); + + } + else if (child instanceof MarkupContainer) + { + // propagate dequeuing to containers + MarkupContainer childContainer = (MarkupContainer)child; + + dequeue.pushContainer(childContainer); + childContainer.dequeue(dequeue); + dequeue.popContainer(); + } + + // pull the close tag off + ComponentTag close = dequeue.takeTag(); + if (!close.closes(tag)) + { + // sanity check + throw new IllegalStateException(String.format( + "Tag '%s' should be the closing one for '%s'", close, tag)); + } + + } /** @see IQueueRegion#newDequeueContext() */ public DequeueContext newDequeueContext() { - IMarkupFragment markup = getAssociatedMarkup(); - + IMarkupFragment markup = getAssociatedMarkup(); + if (markup == null) - { - return null; - } - + { + return null; + } + return new DequeueContext(markup, this, false); } @@ -2134,7 +2135,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp */ protected DequeueTagAction canDequeueTag(ComponentTag tag) { - if (tag instanceof WicketTag) + if (tag instanceof WicketTag) { WicketTag wicketTag = (WicketTag)tag; if (wicketTag.isContainerTag()) http://git-wip-us.apache.org/repos/asf/wicket/blob/9274ee24/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java b/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java index 08caf0d..40a1500 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java @@ -93,11 +93,8 @@ public class ComponentTag extends MarkupElement /** Render the tag as RawMarkup even if no Component can be found */ public final static int RENDER_RAW = 0x0020; - /** - * If true, the component is the parent (or the ancestor) of Wicket component explicitly added. - * (i.e. it contains child tag with a "wicket:id" attribute) - * */ - public final static int CONTAINS_WICKET_ID = 0x0040; + /** If true, the current tag contains a child or a descendant with the "wicket:id" attribute */ + public final static int CONTAINS_WICKET_ID = 0x0040; /** If close tag, than reference to the corresponding open tag */ private ComponentTag openTag; @@ -835,14 +832,14 @@ public class ComponentTag extends MarkupElement } /** - * True if the HTML tag (e.g. br) has no close tag - * - * @param hasNoCloseTag - */ - public void setContainsWicketId(boolean containsWicketId) - { - setFlag(CONTAINS_WICKET_ID, containsWicketId); - } + * True if the HTML tag (e.g. br) has no close tag + * + * @param containsWicketId + */ + public void setContainsWicketId(boolean containsWicketId) + { + setFlag(CONTAINS_WICKET_ID, containsWicketId); + } public boolean containsWicketId() { http://git-wip-us.apache.org/repos/asf/wicket/blob/9274ee24/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java index 5801b4e..9288409 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java @@ -177,19 +177,19 @@ public final class HtmlHandler extends AbstractMarkupFilter * @param tag */ private void setContainsWicketIdFlag(ComponentTag tag) - { - //check if it is a wicket:id component - String wicketIdAttr = getWicketNamespace() + ":" + "id"; - boolean hasWicketId = tag.getAttributes().get(wicketIdAttr) != null; - - if (hasWicketId) - { - for (ComponentTag componentTag : stack) - { - componentTag.setContainsWicketId(hasWicketId); - } - } - } + { + // check if it is a wicket:id component + String wicketIdAttr = getWicketNamespace() + ":" + "id"; + boolean hasWicketId = tag.getAttributes().get(wicketIdAttr) != null; + + if (hasWicketId) + { + for (ComponentTag componentTag : stack) + { + componentTag.setContainsWicketId(hasWicketId); + } + } + } /** * Gets whether this tag does not require a closing tag. http://git-wip-us.apache.org/repos/asf/wicket/blob/9274ee24/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java index 1ab60c8..0523d9a 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java @@ -101,13 +101,13 @@ public final class RelativePathPrefixHandler extends AbstractMarkupFilter }; private static final IAutoComponentFactory FACTORY = new IAutoComponentFactory() - { - @Override - public Component newComponent(MarkupContainer container, ComponentTag tag) - { - return new TransparentWebMarkupContainer(tag.getId()); - } - }; + { + @Override + public Component newComponent(MarkupContainer container, ComponentTag tag) + { + return new TransparentWebMarkupContainer(tag.getId()); + } + }; /** @@ -163,9 +163,9 @@ public final class RelativePathPrefixHandler extends AbstractMarkupFilter { if (tag.getId() == null) { - tag.setId(getWicketRelativePathPrefix(null) + tag.setId(getWicketRelativePathPrefix(null) + componentIndex.getAndIncrement()); - tag.setAutoComponentTag(true); + tag.setAutoComponentTag(true); } tag.addBehavior(RELATIVE_PATH_BEHAVIOR); @@ -195,32 +195,31 @@ public final class RelativePathPrefixHandler extends AbstractMarkupFilter @Override public void postProcess(Markup markup) { - /** - * https://issues.apache.org/jira/browse/WICKET-5724 - * - * Transparent component inside page body must allow - * queued children components. - */ - Iterator<MarkupElement> markupIterator = markup.iterator(); - while (markupIterator.hasNext()) - { - MarkupElement next = markupIterator.next(); - - if(next instanceof ComponentTag) - { - ComponentTag componentTag = (ComponentTag)next; - - /** - * if component tag is for a transparent component - * and contains "wicket:id", must be queueable. - */ - if (componentTag.containsWicketId() && - componentTag.getId().startsWith(getWicketRelativePathPrefix(null))) - { - componentTag.setAutoComponentFactory(FACTORY); - } - } - } + /** + * https://issues.apache.org/jira/browse/WICKET-5724 + * + * Transparent component inside page body must allow queued children components. + */ + Iterator<MarkupElement> markupIterator = markup.iterator(); + while (markupIterator.hasNext()) + { + MarkupElement next = markupIterator.next(); + + if (next instanceof ComponentTag) + { + ComponentTag componentTag = (ComponentTag)next; + + /** + * if component tag is for a transparent component and contains "wicket:id", must be + * queueable. + */ + if (componentTag.containsWicketId() + && componentTag.getId().startsWith(getWicketRelativePathPrefix(null))) + { + componentTag.setAutoComponentFactory(FACTORY); + } + } + } } private String getWicketRelativePathPrefix(final MarkupStream markupStream)
