You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2011/10/24 20:19:24 UTC
svn commit: r1188272 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry5/internal/structure/
main/java/org/apache/tapestry5/internal/transform/
test/groovy/org/apache/tapestry5/integration/app1/
test/java/org/apache/tapestr...
Author: hlship
Date: Mon Oct 24 18:19:23 2011
New Revision: 1188272
URL: http://svn.apache.org/viewvc?rev=1188272&view=rev
Log:
TAP5-1642: A mixin parameter that is required but also provides a default property results in an "unbound parameter" exception, starting in Tapestry 5.3
Added:
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/mixins/AltTitleDefault.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.java
tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.tml
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElementImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java
tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ParameterTests.groovy
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElementImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElementImpl.java?rev=1188272&r1=1188271&r2=1188272&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElementImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElementImpl.java Mon Oct 24 18:19:23 2011
@@ -781,12 +781,19 @@ public class ComponentPageElementImpl ex
mixinAfterOrderer = null;
}
- // For some parameters, bindings (from defaults) are provided inside the callback method, so
- // that is invoked first, before we check for unbound parameters.
+ initializeRenderPhases();
- verifyRequiredParametersAreBound();
+ page.addVerifyListener(new Runnable()
+ {
+ public void run()
+ {
+ // For some parameters, bindings (from defaults) are provided inside the callback method, so
+ // that is invoked first, before we check for unbound parameters.
+
+ verifyRequiredParametersAreBound();
+ }
+ });
- initializeRenderPhases();
loaded = true;
}
@@ -1166,10 +1173,10 @@ public class ComponentPageElementImpl ex
addUnboundParameterNames(name, unbound, mixinIdToComponentResources.get(name));
}
- if (unbound.isEmpty())
- return;
-
- throw new TapestryException(StructureMessages.missingParameters(unbound, this), this, null);
+ if (!unbound.isEmpty())
+ {
+ throw new TapestryException(StructureMessages.missingParameters(unbound, this), this, null);
+ }
}
public Locale getLocale()
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java?rev=1188272&r1=1188271&r2=1188272&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/Page.java Mon Oct 24 18:19:23 2011
@@ -205,6 +205,17 @@ public interface Page
void addResetListener(PageResetListener listener);
/**
+ * Adds a verify callback, which is allowed while the page is loading. Such callbacks are invoked once,
+ * after the page has been loaded succesfully. This was added specifically to ensure that components
+ * only verify that required parameters are bound after all components and mixins of the page have had a chance
+ * to initialize.
+ *
+ * @param callback to be invoked after page loaded
+ * @since 5.3
+ */
+ void addVerifyListener(Runnable callback);
+
+ /**
* Returns true if there are any {@link PageResetListener} listeners.
*
* @since 5.2.0
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java?rev=1188272&r1=1188271&r2=1188272&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java Mon Oct 24 18:19:23 2011
@@ -55,6 +55,8 @@ public class PageImpl implements Page
private final AtomicInteger attachCount = new AtomicInteger();
+ private List<Runnable> pageVerifyCallbacks = CollectionFactory.newList();
+
/**
* Obtained from the {@link org.apache.tapestry5.internal.services.PersistentFieldManager} when
* first needed,
@@ -180,11 +182,26 @@ public class PageImpl implements Page
lock.check();
for (PageLifecycleListener listener : lifecycleListeners)
+ {
listener.containingPageDidLoad();
-
- loadComplete = true;
+ }
lock.lock();
+
+
+ for (Runnable callback : pageVerifyCallbacks)
+ {
+ callback.run();
+ }
+
+ // These are never needed again, so we can get rid of them. The PageLifecycleListener interface is too complicated,
+ // we don't know what's needed when, and rely on the listeners themselves to unregister when no longer needed. A better design
+ // would be more like these pageVerifyCallbacks: just use Runnable and know when it is safe to throw them away. Something
+ // to refactor to over time.
+
+ pageVerifyCallbacks = null;
+
+ loadComplete = true;
}
public void attached()
@@ -237,6 +254,15 @@ public class PageImpl implements Page
resetListeners.add(listener);
}
+ public void addVerifyListener(Runnable callback)
+ {
+ assert callback != null;
+
+ lock.check();
+
+ pageVerifyCallbacks.add(callback);
+ }
+
public void pageReset()
{
for (PageResetListener l : resetListeners)
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java?rev=1188272&r1=1188271&r2=1188272&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java Mon Oct 24 18:19:23 2011
@@ -247,7 +247,7 @@ public class ParameterWorker implements
private Object readFromBinding()
{
- Object result = null;
+ Object result;
try
{
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ParameterTests.groovy
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ParameterTests.groovy?rev=1188272&r1=1188271&r2=1188272&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ParameterTests.groovy (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/ParameterTests.groovy Mon Oct 24 18:19:23 2011
@@ -71,4 +71,14 @@ class ParameterTests extends TapestryCor
assertAttribute "//a[@id='frog']/@alt", "Alt Title"
assertAttribute "//a[@id='frog']/@title", "Frog Title"
}
+
+/** https://issues.apache.org/jira/browse/TAP5-1642 */
+ @Test
+ void mixin_parameter_with_default_no_longer_causes_spurious_exception()
+ {
+ openLinks "Mixin Parameter with Default"
+
+ // This proves the mixin was added and did its job.
+ assertAttribute "//a[@class='testsubject']/@alt", "Default title"
+ }
}
Added: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/mixins/AltTitleDefault.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/mixins/AltTitleDefault.java?rev=1188272&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/mixins/AltTitleDefault.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/mixins/AltTitleDefault.java Mon Oct 24 18:19:23 2011
@@ -0,0 +1,18 @@
+package org.apache.tapestry5.integration.app1.mixins;
+
+import org.apache.tapestry5.BindingConstants;
+import org.apache.tapestry5.MarkupWriter;
+import org.apache.tapestry5.annotations.MixinAfter;
+import org.apache.tapestry5.annotations.Parameter;
+
+@MixinAfter
+public class AltTitleDefault
+{
+ @Parameter(required = true, defaultPrefix = BindingConstants.LITERAL, allowNull = false, value = "Default title")
+ private String title;
+
+ void beginRender(MarkupWriter writer)
+ {
+ writer.attributes("alt", title);
+ }
+}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java?rev=1188272&r1=1188271&r2=1188272&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java Mon Oct 24 18:19:23 2011
@@ -56,6 +56,8 @@ public class Index
private static final List<Item> ITEMS = CollectionFactory
.newList(
+ new Item("MixinParameterDefault", "Mixin Parameter with Default", "Ensure that a mixin parameter with a default value is not reported as unbound."),
+
new Item("MixinVsInformalParameter", "Mixin Parameter vs. Informal Parameter", "Informal Paramters vs. Mixin parameter of same name"),
new Item("inherit/childa", "TAP5-1656 Demo", "Test a reported bug in component inheritance"),
Added: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.java?rev=1188272&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.java Mon Oct 24 18:19:23 2011
@@ -0,0 +1,8 @@
+package org.apache.tapestry5.integration.app1.pages;
+
+/**
+ * Used as part of the test for TAP5-1642
+ */
+public class MixinParameterDefault
+{
+}
Added: tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.tml
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.tml?rev=1188272&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.tml (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/pages/MixinParameterDefault.tml Mon Oct 24 18:19:23 2011
@@ -0,0 +1,8 @@
+<t:border xmlns:t="http://tapestry.apache.org/schema/tapestry_5_3.xsd">
+
+
+ <t:actionlink class="testsubject" t:mixins="alttitledefault">
+ the link
+ </t:actionlink>
+
+</t:border>