From 04780851bab1e66d9b170ac0f0e6f16b995dfd62 Mon Sep 17 00:00:00 2001 From: Artem Savchenko Date: Fri, 9 May 2025 14:14:15 +0700 Subject: [PATCH] UBERF-10408: Add prefix for new history Signed-off-by: Artem Savchenko --- services/gmail/pod-gmail/package.json | 4 +- .../pod-gmail/src/__tests__/syncState.test.ts | 253 ++++++++++++++++++ services/gmail/pod-gmail/src/config.ts | 12 +- services/gmail/pod-gmail/src/message/sync.ts | 65 +---- .../gmail/pod-gmail/src/message/syncState.ts | 74 +++++ services/gmail/pod-gmail/src/message/types.ts | 6 + services/gmail/pod-gmail/src/message/utils.ts | 31 --- services/gmail/pod-gmail/src/types.ts | 5 + 8 files changed, 360 insertions(+), 90 deletions(-) create mode 100644 services/gmail/pod-gmail/src/__tests__/syncState.test.ts create mode 100644 services/gmail/pod-gmail/src/message/syncState.ts delete mode 100644 services/gmail/pod-gmail/src/message/utils.ts diff --git a/services/gmail/pod-gmail/package.json b/services/gmail/pod-gmail/package.json index 8d0aae843e..14049289a2 100644 --- a/services/gmail/pod-gmail/package.json +++ b/services/gmail/pod-gmail/package.json @@ -36,7 +36,6 @@ "@types/express": "^4.17.13", "@types/node": "~20.11.16", "@types/sanitize-html": "^2.15.0", - "@types/turndown": "^5.0.5", "@typescript-eslint/eslint-plugin": "^6.11.0", "@typescript-eslint/parser": "^6.11.0", "esbuild": "^0.24.2", @@ -86,7 +85,6 @@ "jwt-simple": "^0.5.6", "uuid": "^8.3.2", "@hcengineering/analytics-service": "^0.6.0", - "sanitize-html": "^2.15.0", - "turndown": "^7.2.0" + "sanitize-html": "^2.15.0" } } diff --git a/services/gmail/pod-gmail/src/__tests__/syncState.test.ts b/services/gmail/pod-gmail/src/__tests__/syncState.test.ts new file mode 100644 index 0000000000..05a8c747e1 --- /dev/null +++ b/services/gmail/pod-gmail/src/__tests__/syncState.test.ts @@ -0,0 +1,253 @@ +// +// Copyright © 2025 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { PersonId } from '@hcengineering/core' +import { KeyValueClient } from '@hcengineering/kvs-client' + +import { SyncStateManager } from '../message/syncState' +import { IntegrationVersion } from '../types' +import { History } from '../message/types' + +describe('SyncStateManager', () => { + const workspace = 'test-workspace' + const userId = 'test-user-id' as PersonId + const historyId = 'test-history-id' + const pageToken = 'test-page-token' + + let mockKeyValueClient: jest.Mocked + let v1StateManager: SyncStateManager + let v2StateManager: SyncStateManager + + beforeEach(() => { + // Reset mocks before each test + mockKeyValueClient = { + getValue: jest.fn(), + setValue: jest.fn().mockResolvedValue(undefined), + deleteKey: jest.fn().mockResolvedValue(undefined) + } as unknown as jest.Mocked + + // Create state managers for both versions + v1StateManager = new SyncStateManager(mockKeyValueClient, workspace, IntegrationVersion.V1) + v2StateManager = new SyncStateManager(mockKeyValueClient, workspace, IntegrationVersion.V2) + }) + + describe('getHistory', () => { + it('should call getValue with correct key for V1', async () => { + const expectedHistory: History = { + historyId, + userId, + workspace + } + + mockKeyValueClient.getValue.mockResolvedValue(expectedHistory) + + const result = await v1StateManager.getHistory(userId) + + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`history:${workspace}:${userId}`) + expect(result).toEqual(expectedHistory) + }) + + it('should call getValue with correct key for V2', async () => { + const expectedHistory: History = { + historyId, + userId, + workspace + } + + mockKeyValueClient.getValue.mockResolvedValue(expectedHistory) + + const result = await v2StateManager.getHistory(userId) + + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`history-v2:${workspace}:${userId}`) + expect(result).toEqual(expectedHistory) + }) + + it('should return null when no history exists', async () => { + mockKeyValueClient.getValue.mockResolvedValue(null) + + const result = await v1StateManager.getHistory(userId) + + expect(result).toBeNull() + }) + + it('should propagate errors from KeyValueClient', async () => { + const error = new Error('Database error') + mockKeyValueClient.getValue.mockRejectedValue(error) + + await expect(v1StateManager.getHistory(userId)).rejects.toThrow(error) + }) + }) + + describe('clearHistory', () => { + it('should call deleteKey with correct key for V1', async () => { + await v1StateManager.clearHistory(userId) + + expect(mockKeyValueClient.deleteKey).toHaveBeenCalledWith(`history:${workspace}:${userId}`) + }) + + it('should call deleteKey with correct key for V2', async () => { + await v2StateManager.clearHistory(userId) + + expect(mockKeyValueClient.deleteKey).toHaveBeenCalledWith(`history-v2:${workspace}:${userId}`) + }) + + it('should propagate errors from KeyValueClient', async () => { + const error = new Error('Database error') + mockKeyValueClient.deleteKey.mockRejectedValue(error) + + await expect(v1StateManager.clearHistory(userId)).rejects.toThrow(error) + }) + }) + + describe('setHistoryId', () => { + it('should call setValue with correct key and value for V1', async () => { + await v1StateManager.setHistoryId(userId, historyId) + + expect(mockKeyValueClient.setValue).toHaveBeenCalledWith(`history:${workspace}:${userId}`, { + historyId, + userId, + workspace + }) + }) + + it('should call setValue with correct key and value for V2', async () => { + await v2StateManager.setHistoryId(userId, historyId) + + expect(mockKeyValueClient.setValue).toHaveBeenCalledWith(`history-v2:${workspace}:${userId}`, { + historyId, + userId, + workspace + }) + }) + + it('should propagate errors from KeyValueClient', async () => { + const error = new Error('Database error') + mockKeyValueClient.setValue.mockRejectedValue(error) + + await expect(v1StateManager.setHistoryId(userId, historyId)).rejects.toThrow(error) + }) + }) + + describe('getPageToken', () => { + it('should call getValue with correct key for V1', async () => { + mockKeyValueClient.getValue.mockResolvedValue(pageToken) + + const result = await v1StateManager.getPageToken(userId) + + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`page-token:${workspace}:${userId}`) + expect(result).toEqual(pageToken) + }) + + it('should call getValue with correct key for V2', async () => { + mockKeyValueClient.getValue.mockResolvedValue(pageToken) + + const result = await v2StateManager.getPageToken(userId) + + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`page-token-v2:${workspace}:${userId}`) + expect(result).toEqual(pageToken) + }) + + it('should return null when no page token exists', async () => { + mockKeyValueClient.getValue.mockResolvedValue(null) + + const result = await v1StateManager.getPageToken(userId) + + expect(result).toBeNull() + }) + + it('should propagate errors from KeyValueClient', async () => { + const error = new Error('Database error') + mockKeyValueClient.getValue.mockRejectedValue(error) + + await expect(v1StateManager.getPageToken(userId)).rejects.toThrow(error) + }) + }) + + describe('setPageToken', () => { + it('should call setValue with correct key and value for V1', async () => { + await v1StateManager.setPageToken(userId, pageToken) + + expect(mockKeyValueClient.setValue).toHaveBeenCalledWith(`page-token:${workspace}:${userId}`, pageToken) + }) + + it('should call setValue with correct key and value for V2', async () => { + await v2StateManager.setPageToken(userId, pageToken) + + expect(mockKeyValueClient.setValue).toHaveBeenCalledWith(`page-token-v2:${workspace}:${userId}`, pageToken) + }) + + it('should propagate errors from KeyValueClient', async () => { + const error = new Error('Database error') + mockKeyValueClient.setValue.mockRejectedValue(error) + + await expect(v1StateManager.setPageToken(userId, pageToken)).rejects.toThrow(error) + }) + }) + + describe('key generation', () => { + it('should generate different keys for V1 and V2', async () => { + // Test through the public methods to verify the keys are different + mockKeyValueClient.getValue.mockResolvedValue(null) + + await v1StateManager.getHistory(userId) + await v2StateManager.getHistory(userId) + + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`history:${workspace}:${userId}`) + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`history-v2:${workspace}:${userId}`) + + mockKeyValueClient.getValue.mockClear() + + await v1StateManager.getPageToken(userId) + await v2StateManager.getPageToken(userId) + + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`page-token:${workspace}:${userId}`) + expect(mockKeyValueClient.getValue).toHaveBeenCalledWith(`page-token-v2:${workspace}:${userId}`) + }) + }) + + describe('version migration scenario', () => { + it('should allow migrating from V1 to V2', async () => { + // Simulate V1 data existing + const v1History: History = { historyId: 'v1-history', userId, workspace } + mockKeyValueClient.getValue.mockImplementation((key: string) => { + if (key === `history:${workspace}:${userId}`) { + return Promise.resolve(v1History) + } + if (key === `history-v2:${workspace}:${userId}`) { + return Promise.resolve(null) + } + return Promise.resolve(null) + }) + + // V1 manager should find history + const historyFromV1 = await v1StateManager.getHistory(userId) + expect(historyFromV1).toEqual(v1History) + + // V2 manager should not find history yet + const historyFromV2 = await v2StateManager.getHistory(userId) + expect(historyFromV2).toBeNull() + + // Migrate by setting V2 history + await v2StateManager.setHistoryId(userId, 'v2-history') + + // Verify the call to set V2 history + expect(mockKeyValueClient.setValue).toHaveBeenCalledWith(`history-v2:${workspace}:${userId}`, { + historyId: 'v2-history', + userId, + workspace + }) + }) + }) +}) diff --git a/services/gmail/pod-gmail/src/config.ts b/services/gmail/pod-gmail/src/config.ts index 7d22251c63..a565c50e51 100644 --- a/services/gmail/pod-gmail/src/config.ts +++ b/services/gmail/pod-gmail/src/config.ts @@ -15,6 +15,7 @@ // import { BaseConfig } from '@hcengineering/mail-common' import { config as dotenvConfig } from 'dotenv' +import { IntegrationVersion } from './types' dotenvConfig() @@ -26,7 +27,7 @@ interface Config extends BaseConfig { WATCH_TOPIC_NAME: string FooterMessage: string InitLimit: number - Version: 'v1' | 'v2' + Version: IntegrationVersion } const envMap: { [key in keyof Config]: string } = { @@ -46,9 +47,12 @@ const envMap: { [key in keyof Config]: string } = { const parseNumber = (str: string | undefined): number | undefined => (str !== undefined ? Number(str) : undefined) const config: Config = (() => { - const version = process.env[envMap.Version] ?? 'v1' - if (version !== 'v1' && version !== 'v2') { - throw new Error(`Invalid version: ${version}. Must be 'v1' or 'v2'.`) + const versionStr = process.env[envMap.Version] ?? 'v1' + let version: IntegrationVersion + if (versionStr === IntegrationVersion.V1 || versionStr === IntegrationVersion.V2) { + version = versionStr as IntegrationVersion + } else { + throw new Error(`Invalid version: ${versionStr}. Must be 'v1' or 'v2'.`) } const params: Partial = { Port: parseNumber(process.env[envMap.Port]) ?? 8087, diff --git a/services/gmail/pod-gmail/src/message/sync.ts b/services/gmail/pod-gmail/src/message/sync.ts index 96b9166256..cbd9759b98 100644 --- a/services/gmail/pod-gmail/src/message/sync.ts +++ b/services/gmail/pod-gmail/src/message/sync.ts @@ -21,53 +21,22 @@ import { SyncMutex } from '@hcengineering/mail-common' import { RateLimiter } from '../rateLimiter' import { IMessageManager } from './types' - -interface History { - historyId: string - userId: string - workspace: string -} +import { SyncStateManager } from './syncState' +import config from '../config' export class SyncManager { private readonly syncMutex = new SyncMutex() + private readonly stateManager: SyncStateManager constructor ( private readonly ctx: MeasureContext, private readonly messageManager: IMessageManager, private readonly gmail: gmail_v1.Resource$Users, private readonly workspace: string, - private readonly keyValueClient: KeyValueClient, + keyValueClient: KeyValueClient, private readonly rateLimiter: RateLimiter - ) {} - - private async getHistory (userId: PersonId): Promise { - const historyKey = this.getHistoryKey(userId) - return await this.keyValueClient.getValue(historyKey) - } - - private async clearHistory (userId: PersonId): Promise { - const historyKey = this.getHistoryKey(userId) - await this.keyValueClient.deleteKey(historyKey) - } - - private async setHistoryId (userId: PersonId, historyId: string): Promise { - const historyKey = this.getHistoryKey(userId) - const history: History = { - historyId, - userId, - workspace: this.workspace - } - await this.keyValueClient.setValue(historyKey, history) - } - - private async getPageToken (userId: PersonId): Promise { - const pageTokenKey = this.getPageTokenKey(userId) - return await this.keyValueClient.getValue(pageTokenKey) - } - - private async setPageToken (userId: PersonId, pageToken: string): Promise { - const pageTokenKey = this.getPageTokenKey(userId) - await this.keyValueClient.setValue(pageTokenKey, pageToken) + ) { + this.stateManager = new SyncStateManager(keyValueClient, workspace, config.Version) } private async partSync (userId: PersonId, userEmail: string | undefined, historyId: string): Promise { @@ -87,7 +56,7 @@ export class SyncManager { }) } catch (err: any) { this.ctx.error('Part sync get history error', { workspaceUuid: this.workspace, userId, error: err.message }) - await this.clearHistory(userId) + await this.stateManager.clearHistory(userId) void this.sync(userId) return } @@ -113,7 +82,7 @@ export class SyncManager { } } if (history.id != null) { - await this.setHistoryId(userId, history.id) + await this.stateManager.setHistoryId(userId, history.id) } } if (nextPageToken == null) { @@ -134,8 +103,8 @@ export class SyncManager { throw new Error('Cannot sync without user email') } - // Get saved page token if exists to resume sync - let pageToken: string | undefined = (await this.getPageToken(userId)) ?? undefined + // Get saved page token to continue from + let pageToken: string | undefined = (await this.stateManager.getPageToken(userId)) ?? undefined const query: gmail_v1.Params$Resource$Users$Messages$List = { userId: 'me', @@ -190,11 +159,11 @@ export class SyncManager { // Update page token for the next iteration pageToken = messages.data.nextPageToken query.pageToken = pageToken - await this.setPageToken(userId, pageToken) + await this.stateManager.setPageToken(userId, pageToken) } if (currentHistoryId != null) { - await this.setHistoryId(userId, currentHistoryId) + await this.stateManager.setHistoryId(userId, currentHistoryId) } this.ctx.info('Full sync finished', { workspaceUuid: this.workspace, userId, userEmail }) } catch (err) { @@ -218,7 +187,7 @@ export class SyncManager { try { this.ctx.info('Sync history', { workspaceUuid: this.workspace, userId, userEmail }) - const history = await this.getHistory(userId) + const history = await this.stateManager.getHistory(userId) if (history?.historyId != null && history?.historyId !== '') { this.ctx.info('Start part sync', { workspaceUuid: this.workspace, userId, historyId: history.historyId }) await this.partSync(userId, userEmail, history.historyId) @@ -232,12 +201,4 @@ export class SyncManager { releaseLock() } } - - private getHistoryKey (userId: PersonId): string { - return `history:${this.workspace}:${userId}` - } - - private getPageTokenKey (userId: PersonId): string { - return `page-token:${this.workspace}:${userId}` - } } diff --git a/services/gmail/pod-gmail/src/message/syncState.ts b/services/gmail/pod-gmail/src/message/syncState.ts new file mode 100644 index 0000000000..efbb6d26bc --- /dev/null +++ b/services/gmail/pod-gmail/src/message/syncState.ts @@ -0,0 +1,74 @@ +// +// Copyright © 2025 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { PersonId } from '@hcengineering/core' +import { type KeyValueClient } from '@hcengineering/kvs-client' +import { type History } from './types' +import { IntegrationVersion } from '../types' + +/** + * Handles persistent storage for Gmail sync state + */ +export class SyncStateManager { + constructor ( + private readonly keyValueClient: KeyValueClient, + private readonly workspace: string, + private readonly version: IntegrationVersion + ) {} + + async getHistory (userId: PersonId): Promise { + const historyKey = this.getHistoryKey(userId) + return await this.keyValueClient.getValue(historyKey) + } + + async clearHistory (userId: PersonId): Promise { + const historyKey = this.getHistoryKey(userId) + await this.keyValueClient.deleteKey(historyKey) + } + + async setHistoryId (userId: PersonId, historyId: string): Promise { + const historyKey = this.getHistoryKey(userId) + const history: History = { + historyId, + userId, + workspace: this.workspace + } + await this.keyValueClient.setValue(historyKey, history) + } + + async getPageToken (userId: PersonId): Promise { + const pageTokenKey = this.getPageTokenKey(userId) + return await this.keyValueClient.getValue(pageTokenKey) + } + + async setPageToken (userId: PersonId, pageToken: string): Promise { + const pageTokenKey = this.getPageTokenKey(userId) + await this.keyValueClient.setValue(pageTokenKey, pageToken) + } + + private getHistoryKey (userId: PersonId): string { + if (this.version === IntegrationVersion.V2) { + return `history-v2:${this.workspace}:${userId}` + } + return `history:${this.workspace}:${userId}` + } + + private getPageTokenKey (userId: PersonId): string { + if (this.version === IntegrationVersion.V2) { + return `page-token-v2:${this.workspace}:${userId}` + } + return `page-token:${this.workspace}:${userId}` + } +} diff --git a/services/gmail/pod-gmail/src/message/types.ts b/services/gmail/pod-gmail/src/message/types.ts index e1797bdaf7..9c604f50c4 100644 --- a/services/gmail/pod-gmail/src/message/types.ts +++ b/services/gmail/pod-gmail/src/message/types.ts @@ -31,6 +31,12 @@ export interface EmailContact { photoUrl?: string | null } +export interface History { + historyId: string + userId: string + workspace: string +} + export interface IMessageManager { saveMessage: (message: GaxiosResponse, me: string) => Promise } diff --git a/services/gmail/pod-gmail/src/message/utils.ts b/services/gmail/pod-gmail/src/message/utils.ts deleted file mode 100644 index 261adffc8c..0000000000 --- a/services/gmail/pod-gmail/src/message/utils.ts +++ /dev/null @@ -1,31 +0,0 @@ -// -// Copyright © 2025 Hardcore Engineering Inc. -// -// Licensed under the Eclipse Public License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. You may -// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// -// See the License for the specific language governing permissions and -// limitations under the License. -// -import TurndownService from 'turndown' -import sanitizeHtml from 'sanitize-html' -import { EmailMessage } from '@hcengineering/mail-common' -import { MeasureContext } from '@hcengineering/core' - -export function getMdContent (ctx: MeasureContext, email: EmailMessage): string { - if (email.content !== undefined) { - try { - const html = sanitizeHtml(email.content) - const tds = new TurndownService() - return tds.turndown(html) - } catch (error) { - ctx.warn('Failed to parse html content', { error }) - } - } - return email.textContent -} diff --git a/services/gmail/pod-gmail/src/types.ts b/services/gmail/pod-gmail/src/types.ts index 869b079a96..1e4ddc0a74 100644 --- a/services/gmail/pod-gmail/src/types.ts +++ b/services/gmail/pod-gmail/src/types.ts @@ -67,3 +67,8 @@ export const GMAIL_INTEGRATION = 'gmail' export enum SecretType { TOKEN = 'token' } + +export enum IntegrationVersion { + V1 = 'v1', // Save messages in legacy format using gmail.class.Message + V2 = 'v2' // Save messages as thread cards and communication messages +}