Issues with retaining default oneof values

[stephenplusplus]

I'm working on implementing support for the protobuf API of Google Cloud Datastore into gcloud-node. I've hit a sticking point, and I'm not sure exactly where to look in the stack to find the problem.

I made a fork to reproduce: https://github.com/stephenplusplus/protobuf.js. I eliminated all of the tests other than mine that fails: https://github.com/stephenplusplus/ProtoBuf.js/blob/master/tests/suite.js

When I mocked the structure of the Google proto files into one test proto file, the tests passed. But when I plug in the real files, it fails.

Sorry to dump a bunch of code on you; if you think this should be asked somewhere else, feel free to close. Thanks for any help!

[pcostell]

I think this is related to a oneof inside of map values. Here is a PR which has a test that fails on decoding a default value inside a map but not on decoding a default value outside of a map.

[pcostell]

Ah sorry, I messed up my test, they actually pass as expected.

[dcodeIO]

Currently debugging this without success. Is there still an issue?

[stephenplusplus]

Thanks for checking it out. It's still reproducible for me, but only with my setup from the first post when loading the real proto files, no mocks.

The specific issue is any falsy values in the Value.value_type oneof (integer_value of 0, boolean_value of false), end up getting a null "value_type" when decoded: https://github.com/stephenplusplus/ProtoBuf.js/blob/c88a581dee8395f68c9e0ef28bb267e0a70c29e6/tests/suite.js#L97, where we would expect to see "integer_value" or "boolean_value".

[dcodeIO]

Still pinpointing. A minimal .proto causing this seems to be:

syntax = "proto3";

message Value {
    oneof value_type {
        // A boolean value.
        bool boolean_value = 1;
    }
}

With that test:

var builder = ProtoBuf.loadProtoFile("./test.proto");
var Value = builder.build("Value");
var value1 = new Value({
    boolean_value: false
});
test.strictEqual(value1.value_type, "boolean_value");

var value2 = Value.decode(value1.encode());
test.strictEqual(value2.value_type, "boolean_value"); // fails
test.done();

[dcodeIO]

Seems to be an issue with Reflect.Message.Field#hasWirePresence.

Or it's related to default values not being set when proto3 is used.

[dcodeIO]

This is what happens: When boolean_value is set to false, it is effectively set to the default value of the field (bool defaults to false in proto3), which results in the field not being encoded on the wire (also proto3).

When decoding the (empty) message, it will decode with boolean_value being false again - but ProtoBuf.js doesn't know which field to reference from the virtual oneof field (all not present on the wire, all default values).

Do you know how this is handled by the official implementation? Does it encode fields inside of oneof declarations even if set to their default values?

[pcostell]

I believe it does encode the values. I'm not totally familiar with the implementation, but I think the relevant portion is here:
https://github.com/google/protobuf/blob/master/python/google/protobuf/internal/python_message.py#L656

[stephenplusplus]

Thanks very much! It works perfectly. Any chance of tagging a release soon?

[dcodeIO]

4.1.0 is now on npm