Take special care of oneofs when encoding (i.e. when explicitly set to defaults), see #542

Author: dcodeIO

dist/protobuf.js

@@ -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 {
@@ -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())");
 
@@ -156,7 +161,7 @@ encode.generate = function generate(mtype) {
             }
 
         // Non-repeated
-        } else {
+        } else if (!field.partOf) {
             if (!field.required) {
 
                 if (field.long) gen
@@ -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 */

dist/protobuf.js.map

@@ -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
@@ -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);
         });
@@ -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;
@@ -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);
@@ -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);
     });

dist/protobuf.min.js

@@ -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();
     });