| Hello,
Johann and I were having a conversation here:
about the behaviour of ERXGenericRecord.validateValueForKey(Object value, String key). The short version is that Johann noticed a bug in the way _validateValueForKey() was being called, fixed it in 6665a308, and then fixed it some more in 92d51ff3.
The longer version is that what’s going on in ERXGenericRecord.validateValueForKey() in the try block is:
1. super.validateValueForKey(value, key) is called, and is assigned to the ‘result' local variable. 2. ERXEntityClassDescription.validateObjectWithUserInfo(this, value, "validateForKey." + key, key) is called. Note that the argument there is ‘value’. 3. _validateValueForKey(result, key) is called and assigned to ‘result’.
There are a couple of things to discuss here, and I’ll just paste in the part of the method we’re talking about.
try { result = super.validateValueForKey(value, key); EOClassDescription cd = classDescription(); if (cd instanceof ERXEntityClassDescription) { ((ERXEntityClassDescription) cd).validateObjectWithUserInfo(this, value, "validateForKey." + key, key); } result = _validateValueForKey(result, key); }
1. There’s a kind of “cascade” of validation going on, with each successive validation call potentially modifying ‘result’ further. Ignoring step 2 above for a moment, each of super.validateValueForKey() and then any partial entity’s implementation of _validateValueForKey() will (potentially) successively modify result. While this might sound odd, it’s only an issue if two different classes implement the same validateFoo() method for some key ‘foo’, which presumably wouldn’t normally happen. Partial entities, for example, should probably by convention only be implementing validation methods for keys they add to the entity. In any case, the alternative is “last modification wins”, which is the bug Johann was correcting—_validateValueForKey() was previously modifying the original ‘value’ and being allowed to return that, hence trumping any other modifications. I just wondered whether it was worth sanity checking the idea here first, because Johann has pointed out that we probably need to modify _validateValueForKey(Object, String) to do the same thing, as currently that method is “last modification wins”:
@Override protected Object _validateValueForKey(Object value, String key) throws ValidationException { Object result = value; for (ERXPartial partial : _partials()) { result = partial.validateValueForKey(value, key); } return result; }
Surely that should be result = partial.validateValueForKey(result, key), or else a modification by partial P1 gets wiped out by a partial P2 further down the list that doesn’t want to do anything with that key (and hence returns ‘value’ unmodified). Does anyone disagree?
2. I don’t know what ERXEntityClassDescription.validateObjectWithUserInfo() does, but currently (see above) it’s also being passed the original ‘value’. Should this method also be passed ‘result’ at this point?
|