Bugzilla – Full Text Bug Listing
| Summary: | Provide support for alternate validator implementations | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Common Tools | Reporter: | Valentin Baciu <valentinbaciu> | ||||||||||||||||||||
| Component: | wst.validation | Assignee: | Valentin Baciu <valentinbaciu> | ||||||||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||
| Priority: | P3 | CC: | csalter, david_williams, thatnitind, vbhadrir | ||||||||||||||||||||
| Version: | 1.5 | ||||||||||||||||||||||
| Target Milestone: | --- | ||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||
| OS: | Windows XP | ||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||||
| Bug Blocks: | 132230, 132231 | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Valentin Baciu
Created attachment 36537 [details]
Initial contribution
Here's the proposed initial contribution to the validation framework.
Created attachment 36538 [details]
New un-checked disabled icon
Created attachment 36539 [details]
New checked disabled icon
Created attachment 36540 [details]
The new settings icon
Created attachment 36541 [details]
The new settings disabled icon
Created attachment 36631 [details]
New icon from Janet
Created attachment 36632 [details]
New settings disable icon from Janet
Valentin, Chuck is the owner of the wst.validation <wst.validation-inbox@eclipse.org> inbox and only he has the authority to assign it. I have added myself on the CC list to get notfications from you Few changes i wanted to higlight after appling the patches 1. The settings column should only be added to the table if there exists a validator having a delegate validator with addtional preferences. 2. A cleaner appraoch would for the validator extension having some knowledge of the delagate validator i.e one exists for him like an additional optional boolean element in the validator ext pt. stating a delagate validator exists for it with default being false if no element is defined in the implementing extension. 3. Need to guard against existance of an extension to validatorDelegate. Created attachment 36671 [details]
Improved version of the original contribution
Thanks for trying it out and for your comments Vijay. I will discuss your suggestions with Craig.
In the mean time, in order to move forward, this patch is an improved version of the original contribution. I have improved error handling and tested with and witouth a delegating validator and with no/one/two delegates.
To avoid the need to instantiate a validator to find out if it is a delegating validator I implemented a method on the ValidatorMetaData which queries the delegates registry. In the future, this method could simply test the new flag on the validator extension as you proposed.
Please apply to your workspace and let me know how it goes.
Valentin, The new patch looks good..i have committed your changes..thanks Can I ask for clarification on this change? I've tried running what's in head, and see a new "settings" colomn in preferences ... is that related? (I can't see to do anything with it, though). My main concern is the statement about "same file type", and "contribute alternate implementations for a given validator type". So, what's a validtator type? I think it is quite ligitamate to have more than one validator per file type, and I think your requirement is to actual override a specific validtor? Not necessarily anything to do with "per type"? David, these changes are intended to simplify the user experience when dealing with multipe variations of the same 'kind' of validaton. I'll try to explain... Let's take XML Schema as an example. Some adopters want to provide their own XML Schema validator but don't want to 'take away' the Xerces based validator that they get for free with WTP. In addition adopters would like to provide 'advanced' setting for specific validators to exploit customizable features (Xerces has a whole bunch of these goodies). So without this feature adopters could provide additional XML Schema validators (MSXML, XSV, etc). But this surely would confuse 'novice' users since they'd see multiple XML Schema validators showing up in the top level list. With this feature we're able to provide a single 'placeholder' XML Schema validator that shows up in the visible list of validators. Then this 'placeholder' delegates to one of several possible validator impls (Xerces, MSXML, XSV, etc.) The user can select a delegate validator (and specify other options that are specific to this delegate) in the validator preferences view. Hopefully that explains things a bit. I agree that if you're looking at this from the pure WTP UI today this wouldn't make much sense. Once we add additional validation settings for Xerces it'll make more sense in the pure WTP enviroment. Of course in adopter products there'll be more than one available validator implementation in the drop down list so the drop down will make more sense in adopter products too. Craig, part of what I'm asking is how is "kind" defined? For example, I know there are mulitple JSP validtors, each doing some specific thing, so ... what is the difference between that, and what you are talking about? I assume both "replace" and "add to" models will be allowed? Additional question, is the reason you are adding this API because there is some true value add seen by adopters? Why not put all of them in WTP? (And, its fine if seen as value add ... I'm just wondering.) Or, is this more of a nice user feature, but its felt there isn't time to put in WTP 1.5, so this allows them to be put in later? Also, have you figured out how to tie this in to source page validation. That seems like a requirement, that I think your team should do, since you are introducing this change, especially this late. (That is, my team can not contain it). BTW, I would hope the same effort going into this, can be applied to gettng "content type" working in validation framework, since that seems more important to me. Are you also doing that? Maybe I'm being obtuse, but I'm not sure I see the value of putting this into the validation framework and APIs directly as opposed to it being an ability specific to one validator extension. The only confusion I can think of being alleviated is that there won't be 3 different validator extensions identifying themselves by the same name, but then I didn't expect that to happen if adopters played nicely anyway. To boil it down, is there a foreseen and requested case where a user and adopter want to disable one validator for a "type" (ignoring the situation where a validator itself may be generating false errors)? If I were seriously editing schemas, wouldn't I want feedback from Xerces, MSXML, *and* XSV, all of the time? Do we have a second use case that makes it more obvious that this should be at the framework level? To answer David's questions > I assume both "replace" and "add to" models will be allowed? The delegate mechanism is intended to support different versions of validators that should be run in a mutually exclusive fashion. If a validator needs to be contributed in an "add" manner the existing validator framework has always support this. Now if your asking to provide a pattern where delegates are additive, then that's an feature we haven't heard requests for from any adopters but when we do we'll be happy to consider. > Additional question, is the reason you are adding this API because there is > some true value add seen by adopters? Why not put all of them in WTP? Several adopters have expressed an interest in this function including IBM, BEA and others. I should emphasize that we're not adding or changing 'API' these are internal changes to the validation framework plumbing. > Also, have you figured out how to tie this in to source page validation... Yes we have this all figured out and my team will be taking care of this. We'll be contributing a patch soon so that you can look it over. All in all it's a very minimal change to the source editors. In generally validation framework clients are completely unaffected by this change. It's a fairly minor and isolated change. >BTW, I would hope the same effort going into this, can be applied to gettng >"content type" working in validation framework, since that seems more >important to me. Are you also doing that? Bug 86385 is important to me too. My team is making this particular change since it's been explicitly requested by our favourite adopter and is deemed high priority. Once we finish this work I'll be more than happy to work with the validation framework team to address bug 86385. David, will your team be willing to lend a hand on the bug 86385 effort? Created attachment 37456 [details]
DelegatingValidator patch
Modified to implement IValidatorJob instead of IValidator.
The recent patch for Delegating Validator has been released to the 1.5 stream I believe we've completed this work now. Verified. Closing now. |