From b9f179064f3ddf683f13e0d4e17840301be64010 Mon Sep 17 00:00:00 2001 From: dcodeIO Date: Thu, 13 Apr 2017 12:51:53 +0200 Subject: [PATCH] Breaking: ReflectionObject#toJSON properly omits explicit undefined values --- src/enum.js | 8 ++++---- src/field.js | 14 +++++++------- src/mapfield.js | 14 +++++++------- src/method.js | 16 ++++++++-------- src/namespace.js | 8 ++++---- src/oneof.js | 8 ++++---- src/service.js | 10 +++++----- src/type.js | 18 +++++++++--------- src/util.js | 16 ++++++++++++++++ tests/api_enum.js | 3 +-- tests/api_field.js | 2 -- tests/api_mapfield.js | 4 +--- tests/api_namespace.js | 18 +++++++----------- tests/api_oneof.js | 6 ++---- tests/api_service.js | 5 +---- tests/api_type.js | 26 ++++++++------------------ 16 files changed, 84 insertions(+), 92 deletions(-) diff --git a/src/enum.js b/src/enum.js index 96d3d45b6..7ac8819a1 100644 --- 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 index 1043988df..412940a4c 100644 --- 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 index bfbab637a..e0f76ce1e 100644 --- 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 index e09c8b111..1cd8dcb67 100644 --- 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 index 6fd4bfa51..1d8d73dc6 100644 --- 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 index 14ebc7adb..c2d23934d 100644 --- 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 index afc5d2197..a55e53256 100644 --- 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 index 17014d355..21c72e459 100644 --- 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 index 5e4ad2867..dc5e059e0 100644 --- 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.} 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 index 82055f753..705491331 100644 --- 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 index a2f2e80c0..23fa61fd5 100644 --- 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 index 0904e811c..917b52829 100644 --- 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 index 85c067ddc..b7203da68 100644 --- 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 index 06da34fd3..25fd69c2d 100644 --- 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 index 74bfa51c2..be857daa1 100644 --- 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 index e3159ec67..593066190 100644 --- 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() {