Numeric data is not roundtrip safe

[maros]

Encoding/decoding roundtrip via protobuf or JSON does produce a different result when the set value is 0. See the the following code for a failing testcase

#!/usr/bin/env perl

use strict;
use warnings;
use 5.010;

use Google::ProtocolBuffers::Dynamic;
use Test::More;

my $proto = <<PROTO;
syntax = "proto2";
package test;
message TestProto {
    optional string col1 = 1;
    optional double col2 = 2;
    optional double col3 = 3;
    optional double col4 = 4;                // Implicit zero default
    optional double col5 = 5 [default = 10]; // Explicit non zero default
    optional double col6 = 6 [default = 0];  // Explicit zero default
}
PROTO

my $dynamic = Google::ProtocolBuffers::Dynamic->new();
$dynamic->load_string('test.proto',$proto);
$dynamic->map({
    package             => 'test',
    prefix              => 'Test',
    explicit_defaults   => 0,
    #encode_defaults     => 0,
});
$dynamic->resolve_references();

my $obj = Test::TestProto->new({
    col1    => 'test',
    col2    => 1.234,
    col3    => 0,
    # col4 not set!
    # col5 not set!
    # col6 not set!
});

# Initial object
ok($obj->has_col2,'col2 is set');
ok($obj->has_col3,'col3 is set');
ok(! $obj->has_col4,'col4 is not set');
ok(! $obj->has_col5,'col5 is not set');
ok(! $obj->has_col6,'col6 is not set');
is($obj->get_col2,1.234,'col2 returns set value');
is($obj->get_col3,0,'col3 returns set value');
is($obj->get_col4,0,'col4 returns default value');
is($obj->get_col5,10,'col5 returns default value');
is($obj->get_col6,0,'col6 returns default value');

# Roundtrip via protobuf
my $roundtrip_pb = Test::TestProto->decode($obj->encode);
ok($roundtrip_pb->has_col2,'col2 is still set after protobuf roundtrip');
ok($roundtrip_pb->has_col3,'col3 is still set after protobuf roundtrip');
ok(! $roundtrip_pb->has_col4,'col4 is still not set after protobuf roundtrip');
ok(! $roundtrip_pb->has_col5,'col5 is still not set after protobuf roundtrip');
ok(! $roundtrip_pb->has_col6,'col6 is still not set after protobuf roundtrip');

is($roundtrip_pb->get_col3,0,'col3 returns set value after protobuf roundtrip');
is($roundtrip_pb->get_col4,0,'col4 returns default value after protobuf roundtrip');
is($roundtrip_pb->get_col5,10,'col5 returns default value after protobuf roundtrip');
is($roundtrip_pb->get_col6,0,'col6 returns default value after protobuf roundtrip');

# Roundtrip via JSON
my $roundtrip_json = Test::TestProto->decode_json($obj->encode_json);
ok($roundtrip_json->has_col2,'col2 is still set after JSON roundtrip');
ok($roundtrip_json->has_col3,'col3 is still set after JSON roundtrip');
ok(! $roundtrip_json->has_col4,'col4 is still not set after JSON roundtrip');
ok(! $roundtrip_json->has_col5,'col5 is still not set after JSON roundtrip');
ok(! $roundtrip_json->has_col6,'col6 is still not set after JSON roundtrip');

is($roundtrip_pb->get_col3,0,'col3 returns set value after JSON roundtrip');
is($roundtrip_pb->get_col4,0,'col4 returns default value after JSON roundtrip');
is($roundtrip_pb->get_col5,10,'col5 returns default value after JSON roundtrip');
is($roundtrip_pb->get_col6,0,'col6 returns default value after JSON roundtrip');

Tested with version 0.16 and perl 5.22.3

[mbarbon]

TL;DR it's on purpose; you can change this behaviour using the https://metacpan.org/pod/Google::ProtocolBuffers::Dynamic#encode_defaults option

The treatment of scalars and default values in protocol buffers changed slightly between proto2 and proto3; for example proto3 explicitly states that there is no way to know whether a field has been explicitly set to the default value, while for proto2 there are hints that this might be the case after deserialization, but it is not stated explicitly.

When implementing the bindings, I chose to default to the proto3 treatment of default values for proto2 as well (it's one of the possible interpretations anyway), with the option of enabling the behavior you want with the option above.

From the proto2 language definition:

Changing a default value is generally OK, as long as you remember that default values are never sent over the wire. 

From the proto3 language defintion:

If a value is missing in the JSON-encoded data or if its value is null, it will be interpreted as 
the appropriate default value when parsed into a protocol buffer. If a field has the default 
value in the protocol buffer, it will be omitted in the JSON-encoded data by default to save 
space. An implementation may provide options to emit fields with default values in the 
JSON-encoded output.

Hope this helps, Mattia

[maros]

Thank you for your prompt reply. I'm aware of the changed semantic between proto2 and the proto3 implementation, However I still believe that this report is justified since

1. The result of the test is the same no matter if encode_defaults is set or not
2. The getter behaves differently if if explicit defaults are set compared to implicit defaults

I updated the attached test case to highlight the difference.

[mbarbon]

Sorry for the delay in getting back.

The issue is that map() silently ignores unknown parameters; your test passes if you change the call to map to

$dynamic->map({
    package             => 'test',
    prefix              => 'Test',
    options => {
        explicit_defaults   => 0,
        encode_defaults     => 1,
    },
});

I pushed https://github.com/mbarbon/google-protobuf-dynamic/commit/52481131ee46a85caf0c0a3bf2ad8fe5d6719d88 so it will throw an error on unknown parameters.

Version 0.17 should appear soon on CPAN.