The constraintValidatorContext fluent API for building errors. Please review.

Begin forwarded message:

From: Emmanuel Bernard <emmanuel.bernard@jboss.com>
Date:  June 18, 2009 16:55:06  EDT
To: jsr-303-eg@jcp.org
Subject: Re: [jsr-303-eg] Path, string or object model

Here is the solution for ConstraintValidatorContext.
I've designed a fluent API to create error reports and add their message and potentially their sub nodes and the appropriate contextual information.

Here is a usage example

public boolean isValid(String value, ConstraintValidatorContext context) {
//default path
context.buildErrorWithMessage( "this detail is wrong" ).addError();

//default path + "street"
context.buildErrorWithMessage( "this detail is wrong" )
.inSubNode( "street" )
.addError();

//default path + "addresses["home"].country.name
context.buildErrorWithMessage( "this detail is wrong" )
.inSubNode( "addresses" )
.inSubNode( "country" )
.inIterable().atKey( "home" )
.inSubNode( "name" )
.addError();
return false;
}

I've used a nested interface model to keep things clean. Please review and comment.

/**
 * Provide contextual data and operation when applying a given constraint validator
 *
 * @author Emmanuel Bernard
 */
public interface ConstraintValidatorContext {
/**
* Disable the default error message and default ConstraintViolation object generation.
* Useful to set a different error message or generate a ConstraintViolation based on
* a different property
*/
void disableDefaultError();

/**
* @return the current uninterpolated default message
*/
String getDefaultErrorMessage();

/**
* Return an error builder building an error allowing to optionally associate
* the error to a sub path.
* The error message will be interpolated.
* <p/>
* To create the error, one must call either one of
* the #addError method available in one of the
* interfaces of the fluent API.
* If another method is called after #addError() on
* ErrorBuilder or any of its associated nested interfaces
* an IllegalStateException is raised.
* <p/>
* If <code>isValid<code> returns <code>false</code>, a <code>ConstraintViolation</code> object will be built
* per error including the default one unless {@link #disableDefaultError()} has been called.
* <p/>
* <code>ConstraintViolation</code> objects generated from such a call
* contain the same contextual information (root bean, path and so on) unless
* the path has been overriden.
* <p/>
* To create a different error, a new error builder has to be retrieved from
* ConstraintValidatorContext
*
* Here are a few usage examples:
* <pre>//create new error with the default path the constraint
* //is located on
* context.buildErrorWithMessage( "way too long" )
*             .addError();
*
* //create new error in the "street" subnode of the default
* //path the constraint is located on
* context.buildErrorWithMessage( "way too long" )
*              .inSubNode( "street" )
*              .addError();
*
* //create new error in the "addresses["home"].city.name
* //subnode of the default path the constraint is located on
* context.buildErrorWithMessage( "this detail is wrong" )
*              .inSubNode( "addresses" )
*              .inSubNode( "country" )
*                  .inIterable().atKey( "home" )
*              .inSubNode( "name" )
*              .addError();
* </pre>
*
* @param message new uninterpolated error message.
*/
ErrorBuilder buildErrorWithMessage(String message);

/**
* Error builder allowing to optionally associate
* the error to a sub path.
*
* To create the error, one must call either one of
* the #addError method available in one of the
* interfaces of the fluent API.
* If another method is called after #addError() on
* ErrorBuilder or any of its associated objects
* an IllegalStateException is raised.
*/
interface ErrorBuilder {
/**
* Add a subNode to the path the error will be associated to
*
* @param name property
* @return a builder representing the first level node
*/
NodeBuilder inSubNode(String name);

/**
* Add the new error report to be generated if the
* constraint validator mark the value as invalid.
* Methods of this ErrorBuilder instance and its nested
* objects returns IllegalStateException from now on.
*
* @return ConstraintValidatorContext instance the ErrorBuilder comes from 
*/
ConstraintValidatorContext addError();

/**
* Represent asubnode whose context is known
* (ie index, key and isInIterable)
*/
interface NodeBuilder {
/**
* Add a subNode to the path the error will be associated to
*
* @param name property
* @return a builder representing this node
*/
InIterableNodeBuilder inSubNode(String name);

/**
* Add the new error report to be generated if the
* constraint validator mark the value as invalid.
* Methods of the ErrorBuilder instance this object comes
* from and the error builder nested
* objects returns IllegalStateException from now on.
*
* @return ConstraintValidatorContext instance the ErrorBuilder comes from
*/
ConstraintValidatorContext addError();
}

/**
* Represent a subnode whose context is
* configurable (ie index, key and isInIterable)
*/
interface InIterableNodeBuilder {
/**
* Mark the node as being in an Iterable or a Map
* @return a builder representing iterable details
*/
InIterablePropertiesBuilder inIterable();

/**
* Add a subNode to the path the error will be associated to
*
* @param name property
* @return a builder representing this node
*/
InIterableNodeBuilder inSubNode(String name);

/**
* Add the new error report to be generated if the
* constraint validator mark the value as invalid.
* Methods of the ErrorBuilder instance this object comes
* from and the error builder nested
* objects returns IllegalStateException from now on.
*
* @return ConstraintValidatorContext instance the ErrorBuilder comes from
*/
ConstraintValidatorContext addError();
}

/**
* Represent choices for a node which is
* in an Iterator or Map.
* If the iterator is an indexed collection or a map,
* the index or the key should be set.
*/
interface InIterablePropertiesBuilder {
/**
* Define the key the object is into the Map
*
* @param key map key
* @return a builder representing the current node
*/
NodeBuilder atKey(Object key);

/**
* Define the index the object is into the List or array
*
* @param index index
* @return a builder representing the current node
*/
NodeBuilder atIndex(Integer index);

/**
* Add a subNode to the path the error will be associated to
*
* @param name property
* @return a builder representing this node
*/
InIterableNodeBuilder inSubNode(String name);

/**
* Add the new error report to be generated if the
* constraint validator mark the value as invalid.
* Methods of the ErrorBuilder instance this object comes
* from and the error builder nested
* objects returns IllegalStateException from now on.
*
* @return ConstraintValidatorContext instance the ErrorBuilder comes from
*/
ConstraintValidatorContext addError();
}
}
}


