Skip to content

Commit

Permalink
feat: define default types during decode (#62)
Browse files Browse the repository at this point in the history
Proto3 language guide [states](https://developers.google.com/protocol-buffers/docs/proto3#default):

> When a message is parsed, if the encoded message does not contain a particular singular element, the corresponding field in the parsed object is set to the default value for that field.

When decoding objects, create an object with all non-optional fields set to the default values, then overwrite them during decoding.

Given that we create arrays for repeated fields there's no need to check for their existence before returning the deserialized object.

The only weird part in all this is message fields.  The spec says that any non-optional field is required, but it also says that the default value for a sub-message is to be unset and that the actual value there is language-dependent and links to some language guides which don't include JavaScript - I've used `undefined` here.  The upshot of this is that you can have a non-optional field with a default value of `undefined` which essentially makes it optional.  Nice.  At the moment we throw if the sub-message is not present in the protobuf if the field is required though in a future change we may wish to ignore the spec and initialise the sub-message to an instance of it's type with default values set 🤷.

Refs #43
  • Loading branch information
achingbrain authored Aug 11, 2022
1 parent a7d567d commit 6453809
Show file tree
Hide file tree
Showing 12 changed files with 213 additions and 176 deletions.
15 changes: 6 additions & 9 deletions packages/protons-benchmark/src/protons/bench.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ export namespace Yo {
writer.ldelim()
}
}, (reader, length) => {
const obj: any = {}
const obj: any = {
lol: []
}

const end = length == null ? reader.len : reader.pos + length

Expand All @@ -168,7 +170,6 @@ export namespace Yo {

switch (tag >>> 3) {
case 1:
obj.lol = obj.lol ?? []
obj.lol.push(FOO.codec().decode(reader))
break
default:
Expand All @@ -177,12 +178,6 @@ export namespace Yo {
}
}

obj.lol = obj.lol ?? []

if (obj.lol == null) {
throw new Error('Protocol error: value for required field "lol" was not found in protobuf')
}

return obj
})
}
Expand Down Expand Up @@ -230,7 +225,9 @@ export namespace Lol {
writer.ldelim()
}
}, (reader, length) => {
const obj: any = {}
const obj: any = {
b: undefined
}

const end = length == null ? reader.len : reader.pos + length

Expand Down
6 changes: 3 additions & 3 deletions packages/protons-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,12 @@ export interface Reader {
double: () => number

/**
* Reads a sequence of bytes preceeded by its length as a varint
* Reads a sequence of bytes preceded by its length as a varint
*/
bytes: () => number
bytes: () => Uint8Array

/**
* Reads a string preceeded by its byte length as a varint
* Reads a string preceded by its byte length as a varint
*/
string: () => string

Expand Down
89 changes: 83 additions & 6 deletions packages/protons/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const encoderGenerators: Record<string, (val: string) => string> = {
bool: (val) => `writer.bool(${val})`,
bytes: (val) => `writer.bytes(${val})`,
double: (val) => `writer.double(${val})`,
// enumeration: (val) => `writer.double(${val})`,
fixed32: (val) => `writer.fixed32(${val})`,
fixed64: (val) => `writer.fixed64(${val})`,
float: (val) => `writer.float(${val})`,
Expand All @@ -58,7 +57,6 @@ const decoderGenerators: Record<string, () => string> = {
bool: () => 'reader.bool()',
bytes: () => 'reader.bytes()',
double: () => 'reader.double()',
// enumeration: () => `writer.double(${val})`,
fixed32: () => 'reader.fixed32()',
fixed64: () => 'reader.fixed64()',
float: () => 'reader.float()',
Expand All @@ -73,6 +71,24 @@ const decoderGenerators: Record<string, () => string> = {
uint64: () => 'reader.uint64()'
}

const defaultValueGenerators: Record<string, () => string> = {
bool: () => 'false',
bytes: () => 'new Uint8Array(0)',
double: () => '0',
fixed32: () => '0',
fixed64: () => '0n',
float: () => '0',
int32: () => '0',
int64: () => '0n',
sfixed32: () => '0',
sfixed64: () => '0n',
sint32: () => '0',
sint64: () => '0n',
string: () => "''",
uint32: () => '0',
uint64: () => '0n'
}

function findTypeName (typeName: string, classDef: MessageDef, moduleDef: ModuleDef): string {
if (types[typeName] != null) {
return types[typeName]
Expand Down Expand Up @@ -117,6 +133,65 @@ function findDef (typeName: string, classDef: MessageDef, moduleDef: ModuleDef):
throw new Error(`Could not resolve type name "${typeName}"`)
}

function createDefaultObject (fields: Record<string, FieldDef>, messageDef: MessageDef, moduleDef: ModuleDef): string {
const output = Object.entries(fields)
.map(([name, fieldDef]) => {
if (fieldDef.repeated) {
return `${name}: []`
}

if (fieldDef.optional) {
return ''
}

const type: string = fieldDef.type
let defaultValue

if (defaultValueGenerators[type] != null) {
defaultValue = defaultValueGenerators[type]()
} else {
const def = findDef(fieldDef.type, messageDef, moduleDef)

if (isEnumDef(def)) {
// select lowest-value enum - should be 0 but it's not guaranteed
const val = Object.entries(def.values)
.sort((a, b) => {
if (a[1] < b[1]) {
return 1
}

if (a[1] > b[1]) {
return -1
}

return 0
})
.pop()

if (val == null) {
throw new Error(`Could not find default enum value for ${def.fullName}`)
}

defaultValue = `${def.name}.${val[0]}`
} else {
defaultValue = 'undefined'
}
}

return `${name}: ${defaultValue}`
})
.filter(Boolean)
.join(',\n ')

if (output !== '') {
return `
${output}
`
}

return ''
}

const encoders: Record<string, string> = {
bool: 'bool',
bytes: 'bytes',
Expand Down Expand Up @@ -259,7 +334,7 @@ export interface ${messageDef.name} {
const ensureArrayProps = Object.entries(fields)
.map(([name, fieldDef]) => {
// make sure repeated fields have an array if not set
if (fieldDef.rule === 'repeated') {
if (fieldDef.optional && fieldDef.rule === 'repeated') {
return ` obj.${name} = obj.${name} ?? []`
}

Expand All @@ -269,7 +344,7 @@ export interface ${messageDef.name} {
const ensureRequiredFields = Object.entries(fields)
.map(([name, fieldDef]) => {
// make sure required fields are set
if (!fieldDef.optional) {
if (!fieldDef.optional && !fieldDef.repeated) {
return `
if (obj.${name} == null) {
throw new Error('Protocol error: value for required field "${name}" was not found in protobuf')
Expand Down Expand Up @@ -331,7 +406,7 @@ ${Object.entries(fields)
writer.ldelim()
}
}, (reader, length) => {
const obj: any = {}
const obj: any = {${createDefaultObject(fields, messageDef, moduleDef)}}
const end = length == null ? reader.len : reader.pos + length
Expand Down Expand Up @@ -360,8 +435,10 @@ ${Object.entries(fields)
}
return `case ${fieldDef.id}:${fieldDef.rule === 'repeated'
? `${fieldDef.optional
? `
obj.${name} = obj.${name} ?? []
obj.${name} = obj.${name} ?? []`
: ''}
obj.${name}.push(${decoderGenerators[type] == null ? `${codec}.decode(reader${type === 'message' ? ', reader.uint32()' : ''})` : decoderGenerators[type]()})`
: `
obj.${name} = ${decoderGenerators[type] == null ? `${codec}.decode(reader${type === 'message' ? ', reader.uint32()' : ''})` : decoderGenerators[type]()}`}
Expand Down
2 changes: 1 addition & 1 deletion packages/protons/test/fixtures/basic.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ syntax = "proto3";

message Basic {
optional string foo = 1;
required int32 num = 2;
int32 num = 2;
}
4 changes: 3 additions & 1 deletion packages/protons/test/fixtures/basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ export namespace Basic {
writer.ldelim()
}
}, (reader, length) => {
const obj: any = {}
const obj: any = {
num: 0
}

const end = length == null ? reader.len : reader.pos + length

Expand Down
12 changes: 4 additions & 8 deletions packages/protons/test/fixtures/circuit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ export namespace CircuitRelay {
writer.ldelim()
}
}, (reader, length) => {
const obj: any = {}
const obj: any = {
id: new Uint8Array(0),
addrs: []
}

const end = length == null ? reader.len : reader.pos + length

Expand All @@ -124,7 +127,6 @@ export namespace CircuitRelay {
obj.id = reader.bytes()
break
case 2:
obj.addrs = obj.addrs ?? []
obj.addrs.push(reader.bytes())
break
default:
Expand All @@ -133,16 +135,10 @@ export namespace CircuitRelay {
}
}

obj.addrs = obj.addrs ?? []

if (obj.id == null) {
throw new Error('Protocol error: value for required field "id" was not found in protobuf')
}

if (obj.addrs == null) {
throw new Error('Protocol error: value for required field "addrs" was not found in protobuf')
}

return obj
})
}
Expand Down
Loading

0 comments on commit 6453809

Please sign in to comment.