Skip to content

Commit ea3ce19

Browse files
dibenededibenede
committed
Remove deprecated begin/endSubMessage in favor of writeMessage API
1 parent 53271b1 commit ea3ce19

File tree

6 files changed

+150
-71
lines changed

6 files changed

+150
-71
lines changed

‎binary/writer.js

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,6 @@ class BinaryWriter {
7979
* @private @const {!BinaryEncoder}
8080
*/
8181
this.encoder_ = new BinaryEncoder();
82-
83-
/**
84-
* A stack of bookmarks containing the parent blocks for each message
85-
* started via beginSubMessage(), needed as bookkeeping for endSubMessage().
86-
* TODO(aappleby): Deprecated, users should be calling writeMessage().
87-
* @private {!Array<!Array<number>>}
88-
*/
89-
this.bookmarks_ = [];
9082
}
9183

9284
/** @private*/
@@ -190,15 +182,13 @@ class BinaryWriter {
190182
this.blocks_ = [];
191183
this.encoder_.end();
192184
this.totalLength_ = 0;
193-
this.bookmarks_ = [];
194185
}
195186

196187
/**
197188
* Converts the encoded data into a Uint8Array.
198189
* @return {!Uint8Array}
199190
*/
200191
getResultBuffer() {
201-
assert(this.bookmarks_.length == 0);
202192
// flush the encoder to avoid a special case below.
203193
this.pushBlock(this.encoder_.end());
204194
const resultLength = this.totalLength_;
@@ -251,26 +241,6 @@ class BinaryWriter {
251241
}
252242
}
253243

