mgholam / fastJSON

Smallest, fastest polymorphic JSON serializer
https://www.codeproject.com/Articles/159450/fastJSON-Smallest-Fastest-Polymorphic-JSON-Seriali
MIT License
479 stars 147 forks source link

Readonly fields does not honour ShowReadOnlyProperties #104

Closed xmedeko closed 5 years ago

xmedeko commented 5 years ago

I have set ShowReadOnlyProperties = false, but the readonly fields are serialized. I have public static readonly MyConstantClass MyConstant filed and this field is serialized, too.

Please, do not serialize readonly field and do not serialize static fields/properties/methods. Thanks

mgholam commented 5 years ago

ShowReadOnlyProperties = false ?

xmedeko commented 5 years ago

Yes, ShowReadOnlyProperties = false. I've checked the Reflection.GetGetters and the ShowReadOnlyProperties param is not used for fields.

mgholam commented 5 years ago

Thanks, will fix.

mgholam commented 5 years ago

Try v2.2.5

xmedeko commented 5 years ago

Works well, thanks for the quick fix.

xmedeko commented 5 years ago

@mgholam Serialization is OK. But deserialization still deserialize read-only fields if they are in JSON. I think the chage should be in Reflection.CreateSetField(..), probably something like:

if (fieldInfo.IsInitOnly)
    return null;
mgholam commented 5 years ago

Hmm... wouldn't you want to have it set if it is in the json string (and have the serializer controls this)?

xmedeko commented 5 years ago

Well, if you serialize/deserialize same class, then it's like you write. But consider these scenarios:

  1. JSON is from external resource, you want to read/deserialize selected properties only. And, by a coincidence, some JSON property has the same name as your read-only field.
  2. You do changes in your code, mark some filed read-only and do not want to serialize/deserialize it any more. But your new app version has to read the old save data, so your read-only filed is rewritten.

Also, check that Reflection.CreateSetMethod(..) returns null for read-only properties. IMO Reflection.CreateSetField(..) should behave consistently.

mgholam commented 5 years ago

Check the latest commit.

xmedeko commented 5 years ago

But no ShowReadOnlyProperties is involved in the condition. Probably should be:

if (ShowReadOnlyProperties || f.IsInitOnly == false)
    d.setter = Reflection.CreateSetField(type, f);
mgholam commented 5 years ago

For that, it will have to happen higher up the stack, since the setters are being cached (like the serializer).