Skip to content

Commit

Permalink
Merge pull request #341 from xmtp/ar/empty-fallbacks
Browse files Browse the repository at this point in the history
fix: Correctly handle empty/undefined fallbacks
  • Loading branch information
alexrisch authored Apr 2, 2024
2 parents 742281c + 36a6962 commit c0e1659
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 c0e1659

Please sign in to comment.