Community
Participate
Working Groups
In order to use data binding in Workbench or IDE dialogs and preference pages, we need validation in terms of IStatus objects. Using strings is too simplistic. For example, look at the wizard for creating a new class. You will get a warning if the class name does not start with an uppercase letter, or if you don't provide a package name, but you will still be able to click "Finish" to proceed anyway. (Note that IStatus is currently not on the classpath, we need to re-introduce the dependency on org.eclipse.equinox.common, which contains IStatus etc.)
I agree that there is a need to separate fatal validation errors from warning messages. However, I'm not sure that going all the way to IStatus is the answer either. See bug #118570 for one reason.
IStatus is in equinox and is better suited for validation semantics. It is also the de-facto standard in Eclipse for status.
(In reply to comment #2) > IStatus is in equinox and is better suited for validation semantics. It is > also the de-facto standard in Eclipse for status. Good points. But this doesn't answer the concern I raised in bug #118570. In other words, this code might need to run inside a web server, where people don't *want* to drag in Eclipse dependencies.
(And I should add that my current client raised just this exact objection yesterday to our *current* IValidator.)
So, we are in agreement that IStatus like is a better approach than Strings. The question is do we need to support portions of the binding framework (and which ones) to run standalone (without "anything").
Something like IStatus is better than just Strings, agreed. I think that we probably don't want IStatus itself because: 1) It drags in an Eclipse dependency that my client doesn't want. 2) I've only seen a use-case for distinguishing between error messages versus warning messages, and IStatus is much more than that.
I do agree with Dave that the dependency to IStatus may not be the ultimate solution, and would prefere not to have this dependency/solution. but we would also like to think about a more extended error information. could the return of an "object" be a solution ? from my point of view an error may contain more than a string and may be different in different usage szenarios: 1. The error message itself. this is NLS specific or if the validator does not know about the current language a resource id with possible mix in values. 2. a (global) error id 3. some details about the error happend (e.g. stack trace, detailed message, etc.) 4. warning or error could be important. 5. image id to use for display. 6. some information for further informations about such an error -> cheat sheet, help-id, support information etc. warning or errors or information: warnings provided by validators are possible within our use-cases. some example could be the interest rate. it could be warned that the rate should be between 2 and 3 % but errors would be thrown if the value is not within the range of >=1 and <=5 a warning therefore may be required to be stored into the model (target) even if the validator has marked the warning. in additon to that, the user may be asked to vote about this warning should be ignored. etc. overall i think, that there are good reasons to reconsider the implmentation about with a string result and about this beeing != null is the indication about failed validation.
As discussed per mail: The most important feature of a validation for us is, to distinguish between - succesful - warning - error Now, that the result of IValidator.isValid is only the stringed error-message, we have no chance to do this difference (unless we won't parse the errormessage :-)). Our suggestion is, to pack the error in a defined Interface/Exception (see below). Another questions is, ho to inform the caller about an error occured: 1.) public interface IValidator { public IValidationError isPartiallyValid(Object value); public IValidationError isValid(Object value); } In this case it has to be defined, whats the indicator of "successful" - a null returned or IValidationError.getErrorType()==IValidationError.SUCCESSFUL 2.) public interface IValidator { public void isPartiallyValid(Object value) throws ValidationErrorException; public void isValid(Object value) throws ValidationErrorException; } We would prefer the second alternative, because of the possibility of the Exception-chain and there is no nebulosity about the succesful validation. The IValidationError/ValidationErrorException have to provide at least: int getErrorType() // (SUCCESSFUL)/WARNING/ERROR String getErrorMessage() ...probably some further Information The above feature of course impacts a modification of the Update-Pipeline: If an warning occurs, the transformation between model<=>target should be done, but the warning should be stored.
This fix is in CVS now in a branch, but moving to M6 since we are waiting until after M5 to merge with HEAD.
The name ValidationError implies an error condition. Wouldn't it be better named something like ValidationResult or ValidationStatus? In that case, it might also be preferable to have an int value for success (maybe 0?), though, instead of returning null. Also, what about other severity levels besides WARNING and ERROR? If there is already requirement for these two different levels, probably there will arise a need for the INFO level. Also, this is really picking on details, but I find somewhat misleading the method names isValid() and isPartiallyValid() when the return types are not boolean. Other validation frameworks I've seen/used ( http://jakarta.apache.org/commons/validator/api-release/ , http://myfaces.apache.org/api/apidocs/index.html , http://struts.apache.org/1.2.9/api/index.html ) would probably use validate() and validatePartial() in the case when the return type is an object not boolean. PS. These are just personal opinion and I'm definitely not an expert on validation.
As I have now begun using Validators and Converters more, I definitely think that an INFORMATION level would be useful. In that case I can more fully bind the (partial) validation error to a form's or wizard's message, which supports message types of ERROR, WARNING, INFORMATION and NONE (no icon). It would be useful in some cases to give hints while typing and I would like to do this with the INFORMATION icon instead of WARNING.
(In reply to comment #11) > As I have now begun using Validators and Converters more, I definitely think > that an INFORMATION level would be useful. In that case I can more fully bind > the (partial) validation error to a form's or wizard's message, which supports > message types of ERROR, WARNING, INFORMATION and NONE (no icon). It would be > useful in some cases to give hints while typing and I would like to do this > with the INFORMATION icon instead of WARNING. > Oh... I just realized neither WARNING or INFORMATION would work with a partial validation error because it only allows typing to countinue if there is a null error.
(In reply to comment #12) > Oh... I just realized neither WARNING or INFORMATION would work with a partial > validation error because it only allows typing to countinue if there is a null > error. Good point! Looking at the feedback to our current API in this bug, it seems that the methods should be called validate() and partialValidate(), and return something that looks very much like IStatus with its OK/INFO/WARNING/ERROR levels. The partial validation status should only prevent users from typing if the status is an error. Personally, I would just use IStatus from o.e.equinox.common because it would be one less thing that we invent (and client need to learn). I don't understand why using o.e.equinox.common is a bad thing? It is a small JAR, and can be used without OSGi running.
Personally, I would also use IStatus. In that case, I think both IStatus.CANCEL and IStatus.ERROR returned from partialValidate() should prevent typing. You never know -- someone might find a use case for CANCEL :)
Dave Beers, Dave Orme, and Boris Bokowski had a quick phone conversation about this issue and came to the following observations/decisions: 1) We currently have two semantics that we are managing using the ValidationError object. (a) User data entry errors that occur during normal function of the application and that the application anticipates could occur, and (b) Exceptions that we catch that represent malfunctions in the application itself and need to be logged. These situations are not disjoint sets, however. For example, a programmer may choose to ignore the validators and instead cause model object to reject values directly by throwing IllegalArgumentExceptions. Or a programmer may inherit a model that uses this mechanism even though it's not how we intended for things to work. We need to handle this kind of situation as gracefully as possible. Our solution is as follows: 1) We will switch to using IStatus instead of ValidationError as recommended by several commentors and rename the get methods as suggested to make more sense. 2) When an exception is caught inside data binding, we will no longer convert it directly into a ValidationError (or an IStatus). Instead, we will introduce a new kind of BindingEvent. Clients can trap this BindingEvent and handle the exception appropriately there. Since BindingEvents can return validation errors, the client's BindingEvent handler could convert the exception into a validation error (IStatus object). Or the client's BindingEvent handler could choose to log the exception and throw up an error dialog. 3) The default behavior if nobody is listening to binding events when exceptions are thrown will be to do nothing, unless someone can suggest a reasonable default behavior and a reasonable semantic for how to implement it. :-)
Created attachment 50904 [details] Patch: exceptions fire binding events
Created attachment 54928 [details] almost there
Released >20061205.
