Reviewers: rchandia,
Description:
To prevent infinite loops, the Bean Validation implementation must
ignore
the cascading operation if the associated object instance has already
been
validated in the current navigation path (starting from the root
object).
[JSR 303 TCK Result] 108 of 257 (42.02%) Pass with 22 Failures and 9
Errors.
Please review this at http://gwt-code-reviews.appspot.com/1367802/
Affected files:
M user/src/com/google/gwt/validation/client/impl/GwtValidationContext.java
M
user/src/com/google/gwt/validation/rebind/GwtSpecificValidatorCreator.java
M
user/test/org/hibernate/jsr303/tck/tests/constraints/groups/GroupGwtTest.java
M
user/test/org/hibernate/jsr303/tck/tests/validation/graphnavigation/GraphNavigationGwtTest.java
Index:
user/src/com/google/gwt/validation/client/impl/GwtValidationContext.java
===================================================================
---
user/src/com/google/gwt/validation/client/impl/GwtValidationContext.java
(revision 9783)
+++
user/src/com/google/gwt/validation/client/impl/GwtValidationContext.java
(working copy)
@@ -16,6 +16,8 @@
package com.google.gwt.validation.client.impl;
import java.lang.annotation.Annotation;
+import java.util.HashSet;
+import java.util.Set;
import javax.validation.MessageInterpolator;
import javax.validation.metadata.BeanDescriptor;
@@ -24,7 +26,11 @@
/**
* Context for a {@link com.google.gwt.validation.client.GwtValidation}.
*
+ * <p>
+ * NOTE: This class is not thread safe.
+ *
* @param <T> the type of the root bean.
+ *
*/
public class GwtValidationContext<T> {
@@ -35,14 +41,36 @@
private final AbstractGwtValidator validator;
/**
- *
+ * The set of validated object.
+ * <p>
+ * This set is shared with and updated by children contexts created by
+ * {@link #append(String)}, {@link #appendIndex(String, int)} and
+ * {@link #appendKey(String, String)}.
*/
+ private final Set<Object> validatedObjects;
+
public GwtValidationContext(T rootBean, BeanDescriptor beanDescriptor,
MessageInterpolator messageInterpolator, AbstractGwtValidator
validator) {
+ this(rootBean, beanDescriptor, messageInterpolator, validator,
+ new HashSet<Object>());
+ }
+
+ private GwtValidationContext(T rootBean, BeanDescriptor beanDescriptor,
+ MessageInterpolator messageInterpolator, AbstractGwtValidator
validator,
+ Set<Object> validatedObjects) {
this.rootBean = rootBean;
this.beanDescriptor = beanDescriptor;
this.messageInterpolator = messageInterpolator;
this.validator = validator;
+ this.validatedObjects = validatedObjects;
+ }
+
+ public final void addValidatedObject(Object o) {
+ validatedObjects.add(o);
+ }
+
+ public final boolean alreadyValidated(Object o) {
+ return validatedObjects.contains(o);
}
/**
@@ -53,7 +81,7 @@
*/
public GwtValidationContext<T> append(String name) {
GwtValidationContext<T> temp = new GwtValidationContext<T>(rootBean,
- beanDescriptor, messageInterpolator, validator);
+ beanDescriptor, messageInterpolator, validator, validatedObjects);
temp.path = path.append(name);
return temp;
}
@@ -66,7 +94,7 @@
*/
public GwtValidationContext<T> appendIndex(String name, int index) {
GwtValidationContext<T> temp = new GwtValidationContext<T>(rootBean,
- beanDescriptor, messageInterpolator, validator);
+ beanDescriptor, messageInterpolator, validator, validatedObjects);
temp.path = path.appendIndex(name, index);
return temp;
}
@@ -79,7 +107,7 @@
*/
public GwtValidationContext<T> appendKey(String name, String key) {
GwtValidationContext<T> temp = new GwtValidationContext<T>(rootBean,
- beanDescriptor, messageInterpolator, validator);
+ beanDescriptor, messageInterpolator, validator, validatedObjects);
temp.path = path.appendKey(name, key);
return temp;
}
Index:
user/src/com/google/gwt/validation/rebind/GwtSpecificValidatorCreator.java
===================================================================
---
user/src/com/google/gwt/validation/rebind/GwtSpecificValidatorCreator.java
(revision 9783)
+++
user/src/com/google/gwt/validation/rebind/GwtSpecificValidatorCreator.java
(working copy)
@@ -222,7 +222,7 @@
/**
* Gets the best {@link ConstraintValidator}.
- *
+ *
* <p>
* The ConstraintValidator chosen to validate a declared type
* {@code targetType} is the one where the type supported by the
@@ -230,11 +230,11 @@
* no other ConstraintValidator whose supported type is a supertype of
* {@code type} and not a supertype of the chosen ConstraintValidator
* supported type.
- *
+ *
* @param constraint the constraint to find ConstraintValidators for.
* @param targetType The type to find a ConstraintValidator for.
* @return ConstraintValidator
- *
+ *
* @throws UnexpectedTypeException if there is not exactly one maximally
* specific constraint validator for targetType.
*/
@@ -932,6 +932,9 @@
writeNewViolations(sw);
+ // context.addValidatedObject(object);
+ sw.println("context.addValidatedObject(object);");
+
// /// For each group
// TODO(nchalko) handle the sequence in the AbstractValidator
@@ -988,6 +991,7 @@
writeCatchUnexpectedException(sw,
"\"Error validating " + beanHelper.getTypeCanonicalName() + "\"");
+ // }
sw.outdent();
sw.println("}");
}
@@ -1118,11 +1122,11 @@
sw.println("int i = 0;");
// for (Object instance : value) {
- sw.print("for(Object instance : value) {");
- sw.indent();
-
- // if(instance != null ) {
- sw.println(" if(instance != null ) {");
+ sw.println("for(Object instance : value) {");
+ sw.indent();
+
+ // if(instance != null && !context.alreadyValidated(instance)) {
+ sw.println(" if(instance != null
&& !context.alreadyValidated(instance)) {");
sw.indent();
// violations.addAll(
@@ -1160,11 +1164,12 @@
// for (Entry<?, Type> entry : value.entrySet()) {
sw.print("for(");
sw.print(Entry.class.getCanonicalName());
- sw.print("<?, ?> entry : value.entrySet()) {");
- sw.indent();
-
- // if(entry.getValue() != null ) {
- sw.println(" if(entry.getValue() != null ) {");
+ sw.println("<?, ?> entry : value.entrySet()) {");
+ sw.indent();
+
+ // if(entry.getValue() != null &&
+ // !context.alreadyValidated(entry.getValue())) {
+ sw.println(" if(entry.getValue() != null
&& !context.alreadyValidated(entry.getValue())) {");
sw.indent();
// violations.addAll(
@@ -1363,10 +1368,19 @@
}
} else {
createBeanHelper(elementClass);
+
+ // if (!context.alreadyValidated(value)) {
+ sw.println(" if (!context.alreadyValidated(value)) {");
+ sw.indent();
+
// violations.addAll(myContext.getValidator().validate(context,
value,
// groups));
sw.print("violations.addAll(");
sw.println("myContext.getValidator().validate(myContext, value,
groups));");
+
+ // }
+ sw.outdent();
+ sw.println("}");
}
// }
Index:
user/test/org/hibernate/jsr303/tck/tests/constraints/groups/GroupGwtTest.java
===================================================================
---
user/test/org/hibernate/jsr303/tck/tests/constraints/groups/GroupGwtTest.java
(revision 9783)
+++
user/test/org/hibernate/jsr303/tck/tests/constraints/groups/GroupGwtTest.java
(working copy)
@@ -42,7 +42,6 @@
delegate.testCyclicGroupSequence();
}
- @Failing(issue = 5801)
public void testGroups() {
delegate.testGroups();
}
Index:
user/test/org/hibernate/jsr303/tck/tests/validation/graphnavigation/GraphNavigationGwtTest.java
===================================================================
---
user/test/org/hibernate/jsr303/tck/tests/validation/graphnavigation/GraphNavigationGwtTest.java
(revision 9783)
+++
user/test/org/hibernate/jsr303/tck/tests/validation/graphnavigation/GraphNavigationGwtTest.java
(working copy)
@@ -53,9 +53,7 @@
delegate.testGraphNavigationDeterminism();
}
- @Failing(issue = 5946)
public void testNoEndlessLoop() {
- fail("Fail early so othe tests pass");
delegate.testNoEndlessLoop();
}
@@ -63,7 +61,6 @@
delegate.testNullReferencesGetIgnored();
}
- @Failing(issue = 5946)
public void testTypeOfContainedValueIsDeterminedAtRuntime() {
delegate.testTypeOfContainedValueIsDeterminedAtRuntime();
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
