TJvCustomAppStorage.WriteProperty does not check for explicit Stored declarations

[dummzeuch]

The code in TJvCustomAppStorage.WriteProperty looks like this:

begin
  if not Assigned(PersObj) then
    Exit;

  P := GetPropInfo(PersObj, PropName, tkAny);

  // If not storing the default values, then do not do anything if the property
  // is read only, write only or its "stored" function returns False.
  // Note: we do not add a call to IsDefaultPropertyValue here because it would
  // return True for any sub component which is not desirable as we want to
  // always store sub classes whether they are components or not.
  if not StorageOptions.StoreDefaultValues
    and (not Assigned(P.GetProc) or not Assigned(P.SetProc) or not IsStoredProp(PersObj, P))
    and (PropType(PersObj, PropName) <> tkClass) then // Classes can have data without having a stored property
    Exit;

This does not handle the case when a property has an explicit Stored modifier even though the other checks fail.

I propose the following additional check right after the above code:

  if not IsStoredProp(Persobj, P) then
    Exit;

[obones]

But that test is already in the parenthesis and the comment explain the situation quite well.
What situation are you trying to address here?

[dummzeuch]

The case that StorageOptions.StoreDefaultValus is True but the stored modifier returns False.
That first "and" prevents IsStoredProp from being evaluated.

I assumed that's a bug, because it ignores the stored modifier. Is it on purpose?

[obones]

Why would it be a bug that it's not evaluated?
StoreDefaultValues returns True so we want to store everything, regardless of what the class is telling us about default values. So, yes, it makes sense to take a shortcut.

If you want to respect what's defined by the class, set StoreDefaultValues to False

[dummzeuch]

This is a patch I made to my personal copy of the sources in 2018. There was a reason why I didn't like the behavior and why I couldn't set StoreDefaultValues to False, but unfortunately I don't remember it. I guess setting StoreDefaultValue to False had a side effect that I didn't like/expect and I assumed this to be a bug. I just now stumbled upon this change and thought I should finally submit it.

OK, if it is working as designed, it is obviously not a bug.