fix!: singular fields should be optional to write (#83)

As per [`proto3`'s language guide](https://developers.google.com/protocol-buffers/docs/proto3#specifying_field_rules): > Message fields can be one of the following: > - singular: a well-formed message can have zero or one of this field (but not more than one). And this is the default field rule for proto3 syntax. This means that all `proto3` fields are effectively optional when writing, so it's relatively easy to implement by accepting a `Partial` version of the message interface for encoding. When reading messages singular fields are initialized to their default values when reading and optional values are not so the plain non-`Partial` interface can be returned. This also has the nice side effect of not requiring the user to pass empty lists/maps for `repeated` and `map` fields which never really felt right. These values are initialized to their empty forms when reading messages from the wire. Fixes #42
ipfs · Feb 2, 2023 · 229afbc · 229afbc
1 parent fd2e7a7
commit 229afbc
Show file tree

Hide file tree

Showing 16 changed files with 272 additions and 176 deletions.
diff --git a/packages/protons-benchmark/src/protons/rpc.ts b/packages/protons-benchmark/src/protons/rpc.ts
@@ -115,7 +115,7 @@ export namespace RPC {
 
           if (opts.writeDefaults === true || obj.topic !== '') {
             w.uint32(34)
-            w.string(obj.topic)
+            w.string(obj.topic ?? '')
           }
 
           if (obj.signature != null) {

diff --git a/packages/protons-runtime/src/codec.ts b/packages/protons-runtime/src/codec.ts
@@ -16,7 +16,7 @@ export interface EncodeOptions {
 }
 
 export interface EncodeFunction<T> {
-  (value: T, writer: Writer, opts?: EncodeOptions): void
+  (value: Partial<T>, writer: Writer, opts?: EncodeOptions): void
 }
 
 export interface DecodeFunction<T> {

diff --git a/packages/protons-runtime/src/codecs/message.ts b/packages/protons-runtime/src/codecs/message.ts
@@ -6,6 +6,6 @@ export interface Factory<A, T> {
   new (obj: A): T
 }
 
-export function message <T> (encode: (obj: T, writer: Writer, opts?: EncodeOptions) => void, decode: (reader: Reader, length?: number) => T): Codec<T> {
+export function message <T> (encode: (obj: Partial<T>, writer: Writer, opts?: EncodeOptions) => void, decode: (reader: Reader, length?: number) => T): Codec<T> {
   return createCodec('message', CODEC_TYPES.LENGTH_DELIMITED, encode, decode)
 }
diff --git a/packages/protons/src/index.ts b/packages/protons/src/index.ts
@@ -37,22 +37,22 @@ const types: Record<string, string> = {
   uint64: 'bigint'
 }
 
-const encoderGenerators: Record<string, (val: string) => string> = {
-  bool: (val) => `w.bool(${val})`,
-  bytes: (val) => `w.bytes(${val})`,
-  double: (val) => `w.double(${val})`,
-  fixed32: (val) => `w.fixed32(${val})`,
-  fixed64: (val) => `w.fixed64(${val})`,
-  float: (val) => `w.float(${val})`,
-  int32: (val) => `w.int32(${val})`,
-  int64: (val) => `w.int64(${val})`,
-  sfixed32: (val) => `w.sfixed32(${val})`,
-  sfixed64: (val) => `w.sfixed64(${val})`,
-  sint32: (val) => `w.sint32(${val})`,
-  sint64: (val) => `w.sint64(${val})`,
-  string: (val) => `w.string(${val})`,
-  uint32: (val) => `w.uint32(${val})`,
-  uint64: (val) => `w.uint64(${val})`
+const encoderGenerators: Record<string, (val: string, includeDefault: boolean) => string> = {
+  bool: (val, includeDefault) => `w.bool(${val}${includeDefault ? ' ?? false' : ''})`,
+  bytes: (val, includeDefault) => `w.bytes(${val}${includeDefault ? ' ?? new Uint8Array(0)' : ''})`,
+  double: (val, includeDefault) => `w.double(${val}${includeDefault ? ' ?? 0' : ''})`,
+  fixed32: (val, includeDefault) => `w.fixed32(${val}${includeDefault ? ' ?? 0' : ''})`,
+  fixed64: (val, includeDefault) => `w.fixed64(${val}${includeDefault ? ' ?? 0n' : ''})`,
+  float: (val, includeDefault) => `w.float(${val}${includeDefault ? ' ?? 0' : ''})`,
+  int32: (val, includeDefault) => `w.int32(${val}${includeDefault ? ' ?? 0' : ''})`,
+  int64: (val, includeDefault) => `w.int64(${val}${includeDefault ? ' ?? 0n' : ''})`,
+  sfixed32: (val, includeDefault) => `w.sfixed32(${val}${includeDefault ? ' ?? 0' : ''})`,
+  sfixed64: (val, includeDefault) => `w.sfixed64(${val}${includeDefault ? ' ?? 0n' : ''})`,
+  sint32: (val, includeDefault) => `w.sint32(${val}${includeDefault ? ' ?? 0' : ''})`,
+  sint64: (val, includeDefault) => `w.sint64(${val}${includeDefault ? ' ?? 0n' : ''})`,
+  string: (val, includeDefault) => `w.string(${val}${includeDefault ? ' ?? \'\'' : ''})`,
+  uint32: (val, includeDefault) => `w.uint32(${val}${includeDefault ? ' ?? 0' : ''})`,
+  uint64: (val, includeDefault) => `w.uint64(${val}${includeDefault ? ' ?? 0n' : ''})`
 }
 
 const decoderGenerators: Record<string, () => string> = {
@@ -92,21 +92,21 @@ const defaultValueGenerators: Record<string, () => string> = {
 }
 
 const defaultValueTestGenerators: Record<string, (field: string) => string> = {
-  bool: (field) => `${field} !== false`,
+  bool: (field) => `(${field} != null && ${field} !== false)`,
   bytes: (field) => `(${field} != null && ${field}.byteLength > 0)`,
-  double: (field) => `${field} !== 0`,
-  fixed32: (field) => `${field} !== 0`,
-  fixed64: (field) => `${field} !== 0n`,
-  float: (field) => `${field} !== 0`,
-  int32: (field) => `${field} !== 0`,
-  int64: (field) => `${field} !== 0n`,
-  sfixed32: (field) => `${field} !== 0`,
-  sfixed64: (field) => `${field} !== 0n`,
-  sint32: (field) => `${field} !== 0`,
-  sint64: (field) => `${field} !== 0n`,
-  string: (field) => `${field} !== ''`,
-  uint32: (field) => `${field} !== 0`,
-  uint64: (field) => `${field} !== 0n`
+  double: (field) => `(${field} != null && ${field} !== 0)`,
+  fixed32: (field) => `(${field} != null && ${field} !== 0)`,
+  fixed64: (field) => `(${field} != null && ${field} !== 0n)`,
+  float: (field) => `(${field} != null && ${field} !== 0)`,
+  int32: (field) => `(${field} != null && ${field} !== 0)`,
+  int64: (field) => `(${field} != null && ${field} !== 0n)`,
+  sfixed32: (field) => `(${field} != null && ${field} !== 0)`,
+  sfixed64: (field) => `(${field} != null && ${field} !== 0n)`,
+  sint32: (field) => `(${field} != null && ${field} !== 0)`,
+  sint64: (field) => `(${field} != null && ${field} !== 0n)`,
+  string: (field) => `(${field} != null && ${field} !== '')`,
+  uint32: (field) => `(${field} != null && ${field} !== 0)`,
+  uint64: (field) => `(${field} != null && ${field} !== 0n)`
 }
 
 function findTypeName (typeName: string, classDef: MessageDef, moduleDef: ModuleDef): string {
@@ -401,15 +401,26 @@ export interface ${messageDef.name} {
         }
       }
 
-      function createWriteField (valueVar: string): string {
+      function createWriteField (valueVar: string): (includeDefault: boolean) => string {
         const id = (fieldDef.id << 3) | codecTypes[type]
+        let defaultValue = ''
 
-        let writeField = `w.uint32(${id})
-          ${encoderGenerators[type] == null ? `${codec}.encode(${valueVar}, w)` : encoderGenerators[type](valueVar)}`
+        if (fieldDef.enum) {
+          const def = findDef(fieldDef.type, messageDef, moduleDef)
+
+          if (!isEnumDef(def)) {
+            throw new Error(`${fieldDef.type} was not enum def`)
+          }
+
+          defaultValue = Object.keys(def.values)[0]
+        }
+
+        let writeField = (includeDefault: boolean): string => `w.uint32(${id})
+          ${encoderGenerators[type] == null ? `${codec}.encode(${valueVar}${includeDefault ? ` ?? ${typeName}.${defaultValue}` : ''}, w)` : encoderGenerators[type](valueVar, includeDefault)}`
 
         if (type === 'message') {
           // message fields are only written if they have values
-          writeField = `w.uint32(${id})
+          writeField = () => `w.uint32(${id})
           ${typeName}.codec().encode(${valueVar}, w, {
             writeDefaults: ${Boolean(fieldDef.repeated).toString()}
           })`
@@ -422,10 +433,10 @@ export interface ${messageDef.name} {
 
       if (fieldDef.repeated) {
         if (fieldDef.map) {
-          writeField = `
+          writeField = () => `
         for (const [key, value] of obj.${name}.entries()) {
           ${
-              createWriteField('{ key, value }')
+              createWriteField('{ key, value }')(false)
                 .split('\n')
                 .map(s => {
                   const trimmed = s.trim()
@@ -437,10 +448,10 @@ export interface ${messageDef.name} {
           }
           `.trim()
         } else {
-          writeField = `
+          writeField = () => `
           for (const value of obj.${name}) {
           ${
-            createWriteField('value')
+            createWriteField('value')(false)
               .split('\n')
               .map(s => {
                 const trimmed = s.trim()
@@ -456,7 +467,7 @@ export interface ${messageDef.name} {
 
       return `
         if (${valueTest}) {
-          ${writeField}
+          ${writeField(valueTest.includes('opts.writeDefaults === true'))}
         }`
     }).join('\n')
 
@@ -537,7 +548,7 @@ ${encodeFields === '' ? '' : `${encodeFields}\n`}
     return _codec
   }
 
-  export const encode = (obj: ${messageDef.name}): Uint8Array => {
+  export const encode = (obj: Partial<${messageDef.name}>): Uint8Array => {
     return encodeMessage(obj, ${messageDef.name}.codec())
   }
 

diff --git a/packages/protons/test/fixtures/basic.ts b/packages/protons/test/fixtures/basic.ts
@@ -28,9 +28,9 @@ export namespace Basic {
           w.string(obj.foo)
         }
 
-        if (opts.writeDefaults === true || obj.num !== 0) {
+        if (opts.writeDefaults === true || (obj.num != null && obj.num !== 0)) {
           w.uint32(16)
-          w.int32(obj.num)
+          w.int32(obj.num ?? 0)
         }
 
         if (opts.lengthDelimited !== false) {
@@ -66,7 +66,7 @@ export namespace Basic {
     return _codec
   }
 
-  export const encode = (obj: Basic): Uint8Array => {
+  export const encode = (obj: Partial<Basic>): Uint8Array => {
     return encodeMessage(obj, Basic.codec())
   }
 
@@ -112,7 +112,7 @@ export namespace Empty {
     return _codec
   }
 
-  export const encode = (obj: Empty): Uint8Array => {
+  export const encode = (obj: Partial<Empty>): Uint8Array => {
     return encodeMessage(obj, Empty.codec())
   }
 

diff --git a/packages/protons/test/fixtures/circuit.proto b/packages/protons/test/fixtures/circuit.proto
@@ -29,7 +29,7 @@ message CircuitRelay {
   }
 
   message Peer {
-    required bytes id = 1;    // peer id
+    bytes id = 1;    // peer id
     repeated bytes addrs = 2; // peer's known addresses
   }
 

diff --git a/packages/protons/test/fixtures/circuit.ts b/packages/protons/test/fixtures/circuit.ts
@@ -97,7 +97,7 @@ export namespace CircuitRelay {
 
           if (opts.writeDefaults === true || (obj.id != null && obj.id.byteLength > 0)) {
             w.uint32(10)
-            w.bytes(obj.id)
+            w.bytes(obj.id ?? new Uint8Array(0))
           }
 
           if (obj.addrs != null) {
@@ -141,7 +141,7 @@ export namespace CircuitRelay {
       return _codec
     }
 
-    export const encode = (obj: Peer): Uint8Array => {
+    export const encode = (obj: Partial<Peer>): Uint8Array => {
       return encodeMessage(obj, Peer.codec())
     }
 
@@ -220,7 +220,7 @@ export namespace CircuitRelay {
     return _codec
   }
 
-  export const encode = (obj: CircuitRelay): Uint8Array => {
+  export const encode = (obj: Partial<CircuitRelay>): Uint8Array => {
     return encodeMessage(obj, CircuitRelay.codec())
   }