You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by th...@apache.org on 2014/01/10 18:25:27 UTC

git commit: TAP5-1718: Tapestry-beanvalidator isn't validating nested DTO objects / Doesn't mark the invalid fields in the UI

Updated Branches:
  refs/heads/master 8b7be0060 -> 3819b855d


TAP5-1718: Tapestry-beanvalidator isn't validating nested DTO objects / Doesn't mark the invalid fields in the UI


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/3819b855
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/3819b855
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/3819b855

Branch: refs/heads/master
Commit: 3819b855d24ef9a48654e33b6bdc1bcd7bba74d5
Parents: 8b7be00
Author: Thiago H. de Paula Figueiredo <th...@apache.org>
Authored: Fri Jan 10 15:25:12 2014 -0200
Committer: Thiago H. de Paula Figueiredo <th...@apache.org>
Committed: Fri Jan 10 15:25:12 2014 -0200

----------------------------------------------------------------------
 .../beanvalidator/BeanFieldValidator.java       | 43 +++++++++++++++++---
 .../TapestryBeanValidationIntegrationTests.java | 21 ++++++++++
 .../example/testapp/entities/ComplexBean.java   | 13 ++++++
 .../example/testapp/pages/NestedObjectDemo.java | 38 +++++++++++++++++
 .../org/example/testapp/services/AppModule.java |  1 +
 .../src/test/webapp/Index.tml                   |  3 ++
 .../src/test/webapp/NestedObjectDemo.tml        | 13 ++++++
 .../internal/bindings/PropBinding.java          | 12 +++++-
 .../internal/bindings/PropBindingFactory.java   |  2 +-
 .../InternalComponentResourcesImpl.java         |  7 ++++
 10 files changed, 144 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/main/java/org/apache/tapestry5/internal/beanvalidator/BeanFieldValidator.java
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/main/java/org/apache/tapestry5/internal/beanvalidator/BeanFieldValidator.java b/tapestry-beanvalidator/src/main/java/org/apache/tapestry5/internal/beanvalidator/BeanFieldValidator.java
index 81149d7..b26a797 100644
--- a/tapestry-beanvalidator/src/main/java/org/apache/tapestry5/internal/beanvalidator/BeanFieldValidator.java
+++ b/tapestry-beanvalidator/src/main/java/org/apache/tapestry5/internal/beanvalidator/BeanFieldValidator.java
@@ -33,6 +33,7 @@ import javax.validation.ValidatorFactory;
 import javax.validation.metadata.BeanDescriptor;
 import javax.validation.metadata.ConstraintDescriptor;
 import javax.validation.metadata.PropertyDescriptor;
+
 import java.lang.annotation.Annotation;
 import java.util.Iterator;
 import java.util.Map;
@@ -139,15 +140,27 @@ public class BeanFieldValidator implements FieldValidator
         String currentProperty = beanValidationContext.getCurrentProperty();
 
         if (currentProperty == null) return;
+        
+        Class<?> beanType = beanValidationContext.getBeanType();
+        String[] path = currentProperty.split("\\.");
+        BeanDescriptor beanDescriptor = validator.getConstraintsForClass(beanType);
+        
+        for (int i = 1; i < path.length - 1; i++) 
+        {
+            Class<?> constrainedPropertyClass = getConstrainedPropertyClass(beanDescriptor, path[i]);
+            if (constrainedPropertyClass != null) {
+                beanType = constrainedPropertyClass;
+                beanDescriptor = validator.getConstraintsForClass(beanType);
+            }
+        }
 
