Repository: wicket Updated Branches: refs/heads/wicket-7.x a63fb75bd -> 603be7da6
WICKET-6229 Introduce a new setting in ExceptionSettings to control whether to throw exception or log a WARN when requesting for markup id on non-renderable component Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/603be7da Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/603be7da Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/603be7da Branch: refs/heads/wicket-7.x Commit: 603be7da6703b090850740e5f3af5ee4ca4b36ae Parents: a63fb75 Author: Martin Tzvetanov Grigorov <[email protected]> Authored: Tue Aug 23 21:30:46 2016 +0200 Committer: Martin Tzvetanov Grigorov <[email protected]> Committed: Tue Aug 23 21:32:36 2016 +0200 ---------------------------------------------------------------------- .../main/java/org/apache/wicket/Component.java | 65 ++++++-- .../wicket/settings/ExceptionSettings.java | 32 +++- .../wicket/settings/RequestCycleSettings.java | 2 +- ...nSettingsNotRenderableErrorStrategyTest.java | 150 +++++++++++++++++++ 4 files changed, 230 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/603be7da/wicket-core/src/main/java/org/apache/wicket/Component.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java b/wicket-core/src/main/java/org/apache/wicket/Component.java index 0fc5252..2f6715e 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Component.java +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java @@ -79,6 +79,7 @@ import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.request.resource.ResourceReference; import org.apache.wicket.response.StringResponse; import org.apache.wicket.settings.DebugSettings; +import org.apache.wicket.settings.ExceptionSettings; import org.apache.wicket.util.IHierarchical; import org.apache.wicket.util.convert.IConverter; import org.apache.wicket.util.io.IClusterable; @@ -2555,19 +2556,34 @@ public abstract class Component try { // Render open tag - if (getRenderBodyOnly()) + boolean renderBodyOnly = getRenderBodyOnly(); + if (renderBodyOnly) { + ExceptionSettings.NotRenderableErrorStrategy notRenderableErrorStrategy = ExceptionSettings.NotRenderableErrorStrategy.LOG_WARNING; + if (Application.exists()) + { + notRenderableErrorStrategy = getApplication().getExceptionSettings().getNotRenderableErrorStrategy(); + } + if (getFlag(FLAG_OUTPUT_MARKUP_ID)) { - log.warn(String.format( - "Markup id set on a component that renders its body only. " - + "Markup id: %s, component id: %s.", getMarkupId(), getId())); + String message = String.format("Markup id set on a component that renders its body only. " + + "Markup id: %s, component id: %s.", getMarkupId(), getId()); + if (notRenderableErrorStrategy == ExceptionSettings.NotRenderableErrorStrategy.THROW_EXCEPTION) + { + throw new IllegalStateException(message); + } + log.warn(message); } if (getFlag(FLAG_PLACEHOLDER)) { - log.warn(String.format( - "Placeholder tag set on a component that renders its body only. " - + "Component id: %s.", getId())); + String message = String.format("Placeholder tag set on a component that renders its body only. " + + "Component id: %s.", getId()); + if (notRenderableErrorStrategy == ExceptionSettings.NotRenderableErrorStrategy.THROW_EXCEPTION) + { + throw new IllegalStateException(message); + } + log.warn(message); } } else @@ -2586,9 +2602,9 @@ public abstract class Component // Render close tag if (openTag.isOpen()) { - renderClosingComponentTag(markupStream, tag, getRenderBodyOnly()); + renderClosingComponentTag(markupStream, tag, renderBodyOnly); } - else if (getRenderBodyOnly() == false) + else if (renderBodyOnly == false) { if (needToRenderTag(openTag)) { @@ -4037,18 +4053,35 @@ public abstract class Component if ((tag instanceof WicketTag) && !tag.isClose() && !getFlag(FLAG_IGNORE_ATTRIBUTE_MODIFIER)) { + ExceptionSettings.NotRenderableErrorStrategy notRenderableErrorStrategy = ExceptionSettings.NotRenderableErrorStrategy.LOG_WARNING; + if (Application.exists()) + { + notRenderableErrorStrategy = getApplication().getExceptionSettings().getNotRenderableErrorStrategy(); + } + + String tagName = tag.getNamespace() + ":" + tag.getName(); + String componentId = getId(); if (getFlag(FLAG_OUTPUT_MARKUP_ID)) { - log.warn(String.format( - "Markup id set on a component that is usually not rendered into markup. " - + "Markup id: %s, component id: %s, component tag: %s.", getMarkupId(), - getId(), tag.getName())); + String message = String.format("Markup id set on a component that is usually not rendered into markup. " + + "Markup id: %s, component id: %s, component tag: %s.", + getMarkupId(), componentId, tagName); + if (notRenderableErrorStrategy == ExceptionSettings.NotRenderableErrorStrategy.THROW_EXCEPTION) + { + throw new IllegalStateException(message); + } + log.warn(message); } if (getFlag(FLAG_PLACEHOLDER)) { - log.warn(String.format( - "Placeholder tag set on a component that is usually not rendered into markup. " - + "Component id: %s, component tag: %s.", getId(), tag.getName())); + String message = String.format( + "Placeholder tag set on a component that is usually not rendered into markup. " + + "Component id: %s, component tag: %s.", componentId, tagName); + if (notRenderableErrorStrategy == ExceptionSettings.NotRenderableErrorStrategy.THROW_EXCEPTION) + { + throw new IllegalStateException(message); + } + log.warn(message); } } http://git-wip-us.apache.org/repos/asf/wicket/blob/603be7da/wicket-core/src/main/java/org/apache/wicket/settings/ExceptionSettings.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/settings/ExceptionSettings.java b/wicket-core/src/main/java/org/apache/wicket/settings/ExceptionSettings.java index 4f3fcdd..3a11944 100644 --- a/wicket-core/src/main/java/org/apache/wicket/settings/ExceptionSettings.java +++ b/wicket-core/src/main/java/org/apache/wicket/settings/ExceptionSettings.java @@ -78,7 +78,7 @@ public class ExceptionSettings * * @author igor */ - public static enum AjaxErrorStrategy { + public enum AjaxErrorStrategy { /** redirect to error page, just like a normal requset */ REDIRECT_TO_ERROR_PAGE, /** invoke client side failure handler */ @@ -90,7 +90,7 @@ public class ExceptionSettings * * @author papegaaij */ - public static enum ThreadDumpStrategy { + public enum ThreadDumpStrategy { /** Do not dump any stacktraces */ NO_THREADS, /** Dump the stacktrace of the thread holding the lock */ @@ -99,6 +99,24 @@ public class ExceptionSettings ALL_THREADS } + /** + * A strategy defining what to do when a component that will not render its + * markup tag (because of {@link org.apache.wicket.Component#setRenderBodyOnly(boolean) setRenderBodyOnly(true)} + * or used with <wicket:xyz>) is also asked to output a + * markup {@link org.apache.wicket.Component#setOutputMarkupId(boolean) id} or + * {@link org.apache.wicket.Component#setOutputMarkupPlaceholderTag(boolean) placeholder tag} + */ + public enum NotRenderableErrorStrategy { + /** + * Log a message with level {@link org.slf4j.Logger#warn(String) WARNING} + */ + LOG_WARNING, + /** + * Throw a runtime exception + */ + THROW_EXCEPTION + } + /** Type of handling for unexpected exceptions */ private UnexpectedExceptionDisplay unexpectedExceptionDisplay = SHOW_EXCEPTION_PAGE; @@ -112,6 +130,8 @@ public class ExceptionSettings */ private ThreadDumpStrategy threadDumpStrategy = ThreadDumpStrategy.THREAD_HOLDING_LOCK; + private NotRenderableErrorStrategy notRenderableErrorStrategy = NotRenderableErrorStrategy.LOG_WARNING; + /** * @return Returns the unexpectedExceptionDisplay. */ @@ -187,4 +207,12 @@ public class ExceptionSettings { return threadDumpStrategy; } + + public NotRenderableErrorStrategy getNotRenderableErrorStrategy() { + return notRenderableErrorStrategy; + } + + public void setNotRenderableErrorStrategy(final NotRenderableErrorStrategy notRenderableErrorStrategy) { + this.notRenderableErrorStrategy = notRenderableErrorStrategy; + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/603be7da/wicket-core/src/main/java/org/apache/wicket/settings/RequestCycleSettings.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/settings/RequestCycleSettings.java b/wicket-core/src/main/java/org/apache/wicket/settings/RequestCycleSettings.java index ab94abc..47dd8a9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/settings/RequestCycleSettings.java +++ b/wicket-core/src/main/java/org/apache/wicket/settings/RequestCycleSettings.java @@ -85,7 +85,7 @@ public class RequestCycleSettings /** * Enum type for different render strategies */ - public static enum RenderStrategy { + public enum RenderStrategy { /** * All logical parts of a request (the action and render part) are handled within the same * request. http://git-wip-us.apache.org/repos/asf/wicket/blob/603be7da/wicket-core/src/test/java/org/apache/wicket/settings/ExceptionSettingsNotRenderableErrorStrategyTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/settings/ExceptionSettingsNotRenderableErrorStrategyTest.java b/wicket-core/src/test/java/org/apache/wicket/settings/ExceptionSettingsNotRenderableErrorStrategyTest.java new file mode 100644 index 0000000..7ab3aed --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/settings/ExceptionSettingsNotRenderableErrorStrategyTest.java @@ -0,0 +1,150 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.settings; + +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.WicketRuntimeException; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.basic.Label; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +/** + * Tests for {@link ExceptionSettings#notRenderableErrorStrategy} + */ +public class ExceptionSettingsNotRenderableErrorStrategyTest extends WicketTestCase { + + private WicketTagTestPage wicketTagTestPage; + private RenderBodyOnlyTestPage renderBodyOnlyTestPage; + + @Before + public void before() + { + ExceptionSettings exceptionSettings = tester.getApplication().getExceptionSettings(); + exceptionSettings.setNotRenderableErrorStrategy(ExceptionSettings.NotRenderableErrorStrategy.THROW_EXCEPTION); + + wicketTagTestPage = new WicketTagTestPage(); + renderBodyOnlyTestPage = new RenderBodyOnlyTestPage(); + } + + @Test + public void notRenderableErrorStrategy_whenWicketTagAndOutputId_throwException() { + try + { + wicketTagTestPage.component.setOutputMarkupId(true); + startWicketTagPage(); + } catch (WicketRuntimeException wrx) + { + assertWicketTagException(wrx); + } + } + + @Test + public void notRenderableErrorStrategy_whenWicketTagAndOutputPlaceholder_throwException() { + try + { + wicketTagTestPage.component.setOutputMarkupPlaceholderTag(true); + startWicketTagPage(); + } catch (WicketRuntimeException wrx) + { + assertWicketTagException(wrx); + } + } + + @Test + public void notRenderableErrorStrategy_whenRenderBodyOnlyAndOutputId_throwException() { + try + { + renderBodyOnlyTestPage.component.setOutputMarkupId(true); + startRenderBodyOnlyPage(); + } catch (WicketRuntimeException wrx) + { + assertRenderBodyOnlyException(wrx); + } + } + + @Test + public void notRenderableErrorStrategy_whenRenderBodyOnlyAndOutputPlaceholder_throwException() { + try + { + renderBodyOnlyTestPage.component.setOutputMarkupPlaceholderTag(true); + startRenderBodyOnlyPage(); + } catch (WicketRuntimeException wrx) + { + assertRenderBodyOnlyException(wrx); + } + } + + private void startWicketTagPage() { + tester.startPage(wicketTagTestPage); + fail("Should have thrown exception!"); + } + + private void startRenderBodyOnlyPage() { + tester.startPage(renderBodyOnlyTestPage); + fail("Should have thrown exception!"); + } + + private void assertWicketTagException(WicketRuntimeException wrx) { + assertThat(wrx.getCause().getMessage(), is(equalTo("Markup id set on a component that is usually not " + + "rendered into markup. Markup id: test1, component id: test, " + + "component tag: wicket:label."))); + } + + private void assertRenderBodyOnlyException(final WicketRuntimeException wrx) { + assertThat(wrx.getCause().getMessage(), is(equalTo("Markup id set on a component that renders its " + + "body only. Markup id: test1, component id: test."))); + } + + private static class WicketTagTestPage extends WebPage implements IMarkupResourceStreamProvider + { + private Label component; + + private WicketTagTestPage() { + component = new Label("test", "Test"); + add(component); + } + + @Override + public IResourceStream getMarkupResourceStream(final MarkupContainer container, final Class<?> containerClass) { + return new StringResourceStream("<html><body><wicket:label wicket:id='test'/></body></html>"); + } + } + + private static class RenderBodyOnlyTestPage extends WebPage implements IMarkupResourceStreamProvider + { + private Label component; + + private RenderBodyOnlyTestPage() { + component = new Label("test", "Test"); + component.setRenderBodyOnly(true); + add(component); + } + + @Override + public IResourceStream getMarkupResourceStream(final MarkupContainer container, final Class<?> containerClass) { + return new StringResourceStream("<html><body><span wicket:id='test'></span></body></html>"); + } + } +}
