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>
