experiments in a post-browser web
10
fork

Configure Feed

Select the types of activity you want to include in your feed.

docs: syncSource migration test plan

+185
+185
notes/plan-syncsource-migration-test.md
··· 1 + # syncSource Migration Test Plan 2 + 3 + ## Background 4 + 5 + The `syncSource` column was removed from desktop and server schemas as part of the sync refactor. Device metadata now lives inside the item's `metadata` JSON field as `_sync: {createdBy, createdAt, modifiedBy, modifiedAt}` with raw UUIDs (no prefixes). 6 + 7 + ### Current State by Platform 8 + 9 + | Platform | syncSource Status | Details | 10 + |----------|------------------|---------| 11 + | Desktop Electron (`backend/electron/`) | Removed from schema + code | No `syncSource` in generated schema or sync code | 12 + | Server (`backend/server/`) | Removed from schema + code | No `syncSource` in db.js or index.js | 13 + | Unified sync module (`sync/sync.js`) | **STILL USES IT** | Push filter, post-push update, pending count all reference `syncSource` | 14 + | iOS mobile (`backend/tauri-mobile/`) | Still has `sync_source` | Actively used for push filtering, merge, post-push | 15 + | Tauri Desktop (`backend/tauri/`) | Still has `sync_source` | Item struct, column access, sync operations | 16 + | Browser Extension (`backend/extension/`) | Clean | No references | 17 + 18 + ## Issues Found During Investigation 19 + 20 + ### Critical: `sync/sync.js` not refactored 21 + 22 + The unified sync module still actively uses `syncSource`: 23 + - `sync.js:102,107` — push filter: `i.syncSource === ''` 24 + - `sync.js:165` — sets `syncSource: 'server'` after push 25 + - `sync.js:217` — pending count: `i.syncSource === ''` 26 + - `sync.js:279` — server change reset: `syncSource: ''` 27 + - `better-sqlite3.js:34` — adapter schema includes `syncSource TEXT DEFAULT ''` 28 + 29 + If this module is used against a DB without `syncSource`, the `i.syncSource === ''` filter evaluates `undefined === ''` → `false`, causing all items to be excluded from push. 30 + 31 + ### Stale test assertion 32 + 33 + `backend/server/test.js:620` asserts `syncSource` column exists: 34 + ```js 35 + assert.ok(columnNames.includes("syncSource")) 36 + ``` 37 + This will fail with the current schema. 38 + 39 + ### Behavioral change: extension-origin items 40 + 41 + Items with `syncSource = 'history'|'tab'|'bookmark'` from the browser extension previously had `syncedAt = 0` and were excluded from push by the old `syncSource !== ''` guard. After migration, `syncedAt = 0` means "never synced, should push" under the new algorithm → these items will be pushed on first sync after upgrade. This is intentional per the refactor design. 42 + 43 + ## Migration Behavior 44 + 45 + ### Desktop (Electron) 46 + 47 + - **Schema creation**: `CREATE TABLE items` does NOT include `syncSource` 48 + - **Migration**: `migrateSyncColumns` only adds `syncId` — does NOT add or remove `syncSource` 49 + - **Table rebuild** (`migrateItemTypes`): If CHECK constraint update triggers, table is rebuilt WITHOUT `syncSource` → column silently dropped 50 + - **If no rebuild triggers**: `syncSource` persists as orphaned column 51 + - **SELECT * queries**: Return `syncSource` if it exists, but TypeScript `Item` interface ignores it 52 + 53 + ### Server 54 + 55 + - **Schema creation**: No `syncSource` in CREATE TABLE 56 + - **Column renames**: Maps `sync_id`→`syncId`, `synced_at`→`syncedAt`, etc. — NO mapping for `sync_source`/`syncSource` 57 + - **Table rebuild** (`rebuildTableIfNeeded`): If triggered by pending renames, target schema excludes `syncSource` → dropped 58 + - **If no rebuild triggers**: `syncSource` persists as orphaned column 59 + - **Queries**: Use explicit column lists (not SELECT *) → orphaned column is harmless 60 + - **Validation**: `validateSchema()` checks `REQUIRED_SYNC_COLUMNS` from `schema/v1.json` — extra columns ignored 61 + 62 + ### iOS Mobile 63 + 64 + No changes. Still uses `sync_source` for push filtering, merge operations, and post-push updates. iOS has its own local DB schema — not affected by desktop/server refactor. 65 + 66 + ## Test Scenarios 67 + 68 + ### Scenario 1: Server DB with snake_case schema (full migration path) 69 + 70 + **Setup:** Create DB with old snake_case schema including `sync_source`: 71 + ```sql 72 + CREATE TABLE items ( 73 + id TEXT PRIMARY KEY, type TEXT NOT NULL, content TEXT, metadata TEXT, 74 + sync_id TEXT DEFAULT '', sync_source TEXT DEFAULT '', 75 + synced_at INTEGER DEFAULT 0, created_at INTEGER NOT NULL, 76 + updated_at INTEGER NOT NULL, deleted_at INTEGER DEFAULT 0 77 + ); 78 + ``` 79 + 80 + Seed items: 81 + - Item A: `sync_source = ''`, `synced_at = 0` (local, never synced) 82 + - Item B: `sync_source = 'server'`, `synced_at = 1000`, `sync_id = 'remote-1'` (pulled from server) 83 + - Item C: `sync_source = 'server'`, `synced_at = 1000`, `sync_id = 'remote-2'`, `deleted_at = 5000` (soft-deleted synced item) 84 + 85 + **Expected:** 86 + - `rebuildTableIfNeeded` triggers (snake→camelCase renames pending) 87 + - All snake_case columns renamed; `sync_source` dropped (not in target schema) 88 + - `PRAGMA table_info(items)` does NOT contain `syncSource` or `sync_source` 89 + - All data preserved with correct values 90 + - `validateSchema()` passes 91 + 92 + ### Scenario 2: Server DB with camelCase schema + orphaned `syncSource` (no rebuild) 93 + 94 + **Setup:** DB already has camelCase columns but also has `syncSource`: 95 + ```sql 96 + CREATE TABLE items ( 97 + id TEXT PRIMARY KEY, type TEXT NOT NULL CHECK(type IN ('url','text','tagset','image')), 98 + content TEXT, metadata TEXT, syncId TEXT DEFAULT '', syncSource TEXT DEFAULT '', 99 + syncedAt INTEGER DEFAULT 0, createdAt INTEGER NOT NULL, 100 + updatedAt INTEGER NOT NULL, deletedAt INTEGER DEFAULT 0 101 + ); 102 + ``` 103 + 104 + **Expected:** 105 + - `rebuildTableIfNeeded` does NOT trigger (no pending renames) 106 + - `syncSource` column persists (orphaned but harmless) 107 + - `validateSchema()` passes (extra columns ignored) 108 + - All operations (saveItem, getItems, getItemsSince) work correctly 109 + 110 + ### Scenario 3: Items with various `syncSource` values survive migration 111 + 112 + **Setup:** Scenario 1 DB with additional items: 113 + - Item D: `sync_source = 'history'`, `synced_at = 0` (extension import) 114 + - Item E: `sync_source = 'bookmark'`, `synced_at = 0` (extension import) 115 + - Item F: `sync_source = 'tab'`, `synced_at = 0` (extension import) 116 + 117 + **Expected:** 118 + - All items survive migration with correct data 119 + - Under new push algorithm (`syncedAt = 0` → push), items D/E/F become eligible for push 120 + - `getItems` returns all non-deleted items 121 + 122 + ### Scenario 4: Server receives items WITHOUT `syncSource` from refactored desktop 123 + 124 + **Setup:** Fresh server DB (no `syncSource`). Simulate desktop push: `POST /items` with `{ type, content, tags, sync_id, metadata }` (no `syncSource`). 125 + 126 + **Expected:** 127 + - `saveItem` succeeds without errors 128 + - Item stored with all fields 129 + - `getItems` returns item correctly 130 + 131 + ### Scenario 5: `_sync` metadata preserved through migration 132 + 133 + **Setup:** Items where `metadata` JSON contains: 134 + ```json 135 + {"_sync": {"createdBy": "abc-uuid", "createdAt": 1000, "modifiedBy": "abc-uuid", "modifiedAt": 2000}, "title": "Example"} 136 + ``` 137 + 138 + **Expected:** 139 + - `metadata` column value preserved as-is through table rebuild 140 + - `JSON.parse(item.metadata)._sync.createdBy === 'abc-uuid'` 141 + 142 + ### Scenario 6: Desktop table rebuild drops `syncSource` 143 + 144 + **Note:** Requires Electron environment. Manual test or future unit test. 145 + 146 + **Setup:** Desktop DB with old CHECK constraint (`type IN ('note','text','tagset')`) and `syncSource` column, with a 'note' type item. 147 + 148 + **Expected:** 149 + - `migrateItemTypes` triggers table rebuild 150 + - New table does NOT include `syncSource` → column dropped 151 + - Data copied correctly 152 + 153 + ### Scenario 7: Fix stale server test assertion 154 + 155 + `backend/server/test.js:620` asserts `syncSource` exists. Update to assert it does NOT exist, or remove the assertion. 156 + 157 + ## Implementation 158 + 159 + ### Primary: `backend/server/test-migration.js` 160 + 161 + Node.js test file using `node:test` (matches existing `backend/server/test.js` pattern): 162 + - Uses `better-sqlite3` directly to create legacy DBs in temp directories 163 + - Calls `db.js` `initializeSchema` to trigger migrations 164 + - Verifies schema (`PRAGMA table_info`) and data post-migration 165 + - Self-cleaning via `fs.rmSync` 166 + 167 + Run: `node --test backend/server/test-migration.js` 168 + 169 + ### Secondary: Fix `backend/server/test.js` 170 + 171 + Update or remove the stale `syncSource` assertion at line 620. 172 + 173 + ### Future: Refactor `sync/sync.js` 174 + 175 + The unified sync module needs the same refactor as desktop/server — replace `syncSource` filtering with timestamp-based filtering. This is a separate task. 176 + 177 + ## Files to Modify 178 + 179 + | File | Change | 180 + |------|--------| 181 + | `backend/server/test-migration.js` | New file — migration simulation tests (Scenarios 1-5) | 182 + | `backend/server/test.js` | Fix stale `syncSource` assertion (Scenario 7) | 183 + | `sync/sync.js` | Future — refactor to remove `syncSource` references | 184 + 185 + ---