From 887b2e659237f27c55a35eacfc103852737d6402 Mon Sep 17 00:00:00 2001
From: Evan Hahn <me@evanhahn.com>
Date: Thu, 12 Sep 2024 10:37:46 -0500
Subject: [PATCH] fix!: property decode new `oneof` cases (#255)

We have proto definitions that use `oneof`, like this:

```protobuf
message Foo {
  oneof something {
    string bar = 1;
    string baz = 2;
  }
}
```

We also have decoding that does something like this:

```javascript
switch (foo.something?.$case) {
  case 'bar':
    // ...do something...
  case 'baz':
    // ...do something...
  default:
    throw new Error('Unexpected case')
}
```

This will break if we add another case, like this:

```diff
 message Foo {
   oneof something {
     string bar = 1;
     string baz = 2;
+    string qux = 3;
   }
 }
```

This fixes those cases in the codebase, treating an unknown `oneof` as
skipped. We already did this in a few places.

I manually tested this (by adding a new oneof case) to make sure things
would still decode correctly if an old version decoded something from a
new version.
---
 src/lib/decode-conversions.ts | 46 ++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/lib/decode-conversions.ts b/src/lib/decode-conversions.ts
index 242db2f..3b68795 100644
--- a/src/lib/decode-conversions.ts
+++ b/src/lib/decode-conversions.ts
@@ -21,7 +21,7 @@ import {
   type MapeoDocDecode,
 } from '../types.js'
 import { ExhaustivenessError, VersionIdObject, getVersionId } from './utils.js'
-import type { Observation, Track } from '../index.js'
+import type { Icon, Observation, Track } from '../index.js'
 import type {
   Observation_1_Attachment,
   Observation_1_Metadata,
@@ -240,12 +240,16 @@ export const convertCoreOwnership: ConvertFunction<'coreOwnership'> = (
 
 export const convertIcon: ConvertFunction<'icon'> = (message, versionObj) => {
   const { common, schemaVersion, ...rest } = message
+
   const jsonSchemaCommon = convertCommon(common, versionObj)
-  return {
-    ...jsonSchemaCommon,
-    ...rest,
-    variants: message.variants.map((variant) => convertIconVariant(variant)),
+
+  const variants: Icon['variants'] = []
+  for (const variant of message.variants) {
+    const converted = convertIconVariant(variant)
+    if (converted) variants.push(converted)
   }
+
+  return { ...jsonSchemaCommon, ...rest, variants }
 }
 
 export const convertTranslation: ConvertFunction<'translation'> = (
@@ -290,14 +294,20 @@ export const convertTrack: ConvertFunction<'track'> = (message, versionObj) => {
   }
 }
 
-function convertIconVariant(variant: Icon_1_IconVariant) {
-  if (variant.variant?.$case === 'pngIcon') {
-    const { pixelDensity } = variant.variant.pngIcon
-    return convertIconVariantPng({ ...variant, pixelDensity })
-  } else if (variant.variant?.$case === 'svgIcon') {
-    return convertIconVariantSvg(variant)
-  } else {
-    throw new Error('invalid icon variant type')
+function convertIconVariant(
+  variant: Icon_1_IconVariant
+): null | Icon['variants'][number] {
+  switch (variant.variant?.$case) {
+    case 'pngIcon': {
+      const { pixelDensity } = variant.variant.pngIcon
+      return convertIconVariantPng({ ...variant, pixelDensity })
+    }
+    case 'svgIcon':
+      return convertIconVariantSvg(variant)
+    case undefined:
+      return null
+    default:
+      throw new ExhaustivenessError(variant.variant)
   }
 }
 
@@ -362,8 +372,7 @@ function convertTags(tags: { [key: string]: TagValue_1 } | undefined): {
 }
 
 function convertTagValue({ kind }: TagValue_1): JsonTagValue {
-  if (!kind) return undefined
-  switch (kind.$case) {
+  switch (kind?.$case) {
     case 'list_value':
       return kind.list_value.list_value.reduce<
         Exclude<TagValuePrimitive, undefined>[]
@@ -376,6 +385,8 @@ function convertTagValue({ kind }: TagValue_1): JsonTagValue {
       }, [])
     case 'primitive_value':
       return convertTagPrimitive(kind.primitive_value)
+    case undefined:
+      return undefined
     default:
       throw new ExhaustivenessError(kind)
   }
@@ -384,8 +395,7 @@ function convertTagValue({ kind }: TagValue_1): JsonTagValue {
 function convertTagPrimitive({
   kind,
 }: TagValue_1_PrimitiveValue): TagValuePrimitive {
-  if (!kind) return undefined
-  switch (kind.$case) {
+  switch (kind?.$case) {
     case 'null_value':
       return null
     case 'boolean_value':
@@ -394,6 +404,8 @@ function convertTagPrimitive({
       return kind.number_value
     case 'string_value':
       return kind.string_value
+    case undefined:
+      return undefined
     default:
       throw new ExhaustivenessError(kind)
   }