254-
/**
255-
* Begins a new sub-message. The client must call endSubMessage() when they're
256-
* done.
257-
* TODO(aappleby): Deprecated. Move callers to writeMessage().
258-
* @param {number} field The field number of the sub-message.
259-
*/
260-
beginSubMessage(field) {
261-
this.bookmarks_.push(this.beginDelimited_(field));
262-
}
263-
264-
/**
265-
* Finishes a sub-message and packs it into the parent messages' buffer.
266-
* TODO(aappleby): Deprecated. Move callers to writeMessage().
267-
*/
268-
endSubMessage() {
269-
assert(this.bookmarks_.length >= 0);
270-
this.endDelimited_(this.bookmarks_.pop());
271-
}
272-
273-
274244
/**
275245
* Encodes a (field number, wire type) tuple into a wire-format field header
276246
* and stores it in the buffer as a varint.

‎commonjs/export.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ goog.require('jspb.BinaryReader');
1414
goog.require('jspb.BinaryWriter');
1515
goog.require('jspb.ExtensionFieldBinaryInfo');
1616
goog.require('jspb.ExtensionFieldInfo');
17+
goog.require('jspb.internal.public_for_gencode');
1718
goog.require('jspb.Message');
1819
goog.require('jspb.Map');
1920

@@ -27,6 +28,8 @@ if (typeof exports === 'object') {
2728
exports['BinaryWriter'] = jspb.BinaryWriter;
2829
exports['ExtensionFieldInfo'] = jspb.ExtensionFieldInfo;
2930
exports['ExtensionFieldBinaryInfo'] = jspb.ExtensionFieldBinaryInfo;
31+
exports['internal'] = { public_for_gencode: jspb.internal.public_for_gencode };
32+
3033

3134
// These are used by generated code but should not be used directly by clients.
3235
exports['exportSymbol'] = goog.exportSymbol;

‎generator/js_generator.cc

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,6 +1860,7 @@ void Generator::GenerateRequiresImpl(const GeneratorOptions& options,
18601860
bool require_map) const {
18611861
if (require_jspb) {
18621862
required->insert("jspb.Message");
1863+
required->insert("jspb.internal.public_for_gencode");
18631864
required->insert("jspb.BinaryReader");
18641865
required->insert("jspb.BinaryWriter");
18651866
}
@@ -3211,6 +3212,30 @@ void Generator::GenerateClassSerializeBinary(const GeneratorOptions& options,
32113212
"\n");
32123213
}
32133214

3215+
// Generates the code to access a single field value for the binary serializer.
3216+
void GenerateClassSerializeBinaryFieldAccess(const GeneratorOptions& options,
3217+
io::Printer* printer,
3218+
const FieldDescriptor* field) {
3219+
if (field->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) {
3220+
std::string typed_annotation =
3221+
JSFieldTypeAnnotation(options, field,
3222+
/* is_setter_argument = */ false,
3223+
/* force_present = */ false,
3224+
/* singular_if_not_packed = */ false,
3225+
/* bytes_mode = */ BYTES_DEFAULT);
3226+
printer->Print(
3227+
" /** @type {$type$} */ "
3228+
"(message.internal_getField($index$))",
3229+
"index", JSFieldIndex(field), "type", typed_annotation);
3230+
} else {
3231+
printer->Print(
3232+
" message.get$name$($nolazy$)", "name",
3233+
JSGetterName(options, field, BYTES_U8),
3234+
// No lazy creation for maps containers -- fastpath the empty case.
3235+
"nolazy", field->is_map() ? "true" : "");
3236+
}
3237+
}
3238+
32143239
void Generator::GenerateClassSerializeBinaryField(
32153240
const GeneratorOptions& options, io::Printer* printer,
32163241
const FieldDescriptor* field) const {
@@ -3286,15 +3311,20 @@ void Generator::GenerateClassSerializeBinaryField(
32863311
if (field->is_map()) {
32873312
const FieldDescriptor* key_field = MapFieldKey(field);
32883313
const FieldDescriptor* value_field = MapFieldValue(field);
3314+
printer->Print("jspb.internal.public_for_gencode.serializeMapToBinary(\n");
3315+
GenerateClassSerializeBinaryFieldAccess(options, printer, field);
32893316
printer->Print(
3290-
" f.serializeBinary($index$, writer, "
3291-
"$keyWriterFn$, $valueWriterFn$",
3317+
",\n $index$,\n"
3318+
" writer,\n"
3319+
" $keyWriterFn$,\n"
3320+
" $valueWriterFn$",
32923321
"index", absl::StrCat(field->number()), "keyWriterFn",
32933322
JSBinaryWriterMethodName(options, key_field), "valueWriterFn",
32943323
JSBinaryWriterMethodName(options, value_field));
32953324

32963325
if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
3297-
printer->Print(", $messageType$.serializeBinaryToWriter", "messageType",
3326+
printer->Print(",\n $messageType$.serializeBinaryToWriter",
3327+
"messageType",
32983328
GetMessagePath(options, value_field->message_type()));
32993329
}
33003330

‎gulpfile.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ function getClosureCompilerCommand(exportsFile, outputFile) {
171171
'--js=debug.js',
172172
'--js=internal_bytes.js',
173173
'--js=internal_options.js',
174+
'--js=internal_public.js',
174175
'--js=map.js',
175176
'--js=message.js',
176177
'--js=bytestring.js',
@@ -224,7 +225,7 @@ function commonjs_out(cb) {
224225

225226
function closure_make_deps(cb) {
226227
exec(
227-
'./node_modules/.bin/closure-make-deps --closure-path=. --file=node_modules/google-closure-library/closure/goog/deps.js bytestring.js internal_bytes.js internal_options.js binary/arith.js binary/bytesource.js binary/binary_constants.js binary/decoder.js binary/decoder_alias.js binary/encoder.js binary/encoder_alias.js binary/errors.js binary/internal_buffer.js binary/reader.js binary/reader_alias.js binary/test_utils.js binary/utf8.js binary/utils.js binary/writer.js binary/writer_alias.js asserts.js debug.js map.js message.js node_loader.js test_bootstrap.js unsafe_bytestring.js > deps.js',
228+
'./node_modules/.bin/closure-make-deps --closure-path=. --file=node_modules/google-closure-library/closure/goog/deps.js bytestring.js internal_bytes.js internal_options.js internal_public.js binary/arith.js binary/bytesource.js binary/binary_constants.js binary/decoder.js binary/decoder_alias.js binary/encoder.js binary/encoder_alias.js binary/errors.js binary/internal_buffer.js binary/reader.js binary/reader_alias.js binary/test_utils.js binary/utf8.js binary/utils.js binary/writer.js binary/writer_alias.js asserts.js debug.js map.js message.js node_loader.js test_bootstrap.js unsafe_bytestring.js > deps.js',
228229
make_exec_logging_callback(cb));
229230
}
230231

‎internal_public.js

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* @fileoverview Public APIs exposed purely for use by generated code. Use of
3+
* these APIs outside of that context is not supported and actively discouraged.
4+
* @public
5+
*
6+
* DO NOT USE THIS OUTSIDE OF THIS PACKAGE.
7+
*/
8+
9+
goog.module('jspb.internal.public_for_gencode');
10+
goog.module.declareLegacyNamespace();
11+
12+
const asserts = goog.require('goog.asserts');
13+
const { BinaryReader } = goog.require('jspb.binary.reader');
14+
const { BinaryWriter } = goog.requireType('jspb.binary.writer');
15+
const {Map: JspbMap} = goog.requireType('jspb.Map');
16+
17+
/**
18+
* Write this Map field in wire format to a BinaryWriter, using the given
19+
* field number.
20+
* @param {?JspbMap<K,V>} map
21+
* @param {number} fieldNumber
22+
* @param {!BinaryWriter} writer
23+
* @param {function(this:BinaryWriter,number,K_OR_NULL)} keyWriterFn
24+
* The method on BinaryWriter that writes type K to the stream.
25+
* @param {function(this:BinaryWriter,number,V,?=)|
26+
* function(this:BinaryWriter,number,V,?)} valueWriterFn
27+
* The method on BinaryWriter that writes type V to the stream. May be
28+
* writeMessage, in which case the second callback arg form is used.
29+
* @param {function(V,!BinaryWriter)=} valueWriterCallback
30+
* The BinaryWriter serialization callback for type V, if V is a message
31+
* type.
32+
* @template K,V
33+
* Use go/closure-ttl to create a `K|null` type for the keyWriterFn argument
34+
* closure type inference will occasionally infer K based on the keyWriterFn
35+
* argument instead of the map argument which will cause type errors when they
36+
* don't match
37+
* @template K_OR_NULL := union(K, 'null') =:
38+
*/
39+
function serializeMapToBinary(
40+
map, fieldNumber, writer, keyWriterFn, valueWriterFn, valueWriterCallback) {
41+
if (!map) {
42+
return;
43+
}
44+
map.forEach((value, key) => {
45+
writer.writeMessage(
46+
fieldNumber, /* we need a non-null value to pass here */ map,
47+
(ignored, w) => {
48+
keyWriterFn.call(w, 1, key);
49+
valueWriterFn.call(w, 2, value, valueWriterCallback);
50+
});
51+
});
52+
}
53+
54+
/**
55+
* Read one key/value message from the given BinaryReader. Compatible as the
56+
* `reader` callback parameter to BinaryReader.readMessage, to be called
57+
* when a key/value pair submessage is encountered. If the Key is undefined,
58+
* we should default it to 0.
59+
* @template K, V
60+
* @param {!JspbMap<K,V>} map
61+
* @param {!BinaryReader} reader
62+
* @param {function(this:BinaryReader):K} keyReaderFn
63+
* The method on BinaryReader that reads type K from the stream.
64+
*
65+
* @param {K} defaultKey
66+
* The default value for the type of map keys. Accepting map entries with
67+
* unset keys is required for maps to be backwards compatible with the
68+
* repeated message representation described here: goo.gl/zuoLAC
69+
*
70+
* @param {function(this:BinaryReader):V|function(V,!BinaryReader)}
71+
* valueReaderFn
72+
* The method on BinaryReader that reads type V from the stream, or a
73+
* callback for readMessage.
74+
*
75+
* @param {V} defaultValue
76+
* The default value for the type of map values. Accepting map entries with
77+
* unset values is required for maps to be backwards compatible with the
78+
* repeated message representation described here: goo.gl/zuoLAC
79+
*/
80+
function deserializeMapFromBinary(
81+
map, reader, keyReaderFn, defaultKey, valueReaderFn, defaultValue) {
82+
reader.readMessage(map, (message, reader) => {
83+
let key = defaultKey;
84+
let value = defaultValue;
85+
86+
while (reader.nextField()) {
87+
if (reader.isEndGroup()) {
88+
break;
89+
}
90+
const field = reader.getFieldNumber();
91+
92+
if (field == 1) {
93+
// Key.
94+
key = keyReaderFn.call(reader);
95+
} else if (field == 2) {
96+
// Value.
97+
if (map.valueCtor) {
98+
reader.readMessage(value, valueReaderFn);
99+
} else {
100+
value = (/** @type {function(this:BinaryReader):?} */ (valueReaderFn))
101+
.call(reader);
102+
}
103+
}
104+
}
105+
106+
asserts.assert(key != undefined);
107+
asserts.assert(value != undefined);
108+
map.set(key, value);
109+
});
110+
}
111+
112+
exports = {deserializeMapFromBinary, serializeMapToBinary};

‎map.js

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -425,43 +425,6 @@ jspb.Map.prototype.has = function(key) {
425425
return (keyValue in this.map_);
426426
};
427427

428-
429-
/**
430-
* Write this Map field in wire format to a BinaryWriter, using the given field
431-
* number.
432-
* @param {number} fieldNumber
433-
* @param {!jspb.BinaryWriter} writer
434-
* @param {function(this:jspb.BinaryWriter,number,K)} keyWriterFn
435-
* The method on BinaryWriter that writes type K to the stream.
436-
* @param {function(this:jspb.BinaryWriter,number,V,?=)|
437-
* function(this:jspb.BinaryWriter,number,V,?)} valueWriterFn
438-
* The method on BinaryWriter that writes type V to the stream. May be
439-
* writeMessage, in which case the second callback arg form is used.
440-
* @param {function(V,!jspb.BinaryWriter)=} opt_valueWriterCallback
441-
* The BinaryWriter serialization callback for type V, if V is a message
442-
* type.
443-
* @export
444-
*/
445-
jspb.Map.prototype.serializeBinary = function(
446-
fieldNumber, writer, keyWriterFn, valueWriterFn, opt_valueWriterCallback) {
447-
var strKeys = this.stringKeys_();
448-
strKeys.sort();
449-
for (var i = 0; i < strKeys.length; i++) {
450-
var entry = this.map_[strKeys[i]];
451-
writer.beginSubMessage(fieldNumber);
452-
keyWriterFn.call(writer, 1, entry.key);
453-
if (this.valueCtor_) {
454-
valueWriterFn.call(writer, 2, this.wrapEntry_(entry),
455-
opt_valueWriterCallback);
456-
} else {
457-
/** @type {function(this:jspb.BinaryWriter,number,?)} */ (valueWriterFn)
458-
.call(writer, 2, entry.value);
459-
}
460-
writer.endSubMessage();
461-
}
462-
};
463-
464-
465428
/**
466429
* Read one key/value message from the given BinaryReader. Compatible as the
467430
* `reader` callback parameter to jspb.BinaryReader.readMessage, to be called

0 commit comments

Comments
 (0)