You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2007/12/03 02:27:22 UTC
svn commit: r600416 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry/annotations/
main/java/org/apache/tapestry/internal/services/
main/java/org/apache/tapestry/runtime/ site/apt/guide/
test/java/org/apache/tapestry/integ...
Author: hlship
Date: Sun Dec 2 17:27:20 2007
New Revision: 600416
URL: http://svn.apache.org/viewvc?rev=600416&view=rev
Log:
TAPESTRY-1952: The "match any event" feature for the OnEvent handler is not useful and should be removed
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/OnEvent.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentEventImpl.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/OnEventWorker.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/runtime/ComponentEvent.java
tapestry/tapestry5/trunk/tapestry-core/src/site/apt/guide/event.apt
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/EventHandlerDemo.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ComponentEventImplTest.java
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/OnEvent.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/OnEvent.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/OnEvent.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/OnEvent.java Sun Dec 2 17:27:20 2007
@@ -14,6 +14,8 @@
package org.apache.tapestry.annotations;
+import org.apache.tapestry.TapestryConstants;
+
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
@@ -50,11 +52,10 @@
{
/**
- * The event types to match. The handler will only be invoked if the client event type matches
- * the value. If omitted, the default is to match any event. Most components emit, at most, one
- * event named "action".
+ * The event type to match. The handler will only be invoked if the client event type matches
+ * the value. The default value is "action". Matching is case-insensitive.
*/
- String value() default "";
+ String value() default TapestryConstants.ACTION_EVENT;
/**
* The local id of the component from which the event originates. If not specified, then the
@@ -62,6 +63,9 @@
* component's container, it is re-triggered inside the component's grand-container and will
* appear to originate from the container. Thus events that escape a component will appear to
* originate in the component's container, and so forth.
+ * <p/>
+ * <p/>
+ * Matching by component id is case insensitive.
*/
String component() default "";
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentEventImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentEventImpl.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentEventImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentEventImpl.java Sun Dec 2 17:27:20 2007
@@ -55,19 +55,11 @@
_classLoader = classLoader;
}
- public boolean matchesByComponentId(String componentId)
+ public boolean matches(String eventType, String componentId, int parameterCount)
{
- return _originatingComponentId.equalsIgnoreCase(componentId);
- }
-
- public boolean matchesByEventType(String eventType)
- {
- return _eventType.equalsIgnoreCase(eventType);
- }
-
- public boolean matchesByParameterCount(int parameterCount)
- {
- return _context.length >= parameterCount;
+ return _eventType.equalsIgnoreCase(
+ eventType) && _context.length >= parameterCount && (_originatingComponentId.equalsIgnoreCase(
+ componentId) || componentId.equals(""));
}
@SuppressWarnings("unchecked")
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/OnEventWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/OnEventWorker.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/OnEventWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/OnEventWorker.java Sun Dec 2 17:27:20 2007
@@ -15,7 +15,6 @@
package org.apache.tapestry.internal.services;
import org.apache.tapestry.annotations.OnEvent;
-import org.apache.tapestry.ioc.internal.util.InternalUtils;
import org.apache.tapestry.ioc.util.BodyBuilder;
import org.apache.tapestry.model.MutableComponentModel;
import org.apache.tapestry.runtime.Component;
@@ -85,39 +84,18 @@
{
// $1 is the event
- int closeCount = 0;
-
int parameterCount = getParameterCount(method);
- if (parameterCount > 0)
- {
- builder.addln("if ($1.matchesByParameterCount(%d))", parameterCount);
- builder.begin();
-
- closeCount++;
- }
-
OnEvent annotation = transformation.getMethodAnnotation(method, OnEvent.class);
String eventType = extractEventType(method, annotation);
- if (InternalUtils.isNonBlank(eventType))
- {
- builder.addln("if ($1.matchesByEventType(\"%s\"))", eventType);
- builder.begin();
-
- closeCount++;
- }
String componentId = extractComponentId(method, annotation);
- if (InternalUtils.isNonBlank(componentId))
- {
- builder.addln("if ($1.matchesByComponentId(\"%s\"))", componentId);
- builder.begin();
- closeCount++;
- }
+ builder.addln("if ($1.matches(\"%s\", \"%s\", %d))", eventType, componentId, parameterCount);
+ builder.begin();
// Ensure that we return true, because *some* event handler method was invoked,
// even if it chose not to abort the event.
@@ -143,8 +121,7 @@
if (isNonVoid) builder.addln("))) return true;");
else builder.addln(");");
- for (int i = 0; i < closeCount; i++)
- builder.end();
+ builder.end();
}
private String extractComponentId(TransformMethodSignature method, OnEvent annotation)
@@ -172,13 +149,7 @@
int fromx = name.indexOf("From");
- String eventName = fromx == -1 ? name.substring(2) : name.substring(2, fromx);
-
- // This is intended for onAnyFromComponentId, but just onAny works too (and is dangerous).
-
- if (eventName.equalsIgnoreCase("AnyEvent")) return "";
-
- return eventName;
+ return fromx == -1 ? name.substring(2) : name.substring(2, fromx);
}
private int getParameterCount(TransformMethodSignature method)
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/runtime/ComponentEvent.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/runtime/ComponentEvent.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/runtime/ComponentEvent.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/runtime/ComponentEvent.java Sun Dec 2 17:27:20 2007
@@ -28,28 +28,14 @@
public interface ComponentEvent extends Event
{
/**
- * Returns true if the component event's type matches any of the provided values. Comparison is
- * caseless.
+ * Returns true if the event matches the provided criteria.
*
- * @param eventType
- * @return true if there is any match
+ * @param eventType the type of event (case insensitive match)
+ * @param componentId component is to match against (case insensitive), or the empty string
+ * @param parameterCount minimum number of context values
+ * @return true if the event matches.
*/
- boolean matchesByEventType(String eventType);
-
- /**
- * Returns true if the originating component matches any of the components identified by their
- * ids. This filter is only relevent in the immediate container of the originating component (it
- * will never match at higher levels). Comparison is caseless.
- */
- boolean matchesByComponentId(String componentId);
-
- /**
- * Returns true if the event context contains the specified number of parameters (or more).
- *
- * @param parameterCount number of parameters in the event handler method
- * @return true if the event can
- */
- boolean matchesByParameterCount(int parameterCount);
+ boolean matches(String eventType, String componentId, int parameterCount);
/**
* Coerces a context value to a particular type. The context is an array of objects; typically
Modified: tapestry/tapestry5/trunk/tapestry-core/src/site/apt/guide/event.apt
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/site/apt/guide/event.apt?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/site/apt/guide/event.apt (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/site/apt/guide/event.apt Sun Dec 2 17:27:20 2007
@@ -88,9 +88,8 @@
[]
- In the above example, the valueChosen() method will be invoked on <any event> that originates
- in component <<<choose>>> (and has at least one context value). Since ActionLink components only emit a single type of event, "action",
- this will not be a problem.
+ In the above example, the valueChosen() method will be invoked on the default event, "action", that originates
+ in component <<<choose>>> (and has at least one context value).
Some components can emit more than one type of event, in which case you will want to be more specific:
@@ -110,7 +109,8 @@
As elsewhere, the comparison of event type and component id is caseless.
- You should qualify exactly which component(s) you wish to recieve events from.
+ You should qualify exactly which component(s) you wish to recieve events from. Using @OnEvent on a method
+ and not specifying a specific component id means that the method will be invoked for events from <any> component.
Event Handler Method Convention Names
@@ -128,9 +128,7 @@
_value = value;
}
+---+
-
- If your method is named "onAnyEvent", it will be invoked for all types of events. This is rarely what you want!
-
+
Note from Howard: I've found that I prefer the naming convention approach, and reserve the annotation just for situations that don't otherwise fit.
Event Handler Method Return Values
@@ -182,7 +180,7 @@
Event Method Exceptions
- Event methods are allow to throw any exception (not just runtime exceptions). If an event method does
+ Event methods are allowed to throw any exception (not just runtime exceptions). If an event method does
throw an exception, Tapestry will catch the thrown exception and ultimately display the exception report
page.
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java Sun Dec 2 17:27:20 2007
@@ -827,7 +827,7 @@
clickAndWait("fred");
assertTextPresent(
- "[parent.eventHandlerOne(String), parent.eventHandlerZero(), parent.onAction(String), parent.onAction(), child.eventHandlerForFred(), child.eventHandlerOneChild(), child.eventHandlerZeroChild(), child.onAction(String), child.onAction(), child.onActionFromFred(String), child.onActionFromFred(), child.onAnyEventFromFred(String), child.onAnyEventFromFred()]");
+ "[parent.eventHandlerOne(String), parent.eventHandlerZero(), parent.onAction(String), parent.onAction(), child.eventHandlerForFred(), child.eventHandlerOneChild(), child.eventHandlerZeroChild(), child.onAction(String), child.onAction(), child.onActionFromFred(String), child.onActionFromFred()]");
}
@Test
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/EventHandlerDemo.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/EventHandlerDemo.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/EventHandlerDemo.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/EventHandlerDemo.java Sun Dec 2 17:27:20 2007
@@ -43,18 +43,6 @@
addMethodName("child.onActionFromFred(String)");
}
- @SuppressWarnings("unused")
- private void onAnyEventFromFred()
- {
- addMethodName("child.onAnyEventFromFred()");
- }
-
- @SuppressWarnings("unused")
- private void onAnyEventFromFred(String value)
- {
- addMethodName("child.onAnyEventFromFred(String)");
- }
-
@OnEvent(value = "action")
void eventHandlerZeroChild()
{
@@ -75,7 +63,6 @@
public Object[] getTwoContext()
{
- return new Object[]
- {1, 2};
+ return new Object[]{1, 2};
}
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ComponentEventImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ComponentEventImplTest.java?rev=600416&r1=600415&r2=600416&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ComponentEventImplTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ComponentEventImplTest.java Sun Dec 2 17:27:20 2007
@@ -48,8 +48,8 @@
ComponentEvent event = new ComponentEventImpl("eventType", "someId", null, handler, _coercer, null);
- assertTrue(event.matchesByEventType("eventType"));
- assertFalse(event.matchesByEventType("foo"));
+ assertTrue(event.matches("eventType", "someId", 0));
+ assertFalse(event.matches("foo", "someId", 0));
verify();
}
@@ -63,7 +63,7 @@
ComponentEvent event = new ComponentEventImpl("eventType", "someId", null, handler, _coercer, null);
- assertTrue(event.matchesByEventType("EVENTTYPE"));
+ assertTrue(event.matches("EVENTTYPE", "someid", 0));
verify();
}
@@ -77,9 +77,9 @@
ComponentEvent event = new ComponentEventImpl("eventType", "someId", null, handler, _coercer, null);
- assertTrue(event.matchesByComponentId("someId"));
+ assertTrue(event.matches("eventType", "someId", 0));
- assertFalse(event.matchesByComponentId("bar"));
+ assertFalse(event.matches("eventtype", "bar", 0));
verify();
}
@@ -92,7 +92,7 @@
replay();
ComponentEvent event = new ComponentEventImpl("eventType", "someId", null, handler, _coercer, null);
- assertTrue(event.matchesByComponentId("SOMEID"));
+ assertTrue(event.matches("eventtype", "SOMEID", 0));
verify();
}