Community
Participate
Working Groups
For Bindings, the validation status observable should be stale whenever either the target or model observable is stale. For the MultiValidator, the validation status observable should be stale whenever any of its dependencies accessed from within the MultiValidator#validate() is stale.
Created attachment 107270 [details] proposed patch along with some unit tests for the MultiValidator part This is a patch for the MultiValidator part only.
+1. Good patch, good tests. Ovidio, we have a set of classes in our testing plugins: ChangeEventTracker, ValueChangeEventTracker, SetChangeEventTracker, etc. They share the same testing purpose as the StaleCounter and ValueChangeCounter classes from your patch so I thought I would bring them to your attention. Boris, if you have no objections I think we could commit this patch as is.
> Ovidio, we have a set of classes in our testing plugins: ChangeEventTracker, > ValueChangeEventTracker, SetChangeEventTracker, etc. They share the same > testing purpose as the StaleCounter and ValueChangeCounter classes from your > patch so I thought I would bring them to your attention. Thanks for the hint, Matthew. I hadn't seen those classes but I will happily use them in future :). I will also adapt this patch to use them as it is definitely cleaner.
Boris, Ovidio's patch can supersede the changes I made to MultiValidator as part of bug 240590. So don't worry about clobbering my changes as long as the tests still pass.
Created attachment 107357 [details] proposed patch along with some unit tests It's the same patch as the original one which, however, uses the ValueChangeEventTracker class as suggested by Matthew.
I've just found a small bug in my patch for the MultiValidator part when the set of dependencies changes and the new set contains a stale observable while the old set didn't. In that case, a stale event should be fired which was not the case. Sorry for that. New patch to follow.
Created attachment 108485 [details] new patch with the just mentioned fix In the doSetValue(...) method of the ValidationStatusObservableValue it now reads if ((oldStale && !stale) || !Util.equals(oldValue, value)) { fireValueChange(Diffs.createValueDiff(oldValue, value)); } else if (!oldStale && stale) { fireStale(); } where the second if-clause is the new one. I've included an additional unit test which covers that use case. Beside the fix and the new unit test, the patch is the same (but I have already merged it against Matthew's changes as of bug #240590).
I have filed a separate bug (bug #242495) for the Binding part in order to cleanly track the changes to the MultiValidator part only as part of this bug here.
Patch released to HEAD. Thanks!
Matthew and Boris, thanks at this point for taking the time to look at my patches!
Verified by checking the test results for I20080915-1800.
