Zero integer values lost on serialization

[rouen-sk]

Hi, first thing first - great project, huge timesaver. I know it is intended only as decoder, but I am using it with success to do minor edits to vector tiles - deserialize, rename some layers, remove some layers, remove some features from layers, and serialize back. It generally works great, with one exception - attributes (properties) which are for example integer value zero, and are correctly deserialized as HasIntValue=true and IntValue=0 are not correctly serialized back (if you deserialize the tile serialized this way, HasIntValue' isfalsein thatValue` object. Do you think this can be fixed easily somehow? Thanks!

[bertt]

Hi, can you provide a (minimal) reproducible case? Thanks.

[rouen-sk]

Sure, I can provide real-world case (it is not minimal, but i want to use untouched tile, as generated by mapnik-vector-tile, to prevent any errors on my side).

See tile attached. Layers[4] is layer named road___1. Values[1] in this layer is our subject: HasIntValue=true and IntValue=0.

Now just deserialize it into stream, serialize it back (save it to file maybe, i do that), deserialize this new file, and the same Value object will be HasIntValue=false (in fact, it has all of the HasAnythingValue false, so I guess it is invalid.

16_34440_23455_raw.zip

[bertt]

started a unit test to reproduce this issue, see https://github.com/bertt/mapbox-vector-tile-cs/blob/master/tests/mapbox.vector.tile.tests/DeserializeSerializeBackTests.cs

if you can add some missing code it will be great, otherwise I'll complete it later this week.

[bertt]

Thanks I've added the code from https://github.com/bertt/mapbox-vector-tile-cs/commit/067c508c2ac19448ee2ddbaa40d8748838dada44#commitcomment-27925502

unit test is indeed failing, let's find a way to fix it

[bertt]

so this only happens when attribute type=int and value=0? or also with other types? maybe same behaviour with empty strings ("")?

if so, then it should be related to the default values, for example https://github.com/bertt/mapbox-vector-tile-cs/blob/master/src/VectorTile.cs#L68

[bertt]

unit test is working when changing https://github.com/bertt/mapbox-vector-tile-cs/blob/master/src/VectorTile.cs#L68 from '[System.ComponentModel.DefaultValue(default(long))]' to '[System.ComponentModel.DefaultValue(int.MinValue)]'

lets determine impact for other types.

[rouen-sk]

my guess is that is happens when the value is default for the particular type (such as 0 for Int32). Since empty string is not default for string, and nulls are not supported by MVT specification, I suspect no problem here. But of course, needs to be tested.

[bertt]

i've created a new NuGet package (4.0.1) can you try (once it's indexed)? https://www.nuget.org/packages/mapbox-vector-tile/4.0.1

[rouen-sk]

@bertt I dont use nuget package, I have sources cloned in my project, since I extended them slightly (for example implemented System.IEquatable<Value> on Value). Anyway - fix is not good. I applied changes in https://github.com/bertt/mapbox-vector-tile-cs/commit/76eaefc81c1a33426603d425ec64507889aaaf21 locally, and now the serialized-deserialized tile's Value has HasFloatValue, HasIntValue and HasStringValue set to true.

Add to the unit test:

Assert.IsFalse(deserializedTile.Layers[4].Values[1].HasFloatValue);
Assert.IsFalse(deserializedTile.Layers[4].Values[1].HasStringValue);

[bertt]

ah ok, little bit too quick :-) I'll take a look later this week again

[rouen-sk]

sure, thanks, I will try to look into it too, but I am pretty swamped. Btw, I recommend you remove nuget 4.0.1. since it can cause regressions even for people not suffering the issue we are solving.

[bertt]

yes good idea, 4.0.1 is unlisted

[rouen-sk]

FYI, since I think this has basically nothing to do with mapbox-vector-tile-cs and it is protobuf-net issue, I raised the question there: https://github.com/mgravell/protobuf-net/issues/363

[bertt]

ok great! I've reverted VectorTile.cs so the back-to-back serialize unit test fails again. Also added the checks on HasFloatValue and HasStringValue

[rouen-sk]

So, guys from protobuf-net proposed easy solution for this, see https://github.com/mgravell/protobuf-net/issues/363#issuecomment-371161632

You just need to add this code into Value class, and done.

            public bool ShouldSerializeStringValue() => HasStringValue;
            public bool ShouldSerializeFloatValue() => HasFloatValue;
            public bool ShouldSerializeDoubleValue() => HasDoubleValue;
            public bool ShouldSerializeIntValue() => HasIntValue;
            public bool ShouldSerializeUIntValue() => HasUIntValue;
            public bool ShouldSerializeSIntValue() => HasSIntValue;
            public bool ShouldSerializeBoolValue() => HasBoolValue;

I have tested in on my local version and my use case, and it works.

[bertt]

great it works, all unit tests working again. Closing this issue.