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/09 15:24:48 UTC

svn commit: r602673 - in /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry: ./ corelib/components/ internal/services/ internal/structure/ internal/test/ services/

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 {@link ComponentEvent}, notified when a non-null value is returned from some event
+ * Handler for a a {@linkplain org.apache.tapestry.runtime.Event render phase event) or
+ * {@link 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
+     * {@link 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();
 }