Skip to content

Commit

Permalink
Merge pull request #462 from xmtp/cv/fix-ios-pool-error
Browse files Browse the repository at this point in the history 
Encode groups and conversations sequentially to fix iOS pool errors
  • Loading branch information
@cameronvoell
cameronvoell committed Aug 5, 2024
2 parents 4d0adbf + be21834 commit 37478c8
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 98 deletions.
3 changes: 0 additions & 3 deletions example/src/TestScreen.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { useRoute } from '@react-navigation/native'
import React, { useEffect, useState } from 'react'
import { View, Text, Button, ScrollView } from 'react-native'

import { createdAtTests } from './tests/createdAtTests'
import { groupPermissionsTests } from './tests/groupPermissionsTests'
import { groupTests } from './tests/groupTests'
import { restartStreamTests } from './tests/restartStreamsTests'
Expand Down Expand Up @@ -107,7 +106,6 @@ export enum TestCategory {
all = 'all',
tests = 'tests',
group = 'group',
createdAt = 'createdAt',
restartStreans = 'restartStreams',
groupPermissions = 'groupPermissions',
}
Expand All @@ -121,7 +119,6 @@ export default function TestScreen(): JSX.Element {
const allTests = [
...tests,
...groupTests,
...createdAtTests,
...restartStreamTests,
...groupPermissionsTests,
]
Expand Down
89 changes: 40 additions & 49 deletions example/src/tests/createdAtTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ test('group createdAt matches listGroups', async () => {
await alix.conversations.syncGroups()
const alixGroups = await alix.conversations.listGroups()

// BUG - List returns in Reverse Chronological order on iOS
// and Chronological order on Android
const first = isIos() ? 1 : 0
const second = isIos() ? 0 : 1
const first = 0
const second = 1
assert(alixGroups.length === 2, 'Alix should have two groups')
assert(
alixGroups[first].id === alixGroup.id,
Expand All @@ -47,6 +45,7 @@ test('group createdAt matches listGroups', async () => {
alixGroups[second].id === boGroup.id,
'Bo group createdAt should match'
)

// Below assertion fails on Android
if (isIos()) {
assert(
Expand Down Expand Up @@ -80,35 +79,37 @@ test('group createdAt matches listAll', async () => {
assert(alixGroups.length === 2, 'alix should have two groups')

// Returns reverse Chronological order on Android and iOS
const first = 1
const second = 0
const first = 0
const second = 1
assert(
alixGroups[1].topic === alixGroup.topic,
alixGroups[first].topic === alixGroup.topic,
'First group returned from listGroups shows ' +
alixGroups[1].topic +
alixGroups[first].topic +
' but should be ' +
alixGroup.topic
)
assert(
alixGroups[0].topic === boGroup.topic,
alixGroups[second].topic === boGroup.topic,
'Second group returned from listGroups shows ' +
alixGroups[0].topic +
alixGroups[second].topic +
' but should be ' +
boGroup.topic
)
assert(
alixGroups[first].createdAt === alixGroup.createdAt,
'alix group returned from listGroups shows createdAt ' +
alixGroups[1].createdAt +
' but should be ' +
alixGroup.createdAt
)
// Below assertion fail on Android

// Below assertion fails on Android
if (isIos()) {
assert(
alixGroups[first].createdAt === alixGroup.createdAt,
'alix group returned from listGroups shows createdAt ' +
alixGroups[first].createdAt +
' but should be ' +
alixGroup.createdAt
)

assert(
alixGroups[second].createdAt === boGroup.createdAt,
'bo group returned from listGroups shows createdAt ' +
alixGroups[0].createdAt +
alixGroups[second].createdAt +
' but should be ' +
boGroup.createdAt
)
Expand Down Expand Up @@ -152,19 +153,15 @@ test('group createdAt matches streamGroups', async () => {
'second ' + allGroups[1].id + ' != ' + caroGroup.id
)

// CreatedAt returned from stream matches createAt from create function
// Assertion below fails on Android
if (isIos()) {
assert(
allGroups[0].createdAt === boGroup.createdAt,
'first ' + allGroups[0].createdAt + ' != ' + boGroup.createdAt
)
assert(
allGroups[0].createdAt === boGroup.createdAt,
'first ' + allGroups[0].createdAt + ' != ' + boGroup.createdAt
)

assert(
allGroups[1].createdAt === caroGroup.createdAt,
'second ' + allGroups[1].createdAt + ' != ' + caroGroup.createdAt
)
}
assert(
allGroups[1].createdAt === caroGroup.createdAt,
'second ' + allGroups[1].createdAt + ' != ' + caroGroup.createdAt
)

cancelStream()

Expand Down Expand Up @@ -207,18 +204,14 @@ test('group createdAt matches streamAll', async () => {
'second ' + allGroups[1].topic + ' != ' + caroGroup.topic
)

// CreatedAt returned from stream matches createAt from create function
// Assertion below fails on Android
if (isIos()) {
assert(
allGroups[0].createdAt === boGroup.createdAt,
'first ' + allGroups[0].createdAt + ' != ' + boGroup.createdAt
)
assert(
allGroups[1].createdAt === caroGroup.createdAt,
'second ' + allGroups[1].createdAt + ' != ' + caroGroup.createdAt
)
}
assert(
allGroups[0].createdAt === boGroup.createdAt,
'first ' + allGroups[0].createdAt + ' != ' + boGroup.createdAt
)
assert(
allGroups[1].createdAt === caroGroup.createdAt,
'second ' + allGroups[1].createdAt + ' != ' + caroGroup.createdAt
)

cancelStream()

Expand All @@ -243,8 +236,8 @@ test('conversation createdAt matches list', async () => {

// BUG - List returns in Chronological order on iOS
// and reverse Chronological order on Android
const first = isIos() ? 0 : 1
const second = isIos() ? 1 : 0
const first = 0
const second = 1

assert(
alixConversations[first].topic === alixConversation.topic,
Expand Down Expand Up @@ -284,10 +277,8 @@ test('conversation createdAt matches listAll', async () => {
const alixConversations = await alix.conversations.listAll()
assert(alixConversations.length === 2, 'alix should have two conversations')

// BUG - List returns in Chronological order on iOS
// and reverse Chronological order on Android
const first = isIos() ? 0 : 1
const second = isIos() ? 1 : 0
const first = 0
const second = 1

// List returns in reverse Chronological order
assert(
Expand Down
55 changes: 52 additions & 3 deletions example/src/tests/groupTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
assert,
createClients,
delayToPropogate,
isIos,
} from './test-utils'
import {
Client,
Expand Down Expand Up @@ -49,6 +48,28 @@ test('can make a MLS V3 client', async () => {
return true
})

async function createGroups(
client: Client,
peers: Client[],
numGroups: number,
numMessages: number
): Promise<Group[]> {
const groups = []
const addresses: string[] = peers.map((client) => client.address)
for (let i = 0; i < numGroups; i++) {
const group = await client.conversations.newGroup(addresses, {
name: `group ${i}`,
imageUrlSquare: `www.group${i}.com`,
description: `group ${i}`,
})
groups.push(group)
for (let i = 0; i < numMessages; i++) {
await group.send({ text: `Message ${i}` })
}
}
return groups
}

test('calls preAuthenticateToInboxCallback when supplied', async () => {
let isCallbackCalled = 0
let isPreAuthCalled = false
Expand Down Expand Up @@ -184,6 +205,34 @@ test('can make a MLS V3 client with encryption key and database directory', asyn
return true
})

test('testing large group listing with metadata performance', async () => {
const [alixClient, boClient] = await createClients(2)

await createGroups(alixClient, [boClient], 50, 10)

let start = Date.now()
let groups = await alixClient.conversations.listGroups()
let end = Date.now()
console.log(`Alix loaded ${groups.length} groups in ${end - start}ms`)

start = Date.now()
await alixClient.conversations.syncGroups()
end = Date.now()
console.log(`Alix synced ${groups.length} groups in ${end - start}ms`)

start = Date.now()
await boClient.conversations.syncGroups()
end = Date.now()
console.log(`Bo synced ${groups.length} groups in ${end - start}ms`)

start = Date.now()
groups = await boClient.conversations.listGroups()
end = Date.now()
console.log(`Bo loaded ${groups.length} groups in ${end - start}ms`)

return true
})

test('can drop a local database', async () => {
const [client, anotherClient] = await createClients(2)

Expand Down Expand Up @@ -1029,8 +1078,8 @@ test('can list all groups and conversations', async () => {
// Verify information in listed containers is correct
// BUG - List All returns in Chronological order on iOS
// and reverse Chronological order on Android
const first = isIos() ? 1 : 0
const second = isIos() ? 0 : 1
const first = 0
const second = 1
if (
listedContainers[first].topic !== boGroup.topic ||
listedContainers[first].version !== ConversationVersion.GROUP ||
Expand Down
66 changes: 23 additions & 43 deletions ios/XMTPModule.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,44 +400,31 @@ public class XMTPModule: Module {
}

let conversations = try await client.conversations.list()

return try await withThrowingTaskGroup(of: String.self) { group in
for conversation in conversations {
group.addTask {
await self.conversationsManager.set(conversation.cacheKey(inboxId), conversation)
return try ConversationWrapper.encode(conversation, client: client)
}
}

var results: [String] = []
for try await result in group {
results.append(result)
}

return results

var results: [String] = []
for conversation in conversations {
await self.conversationsManager.set(conversation.cacheKey(inboxId), conversation)
let encodedConversation = try ConversationWrapper.encode(conversation, client: client)
results.append(encodedConversation)
}

return results
}

AsyncFunction("listGroups") { (inboxId: String) -> [String] in
guard let client = await clientsManager.getClient(key: inboxId) else {
throw Error.noClient
}
let groupList = try await client.conversations.groups()
return try await withThrowingTaskGroup(of: String.self) { taskGroup in
for group in groupList {
taskGroup.addTask {
await self.groupsManager.set(group.cacheKey(inboxId), group)
return try GroupWrapper.encode(group, client: client)
}
}

var results: [String] = []
for try await result in taskGroup {
results.append(result)
}

return results

var results: [String] = []
for group in groupList {
await self.groupsManager.set(group.cacheKey(inboxId), group)
let encodedGroup = try GroupWrapper.encode(group, client: client)
results.append(encodedGroup)
}

return results
}

AsyncFunction("listAll") { (inboxId: String) -> [String] in
Expand All @@ -446,21 +433,14 @@ public class XMTPModule: Module {
}
let conversationContainerList = try await client.conversations.list(includeGroups: true)

return try await withThrowingTaskGroup(of: String.self) { taskGroup in
for conversation in conversationContainerList {
taskGroup.addTask {
await self.conversationsManager.set(conversation.cacheKey(inboxId), conversation)
return try ConversationContainerWrapper.encode(conversation, client: client)
}
}

var results: [String] = []
for try await result in taskGroup {
results.append(result)
}

return results
var results: [String] = []
for conversation in conversationContainerList {
await self.conversationsManager.set(conversation.cacheKey(inboxId), conversation)
let encodedConversationContainer = try ConversationContainerWrapper.encode(conversation, client: client)
results.append(encodedConversationContainer)
}

return results
}

AsyncFunction("loadMessages") { (inboxId: String, topic: String, limit: Int?, before: Double?, after: Double?, direction: String?) -> [String] in
Expand Down

0 comments on commit 37478c8

Please sign in to comment.