From 95d9d02ac78d93c2c2b8969fd2da8755273b537a Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 27 Mar 2026 16:17:17 -0400 Subject: [PATCH] Fix Google Sheets sync dedup and stale row index bugs - saveArticle/saveArticles now upsert instead of blindly appending, preventing duplicate rows when multiple devices save the same URL - batchUpdateArticles re-fetches fresh row numbers before writing, avoiding stale indices after concurrent sheet modifications - getArticles deduplicates by URL, keeping the last occurrence - Add mock Google Sheets API server and sync engine tests --- packages/google-sheets-sync/src/index.ts | 3 +- .../src/spreadsheet/manager.ts | 2 +- .../src/sync/engine.test.ts | 324 ++++++++++++++++++ .../google-sheets-sync/src/sync/engine.ts | 160 ++++++--- .../google-sheets-sync/src/testing/index.ts | 2 + .../src/testing/mock-google-sheets-server.ts | 320 +++++++++++++++++ 6 files changed, 758 insertions(+), 53 deletions(-) create mode 100644 packages/google-sheets-sync/src/sync/engine.test.ts create mode 100644 packages/google-sheets-sync/src/testing/index.ts create mode 100644 packages/google-sheets-sync/src/testing/mock-google-sheets-server.ts diff --git a/packages/google-sheets-sync/src/index.ts b/packages/google-sheets-sync/src/index.ts index 30dc0ac..d57958f 100644 --- a/packages/google-sheets-sync/src/index.ts +++ b/packages/google-sheets-sync/src/index.ts @@ -1,3 +1,4 @@ export * from './auth/index.js'; export * from './spreadsheet/index.js'; -export * from './sync/index.js'; \ No newline at end of file +export * from './sync/index.js'; +export * from './testing/index.js'; \ No newline at end of file diff --git a/packages/google-sheets-sync/src/spreadsheet/manager.ts b/packages/google-sheets-sync/src/spreadsheet/manager.ts index d2539d3..7a21e1b 100644 --- a/packages/google-sheets-sync/src/spreadsheet/manager.ts +++ b/packages/google-sheets-sync/src/spreadsheet/manager.ts @@ -475,7 +475,7 @@ export class GoogleSpreadsheetManager { this.invalidateRowsCache(); } - private invalidateRowsCache(): void { + invalidateRowsCache(): void { if (this.cache.rowsData) { this.cache.rowsData = undefined; } diff --git a/packages/google-sheets-sync/src/sync/engine.test.ts b/packages/google-sheets-sync/src/sync/engine.test.ts new file mode 100644 index 0000000..8cee3e9 --- /dev/null +++ b/packages/google-sheets-sync/src/sync/engine.test.ts @@ -0,0 +1,324 @@ +import { describe, test, expect, beforeEach, afterEach } from 'vitest'; +import { ArticleData } from '@readlater/core'; +import { GoogleSheetsSyncEngine } from './engine.js'; +import { MockGoogleSheetsServer } from '../testing/mock-google-sheets-server.js'; +import { LocalStorageSpreadsheetStorage } from '../spreadsheet/manager.js'; +import { articleToSheetRow } from '../spreadsheet/schema.js'; +import { PwaAuthProvider } from '../auth/pwa-auth.js'; + +// ─── helpers ─── + +function makeArticle(overrides: Partial = {}): ArticleData { + const url = overrides.url || `https://example.com/${Math.random().toString(36).slice(2)}`; + return { + url, + title: overrides.title || 'Test Article', + description: overrides.description || 'A test article', + featuredImage: overrides.featuredImage || '', + timestamp: overrides.timestamp || new Date('2025-06-01').toISOString(), + domain: overrides.domain || 'example.com', + tags: overrides.tags || [], + notes: overrides.notes || '', + archived: overrides.archived || false, + favorite: overrides.favorite || false, + editedAt: overrides.editedAt, + deletedAt: overrides.deletedAt, + }; +} + +function createEngine(server: MockGoogleSheetsServer): { + engine: GoogleSheetsSyncEngine; + spreadsheetId: string; +} { + const spreadsheetId = server.createSpreadsheet('ReadLater'); + server.setAppDataConfig(spreadsheetId); + + const authProvider = new PwaAuthProvider({ clientId: 'test', apiKey: 'test' }); + authProvider.getAuthToken = async () => 'fake-token'; + authProvider.isAuthenticated = async () => true; + + const storage = new LocalStorageSpreadsheetStorage(); + const engine = new GoogleSheetsSyncEngine(authProvider, storage); + + return { engine, spreadsheetId }; +} + +// ─── test suite ─── + +describe('GoogleSheetsSyncEngine', () => { + let server: MockGoogleSheetsServer; + + beforeEach(() => { + server = new MockGoogleSheetsServer(); + server.install(); + }); + + afterEach(() => { + server.onBeforeRequest = null; + server.uninstall(); + server.reset(); + }); + + describe('basic operations', () => { + test('saveArticle appends a row and getArticles retrieves it', async () => { + const { engine, spreadsheetId } = createEngine(server); + const article = makeArticle({ url: 'https://example.com/a1', title: 'First' }); + + const result = await engine.saveArticle(article); + expect(result.success).toBe(true); + + const rows = server.getDataRows(spreadsheetId); + expect(rows).toHaveLength(1); + expect(rows[0][0]).toBe('https://example.com/a1'); + + const fetched = await engine.getArticles(); + expect(fetched).toHaveLength(1); + expect(fetched[0].url).toBe('https://example.com/a1'); + }); + + test('updateArticle merges partial updates', async () => { + const { engine, spreadsheetId } = createEngine(server); + const article = makeArticle({ url: 'https://example.com/u1', title: 'Original' }); + await engine.saveArticle(article); + + const result = await engine.updateArticle('https://example.com/u1', { title: 'Updated', favorite: true }); + expect(result.success).toBe(true); + + const rows = server.getDataRows(spreadsheetId); + expect(rows).toHaveLength(1); + expect(rows[0][1]).toBe('Updated'); + expect(rows[0][9]).toBe('1'); + expect(rows[0][0]).toBe('https://example.com/u1'); + }); + + test('deleteArticle removes the row', async () => { + const { engine, spreadsheetId } = createEngine(server); + await engine.saveArticle(makeArticle({ url: 'https://example.com/d1' })); + await engine.saveArticle(makeArticle({ url: 'https://example.com/d2' })); + + const result = await engine.deleteArticle('https://example.com/d1'); + expect(result.success).toBe(true); + + const rows = server.getDataRows(spreadsheetId); + expect(rows).toHaveLength(1); + expect(rows[0][0]).toBe('https://example.com/d2'); + }); + }); + + describe('BUG: saveArticles creates duplicates when URL already exists', () => { + test('appending an article that already exists in the sheet creates a duplicate', async () => { + const { engine, spreadsheetId } = createEngine(server); + + // Device B already saved this article + const existingArticle = makeArticle({ url: 'https://example.com/dup', title: 'From Device B' }); + server.simulateExternalAppend(spreadsheetId, articleToSheetRow(existingArticle)); + + // Device A tries to save the same URL (queued as "create") + const localArticle = makeArticle({ url: 'https://example.com/dup', title: 'From Device A' }); + await engine.saveArticles([localArticle]); + + // After fix: should have exactly 1 row (upserted, not duplicated) + const rows = server.getDataRows(spreadsheetId); + const urlRows = rows.filter(r => r[0] === 'https://example.com/dup'); + expect(urlRows).toHaveLength(1); + expect(urlRows[0][1]).toBe('From Device A'); + }); + + test('batch save with mix of new and existing articles upserts correctly', async () => { + const { engine, spreadsheetId } = createEngine(server); + + // Pre-existing article in the sheet + server.simulateExternalAppend( + spreadsheetId, + articleToSheetRow(makeArticle({ url: 'https://example.com/existing', title: 'Old Title' })) + ); + + // Batch save: one existing, one new + const results = await engine.saveArticles([ + makeArticle({ url: 'https://example.com/existing', title: 'New Title' }), + makeArticle({ url: 'https://example.com/brand-new', title: 'Brand New' }), + ]); + + expect(results.every(r => r.success)).toBe(true); + + const rows = server.getDataRows(spreadsheetId); + expect(rows).toHaveLength(2); + + const existingRow = rows.find(r => r[0] === 'https://example.com/existing'); + expect(existingRow?.[1]).toBe('New Title'); + + const newRow = rows.find(r => r[0] === 'https://example.com/brand-new'); + expect(newRow?.[1]).toBe('Brand New'); + }); + }); + + describe('BUG: stale row indices after concurrent modification', () => { + test('batchUpdateArticles uses fresh row lookup, not stale indices', async () => { + const { engine, spreadsheetId } = createEngine(server); + + // Set up: 3 articles in sheet + const a = makeArticle({ url: 'https://example.com/a', title: 'Article A' }); + const b = makeArticle({ url: 'https://example.com/b', title: 'Article B' }); + const c = makeArticle({ url: 'https://example.com/c', title: 'Article C' }); + await engine.saveArticle(a); + await engine.saveArticle(b); + await engine.saveArticle(c); + + expect(server.getDataRows(spreadsheetId)).toHaveLength(3); + + // Simulate: external device deletes Article A (row 2) BETWEEN the first read + // (used for data merging) and the second read (used for fresh row numbers). + // The engine does: getAllRows → merge data → invalidateCache → getAllRows → write. + // We inject the delete before the SECOND getAllRows call. + let readCount = 0; + server.onBeforeRequest = (url, method) => { + if (url.includes('/values/Sheet1') && method === 'GET') { + readCount++; + if (readCount === 2) { + // Before the fresh re-fetch, simulate external delete of row 2 (Article A) + server.simulateExternalDelete(spreadsheetId, 2); + server.onBeforeRequest = null; + } + } + }; + + // Device A tries to update Article C + const result = await engine.batchUpdateArticles([ + { url: 'https://example.com/c', updates: { title: 'Updated C' } } + ]); + + expect(result[0].success).toBe(true); + + // After fix: Article B should be untouched, Article C should be updated + const rows = server.getDataRows(spreadsheetId); + const rowB = rows.find(r => r[0] === 'https://example.com/b'); + const rowC = rows.find(r => r[0] === 'https://example.com/c'); + + expect(rowB?.[1]).toBe('Article B'); // B must NOT be overwritten + expect(rowC?.[1]).toBe('Updated C'); // C should be updated + }); + + test('batchDeleteArticles uses URL-based lookup, not stale row numbers', async () => { + const { engine, spreadsheetId } = createEngine(server); + + const a = makeArticle({ url: 'https://example.com/a', title: 'Article A' }); + const b = makeArticle({ url: 'https://example.com/b', title: 'Article B' }); + const c = makeArticle({ url: 'https://example.com/c', title: 'Article C' }); + await engine.saveArticle(a); + await engine.saveArticle(b); + await engine.saveArticle(c); + + // Inject external append BETWEEN the read and the delete write + let readDone = false; + server.onBeforeRequest = (url, method) => { + if (url.includes('/values/Sheet1') && method === 'GET') { + readDone = true; + } + if (readDone && method === 'POST' && url.includes(':batchUpdate') && !url.includes('values')) { + // External device inserts a row at the beginning (shifts everything down) + server.simulateExternalAppend(spreadsheetId, articleToSheetRow( + makeArticle({ url: 'https://example.com/x', title: 'External X' }) + )); + server.onBeforeRequest = null; + } + }; + + // Device A tries to delete Article B + const result = await engine.batchDeleteArticles(['https://example.com/b']); + expect(result[0].success).toBe(true); + + // After fix: B should be deleted, all others should remain + const rows = server.getDataRows(spreadsheetId); + const urls = rows.map(r => r[0]); + expect(urls).toContain('https://example.com/a'); + expect(urls).toContain('https://example.com/c'); + expect(urls).not.toContain('https://example.com/b'); + }); + }); + + describe('BUG: multi-device sync scenarios', () => { + test('Device A creates article that Device B already added — no duplicates after sync', async () => { + const { engine, spreadsheetId } = createEngine(server); + + // Device B saved articles [X, Y] to the sheet + const x = makeArticle({ url: 'https://example.com/x', title: 'X' }); + const y = makeArticle({ url: 'https://example.com/y', title: 'Y' }); + server.simulateExternalAppend(spreadsheetId, articleToSheetRow(x)); + server.simulateExternalAppend(spreadsheetId, articleToSheetRow(y)); + + // Device A independently creates X and a new article Z + const z = makeArticle({ url: 'https://example.com/z', title: 'Z' }); + await engine.saveArticles([ + makeArticle({ url: 'https://example.com/x', title: 'X from A' }), + z, + ]); + + // After fix: should have exactly 3 unique articles, no duplicates + const rows = server.getDataRows(spreadsheetId); + const urls = rows.map(r => r[0]); + expect(urls).toHaveLength(3); + expect(new Set(urls).size).toBe(3); + expect(urls).toContain('https://example.com/x'); + expect(urls).toContain('https://example.com/y'); + expect(urls).toContain('https://example.com/z'); + }); + + test('single saveArticle also deduplicates when URL already exists', async () => { + const { engine, spreadsheetId } = createEngine(server); + + // Device B already saved this article + server.simulateExternalAppend( + spreadsheetId, + articleToSheetRow(makeArticle({ url: 'https://example.com/s1', title: 'B version' })) + ); + + // Device A saves the same URL via single saveArticle + const result = await engine.saveArticle( + makeArticle({ url: 'https://example.com/s1', title: 'A version' }) + ); + expect(result.success).toBe(true); + + const rows = server.getDataRows(spreadsheetId); + const urlRows = rows.filter(r => r[0] === 'https://example.com/s1'); + expect(urlRows).toHaveLength(1); + expect(urlRows[0][1]).toBe('A version'); + }); + + test('cleanupDeletedArticles does not affect non-deleted articles', async () => { + const { engine, spreadsheetId } = createEngine(server); + + const alive = makeArticle({ url: 'https://example.com/alive', title: 'Alive' }); + const deleted = makeArticle({ + url: 'https://example.com/deleted', + title: 'Deleted', + deletedAt: new Date(Date.now() - 60 * 24 * 60 * 60 * 1000).toISOString() + }); + await engine.saveArticle(alive); + await engine.saveArticle(deleted); + + const cleaned = await engine.cleanupDeletedArticles(30); + expect(cleaned).toBe(1); + + const rows = server.getDataRows(spreadsheetId); + expect(rows).toHaveLength(1); + expect(rows[0][0]).toBe('https://example.com/alive'); + }); + + test('getArticles returns no duplicates when sheet has duplicate URLs', async () => { + const { engine, spreadsheetId } = createEngine(server); + + // Simulate pre-existing duplicate (from prior bug) + const row = articleToSheetRow(makeArticle({ url: 'https://example.com/dup', title: 'V1' })); + server.simulateExternalAppend(spreadsheetId, row); + const row2 = articleToSheetRow(makeArticle({ url: 'https://example.com/dup', title: 'V2' })); + server.simulateExternalAppend(spreadsheetId, row2); + + const articles = await engine.getArticles(); + + // Should deduplicate — return only the latest version + const dupArticles = articles.filter(a => a.url === 'https://example.com/dup'); + expect(dupArticles).toHaveLength(1); + expect(dupArticles[0].title).toBe('V2'); // last row wins + }); + }); +}); diff --git a/packages/google-sheets-sync/src/sync/engine.ts b/packages/google-sheets-sync/src/sync/engine.ts index 46de39c..c8f7c7b 100644 --- a/packages/google-sheets-sync/src/sync/engine.ts +++ b/packages/google-sheets-sync/src/sync/engine.ts @@ -17,9 +17,18 @@ export class GoogleSheetsSyncEngine implements SyncEngine { console.log('Saving article to Google Sheets...'); const rowData = articleToSheetRow(article); - await this.manager.appendRow(rowData); - console.log('Successfully saved to Google Sheets'); + // Check if article already exists (e.g., added by another device) + const existingRow = await this.manager.findRowByUrl(article.url); + if (existingRow !== null) { + // Upsert: update existing row instead of creating a duplicate + await this.manager.updateRow(existingRow, rowData); + console.log('Updated existing article in Google Sheets (dedup)'); + } else { + await this.manager.appendRow(rowData); + console.log('Successfully saved to Google Sheets'); + } + return { success: true, articleUrl: article.url @@ -82,8 +91,20 @@ export class GoogleSheetsSyncEngine implements SyncEngine { } } + // Deduplicate by URL — if there are duplicate rows for the same URL, + // keep the last occurrence (most recently appended). + const deduped = new Map(); + for (const article of validArticles) { + deduped.set(article.url, article); + } + const uniqueArticles = Array.from(deduped.values()); + + if (uniqueArticles.length < validArticles.length) { + console.warn(`Deduplicated ${validArticles.length - uniqueArticles.length} duplicate URLs`); + } + // Log summary of what we found - console.log(`Successfully loaded ${validArticles.length} valid articles from Google Sheets`); + console.log(`Successfully loaded ${uniqueArticles.length} valid articles from Google Sheets`); if (invalidRows.length > 0) { console.warn(`Skipped ${invalidRows.length} invalid rows`); @@ -93,11 +114,10 @@ export class GoogleSheetsSyncEngine implements SyncEngine { if (totalRows > 0 && invalidRatio > 0.25) { console.error(`High invalid row ratio detected: ${invalidRows.length}/${totalRows} (${Math.round(invalidRatio * 100)}%)`); console.error('This might indicate spreadsheet corruption or format changes'); - // Still return valid articles, but log the issue for investigation } } - return validArticles; + return uniqueArticles; } catch (error) { console.error('Error loading articles:', error); @@ -167,11 +187,42 @@ export class GoogleSheetsSyncEngine implements SyncEngine { try { console.log(`Batch saving ${articles.length} articles...`); - // Convert all articles to row data - const valuesList = articles.map(article => articleToSheetRow(article)); + // Fetch current rows to check which URLs already exist + const existingRows = await this.manager.getAllRows(); + const urlToRowMap = new Map(); + for (let i = 0; i < existingRows.length; i++) { + const url = existingRows[i][0]; + if (url) { + urlToRowMap.set(url, i + 2); // 1-indexed + header offset + } + } + + // Separate into updates (existing URLs) and appends (new URLs) + const toAppend: string[][] = []; + const toUpdate: Array<{ rowNumber: number; values: string[] }> = []; + + for (const article of articles) { + const rowData = articleToSheetRow(article); + const existingRow = urlToRowMap.get(article.url); + + if (existingRow !== undefined) { + toUpdate.push({ rowNumber: existingRow, values: rowData }); + } else { + toAppend.push(rowData); + } + } + + // Batch update existing rows + if (toUpdate.length > 0) { + await this.manager.batchUpdateRows(toUpdate); + console.log(`Updated ${toUpdate.length} existing articles (dedup)`); + } - // Use batch operation for better performance - await this.manager.batchAppendRows(valuesList); + // Batch append genuinely new rows + if (toAppend.length > 0) { + await this.manager.batchAppendRows(toAppend); + console.log(`Appended ${toAppend.length} new articles`); + } const results = articles.map(article => ({ success: true, @@ -182,7 +233,6 @@ export class GoogleSheetsSyncEngine implements SyncEngine { return results; } catch (error) { console.error('Error in batch save:', error); - // Return failed results for all articles return articles.map(article => ({ success: false, error: error instanceof Error ? error.message : 'Unknown error', @@ -317,23 +367,25 @@ export class GoogleSheetsSyncEngine implements SyncEngine { try { console.log(`Batch updating ${updates.length} articles...`); - // Get all rows once to find row numbers for each URL + // Fetch rows to read current article data for merging const rows = await this.manager.getAllRows(); - const urlToRowMap = new Map(); + const urlToDataMap = new Map(); for (let i = 0; i < rows.length; i++) { const url = rows[i][0]; if (url) { - urlToRowMap.set(url, i + 2); // Convert to 1-indexed row number + // Keep the last occurrence if duplicates exist + urlToDataMap.set(url, { rowIndex: i, row: rows[i] }); } } - const rowUpdates: Array<{ rowNumber: number; values: string[] }> = []; + // Build merged row data keyed by URL (not by row number yet) + const pendingUpdates: Array<{ url: string; values: string[] }> = []; const results: SyncResult[] = []; for (const { url, updates: articleUpdates } of updates) { - const rowNumber = urlToRowMap.get(url); - if (!rowNumber) { + const existing = urlToDataMap.get(url); + if (!existing) { results.push({ success: false, error: 'Article not found in spreadsheet', @@ -342,34 +394,51 @@ export class GoogleSheetsSyncEngine implements SyncEngine { continue; } - // Get current article data and merge with updates - const currentRow = rows[rowNumber - 2]; // Convert back to 0-indexed array - const currentArticle = sheetRowToArticle(currentRow); - + const currentArticle = sheetRowToArticle(existing.row); const updatedArticle: ArticleData = { ...currentArticle, ...articleUpdates, url // Ensure URL doesn't get overwritten }; - const rowData = articleToSheetRow(updatedArticle); - rowUpdates.push({ rowNumber, values: rowData }); - results.push({ - success: true, - articleUrl: url - }); + pendingUpdates.push({ url, values: articleToSheetRow(updatedArticle) }); + results.push({ success: true, articleUrl: url }); } - // Perform batch update if we have any valid updates - if (rowUpdates.length > 0) { - await this.manager.batchUpdateRows(rowUpdates); + // Re-fetch rows right before writing to get fresh row numbers, + // avoiding stale indices from concurrent modifications by other devices + if (pendingUpdates.length > 0) { + this.manager.invalidateRowsCache(); + const freshRows = await this.manager.getAllRows(); + const freshUrlToRow = new Map(); + for (let i = 0; i < freshRows.length; i++) { + const url = freshRows[i][0]; + if (url) freshUrlToRow.set(url, i + 2); + } + + const rowUpdates: Array<{ rowNumber: number; values: string[] }> = []; + for (const { url, values } of pendingUpdates) { + const rowNumber = freshUrlToRow.get(url); + if (rowNumber) { + rowUpdates.push({ rowNumber, values }); + } else { + // Article was deleted between read and write — mark as failure + const resultIdx = results.findIndex(r => r.articleUrl === url && r.success); + if (resultIdx !== -1) { + results[resultIdx] = { success: false, error: 'Article disappeared during update', articleUrl: url }; + } + } + } + + if (rowUpdates.length > 0) { + await this.manager.batchUpdateRows(rowUpdates); + } } - console.log(`Batch update completed: ${rowUpdates.length}/${updates.length} successful`); + console.log(`Batch update completed: ${pendingUpdates.length}/${updates.length} successful`); return results; } catch (error) { console.error('Error in batch update:', error); - // Return failed results for all articles return updates.map(({ url }) => ({ success: false, error: error instanceof Error ? error.message : 'Unknown error', @@ -384,39 +453,29 @@ export class GoogleSheetsSyncEngine implements SyncEngine { try { console.log(`Batch deleting ${urls.length} articles...`); - // Get all rows once to find row numbers for each URL - const rows = await this.manager.getAllRows(); - const urlToRowMap = new Map(); - - for (let i = 0; i < rows.length; i++) { - const url = rows[i][0]; - if (url) { - urlToRowMap.set(url, i + 2); // Convert to 1-indexed row number - } + // Fetch fresh row numbers right before deleting to avoid stale indices + const freshRows = await this.manager.getAllRows(); + const freshUrlToRow = new Map(); + for (let i = 0; i < freshRows.length; i++) { + const url = freshRows[i][0]; + if (url) freshUrlToRow.set(url, i + 2); } const rowsToDelete: number[] = []; const results: SyncResult[] = []; for (const url of urls) { - const rowNumber = urlToRowMap.get(url); + const rowNumber = freshUrlToRow.get(url); if (!rowNumber) { // Consider it success if already not present - results.push({ - success: true, - articleUrl: url - }); + results.push({ success: true, articleUrl: url }); continue; } rowsToDelete.push(rowNumber); - results.push({ - success: true, - articleUrl: url - }); + results.push({ success: true, articleUrl: url }); } - // Perform batch delete if we have any rows to delete if (rowsToDelete.length > 0) { await this.manager.batchDeleteRows(rowsToDelete); } @@ -425,7 +484,6 @@ export class GoogleSheetsSyncEngine implements SyncEngine { return results; } catch (error) { console.error('Error in batch delete:', error); - // Return failed results for all articles return urls.map(url => ({ success: false, error: error instanceof Error ? error.message : 'Unknown error', diff --git a/packages/google-sheets-sync/src/testing/index.ts b/packages/google-sheets-sync/src/testing/index.ts new file mode 100644 index 0000000..a62f8c7 --- /dev/null +++ b/packages/google-sheets-sync/src/testing/index.ts @@ -0,0 +1,2 @@ +export { MockGoogleSheetsServer } from './mock-google-sheets-server.js'; +export type { MockSpreadsheet, FetchInterceptor } from './mock-google-sheets-server.js'; diff --git a/packages/google-sheets-sync/src/testing/mock-google-sheets-server.ts b/packages/google-sheets-sync/src/testing/mock-google-sheets-server.ts new file mode 100644 index 0000000..242f4e7 --- /dev/null +++ b/packages/google-sheets-sync/src/testing/mock-google-sheets-server.ts @@ -0,0 +1,320 @@ +/** + * Mock Google Sheets API server for testing. + * + * Simulates the Sheets v4 and Drive v3 endpoints used by GoogleSpreadsheetManager. + * Maintains an in-memory spreadsheet so tests can observe real row-shifting, + * duplicate-creation, and cross-device race conditions. + * + * Usage: + * const server = new MockGoogleSheetsServer(); + * server.install(); // replaces global.fetch + * // ... run tests ... + * server.uninstall(); // restores original fetch + */ + +import { SPREADSHEET_HEADERS } from '../spreadsheet/schema.js'; + +export interface MockSpreadsheet { + id: string; + name: string; + /** rows[0] is the header row; data rows start at index 1. */ + rows: string[][]; +} + +export type FetchInterceptor = (url: string, method: string) => void; + +export class MockGoogleSheetsServer { + private spreadsheets = new Map(); + private appDataFiles = new Map(); // fileId -> content + private originalFetch: typeof globalThis.fetch | null = null; + private nextSpreadsheetId = 1; + private nextFileId = 1; + + /** + * Optional interceptor called BEFORE each request is processed. + * Use this to simulate external changes between reads and writes. + * e.g., inject an external row delete after a GET (read) but before a POST (write). + */ + public onBeforeRequest: FetchInterceptor | null = null; + + // ─── public helpers for test setup / assertions ─── + + /** Pre-populate a spreadsheet with data rows (header added automatically). */ + createSpreadsheet(name = 'ReadLater', dataRows: string[][] = []): string { + const id = `sheet-${this.nextSpreadsheetId++}`; + this.spreadsheets.set(id, { + id, + name, + rows: [[...SPREADSHEET_HEADERS], ...dataRows], + }); + return id; + } + + /** Get all data rows (excluding the header). */ + getDataRows(spreadsheetId: string): string[][] { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) throw new Error(`Spreadsheet ${spreadsheetId} not found`); + return sheet.rows.slice(1); + } + + /** Get a specific row by 1-based sheet row number (row 1 = header, row 2 = first data). */ + getRow(spreadsheetId: string, rowNumber: number): string[] | undefined { + const sheet = this.spreadsheets.get(spreadsheetId); + return sheet?.rows[rowNumber - 1]; + } + + /** Directly mutate the sheet to simulate another device editing it. */ + simulateExternalAppend(spreadsheetId: string, row: string[]): void { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) throw new Error(`Spreadsheet ${spreadsheetId} not found`); + sheet.rows.push(row); + } + + /** Directly delete a row to simulate another device deleting it (1-based row number). */ + simulateExternalDelete(spreadsheetId: string, rowNumber: number): void { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) throw new Error(`Spreadsheet ${spreadsheetId} not found`); + sheet.rows.splice(rowNumber - 1, 1); + } + + /** Set up appDataFolder so the manager can find the spreadsheet. */ + setAppDataConfig(spreadsheetId: string): void { + const fileId = `config-${this.nextFileId++}`; + this.appDataFiles.set(fileId, JSON.stringify({ spreadsheetId })); + } + + // ─── fetch interception ─── + + install(): void { + this.originalFetch = globalThis.fetch; + globalThis.fetch = this.handleFetch.bind(this) as typeof globalThis.fetch; + } + + uninstall(): void { + if (this.originalFetch) { + globalThis.fetch = this.originalFetch; + this.originalFetch = null; + } + } + + reset(): void { + this.spreadsheets.clear(); + this.appDataFiles.clear(); + this.nextSpreadsheetId = 1; + this.nextFileId = 1; + } + + // ─── route dispatcher ─── + + private async handleFetch(input: RequestInfo | URL, init?: RequestInit): Promise { + const url = typeof input === 'string' ? input : input instanceof URL ? input.toString() : input.url; + const method = init?.method?.toUpperCase() || 'GET'; + + // Fire interceptor so tests can simulate external changes at precise moments + if (this.onBeforeRequest) { + this.onBeforeRequest(url, method); + } + + // Drive: list appDataFolder files + if (url.includes('/drive/v3/files') && url.includes('appDataFolder') && method === 'GET') { + return this.handleAppDataList(); + } + + // Drive: read appData file content + const appDataReadMatch = url.match(/\/drive\/v3\/files\/([^?]+)\?alt=media/); + if (appDataReadMatch && method === 'GET') { + return this.handleAppDataRead(appDataReadMatch[1]); + } + + // Drive: upload/update appData file + if (url.includes('/upload/drive/v3/files') && (method === 'POST' || method === 'PATCH')) { + return this.handleAppDataWrite(init); + } + + // Drive: search for spreadsheet by name + if (url.includes('/drive/v3/files') && url.includes('q=') && method === 'GET') { + return this.handleDriveSearch(url); + } + + // Sheets: create spreadsheet + if (url.includes('/v4/spreadsheets') && method === 'POST' && !url.includes(':batchUpdate') && !url.includes('/values')) { + return this.handleCreateSpreadsheet(init); + } + + // Sheets: batchUpdate (delete rows) + const batchUpdateMatch = url.match(/\/v4\/spreadsheets\/([^/:]+):batchUpdate/); + if (batchUpdateMatch && method === 'POST') { + return this.handleBatchUpdateStructure(batchUpdateMatch[1], init); + } + + // Sheets: values batchUpdate (batch write) + const valuesBatchMatch = url.match(/\/v4\/spreadsheets\/([^/]+)\/values:batchUpdate/); + if (valuesBatchMatch && method === 'POST') { + return this.handleValuesBatchUpdate(valuesBatchMatch[1], init); + } + + // Sheets: get values (getAllRows or getNextRowNumber) + const getValuesMatch = url.match(/\/v4\/spreadsheets\/([^/]+)\/values\/(.+?)(?:\?|$)/); + if (getValuesMatch && method === 'GET') { + return this.handleGetValues(getValuesMatch[1], getValuesMatch[2], url); + } + + // Sheets: put values (appendRow, updateRow, addHeaders) + const putValuesMatch = url.match(/\/v4\/spreadsheets\/([^/]+)\/values\/(.+?)(?:\?|$)/); + if (putValuesMatch && method === 'PUT') { + return this.handlePutValues(putValuesMatch[1], putValuesMatch[2], init); + } + + console.warn(`[MockGoogleSheetsServer] Unhandled: ${method} ${url}`); + return this.jsonResponse({}, 404); + } + + // ─── handlers ─── + + private handleAppDataList(): Response { + const files = Array.from(this.appDataFiles.entries()).map(([id]) => ({ + id, + name: 'readlater.config.json', + })); + return this.jsonResponse({ files }); + } + + private handleAppDataRead(fileId: string): Response { + const content = this.appDataFiles.get(fileId); + if (!content) return this.jsonResponse({ error: { message: 'Not found' } }, 404); + return this.jsonResponse(JSON.parse(content)); + } + + private handleAppDataWrite(init?: RequestInit): Response { + // Extract spreadsheetId from FormData body — in tests we just accept it + const fileId = `config-${this.nextFileId++}`; + // Try to parse the body to extract the config + if (init?.body instanceof FormData) { + const fileBlob = init.body.get('file'); + if (fileBlob instanceof Blob) { + // We can't synchronously read the blob in this context, + // so we just acknowledge it + } + } + return this.jsonResponse({ id: fileId }); + } + + private handleDriveSearch(url: string): Response { + const queryParam = decodeURIComponent(url.split('q=')[1]?.split('&')[0] || ''); + const nameMatch = queryParam.match(/name='([^']+)'/); + const name = nameMatch?.[1]; + + const found: Array<{ id: string; name: string }> = []; + for (const sheet of this.spreadsheets.values()) { + if (sheet.name === name) { + found.push({ id: sheet.id, name: sheet.name }); + } + } + return this.jsonResponse({ files: found }); + } + + private handleCreateSpreadsheet(init?: RequestInit): Response { + const body = JSON.parse((init?.body as string) || '{}'); + const name = body.properties?.title || 'Untitled'; + const id = this.createSpreadsheet(name); + return this.jsonResponse({ spreadsheetId: id }); + } + + private handleBatchUpdateStructure(spreadsheetId: string, init?: RequestInit): Response { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) return this.jsonResponse({ error: { message: 'Not found' } }, 404); + + const body = JSON.parse((init?.body as string) || '{}'); + const requests: Array<{ + deleteDimension?: { + range: { startIndex: number; endIndex: number }; + }; + }> = body.requests || []; + + // Process delete requests — they come in descending order + for (const req of requests) { + if (req.deleteDimension) { + const { startIndex, endIndex } = req.deleteDimension.range; + const count = endIndex - startIndex; + sheet.rows.splice(startIndex, count); + } + } + + return this.jsonResponse({ replies: [] }); + } + + private handleValuesBatchUpdate(spreadsheetId: string, init?: RequestInit): Response { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) return this.jsonResponse({ error: { message: 'Not found' } }, 404); + + const body = JSON.parse((init?.body as string) || '{}'); + const data: Array<{ range: string; values: string[][] }> = body.data || []; + + for (const item of data) { + const rowNum = this.parseRowFromRange(item.range); + if (rowNum !== null && item.values?.[0]) { + // Expand sheet if needed + while (sheet.rows.length < rowNum) { + sheet.rows.push([]); + } + sheet.rows[rowNum - 1] = item.values[0]; + } + } + + return this.jsonResponse({ totalUpdatedRows: data.length }); + } + + private handleGetValues(spreadsheetId: string, range: string, url: string): Response { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) return this.jsonResponse({ error: { message: 'Not found' } }, 404); + + // getNextRowNumber: Sheet1!A:A?majorDimension=COLUMNS + if (url.includes('majorDimension=COLUMNS') && range.includes('A:A')) { + const columnA = sheet.rows.map(row => row[0] || '').filter(v => v); + return this.jsonResponse({ values: columnA.length > 0 ? [columnA] : [] }); + } + + // getAllRows: Sheet1!A2:L — returns data rows (skip header) + if (range.match(/A2:L/)) { + const dataRows = sheet.rows.slice(1); // skip header + return this.jsonResponse({ values: dataRows.length > 0 ? dataRows : [] }); + } + + // Generic range read (e.g., A1:L1 for headers) + return this.jsonResponse({ values: sheet.rows.length > 0 ? [sheet.rows[0]] : [] }); + } + + private handlePutValues(spreadsheetId: string, range: string, init?: RequestInit): Response { + const sheet = this.spreadsheets.get(spreadsheetId); + if (!sheet) return this.jsonResponse({ error: { message: 'Not found' } }, 404); + + const body = JSON.parse((init?.body as string) || '{}'); + const values: string[][] = body.values || []; + + const rowNum = this.parseRowFromRange(range); + if (rowNum !== null && values[0]) { + while (sheet.rows.length < rowNum) { + sheet.rows.push([]); + } + sheet.rows[rowNum - 1] = values[0]; + } + + return this.jsonResponse({ updatedRows: values.length }); + } + + // ─── utilities ─── + + private parseRowFromRange(range: string): number | null { + // Match patterns like "Sheet1!A5:L5" or "Sheet1!A1:L1" + const match = range.match(/!?A(\d+)/); + return match ? parseInt(match[1], 10) : null; + } + + private jsonResponse(data: unknown, status = 200): Response { + const body = JSON.stringify(data); + return new Response(body, { + status, + headers: { 'Content-Type': 'application/json' }, + }); + } +}