fix: encode enum values (#30)

Previously we were encoding enum keys which is wrong, we should be encoding values.
ipfs · May 10, 2022 · 676c01d · 676c01d
1 parent d85a9f4
commit 676c01d
Show file tree

Hide file tree

Showing 8 changed files with 174 additions and 36 deletions.
diff --git a/packages/protons-benchmark/package.json b/packages/protons-benchmark/package.json
@@ -62,19 +62,17 @@
     "clean": "aegir clean",
     "lint": "aegir lint",
     "dep-check": "aegir dep-check",
-    "build": "aegir build && cp -R src/protobufjs dist/src/protobufjs",
+    "build": "aegir build --no-bundle && cp -R src/protobufjs dist/src/protobufjs",
     "prestart": "npm run build",
     "start": "node dist/src/index.js"
   },
   "dependencies": {
+    "aegir": "^37.0.5",
     "benny": "^3.7.1",
     "pbjs": "^0.0.14",
     "protobufjs": "^6.11.2",
     "protons": "^2.0.0",
     "protons-runtime": "^0.0.0"
   },
-  "devDependencies": {
-     "aegir": "^37.0.5"
-  },
   "private": true
 }
diff --git a/packages/protons-runtime/src/codecs/enum.ts b/packages/protons-runtime/src/codecs/enum.ts
@@ -3,33 +3,43 @@ import { unsigned } from '../utils/varint.js'
 import { createCodec, CODEC_TYPES } from '../codec.js'
 import type { DecodeFunction, EncodeFunction, EncodingLengthFunction, Codec } from '../codec.js'
 
