Skip to content

Commit

Permalink
Support unknown fields in clone (#758)
Browse files Browse the repository at this point in the history
Support unknown fields in the method `Message.clone()`. 

Extensions are stored as unknown fields on a message, and cloning a
message will currently discard any set extension. This behavior is
unexpected, since it would be logical that a deep copy includes all
properties that were explicitly set, whether it is a regular field, or
an extension.

This PR fixes this flaw, and includes unknown fields when cloning a
message.

This also documents equal's behavior of ignoring the unknown fields and
extensions.
  • Loading branch information
srikrsna-buf authored Mar 21, 2024
1 parent a0ba45a commit 503e2f7
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/protobuf-bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ server would usually do.

| code generator | bundle size | minified | compressed |
|---------------------|------------------------:|-----------------------:|-------------------:|
| protobuf-es | 97,510 b | 41,660 b | 10,845 b |
| protobuf-es | 97,734 b | 41,763 b | 10,857 b |
| protobuf-javascript | 394,384 b | 288,654 b | 45,122 b |
23 changes: 22 additions & 1 deletion packages/protobuf-test/src/clone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import { describe, expect } from "@jest/globals";
import { MessageFieldMessage as TS_MessageFieldMessage } from "./gen/ts/extra/msg-message_pb.js";
import { MessageFieldMessage as JS_MessageFieldMessage } from "./gen/js/extra/msg-message_pb.js";
import { Empty as TS_Empty } from "./gen/ts/google/protobuf/empty_pb.js";
import { Empty as JS_Empty } from "./gen/js/google/protobuf/empty_pb.js";
import {
RepeatedScalarValuesMessage as TS_RepeatedScalarValuesMessage,
ScalarValuesMessage as TS_ScalarValuesMessage,
Expand All @@ -26,7 +28,7 @@ import {
import { WrappersMessage as TS_WrappersMessage } from "./gen/ts/extra/wkt-wrappers_pb.js";
import { WrappersMessage as JS_WrappersMessage } from "./gen/js/extra/wkt-wrappers_pb.js";
import { testMT } from "./helpers.js";
import { BoolValue, protoInt64 } from "@bufbuild/protobuf";
import { BoolValue, WireType, protoInt64 } from "@bufbuild/protobuf";

/* eslint-disable @typescript-eslint/ban-ts-comment */

Expand Down Expand Up @@ -146,4 +148,23 @@ describe("clone", function () {
a.doubleValueField = 0.1;
expect(b.doubleValueField).not.toBe(a.doubleValueField);
});
// We are using Empty wkt to test unknown fields.
testMT({ ts: TS_Empty, js: JS_Empty }, (messageType) => {
const a = new messageType();
const bin = messageType.runtime.bin;
const unknownFields = [
{ no: 1, wireType: WireType.Bit32, data: new Uint8Array([1, 2, 3, 4]) },
];
for (const unknowField of unknownFields) {
bin.onUnknownField(
a,
unknowField.no,
unknowField.wireType,
unknowField.data,
);
}
const b = a.clone();
expect(b).toStrictEqual(a);
expect(bin.listUnknownFields(b)).toStrictEqual(unknownFields);
});
});
1 change: 1 addition & 0 deletions packages/protobuf/src/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface AnyMessage extends Message<AnyMessage> {
export class Message<T extends Message<T> = AnyMessage> {
/**
* Compare with a message of the same type.
* Note that this function disregards extensions and unknown fields.
*/
equals(other: T | PlainMessage<T> | undefined | null): boolean {
return this.getType().runtime.util.equals(
Expand Down
3 changes: 3 additions & 0 deletions packages/protobuf/src/private/util-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
}
any[member.localName] = copy;
}
for (const uf of type.runtime.bin.listUnknownFields(message)) {
type.runtime.bin.onUnknownField(any, uf.no, uf.wireType, uf.data);
}
return target;
},
};
Expand Down

0 comments on commit 503e2f7

Please sign in to comment.