diff --git a/cli/pbjs.js b/cli/pbjs.js index 6b69397f3..801f40144 100644 --- a/cli/pbjs.js +++ b/cli/pbjs.js @@ -41,7 +41,7 @@ exports.main = function main(args, callback) { "force-message": "strict-message" }, string: [ "target", "out", "path", "wrap", "dependency", "root", "lint" ], - boolean: [ "create", "encode", "decode", "verify", "convert", "delimited", "beautify", "comments", "service", "es6", "sparse", "keep-case", "force-long", "force-number", "force-enum-string", "force-message" ], + boolean: [ "create", "encode", "decode", "verify", "convert", "delimited", "beautify", "comments", "service", "es6", "sparse", "keep-case", "force-long", "force-number", "force-enum-string", "force-message", "null-defaults" ], default: { target: "json", create: true, @@ -59,7 +59,8 @@ exports.main = function main(args, callback) { "force-long": false, "force-number": false, "force-enum-string": false, - "force-message": false + "force-message": false, + "null-defaults": false, } }); @@ -139,6 +140,8 @@ exports.main = function main(args, callback) { " --force-number Enforces the use of 'number' for s-/u-/int64 and s-/fixed64 fields.", " --force-message Enforces the use of message instances instead of plain objects.", "", + " --null-defaults Default value for optional fields is null instead of zero value.", + "", "usage: " + chalk.bold.green("pbjs") + " [options] file1.proto file2.json ..." + chalk.gray(" (or pipe) ") + "other | " + chalk.bold.green("pbjs") + " [options] -", "" ].join("\n")); diff --git a/cli/targets/static.js b/cli/targets/static.js index a738f3585..4d6e6a102 100644 --- a/cli/targets/static.js +++ b/cli/targets/static.js @@ -394,7 +394,7 @@ function buildType(ref, type) { if (config.comments) { push(""); var jsType = toJsType(field); - if (field.optional && !field.map && !field.repeated && field.resolvedType instanceof Type || field.partOf) + if (field.optional && !field.map && !field.repeated && (field.resolvedType instanceof Type || config["null-defaults"]) || field.partOf) jsType = jsType + "|null|undefined"; pushComment([ field.comment || type.name + " " + field.name + ".", @@ -410,7 +410,7 @@ function buildType(ref, type) { push(escapeName(type.name) + ".prototype" + prop + " = $util.emptyArray;"); // overwritten in constructor else if (field.map) push(escapeName(type.name) + ".prototype" + prop + " = $util.emptyObject;"); // overwritten in constructor - else if (field.partOf) + else if (field.partOf || (field.optional && config["null-defaults"])) push(escapeName(type.name) + ".prototype" + prop + " = null;"); // do not set default value for oneof members else if (field.long) push(escapeName(type.name) + ".prototype" + prop + " = $util.Long ? $util.Long.fromBits(" diff --git a/tests/cli.js b/tests/cli.js index a73ac0181..177d721ae 100644 --- a/tests/cli.js +++ b/tests/cli.js @@ -5,7 +5,7 @@ var path = require("path"); var Module = require("module"); var protobuf = require(".."); -tape.test("pbjs generates static code", function(test) { +function cliTest(test, testFunc) { // pbjs does not seem to work with Node v4, so skip this test if we're running on it if (process.versions.node.match(/^4\./)) { test.end(); @@ -23,54 +23,133 @@ tape.test("pbjs generates static code", function(test) { }; require.cache.protobufjs = require.cache[path.resolve("index.js")]; - var staticTarget = require("../cli/targets/static"); - - var root = protobuf.loadSync("tests/data/cli/test.proto"); - root.resolveAll(); - - staticTarget(root, { - create: true, - decode: true, - encode: true, - convert: true, - }, function(err, jsCode) { - test.error(err, 'static code generation worked'); - - // jsCode is the generated code; we'll eval it - // (since this is what we normally does with the code, right?) - // This is a test code. Do not use this in production. - var $protobuf = protobuf; - eval(jsCode); - - var OneofContainer = protobuf.roots.default.OneofContainer; - var Message = protobuf.roots.default.Message; - test.ok(OneofContainer, "type is loaded"); - test.ok(Message, "type is loaded"); - - // Check that fromObject and toObject work for plain object - var obj = { - messageInOneof: { - value: 42, - }, - regularField: "abc", - }; - var obj1 = OneofContainer.toObject(OneofContainer.fromObject(obj)); - test.deepEqual(obj, obj1, "fromObject and toObject work for plain object"); - - // Check that dynamic fromObject and toObject work for static instance + try { + testFunc(); + } finally { + // Rollback all the require() related mess we made + delete require.cache.protobufjs; + Module._resolveFilename = savedResolveFilename; + } +} + +tape.test("pbjs generates static code", function(test) { + cliTest(test, function() { var root = protobuf.loadSync("tests/data/cli/test.proto"); - var OneofContainerDynamic = root.lookup("OneofContainer"); - var instance = new OneofContainer(); - instance.messageInOneof = new Message(); - instance.messageInOneof.value = 42; - instance.regularField = "abc"; - var instance1 = OneofContainerDynamic.toObject(OneofContainerDynamic.fromObject(instance)); - test.deepEqual(instance, instance1, "fromObject and toObject work for instance of the static type"); - - test.end(); + root.resolveAll(); + + var staticTarget = require("../cli/targets/static"); + + staticTarget(root, { + create: true, + decode: true, + encode: true, + convert: true, + }, function(err, jsCode) { + test.error(err, 'static code generation worked'); + + // jsCode is the generated code; we'll eval it + // (since this is what we normally does with the code, right?) + // This is a test code. Do not use this in production. + var $protobuf = protobuf; + eval(jsCode); + + var OneofContainer = protobuf.roots.default.OneofContainer; + var Message = protobuf.roots.default.Message; + test.ok(OneofContainer, "type is loaded"); + test.ok(Message, "type is loaded"); + + // Check that fromObject and toObject work for plain object + var obj = { + messageInOneof: { + value: 42, + }, + regularField: "abc", + }; + var obj1 = OneofContainer.toObject(OneofContainer.fromObject(obj)); + test.deepEqual(obj, obj1, "fromObject and toObject work for plain object"); + + // Check that dynamic fromObject and toObject work for static instance + var root = protobuf.loadSync("tests/data/cli/test.proto"); + var OneofContainerDynamic = root.lookup("OneofContainer"); + var instance = new OneofContainer(); + instance.messageInOneof = new Message(); + instance.messageInOneof.value = 42; + instance.regularField = "abc"; + var instance1 = OneofContainerDynamic.toObject(OneofContainerDynamic.fromObject(instance)); + test.deepEqual(instance, instance1, "fromObject and toObject work for instance of the static type"); + + test.end(); + }); }); +}); + +tape.test("without null-defaults, absent optional fields have zero values", function(test) { + cliTest(test, function() { + var root = protobuf.loadSync("tests/data/cli/null-defaults.proto"); + root.resolveAll(); + + var staticTarget = require("../cli/targets/static"); - // Rollback all the require() related mess we made - delete require.cache.protobufjs; - Module._resolveFilename = savedResolveFilename; + staticTarget(root, { + create: true, + decode: true, + encode: true, + convert: true, + }, function(err, jsCode) { + test.error(err, 'static code generation worked'); + + // jsCode is the generated code; we'll eval it + // (since this is what we normally does with the code, right?) + // This is a test code. Do not use this in production. + var $protobuf = protobuf; + eval(jsCode); + + var OptionalFields = protobuf.roots.default.OptionalFields; + test.ok(OptionalFields, "type is loaded"); + + // Check default values + var msg = OptionalFields.fromObject({}); + test.equal(msg.a, null, "default submessage is null"); + test.equal(msg.b, "", "default string is empty"); + test.equal(msg.c, 0, "default integer is 0"); + + test.end(); + }); + }); +}); + +tape.test("with null-defaults, absent optional fields have null values", function(test) { + cliTest(test, function() { + var root = protobuf.loadSync("tests/data/cli/null-defaults.proto"); + root.resolveAll(); + + var staticTarget = require("../cli/targets/static"); + + staticTarget(root, { + create: true, + decode: true, + encode: true, + convert: true, + "null-defaults": true, + }, function(err, jsCode) { + test.error(err, 'static code generation worked'); + + // jsCode is the generated code; we'll eval it + // (since this is what we normally does with the code, right?) + // This is a test code. Do not use this in production. + var $protobuf = protobuf; + eval(jsCode); + + var OptionalFields = protobuf.roots.default.OptionalFields; + test.ok(OptionalFields, "type is loaded"); + + // Check default values + var msg = OptionalFields.fromObject({}); + test.equal(msg.a, null, "default submessage is null"); + test.equal(msg.b, null, "default string is null"); + test.equal(msg.c, null, "default integer is null"); + + test.end(); + }); + }); }); diff --git a/tests/data/cli/null-defaults.proto b/tests/data/cli/null-defaults.proto new file mode 100644 index 000000000..6a140b1e2 --- /dev/null +++ b/tests/data/cli/null-defaults.proto @@ -0,0 +1,11 @@ +syntax = "proto2"; + +message OptionalFields { + message SubMessage { + required string a = 1; + } + + optional SubMessage a = 1; + optional string b = 2; + optional uint32 c = 3; +}