-        BeanDescriptor beanDescriptor = validator.getConstraintsForClass(beanValidationContext.getBeanType());
-
-        PropertyDescriptor propertyDescriptor = beanDescriptor.getConstraintsForProperty(currentProperty);
+        final String propertyName = path[path.length - 1];
+        PropertyDescriptor propertyDescriptor = beanDescriptor.getConstraintsForProperty(propertyName);
 
         if (propertyDescriptor == null) return;
 
         final Set<ConstraintViolation<Object>> violations = validator.validateValue(
-                (Class<Object>) beanValidationContext.getBeanType(), currentProperty,
+                (Class<Object>) beanType, propertyName,
                 value, beanValidationGroupSource.get());
 
         if (violations.isEmpty())
@@ -157,9 +170,9 @@ public class BeanFieldValidator implements FieldValidator
 
         final StringBuilder builder = new StringBuilder();
 
-        for (Iterator iterator = violations.iterator(); iterator.hasNext(); )
+        for (Iterator<ConstraintViolation<Object>> iterator = violations.iterator(); iterator.hasNext(); )
         {
-            ConstraintViolation<?> violation = (ConstraintViolation<Object>) iterator.next();
+            ConstraintViolation<?> violation = iterator.next();
 
             builder.append(format("%s %s", field.getLabel(), violation.getMessage()));
 
@@ -172,6 +185,24 @@ public class BeanFieldValidator implements FieldValidator
 
     }
 
+    /**
+     * Returns the class of a given property, but only if it is a constrained property of the
+     * parent class. Otherwise, it returns null.
+     */
+    final private static Class<?> getConstrainedPropertyClass(BeanDescriptor beanDescriptor, String propertyName)
+    {
+        Class<?> clasz = null;
+        for (PropertyDescriptor descriptor : beanDescriptor.getConstrainedProperties()) 
+        {
+            if (descriptor.getPropertyName().equals(propertyName)) 
+            {
+                clasz = descriptor.getElementClass();
+                break;
+            }
+        }
+        return clasz;
+    }
+
     private String interpolateMessage(final ConstraintDescriptor<?> descriptor)
     {
         String messageTemplate = (String) descriptor.getAttributes().get("message");

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/test/java/org/apache/tapestry5/beanvalidator/integration/TapestryBeanValidationIntegrationTests.java
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/test/java/org/apache/tapestry5/beanvalidator/integration/TapestryBeanValidationIntegrationTests.java b/tapestry-beanvalidator/src/test/java/org/apache/tapestry5/beanvalidator/integration/TapestryBeanValidationIntegrationTests.java
index c4fba7c..ef97a54 100644
--- a/tapestry-beanvalidator/src/test/java/org/apache/tapestry5/beanvalidator/integration/TapestryBeanValidationIntegrationTests.java
+++ b/tapestry-beanvalidator/src/test/java/org/apache/tapestry5/beanvalidator/integration/TapestryBeanValidationIntegrationTests.java
@@ -252,5 +252,26 @@ public class TapestryBeanValidationIntegrationTests extends SeleniumTestCase
                 "Min Value must be greater than or equal to 6", "Not Null String may not be null");
     }
 
+    // TAP5-1718
+    @Test
+    public void nested_object_validation() throws Exception
+    {
+        
+        final String locatorTemplate = "//p[@data-error-block-for='%s']";
+        
+        openLinks("NestedObject Demo");
+
+        clickAndWait(SUBMIT);
+        
+        assertEquals("You must provide a value for Not Null String.", 
+                getText(String.format(locatorTemplate, "notNullString")));
+        assertEquals("Simple Not Null Property may not be null", 
+                getText(String.format(locatorTemplate, "simpleNotNullProperty")));
+        assertEquals("Min Value must be greater than or equal to 6", 
+                getText(String.format(locatorTemplate, "minValue")));
+
+    }
+
+
 
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/test/java/org/example/testapp/entities/ComplexBean.java
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/test/java/org/example/testapp/entities/ComplexBean.java b/tapestry-beanvalidator/src/test/java/org/example/testapp/entities/ComplexBean.java
index beaa840..44af90d 100644
--- a/tapestry-beanvalidator/src/test/java/org/example/testapp/entities/ComplexBean.java
+++ b/tapestry-beanvalidator/src/test/java/org/example/testapp/entities/ComplexBean.java
@@ -1,10 +1,13 @@
 package org.example.testapp.entities;
 
