You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2010/01/12 20:29:12 UTC

svn commit: r898476 - in /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5: ValidationTrackerImpl.java ValidationTrackerWrapper.java corelib/components/Form.java

Author: hlship
Date: Tue Jan 12 19:29:11 2010
New Revision: 898476

URL: http://svn.apache.org/viewvc?rev=898476&view=rev
Log:
TAP5-979: Form component should be more careful with the validation tracker to ensure that a session is not created unless needed

Added:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerWrapper.java   (with props)
Modified:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerImpl.java?rev=898476&r1=898475&r2=898476&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerImpl.java Tue Jan 12 19:29:11 2010
@@ -1,4 +1,4 @@
-// Copyright 2006, 2008 The Apache Software Foundation
+// Copyright 2006, 2008, 2010 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,7 +22,8 @@
 import java.util.Map;
 
 /**
- * Standard implmentation of {@link ValidationTracker}. Works pretty hard to ensure a minimum amount of data is stored
+ * Standard implementation of {@link ValidationTracker}. Works pretty hard to ensure a minimum
+ * amount of data is stored
  * in the HttpSession.
  */
 public final class ValidationTrackerImpl extends BaseOptimizedSessionPersistedObject implements ValidationTracker, Serializable

Added: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerWrapper.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerWrapper.java?rev=898476&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerWrapper.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerWrapper.java Tue Jan 12 19:29:11 2010
@@ -0,0 +1,84 @@
+// Copyright 2010 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5;
+
+import java.util.List;
+
+/**
+ * Wrapper around a {@link ValidationTracker} that delegates all methods to the wrapped instance.
+ * Subclasses will often override specific methods.
+ * 
+ * @since 5.2.0
+ */
+public class ValidationTrackerWrapper implements ValidationTracker
+{
+    private final ValidationTracker delegate;
+
+    public ValidationTrackerWrapper(ValidationTracker delegate)
+    {
+        this.delegate = delegate;
+    }
+
+    public void clear()
+    {
+        delegate.clear();
+    }
+
+    public String getError(Field field)
+    {
+        return delegate.getError(field);
+    }
+
+    public List<String> getErrors()
+    {
+        return delegate.getErrors();
+    }
+
+    public boolean getHasErrors()
+    {
+        return delegate.getHasErrors();
+    }
+
+    public String getInput(Field field)
+    {
+        return delegate.getInput(field);
+    }
+
+    public boolean inError(Field field)
+    {
+        return delegate.inError(field);
+    }
+
+    public void recordError(Field field, String errorMessage)
+    {
+        delegate.recordError(field, errorMessage);
+    }
+
+    public void recordError(String errorMessage)
+    {
+        delegate.recordError(errorMessage);
+    }
+
+    public void recordInput(Field field, String input)
+    {
+        delegate.recordInput(field, input);
+    }
+
+    /** Returns the instance to which methods are delegated. */
+    protected ValidationTracker getDelegate()
+    {
+        return delegate;
+    }
+}

Propchange: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/ValidationTrackerWrapper.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java?rev=898476&r1=898475&r2=898476&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java Tue Jan 12 19:29:11 2010
@@ -4,7 +4,7 @@
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
-//     http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ -46,27 +46,24 @@
  * types of fields.
  * <p/>
  * A Form emits many notification events. When it renders, it fires a