-export function enumeration <T> (e: T): Codec<T> {
-  const encodingLength: EncodingLengthFunction<string> = function enumEncodingLength (val: string) {
-    const keys = Object.keys(e)
-    const index = keys.indexOf(val)
+export function enumeration <T> (v: any): Codec<T> {
+  function findValue (val: string | number): number {
+    if (v[val.toString()] == null) {
+      throw new Error('Invalid enum value')
+    }
+
+    if (typeof val === 'number') {
+      return val
+    }
+
+    return v[val]
+  }
 
-    return unsigned.encodingLength(index)
+  const encodingLength: EncodingLengthFunction<number | string> = function enumEncodingLength (val) {
+    return unsigned.encodingLength(findValue(val))
   }
 
-  const encode: EncodeFunction<string> = function enumEncode (val) {
-    const keys = Object.keys(e)
-    const index = keys.indexOf(val)
-    const buf = new Uint8Array(unsigned.encodingLength(index))
+  const encode: EncodeFunction<number | string> = function enumEncode (val) {
+    const enumValue = findValue(val)
 
-    unsigned.encode(index, buf)
+    const buf = new Uint8Array(unsigned.encodingLength(enumValue))
+    unsigned.encode(enumValue, buf)
 
     return buf
   }
 
-  const decode: DecodeFunction<string> = function enumDecode (buf, offset) {
-    const index = unsigned.decode(buf, offset)
-    const keys = Object.keys(e)
+  const decode: DecodeFunction<number | string> = function enumDecode (buf, offset) {
+    const value = unsigned.decode(buf, offset)
+    const strValue = value.toString()
 
-    if (keys[index] == null) {
-      throw new Error('Could not find enum key for value')
+    // Use the reverse mapping to look up the enum key for the stored value
+    // https://www.typescriptlang.org/docs/handbook/enums.html#reverse-mappings
+    if (v[strValue] == null) {
+      throw new Error('Invalid enum value')
     }
 
-    return keys[index]
+    return v[strValue]
   }
 
   // @ts-expect-error yeah yeah

diff --git a/packages/protons/src/index.ts b/packages/protons/src/index.ts
@@ -131,15 +131,23 @@ function compileMessage (messageDef: MessageDef, moduleDef: ModuleDef): string {
     return `
 export enum ${messageDef.name} {
   ${
-    Object.keys(messageDef.values).map(enumValueName => {
-      return `${enumValueName} = '${enumValueName}'`
+    Object.keys(messageDef.values).map(name => {
+      return `${name} = '${name}'`
+    }).join(',\n  ').trim()
+  }
+}
+
+enum __${messageDef.name}Values {
+  ${
+    Object.entries(messageDef.values).map(([name, value]) => {
+      return `${name} = ${value}`
     }).join(',\n  ').trim()
   }
 }
 
 export namespace ${messageDef.name} {
   export const codec = () => {
-    return enumeration<typeof ${messageDef.name}>(${messageDef.name})
+    return enumeration<typeof ${messageDef.name}>(__${messageDef.name}Values)
   }
 }`.trim()
   }

diff --git a/packages/protons/test/fixtures/circuit.ts b/packages/protons/test/fixtures/circuit.ts
@@ -31,9 +31,28 @@ export namespace CircuitRelay {
     MALFORMED_MESSAGE = 'MALFORMED_MESSAGE'
   }
 
+  enum __StatusValues {
+    SUCCESS = 100,
+    HOP_SRC_ADDR_TOO_LONG = 220,
+    HOP_DST_ADDR_TOO_LONG = 221,
+    HOP_SRC_MULTIADDR_INVALID = 250,
+    HOP_DST_MULTIADDR_INVALID = 251,
+    HOP_NO_CONN_TO_DST = 260,
+    HOP_CANT_DIAL_DST = 261,
+    HOP_CANT_OPEN_DST_STREAM = 262,
+    HOP_CANT_SPEAK_RELAY = 270,
+    HOP_CANT_RELAY_TO_SELF = 280,
+    STOP_SRC_ADDR_TOO_LONG = 320,
+    STOP_DST_ADDR_TOO_LONG = 321,
+    STOP_SRC_MULTIADDR_INVALID = 350,
+    STOP_DST_MULTIADDR_INVALID = 351,
+    STOP_RELAY_REFUSED = 390,
+    MALFORMED_MESSAGE = 400
+  }
+
   export namespace Status {
     export const codec = () => {
-      return enumeration<typeof Status>(Status)
+      return enumeration<typeof Status>(__StatusValues)
     }
   }
 
@@ -44,9 +63,16 @@ export namespace CircuitRelay {
     CAN_HOP = 'CAN_HOP'
   }
 
+  enum __TypeValues {
+    HOP = 1,
+    STOP = 2,
+    STATUS = 3,
+    CAN_HOP = 4
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 

diff --git a/packages/protons/test/fixtures/daemon.ts b/packages/protons/test/fixtures/daemon.ts
@@ -30,9 +30,22 @@ export namespace Request {
     PEERSTORE = 'PEERSTORE'
   }
 
+  enum __TypeValues {
+    IDENTIFY = 0,
+    CONNECT = 1,
+    STREAM_OPEN = 2,
+    STREAM_HANDLER = 3,
+    DHT = 4,
+    LIST_PEERS = 5,
+    CONNMANAGER = 6,
+    DISCONNECT = 7,
+    PUBSUB = 8,
+    PEERSTORE = 9
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 
@@ -76,9 +89,14 @@ export namespace Response {
     ERROR = 'ERROR'
   }
 
+  enum __TypeValues {
+    OK = 0,
+    ERROR = 1
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 
@@ -263,9 +281,21 @@ export namespace DHTRequest {
     PROVIDE = 'PROVIDE'
   }
 
+  enum __TypeValues {
+    FIND_PEER = 0,
+    FIND_PEERS_CONNECTED_TO_PEER = 1,
+    FIND_PROVIDERS = 2,
+    GET_CLOSEST_PEERS = 3,
+    GET_PUBLIC_KEY = 4,
+    GET_VALUE = 5,
+    SEARCH_VALUE = 6,
+    PUT_VALUE = 7,
+    PROVIDE = 8
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 
@@ -303,9 +333,15 @@ export namespace DHTResponse {
     END = 'END'
   }
 
+  enum __TypeValues {
+    BEGIN = 0,
+    VALUE = 1,
+    END = 2
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 
@@ -362,9 +398,15 @@ export namespace ConnManagerRequest {
     TRIM = 'TRIM'
   }
 
+  enum __TypeValues {
+    TAG_PEER = 0,
+    UNTAG_PEER = 1,
+    TRIM = 2
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 
@@ -420,9 +462,16 @@ export namespace PSRequest {
     SUBSCRIBE = 'SUBSCRIBE'
   }
 
+  enum __TypeValues {
+    GET_TOPICS = 0,
+    LIST_PEERS = 1,
+    PUBLISH = 2,
+    SUBSCRIBE = 3
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 
@@ -507,9 +556,14 @@ export namespace PeerstoreRequest {
     GET_PEER_INFO = 'GET_PEER_INFO'
   }
 
+  enum __TypeValues {
+    GET_PROTOCOLS = 1,
+    GET_PEER_INFO = 2
+  }
+
   export namespace Type {
     export const codec = () => {
-      return enumeration<typeof Type>(Type)
+      return enumeration<typeof Type>(__TypeValues)
     }
   }
 

diff --git a/packages/protons/test/fixtures/dht.ts b/packages/protons/test/fixtures/dht.ts
@@ -51,9 +51,18 @@ export namespace Message {
     PING = 'PING'
   }
 
+  enum __MessageTypeValues {
+    PUT_VALUE = 0,
+    GET_VALUE = 1,
+    ADD_PROVIDER = 2,
+    GET_PROVIDERS = 3,
+    FIND_NODE = 4,
+    PING = 5
+  }
+
   export namespace MessageType {
     export const codec = () => {
-      return enumeration<typeof MessageType>(MessageType)
+      return enumeration<typeof MessageType>(__MessageTypeValues)
     }
   }
 
@@ -64,9 +73,16 @@ export namespace Message {
     CANNOT_CONNECT = 'CANNOT_CONNECT'
   }
 
+  enum __ConnectionTypeValues {
+    NOT_CONNECTED = 0,
+    CONNECTED = 1,
+    CAN_CONNECT = 2,
+    CANNOT_CONNECT = 3
+  }
+
   export namespace ConnectionType {
     export const codec = () => {
-      return enumeration<typeof ConnectionType>(ConnectionType)
+      return enumeration<typeof ConnectionType>(__ConnectionTypeValues)
     }
   }
 

diff --git a/packages/protons/test/fixtures/test.ts b/packages/protons/test/fixtures/test.ts
@@ -9,9 +9,14 @@ export enum AnEnum {
   DERP = 'DERP'
 }
 
+enum __AnEnumValues {
+  HERP = 0,
+  DERP = 1
+}
+
 export namespace AnEnum {
   export const codec = () => {
-    return enumeration<typeof AnEnum>(AnEnum)
+    return enumeration<typeof AnEnum>(__AnEnumValues)
   }
 }
 export interface SubMessage {

diff --git a/packages/protons/test/index.spec.ts b/packages/protons/test/index.spec.ts
@@ -6,8 +6,9 @@ import pbjs from 'pbjs'
 import { Basic } from './fixtures/basic.js'
 import { AllTheTypes, AnEnum } from './fixtures/test.js'
 import fs from 'fs'
-import protobufjs from 'protobufjs'
+import protobufjs, { Type as PBType } from 'protobufjs'
 import { Peer } from './fixtures/peer.js'
+import { CircuitRelay } from './fixtures/circuit.js'
 
 const Long = protobufjs.util.Long
 
@@ -160,4 +161,24 @@ describe('encode', () => {
     expect(Peer.decode(encoded)).to.deep.equal(peer)
     expect(Peer.decode(pbjsBuf)).to.deep.equal(peer)
   })
+
+  it('decodes enums with values that are not 0-n', () => {
+    const message: CircuitRelay = {
+      type: CircuitRelay.Type.STOP,
+      code: CircuitRelay.Status.HOP_NO_CONN_TO_DST
+    }
+
+    const root = protobufjs.loadSync('./test/fixtures/circuit.proto')
+    // @ts-expect-error
+    const PbCircuitRelay = root.nested.CircuitRelay as PBType
+
+    const pbufJsBuf = PbCircuitRelay.encode(PbCircuitRelay.fromObject(message)).finish()
+
+    const encoded = CircuitRelay.encode(message)
+
+    expect(encoded).to.equalBytes(pbufJsBuf)
+
+    expect(CircuitRelay.decode(encoded)).to.deep.equal(message)
+    expect(CircuitRelay.decode(pbufJsBuf)).to.deep.equal(message)
+  })
 })