fix(imap-smtp-caldav): eliminate duplicate CalDAV discovery and batch folder inserts
https://sonarly.com/issue/22577?type=bug Setting up an IMAP/SMTP/CalDAV connected account takes ~11 seconds due to duplicate CalDAV server discovery (3.7s wasted) and N+1 message folder inserts with per-row transactions (1.3s wasted on transaction overhead). Fix: **Two changes to eliminate ~3.5s of wasted time during IMAP/SMTP/CalDAV account setup:** **1. Eliminate duplicate CalDAV server discovery** (`caldav.client.ts`) Extracted the CalDAV fetch logic into a private `fetchDAVCalendars()` method shared by both `listCalendars()` and `validateSyncCollectionSupport()`. Added a new `listCalendarsAndValidateSync()` method that does both operations in a single discovery pass. Extracted the sync-collection validation into `assertSyncCollectionSupported()` private method for reuse. The existing `listCalendars()` and `validateSyncCollectionSupport()` public methods are preserved with identical signatures (used elsewhere in the codebase), but now delegate to the shared internal methods. Updated `testCaldavConnection()` in `imap-smtp-caldav-connection.service.ts` to call the new combined method instead of two separate ones. This eliminates one full CalDAV discovery round-trip (~1.8s saved with 4+ calendars). **2. Batch message folder inserts** (`sync-message-folders.service.ts`) Changed the N+1 folder creation loop from individual `.save()` calls to a single `.save([...])` call with an array. TypeORM's `save()` method natively supports arrays and will execute a single INSERT with all rows in one transaction instead of N separate transactions. This eliminates ~1.3s of transaction overhead (7 START TRANSACTION + COMMIT pairs reduced to 1). Updated the test file to handle the array-based save call (mock and assertions).
This commit is contained in:
+1
-2
@@ -131,8 +131,7 @@ export class ImapSmtpCaldavService {
|
||||
});
|
||||
|
||||
try {
|
||||
await client.listCalendars();
|
||||
await client.validateSyncCollectionSupport();
|
||||
await client.listCalendarsAndValidateSync();
|
||||
} catch (error) {
|
||||
this.logger.error(
|
||||
`CALDAV connection failed: ${error.message}`,
|
||||
|
||||
+56
-13
@@ -108,16 +108,24 @@ export class CalDAVClient {
|
||||
});
|
||||
}
|
||||
|
||||
private async fetchDAVCalendars(): Promise<
|
||||
(Omit<DAVCalendar, 'displayName'> & {
|
||||
displayName?: string | Record<string, unknown>;
|
||||
})[]
|
||||
> {
|
||||
const account = await this.getAccount();
|
||||
|
||||
return (await fetchCalendars({
|
||||
account,
|
||||
headers: this.headers,
|
||||
})) as (Omit<DAVCalendar, 'displayName'> & {
|
||||
displayName?: string | Record<string, unknown>;
|
||||
})[];
|
||||
}
|
||||
|
||||
async listCalendars(): Promise<SimpleCalendar[]> {
|
||||
try {
|
||||
const account = await this.getAccount();
|
||||
|
||||
const calendars = (await fetchCalendars({
|
||||
account,
|
||||
headers: this.headers,
|
||||
})) as (Omit<DAVCalendar, 'displayName'> & {
|
||||
displayName?: string | Record<string, unknown>;
|
||||
})[];
|
||||
const calendars = await this.fetchDAVCalendars();
|
||||
|
||||
return calendars.reduce<SimpleCalendar[]>((result, calendar) => {
|
||||
if (!calendar.components?.includes('VEVENT')) return result;
|
||||
@@ -146,13 +154,48 @@ export class CalDAVClient {
|
||||
}
|
||||
|
||||
async validateSyncCollectionSupport(): Promise<void> {
|
||||
const account = await this.getAccount();
|
||||
const calendars = await this.fetchDAVCalendars();
|
||||
|
||||
const calendars = await fetchCalendars({
|
||||
account,
|
||||
headers: this.headers,
|
||||
});
|
||||
this.assertSyncCollectionSupported(calendars);
|
||||
}
|
||||
|
||||
async listCalendarsAndValidateSync(): Promise<SimpleCalendar[]> {
|
||||
try {
|
||||
const calendars = await this.fetchDAVCalendars();
|
||||
|
||||
this.assertSyncCollectionSupported(calendars);
|
||||
|
||||
return calendars.reduce<SimpleCalendar[]>((result, calendar) => {
|
||||
if (!calendar.components?.includes('VEVENT')) return result;
|
||||
|
||||
result.push({
|
||||
id: calendar.url,
|
||||
url: calendar.url,
|
||||
name:
|
||||
typeof calendar.displayName === 'string'
|
||||
? calendar.displayName
|
||||
: 'Unnamed Calendar',
|
||||
isPrimary: false,
|
||||
});
|
||||
|
||||
return result;
|
||||
}, []);
|
||||
} catch (error) {
|
||||
this.logger.error(
|
||||
`Error in ${CalDavGetEventsService.name} - getCalendarEvents`,
|
||||
error.code,
|
||||
error,
|
||||
);
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
private assertSyncCollectionSupported(
|
||||
calendars: (Omit<DAVCalendar, 'displayName'> & {
|
||||
displayName?: string | Record<string, unknown>;
|
||||
})[],
|
||||
): void {
|
||||
const eventCalendar = calendars.find((calendar) =>
|
||||
calendar.components?.includes('VEVENT'),
|
||||
);
|
||||
|
||||
+44
-34
@@ -129,15 +129,21 @@ describe('SyncMessageFoldersService', () => {
|
||||
mockMessageFolderRepository = {
|
||||
delete: jest.fn(),
|
||||
update: jest.fn().mockResolvedValue(undefined),
|
||||
save: jest.fn().mockImplementation(async (folder) => {
|
||||
createdFolderRecords.push({
|
||||
...folder,
|
||||
id: `new-folder-${createdFolderRecords.length}-${Math.random().toString(36).substring(7)}`,
|
||||
isSynced: false,
|
||||
syncCursor: null,
|
||||
pendingSyncAction: MessageFolderPendingSyncAction.NONE,
|
||||
externalId: folder.externalId as string,
|
||||
});
|
||||
save: jest.fn().mockImplementation(async (folderOrFolders) => {
|
||||
const folders = Array.isArray(folderOrFolders)
|
||||
? folderOrFolders
|
||||
: [folderOrFolders];
|
||||
|
||||
for (const folder of folders) {
|
||||
createdFolderRecords.push({
|
||||
...folder,
|
||||
id: `new-folder-${createdFolderRecords.length}-${Math.random().toString(36).substring(7)}`,
|
||||
isSynced: false,
|
||||
syncCursor: null,
|
||||
pendingSyncAction: MessageFolderPendingSyncAction.NONE,
|
||||
externalId: folder.externalId as string,
|
||||
});
|
||||
}
|
||||
}),
|
||||
find: jest.fn().mockImplementation(async ({ where }) => {
|
||||
if (!where?.externalId) {
|
||||
@@ -227,22 +233,22 @@ describe('SyncMessageFoldersService', () => {
|
||||
});
|
||||
|
||||
expect(mockMessageFolderRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
name: 'INBOX',
|
||||
externalId: 'inbox-ext',
|
||||
messageChannelId: 'channel-123',
|
||||
isSentFolder: false,
|
||||
}),
|
||||
);
|
||||
expect(mockMessageFolderRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
name: 'Sent',
|
||||
externalId: 'sent-ext',
|
||||
messageChannelId: 'channel-123',
|
||||
isSentFolder: true,
|
||||
}),
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
name: 'INBOX',
|
||||
externalId: 'inbox-ext',
|
||||
messageChannelId: 'channel-123',
|
||||
isSentFolder: false,
|
||||
}),
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
name: 'Sent',
|
||||
externalId: 'sent-ext',
|
||||
messageChannelId: 'channel-123',
|
||||
isSentFolder: true,
|
||||
}),
|
||||
]),
|
||||
);
|
||||
expect(result).toHaveLength(2);
|
||||
});
|
||||
@@ -274,11 +280,13 @@ describe('SyncMessageFoldersService', () => {
|
||||
});
|
||||
|
||||
expect(mockMessageFolderRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
name: 'Projects',
|
||||
parentFolderId: 'parent-folder-id',
|
||||
}),
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
name: 'Projects',
|
||||
parentFolderId: 'parent-folder-id',
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -508,10 +516,12 @@ describe('SyncMessageFoldersService', () => {
|
||||
expect.objectContaining({ name: 'New Name' }),
|
||||
);
|
||||
expect(mockMessageFolderRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
externalId: 'new-ext',
|
||||
}),
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
workspaceId,
|
||||
externalId: 'new-ext',
|
||||
}),
|
||||
]),
|
||||
);
|
||||
expect(result).toHaveLength(4);
|
||||
expect(result).toContainEqual(
|
||||
|
||||
+4
-4
@@ -154,12 +154,12 @@ export class SyncMessageFoldersService {
|
||||
}
|
||||
|
||||
if (foldersToCreate.length > 0) {
|
||||
for (const folderToCreate of foldersToCreate) {
|
||||
await this.messageFolderRepository.save({
|
||||
await this.messageFolderRepository.save(
|
||||
foldersToCreate.map((folderToCreate) => ({
|
||||
...folderToCreate,
|
||||
workspaceId,
|
||||
});
|
||||
}
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
const createdFolders =
|
||||
|
||||
Reference in New Issue
Block a user