Summary: | [DataBinding] Common Constraint Validator with Dependency Support | ||||||
---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Matt Carter <matt> | ||||
Component: | UI | Assignee: | Boris Bokowski <bokowski> | ||||
Status: | CLOSED DUPLICATE | QA Contact: | |||||
Severity: | enhancement | ||||||
Priority: | P3 | CC: | bradleyjames, caniszczyk, mallo.ovidio, matt, qualidafial | ||||
Version: | 3.3 | ||||||
Target Milestone: | --- | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Matt Carter
2007-07-25 10:36:35 EDT
Matt, can you post some code so that we can discuss it? 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.) (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. 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. 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? (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. Matt, if you could post a patch I would also be interested in your constraint validators. Have some time off soon. Will post code. Created attachment 82783 [details]
Inter-Observable Validation Dependency Support with Snippet
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 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. Something like this would be useful. Also, something like a CompositeValidator would be nice We have a class called MultiValidator, does this help you Chris? 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. Something like this then? dbc.bindValue(target, model, null, new UpdateValueStrategy().setAfterGetValidator( new AndValidator( new NonEmptyStringValidator(), new DoubleValidator(), new MinimumNumberValidator(5), new MaximumNumberValidator(10) ) ); (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 ;) Oops, my arguments were in the wrong order but you get the picture. (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. 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. 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. (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.) Ok. Marking as dupe *** This bug has been marked as a duplicate of bug 183055 *** |