Breaking: ReflectionObject#toJSON properly omits explicit undefined v…

…alues
protobufjs · Apr 13, 2017 · b9f1790 · b9f1790
1 parent 6493f52
commit b9f1790
Show file tree

Hide file tree

Showing 16 changed files with 84 additions and 92 deletions.
diff --git a/src/enum.js b/src/enum.js
@@ -74,10 +74,10 @@ Enum.fromJSON = function fromJSON(name, json) {
  * @returns {EnumDescriptor} Enum descriptor
  */
 Enum.prototype.toJSON = function toJSON() {
-    return {
-        options : this.options,
-        values  : this.values
-    };
+    return util.toObject([
+        "options" , this.options,
+        "values"  , this.values
+    ]);
 };
 
 /**

diff --git a/src/field.js b/src/field.js
@@ -235,13 +235,13 @@ Field.prototype.setOption = function setOption(name, value, ifNotSet) {
  * @returns {FieldDescriptor} Field descriptor
  */
 Field.prototype.toJSON = function toJSON() {
-    return {
-        rule    : this.rule !== "optional" && this.rule || undefined,
-        type    : this.type,
-        id      : this.id,
-        extend  : this.extend,
-        options : this.options
-    };
+    return util.toObject([
+        "rule"    , this.rule !== "optional" && this.rule || undefined,
+        "type"    , this.type,
+        "id"      , this.id,
+        "extend"  , this.extend,
+        "options" , this.options
+    ]);
 };
 
 /**

diff --git a/src/mapfield.js b/src/mapfield.js
@@ -79,13 +79,13 @@ MapField.fromJSON = function fromJSON(name, json) {
  * @returns {MapFieldDescriptor} Map field descriptor
  */
 MapField.prototype.toJSON = function toJSON() {
-    return {
-        keyType : this.keyType,
-        type    : this.type,
-        id      : this.id,
-        extend  : this.extend,
-        options : this.options
-    };
+    return util.toObject([
+        "keyType" , this.keyType,
+        "type"    , this.type,
+        "id"      , this.id,
+        "extend"  , this.extend,
+        "options" , this.options
+    ]);
 };
 
 /**

diff --git a/src/method.js b/src/method.js
@@ -115,14 +115,14 @@ Method.fromJSON = function fromJSON(name, json) {
  * @returns {MethodDescriptor} Method descriptor
  */
 Method.prototype.toJSON = function toJSON() {
-    return {
-        type           : this.type !== "rpc" && /* istanbul ignore next */ this.type || undefined,
-        requestType    : this.requestType,
-        requestStream  : this.requestStream,
-        responseType   : this.responseType,
-        responseStream : this.responseStream,
-        options        : this.options
-    };
+    return util.toObject([
+        "type"           , this.type !== "rpc" && /* istanbul ignore next */ this.type || undefined,
+        "requestType"    , this.requestType,
+        "requestStream"  , this.requestStream,
+        "responseType"   , this.responseType,
+        "responseStream" , this.responseStream,
+        "options"        , this.options
+    ]);
 };
 
 /**

diff --git a/src/namespace.js b/src/namespace.js
@@ -131,10 +131,10 @@ Object.defineProperty(Namespace.prototype, "nestedArray", {
  * @returns {NamespaceBaseDescriptor} Namespace descriptor
  */
 Namespace.prototype.toJSON = function toJSON() {
-    return {
-        options : this.options,
-        nested  : arrayToJSON(this.nestedArray)
-    };
+    return util.toObject([
+        "options" , this.options,
+        "nested"  , arrayToJSON(this.nestedArray)
+    ]);
 };
 
 /**

diff --git a/src/oneof.js b/src/oneof.js
@@ -66,10 +66,10 @@ OneOf.fromJSON = function fromJSON(name, json) {
  * @returns {OneOfDescriptor} Oneof descriptor
  */
 OneOf.prototype.toJSON = function toJSON() {
-    return {
-        oneof   : this.oneof,
-        options : this.options
-    };
+    return util.toObject([
+        "options" , this.options,
+        "oneof"   , this.oneof
+    ]);
 };
 
 /**

diff --git a/src/service.js b/src/service.js
@@ -68,11 +68,11 @@ Service.fromJSON = function fromJSON(name, json) {
  */
 Service.prototype.toJSON = function toJSON() {
     var inherited = Namespace.prototype.toJSON.call(this);
-    return {
-        options : inherited && inherited.options || undefined,
-        methods : Namespace.arrayToJSON(this.methodsArray) || /* istanbul ignore next */ {},
-        nested  : inherited && inherited.nested || undefined
-    };
+    return util.toObject([
+        "options" , inherited && inherited.options || undefined,
+        "methods" , Namespace.arrayToJSON(this.methodsArray) || /* istanbul ignore next */ {},
+        "nested"  , inherited && inherited.nested || undefined
+    ]);
 };
 
 /**

diff --git a/src/type.js b/src/type.js
@@ -281,15 +281,15 @@ Type.fromJSON = function fromJSON(name, json) {
  */
 Type.prototype.toJSON = function toJSON() {
     var inherited = Namespace.prototype.toJSON.call(this);
-    return {
-        options    : inherited && inherited.options || undefined,
-        oneofs     : Namespace.arrayToJSON(this.oneofsArray),
-        fields     : Namespace.arrayToJSON(this.fieldsArray.filter(function(obj) { return !obj.declaringField; })) || {},
-        extensions : this.extensions && this.extensions.length ? this.extensions : undefined,
-        reserved   : this.reserved && this.reserved.length ? this.reserved : undefined,
-        group      : this.group || undefined,
-        nested     : inherited && inherited.nested || undefined
-    };
+    return util.toObject([
+        "options"    , inherited && inherited.options || undefined,
+        "oneofs"     , Namespace.arrayToJSON(this.oneofsArray),
+        "fields"     , Namespace.arrayToJSON(this.fieldsArray.filter(function(obj) { return !obj.declaringField; })) || {},
+        "extensions" , this.extensions && this.extensions.length ? this.extensions : undefined,
+        "reserved"   , this.reserved && this.reserved.length ? this.reserved : undefined,
+        "group"      , this.group || undefined,
+        "nested"     , inherited && inherited.nested || undefined
+    ]);
 };
 
 /**

diff --git a/src/util.js b/src/util.js
@@ -34,6 +34,22 @@ util.toArray = function toArray(object) {
     return array;
 };
 
+/**
+ * Converts an array of keys immediately followed by their respective value to an object, omitting undefined values.
+ * @param {Array.<*>} array Array to convert
+ * @returns {Object.<string,*>} Converted object
+ */
+util.toObject = function toObject(array) {
+    var object = {};
+    for (var i = 0; i < array.length; i += 2) {
+        var key = array[i    ],
+            val = array[i + 1];
+        if (val !== undefined)
+            object[key] = val;
+    }
+    return object;
+};
+
 var safePropBackslashRe = /\\/g,
     safePropQuoteRe     = /"/g;
 

diff --git a/tests/api_enum.js b/tests/api_enum.js
@@ -68,12 +68,11 @@ tape.test("reflected enums", function(test) {
     }, "should no longer expose any removed values by id");
 
     test.same(enm.toJSON(), {
-        options: undefined,
         values: {
             a: 1,
             c: 3
         }
-    }, "should export options and values to JSON");
+    }, "should export values to JSON");
 
     enm_allow_alias.add( 'b', 0 );
     test.same( enm_allow_alias.values, {

diff --git a/tests/api_field.js b/tests/api_field.js
@@ -43,10 +43,8 @@ tape.test("reflected fields", function(test) {
         field = new protobuf.Field("a", 1, "uint32", /* rule */ undefined, /* skipped extend, */ /* options */ {});
 
     test.same(field.toJSON(), {
-        rule: undefined,
         type: "uint32",
         id: 1,
-        extend: undefined,
         options: {}
     }, "should export to JSON");
 

diff --git a/tests/api_mapfield.js b/tests/api_mapfield.js
@@ -5,9 +5,7 @@ var protobuf = require("..");
 var def = {
     keyType: "bytes",
     type: "string",
-    id: 1,
-    extend: undefined,
-    options: undefined
+    id: 1
 };
 
 tape.test("reflected map fields", function(test) {

diff --git a/tests/api_namespace.js b/tests/api_namespace.js
@@ -2,10 +2,7 @@ var tape = require("tape");
 
 var protobuf = require("..");
 
-var def = {
-    nested: undefined,
-    options: undefined
-};
+var def = {};
 
 var proto = "package ns;\
 enum Enm {\
@@ -124,13 +121,12 @@ tape.test("reflected namespaces", function(test) {
     });
     test.same(ns.toJSON(), {
         nested: {
-            Message: { extensions: undefined, fields: {}, group: undefined, nested: undefined, oneofs: undefined, options: undefined, reserved: undefined },
-            Enum: { options: undefined, values: {} },
-            Service: { methods: {}, nested: undefined, options: undefined },
-            extensionField: { extend: "Message", id: 1000, options: undefined, rule: undefined, type: "string" },
-            Other: { nested: undefined, options: undefined }
-        },
-        options: undefined
+            Message: { fields: {} },
+            Enum: { values: {} },
+            Service: { methods: {} },
+            extensionField: { extend: "Message", id: 1000, type: "string" },
+            Other: { }
+        }
     }, "should create from Type, Enum, Service, extension Field and Namespace JSON");
 
     test.end();

diff --git a/tests/api_oneof.js b/tests/api_oneof.js
@@ -31,15 +31,13 @@ tape.test("reflected oneofs", function(test) {
 
     kind.add(field);
     test.same(kind.toJSON(), {
-        oneof: ["a", "b", "c"],
-        options: undefined
+        oneof: ["a", "b", "c"]
     }, "should allow adding fields");
     test.ok(Test.get("c"), "should still have the field on the parent");
 
     kind.remove(field);
     test.same(kind.toJSON(), {
-        oneof: ["a", "b"],
-        options: undefined
+        oneof: ["a", "b"]
     }, "should allow removing fields");
     test.ok(Test.get("c"), "should still have the field on the parent");
 

diff --git a/tests/api_service.js b/tests/api_service.js
@@ -6,15 +6,12 @@ var def = {
     methods: {},
     nested: {
         SomeEnum: {
-            options: undefined,
             values: {}
         }
-    },
-    options: undefined
+    }
 };
 
 var methodDef = {
-    type: undefined,
     requestType: "MyRequest",
     requestStream: true,
     responseType: "MyResponse",

diff --git a/tests/api_type.js b/tests/api_type.js
@@ -3,13 +3,7 @@ var tape = require("tape");
 var protobuf = require("..");
 
 var def = {
-    fields: {},
-    oneofs: undefined,
-    extensions: undefined,
-    reserved: undefined,
-    group: undefined,
-    nested: undefined,
-    options: undefined,
+    fields: {}
 };
 
 var def2 = {
@@ -75,20 +69,16 @@ tape.test("reflected types", function(test) {
     });
     test.same(type.toJSON(), {
         fields: {
-            a: { extend: undefined, id: 1, options: undefined, rule: undefined, type: "string" }
+            a: { id: 1, type: "string" }
         },
-        oneofs: undefined,
-        extensions: undefined,
         reserved: [[900, 999], "b"],
-        group: undefined,
         nested: {
-            Type: { extensions: undefined, fields: {}, group: undefined, nested: undefined, oneofs: undefined, options: undefined, reserved: undefined },
-            Enum: { options: undefined, values: {} },
-            Service: { methods: {}, nested: undefined, options: undefined },
-            extensionField: { extend: "Message", id: 1000, options: undefined, rule: undefined, type: "string" },
-            Other: { nested: undefined, options: undefined }
-        },
-        options: undefined
+            Type: { fields: {} },
+            Enum: { values: {} },
+            Service: { methods: {} },
+            extensionField: { extend: "Message", id: 1000, type: "string" },
+            Other: { }
+        }
     }, "should create from Field, Type, Enum, Service, extension Field and Namespace JSON");
 
     test.throws(function() {