Skip to content

Commit 5f9bede

Browse files
committed
Fix asJSON defaults option, make it work for repeated fields.
Fixing two issues in here: 1. When the `defaults` option was enabled, we were using `Object.keys(this)` which ends up returning an array that includes things like `$root` and asJSON. Instead we can use the `fields` we already have available. 2. For repeated values we were not setting the empty list default because the length check would break earlier. We're now passing through when the defaults option is set as well. Added a test on this stuff as well.
1 parent c33835c commit 5f9bede

File tree

3 files changed

+36
-5
lines changed

3 files changed

+36
-5
lines changed

Diff for: src/message.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module.exports = Message;
33

44
/**
55
* Constructs a new message instance.
6-
*
6+
*
77
* This method should be called from your custom constructors, i.e. `Message.call(this, properties)`.
88
* @classdesc Abstract runtime message.
99
* @extends {Object}
@@ -46,17 +46,15 @@ MessagePrototype.asJSON = function asJSON(options) {
4646
json = {};
4747
var keys;
4848
if (options.defaults) {
49-
keys = [];
50-
for (var k in this) // eslint-disable-line guard-for-in
51-
keys.push(k);
49+
keys = Object.keys(fields);
5250
} else
5351
keys = Object.keys(this);
5452
for (var i = 0, key; i < keys.length; ++i) {
5553
var field = fields[key = keys[i]],
5654
value = this[key];
5755
if (field) {
5856
if (field.repeated) {
59-
if (value && value.length) {
57+
if (value && (value.length || options.defaults)) {
6058
var array = new Array(value.length);
6159
for (var j = 0, l = value.length; j < l; ++j)
6260
array[j] = field.jsonConvert(value[j], options);

Diff for: tests/data/message.proto

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
syntax = "proto3";
2+
3+
package test;
4+
5+
message TestRepeatedDefaults {
6+
required string aString = 1;
7+
repeated string repeatString = 2;
8+
}

Diff for: tests/message.js

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
var tape = require("tape");
2+
var protobuf = require("..");
3+
4+
tape.test("test asJSON repeated defaults", function(test) {
5+
6+
protobuf.load("tests/data/message.proto", function(err, root) {
7+
if (err)
8+
return test.fail(err.message);
9+
test.ok(true, "should parse without errors");
10+
11+
var TestMessage = root.lookup("test.TestRepeatedDefaults");
12+
var msg1 = TestMessage.create({ aString: 'foo' });
13+
14+
var defaultsOn = msg1.asJSON({ defaults: true });
15+
test.equal(defaultsOn.aString, 'foo', "should set aString value");
16+
test.same(defaultsOn.repeatString, [], "should set repeated default");
17+
18+
var defaultsOff = msg1.asJSON({ defaults: false });
19+
test.equal(defaultsOff.aString, 'foo', "should set aString value");
20+
test.same(defaultsOff.repeatString, undefined, "should not set repeated default");
21+
22+
test.end();
23+
});
24+
25+
});

0 commit comments

Comments
 (0)