Author: hlship
Date: Sun Dec  9 06:24:44 2007
New Revision: 602673

URL: http://svn.apache.org/viewvc?rev=602673&view=rev
Log:
TAPESTRY-1936:  Non-null return value from form action event causes exception

Modified:
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/ComponentEventHandler.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/corelib/components/Form.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentActionRequestHandlerImpl.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/EventImpl.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ResponseImpl.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/structure/ComponentPageElementImpl.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/TestableResponseImpl.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Response.java

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/ComponentEventHandler.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/ComponentEventHandler.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/ComponentEventHandler.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/ComponentEventHandler.java
 Sun Dec  9 06:24:44 2007
@@ -15,19 +15,23 @@
 package org.apache.tapestry;
 
 import org.apache.tapestry.runtime.Component;
-import org.apache.tapestry.runtime.ComponentEvent;
 
 /**
- * Handler for a [EMAIL PROTECTED] ComponentEvent}, notified when a non-null 
value is returned from some event
+ * Handler for a a [EMAIL PROTECTED] org.apache.tapestry.runtime.Event render 
phase event) or
+ * [EMAIL PROTECTED] org.apache.tapestry.runtime.ComponentEvent }, notified 
when a non-null value is returned from some event
  * handler method.
- * <p/>
- * TODO: Multiple handlers for different result types / strategy pattern?
  */
 public interface ComponentEventHandler<T>
 {
     /**
      * Invoked to handle a non-null event handler method result. The handler 
should determine
      * whether the value is acceptible, and throw an exception if not.
+     * <p/>
+     * <p/>
+     * Boolean values are <em>not</em> passed to the handler.  Booleans are 
used to indicate
+     * that the event has been handled (true) or that a further search for 
handlers
+     * should continue (true).  If a component event method returns true, then
+     * [EMAIL PROTECTED] org.apache.tapestry.runtime.Event#isAborted()} will 
return true.
      *
      * @param result            the result value provided by a method
      * @param component         the component from which the result was 
obtained

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/corelib/components/Form.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/corelib/components/Form.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/corelib/components/Form.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/corelib/components/Form.java
 Sun Dec  9 06:24:44 2007
@@ -301,8 +301,6 @@
             {
                 public boolean handleResult(Object result, Component 
component, String methodDescription)
                 {
-                    if (result instanceof Boolean) return ((Boolean) result);
-
                     // We want to process the event here, so that the 
component and method description are
                     // properly identified. But that's going to cause a 
headache aborting the
                     // event.

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentActionRequestHandlerImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentActionRequestHandlerImpl.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentActionRequestHandlerImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentActionRequestHandlerImpl.java
 Sun Dec  9 06:24:44 2007
@@ -89,10 +89,11 @@
 
         if (holder.hasValue()) return;
 
-        Link link = _linkFactory.createPageLink(page, false);
+        if (!_response.isCommitted())
+        {
+            Link link = _linkFactory.createPageLink(page, false);
 
-        _response.sendRedirect(link);
-
-        return;
+            _response.sendRedirect(link);
+        }
     }
 }

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/EventImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/EventImpl.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/EventImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/EventImpl.java
 Sun Dec  9 06:24:44 2007
@@ -53,13 +53,11 @@
         // this should never, ever happen. But what the hell,
         // let's check anyway.
 
-        if (_aborted)
-            throw new IllegalStateException(ServicesMessages
-                    .componentEventIsAborted(_methodDescription));
+        if (_aborted) throw new IllegalStateException(ServicesMessages
+                .componentEventIsAborted(_methodDescription));
 
-        if (result != null)
 
-            _aborted |= _handler.handleResult(result, _component, 
_methodDescription);
+        if (result != null) _aborted |= _handler.handleResult(result, 
_component, _methodDescription);
 
         return _aborted;
     }

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ResponseImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ResponseImpl.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ResponseImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ResponseImpl.java
 Sun Dec  9 06:24:44 2007
@@ -105,5 +105,8 @@
         _response.setIntHeader(name, value);
     }
 
-
+    public boolean isCommitted()
+    {
+        return _response.isCommitted();
+    }
 }

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/structure/ComponentPageElementImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/structure/ComponentPageElementImpl.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/structure/ComponentPageElementImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/structure/ComponentPageElementImpl.java
 Sun Dec  9 06:24:44 2007
@@ -953,16 +953,30 @@
     {
         boolean result = false;
 
+        ComponentPageElement component = this;
+        String componentId = "";
+
         // Provide a default handler for when the provided handler is null.
+        final ComponentEventHandler providedHandler = handler == null ? new 
NotificationEventHandler(eventType,
+                                                                               
                      _completeId) : handler;
+
+        ComponentEventHandler wrappedHandler = new ComponentEventHandler()
+        {
+            public boolean handleResult(Object result, Component component, 
String methodDescription)
+            {
+                // Boolean value is not passed to the handler; it will be true 
(abort event)
+                // or false (continue looking for event handlers).
 
-        if (handler == null) handler = new NotificationEventHandler(eventType, 
_completeId);
+                if (result instanceof Boolean) return (Boolean) result;
+
+                return providedHandler.handleResult(result, component, 
methodDescription);
+            }
+        };
 
-        ComponentPageElement component = this;
-        String componentId = "";
 
         while (component != null)
         {
-            ComponentEvent event = new ComponentEventImpl(eventType, 
componentId, context, handler, _typeCoercer,
+            ComponentEvent event = new ComponentEventImpl(eventType, 
componentId, context, wrappedHandler, _typeCoercer,
                                                           _classLoader);
 
             result |= component.handleEvent(event);

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/TestableResponseImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/TestableResponseImpl.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/TestableResponseImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/TestableResponseImpl.java
 Sun Dec  9 06:24:44 2007
@@ -25,6 +25,8 @@
 {
     private Link _link;
 
+    private boolean _committed;
+
     private void nyi(String methodName)
     {
         throw new RuntimeException(String.format("TestableResponse: Method 
%s() not yet implemented.", methodName));
@@ -39,7 +41,9 @@
 
     public PrintWriter getPrintWriter(String contentType) throws IOException
     {
-        // Welll, the output isn't accessible, but I guess we see that it 
could be generated from
+        _committed = true;
+
+        // Well, the output isn't accessible, but I guess we see that it could 
be generated from
         // the DOM.
         return new PrintWriter(new ByteArrayOutputStream());
     }
@@ -76,6 +80,8 @@
 
     public void sendRedirect(Link link) throws IOException
     {
+        _committed = true;
+
         _link = link;
     }
 
@@ -94,8 +100,14 @@
         return _link;
     }
 
+    public boolean isCommitted()
+    {
+        return _committed;
+    }
+
     public void clear()
     {
+        _committed = false;
         _link = null;
     }
 }

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Response.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Response.java?rev=602673&r1=602672&r2=602673&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Response.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Response.java
 Sun Dec  9 06:24:44 2007
@@ -130,4 +130,12 @@
      * @return the same URL or a different one with additional information to 
track the user session
      */
     String encodeRedirectURL(String URL);
+
+    /**
+     * Returns true if the response has already been sent, either as a 
redirect or as a stream
+     * of content.
+     *
+     * @return true if response already sent
+     */
+    boolean isCommitted();
 }


Reply via email to