Skip to content

Commit

Permalink
Take special care of oneofs when encoding (i.e. when explicitly set t…
Browse files Browse the repository at this point in the history
…o defaults), see #542
  • Loading branch information
dcodeIO committed Dec 11, 2016
1 parent aff21a7 commit e789367
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 23 deletions.
67 changes: 58 additions & 9 deletions dist/protobuf.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/protobuf.js.map

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/protobuf.min.js

Large diffs are not rendered by default.

Binary file modified dist/protobuf.min.js.gz
Binary file not shown.
2 changes: 1 addition & 1 deletion dist/protobuf.min.js.map

Large diffs are not rendered by default.

41 changes: 39 additions & 2 deletions src/encode.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ function encode(message, writer) {
// Non-repeated
} else {
var value = message[field.name];
if (field.required || value !== undefined && field.long ? util.longNeq(value, field.defaultValue) : value !== field.defaultValue) {
if (
field.partOf && message[field.partOf.name] === field.name
||
(field.required || value !== undefined) && (field.long ? util.longNeq(value, field.defaultValue) : value !== field.defaultValue)
) {
if (wireType !== undefined)
writer.tag(field.id, wireType)[type](value);
else {
Expand Down Expand Up @@ -97,6 +101,7 @@ function encode(message, writer) {
encode.generate = function generate(mtype) {
/* eslint-disable no-unexpected-multiline */
var fields = mtype.getFieldsArray();
var oneofs = mtype.getOneofsArray();
var gen = util.codegen("m", "w")
("w||(w=Writer.create())");

Expand Down Expand Up @@ -156,7 +161,7 @@ encode.generate = function generate(mtype) {
}

// Non-repeated
} else {
} else if (!field.partOf) {
if (!field.required) {

if (field.long) gen
Expand All @@ -180,6 +185,38 @@ encode.generate = function generate(mtype) {

}
}
for (var i = 0; i < oneofs.length; ++i) { gen
var oneof = oneofs[i],
prop = util.safeProp(oneof.name);
gen
("switch(m%s){", prop);
var oneofFields = oneof.getFieldsArray();
for (var j = 0; j < oneofFields.length; ++j) {
var field = oneofFields[j],
type = field.resolvedType instanceof Enum ? "uint32" : field.type,
wireType = types.basic[type],
prop = util.safeProp(field.name);
gen
("case%j:", field.name);

if (wireType !== undefined) gen

("w.tag(%d,%d).%s(m%s)", field.id, wireType, type, prop);

else if (field.required) gen

("types[%d].encode(m%s,w.tag(%d,2).fork()).ldelim()", fields.indexOf(field), prop, field.id);

else gen

("types[%d].encode(m%s,w.fork()).len&&w.ldelim(%d)||w.reset()", fields.indexOf(field), prop, field.id);
gen
("break;");

} gen
("}");
}

return gen
("return w");
/* eslint-enable no-unexpected-multiline */
Expand Down
24 changes: 18 additions & 6 deletions src/oneof.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,21 @@ function OneOf(name, fieldNames, options) {
* @type {Field[]}
* @private
*/
this._fields = [];
this._fieldsArray = [];
}

/**
* Fields that belong to this oneof as an array for iteration.
* @name OneOf#fieldsArray
* @type {Field[]}
* @readonly
*/
util.prop(OneOfPrototype, "fieldsArray", {
get: function getFieldsArray() {
return this._fieldsArray;
}
});

/**
* Tests if the specified JSON object describes a oneof.
* @param {*} json JSON object
Expand Down Expand Up @@ -87,7 +99,7 @@ OneOfPrototype.toJSON = function toJSON() {
*/
function addFieldsToParent(oneof) {
if (oneof.parent)
oneof._fields.forEach(function(field) {
oneof._fieldsArray.forEach(function(field) {
if (!field.parent)
oneof.parent.add(field);
});
Expand All @@ -104,7 +116,7 @@ OneOfPrototype.add = function add(field) {
if (field.parent)
field.parent.remove(field);
this.oneof.push(field.name);
this._fields.push(field);
this._fieldsArray.push(field);
field.partOf = this; // field.parent remains null
addFieldsToParent(this);
return this;
Expand All @@ -118,10 +130,10 @@ OneOfPrototype.add = function add(field) {
OneOfPrototype.remove = function remove(field) {
if (!(field instanceof Field))
throw _TypeError("field", "a Field");
var index = this._fields.indexOf(field);
var index = this._fieldsArray.indexOf(field);
if (index < 0)
throw Error(field + " is not a member of " + this);
this._fields.splice(index, 1);
this._fieldsArray.splice(index, 1);
index = this.oneof.indexOf(field.name);
if (index > -1)
this.oneof.splice(index, 1);
Expand All @@ -143,7 +155,7 @@ OneOfPrototype.onAdd = function onAdd(parent) {
* @override
*/
OneOfPrototype.onRemove = function onRemove(parent) {
this._fields.forEach(function(field) {
this._fieldsArray.forEach(function(field) {
if (field.parent)
field.parent.remove(field);
});
Expand Down
19 changes: 18 additions & 1 deletion tests/oneof.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,24 @@ tape.test("oneofs", function(test) {
test.notOk(message.hasOwnProperty('num'), "should delete the previous value");
test.equal(message.str, "a", "should set the new value");
test.equal(message.kind, "str", "should reference the new value");


message.num = 0; // default
message.setKind('num');
test.notOk(message.hasOwnProperty('str'), "should delete the previous value");
test.equal(message.num, 0, "should set the new value");
test.equal(message.kind, "num", "should reference the new value");
test.equal(message.hasOwnProperty("num"), true, "should have the new value on the instance, not just the prototype");

var buf = Message.encode(message).finish();
test.equal(buf.length, 2, "should write a total of 2 bytes");
test.equal(buf[0], 16, "should write id 1, wireType 0");
test.equal(buf[1], 0, "should write a value of 0");

buf = protobuf.encode.call(Message, message).finish();
test.equal(buf.length, 2, "should write a total of 2 bytes (fallback)");
test.equal(buf[0], 16, "should write id 1, wireType 0 (fallback)");
test.equal(buf[1], 0, "should write a value of 0 (fallback)");

test.end();
});

Expand Down

0 comments on commit e789367

Please sign in to comment.