Skip to content

Commit

Permalink
Clarify map behavior with message_encoding = DELIMITED
Browse files Browse the repository at this point in the history
  • Loading branch information
timostamm committed May 3, 2024
1 parent d80f40d commit fea4d8d
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 23 deletions.
7 changes: 5 additions & 2 deletions packages/protobuf-test/extra/edition2023-map-encoding.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ option features.message_encoding = DELIMITED;

// Map fields are syntactic sugar for a repeated message field with field 1 for
// key and field 2 for value. Despite that, the file feature message_encoding =
// DELIMITED should NOT apply to this "synthetic" message.
// DELIMITED should NOT apply to this "synthetic" message, and it should also
// not apply to map message values.
message Edition2023MapEncodingMessage {
map<int32, bool> map_field = 77;
map<int32, string> string_map = 77;
map<int32, Child> message_map = 88;
message Child {}
}
67 changes: 55 additions & 12 deletions packages/protobuf-test/src/edition2023-serialize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import {
import * as edition2023_ts from "./gen/ts/extra/edition2023_pb.js";
import * as edition2023_proto2_ts from "./gen/ts/extra/edition2023-proto2_pb.js";
import * as edition2023_proto3_ts from "./gen/ts/extra/edition2023-proto3_pb.js";
import { Edition2023MapEncodingMessageDesc } from "./gen/ts/extra/edition2023-map-encoding_pb.js";
import {
Edition2023MapEncodingMessage_ChildDesc,
Edition2023MapEncodingMessageDesc,
} from "./gen/ts/extra/edition2023-map-encoding_pb.js";
import { BinaryReader, BinaryWriter, WireType } from "@bufbuild/protobuf/wire";

describe("edition2023 serialization", () => {
Expand Down Expand Up @@ -213,26 +216,40 @@ describe("edition2023 serialization", () => {
describe("message_encoding DELIMITED with maps", () => {
test("should round-trip", () => {
const a = create(Edition2023MapEncodingMessageDesc);
a.mapField[123] = true;
a.stringMap[123] = "abc";
const bytes = toBinary(Edition2023MapEncodingMessageDesc, a);
const b = fromBinary(Edition2023MapEncodingMessageDesc, bytes);
expect(b).toStrictEqual(a);
});
test("should expect LENGTH_PREFIXED", () => {
test("should expect LENGTH_PREFIXED map entry", () => {
const w = new BinaryWriter();
w.tag(77, WireType.LengthDelimited);
w.uint32(4);
w.fork();
w.tag(1, WireType.Varint).int32(123);
w.tag(2, WireType.Varint).string("abc");
w.join();
const bytes = w.finish();
const msg = fromBinary(Edition2023MapEncodingMessageDesc, bytes);
expect(msg.stringMap).toStrictEqual({
123: "abc",
});
});
test("should expect LENGTH_PREFIXED map value message", () => {
const w = new BinaryWriter();
w.tag(88, WireType.LengthDelimited);
w.fork();
w.tag(1, WireType.Varint).int32(123);
w.tag(2, WireType.Varint).bool(true);
w.tag(2, WireType.LengthDelimited).fork().join();
w.join();
const bytes = w.finish();
const msg = fromBinary(Edition2023MapEncodingMessageDesc, bytes);
expect(msg.mapField).toStrictEqual({
123: true,
expect(msg.messageMap).toStrictEqual({
123: create(Edition2023MapEncodingMessage_ChildDesc),
});
});
test("should serialize LENGTH_PREFIXED", () => {
test("should serialize map entry LENGTH_PREFIXED", () => {
const msg = create(Edition2023MapEncodingMessageDesc);
msg.mapField[123] = true;
msg.stringMap[123] = "abc";
const bytes = toBinary(Edition2023MapEncodingMessageDesc, msg);
const r = new BinaryReader(bytes);
{
Expand All @@ -242,17 +259,43 @@ describe("edition2023 serialization", () => {
const length = r.uint32();
expect(length).toBe(r.len - r.pos);
}
{
const [number] = r.tag();
expect(number).toBe(1);
expect(r.int32()).toBe(123);
}
{
const [number] = r.tag();
expect(number).toBe(2);
expect(r.string()).toBe("abc");
}
{
expect(r.pos).toBe(r.len);
}
});
test("should serialize map value message LENGTH_PREFIXED", () => {
const msg = create(Edition2023MapEncodingMessageDesc);
msg.messageMap[123] = create(Edition2023MapEncodingMessage_ChildDesc);
const bytes = toBinary(Edition2023MapEncodingMessageDesc, msg);
const r = new BinaryReader(bytes);
{
const [number, wireType] = r.tag();
expect(number).toBe(88);
expect(wireType).toBe(WireType.LengthDelimited);
const length = r.uint32();
expect(length).toBe(r.len - r.pos);
}
{
const [number] = r.tag();
expect(number).toBe(1);
expect(wireType).toBe(WireType.Varint);
expect(r.int32()).toBe(123);
}
{
const [number, wireType] = r.tag();
expect(number).toBe(2);
expect(wireType).toBe(WireType.Varint);
expect(r.bool()).toBe(true);
expect(wireType).toBe(WireType.LengthDelimited);
const length = r.uint32();
expect(length).toBe(0);
}
{
expect(r.pos).toBe(r.len);
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/protobuf/src/desc-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,8 @@ type descFieldMapCommon<T extends ScalarType = ScalarType> = T extends Exclude<S
readonly oneof: undefined;
/**
* Encode the map entry message delimited (a.k.a. proto2 group encoding),
* or length-prefixed? This is always false for map fields.
* or length-prefixed? As of Edition 2023, this is always false for map fields,
* and also applies to map values, if they are messages.
*/
readonly delimitedEncoding: false;
} : never;
Expand Down

0 comments on commit fea4d8d

Please sign in to comment.