From 0abb89c9cc195aafb66d8608812542efe4fb41de Mon Sep 17 00:00:00 2001 From: Sam Nosenzo Date: Fri, 22 Mar 2024 12:31:53 -0400 Subject: [PATCH 1/4] defaults for non-optional mutable members --- .../src/DeserializationInfoCache.test.ts | 103 ++++++++++- .../src/DeserializationInfoCache.ts | 160 ++++++++++++++++- .../src/MessageReader.test.ts | 168 ++++++++++++++++-- .../omgidl-serialization/src/MessageReader.ts | 27 +-- .../omgidl-serialization/src/defaultValues.ts | 4 + 5 files changed, 427 insertions(+), 35 deletions(-) create mode 100644 packages/omgidl-serialization/src/defaultValues.ts diff --git a/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts b/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts index 7e1c075..0a5ecd6 100644 --- a/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts +++ b/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts @@ -1,10 +1,12 @@ -import { IDLMessageDefinition } from "@foxglove/omgidl-parser"; +import { IDLMessageDefinition, IDLMessageDefinitionField } from "@foxglove/omgidl-parser"; import { DeserializationInfoCache, + FieldDeserializationInfo, PRIMITIVE_ARRAY_DESERIALIZERS, PRIMITIVE_DESERIALIZERS, } from "./DeserializationInfoCache"; +import { UNION_DISCRIMINATOR_PROPERTY_KEY } from "./constants"; const TRANSFORM_DEFINITION: IDLMessageDefinition = { name: "geometry_msgs::msg::Transform", @@ -51,6 +53,16 @@ const FLOAT64_PRIMITIVE_DESER_INFO = { }, }; +function makeFieldDeserFromComplexDef( + complexDefinition: IDLMessageDefinition, + deserializationInfoCache: DeserializationInfoCache, +): FieldDeserializationInfo { + return deserializationInfoCache.buildFieldDeserInfo({ + name: `${complexDefinition.name ?? ""}_field`, + type: complexDefinition.name ?? "", + isComplex: true, + } as IDLMessageDefinitionField); +} describe("DeserializationInfoCache", () => { it("creates deserialization info for struct with primitive fields", () => { const deserializationInfoCache = new DeserializationInfoCache([TIME_DEFINITION]); @@ -239,7 +251,96 @@ describe("DeserializationInfoCache", () => { }, }); }); + it("creates default value for struct with primitive fields", () => { + const deserializationInfoCache = new DeserializationInfoCache([TIME_DEFINITION]); + const fieldDeserInfo = makeFieldDeserFromComplexDef(TIME_DEFINITION, deserializationInfoCache); + expect(deserializationInfoCache.getFieldDefault(fieldDeserInfo)).toMatchObject({ + sec: 0, + nanosec: 0, + }); + }); + + it("creates default value for struct with complex fields", () => { + const deserializationInfoCache = new DeserializationInfoCache([ + TRANSFORM_DEFINITION, + VECTOR_DEFINITION, + QUATERNION_DEFINITION, + ]); + const fieldDeserInfo = makeFieldDeserFromComplexDef( + TRANSFORM_DEFINITION, + deserializationInfoCache, + ); + expect(deserializationInfoCache.getFieldDefault(fieldDeserInfo)).toMatchObject({ + translation: { x: 0, y: 0, z: 0 }, + rotation: { x: 0, y: 0, z: 0, w: 0 }, + }); + }); + it("creates default value for primitive field", () => { + const deserializationInfoCache = new DeserializationInfoCache([]); + const fieldDeserInfo = deserializationInfoCache.buildFieldDeserInfo({ + name: "some_field_name", + type: "float64", + }); + expect(deserializationInfoCache.getFieldDefault(fieldDeserInfo)).toEqual(0); + }); + it("creates correct default for a complex array field", () => { + const deserializationInfoCache = new DeserializationInfoCache([VECTOR_DEFINITION]); + const vectorFieldDeserInfo = deserializationInfoCache.buildFieldDeserInfo({ + isComplex: true, + isArray: true, + name: "vectors", + type: "geometry_msgs::msg::Vector3", + }); + expect(deserializationInfoCache.getFieldDefault(vectorFieldDeserInfo)).toEqual([]); + }); + it("creates correct default for a union field with a default case", () => { + const unionDefinition: IDLMessageDefinition = { + name: "test::Union", + aggregatedKind: "union", + switchType: "uint32", + cases: [ + { + predicates: [0], + type: { name: "a", type: "int32", isComplex: false }, + }, + { + predicates: [1], + type: { name: "b", type: "float64", isComplex: false }, + }, + ], + defaultCase: { name: "c", type: "string", isComplex: false }, + }; + const deserializationInfoCache = new DeserializationInfoCache([unionDefinition]); + const fieldDeserInfo = makeFieldDeserFromComplexDef(unionDefinition, deserializationInfoCache); + expect(deserializationInfoCache.getFieldDefault(fieldDeserInfo)).toEqual({ + [UNION_DISCRIMINATOR_PROPERTY_KEY]: undefined, + c: "", + }); + }); + it("creates correct default for a union field without a default case", () => { + const unionDefinition: IDLMessageDefinition = { + name: "test::Union", + aggregatedKind: "union", + switchType: "uint32", + cases: [ + { + predicates: [1], + type: { name: "a", type: "uint8", isComplex: false }, + }, + { + predicates: [0], // default case because default value for switch case is 0 + type: { name: "b", type: "float64", isComplex: false }, + }, + ], + }; + const deserializationInfoCache = new DeserializationInfoCache([unionDefinition]); + const fieldDeserInfo = makeFieldDeserFromComplexDef(unionDefinition, deserializationInfoCache); + expect(deserializationInfoCache.getFieldDefault(fieldDeserInfo)).toEqual({ + [UNION_DISCRIMINATOR_PROPERTY_KEY]: 0, + b: 0, + }); + }); it("throws if required type definitions are not found", () => { const deserializationInfoCache = new DeserializationInfoCache([TIME_DEFINITION]); expect(() => diff --git a/packages/omgidl-serialization/src/DeserializationInfoCache.ts b/packages/omgidl-serialization/src/DeserializationInfoCache.ts index 87ad6ac..f8042c1 100644 --- a/packages/omgidl-serialization/src/DeserializationInfoCache.ts +++ b/packages/omgidl-serialization/src/DeserializationInfoCache.ts @@ -2,9 +2,18 @@ import { CdrReader } from "@foxglove/cdr"; import { IDLMessageDefinition, IDLMessageDefinitionField, + IDLStructDefinition, IDLUnionDefinition, } from "@foxglove/omgidl-parser"; +import { UNION_DISCRIMINATOR_PROPERTY_KEY } from "./constants"; +import { + DEFAULT_BOOLEAN_VALUE, + DEFAULT_BYTE_VALUE, + DEFAULT_NUMERICAL_VALUE, + DEFAULT_STRING_VALUE, +} from "./defaultValues"; + export type Deserializer = ( reader: CdrReader, /** @@ -51,6 +60,7 @@ export type PrimitiveArrayDeserializationInfo = { export type StructDeserializationInfo = HeaderOptions & { type: "struct"; fields: FieldDeserializationInfo[]; + definition: IDLStructDefinition; }; export type UnionDeserializationInfo = HeaderOptions & { @@ -65,13 +75,23 @@ export type PrimitiveTypeDeserInfo = | PrimitiveDeserializationInfo | PrimitiveArrayDeserializationInfo; -export type FieldDeserializationInfo = { +export type FieldDeserializationInfo< + DeserInfo extends ComplexDeserializationInfo | PrimitiveTypeDeserInfo = + | ComplexDeserializationInfo + | PrimitiveTypeDeserInfo, +> = { name: string; type: string; - typeDeserInfo: ComplexDeserializationInfo | PrimitiveTypeDeserInfo; + typeDeserInfo: DeserInfo; isArray?: boolean; arrayLengths?: number[]; definitionId?: number; + /** Optional fields show undefined if not present in the message. + * Non-optional fields show a default value per the spec: + * https://www.omg.org/spec/DDS-XTypes/1.3/PDF 7.2.2.4.4.4.7 + */ + isOptional: boolean; + isComplex: DeserInfo extends ComplexDeserializationInfo ? true : false; }; export class DeserializationInfoCache { @@ -131,6 +151,7 @@ export class DeserializationInfoCache { const deserInfo: StructDeserializationInfo = { type: "struct", ...getHeaderNeeds(definition), + definition, fields: definition.definitions.reduce( (fieldsAccum, fieldDef) => fieldDef.isConstant === true @@ -170,6 +191,8 @@ export class DeserializationInfoCache { isArray, arrayLengths, definitionId: getDefinitionId(definition), + isOptional: isOptional(definition), + isComplex: true, }; } @@ -209,11 +232,97 @@ export class DeserializationInfoCache { name, type, typeDeserInfo: fieldDeserInfo, + isComplex: false, isArray, arrayLengths, definitionId: getDefinitionId(definition), + isOptional: isOptional(definition), }; } + + public getFieldDefault(deserInfo: FieldDeserializationInfo): unknown { + const { isArray, arrayLengths, type, isComplex } = deserInfo; + + if (isArray === true && arrayLengths == undefined) { + return []; + } + let defaultValueGetter; + if (isComplex) { + defaultValueGetter = this.#createOrGetComplexDefaultValueGetter( + deserInfo.typeDeserInfo as ComplexDeserializationInfo, + ); + } else { + // fixed length arrays are filled with default values + defaultValueGetter = PRIMITIVE_DEFAULT_VALUE_GETTERS.get(type); + if (!defaultValueGetter) { + throw new Error(`Failed to find default value getter for type ${type}`); + } + } + // Used for fixed length arrays that may be nested + const needsNestedArray = isArray === true && arrayLengths != undefined; + return needsNestedArray + ? makeNestedArray(defaultValueGetter, arrayLengths, 0) + : defaultValueGetter(); + } + + #complexDefaultValueGetters = new Map Record>(); + + #createOrGetComplexDefaultValueGetter = ( + deserInfo: ComplexDeserializationInfo, + ): (() => Record) => { + const mapKey = deserInfo.definition.name ?? ""; + + if (this.#complexDefaultValueGetters.has(mapKey)) { + return this.#complexDefaultValueGetters.get(mapKey)!; + } + let defaultMessage: Record | undefined = undefined; + const defaultValueGetter: () => Record = () => { + // if `structuredClone` is part of the environment, use it to clone the default message + // want to avoid defaultValues having references to the same object + if (defaultMessage != undefined && typeof structuredClone !== "undefined") { + return structuredClone(defaultMessage); + } + if (deserInfo.type === "union") { + const { definition: unionDef } = deserInfo; + const { switchType } = unionDef; + + defaultMessage = {}; + let defaultCase: IDLMessageDefinitionField | undefined = unionDef.defaultCase; + // use existing default case if there is one + if (!defaultCase) { + // choose default based off of default value of switch case type + const switchTypeDefaultGetter = PRIMITIVE_DEFAULT_VALUE_GETTERS.get(switchType); + if (switchTypeDefaultGetter == undefined) { + throw new Error(`Failed to find default value getter for type ${switchType}`); + } + const switchValue = switchTypeDefaultGetter() as number | boolean; + + defaultCase = unionDef.cases.find((c) => c.predicates.includes(switchValue))?.type; + if (!defaultCase) { + throw new Error(`Failed to find default case for union ${unionDef.name ?? ""}`); + } + defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = switchValue; + } else { + // default exists, default value of switch case type is not needed + defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = undefined; + } + const defaultCaseDeserInfo = this.buildFieldDeserInfo(defaultCase); + defaultMessage[defaultCaseDeserInfo.name] = this.getFieldDefault(defaultCaseDeserInfo); + } else if (deserInfo.type === "struct") { + defaultMessage = {}; + for (const field of deserInfo.fields) { + defaultMessage[field.name] = this.getFieldDefault(field); + } + } + + if (defaultMessage == undefined) { + throw new Error(`Unrecognized complex type ${deserInfo.type as string}`); + } + return defaultMessage; + }; + this.#complexDefaultValueGetters.set(deserInfo.definition.name ?? "", defaultValueGetter); + return defaultValueGetter; + }; } function typeToByteLength(type: string): number | undefined { @@ -269,6 +378,43 @@ export const PRIMITIVE_ARRAY_DESERIALIZERS = new Map( ["string", readStringArray], ]); +export const PRIMITIVE_DEFAULT_VALUE_GETTERS = new Map unknown>([ + ["bool", () => DEFAULT_BOOLEAN_VALUE], + ["int8", () => DEFAULT_BYTE_VALUE], + ["uint8", () => DEFAULT_BYTE_VALUE], + ["int16", () => DEFAULT_NUMERICAL_VALUE], + ["uint16", () => DEFAULT_NUMERICAL_VALUE], + ["int32", () => DEFAULT_NUMERICAL_VALUE], + ["uint32", () => DEFAULT_NUMERICAL_VALUE], + ["int64", () => DEFAULT_NUMERICAL_VALUE], + ["uint64", () => DEFAULT_NUMERICAL_VALUE], + ["float32", () => DEFAULT_NUMERICAL_VALUE], + ["float64", () => DEFAULT_NUMERICAL_VALUE], + ["string", () => DEFAULT_STRING_VALUE], +]); + +export function makeNestedArray( + getValue: () => unknown, + arrayLengths: number[], + depth: number, +): unknown[] { + if (depth > arrayLengths.length - 1 || depth < 0) { + throw Error(`Invalid depth ${depth} for array of length ${arrayLengths.length}`); + } + + const array = []; + + for (let i = 0; i < arrayLengths[depth]!; i++) { + if (depth === arrayLengths.length - 1) { + array.push(getValue()); + } else { + array.push(makeNestedArray(getValue, arrayLengths, depth + 1)); + } + } + + return array; +} + function readBoolArray(reader: CdrReader, count: number): boolean[] { const array = new Array(count); for (let i = 0; i < count; i++) { @@ -285,6 +431,16 @@ function readStringArray(reader: CdrReader, count: number): string[] { return array; } +function isOptional(definition: IDLMessageDefinitionField): boolean { + const { annotations } = definition; + + if (!annotations) { + return false; + } + + return "optional" in annotations; +} + function getHeaderNeeds(definition: IDLMessageDefinition): { usesDelimiterHeader: boolean; usesMemberHeader: boolean; diff --git a/packages/omgidl-serialization/src/MessageReader.test.ts b/packages/omgidl-serialization/src/MessageReader.test.ts index 32f3270..07ddfc3 100644 --- a/packages/omgidl-serialization/src/MessageReader.test.ts +++ b/packages/omgidl-serialization/src/MessageReader.test.ts @@ -1013,7 +1013,7 @@ module builtin_interfaces { `; const data = { bittybyte: 5, - inner: undefined, + inner: undefined, // optional fields are populated with undefined bytier: 9, }; const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR_LE }); @@ -1031,11 +1031,11 @@ module builtin_interfaces { expect(msgout).toEqual(data); }); - it("Reads mutable struct with inner struct in middle with absent member", () => { + it("Reads mutable struct with inner struct in middle with absent optional member", () => { const msgDef = ` @mutable struct InnerMessage { - @id(100) float floaty; + @optional @id(100) float floaty; }; @mutable struct Message { @@ -1047,7 +1047,7 @@ module builtin_interfaces { const data = { bittybyte: 5, inner: { - floaty: undefined, + floaty: undefined, // optional fields are populated with undefined }, bytier: 9, }; @@ -1068,11 +1068,11 @@ module builtin_interfaces { expect(msgout).toEqual(data); }); - it("Reads mutable struct with inner struct last with absent member", () => { + it("Reads mutable struct with inner struct last with absent optional member", () => { const msgDef = ` @mutable struct InnerMessage { - @id(100) float floaty; + @optional @id(100) float floaty; }; @mutable struct Message { @@ -1085,7 +1085,7 @@ module builtin_interfaces { bittybyte: 5, bytier: 9, inner: { - floaty: undefined, + floaty: undefined, // optional fields are populated with undefined }, }; const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR_LE }); @@ -1109,7 +1109,7 @@ module builtin_interfaces { const msgDef = ` @mutable struct InnerMessage { - @id(100) float floaty; + @optional @id(100) float floaty; }; @mutable struct Message { @@ -1120,7 +1120,7 @@ module builtin_interfaces { `; const data = { bittybyte: 5, - bytier: undefined, + bytier: undefined, // optional fields are populated with undefined inner: { floaty: undefined, }, @@ -1140,11 +1140,11 @@ module builtin_interfaces { expect(msgout).toEqual(data); }); - it("Reads mutable struct with absent field before inner struct member with empty field that isn't the last populated field", () => { + it("Reads mutable struct with absent field before inner struct optional member with empty field that isn't the last populated field", () => { const msgDef = ` @mutable struct InnerMessage { - @id(100) float floaty; + @optional @id(100) float floaty; }; @mutable struct Message { @@ -1154,7 +1154,7 @@ module builtin_interfaces { }; `; const data = { - bittybyte: undefined, + bittybyte: undefined, // optional fields are populated with undefined inner: { floaty: undefined, }, @@ -1173,6 +1173,150 @@ module builtin_interfaces { const msgout = reader.readMessage(writer.data); + expect(msgout).toEqual(data); + }); + it("Reads mutable struct with inner struct in middle with absent non-optional member", () => { + const msgDef = ` + @mutable + struct InnerMessage { + @id(100) float floaty; + }; + @mutable + struct Message { + @id(100) uint8 bittybyte; + @id(200) InnerMessage inner; + @id(300) uint32 bytier; + }; + `; + const data = { + bittybyte: 5, + inner: { + floaty: 0, + }, + bytier: 9, + }; + const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR_LE }); + writer.emHeader(false, 100, 1, 0); + writer.uint32(data.bittybyte); + writer.emHeader(false, 200, 4, 0); // size 4 because it should include sentinel header + writer.sentinelHeader(); // end of inner struct + writer.emHeader(false, 300, 4, 0); + writer.uint32(data.bytier); + writer.sentinelHeader(); // end of struct + + const rootDef = "Message"; + + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + const msgout = reader.readMessage(writer.data); + + expect(msgout).toEqual(data); + }); + it("Reads mutable struct with inner struct last with absent non-optional member", () => { + const msgDef = ` + @mutable + struct InnerMessage { + @id(100) float floaty; + }; + @mutable + struct Message { + @id(100) uint8 bittybyte; + @id(200) uint32 bytier; + @id(300) InnerMessage inner; + }; + `; + const data = { + bittybyte: 5, + bytier: 9, + inner: { + floaty: 0, // non-optional fields populated with defaults + }, + }; + const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR_LE }); + writer.emHeader(false, 100, 1, 0); + writer.uint32(data.bittybyte); + writer.emHeader(false, 200, 4, 0); + writer.uint32(data.bytier); + writer.emHeader(false, 300, 4, 0); // size 4 because it should include sentinel header + writer.sentinelHeader(); // end of inner struct + writer.sentinelHeader(); // end of struct + + const rootDef = "Message"; + + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + const msgout = reader.readMessage(writer.data); + + expect(msgout).toEqual(data); + }); + it("Reads mutable struct with absent field before inner struct non-optional member with empty field", () => { + const msgDef = ` + @mutable + struct InnerMessage { + @id(100) float floaty; + }; + @mutable + struct Message { + @id(100) uint8 bittybyte; + @id(200) uint32 bytier; + @id(300) InnerMessage inner; + }; + `; + const data = { + bittybyte: 5, + bytier: 0, // non-optional fields populated with defaults + inner: { + floaty: 0, + }, + }; + const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR_LE }); + writer.emHeader(false, 100, 1, 0); + writer.uint32(data.bittybyte); + writer.emHeader(false, 300, 4, 0); // size 4 because it should include sentinel header + writer.sentinelHeader(); // end of inner struct + writer.sentinelHeader(); // end of struct + + const rootDef = "Message"; + + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + const msgout = reader.readMessage(writer.data); + + expect(msgout).toEqual(data); + }); + it("Reads mutable struct with absent non-optional field before inner struct member with empty field that isn't the last populated field", () => { + const msgDef = ` + @mutable + struct InnerMessage { + @id(100) float floaty; + }; + @mutable + struct Message { + @id(100) uint8 bittybyte; + @id(200) InnerMessage inner; + @id(300) uint32 bytier; + }; + `; + const data = { + bittybyte: 0, // non-optional fields populated with defaults + inner: { + floaty: 0, + }, + bytier: 24, + }; + const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR_LE }); + writer.emHeader(false, 200, 4, 0); // size 4 because it should include sentinel header + writer.sentinelHeader(); // end of inner struct + writer.emHeader(false, 300, 4, 0); + writer.uint32(data.bytier); + writer.sentinelHeader(); // end of struct + + const rootDef = "Message"; + + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + const msgout = reader.readMessage(writer.data); + expect(msgout).toEqual(data); }); }); diff --git a/packages/omgidl-serialization/src/MessageReader.ts b/packages/omgidl-serialization/src/MessageReader.ts index eba9097..0f75ecb 100644 --- a/packages/omgidl-serialization/src/MessageReader.ts +++ b/packages/omgidl-serialization/src/MessageReader.ts @@ -12,6 +12,7 @@ import { HeaderOptions, StructDeserializationInfo, UnionDeserializationInfo, + makeNestedArray, } from "./DeserializationInfoCache"; import { UNION_DISCRIMINATOR_PROPERTY_KEY } from "./constants"; @@ -171,7 +172,11 @@ export class MessageReader { if (definitionId != undefined && emHeader.id !== definitionId) { // ID mismatch, save emHeader for next field. Could also be a sentinel header this.#unusedEmHeader = emHeader; - return undefined; + if (field.isOptional) { + return undefined; + } else { + return this.deserializationInfoCache.getFieldDefault(field); + } } // emHeader is now being used and should be cleared @@ -222,7 +227,7 @@ export class MessageReader { }; // last arrayLengths length is handled in deserializer. It returns an array - return readNestedArray(typedArrayDeserializer, arrayLengths.slice(0, -1), 0); + return makeNestedArray(typedArrayDeserializer, arrayLengths.slice(0, -1), 0); } else { return deser(reader, arrayLengths[0]!); } @@ -263,24 +268,6 @@ export class MessageReader { } } -function readNestedArray(deser: () => unknown, arrayLengths: number[], depth: number): unknown[] { - if (depth > arrayLengths.length - 1 || depth < 0) { - throw Error(`Invalid depth ${depth} for array of length ${arrayLengths.length}`); - } - - const array = []; - - for (let i = 0; i < arrayLengths[depth]!; i++) { - if (depth === arrayLengths.length - 1) { - array.push(deser()); - } else { - array.push(readNestedArray(deser, arrayLengths, depth + 1)); - } - } - - return array; -} - function getCaseForDiscriminator( unionDef: IDLUnionDefinition, discriminatorValue: number | boolean, diff --git a/packages/omgidl-serialization/src/defaultValues.ts b/packages/omgidl-serialization/src/defaultValues.ts new file mode 100644 index 0000000..ff6c76a --- /dev/null +++ b/packages/omgidl-serialization/src/defaultValues.ts @@ -0,0 +1,4 @@ +export const DEFAULT_BOOLEAN_VALUE = false; +export const DEFAULT_STRING_VALUE = ""; +export const DEFAULT_NUMERICAL_VALUE = 0; +export const DEFAULT_BYTE_VALUE = 0x00; From e86f52d4ccef993b508217000221672811e9009e Mon Sep 17 00:00:00 2001 From: Sam Nosenzo Date: Fri, 22 Mar 2024 12:48:49 -0400 Subject: [PATCH 2/4] properly creates default for struct with optional and non-optional values --- .../src/DeserializationInfoCache.test.ts | 23 +++++++++++++++++++ .../src/DeserializationInfoCache.ts | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts b/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts index 0a5ecd6..a669bed 100644 --- a/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts +++ b/packages/omgidl-serialization/src/DeserializationInfoCache.test.ts @@ -341,6 +341,29 @@ describe("DeserializationInfoCache", () => { b: 0, }); }); + it("creates correct default for a struct field with optional and non-optional members", () => { + const unionDefinition: IDLMessageDefinition = { + name: "test::Message", + aggregatedKind: "struct", + definitions: [ + { name: "a", type: "uint32", isComplex: false }, + { + name: "b", + type: "uint32", + isComplex: false, + annotations: { + optional: { type: "no-params", name: "optional" }, + }, + }, + ], + }; + const deserializationInfoCache = new DeserializationInfoCache([unionDefinition]); + const fieldDeserInfo = makeFieldDeserFromComplexDef(unionDefinition, deserializationInfoCache); + expect(deserializationInfoCache.getFieldDefault(fieldDeserInfo)).toEqual({ + a: 0, + b: undefined, + }); + }); it("throws if required type definitions are not found", () => { const deserializationInfoCache = new DeserializationInfoCache([TIME_DEFINITION]); expect(() => diff --git a/packages/omgidl-serialization/src/DeserializationInfoCache.ts b/packages/omgidl-serialization/src/DeserializationInfoCache.ts index f8042c1..8ab49af 100644 --- a/packages/omgidl-serialization/src/DeserializationInfoCache.ts +++ b/packages/omgidl-serialization/src/DeserializationInfoCache.ts @@ -311,7 +311,9 @@ export class DeserializationInfoCache { } else if (deserInfo.type === "struct") { defaultMessage = {}; for (const field of deserInfo.fields) { - defaultMessage[field.name] = this.getFieldDefault(field); + if (!field.isOptional) { + defaultMessage[field.name] = this.getFieldDefault(field); + } } } From 681b62ca1e22b9e7adc07b8dcdd9ccdfa935527f Mon Sep 17 00:00:00 2001 From: Sam Nosenzo Date: Thu, 4 Apr 2024 16:34:57 -0400 Subject: [PATCH 3/4] defaultValue stored on deserialization info --- .../src/DeserializationInfoCache.ts | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/omgidl-serialization/src/DeserializationInfoCache.ts b/packages/omgidl-serialization/src/DeserializationInfoCache.ts index 8ab49af..634567a 100644 --- a/packages/omgidl-serialization/src/DeserializationInfoCache.ts +++ b/packages/omgidl-serialization/src/DeserializationInfoCache.ts @@ -61,6 +61,8 @@ export type StructDeserializationInfo = HeaderOptions & { type: "struct"; fields: FieldDeserializationInfo[]; definition: IDLStructDefinition; + /** optional allows for defaultValues to be calculated lazily */ + defaultValue?: Record; }; export type UnionDeserializationInfo = HeaderOptions & { @@ -68,6 +70,8 @@ export type UnionDeserializationInfo = HeaderOptions & { switchTypeDeser: Deserializer; switchTypeLength: number; definition: IDLUnionDefinition; + /** optional allows for defaultValues to be calculated lazily */ + defaultValue?: Record; }; export type ComplexDeserializationInfo = StructDeserializationInfo | UnionDeserializationInfo; @@ -92,6 +96,8 @@ export type FieldDeserializationInfo< */ isOptional: boolean; isComplex: DeserInfo extends ComplexDeserializationInfo ? true : false; + /** optional allows for defaultValues to be calculated lazily */ + defaultValue?: unknown; }; export class DeserializationInfoCache { @@ -240,11 +246,26 @@ export class DeserializationInfoCache { }; } + /** Returns default value for given FieldDeserializationInfo. + * If defaultValue is not defined on FieldDeserializationInfo, it will be calculated and set. + */ public getFieldDefault(deserInfo: FieldDeserializationInfo): unknown { - const { isArray, arrayLengths, type, isComplex } = deserInfo; + if (deserInfo.defaultValue != undefined) { + return deserInfo.defaultValue; + } + + return this.#updateFieldDeserInfoWithDefaultValue(deserInfo); + } + + #updateFieldDeserInfoWithDefaultValue(deserInfo: FieldDeserializationInfo): unknown { + const { isArray, arrayLengths, type, isComplex, defaultValue } = deserInfo; + if (defaultValue != undefined) { + return deserInfo.defaultValue; + } if (isArray === true && arrayLengths == undefined) { - return []; + deserInfo.defaultValue = []; + return deserInfo.defaultValue; } let defaultValueGetter; if (isComplex) { @@ -260,9 +281,10 @@ export class DeserializationInfoCache { } // Used for fixed length arrays that may be nested const needsNestedArray = isArray === true && arrayLengths != undefined; - return needsNestedArray + deserInfo.defaultValue = needsNestedArray ? makeNestedArray(defaultValueGetter, arrayLengths, 0) : defaultValueGetter(); + return deserInfo.defaultValue; } #complexDefaultValueGetters = new Map Record>(); @@ -272,21 +294,22 @@ export class DeserializationInfoCache { ): (() => Record) => { const mapKey = deserInfo.definition.name ?? ""; + // need to make sure all complexDefaultValueGetters for a given type are the same if (this.#complexDefaultValueGetters.has(mapKey)) { return this.#complexDefaultValueGetters.get(mapKey)!; } - let defaultMessage: Record | undefined = undefined; const defaultValueGetter: () => Record = () => { // if `structuredClone` is part of the environment, use it to clone the default message // want to avoid defaultValues having references to the same object - if (defaultMessage != undefined && typeof structuredClone !== "undefined") { - return structuredClone(defaultMessage); + if (deserInfo.defaultValue != undefined && typeof structuredClone !== "undefined") { + return structuredClone(deserInfo.defaultValue); } + deserInfo.defaultValue = {}; + const defaultMessage = deserInfo.defaultValue; if (deserInfo.type === "union") { const { definition: unionDef } = deserInfo; const { switchType } = unionDef; - defaultMessage = {}; let defaultCase: IDLMessageDefinitionField | undefined = unionDef.defaultCase; // use existing default case if there is one if (!defaultCase) { @@ -307,12 +330,12 @@ export class DeserializationInfoCache { defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = undefined; } const defaultCaseDeserInfo = this.buildFieldDeserInfo(defaultCase); - defaultMessage[defaultCaseDeserInfo.name] = this.getFieldDefault(defaultCaseDeserInfo); + defaultMessage[defaultCaseDeserInfo.name] = + this.#updateFieldDeserInfoWithDefaultValue(defaultCaseDeserInfo); } else if (deserInfo.type === "struct") { - defaultMessage = {}; for (const field of deserInfo.fields) { if (!field.isOptional) { - defaultMessage[field.name] = this.getFieldDefault(field); + defaultMessage[field.name] = this.#updateFieldDeserInfoWithDefaultValue(field); } } } From 1de749c38b25af3bee4f4c1e1fba6f54a66219b1 Mon Sep 17 00:00:00 2001 From: Sam Nosenzo Date: Fri, 5 Apr 2024 15:27:48 -0400 Subject: [PATCH 4/4] remove indirection in complex default value getters --- .../src/DeserializationInfoCache.ts | 115 +++++++----------- 1 file changed, 47 insertions(+), 68 deletions(-) diff --git a/packages/omgidl-serialization/src/DeserializationInfoCache.ts b/packages/omgidl-serialization/src/DeserializationInfoCache.ts index 634567a..9339581 100644 --- a/packages/omgidl-serialization/src/DeserializationInfoCache.ts +++ b/packages/omgidl-serialization/src/DeserializationInfoCache.ts @@ -253,15 +253,7 @@ export class DeserializationInfoCache { if (deserInfo.defaultValue != undefined) { return deserInfo.defaultValue; } - - return this.#updateFieldDeserInfoWithDefaultValue(deserInfo); - } - - #updateFieldDeserInfoWithDefaultValue(deserInfo: FieldDeserializationInfo): unknown { - const { isArray, arrayLengths, type, isComplex, defaultValue } = deserInfo; - if (defaultValue != undefined) { - return deserInfo.defaultValue; - } + const { isArray, arrayLengths, type, isComplex } = deserInfo; if (isArray === true && arrayLengths == undefined) { deserInfo.defaultValue = []; @@ -269,9 +261,11 @@ export class DeserializationInfoCache { } let defaultValueGetter; if (isComplex) { - defaultValueGetter = this.#createOrGetComplexDefaultValueGetter( - deserInfo.typeDeserInfo as ComplexDeserializationInfo, - ); + defaultValueGetter = () => { + return this.#getComplexDeserInfoDefault( + deserInfo.typeDeserInfo as ComplexDeserializationInfo, + ); + }; } else { // fixed length arrays are filled with default values defaultValueGetter = PRIMITIVE_DEFAULT_VALUE_GETTERS.get(type); @@ -287,67 +281,52 @@ export class DeserializationInfoCache { return deserInfo.defaultValue; } - #complexDefaultValueGetters = new Map Record>(); - - #createOrGetComplexDefaultValueGetter = ( - deserInfo: ComplexDeserializationInfo, - ): (() => Record) => { - const mapKey = deserInfo.definition.name ?? ""; - - // need to make sure all complexDefaultValueGetters for a given type are the same - if (this.#complexDefaultValueGetters.has(mapKey)) { - return this.#complexDefaultValueGetters.get(mapKey)!; + /** Computes and sets the default value on the complex deserialization info */ + #getComplexDeserInfoDefault(deserInfo: ComplexDeserializationInfo): Record { + // if `structuredClone` is part of the environment, use it to clone the default message + // want to avoid defaultValues having references to the same object + if (deserInfo.defaultValue != undefined && typeof structuredClone !== "undefined") { + return structuredClone(deserInfo.defaultValue); } - const defaultValueGetter: () => Record = () => { - // if `structuredClone` is part of the environment, use it to clone the default message - // want to avoid defaultValues having references to the same object - if (deserInfo.defaultValue != undefined && typeof structuredClone !== "undefined") { - return structuredClone(deserInfo.defaultValue); - } - deserInfo.defaultValue = {}; - const defaultMessage = deserInfo.defaultValue; - if (deserInfo.type === "union") { - const { definition: unionDef } = deserInfo; - const { switchType } = unionDef; - - let defaultCase: IDLMessageDefinitionField | undefined = unionDef.defaultCase; - // use existing default case if there is one - if (!defaultCase) { - // choose default based off of default value of switch case type - const switchTypeDefaultGetter = PRIMITIVE_DEFAULT_VALUE_GETTERS.get(switchType); - if (switchTypeDefaultGetter == undefined) { - throw new Error(`Failed to find default value getter for type ${switchType}`); - } - const switchValue = switchTypeDefaultGetter() as number | boolean; - - defaultCase = unionDef.cases.find((c) => c.predicates.includes(switchValue))?.type; - if (!defaultCase) { - throw new Error(`Failed to find default case for union ${unionDef.name ?? ""}`); - } - defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = switchValue; - } else { - // default exists, default value of switch case type is not needed - defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = undefined; + deserInfo.defaultValue = {}; + const defaultMessage = deserInfo.defaultValue; + if (deserInfo.type === "union") { + const { definition: unionDef } = deserInfo; + const { switchType } = unionDef; + + let defaultCase: IDLMessageDefinitionField | undefined = unionDef.defaultCase; + // use existing default case if there is one + if (!defaultCase) { + // choose default based off of default value of switch case type + const switchTypeDefaultGetter = PRIMITIVE_DEFAULT_VALUE_GETTERS.get(switchType); + if (switchTypeDefaultGetter == undefined) { + throw new Error(`Failed to find default value getter for type ${switchType}`); } - const defaultCaseDeserInfo = this.buildFieldDeserInfo(defaultCase); - defaultMessage[defaultCaseDeserInfo.name] = - this.#updateFieldDeserInfoWithDefaultValue(defaultCaseDeserInfo); - } else if (deserInfo.type === "struct") { - for (const field of deserInfo.fields) { - if (!field.isOptional) { - defaultMessage[field.name] = this.#updateFieldDeserInfoWithDefaultValue(field); - } + const switchValue = switchTypeDefaultGetter() as number | boolean; + + defaultCase = unionDef.cases.find((c) => c.predicates.includes(switchValue))?.type; + if (!defaultCase) { + throw new Error(`Failed to find default case for union ${unionDef.name ?? ""}`); } + defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = switchValue; + } else { + // default exists, default value of switch case type is not needed + defaultMessage[UNION_DISCRIMINATOR_PROPERTY_KEY] = undefined; } - - if (defaultMessage == undefined) { - throw new Error(`Unrecognized complex type ${deserInfo.type as string}`); + const defaultCaseDeserInfo = this.buildFieldDeserInfo(defaultCase); + defaultMessage[defaultCaseDeserInfo.name] = this.getFieldDefault(defaultCaseDeserInfo); + } else if (deserInfo.type === "struct") { + for (const field of deserInfo.fields) { + if (!field.isOptional) { + defaultMessage[field.name] = this.getFieldDefault(field); + } } - return defaultMessage; - }; - this.#complexDefaultValueGetters.set(deserInfo.definition.name ?? "", defaultValueGetter); - return defaultValueGetter; - }; + } + if (defaultMessage == undefined) { + throw new Error(`Unrecognized complex type ${deserInfo.type as string}`); + } + return defaultMessage; + } } function typeToByteLength(type: string): number | undefined {