+import javax.validation.Valid;
 import javax.validation.constraints.NotNull;
 
 public class ComplexBean
 {
 
+    @Valid
+    private SomeSimpleBean otherSimpleBean;
     private SomeSimpleBean someSimpleBean;
     private SomeOtherSimpleBean someOtherSimpleBean;
 
@@ -41,4 +44,14 @@ public class ComplexBean
         this.simpleNotNullProperty = simpleNotNullProperty;
     }
 
+    public SomeSimpleBean getOtherSimpleBean()
+    {
+        return otherSimpleBean;
+    }
+
+    public void setOtherSimpleBean(SomeSimpleBean otherSimpleBean)
+    {
+        this.otherSimpleBean = otherSimpleBean;
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/test/java/org/example/testapp/pages/NestedObjectDemo.java
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/test/java/org/example/testapp/pages/NestedObjectDemo.java b/tapestry-beanvalidator/src/test/java/org/example/testapp/pages/NestedObjectDemo.java
new file mode 100644
index 0000000..f6406bd
--- /dev/null
+++ b/tapestry-beanvalidator/src/test/java/org/example/testapp/pages/NestedObjectDemo.java
@@ -0,0 +1,38 @@
+package org.example.testapp.pages;
+
+import org.apache.tapestry5.alerts.AlertManager;
+import org.apache.tapestry5.alerts.Duration;
+import org.apache.tapestry5.alerts.Severity;
+import org.apache.tapestry5.annotations.Persist;
+import org.apache.tapestry5.annotations.Property;
+import org.apache.tapestry5.ioc.annotations.Inject;
+import org.example.testapp.entities.ComplexBean;
+import org.example.testapp.entities.SomeSimpleBean;
+
+public class NestedObjectDemo
+{
+
+    @Property
+    @Persist
+    private ComplexBean complexBean;
+    
+    @Property
+    @Persist
+    private String notNullString;
+    
+    @Inject
+    private AlertManager alertManager;
+    
+    public void onActivate() {
+        if (complexBean == null) { 
+            complexBean = new ComplexBean();
+            SomeSimpleBean otherSimpleBean = new SomeSimpleBean();
+            complexBean.setOtherSimpleBean(otherSimpleBean);
+        }
+    }
+    
+    void onSuccess() {
+        alertManager.alert(Duration.TRANSIENT, Severity.SUCCESS, "Validation passed");
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/test/java/org/example/testapp/services/AppModule.java
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/test/java/org/example/testapp/services/AppModule.java b/tapestry-beanvalidator/src/test/java/org/example/testapp/services/AppModule.java
index 2a4344a..cb3b5b4 100644
--- a/tapestry-beanvalidator/src/test/java/org/example/testapp/services/AppModule.java
+++ b/tapestry-beanvalidator/src/test/java/org/example/testapp/services/AppModule.java
@@ -28,6 +28,7 @@ public class AppModule
     public static void contributeApplicationDefaults(MappedConfiguration<String, String> configuration)
     {
         configuration.add(SymbolConstants.PRODUCTION_MODE, "false");
+        configuration.add(SymbolConstants.HMAC_PASSPHRASE, "u93490jhsprf2904rh29-3uj");
     }
     
 	public static void contributeBeanValidatorSource(

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/test/webapp/Index.tml
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/test/webapp/Index.tml b/tapestry-beanvalidator/src/test/webapp/Index.tml
index 0970c96..6734533 100644
--- a/tapestry-beanvalidator/src/test/webapp/Index.tml
+++ b/tapestry-beanvalidator/src/test/webapp/Index.tml
@@ -27,6 +27,9 @@
             <li>
                 <t:pagelink page="ComplexBeanDemo">ComplexBean Demo</t:pagelink>
             </li>            
+            <li>
+                <t:pagelink page="NestedObjectDemo">NestedObject Demo</t:pagelink>
+            </li>            
         </ul>
     </body>
 </html>

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-beanvalidator/src/test/webapp/NestedObjectDemo.tml
----------------------------------------------------------------------
diff --git a/tapestry-beanvalidator/src/test/webapp/NestedObjectDemo.tml b/tapestry-beanvalidator/src/test/webapp/NestedObjectDemo.tml
new file mode 100644
index 0000000..a42a1bc
--- /dev/null
+++ b/tapestry-beanvalidator/src/test/webapp/NestedObjectDemo.tml
@@ -0,0 +1,13 @@
+<html xmlns:t="http://tapestry.apache.org/schema/tapestry_5_1_0.xsd">
+    <body>
+    	<t:alerts/>
+        <t:form clientValidation="none" validate="complexBean">
+	    	<t:errors/>
+	    	<t:textfield t:id="notNullString" t:mixins="FormGroup" t:validate="required"/>
+        	<t:textfield t:id="simpleNotNullProperty" t:value="complexBean.simpleNotNullProperty" t:mixins="FormGroup"/>
+        	<t:textfield t:id="minValue" t:value="complexBean.otherSimpleBean.minValue" t:mixins="FormGroup"/>
+<!-- 			<t:beaneditor object="complexBean"/> -->
+        	<input type="submit" id="submit" value="Go"/>
+        </t:form>
+     </body>
+</html>
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java
index 4656fe2..0e7e097 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java
@@ -35,14 +35,16 @@ public class PropBinding extends AbstractBinding implements InternalPropBinding
     private final String toString;
 
     private boolean invariant;
+    
+    private final String expression;
 
-    public PropBinding(final Location location, final Object root, final PropertyConduit conduit, final String toString
-    )
+    public PropBinding(final Location location, final Object root, final PropertyConduit conduit, final String expression, final String toString)
     {
         super(location);
 
         this.root = root;
         this.conduit = conduit;
+        this.expression = expression;
         this.toString = toString;
 
         invariant = conduit.getAnnotation(Invariant.class) != null;
@@ -109,4 +111,10 @@ public class PropBinding extends AbstractBinding implements InternalPropBinding
 	{
 		return TapestryInternalUtils.toInternalPropertyConduit(conduit).getPropertyName();
 	}
+
+    public String getExpression()
+    {
+        return expression;
+    }
+	
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java
index 2865808..4f65bac 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java
@@ -51,6 +51,6 @@ public class PropBindingFactory implements BindingFactory
         String toString = interner.format("PropBinding[%s %s(%s)]", description, container
                 .getCompleteId(), expression);
 
-        return new PropBinding(location, target, conduit, toString);
+        return new PropBinding(location, target, conduit, expression, toString);
     }
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/3819b855/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
index 78cdd07..39defda 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
@@ -18,6 +18,7 @@ import org.apache.tapestry5.*;
 import org.apache.tapestry5.func.Worker;
 import org.apache.tapestry5.internal.InternalComponentResources;
 import org.apache.tapestry5.internal.bindings.InternalPropBinding;
+import org.apache.tapestry5.internal.bindings.PropBinding;
 import org.apache.tapestry5.internal.services.Instantiator;
 import org.apache.tapestry5.internal.transform.ParameterConduit;
 import org.apache.tapestry5.internal.util.NamedSet;
@@ -668,6 +669,12 @@ public class InternalComponentResourcesImpl extends LockSupport implements Inter
             return null;
         }
 
+        // TAP5-1718: we need the full prop binding expression, not just the (final) property name
+        if (binding instanceof PropBinding) 
+        {
+            return ((PropBinding) binding).getExpression();
+        }
+        
         if (binding instanceof InternalPropBinding)
         {
             return ((InternalPropBinding) binding).getPropertyName();