Skip to content

Commit

Permalink
fix: Correctly handle empty/undefined fallbacks
Browse files Browse the repository at this point in the history
Swift/Kotlin protos always set a default value for optional fields
this results in differences between sdks and no real way for an undefined value to be returned even if it is expected and typed

Updated to check if there is a fallback, if there is use it, if not set back to undefined
  • Loading branch information
Alex Risch authored and Alex Risch committed Apr 2, 2024
1 parent 742281c commit 36a6962
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,19 @@ class DecodedMessageWrapper {
return gson.toJson(message)
}

fun encodeMap(model: DecryptedMessage): Map<String, Any> = mapOf(
"id" to model.id,
"topic" to model.topic,
"contentTypeId" to model.encodedContent.type.description,
"content" to ContentJson(model.encodedContent).toJsonMap(),
"senderAddress" to model.senderAddress,
"sent" to model.sentAt.time,
"fallback" to model.encodedContent.fallback
)
fun encodeMap(model: DecryptedMessage): Map<String, Any?> {
// Kotlin/Java Protos don't support null values and will always put the default ""
// Check if there is a fallback, if there is then make it the set fallback, if not null
val fallback = if (model.encodedContent.hasFallback()) model.encodedContent.fallback else null
return mapOf(
"id" to model.id,
"topic" to model.topic,
"contentTypeId" to model.encodedContent.type.description,
"content" to ContentJson(model.encodedContent).toJsonMap(),
"senderAddress" to model.senderAddress,
"sent" to model.sentAt.time,
"fallback" to fallback
)
}
}
}
84 changes: 84 additions & 0 deletions example/src/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ const ContentTypeNumber: ContentTypeId = {
versionMinor: 0,
}

const ContentTypeNumberWithUndefinedFallback: ContentTypeId = {
authorityId: 'org',
typeId: 'number_undefined_fallback',
versionMajor: 1,
versionMinor: 0,
}

const ContentTypeNumberWithEmptyFallback: ContentTypeId = {
authorityId: 'org',
typeId: 'number_empty_fallback',
versionMajor: 1,
versionMinor: 0,
}

export type NumberRef = {
topNumber: {
bottomNumber: number
Expand Down Expand Up @@ -66,6 +80,20 @@ class NumberCodec implements JSContentCodec<NumberRef> {
}
}

class NumberCodecUndefinedFallback extends NumberCodec {
contentType = ContentTypeNumberWithUndefinedFallback
fallback(content: NumberRef): string | undefined {
return undefined
}
}

class NumberCodecEmptyFallback extends NumberCodec {
contentType = ContentTypeNumberWithEmptyFallback
fallback(content: NumberRef): string | undefined {
return ''
}
}

const LONG_STREAM_DELAY = 20000

export type Test = {
Expand Down Expand Up @@ -1184,6 +1212,62 @@ test('correctly handles lowercase addresses', async () => {
return true
})

test('handle fallback types appropriately', async () => {
const bob = await Client.createRandom({
env: 'local',
codecs: [
new NumberCodecEmptyFallback(),
new NumberCodecUndefinedFallback(),
],
})
const alice = await Client.createRandom({
env: 'local',
})
bob.register(new NumberCodecEmptyFallback())
bob.register(new NumberCodecUndefinedFallback())
const bobConvo = await bob.conversations.newConversation(alice.address)
const aliceConvo = await alice.conversations.newConversation(bob.address)

await bobConvo.send(12, { contentType: ContentTypeNumberWithEmptyFallback })

await bobConvo.send(12, {
contentType: ContentTypeNumberWithUndefinedFallback,
})

const messages = await aliceConvo.messages()
assert(messages.length === 2, 'did not get messages')

const messageUndefinedFallback = messages[0]
const messageWithDefinedFallback = messages[1]

let message1Content = undefined
try {
message1Content = messageUndefinedFallback.content()
} catch {
message1Content = messageUndefinedFallback.fallback
}

assert(
message1Content === undefined,
'did not get content properly when empty fallback: ' +
JSON.stringify(message1Content)
)

let message2Content = undefined
try {
message2Content = messageWithDefinedFallback.content()
} catch {
message2Content = messageWithDefinedFallback.fallback
}

assert(
message2Content === '',
'did not get content properly: ' + JSON.stringify(message2Content)
)

return true
})

test('instantiate frames client correctly', async () => {
const frameUrl =
'https://fc-polls-five.vercel.app/polls/01032f47-e976-42ee-9e3d-3aac1324f4b8'
Expand Down
5 changes: 4 additions & 1 deletion ios/Wrappers/DecodedMessageWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ import XMTP
// into react native.
struct DecodedMessageWrapper {
static func encodeToObj(_ model: XMTP.DecryptedMessage, client: Client) throws -> [String: Any] {
// Swift Protos don't support null values and will always put the default ""
// Check if there is a fallback, if there is then make it the set fallback, if not null
let fallback = model.encodedContent.hasFallback ? model.encodedContent.fallback : nil
return [
"id": model.id,
"topic": model.topic,
"contentTypeId": model.encodedContent.type.description,
"content": try ContentJson.fromEncoded(model.encodedContent, client: client).toJsonMap() as Any,
"senderAddress": model.senderAddress,
"sent": UInt64(model.sentAt.timeIntervalSince1970 * 1000),
"fallback": model.encodedContent.fallback,
"fallback": fallback,
]
}

Expand Down
9 changes: 4 additions & 5 deletions src/lib/DecodedMessage.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { Buffer } from 'buffer'

import { Client } from './Client'
import {
ContentCodec,
JSContentCodec,
NativeContentCodec,
NativeMessageContent,
} from './ContentCodec'
import { ReplyCodec } from './NativeCodecs/ReplyCodec'
import { TextCodec } from './NativeCodecs/TextCodec'
import { Buffer } from 'buffer'

const allowEmptyProperties: (keyof NativeMessageContent)[] = [
'text',
Expand Down Expand Up @@ -81,7 +79,8 @@ export class DecodedMessage<ContentTypes = any> {
this.senderAddress = senderAddress
this.sent = sent
this.nativeContent = content
this.fallback = fallback
// undefined comes back as null when bridged, ensure undefined so integrators don't have to add a new check for null as well
this.fallback = fallback ?? undefined
}

content(): ContentTypes {
Expand Down

0 comments on commit 36a6962

Please sign in to comment.