From a7785040efd83ba8e6206aca594c032dbde0d5a1 Mon Sep 17 00:00:00 2001 From: Gerald Leban Date: Sun, 11 Oct 2026 04:52:35 +0200 Subject: [PATCH] refactor(sync): standardize syncAccount ubiquitous language and decompose RepoSyncService --- app/auth/controllers/callback_controller.ts | 4 +- app/kits/services/repo_sync_service.ts | 112 +++++++++++-------- commands/kits/sync.ts | 2 +- database/seeders/sync_user_records_seeder.ts | 2 +- tests/helpers/fake_atproto_user.ts | 2 +- tests/helpers/fake_repo_sync_service.ts | 8 +- tests/helpers/testable_repo_sync_service.ts | 2 +- tests/unit/repo_sync_service.spec.ts | 18 +-- 8 files changed, 84 insertions(+), 66 deletions(-) diff --git a/app/auth/controllers/callback_controller.ts b/app/auth/controllers/callback_controller.ts index e2c2805..3d55708 100644 --- a/app/auth/controllers/callback_controller.ts +++ b/app/auth/controllers/callback_controller.ts @@ -25,12 +25,12 @@ export default class CallbackController { const isNewUser = !(await Kit.hasKitsFor(did)) if (isNewUser) { - await this.repoSyncService.syncUser(did).catch((err) => { + await this.repoSyncService.syncAccount(did).catch((err) => { logger.warn({ err, did }, 'PDS sync failed on first login') }) } else { this.repoSyncService - .syncUser(did) + .syncAccount(did) .then(({ handle, kitsSynced }) => { logger.info({ did, handle, kitsSynced }, `PDS sync completed on login for ${handle}`) }) diff --git a/app/kits/services/repo_sync_service.ts b/app/kits/services/repo_sync_service.ts index 25b90c9..faa46e2 100644 --- a/app/kits/services/repo_sync_service.ts +++ b/app/kits/services/repo_sync_service.ts @@ -10,11 +10,12 @@ import { } from '@atproto/oauth-client-node' import { Client, isBlobRef, getBlobCidString } from '@atproto/lex' import gunplaKit from '#lexicons/build/gunpla/kit' +import type { Main as KitRecordValue } from '#lexicons/build/gunpla/kit.defs' import Kit from '#kits/models/kit' import KitLike from '#kits/models/kit_like' import { DateTime } from 'luxon' -export interface SyncResult { +export type SyncResult = { did: string handle: string pdsEndpoint: string @@ -22,14 +23,14 @@ export interface SyncResult { kitsPruned: number } -export interface SyncAccountResult { +export type SyncAccountResult = { identifier: string success: boolean data?: SyncResult error?: string } -export interface BatchSyncResult { +export type BatchSyncResult = { results: SyncAccountResult[] totalKits: number totalPruned: number @@ -67,53 +68,16 @@ export class RepoSyncService { if (!record.valid) continue remoteUris.add(record.uri) - const kit = record.value - const photoCids: string[] = [] - if (Array.isArray(kit.photos)) { - for (const p of kit.photos) { - if (isBlobRef(p)) { - const cidStr = getBlobCidString(p) - if (cidStr) photoCids.push(cidStr) - } - } - } - const photoCid = photoCids[0] ?? null - - await Kit.updateOrCreate( - { uri: record.uri }, - { - cid: record.cid, - did: repoDid, - name: kit.name, - grade: kit.grade ?? null, - scale: kit.scale ?? null, - manufacturer: kit.manufacturer ?? null, - status: kit.status ?? 'backlog', - acquiredAt: kit.acquiredAt ? DateTime.fromISO(kit.acquiredAt) : null, - photoCid, - photos: photoCids.length > 0 ? photoCids : null, - recordCreatedAt: kit.createdAt ? DateTime.fromISO(kit.createdAt) : null, - indexedAt: DateTime.now(), - } - ) + const photoCids = this.extractPhotoCids(record.value) + await this.upsertKit(record, repoDid, photoCids) synced++ } - // Reconcile and prune local kits missing from remote repository - const localKits = await Kit.query().where('did', repoDid) - const toPrune = localKits.filter((k) => !remoteUris.has(k.uri)) - - if (toPrune.length > 0) { - const pruneIds = toPrune.map((k) => k.id) - await KitLike.query().whereIn('kit_id', pruneIds).delete() - await Kit.query().whereIn('id', pruneIds).delete() - logger.info({ repoDid, count: toPrune.length }, 'Pruned kits removed from remote PDS') - } - - return { synced, pruned: toPrune.length } + const pruned = await this.pruneMissingKits(repoDid, remoteUris) + return { synced, pruned } } - async syncUser(identifier: string): Promise { + async syncAccount(identifier: string): Promise { const identity = await this.identityService.resolve(identifier) if (!identity) { throw new Error(`Could not resolve ATProto identity for "${identifier}"`) @@ -141,7 +105,7 @@ export class RepoSyncService { } } - async syncUsers(identifiers: string[]): Promise { + async syncAccounts(identifiers: string[]): Promise { const results: SyncAccountResult[] = [] let totalKits = 0 let totalPruned = 0 @@ -149,7 +113,7 @@ export class RepoSyncService { for (const identifier of identifiers) { try { - const syncData = await this.syncUser(identifier) + const syncData = await this.syncAccount(identifier) results.push({ identifier, success: true, @@ -174,4 +138,58 @@ export class RepoSyncService { hasFailures, } } + + private extractPhotoCids(kit: KitRecordValue): string[] { + const photoCids: string[] = [] + if (Array.isArray(kit.photos)) { + for (const p of kit.photos) { + if (isBlobRef(p)) { + const cidStr = getBlobCidString(p) + if (cidStr) photoCids.push(cidStr) + } + } + } + return photoCids + } + + private async upsertKit( + record: { uri: string; cid?: string; value: KitRecordValue }, + repoDid: AtprotoDid, + photoCids: string[] + ) { + const kit = record.value + const photoCid = photoCids[0] ?? null + + await Kit.updateOrCreate( + { uri: record.uri }, + { + cid: record.cid, + did: repoDid, + name: kit.name, + grade: kit.grade ?? null, + scale: kit.scale ?? null, + manufacturer: kit.manufacturer ?? null, + status: kit.status ?? 'backlog', + acquiredAt: kit.acquiredAt ? DateTime.fromISO(kit.acquiredAt) : null, + photoCid, + photos: photoCids.length > 0 ? photoCids : null, + recordCreatedAt: kit.createdAt ? DateTime.fromISO(kit.createdAt) : null, + indexedAt: DateTime.now(), + } + ) + } + + private async pruneMissingKits(repoDid: AtprotoDid, remoteUris: Set): Promise { + const localKits = await Kit.query().where('did', repoDid) + const toPrune = localKits.filter((k) => !remoteUris.has(k.uri)) + + if (toPrune.length > 0) { + const pruneIds = toPrune.map((k) => k.id) + await KitLike.query().whereIn('kit_id', pruneIds).delete() + await Kit.query().whereIn('id', pruneIds).delete() + logger.info({ repoDid, count: toPrune.length }, 'Pruned kits removed from remote PDS') + } + + return toPrune.length + } } diff --git a/commands/kits/sync.ts b/commands/kits/sync.ts index 172819e..51530cc 100644 --- a/commands/kits/sync.ts +++ b/commands/kits/sync.ts @@ -38,7 +38,7 @@ export default class KitsSync extends BaseCommand { } } - const batchResult = await syncService.syncUsers(validIdentifiers) + const batchResult = await syncService.syncAccounts(validIdentifiers) for (const res of batchResult.results) { if (res.success && res.data) { diff --git a/database/seeders/sync_user_records_seeder.ts b/database/seeders/sync_user_records_seeder.ts index 58c221c..3a05f66 100644 --- a/database/seeders/sync_user_records_seeder.ts +++ b/database/seeders/sync_user_records_seeder.ts @@ -15,7 +15,7 @@ export default class extends BaseSeeder { } const syncService = await app.container.make(RepoSyncService) - const result = await syncService.syncUser(syncUser) + const result = await syncService.syncAccount(syncUser) logger.info(`Synced @${result.handle} (${result.kitsSynced} kits) from ${result.pdsEndpoint}`) } } diff --git a/tests/helpers/fake_atproto_user.ts b/tests/helpers/fake_atproto_user.ts index 62d5cba..15f4281 100644 --- a/tests/helpers/fake_atproto_user.ts +++ b/tests/helpers/fake_atproto_user.ts @@ -10,7 +10,7 @@ import { asAtUriString, } from '@atproto/lex' -export interface FakeAtprotoUserOptions { +export type FakeAtprotoUserOptions = { did?: `did:plc:${string}` createHandler?: () => Promise uploadBlobHandler?: () => Promise diff --git a/tests/helpers/fake_repo_sync_service.ts b/tests/helpers/fake_repo_sync_service.ts index 770d754..195b288 100644 --- a/tests/helpers/fake_repo_sync_service.ts +++ b/tests/helpers/fake_repo_sync_service.ts @@ -4,7 +4,7 @@ import { type BatchSyncResult, } from '#kits/services/repo_sync_service' -export interface FakeRepoSyncOptions { +export type FakeRepoSyncOptions = { shouldThrow?: boolean handler?: (identifier: string) => Promise } @@ -14,7 +14,7 @@ export class FakeRepoSyncService extends RepoSyncService { super(null as any, null as any) } - async syncUser(identifier: string): Promise { + async syncAccount(identifier: string): Promise { if (this.options.handler) { return this.options.handler(identifier) } @@ -30,7 +30,7 @@ export class FakeRepoSyncService extends RepoSyncService { } } - async syncUsers(identifiers: string[]): Promise { - return super.syncUsers(identifiers) + async syncAccounts(identifiers: string[]): Promise { + return super.syncAccounts(identifiers) } } diff --git a/tests/helpers/testable_repo_sync_service.ts b/tests/helpers/testable_repo_sync_service.ts index f9d4d83..7352470 100644 --- a/tests/helpers/testable_repo_sync_service.ts +++ b/tests/helpers/testable_repo_sync_service.ts @@ -4,7 +4,7 @@ import type { ProfileService } from '#profile/services/profile_service' import type { Client } from '@atproto/lex' import type { AtprotoDid } from '@atproto/oauth-client-node' -export interface TestableRepoSyncOptions { +export type TestableRepoSyncOptions = { pdsEndpoint?: string | null kitsSynced?: number kitsPruned?: number diff --git a/tests/unit/repo_sync_service.spec.ts b/tests/unit/repo_sync_service.spec.ts index 4d9f3e6..becb101 100644 --- a/tests/unit/repo_sync_service.spec.ts +++ b/tests/unit/repo_sync_service.spec.ts @@ -27,14 +27,14 @@ test.group('RepoSyncService', () => { test('fails gracefully when syncing an invalid handle', async ({ assert }) => { await assert.rejects( - () => syncService.syncUser('invalid handle with spaces'), + () => syncService.syncAccount('invalid handle with spaces'), /Could not resolve ATProto identity/ ) }) - test('syncUsers aggregates results for multiple identifiers', async ({ assert }) => { + test('syncAccounts aggregates results for multiple identifiers', async ({ assert }) => { const customService = new RepoSyncService(identityService, fakeProfileService) - customService.syncUser = async (id: string): Promise => { + customService.syncAccount = async (id: string): Promise => { if (id === 'user1.bsky.social') { return { did: 'did:plc:user1', @@ -53,7 +53,7 @@ test.group('RepoSyncService', () => { } } - const result = await customService.syncUsers(['user1.bsky.social', 'user2.bsky.social']) + const result = await customService.syncAccounts(['user1.bsky.social', 'user2.bsky.social']) assert.equal(result.totalKits, 5) assert.isFalse(result.hasFailures) assert.lengthOf(result.results, 2) @@ -61,9 +61,9 @@ test.group('RepoSyncService', () => { assert.isTrue(result.results[1].success) }) - test('syncUsers isolates individual errors and marks hasFailures true', async ({ assert }) => { + test('syncAccounts isolates individual errors and marks hasFailures true', async ({ assert }) => { const customService = new RepoSyncService(identityService, fakeProfileService) - customService.syncUser = async (id: string): Promise => { + customService.syncAccount = async (id: string): Promise => { if (id === 'invalid.bsky.social') { throw new Error('Connection refused to PDS') } @@ -76,7 +76,7 @@ test.group('RepoSyncService', () => { } } - const result = await customService.syncUsers(['invalid.bsky.social', 'valid.bsky.social']) + const result = await customService.syncAccounts(['invalid.bsky.social', 'valid.bsky.social']) assert.isTrue(result.hasFailures) assert.lengthOf(result.results, 2) assert.isFalse(result.results[0].success) @@ -85,7 +85,7 @@ test.group('RepoSyncService', () => { assert.equal(result.totalKits, 4) }) - test('syncUser invokes ProfileService.syncProfile when profileService is injected', async ({ + test('syncAccount invokes ProfileService.syncProfile when profileService is injected', async ({ assert, }) => { const fakeIdentity = new FakeIdentityService({ @@ -96,7 +96,7 @@ test.group('RepoSyncService', () => { kitsSynced: 2, }) - const result = await service.syncUser('mockuser.bsky.social') + const result = await service.syncAccount('mockuser.bsky.social') assert.equal(result.did, 'did:plc:abiaoe3hpp2cqiuptrss75fp') assert.equal(result.kitsSynced, 2) assert.lengthOf(fakeProfile.syncedProfiles, 1) -- 2.54.0