Bug 197807 - [DataBinding] Common Constraint Validator with Dependency Support
Summary: [DataBinding] Common Constraint Validator with Dependency Support
Status: CLOSED DUPLICATE of bug 183055
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-25 10:36 EDT by Matt Carter CLA
Modified: 2009-09-03 09:09 EDT (History)
5 users (show)

See Also:


Attachments
Inter-Observable Validation Dependency Support with Snippet (81.06 KB, patch)
2007-11-13 11:57 EST, Matt Carter CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Carter CLA 2007-07-25 10:36:35 EDT
I have created a new validator called ConstraintValidator which I have found to be generically useful.

I would like to discuss with others whether this sort of validator would be appropriate for inclusion in the API.. perhaps under a 'utils' package.

The proposed validator is attached as an afterConvertValidator or after, and performs one or more common validation tasks on the model including:

- Required Field
- Numeric Range / Min / Max restrictions    (e.g. between 1 and 99)
- Limited set of value options   (e.g. "A", "B" or "C", such as a String database field which is only valid if set to one of certain values)
etc.

These are the sort of types of common constraint found in field metadata, and in APIs such as Hibernate Validation (@Range(min=N,max=N,message="Bad value"), @Required, etc.).

With this validator I can efficiently cover several common model validation checks in one (CompositeValidator style!) and have a complete/unified validation status report at the presentation layer.

It also comes with a pair of 'DescriptionToValue' converters which convert backend value options such as status codes in the database which are *not* stored relationally (i.e. lookup table ID), into human-readable descriptions of those codes suitable for binding into a combo box.
e.g.

N<->New Record
V<->Validated
S<->Submitted
C<->Cancelled
F<->Finished

I will post more on this topic when I have added more functionality to the ConstraintValidator.
Comment 1 Brad Reynolds CLA 2007-07-28 19:19:59 EDT
Matt, can you post some code so that we can discuss it?
Comment 2 Peter Centgraf CLA 2007-08-21 13:33:45 EDT
I'd also like to see this code.  I've already implemented my own range and required validators, and I'd like to see how you composite them.  (My classes depend on custom Java 5 annotations, so I didn't consider them appropriate for contribution.)
Comment 3 Matt Carter CLA 2007-08-21 13:36:51 EDT
(In reply to comment #2)
> I'd also like to see this code.  I've already implemented my own range and
> required validators, and I'd like to see how you composite them.  (My classes
> depend on custom Java 5 annotations, so I didn't consider them appropriate for
> contribution.) 

So does my code Peter, but I've decoupled the annotation parser (form generator) from the ConstraintValidator, it takes the underlying constraint metadata as arguments now to make it suitable for contribution, instead of just taking in a bunch of annotations directly.
Comment 4 Matt Carter CLA 2007-08-21 13:39:15 EDT
The ConstraintValidator stuff is all working (and supports things like mutually dependent constraints; between target observables, too).

I'm waiting for the org.eclipse.core.databinding.extended package to be agreed and created, as that is where it will go if we accept it.
Comment 5 Boris Bokowski CLA 2007-08-21 14:53:26 EDT
I was hoping that the main part of this could go into org.eclipse.core.databinding. Is there a way to separate out the Java 5 specific functionality?
Comment 6 Matt Carter CLA 2007-08-22 06:17:51 EDT
(In reply to comment #5)
> I was hoping that the main part of this could go into
> org.eclipse.core.databinding. Is there a way to separate out the Java 5
> specific functionality?

There is no Java 5 functionality.. It could go into core. 

I was unclear of the scope we have decided for extended. "Java 5 only" it seems to be now. If it's going into core I don't have to wait for extended. I will post the code for discussion.
Comment 7 Matthew Hall CLA 2007-10-31 16:23:41 EDT
Matt, if you could post a patch I would also be interested in your constraint validators.
Comment 8 Matt Carter CLA 2007-11-08 06:03:16 EST
Have some time off soon. Will post code.
Comment 9 Matt Carter CLA 2007-11-13 11:57:08 EST
Created attachment 82783 [details]
Inter-Observable Validation Dependency Support with Snippet
Comment 10 Matt Carter CLA 2007-11-13 12:17:10 EST
OK here is a patch which implements a series of mutually dependent validation constraints.

Points of interest are:

New package:  
  
  org.eclipse.databinding.constraints.*

New snippet in org.eclipse.jface.examples.databinding.snippets:

  Snippet016DependentConstraints.java

The main classes are:
  ConstraintValidator and ConstrainedObservableGroup.

The most alien concept will be 'named observables'. This allows references to be established between observables which may not have been created yet, and is useful when building validation status messages containing human-readable observable names).


I already want to further refactor some parts of this (have already done so extensively). I think a better design would be:

- Create a CompositeValidator class   (generically useful)

- Have builder methods add*Validator(), e.g. addAfterConvertValidator() on the new binding builder to allow multiple validators to be added to each validation point.

  (This would call setAfterConvertValidator() when the first validator 
  is applied, and when a second validator applies it wraps 
  the two validators in a CompositeValidator. When any further validators 
  are added, they are added to the CompositeValidator).

