Skip to content

Commit

Permalink
fix!: property decode new oneof cases (#255)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EvanHahn authored Sep 12, 2024
1 parent 601f42c commit 887b2e6
Showing 1 changed file with 29 additions and 17 deletions.
46 changes: 29 additions & 17 deletions src/lib/decode-conversions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'> = (
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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>[]
Expand All @@ -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)
}
Expand All @@ -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':
Expand All @@ -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)
}
Expand Down

0 comments on commit 887b2e6

Please sign in to comment.