IMAP support non RFC compliant servers (#22153)
Some non complaint IMAP server don't send `UIDNEXT` UIDNEXT is the next message id you subtract with 1 to get total current messages This does a fallback to searching all UIDs and taking the highest <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22153?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+45
@@ -30,6 +30,7 @@ const createMockFolder = (
|
||||
describe('ImapGetMessageListService', () => {
|
||||
let service: ImapGetMessageListService;
|
||||
let imapClientProvider: ImapClientProvider;
|
||||
let imapSyncService: ImapSyncService;
|
||||
|
||||
const mockConnectedAccount: Pick<
|
||||
ConnectedAccountEntity,
|
||||
@@ -93,6 +94,7 @@ describe('ImapGetMessageListService', () => {
|
||||
|
||||
service = module.get<ImapGetMessageListService>(ImapGetMessageListService);
|
||||
imapClientProvider = module.get<ImapClientProvider>(ImapClientProvider);
|
||||
imapSyncService = module.get<ImapSyncService>(ImapSyncService);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -229,4 +231,47 @@ describe('ImapGetMessageListService', () => {
|
||||
expect(imapClientProvider.closeClient).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('incremental sync skip', () => {
|
||||
const syncedFolder = createMockFolder({
|
||||
name: 'INBOX',
|
||||
externalId: 'INBOX:12345',
|
||||
isSynced: true,
|
||||
syncCursor: JSON.stringify({
|
||||
highestUid: 99,
|
||||
uidValidity: 12345,
|
||||
modSeq: '1000',
|
||||
}),
|
||||
});
|
||||
|
||||
const runSync = () =>
|
||||
service.getMessageLists({
|
||||
connectedAccount: mockConnectedAccount,
|
||||
messageChannel: {
|
||||
syncCursor: '',
|
||||
id: 'channel-1',
|
||||
messageFolderImportPolicy: MessageFolderImportPolicy.ALL_FOLDERS,
|
||||
},
|
||||
messageFolders: [syncedFolder],
|
||||
});
|
||||
|
||||
it('skips folders whose cursor already covers the latest UID', async () => {
|
||||
const [result] = await runSync();
|
||||
|
||||
expect(imapSyncService.syncFolder).not.toHaveBeenCalled();
|
||||
expect(result.messageExternalIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not skip when the server omits UIDNEXT on STATUS', async () => {
|
||||
mockImapClient.status.mockResolvedValueOnce({
|
||||
uidValidity: 12345,
|
||||
highestModseq: '1000',
|
||||
});
|
||||
|
||||
const [result] = await runSync();
|
||||
|
||||
expect(imapSyncService.syncFolder).toHaveBeenCalledTimes(1);
|
||||
expect(result.messageExternalIds).not.toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+15
-3
@@ -14,7 +14,7 @@ import { ImapClientProvider } from 'src/modules/messaging/message-import-manager
|
||||
import { ImapMessageListFetchErrorHandler } from 'src/modules/messaging/message-import-manager/drivers/imap/services/imap-message-list-fetch-error-handler.service';
|
||||
import { ImapSyncService } from 'src/modules/messaging/message-import-manager/drivers/imap/services/imap-sync.service';
|
||||
import { createSyncCursor } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/create-sync-cursor.util';
|
||||
import { extractMailboxState } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util';
|
||||
import { resolveMailboxState } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util';
|
||||
import { getImapFolderPath } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/get-imap-folder-path.util';
|
||||
import { parseSyncCursor } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/parse-sync-cursor.util';
|
||||
import { type GetMessageListsArgs } from 'src/modules/messaging/message-import-manager/types/get-message-lists-args.type';
|
||||
@@ -116,7 +116,11 @@ export class ImapGetMessageListService {
|
||||
);
|
||||
}
|
||||
|
||||
const mailboxState = extractMailboxState(mailbox);
|
||||
const mailboxState = await resolveMailboxState(
|
||||
client,
|
||||
folderPath,
|
||||
mailbox,
|
||||
);
|
||||
|
||||
const { messageUids } = await this.imapSyncService.syncFolder(
|
||||
client,
|
||||
@@ -181,7 +185,15 @@ export class ImapGetMessageListService {
|
||||
return false;
|
||||
}
|
||||
|
||||
const uidNext = Number(status.uidNext ?? 1);
|
||||
if (!isDefined(status.uidNext)) {
|
||||
this.logger.debug(
|
||||
`Folder ${folderPath}: Server missing UIDNEXT. Sync required.`,
|
||||
);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
const uidNext = Number(status.uidNext);
|
||||
const uidValidity = Number(status.uidValidity);
|
||||
|
||||
if (previousCursor.uidValidity !== uidValidity) {
|
||||
|
||||
+126
@@ -0,0 +1,126 @@
|
||||
import { type ImapFlow } from 'imapflow';
|
||||
|
||||
import { resolveMailboxState } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/extract-mailbox-state.util';
|
||||
|
||||
type MockClient = {
|
||||
status: jest.Mock;
|
||||
search: jest.Mock;
|
||||
};
|
||||
|
||||
const createClient = (): MockClient => ({
|
||||
status: jest.fn(),
|
||||
search: jest.fn(),
|
||||
});
|
||||
|
||||
const asImapFlow = (client: MockClient) => client as unknown as ImapFlow;
|
||||
|
||||
const createMailbox = (
|
||||
overrides: Partial<NonNullable<ImapFlow['mailbox']>> = {},
|
||||
) =>
|
||||
({
|
||||
uidValidity: BigInt(100),
|
||||
uidNext: 51,
|
||||
highestModseq: BigInt(9),
|
||||
...overrides,
|
||||
}) as NonNullable<ImapFlow['mailbox']>;
|
||||
|
||||
describe('resolveMailboxState', () => {
|
||||
it('uses the SELECT uidNext without extra round-trips when the server provides it', async () => {
|
||||
const client = createClient();
|
||||
|
||||
const state = await resolveMailboxState(
|
||||
asImapFlow(client),
|
||||
'INBOX',
|
||||
createMailbox({ uidNext: 51 }),
|
||||
);
|
||||
|
||||
expect(state).toEqual({
|
||||
uidValidity: 100,
|
||||
uidNext: 51,
|
||||
maxUid: 50,
|
||||
highestModSeq: BigInt(9),
|
||||
});
|
||||
expect(client.status).not.toHaveBeenCalled();
|
||||
expect(client.search).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to STATUS when the server omits uidNext on SELECT', async () => {
|
||||
const client = createClient();
|
||||
|
||||
client.status.mockResolvedValue({ uidNext: 71 });
|
||||
|
||||
const state = await resolveMailboxState(
|
||||
asImapFlow(client),
|
||||
'INBOX',
|
||||
createMailbox({ uidNext: undefined }),
|
||||
);
|
||||
|
||||
expect(client.status).toHaveBeenCalledWith('INBOX', { uidNext: true });
|
||||
expect(client.search).not.toHaveBeenCalled();
|
||||
expect(state.uidNext).toBe(71);
|
||||
expect(state.maxUid).toBe(70);
|
||||
});
|
||||
|
||||
it('falls back to the highest live UID when both SELECT and STATUS omit uidNext', async () => {
|
||||
const client = createClient();
|
||||
|
||||
client.status.mockResolvedValue({ uidNext: 0 });
|
||||
client.search.mockResolvedValue([3, 41, 17]);
|
||||
|
||||
const state = await resolveMailboxState(
|
||||
asImapFlow(client),
|
||||
'INBOX',
|
||||
createMailbox({ uidNext: undefined }),
|
||||
);
|
||||
|
||||
expect(client.search).toHaveBeenCalledWith({ uid: '1:*' }, { uid: true });
|
||||
expect(state.uidNext).toBe(42);
|
||||
expect(state.maxUid).toBe(41);
|
||||
});
|
||||
|
||||
it('handles a mailbox with 100k+ UIDs without overflowing the call stack', async () => {
|
||||
const client = createClient();
|
||||
|
||||
client.status.mockResolvedValue({ uidNext: 0 });
|
||||
client.search.mockResolvedValue(
|
||||
Array.from({ length: 200_000 }, (_, index) => index + 1),
|
||||
);
|
||||
|
||||
const state = await resolveMailboxState(
|
||||
asImapFlow(client),
|
||||
'INBOX',
|
||||
createMailbox({ uidNext: undefined }),
|
||||
);
|
||||
|
||||
expect(state.uidNext).toBe(200_001);
|
||||
expect(state.maxUid).toBe(200_000);
|
||||
});
|
||||
|
||||
it('treats an empty mailbox as uidNext 1 when no UID source is available', async () => {
|
||||
const client = createClient();
|
||||
|
||||
client.status.mockResolvedValue({});
|
||||
client.search.mockResolvedValue([]);
|
||||
|
||||
const state = await resolveMailboxState(
|
||||
asImapFlow(client),
|
||||
'INBOX',
|
||||
createMailbox({ uidNext: undefined }),
|
||||
);
|
||||
|
||||
expect(state.uidNext).toBe(1);
|
||||
expect(state.maxUid).toBe(0);
|
||||
});
|
||||
|
||||
it('throws when the mailbox is not selected', async () => {
|
||||
const client = createClient();
|
||||
|
||||
await expect(
|
||||
resolveMailboxState(
|
||||
asImapFlow(client),
|
||||
'INBOX',
|
||||
true as unknown as NonNullable<ImapFlow['mailbox']>,
|
||||
),
|
||||
).rejects.toThrow('Invalid mailbox state');
|
||||
});
|
||||
});
|
||||
+32
-3
@@ -1,4 +1,5 @@
|
||||
import { type ImapFlow } from 'imapflow';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
export type MailboxState = {
|
||||
uidValidity: number;
|
||||
@@ -7,14 +8,42 @@ export type MailboxState = {
|
||||
highestModSeq?: bigint;
|
||||
};
|
||||
|
||||
export const extractMailboxState = (
|
||||
// David.fx and other non-RFC servers omit the required UIDNEXT on SELECT;
|
||||
// fall back to STATUS, then the highest live UID, so the sync range isn't empty.
|
||||
const resolveUidNext = async (
|
||||
client: ImapFlow,
|
||||
folderPath: string,
|
||||
mailboxUidNext: number | undefined,
|
||||
): Promise<number> => {
|
||||
if (isDefined(mailboxUidNext)) {
|
||||
return Number(mailboxUidNext);
|
||||
}
|
||||
|
||||
const status = await client.status(folderPath, { uidNext: true });
|
||||
const statusUidNext = Number(status.uidNext ?? 0);
|
||||
|
||||
if (statusUidNext > 0) {
|
||||
return statusUidNext;
|
||||
}
|
||||
|
||||
const uids = await client.search({ uid: '1:*' }, { uid: true });
|
||||
const highestUid = Array.isArray(uids)
|
||||
? uids.reduce((max, uid) => (uid > max ? uid : max), 0)
|
||||
: 0;
|
||||
|
||||
return highestUid + 1;
|
||||
};
|
||||
|
||||
export const resolveMailboxState = async (
|
||||
client: ImapFlow,
|
||||
folderPath: string,
|
||||
mailbox: NonNullable<ImapFlow['mailbox']>,
|
||||
): MailboxState => {
|
||||
): Promise<MailboxState> => {
|
||||
if (typeof mailbox === 'boolean') {
|
||||
throw new Error('Invalid mailbox state');
|
||||
}
|
||||
|
||||
const uidNext = Number(mailbox.uidNext ?? 1);
|
||||
const uidNext = await resolveUidNext(client, folderPath, mailbox.uidNext);
|
||||
|
||||
return {
|
||||
uidValidity: Number(mailbox.uidValidity ?? 0),
|
||||
|
||||
Reference in New Issue
Block a user