Bugzilla – Full Text Bug Listing
| Summary: | Need a way to add a constraint filter to any validator (live or batch) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF Services | Reporter: | Alanna Zito <azito> | ||||||||
| Component: | Validation | Assignee: | Christian Damus <give.a.damus> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | ahunter.eclipse | ||||||||
| Version: | unspecified | Keywords: | noteworthy, plan | ||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | Broader Community | ||||||||||
| Bug Depends on: | 252302 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Alanna Zito
There is already an extensibility mechanism that allows applications to select which constraints they would like to include in validation: the constraint-bindings extension point. Does your application define a client context for itself? Why don't you have access to the validator? The framework expects your application to own and manage the lifecycle of its validators. Perhaps you just need to devise a way to get access to it. I'm concerned about the possibility of filters contributed by one application "leaking" into other application contexts. Any solution implemented in the framework would be tied to the client-context mechanism, so you would need that, anyway (and I think it may already address your need). I had briefly looked at the constraint binding/client context extension, but it didn't seem to me that it would solve our particular problem. The issue is that another (lower-layer) plug-in is the one that contributes the constraints, defines the client context, and binds the constraints to the context. Our models also "match" this same context (which I think is reasonable, since the meta-model is the same; the only thing that distinguishes "our" models is a UML profile). So to be able to exclude certain constraints using client contexts, the lower-level plug-in would have to change the definition of their client context such that our models are not included in it; since it's not aware of our profile, that doesn't really seem possible. Does that make sense? I'm not that familiar client contexts, so perhaps there's something I missed. We don't have access to the validators because we're built on top of other components - so for example, the action that runs batch validation was not contributed by us, and there's currently no way for us to hook into it or change it. Same problem with live validation. Would it maybe be possible to have some kind of "constraintExclusions" extension point that mirrors the constraintBindings one? Instead of specifying which constraints should be evaluated in a certain client context, it would specify which constraints should be ignored. Then we could define a specialized client context in which to exclude the constraints we don't want. (In reply to comment #2) > I had briefly looked at the constraint binding/client context extension, but it > didn't seem to me that it would solve our particular problem. > > The issue is that another (lower-layer) plug-in is the one that contributes the > constraints, defines the client context, and binds the constraints to the > context. Our models also "match" this same context (which I think is > reasonable, since the meta-model is the same; the only thing that distinguishes > "our" models is a UML profile). So to be able to exclude certain constraints Ah, yes, but that was precisely the impetus behind the client-context: for applications that share a metamodel to be able to distinguish their resources based on some discernable characteristic. it's a wider scope than the content-type concept, but it would be nice to integrate with content-types, too. > using client contexts, the lower-level plug-in would have to change the > definition of their client context such that our models are not included in it; > since it's not aware of our profile, that doesn't really seem possible. Does > that make sense? I'm not that familiar client contexts, so perhaps there's > something I missed. Nope, you didn't miss anything. As an extender of this other component, the only recourse you have currently is not to use its client-context, but instead to define your own and re-bind all of the constraints that you need. However, there's no defence against that application's context being too general in its matching criteria and matching your models. > We don't have access to the validators because we're built on top of other > components - so for example, the action that runs batch validation was not > contributed by us, and there's currently no way for us to hook into it or > change it. Same problem with live validation. I suppose that you could ask this other component to provide access to the validators, or else simply create your own action. Perhaps that's not practical ... > Would it maybe be possible to have some kind of "constraintExclusions" > extension point that mirrors the constraintBindings one? Instead of specifying > which constraints should be evaluated in a certain client context, it would > specify which constraints should be ignored. Then we could define a > specialized client context in which to exclude the constraints we don't want. Yes, indeed, adding the dual of the inclusions mechanism into the constraintBindings extension point is something that we should do, anyway. Arguably, it should have been done at the outset. Currently, client-contexts don't have any specialization relationship. That, too, is an idea worth considering. It may even be necessary to effect a sensible precedence of inclusion and exclusion criteria. If you think that context specialization and exclusions will meet your need, this sounds to me like a priority for the Galileo plan. Can we count on you to test a patch when it is ready? (In reply to comment #3) > Nope, you didn't miss anything. As an extender of this other component, the > only recourse you have currently is not to use its client-context, but instead > to define your own and re-bind all of the constraints that you need. And having to re-bind all of the constraints wouldn't be ideal either, since we would have to react every time the other component added new constraints that we wanted, or changed constraint IDs. >However, > there's no defence against that application's context being too general in its > matching criteria and matching your models. > I suppose that you could ask this other component to provide access to the > validators, or else simply create your own action. Perhaps that's not > practical ... No, creating our own action is not really an option. We did think about asking for hooks into the validators, but that would involve asking a few different components for hooks into their validators, and we could still run into problems every time new components that do validation are added - we'd prefer a more "complete" solution, that would work no matter where/how validation is invoked. > > > Yes, indeed, adding the dual of the inclusions mechanism into the > constraintBindings extension point is something that we should do, anyway. > Arguably, it should have been done at the outset. > > Currently, client-contexts don't have any specialization relationship. That, > too, is an idea worth considering. It may even be necessary to effect a > sensible precedence of inclusion and exclusion criteria. > > If you think that context specialization and exclusions will meet your need, > this sounds to me like a priority for the Galileo plan. Can we count on you to > test a patch when it is ready? > Yes, I think what you're describing would solve our problem. I'd be happy to test a patch when it's done. Created attachment 114267 [details]
Proposed enhancement for constraint exclusions
Attached a patch that adds support for category and constraint exclusions in client-context bindings, in addition to inclusions.
New <excludeCategory> and <excludeConstraint> elements in the constraintBindings schema reference the categories and constraints, respectively, to exclude. Bindings are ordered: subsequent category/constraint references take precedence over any declared before. Thus, nested categories or constraints can be re-included after nesting categories are excluded, and tested categories or constraints can be re-excluded after nesting categories ore included (note that constraints don't nest).
The precedence of these bindings is well-defined within any one <constraintBinding> element. Ordering of bindings contributed by different extensions within the plug-in, or different plug-ins, is undefined.
Alanna, does this work for your application? I'm not convinced, yet, that defining relationships (specialization) between client contexts is necessary. It might ensure a partial ordering of bindings contributed by the base and specializing contexts, but applications that require such complexity probably need to re-think their approach. Perhaps your experience as a client can demonstrate otherwise.
> The precedence of these bindings is well-defined within any one > <constraintBinding> element. Ordering of bindings contributed by different > extensions within the plug-in, or different plug-ins, is undefined. Unfortunately, I think this is the key point that our application needs, since we want to be able to exclude a constraint that another plug-in has bound to the client context. This suggests to me that there needs to be a defined order in which their "includes" and our "excludes" are processed. > > Alanna, does this work for your application? I'm not convinced, yet, that > defining relationships (specialization) between client contexts is necessary. > It might ensure a partial ordering of bindings contributed by the base and > specializing contexts, but applications that require such complexity probably > need to re-think their approach. Perhaps your experience as a client can > demonstrate otherwise. > I don't know if explicit specialization of client contexts is necessarily required, but we do need a way to handle a constraint that is bound to one client context and then excluded in another (overlapping or subsetting) context. For example, suppose one plug-in contributes a constraint "constraint1" and binds it to "clientContext1", and my plug-in defines "clientContext2" and excludes "constraint1" from it. Then, if I have an eObject that matches both "clientContext1" and "clientContext2", I would like "constraint1" to be excluded. So perhaps a simpler solution than context specialization would be to give exclusions precedence over bindings? The algorithm for gathering relevant constraint would make one pass to gather all the constraints bound to matching client contexts, and then a second pass to subtract all excluded constraints from that set. Created attachment 114617 [details]
Updates to support client-context extension
Attached an updated patch that adds support for client-context extension. The <binding> element can now contain an <extendClientContext> element that references another context by ID, and includes its constraint bindings at that point in the <binding> filters. See the extension point schema definition for details.
This actually turned out to be very easy to implement, and quite clean, thanks to the new BindingFilter design.
The JUnit tests are updated to test this new construct.
I eagerly await your feed-back from testing this in your app.
(In reply to comment #7) > Created an attachment (id=114617) [details] > Updates to support client-context extension > > Attached an updated patch that adds support for client-context extension. The > <binding> element can now contain an <extendClientContext> element that > references another context by ID, and includes its constraint bindings at that > point in the <binding> filters. See the extension point schema definition for > details. I ran into some problems with this patch in our application. Here's what I did: I extended the constraintBindings extension point with: -A client context, which basically just selects elements with our UML profile applied -A binding on the client context to mark it as an extension of the other plug-in's context, and to exclude the constraint that I would like filtered out for element's matching my client context However, when I created a model and ran validation on it, I found that this constraint was still being evaluated for elements that matched my client context. I debugged it a bit, and it looks like the constraints that are included/excluded from "my" context are calculated correctly; the problem seems to happen during validation itself, in ClientContextManager#getBindings(Collection<? extends IClientContext>, Collection<? extends T>). This method adds a constraint to the result set as long as it is included by *any* of the matching client contexts (there's a break statement there which means that it doesn't even consider any of the other contexts). So although the constraint is correctly excluded from my client context, since it is included in the client context that I'm extending (since the element matches both contexts), it winds up being evaluated anyways. (In reply to comment #8) Thanks for testing this! And sorry about the boo-boo. I didn't have a test for the matching of contexts to elements in a validation operation. I'll attach an update. Created attachment 115192 [details] Update to filter extended contexts from matching Attached a new patch that updates the previous to address comment #8. The ClientContextManager now doesn't return, when finding the contexts matching an object, any contexts that are extended by other contexts that also matched. Of course, it would be odd for a client-context not to match when a context that extends it does (one would expect a subsetting relationship), but that's the client's concern. The new patch adds a JUnit test to verify that extended contexts are excluded when extending contexts are matched. (In reply to comment #10) Sorry it took me a few days to get back to you on this; I was having some problems with the patch, and was trying to figure out if they were mistakes on my end. Overall, it seems to work very well for live validation; the implementation & extension point are quite clean and easy-to-use (especially compared to the hack we're currently using to try and filter constraints :-) ) I'm still having a couple of issues though: 1) Things don't work quite as I would have expected during batch validation; not sure if it's really a bug though. During batch validation, it appears that the matching client contexts are only calculated once on the root eObject being validated (so for example, if I run validation on my whole model, it only gets client contexts that match the model - I assume this is for performance reasons?). Since my extending client context only matches certain elements within the model, and not the model itself, that means that it doesn't get considered. Would it be too expensive if the batch validator recomputed matching client contexts for each eObject encountered during validation? 2) Some constraints that I had provided myself are not being executed anymore. I've discovered that the problem is that my constraints aren't bound to any client context; the only reason they worked in the first place was that the client context I'm extending is a default context - introducing my extending context breaks some assumptions that I had unknowingly been making. So I think that I need to explicitly bind my constraint category to the base client context (not my extending context, because it's too narrow) so that they'll also be included in the extending one. But I get errors of the form "Client context <other plug-in's context id> does not exist in constraint binding defined by plug-in <my plug-in id>." when I try to do this. The documentation for the extension point seems to indicate that I should be able to bind constraints to a context that is defined by another plug-in - is this not the case? (In reply to comment #11) > (In reply to comment #10) > Sorry it took me a few days to get back to you on this; I was having some No problem ... I was on vacation last week, so there :-P > problems with the patch, and was trying to figure out if they were mistakes on > my end. Overall, it seems to work very well for live validation; the > implementation & extension point are quite clean and easy-to-use (especially > compared to the hack we're currently using to try and filter constraints :-) ) Heh heh ... thanks, that's good to hear. > I'm still having a couple of issues though: > > 1) Things don't work quite as I would have expected during batch validation; > not sure if it's really a bug though. During batch validation, it appears that > the matching client contexts are only calculated once on the root eObject being > validated (so for example, if I run validation on my whole model, it only gets > client contexts that match the model - I assume this is for performance Yes. The "traversal strategy" is responsible for determine when the traversal of a selection may need to recompute client-contexts. It's not a particularly well-placed responsibility (in general, how is a traversal to know?) but it was the best fit available to avoid the considerable overhead of matching context selectors, especially the XML expressions. The default recursive traversal makes the fairly reasonable assumption that the roots of the selection can possibly be in different contexts, but not their contents. > reasons?). Since my extending client context only matches certain elements > within the model, and not the model itself, that means that it doesn't get > considered. Would it be too expensive if the batch validator recomputed > matching client contexts for each eObject encountered during validation? Basically, yes. Can you let your client-context match any element in models with your profile applied? > 2) Some constraints that I had provided myself are not being executed anymore. > I've discovered that the problem is that my constraints aren't bound to any > client context; the only reason they worked in the first place was that the > client context I'm extending is a default context - introducing my extending > context breaks some assumptions that I had unknowingly been making. So I think > that I need to explicitly bind my constraint category to the base client > context (not my extending context, because it's too narrow) so that they'll Yes, that's what I would recommend. Using default contexts is discouraged; the default attribute only exists for compatibility with constraints developed before the client-context mechanism was introduced. > also be included in the extending one. But I get errors of the form "Client > context <other plug-in's context id> does not exist in constraint binding > defined by plug-in <my plug-in id>." when I try to do this. The documentation Oo. That's not good. > for the extension point seems to indicate that I should be able to bind > constraints to a context that is defined by another plug-in - is this not the > case? Definitely, you should be able to do this. It looks like a regression caused by the contribution of bug 137213. I have raised bug 252302 for that. (In reply to comment #12) > > Yes. The "traversal strategy" is responsible for determine when the traversal > of a selection may need to recompute client-contexts. It's not a particularly > well-placed responsibility (in general, how is a traversal to know?) but it was > the best fit available to avoid the considerable overhead of matching context > selectors, especially the XML expressions. The default recursive traversal > makes the fairly reasonable assumption that the roots of the selection can > possibly be in different contexts, but not their contents. OK, that makes sense. > > > reasons?). Since my extending client context only matches certain elements > > within the model, and not the model itself, that means that it doesn't get > > considered. Would it be too expensive if the batch validator recomputed > > matching client contexts for each eObject encountered during validation? > > Basically, yes. Can you let your client-context match any element in models > with your profile applied? > > Possibly - it's a bit of a complicated situation, because I want to exclude the constraint for only some of the element types that it applies to - but I can try to work something out. And if not, we would probably be able to live with having the constraint included in batch validation; having it excluded from live validation is more important to us. (In reply to comment #13) > > Possibly - it's a bit of a complicated situation, because I want to exclude the > constraint for only some of the element types that it applies to - but I can > try to work something out. And if not, we would probably be able to live with > having the constraint included in batch validation; having it excluded from > live validation is more important to us. Hmmm ... perhaps you could re-declare the constraint with different target metaclasses (and a different ID), if that is an option. Otherwise, perhaps a separate enhancement could look at making the client-context selection more flexible. (In reply to comment #14) > > Hmmm ... perhaps you could re-declare the constraint with different target > metaclasses (and a different ID), if that is an option. Otherwise, perhaps a > separate enhancement could look at making the client-context selection more > flexible. > yes, I think something along the lines of re-declaring the constraint would probably work in this case. Committed the patch in attachment #115192 [details].
Thanks, Alanna, for your contribution to this valuable enhancement!
Fix available in HEAD: 1.3.0.I200811021725. Restore original target after milestones were deranged. |