On  Jun 17, 2009, at 14:41, Emmanuel Bernard wrote:

Here is the solution I cooked. I think it's acceptable but add some complexity in the ConstraintValidatorContext API (see other email). Please review (pay special attention to the examples).

A Path represents the path and is the one accepted by the path consuming APIs. A Path is an Iterable of Nodes.

/**
 * Represent a navigation path from an object to another.
 * Each path element is represented by a Node.
 *
 * The path corresponds to the succession of nodes
 * in the order they are retured by the Iterator
 *
 * @author Emmanuel Bernard
 */
public interface Path extends Iterable<Node> {
}


A node represent a path element.

/**
 * Represents an element of a navigation path
 *
 * @author Emmanuel Bernard
 */
public interface Node {
/**
* Property name the node represents
* or null if the leaf node and representing an entity
* (in particular the node representing the root object
* has its name null)
*/
String getName();

/**
* True if the node represents an object contained in an Iterable
* or in a Map.
*/
boolean isInIterable();

/**
* The index the node is placed in if contained
* in an array or List. Null otherwise.
*/
Integer getIndex();

/**
* The key the node is placed in if contained
* in a Map. Null otherwise.
*/
Object getKey();
}

A few interesting points:
 - the index / key is hosted by the node after the collection node

Here are a few examples and their Node equivalent

""
0: Node(name:null, isInIterable:false, index:null, key:null)

"email"
0: Node(name:email, isInIterable:false, index:null, key:null)

"addresses"
0: Node(name:addresses, isInIterable:false, index:null, key:null)

"addresses["home"]" represent the bean level of an Address object
0: Node(name:addresses, isInIterable:false, index:null, key:null)
1: Node(name:null, isInIterable:true, index:null, key:home)

"addresses["home"].city"
0: Node(name:addresses, isInIterable:false, index:null, key:null)
1: Node(name:city, isInIterable:true, index:null, key:home)

"billingAddresses[3].country.name"
0: Node(name:billingAddresses, isInIterable:false, index:null, key:null)
1: Node(name:country, isInIterable:true, index:3, key:null)
2: Node(name:name, isInIterable:false, index:null, key:null)


ConstraintViolation renders a Path

public interface ConstraintViolation<T> {
    Path getPropertyPath();
}

TraversableResolver accepts a path

public interface TraversableResolver {
boolean isReachable(Object traversableObject,
 String traversableProperty,
 Class<?> rootBeanType,
 Path pathToTraversableObject,
 ElementType elementType);
    ...
}

PS: should String traversableProperty be Node traversableProperty ?


On  May 27, 2009, at 12:19, Emmanuel Bernard wrote:

In several areas we do describe path:
 - ConstraintViolation
 - ConstraintValidatorContext (with addError(String, String) which allows to concatenate substrings

So far we use the notion of string to represent it
 - address.name
 - cards[3].color
 - addresses["home"].city

I have added the idea of using [] for simple Iterable objects (ie non indexed, like a Set)
 - accounts[].balance

Anybody objects to that?

Second point
Do we want to replace this String approach with a path object mode?

______
path are today strings with dot separating properties. But it break when Set or Iterable are used. 
We could replace that with 
--- First strawman, must evolve -- 
class PathElement { 
  String getName(); 
  PathElement getParentPath(); 
  boolean isIterable(); 
  boolean isIndexed(); 
  Object getIndex(); 
  //TODO int getIndex()? 

  // not happy about that as it is only useful for Constraintciolation 
  PathElement getChild(); 


PathElement would be used for Constraintvuilation, maybe CVContext etc 

can this be refactored using inheritance + generics to have an IndexedPathElement only when it matters (probably no unfortunately) 
______


Pros:
 - less string manipulation by the user and the TraversableResolver implementation
 - the map index no longer rely on "[" + toString() + "]" and is likely more easily handled

Cons:
 - ConstraintValidatorContext becomes more complex as it needs to expose some kind of path element builder.
 - we would like need to standardize some kind of String serialization anyway
 - I don't see Pros as huge advantages

WDYT?

_______________________________________________
jsr-303-eg mailing list jsr-303-eg@jcp.org