Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 120582 - [DataBinding] Current validators mix data type conversion and business rule validation concerns
Summary: [DataBinding] Current validators mix data type conversion and business rule v...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: PC All
: P1 normal (vote)
Target Milestone: 3.2 M5   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 118570 119504 (view as bug list)
Depends on: 118323
Blocks: 124403
  Show dependency tree
 
Reported: 2005-12-13 10:25 EST by Dave Orme CLA
Modified: 2006-02-16 12:38 EST (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Orme CLA 2005-12-13 10:25:59 EST
In bug 104570, comment #46, Christian Schaefer writes that he thinks validators should be chained.  When I think more closesly about this, I think he's right.

Currently, our IValidator architecture mixes two separate concerns:

1) Making sure that a less restricted type (ie: a String) can be successfully converted to a more restricted type (ie: a float).

2) Applying business logic like restricting ranges, etc.

What we should do is the following:

a) Have two kinds of validators: type conversion validators and business rule validators.  Type conversion validators live on the "edges" of the data flow graph.  Business rule validators live on the "nodes" of the data flow graph.

b) Type conversion validators are applied first, then the converter is applied, then finally the business rule validator is applied if it is found.

c) The business rule validator always operates in the model's native data type.
Comment 2 Dave Orme CLA 2005-12-23 11:20:37 EST
From eclipse.tools.ve:

> 1- Was the Property object ever considered to own converters and
> validators? The Property object seems like _metadata_ for the actual
> domain-object values and a natural place for converters/validators.

Hmmmm....  Validators-- a qualified yes.  Converters, no.

Suppose you have two "things", A and B and you want to bind them together so that changes to A are "updated" in B and vice versa.

In this situation, A and B form a graph.  A and B are the nodes, and data binding makes up the edge (or arc or line) between A and B.  

But A and B may be of different data types, so they can't be bound unless there is a pair of data type conversion functions between them to convert A's type to B's type and vice versa.

We believe that the data type conversion functions live on the edge between A and B, and not on either A or B themselves.

Similarly, either A or B might be a more relaxed data type than the other.  For example, supposing A is a String and B is an int, A is a more relaxed data type than B.  In this situation, before we attempt to convert A's String value to an int for B, we have to validate that A's value *can* be converted to an integer to begin with.

Since this validation is associated with the data type conversion function, we believe that this validation function also lives on the edge between A and B.

That is why the call to bind() accepts 3 parameters:

bind(A, B, bindSpec)

bindSpec is the stuff associated with the edge in the data binding graph.

But you raise a valid point about business rule validation being mixed with the validation associated with data type conversion.  We have a bug for that: See bug #120582 where I argue that business rule validation needs to be associated with the business object.

In this context, your suggestion of putting this optional validator in the Property object is very interesting.  Thanks!
Comment 3 Christian Schaefer CLA 2006-01-13 06:20:27 EST
from our point of view, the chain of validation is something like that:

besides the keystroke validations on "leaving the field" the following should happen:
1. user input is auto-completed. a promienent example would be date. input "12" output nls specific but could be 12/01/2006. 
2.) type conversion: string to date
3.) one or more type specific validations: e.g. before date, is workday, is holiday, is bank-day, is-birthday ...
4.) cross field validation on the ui-model object

if the client is not a full rich client with the business logic layer on the client, the client side validation should avoid validations which requires data access and or business rule inclusion.

5.) the domain model within the business logic should re-validate rule of point 3 above, and some of 4 if the mapping from ui-model to domain model allows that.  to be able to use the business logic as a stand alone service without any gui.

6.) some validations even some of 3. above can only be done within the business-layer with data-access facilities. this for example "birthday".
7.) business rules require the br-engine. and will be done in the business layer. 

the ui-model for the mvc concept has a defined relationship to one or more objects within the domain model. in our case the domain model has defined a set of required validations for each attribute. within the domain model the basic datatyp is already given. due to the relation-ship from domain model to ui-model we are going to integrate some of the business validations already on the client but only simple validations without the requirement of data or business rules or something else which is server specific.

the client layer should be able to ask the model if it is fully valid, by re-triggering the full chain of validations over the model and all cross field validations. this means that the simple field validation should re-included in this ismodelvalid method except of course the simple sting to datatype conversion. 

generally a model can be sent to the server when all required fields are filled out. the model is fully valid, and there is no user input field marked as dirty. 



Comment 4 Dave Orme CLA 2006-01-18 14:20:00 EST
*** Bug 119504 has been marked as a duplicate of this bug. ***
Comment 5 Dave Orme CLA 2006-01-24 15:24:43 EST
A fix for bug 118323 landed today.  Basically, this lets clients observe and modify the data flow pipeline inside data binding at any step along the way.  This includes replacing values gotten from the UI with your own values, augmenting conversion or validation logic, and interrupting processing at any step along the way.

This can be used to implement custom validation support pretty much anywhere it makes sense to validate, to log what the data binding framework is doing for debugging purposes, etc.

If anyone finds a use-case not easily handled by the events defined so far, please add a comment on this bug or reopen bug 118323 and comment there.

Synopsis:

IDataBindingContext#bind now returns an IBinding.  Both IDataBindingContext and IBinding define addBindingEventListener and removeBindingEventListener.  See BindingEvent for the constants that define the places where you can listen for events.  The names are overloaded somewhat, so it's useful if you log these events in order to see what's going on here.

