In thinking more about issue #251, I realized there were a few other transitory cases that weren't handled well. All of these issues were exemplified by bugs and edge cases in JSONPropertySupport, which is what has changed most here. This PR fixes the following:
AbstractModel.clear() now clears transitory values in addition to database and set values. This seems like the "correct" behavior, but I'd be open to adding something like clearModelValues() to clear only model values and NOT transitory values if we thought it would be valuable.
Several bugs have been fixed in JSONPropertySupport. Most notably, code has been added to validate that the cached parsed value still corresponds to the string contained in the model, so the cache validity can be automatically checked when using a JSON getter. This is useful because even with clearing the transitory cache when calling clear(), it's not so feasible to invalidate that cache if using things like the generic set(Property<?>) or writing directly to the model's ValuesStorage objects.
If no value is present in the model for the JSONProperty being read, an exception will now be thrown as it is for all other property types.
Documentation has been added to clarify that the readPropertyFrom<source> methods also clear transitory values.
New test cases have been added to verify the above fixes.
@jdkoren if you have any thoughts or think I've missed any other cases with transitory handling that we ought to consider, your feedback would be very helpful.
jdkoren
commented
7 years ago
In thinking more about issue #251, I realized there were a few other transitory cases that weren't handled well. All of these issues were exemplified by bugs and edge cases in
JSONPropertySupport, which is what has changed most here. This PR fixes the following:AbstractModel.clear()now clears transitory values in addition to database and set values. This seems like the "correct" behavior, but I'd be open to adding something likeclearModelValues()to clear only model values and NOT transitory values if we thought it would be valuable.JSONPropertySupport. Most notably, code has been added to validate that the cached parsed value still corresponds to the string contained in the model, so the cache validity can be automatically checked when using a JSON getter. This is useful because even with clearing the transitory cache when callingclear(), it's not so feasible to invalidate that cache if using things like the genericset(Property<?>)or writing directly to the model's ValuesStorage objects.readPropertyFrom<source>methods also clear transitory values.@jdkoren if you have any thoughts or think I've missed any other cases with transitory handling that we ought to consider, your feedback would be very helpful.