- Split ConstraintValidator into separate validators. One for a RequiredField constraint, one for a NotAllowedIf constraint, and so on.

- Look at ConstraintBuildingContext with a view to integrating this into a main builder.

- Look again at the concept of named observables which has been used. 


There is some additional state that needs to be kept in order to do dependent validation between observables, beyond what is currently possible with existing objects. This seems to make dependent validation functionality dependent these new things:

- A stateful constraint binding builder of some kind
- Observable Group concept (not tied to DBCs)

It also creates frustration at the following property of JDB as it stands:
- Validators cannot be modified or added after binding has occurred.


Regards
Matt
Comment 11 Matt Carter CLA 2007-11-13 12:27:41 EST
Current limitations:
- Reports only the first constraint's validation error. No MultiStatus yet.
- Only supports ObservableValue, not ObservableList (not sure how this would work yet, nor have I had a need for it).

There is also a bug workaround in the snippet for the "Initial Validation Status not correct for one observable until revalidated" problem. If removed the issue can be observed via this snippet.
Comment 12 Chris Aniszczyk CLA 2008-06-12 18:18:04 EDT
Something like this would be useful.

Also, something like a CompositeValidator would be nice
Comment 13 Boris Bokowski CLA 2008-06-12 18:33:58 EDT
We have a class called MultiValidator, does this help you Chris?
Comment 14 Chris Aniszczyk CLA 2008-06-12 18:40:38 EDT
I'd envision something like this Boris.

Imagine having a pool of pre-built validators:

FloatValidator
NonEmptyStringValidator
PhoneNumberValidator

You than want to combine these in some fasion... possibly AND them... maybe OR them... right now it seems you can only set one validator at a time depending when you want it ran.

Does this make sense a bit Boris?

I don't see how MultiValidator fulfills that case.
Comment 15 Matthew Hall CLA 2008-06-12 18:43:30 EDT
Something like this then?

dbc.bindValue(target, model, null,
  new UpdateValueStrategy().setAfterGetValidator(
    new AndValidator(
      new NonEmptyStringValidator(),
      new DoubleValidator(),
      new MinimumNumberValidator(5),
      new MaximumNumberValidator(10) ) );
Comment 16 Chris Aniszczyk CLA 2008-06-12 18:44:21 EDT
(In reply to comment #15)
> Something like this then?
> 
> dbc.bindValue(target, model, null,
>   new UpdateValueStrategy().setAfterGetValidator(
>     new AndValidator(
>       new NonEmptyStringValidator(),
>       new DoubleValidator(),
>       new MinimumNumberValidator(5),
>       new MaximumNumberValidator(10) ) );
> 

ya, that hits the spot Matt ;)
Comment 17 Matthew Hall CLA 2008-06-12 18:44:56 EDT
Oops, my arguments were in the wrong order but you get the picture.
Comment 18 Matthew Hall CLA 2009-09-01 17:10:36 EDT
(In reply to comment #17)
> Oops, my arguments were in the wrong order but you get the picture.

I have no idea what I was talking about--probably posting to the wrong bug again :-o

There is ongoing work in bug 183055 that overlaps several things discussed in this bug.
Comment 19 Matt Carter CLA 2009-09-02 04:56:52 EDT
Additional comment to pass on the remaining missing information... :)

This patch was not intended to be a well architected implementation... I had to implement inter-dependent contraint validation for a project, and quickly, so I did, and I posted the work to the community for discussion.

(In reply to comment ~ #14)

The reason I eventually grouped common validators together into one object was performance. I first implemented the strategy of using many small validators attached to each field (e.g. similar to AndValidator, OrValidator, NotNullValidator, RangeValidator etc. discussed here). Once I had applied inter-observable constraints, in production use I found that the code was having to create, configure and associate 20+ objects for every single field, and on large forms this was a high number of objects and method calls resulting in slow construction and validation. So, I banded the most common constraints into one object, after which the forms opened and functioned at a more acceptable speed (much the same speed they would normally under Data Binding).

It might be a good idea to bear performance in mind in the final design and/or implementation, as inter-relational dependencies seem to incur an exponential performance penalty particularly with Observable Groups  (at least thats what I experienced with my implementation).

I concluded that, in this case at least, it is best to avoid creating and associating hundreds of small objects and better to aggregate them in some way.
Comment 20 Matthew Hall CLA 2009-09-02 17:04:35 EDT
Matt,

With regard to cross-field validation, have you taken a look at MultiValidator?  If you haven't seen it yet, it handles cross-field validation concerns pretty well and without having to explicitly manage dependencies (as long as your dependencies are expressed as observables).  There are some snippets in the examples project that should give you a good idea of what's possible.

As for a constraints API, I think the work going on in bug 183055 is where we are headed.  Let's find out which features are in your patch but are lacking in bug 183055 and other existing APIs and proceed from that point.
Comment 21 Matt Carter CLA 2009-09-03 07:36:15 EDT
(In reply to comment #20)
Hi Matt,

Yes, lets close this bug. It's not the way forward.
(I'll leave you to select the closed resolution.)
Comment 22 Matthew Hall CLA 2009-09-03 09:09:40 EDT
Ok.  Marking as dupe

*** This bug has been marked as a duplicate of bug 183055 ***