Comment 6 Dave Orme CLA 2006-01-24 15:33:38 EST
(In reply to comment #3)
> from our point of view, the chain of validation is something like that:
> 
> besides the keystroke validations on "leaving the field" the following should
> happen:
> 1. user input is auto-completed. a promienent example would be date. input "12"
> output nls specific but could be 12/01/2006. 
> 2.) type conversion: string to date

Our built-in converter/validator pairs don't do this, but it would be possible to create converter/validator pairs that do.

> 3.) one or more type specific validations: e.g. before date, is workday, is
> holiday, is bank-day, is-birthday ...

You can hook this up to the new business rule validation event.  We want to support this use-case directly though.  We just haven't decided on what the API should be.  Feedback here is welcome.

> 4.) cross field validation on the ui-model object

In our application, we are hooking this up to the after-change event.

> 5.) the domain model within the business logic should re-validate rule of point
> 3 above, and some of 4 if the mapping from ui-model to domain model allows
> that.  to be able to use the business logic as a stand alone service without
> any gui.

In our application, we only use data binding's service that converts/validates the UI's data type to the model's data type.  All other validation is kicked off in an after-change event.  We use separate validator objects that stand alone and can be called either from the UI or separately as needed.  We *don't* put validation in business object setters because then our validation logic would run whenever Hibernate reloads our business object--not usually what we want.

Comment 7 Wolfgang Herr CLA 2006-01-25 04:20:53 EST
The opportunity to listen for BindingEvents is very useful and gives us the chance to handle all the things i've described in bug #119791, so i think that my comment there is obsolete.

I have only one enhacement:
Class ValueBinding:
public void updateModelFromTarget() {
//...
  String validationError = doValidate(e.originalValue);
  if (validationError != null) {
    context.updatePartialValidationError(targetChangeListener, validationError);
    // fire a binding Event, when an ValidationError occured
    e.pipelinePosition = BindingEvent.PIPELINE_VALIDATION_FAILED;
    fireBindingEvent(e);
    // -----------------
    return;
  }
  //...
}

I know, that is implicit possible to know, that an validation-error occured (if the BindingListener will not reach the end of the chain), but it will be more convenient for the BindingListener to be explicit informed, that an validation error occured.
Comment 8 Wolfgang Herr CLA 2006-01-25 06:25:49 EST
I played with the latest version of IBinding and IDatabindingContext and got a problem with the Binding-class.

I registered a IBindingListener on a IBinding, but no Listener on the DatabingContext himself. My Listener returned a error-String but this error is swallowed because of 

result = context.fireBindingEvent(event);

at Line 66 of the Binding class (method fireBindingEvent).
Comment 9 Dave Orme CLA 2006-01-25 16:51:02 EST
(In reply to comment #8)
> I played with the latest version of IBinding and IDatabindingContext and got a
> problem with the Binding-class.
> 
> I registered a IBindingListener on a IBinding, but no Listener on the
> DatabingContext himself. My Listener returned a error-String but this error is
> swallowed because of 
> 
> result = context.fireBindingEvent(event);
> 
> at Line 66 of the Binding class (method fireBindingEvent).
> 


Fixed > HEAD.

Thanks!  Nice catch.
Comment 10 Dave Orme CLA 2006-02-06 18:19:40 EST
*** Bug 118570 has been marked as a duplicate of this bug. ***
Comment 11 Nicolas Gouy CLA 2006-02-07 13:09:12 EST
(In reply to comment #7)
> The opportunity to listen for BindingEvents is very useful and gives us the
> chance to handle all the things i've described in bug #119791, so i think that
> my comment there is obsolete.
> 
> I have only one enhacement:
> Class ValueBinding:
> public void updateModelFromTarget() {
> //...
>   String validationError = doValidate(e.originalValue);
>   if (validationError != null) {
>     context.updatePartialValidationError(targetChangeListener,
> validationError);
>     // fire a binding Event, when an ValidationError occured
>     e.pipelinePosition = BindingEvent.PIPELINE_VALIDATION_FAILED;
>     fireBindingEvent(e);
>     // -----------------
>     return;
>   }
>   //...
> }
> 
> I know, that is implicit possible to know, that an validation-error occured (if
> the BindingListener will not reach the end of the chain), but it will be more
> convenient for the BindingListener to be explicit informed, that an validation
> error occured.
> 

Hello,
I also need to be explicitly informed of the validation error. I registered my own ValueBinding class, because I don't see how to implement the workaround you exposed ("the BindingListener will not reach the end of the chain"). My need is to show a Jface error decoration when validation error occurs.

Thanks for any suggestions (the validation_error event is a good concept I think)
Comment 12 Dave Orme CLA 2006-02-07 22:29:59 EST
This is partially handled by BindingEvent.PIPELINE_AFTER_BUSINESS_VALIDATE

Proposal: Add a new validator type and field to IBindSpec.  This new validator will operate on the converted value and will be called immediately before the BindingEvent.PIPELINE_AFTER_BUSINESS_VALIDATE is fired.
Comment 13 Dave Orme CLA 2006-02-08 00:01:15 EST
Committed to HEAD:

Added IDomainValidator that is now an optional IBindSpec member.  It is called just before the converter value is set back into the model object.  

BeanBindSupportFactory can optionally grab these out of your business model objects themselves.

Still have to add the validation event that several have requested...
Comment 14 Dave Orme CLA 2006-02-08 00:03:39 EST
Validation event will be tracked on bug 118323.  Closing as fixed.
Comment 15 Boris Bokowski CLA 2006-02-16 12:38:29 EST
Marking as VERIFIED without actually verifying individually since a complete refactoring of the data binding framework will be released soon.