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();
}
