Skip to content

Commit 6e713ba

Browse files
sloonzalexander-fenster
sloonz
and
alexander-fenster
authored
feat: add null-defaults option (protobufjs#1611)
* feat: add null-defaults option * fix: linting Co-authored-by: Alexander Fenster <github@fenster.name> Co-authored-by: Alexander Fenster <fenster@google.com>
1 parent 9011aac commit 6e713ba

File tree

4 files changed

+145
-52
lines changed

4 files changed

+145
-52
lines changed

‎cli/pbjs.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ exports.main = function main(args, callback) {
4141
"force-message": "strict-message"
4242
},
4343
string: [ "target", "out", "path", "wrap", "dependency", "root", "lint" ],
44-
boolean: [ "create", "encode", "decode", "verify", "convert", "delimited", "beautify", "comments", "service", "es6", "sparse", "keep-case", "force-long", "force-number", "force-enum-string", "force-message" ],
44+
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" ],
4545
default: {
4646
target: "json",
4747
create: true,
@@ -59,7 +59,8 @@ exports.main = function main(args, callback) {
5959
"force-long": false,
6060
"force-number": false,
6161
"force-enum-string": false,
62-
"force-message": false
62+
"force-message": false,
63+
"null-defaults": false,
6364
}
6465
});
6566

@@ -139,6 +140,8 @@ exports.main = function main(args, callback) {
139140
" --force-number Enforces the use of 'number' for s-/u-/int64 and s-/fixed64 fields.",
140141
" --force-message Enforces the use of message instances instead of plain objects.",
141142
"",
143+
" --null-defaults Default value for optional fields is null instead of zero value.",
144+
"",
142145
"usage: " + chalk.bold.green("pbjs") + " [options] file1.proto file2.json ..." + chalk.gray(" (or pipe) ") + "other | " + chalk.bold.green("pbjs") + " [options] -",
143146
""
144147
].join("\n"));