- * {@link org.apache.tapestry5.EventConstants#PREPARE_FOR_RENDER} notification,
- * followed by a {@link org.apache.tapestry5.EventConstants#PREPARE}
- * notification.
+ * {@link org.apache.tapestry5.EventConstants#PREPARE_FOR_RENDER} notification, followed by a
+ * {@link org.apache.tapestry5.EventConstants#PREPARE} notification.
  * <p/>
- * When the form is submitted, the component emits several notifications: first
- * a {@link org.apache.tapestry5.EventConstants#PREPARE_FOR_SUBMIT}, then a
- * {@link org.apache.tapestry5.EventConstants#PREPARE}: these allow the page to
- * update its state as necessary to prepare for the form submission, then (after
- * components enclosed by the form have operated), a
- * {@link org.apache.tapestry5.EventConstants#VALIDATE_FORM} event is emitted,
- * to allow for cross-form validation. After that, either a
+ * When the form is submitted, the component emits several notifications: first a
+ * {@link org.apache.tapestry5.EventConstants#PREPARE_FOR_SUBMIT}, then a
+ * {@link org.apache.tapestry5.EventConstants#PREPARE}: these allow the page to update its state as
+ * necessary to prepare for the form submission, then (after components enclosed by the form have
+ * operated), a {@link org.apache.tapestry5.EventConstants#VALIDATE_FORM} event is emitted, to allow
+ * for cross-form validation. After that, either a
  * {@link org.apache.tapestry5.EventConstants#SUCCESS} OR
- * {@link org.apache.tapestry5.EventConstants#FAILURE} event (depending on
- * whether the {@link ValidationTracker} has recorded any errors). Lastly, a
- * {@link org.apache.tapestry5.EventConstants#SUBMIT} event, for any listeners
- * that care only about form submission, regardless of success or failure.
+ * {@link org.apache.tapestry5.EventConstants#FAILURE} event (depending on whether the
+ * {@link ValidationTracker} has recorded any errors). Lastly, a
+ * {@link org.apache.tapestry5.EventConstants#SUBMIT} event, for any listeners that care only about
+ * form submission, regardless of success or failure.
  * <p/>
- * For all of these notifications, the event context is derived from the
- * <strong>context</strong> parameter. This context is encoded into the form's
- * action URI (the parameter is not read when the form is submitted, instead the
- * values encoded into the form are used).
+ * For all of these notifications, the event context is derived from the <strong>context</strong>
+ * parameter. This context is encoded into the form's action URI (the parameter is not read when the
+ * form is submitted, instead the values encoded into the form are used).
  */
 @Events(
 { EventConstants.PREPARE_FOR_RENDER, EventConstants.PREPARE, EventConstants.PREPARE_FOR_SUBMIT,
@@ -75,44 +72,37 @@
 public class Form implements ClientElement, FormValidationControl
 {
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String PREPARE_FOR_RENDER = EventConstants.PREPARE_FOR_RENDER;
 
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String PREPARE_FOR_SUBMIT = EventConstants.PREPARE_FOR_SUBMIT;
 
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String PREPARE = EventConstants.PREPARE;
 
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String SUBMIT = EventConstants.SUBMIT;
 
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String VALIDATE_FORM = EventConstants.VALIDATE_FORM;
 
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String SUCCESS = EventConstants.SUCCESS;
 
     /**
-     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants}
-     *             instead.
+     * @deprecated Use constant from {@link org.apache.tapestry5.EventConstants} instead.
      */
     public static final String FAILURE = EventConstants.FAILURE;
 
@@ -135,7 +125,7 @@
     /**
      * The object which will record user input and validation errors. The object
      * must be persistent between requests
-     * (since the form submission and validation occurs in an component event
+     * (since the form submission and validation occurs in a component event
      * request and the subsequent render occurs
      * in a render request). The default is a persistent property of the Form
      * component and this is sufficient for
@@ -182,8 +172,7 @@
     /**
      * Prefix value used when searching for validation messages and constraints.
      * The default is the Form component's
-     * id. This is overriden by
-     * {@link org.apache.tapestry5.corelib.components.BeanEditForm}.
+     * id. This is overriden by {@link org.apache.tapestry5.corelib.components.BeanEditForm}.
      * 
      * @see org.apache.tapestry5.services.FormSupport#getFormValidationId()
      */
@@ -192,8 +181,7 @@
 
     /**
      * Object to validate during the form submission process. The value of this
-     * parameter is pushed as a
-     * {@link org.apache.tapestry5.services.BeanValidationContext} into the
+     * parameter is pushed as a {@link org.apache.tapestry5.services.BeanValidationContext} into the
      * environment. This parameter should
      * only be used in combination with the Bean Validation Library.
      */
@@ -263,11 +251,61 @@
         return resources.getContainer();
     }
 
-    public ValidationTracker getDefaultTracker()
+    /**
+     * Returns a wrapped version of the tracker parameter (which is usually bound to the
+     * defaultTracker persistent field).
+     * If tracker is currently null, a new instance of {@link ValidationTrackerImpl} is created.
+     * The tracker is then wrapped, such that the tracker parameter
+     * is only updated the first time an error is recorded into the tracker (this will typically
+     * propagate to the defaultTracker
+     * persistent field and be stored into the session). This means that if no errors are recorded,
+     * the tracker parameter is not updated and (in the default case) no data is stored into the
+     * session.
+     * 
+     * @see <a href="https://issues.apache.org/jira/browse/TAP5-979">TAP5-979</a>
+     * @return a tracker ready to receive data (possibly a previously stored tracker with field
+     *         input and errors)
+     */
+    private ValidationTracker getWrappedTracker()
     {
-        if (defaultTracker == null)
-            defaultTracker = new ValidationTrackerImpl();
+        ValidationTracker innerTracker = tracker == null ? new ValidationTrackerImpl() : tracker;
+
+        ValidationTracker wrapper = new ValidationTrackerWrapper(innerTracker)
+        {
+            private boolean saved = false;
+
+            private void save()
+            {
+                if (!saved)
+                {
+                    tracker = getDelegate();
+
+                    saved = true;
+                }
+            }
+
+            @Override
+            public void recordError(Field field, String errorMessage)
+            {
+                super.recordError(field, errorMessage);
+
+                save();
+            }
+
+            @Override
+            public void recordError(String errorMessage)
+            {
+                super.recordError(errorMessage);
 
+                save();
+            }
+        };
+
+        return wrapper;
+    }
+
+    public ValidationTracker getDefaultTracker()
+    {
         return defaultTracker;
     }
 
@@ -294,12 +332,12 @@
 
         // Pre-register some names, to prevent client-side collisions with function names
         // attached to the JS Form object.
-        
+
         IdAllocator allocator = new IdAllocator();
-        
+
         allocator.allocateId("reset");
         allocator.allocateId("submit");
-        
+
         formSupport = createRenderTimeFormSupport(name, actionSink, allocator);
 
         if (zone != null)
@@ -309,15 +347,16 @@
         // of a push() method
         // for this kind of check?
 
+        ValidationTracker wrapped = getWrappedTracker();
+
         environment.push(FormSupport.class, formSupport);
-        environment.push(ValidationTracker.class, tracker);
+        environment.push(ValidationTracker.class, wrapped);
         environment.push(BeanValidationContext.class, new BeanValidationContextImpl(validate));
 
-
         if (autofocus)
         {
             ValidationDecorator autofocusDecorator = new AutofocusValidationDecorator(environment
-                    .peek(ValidationDecorator.class), tracker, renderSupport);
+                    .peek(ValidationDecorator.class), wrapped, renderSupport);
             environment.push(ValidationDecorator.class, autofocusDecorator);
         }
 
@@ -350,13 +389,12 @@
         }
 
         writer.end(); // div
-        
+
         environment.peek(Heartbeat.class).begin();
     }
 
     /**
-     * Creates an
-     * {@link org.apache.tapestry5.corelib.internal.InternalFormSupport} for
+     * Creates an {@link org.apache.tapestry5.corelib.internal.InternalFormSupport} for
      * this Form. This method is used
      * by {@link org.apache.tapestry5.corelib.components.FormInjector}.
      * 
@@ -405,7 +443,7 @@
         formSupport = null;
 
         environment.pop(ValidationTracker.class);
-        
+
         environment.pop(BeanValidationContext.class);
     }
 
@@ -414,11 +452,13 @@
     @Log
     Object onAction(EventContext context) throws IOException
     {
-        tracker.clear();
+        ValidationTracker wrapped = getWrappedTracker();
+
+        wrapped.clear();
 
         formSupport = new FormSupportImpl(resources, validationId);
 
-        environment.push(ValidationTracker.class, tracker);
+        environment.push(ValidationTracker.class, wrapped);
         environment.push(FormSupport.class, formSupport);
         environment.push(BeanValidationContext.class, new BeanValidationContextImpl(validate));
 
@@ -463,10 +503,10 @@
             // true persistent data, not value from the previous form
             // submission.
 
-            if (!tracker.getHasErrors())
-                tracker.clear();
+            if (!wrapped.getHasErrors())
+                wrapped.clear();
 
-            resources.triggerContextEvent(tracker.getHasErrors() ? EventConstants.FAILURE
+            resources.triggerContextEvent(wrapped.getHasErrors() ? EventConstants.FAILURE
                     : EventConstants.SUCCESS, context, callback);
 
             // Lastly, tell anyone whose interested that the form is completely
@@ -484,13 +524,7 @@
             environment.pop(Heartbeat.class);
             environment.pop(FormSupport.class);
 
-            // This forces an update that feeds through the system and gets the
-            // updated
-            // state of the tracker (if using the Form's defaultTracker
-            // property, which is flash persisted)
-            // stored back into the session.
-
-            tracker = environment.pop(ValidationTracker.class);
+            environment.pop(ValidationTracker.class);
 
             environment.pop(BeanValidationContext.class);
         }
@@ -509,7 +543,10 @@
 
             if (ve != null)
             {
-                recordError(ve.getMessage());
+                ValidationTracker tracker = environment.peek(ValidationTracker.class);
+
+                tracker.recordError(ve.getMessage());
+
                 return;
             }
 
@@ -581,22 +618,22 @@
 
     public void recordError(String errorMessage)
     {
-        tracker.recordError(errorMessage);
+        getWrappedTracker().recordError(errorMessage);
     }
 
     public void recordError(Field field, String errorMessage)
     {
-        tracker.recordError(field, errorMessage);
+        getWrappedTracker().recordError(field, errorMessage);
     }
 
     public boolean getHasErrors()
     {
-        return tracker.getHasErrors();
+        return getWrappedTracker().getHasErrors();
     }
 
     public boolean isValid()
     {
-        return !tracker.getHasErrors();
+        return !getWrappedTracker().getHasErrors();
     }
 
     // For testing:
@@ -608,7 +645,7 @@
 
     public void clearErrors()
     {
-        tracker.clear();
+        getWrappedTracker().clear();
     }
 
     /**



Re: svn commit: r898476 - in /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5: ValidationTrackerImpl.java ValidationTrackerWrapper.java corelib/components/Form.java

Posted by Massimo Lusetti <ml...@gmail.com>.
On Thu, Jan 14, 2010 at 6:23 PM, Howard Lewis Ship <hl...@gmail.com> wrote:

> Please add an issue, I'll check into it. Behavior was not intended to
> have changed, and not sure exactly how such a change slipped in.  Good
> catch!

Done: https://issues.apache.org/jira/browse/TAP5-987

Cheers
-- 
Massimo
http://meridio.blogspot.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: svn commit: r898476 - in /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5: ValidationTrackerImpl.java ValidationTrackerWrapper.java corelib/components/Form.java

Posted by Howard Lewis Ship <hl...@gmail.com>.
Please add an issue, I'll check into it. Behavior was not intended to
have changed, and not sure exactly how such a change slipped in.  Good
catch!

On Thu, Jan 14, 2010 at 8:54 AM, Massimo Lusetti <ml...@gmail.com> wrote:
> On Thu, Jan 14, 2010 at 5:51 PM, Massimo Lusetti <ml...@gmail.com> wrote:
>
>> void onValidateFormFromFormId()
>> {
>>    // while here do me check and...
>>    formId.recordErorr("BIG ERROR");
>> }
>
>
> For the records changing onValidate from void to object and return
> this in case of error does reproduce the previous behavior...
>
> Cheers
> --
> Massimo
> http://meridio.blogspot.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>



-- 
Howard M. Lewis Ship

Creator of Apache Tapestry

The source for Tapestry training, mentoring and support. Contact me to
learn how I can get you up and productive in Tapestry fast!

(971) 678-5210
http://howardlewisship.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: svn commit: r898476 - in /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5: ValidationTrackerImpl.java ValidationTrackerWrapper.java corelib/components/Form.java

Posted by Massimo Lusetti <ml...@gmail.com>.
On Thu, Jan 14, 2010 at 5:51 PM, Massimo Lusetti <ml...@gmail.com> wrote:

> void onValidateFormFromFormId()
> {
>    // while here do me check and...
>    formId.recordErorr("BIG ERROR");
> }


For the records changing onValidate from void to object and return
this in case of error does reproduce the previous behavior...

Cheers
-- 
Massimo
http://meridio.blogspot.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: svn commit: r898476 - in /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5: ValidationTrackerImpl.java ValidationTrackerWrapper.java corelib/components/Form.java

Posted by Massimo Lusetti <ml...@gmail.com>.
On Tue, Jan 12, 2010 at 8:29 PM,  <hl...@apache.org> wrote:

> Author: hlship
> Date: Tue Jan 12 19:29:11 2010
> New Revision: 898476
>
> URL: http://svn.apache.org/viewvc?rev=898476&view=rev
> Log:
> TAP5-979: Form component should be more careful with the validation tracker to ensure that a session is not created unless needed
>

I think this is causing some hassle to validation mechanism, i got a form with:

@Component
private Form formId;

void onValidateFormFromFormId()
{
    // while here do me check and...
    formId.recordErorr("BIG ERROR");
}

void onFailureFromFormId()
{
     // clean up
}


void onSuccessFromFormId()
{
    // record into db and go on...
}


Well my onFailure is never called while onSuccess is always... does
anyone else has noted this behavior?

Cheers
-- 
Massimo
http://meridio.blogspot.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org