martin-g commented on a change in pull request #457:
URL: https://github.com/apache/wicket/pull/457#discussion_r508031845
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
Review comment:
s/whiches/wishes/
##########
File path:
wicket-core/src/test/java/org/apache/wicket/markup/transformer/AbstractTransformerBehaviorTest.java
##########
@@ -69,7 +75,77 @@ public void transformationInAjaxRequest()
tester.clickLink("updateLabel", true);
tester.assertContains("ajax request");
tester.assertContainsNot("normal request");
+ }
+ /**
+ * Test how multiple different transformers applied to the same
component behave.
+ * <p>
+ * The current implementation of {@link AbstractTransformerBehavior}
doesn't support multiple
+ * instances on the same component, a container needs to be used
explicitly instead. So make
+ * sure the implementation is as expected, as otherwise the container
might not be necessary at
+ * all anymore, and that the container really works around the problem.
+ * </p>
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ */
+ @Test
+ public void multiTransesSameComp()
+ {
+ TestPage testPage = new TestPage();
+ Label label = new Label(TestPage.LABEL_ID,
TestPage.LABEL_VAL);
+
+ Function<String, AbstractTransformerBehavior> transProd = (val)
->
Review comment:
s/transProd/transformerProducer/
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
+ * support nesting itself properly, which results in missing rendered output
and most likely broken
+ * HTML documents in the end.
+ * </p>
* @see org.apache.wicket.markup.transformer.AbstractOutputTransformerContainer
- *
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ *
* @author Juergen Donnerstag
*/
public abstract class AbstractTransformerBehavior extends Behavior implements
ITransformer
{
private static final long serialVersionUID = 1L;
+ /**
+ * Container to apply multiple {@link AbstractTransformerBehavior} to
some component.
+ * <p>
+ * This container is by design NOT about multiple arbitrary
transformations, but really only for the
+ * one use case supporting multiple instances of {@link
AbstractTransformerBehavior} on one and the
+ * same component. The current implementation of that works with
temporary responses, but doesn't
+ * support nesting itself properly in case multiple behaviors assigned
to the same component, which
+ * results in missing rendered output and most likely entirely broken
HTML documents in the end.
+ * </p>
+ * <p>
+ * The easiest workaround for that problem is simply introducing this
container which users need to
+ * use in those cases: An instance needs to be created with all
transformers of interest in the
+ * order they should be applied and the container takes care of doing
so. Because the container is
+ * an {@link AbstractTransformerBehavior} itself, things simply work
like with individual behaviors,
+ * while response handling is only managed by the container. So when
used with this container, the
+ * callbacks of the maintained instances like {@link
AbstractTransformerBehavior#afterRender(Component)}
+ * etc., are NOT used anymore! OTOH, the individual behaviors stay
useful without the container as
+ * well.
+ * </p>
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ */
+ public static class Multi extends AbstractTransformerBehavior
+ {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * All transformers which need to be applied in the order they
need to be applied.
Review comment:
Too many "need to be applied". The sentence does not sound good/clear.
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
+ * support nesting itself properly, which results in missing rendered output
and most likely broken
+ * HTML documents in the end.
+ * </p>
* @see org.apache.wicket.markup.transformer.AbstractOutputTransformerContainer
- *
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ *
* @author Juergen Donnerstag
*/
public abstract class AbstractTransformerBehavior extends Behavior implements
ITransformer
{
private static final long serialVersionUID = 1L;
+ /**
+ * Container to apply multiple {@link AbstractTransformerBehavior} to
some component.
+ * <p>
+ * This container is by design NOT about multiple arbitrary
transformations, but really only for the
+ * one use case supporting multiple instances of {@link
AbstractTransformerBehavior} on one and the
+ * same component. The current implementation of that works with
temporary responses, but doesn't
+ * support nesting itself properly in case multiple behaviors assigned
to the same component, which
+ * results in missing rendered output and most likely entirely broken
HTML documents in the end.
+ * </p>
+ * <p>
+ * The easiest workaround for that problem is simply introducing this
container which users need to
+ * use in those cases: An instance needs to be created with all
transformers of interest in the
+ * order they should be applied and the container takes care of doing
so. Because the container is
+ * an {@link AbstractTransformerBehavior} itself, things simply work
like with individual behaviors,
+ * while response handling is only managed by the container. So when
used with this container, the
+ * callbacks of the maintained instances like {@link
AbstractTransformerBehavior#afterRender(Component)}
+ * etc., are NOT used anymore! OTOH, the individual behaviors stay
useful without the container as
+ * well.
+ * </p>
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ */
+ public static class Multi extends AbstractTransformerBehavior
+ {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * All transformers which need to be applied in the order they
need to be applied.
+ */
+ private final List<AbstractTransformerBehavior> transes;
Review comment:
s/trances/transformers/
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
+ * support nesting itself properly, which results in missing rendered output
and most likely broken
+ * HTML documents in the end.
+ * </p>
* @see org.apache.wicket.markup.transformer.AbstractOutputTransformerContainer
- *
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ *
* @author Juergen Donnerstag
*/
public abstract class AbstractTransformerBehavior extends Behavior implements
ITransformer
{
private static final long serialVersionUID = 1L;
+ /**
+ * Container to apply multiple {@link AbstractTransformerBehavior} to
some component.
Review comment:
This javadoc should really be a comment in the PR, not Javadoc.
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
Review comment:
This Javadoc is obsolete with the improvement below, no ?
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
Review comment:
s/The current implementation of works with temporary responses/The
current implementation works with temporary responses/
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
+ * support nesting itself properly, which results in missing rendered output
and most likely broken
+ * HTML documents in the end.
+ * </p>
* @see org.apache.wicket.markup.transformer.AbstractOutputTransformerContainer
- *
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ *
* @author Juergen Donnerstag
*/
public abstract class AbstractTransformerBehavior extends Behavior implements
ITransformer
{
private static final long serialVersionUID = 1L;
+ /**
+ * Container to apply multiple {@link AbstractTransformerBehavior} to
some component.
+ * <p>
+ * This container is by design NOT about multiple arbitrary
transformations, but really only for the
+ * one use case supporting multiple instances of {@link
AbstractTransformerBehavior} on one and the
+ * same component. The current implementation of that works with
temporary responses, but doesn't
+ * support nesting itself properly in case multiple behaviors assigned
to the same component, which
+ * results in missing rendered output and most likely entirely broken
HTML documents in the end.
+ * </p>
+ * <p>
+ * The easiest workaround for that problem is simply introducing this
container which users need to
+ * use in those cases: An instance needs to be created with all
transformers of interest in the
+ * order they should be applied and the container takes care of doing
so. Because the container is
+ * an {@link AbstractTransformerBehavior} itself, things simply work
like with individual behaviors,
+ * while response handling is only managed by the container. So when
used with this container, the
+ * callbacks of the maintained instances like {@link
AbstractTransformerBehavior#afterRender(Component)}
+ * etc., are NOT used anymore! OTOH, the individual behaviors stay
useful without the container as
+ * well.
+ * </p>
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ */
+ public static class Multi extends AbstractTransformerBehavior
+ {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * All transformers which need to be applied in the order they
need to be applied.
+ */
+ private final List<AbstractTransformerBehavior> transes;
+
+ /**
+ * CTOR simply storing the given transformers.
Review comment:
s/CTOR/Constructor/
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
+ * support nesting itself properly, which results in missing rendered output
and most likely broken
+ * HTML documents in the end.
+ * </p>
* @see org.apache.wicket.markup.transformer.AbstractOutputTransformerContainer
- *
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ *
* @author Juergen Donnerstag
*/
public abstract class AbstractTransformerBehavior extends Behavior implements
ITransformer
{
private static final long serialVersionUID = 1L;
+ /**
+ * Container to apply multiple {@link AbstractTransformerBehavior} to
some component.
+ * <p>
+ * This container is by design NOT about multiple arbitrary
transformations, but really only for the
+ * one use case supporting multiple instances of {@link
AbstractTransformerBehavior} on one and the
+ * same component. The current implementation of that works with
temporary responses, but doesn't
+ * support nesting itself properly in case multiple behaviors assigned
to the same component, which
+ * results in missing rendered output and most likely entirely broken
HTML documents in the end.
+ * </p>
+ * <p>
+ * The easiest workaround for that problem is simply introducing this
container which users need to
+ * use in those cases: An instance needs to be created with all
transformers of interest in the
+ * order they should be applied and the container takes care of doing
so. Because the container is
+ * an {@link AbstractTransformerBehavior} itself, things simply work
like with individual behaviors,
+ * while response handling is only managed by the container. So when
used with this container, the
+ * callbacks of the maintained instances like {@link
AbstractTransformerBehavior#afterRender(Component)}
+ * etc., are NOT used anymore! OTOH, the individual behaviors stay
useful without the container as
+ * well.
+ * </p>
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
+ */
+ public static class Multi extends AbstractTransformerBehavior
+ {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * All transformers which need to be applied in the order they
need to be applied.
+ */
+ private final List<AbstractTransformerBehavior> transes;
+
+ /**
+ * CTOR simply storing the given transformers.
+ *
+ * @param transes, which must not be {@code null} or empty, as
neither make sense here.
+ */
+ private Multi(List<AbstractTransformerBehavior> transes)
+ {
+ if ((transes == null) || transes.isEmpty())
+ {
+ throw new IllegalArgumentException("No
transformers given.");
+ }
+
+ this.transes = transes;
+ }
+
+ @Override
+ public CharSequence transform(Component component, CharSequence
output) throws Exception
+ {
+ CharSequence retVal = output;
+ for (AbstractTransformerBehavior trans : this.transes)
+ {
+ retVal = trans.transform(component, retVal);
+ }
+
+ return retVal;
+ }
+
+ /**
+ * Create a new container with the given transformers and with
keeping their order.
+ * <p>
+ * This factory expects multiple individual transformers
already, because creating a container
+ * with less doesn't make too much sense and users should
reconsider then if this container is
+ * of use at all. In most cases users do have individual
transformers to apply only anyway and
+ * don't need to provide a collection themself this way. OTOH,
a collection could be empty or
+ * contain only one element would then defeat the purpose of
this container again.
+ * </p>
+ * @param first First transformer to apply.
+ * @param second Second transformer to apply.
+ * @param moreIf All other transformers to apply, if at all, in
given order.
+ * @return A container with multiple transformers being applied.
+ */
+ public static Multi newFor( AbstractTransformerBehavior
first,
Review comment:
s/newFor/of/ ?! Like Model.of() and the new collection methods in Java 9
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/transformer/AbstractTransformerBehavior.java
##########
@@ -26,17 +31,114 @@
import org.apache.wicket.request.http.WebResponse;
/**
- * A {@link Behavior} which can be added to any component. It allows to
post-process (transform) the
+ * A {@link Behavior} which can be added to any component, allowing to
post-process (transform) the
* markup generated by the component.
- *
+ * <p>
+ * There's one important limitation with the current implementation: Multiple
different instances of
+ * this behavior CAN NOT be assigned to the same component! If one whiches to
do so, the contained
+ * container needs to be used to wrap existing behaviors and that container
needs to be added to the
+ * component instead. The current implementation of works with temporary
responses, but does not
+ * support nesting itself properly, which results in missing rendered output
and most likely broken
+ * HTML documents in the end.
+ * </p>
* @see org.apache.wicket.markup.transformer.AbstractOutputTransformerContainer
- *
+ * @see <a
href="https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6823";>JIRA
issue</a>
Review comment:
No need of this link here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