‎cli/targets/static.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ function buildType(ref, type) {
394394
if (config.comments) {
395395
push("");
396396
var jsType = toJsType(field);
397-
if (field.optional && !field.map && !field.repeated && field.resolvedType instanceof Type || field.partOf)
397+
if (field.optional && !field.map && !field.repeated && (field.resolvedType instanceof Type || config["null-defaults"]) || field.partOf)
398398
jsType = jsType + "|null|undefined";
399399
pushComment([
400400
field.comment || type.name + " " + field.name + ".",
@@ -410,7 +410,7 @@ function buildType(ref, type) {
410410
push(escapeName(type.name) + ".prototype" + prop + " = $util.emptyArray;"); // overwritten in constructor
411411
else if (field.map)
412412
push(escapeName(type.name) + ".prototype" + prop + " = $util.emptyObject;"); // overwritten in constructor
413-
else if (field.partOf)
413+
else if (field.partOf || field.optional && config["null-defaults"])
414414
push(escapeName(type.name) + ".prototype" + prop + " = null;"); // do not set default value for oneof members
415415
else if (field.long)
416416
push(escapeName(type.name) + ".prototype" + prop + " = $util.Long ? $util.Long.fromBits("

‎tests/cli.js‎

Lines changed: 127 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ var path = require("path");
55
var Module = require("module");
66
var protobuf = require("..");
77

8-
tape.test("pbjs generates static code", function(test) {
8+
function cliTest(test, testFunc) {
99
// pbjs does not seem to work with Node v4, so skip this test if we're running on it
1010
if (process.versions.node.match(/^4\./)) {
1111
test.end();
@@ -23,54 +23,133 @@ tape.test("pbjs generates static code", function(test) {
2323
};
2424
require.cache.protobufjs = require.cache[path.resolve("index.js")];
2525

26-
var staticTarget = require("../cli/targets/static");
27-
28-
var root = protobuf.loadSync("tests/data/cli/test.proto");
29-
root.resolveAll();
30-
31-
staticTarget(root, {
32-
create: true,
33-
decode: true,
34-
encode: true,
35-
convert: true,
36-
}, function(err, jsCode) {
37-
test.error(err, 'static code generation worked');
38-
39-
// jsCode is the generated code; we'll eval it
40-
// (since this is what we normally does with the code, right?)
41-
// This is a test code. Do not use this in production.
42-
var $protobuf = protobuf;
43-
eval(jsCode);
44-
45-
var OneofContainer = protobuf.roots.default.OneofContainer;
46-
var Message = protobuf.roots.default.Message;
47-
test.ok(OneofContainer, "type is loaded");
48-
test.ok(Message, "type is loaded");
49-
50-
// Check that fromObject and toObject work for plain object
51-
var obj = {
52-
messageInOneof: {
53-
value: 42,
54-
},
55-
regularField: "abc",
56-
};
57-
var obj1 = OneofContainer.toObject(OneofContainer.fromObject(obj));
58-
test.deepEqual(obj, obj1, "fromObject and toObject work for plain object");
59-
60-
// Check that dynamic fromObject and toObject work for static instance
26+
try {
27+
testFunc();
28+
} finally {
29+
// Rollback all the require() related mess we made
30+
delete require.cache.protobufjs;
31+
Module._resolveFilename = savedResolveFilename;
32+
}
33+
}
34+
35+
tape.test("pbjs generates static code", function(test) {
36+
cliTest(test, function() {
6137
var root = protobuf.loadSync("tests/data/cli/test.proto");
62-
var OneofContainerDynamic = root.lookup("OneofContainer");
63-
var instance = new OneofContainer();
64-
instance.messageInOneof = new Message();
65-
instance.messageInOneof.value = 42;
66-
instance.regularField = "abc";
67-
var instance1 = OneofContainerDynamic.toObject(OneofContainerDynamic.fromObject(instance));
68-
test.deepEqual(instance, instance1, "fromObject and toObject work for instance of the static type");
69-
70-
test.end();
38+
root.resolveAll();
39+
40+
var staticTarget = require("../cli/targets/static");
41+
42+
staticTarget(root, {
43+
create: true,
44+
decode: true,
45+
encode: true,
46+
convert: true,
47+
}, function(err, jsCode) {
48+
test.error(err, 'static code generation worked');
49+
50+
// jsCode is the generated code; we'll eval it
51+
// (since this is what we normally does with the code, right?)
52+
// This is a test code. Do not use this in production.
53+
var $protobuf = protobuf;
54+
eval(jsCode);
55+
56+
var OneofContainer = protobuf.roots.default.OneofContainer;
57+
var Message = protobuf.roots.default.Message;
58+
test.ok(OneofContainer, "type is loaded");
59+
test.ok(Message, "type is loaded");
60+
61+
// Check that fromObject and toObject work for plain object
62+
var obj = {
63+
messageInOneof: {
64+
value: 42,
65+
},
66+
regularField: "abc",
67+
};
68+
var obj1 = OneofContainer.toObject(OneofContainer.fromObject(obj));
69+
test.deepEqual(obj, obj1, "fromObject and toObject work for plain object");
70+
71+
// Check that dynamic fromObject and toObject work for static instance
72+
var root = protobuf.loadSync("tests/data/cli/test.proto");
73+
var OneofContainerDynamic = root.lookup("OneofContainer");
74+
var instance = new OneofContainer();
75+
instance.messageInOneof = new Message();
76+
instance.messageInOneof.value = 42;
77+
instance.regularField = "abc";
78+
var instance1 = OneofContainerDynamic.toObject(OneofContainerDynamic.fromObject(instance));
79+
test.deepEqual(instance, instance1, "fromObject and toObject work for instance of the static type");
80+
81+
test.end();
82+
});
7183
});
84+
});
85+
86+
tape.test("without null-defaults, absent optional fields have zero values", function(test) {
87+
cliTest(test, function() {
88+
var root = protobuf.loadSync("tests/data/cli/null-defaults.proto");
89+
root.resolveAll();
90+
91+
var staticTarget = require("../cli/targets/static");
7292

73-
// Rollback all the require() related mess we made
74-
delete require.cache.protobufjs;
75-
Module._resolveFilename = savedResolveFilename;
93+
staticTarget(root, {
94+
create: true,
95+
decode: true,
96+
encode: true,
97+
convert: true,
98+
}, function(err, jsCode) {
99+
test.error(err, 'static code generation worked');
100+
101+
// jsCode is the generated code; we'll eval it
102+
// (since this is what we normally does with the code, right?)
103+
// This is a test code. Do not use this in production.
104+
var $protobuf = protobuf;
105+
eval(jsCode);
106+
107+
var OptionalFields = protobuf.roots.default.OptionalFields;
108+
test.ok(OptionalFields, "type is loaded");
109+
110+
// Check default values
111+
var msg = OptionalFields.fromObject({});
112+
test.equal(msg.a, null, "default submessage is null");
113+
test.equal(msg.b, "", "default string is empty");
114+
test.equal(msg.c, 0, "default integer is 0");
115+
116+
test.end();
117+
});
118+
});
119+
});
120+
121+
tape.test("with null-defaults, absent optional fields have null values", function(test) {
122+
cliTest(test, function() {
123+
var root = protobuf.loadSync("tests/data/cli/null-defaults.proto");
124+
root.resolveAll();
125+
126+
var staticTarget = require("../cli/targets/static");
127+
128+
staticTarget(root, {
129+
create: true,
130+
decode: true,
131+
encode: true,
132+
convert: true,
133+
"null-defaults": true,
134+
}, function(err, jsCode) {
135+
test.error(err, 'static code generation worked');
136+
137+
// jsCode is the generated code; we'll eval it
138+
// (since this is what we normally does with the code, right?)
139+
// This is a test code. Do not use this in production.
140+
var $protobuf = protobuf;
141+
eval(jsCode);
142+
143+
var OptionalFields = protobuf.roots.default.OptionalFields;
144+
test.ok(OptionalFields, "type is loaded");
145+
146+
// Check default values
147+
var msg = OptionalFields.fromObject({});
148+
test.equal(msg.a, null, "default submessage is null");
149+
test.equal(msg.b, null, "default string is null");
150+
test.equal(msg.c, null, "default integer is null");
151+
152+
test.end();
153+
});
154+
});
76155
});

‎tests/data/cli/null-defaults.proto‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
syntax = "proto2";
2+
3+
message OptionalFields {
4+
message SubMessage {
5+
required string a = 1;
6+
}
7+
8+
optional SubMessage a = 1;
9+
optional string b = 2;
10+
optional uint32 c = 3;
11+
}

0 commit comments

Comments
 (0)