diff --git a/src/modules/cards/application/useCases/commands/CreateCollectionUseCase.ts b/src/modules/cards/application/useCases/commands/CreateCollectionUseCase.ts index 41686842..c804f8f5 100644 --- a/src/modules/cards/application/useCases/commands/CreateCollectionUseCase.ts +++ b/src/modules/cards/application/useCases/commands/CreateCollectionUseCase.ts @@ -105,11 +105,10 @@ export class CreateCollectionUseCase collection.markAsPublished(publishResult.value); } - // Save updated collection with published record ID - const saveUpdatedResult = - await this.collectionRepository.save(collection); - if (saveUpdatedResult.isErr()) { - return err(AppError.UnexpectedError.create(saveUpdatedResult.error)); + // Create the new collection + const createResult = await this.collectionRepository.create(collection); + if (createResult.isErr()) { + return err(AppError.UnexpectedError.create(createResult.error)); } return ok({ diff --git a/src/modules/cards/domain/Collection.ts b/src/modules/cards/domain/Collection.ts index 378c0f20..6639f6fb 100644 --- a/src/modules/cards/domain/Collection.ts +++ b/src/modules/cards/domain/Collection.ts @@ -67,7 +67,6 @@ interface CollectionProps { export class Collection extends AggregateRoot { private pendingCommands: CollectionCommand[] = []; - private isFullySynced: boolean = true; // Flag to indicate if we have full data or just metadata get collectionId(): CollectionId { return CollectionId.create(this._id).unwrap(); @@ -502,12 +501,4 @@ export class Collection extends AggregateRoot { public hasPendingCommands(): boolean { return this.pendingCommands.length > 0; } - - public markAsFullySynced(synced: boolean = true): void { - this.isFullySynced = synced; - } - - public getIsFullySynced(): boolean { - return this.isFullySynced; - } } diff --git a/src/modules/cards/domain/ICollectionRepository.ts b/src/modules/cards/domain/ICollectionRepository.ts index c9f511c1..4e057bb3 100644 --- a/src/modules/cards/domain/ICollectionRepository.ts +++ b/src/modules/cards/domain/ICollectionRepository.ts @@ -18,7 +18,13 @@ export interface ICollectionRepository { cardId: CardId, addedBy: CuratorId, ): Promise>; + + // Create a new collection (initial insert only) + create(collection: Collection): Promise>; + + // Update an existing collection using pending commands save(collection: Collection): Promise>; + delete(collectionId: CollectionId): Promise>; // Lightweight update for collection metadata only diff --git a/src/modules/cards/infrastructure/repositories/DrizzleCollectionRepository.ts b/src/modules/cards/infrastructure/repositories/DrizzleCollectionRepository.ts index d52ee48d..6c362f6a 100644 --- a/src/modules/cards/infrastructure/repositories/DrizzleCollectionRepository.ts +++ b/src/modules/cards/infrastructure/repositories/DrizzleCollectionRepository.ts @@ -611,252 +611,47 @@ export class DrizzleCollectionRepository implements ICollectionRepository { const collectionId = collection.collectionId.getStringValue(); const pendingCommands = collection.getPendingCommands(); - // If we have pending commands, use optimized targeted operations - if (pendingCommands.length > 0) { - await this.db.transaction(async (tx) => { - // First, ensure the collection exists (upsert) - const collectionData = - CollectionMapper.toPersistence(collection).collection; - - // Handle collection published record if it exists - let publishedRecordId: string | undefined; - if (collection.publishedRecordId) { - // Check if record already exists - const existing = await tx - .select({ id: publishedRecords.id }) - .from(publishedRecords) - .where( - and( - eq(publishedRecords.uri, collection.publishedRecordId.uri), - eq(publishedRecords.cid, collection.publishedRecordId.cid), - ), - ) - .limit(1); - - if (existing[0]) { - publishedRecordId = existing[0].id; - } else { - const recordId = new UniqueEntityID().toString(); - await tx.insert(publishedRecords).values({ - id: recordId, - uri: collection.publishedRecordId.uri, - cid: collection.publishedRecordId.cid, - recordedAt: new Date(), - }); - publishedRecordId = recordId; - } - } - - // Upsert the collection - await tx - .insert(collections) - .values({ - ...collectionData, - publishedRecordId: publishedRecordId, - }) - .onConflictDoUpdate({ - target: collections.id, - set: { - authorId: collectionData.authorId, - name: collectionData.name, - description: collectionData.description, - accessType: collectionData.accessType, - cardCount: collectionData.cardCount, - updatedAt: collectionData.updatedAt, - publishedRecordId: publishedRecordId, - }, - }); - - // Process each command - for (const command of pendingCommands) { - switch (command.type) { - case CollectionCommandType.ADD_CARD: { - const link = command.payload as CardLink; - const cardLinkId = new UniqueEntityID().toString(); - - // Handle published record if present - let publishedRecordId: string | undefined; - if (link.publishedRecordId) { - // Check if record already exists - const existing = await tx - .select({ id: publishedRecords.id }) - .from(publishedRecords) - .where( - and( - eq(publishedRecords.uri, link.publishedRecordId.uri), - eq(publishedRecords.cid, link.publishedRecordId.cid), - ), - ) - .limit(1); - - if (existing[0]) { - publishedRecordId = existing[0].id; - } else { - const recordId = new UniqueEntityID().toString(); - await tx.insert(publishedRecords).values({ - id: recordId, - uri: link.publishedRecordId.uri, - cid: link.publishedRecordId.cid, - recordedAt: new Date(), - }); - publishedRecordId = recordId; - } - } - - // Insert the new card link - await tx - .insert(collectionCards) - .values({ - id: cardLinkId, - collectionId: collectionId, - cardId: link.cardId.getStringValue(), - addedBy: link.addedBy.value, - addedAt: link.addedAt, - viaCardId: link.viaCardId?.getStringValue(), - publishedRecordId: publishedRecordId, - }) - .onConflictDoNothing(); // Idempotent - ignore if already exists - break; - } - - case CollectionCommandType.UPDATE_CARD_LINK: { - const { cardId, publishedRecordId } = command.payload; - - // Handle published record - let recordId: string | undefined; - if (publishedRecordId) { - // Check if record already exists - const existing = await tx - .select({ id: publishedRecords.id }) - .from(publishedRecords) - .where( - and( - eq(publishedRecords.uri, publishedRecordId.uri), - eq(publishedRecords.cid, publishedRecordId.cid), - ), - ) - .limit(1); - - if (existing[0]) { - recordId = existing[0].id; - } else { - const newRecordId = new UniqueEntityID().toString(); - await tx.insert(publishedRecords).values({ - id: newRecordId, - uri: publishedRecordId.uri, - cid: publishedRecordId.cid, - recordedAt: new Date(), - }); - recordId = newRecordId; - } - } - - // Update the card link - await tx - .update(collectionCards) - .set({ - publishedRecordId: recordId, - }) - .where( - and( - eq(collectionCards.collectionId, collectionId), - eq(collectionCards.cardId, cardId.getStringValue()), - ), - ); - break; - } - - case CollectionCommandType.REMOVE_CARD: { - const { cardId } = command.payload; - - // Delete the card link - await tx - .delete(collectionCards) - .where( - and( - eq(collectionCards.collectionId, collectionId), - eq(collectionCards.cardId, cardId.getStringValue()), - ), - ); - break; - } - - case CollectionCommandType.ADD_COLLABORATOR: - case CollectionCommandType.REMOVE_COLLABORATOR: - // Handle collaborator changes if needed - break; - } - } - - // Update collection metadata only (count and timestamp) - // Other fields were already handled in the upsert above - await tx - .update(collections) - .set({ - cardCount: collectionData.cardCount, - updatedAt: collectionData.updatedAt, - }) - .where(eq(collections.id, collectionId)); - }); - - // Clear commands after successful save - collection.clearPendingCommands(); - return ok(undefined); + // save() is for updates only - use create() for new collections + if (pendingCommands.length === 0) { + return err( + new Error( + 'save() called with no pending commands. Use create() for new collections or updateMetadata() for metadata-only updates.', + ), + ); } - // Fall back to full save for collections without commands (e.g., initial creation) - const { - collection: collectionData, - collaborators, - cardLinks, - publishedRecord, - linkPublishedRecords, - } = CollectionMapper.toPersistence(collection); - + // Process pending commands with optimized targeted operations await this.db.transaction(async (tx) => { - // Handle collection published record if it exists - optimized - let publishedRecordId: string | undefined = undefined; - - if (publishedRecord) { - const recordedAt = publishedRecord.recordedAt || new Date(); - const publishedRecordResult = await tx - .insert(publishedRecords) - .values({ - id: publishedRecord.id, - uri: publishedRecord.uri, - cid: publishedRecord.cid, - recordedAt: recordedAt, - }) - .onConflictDoUpdate({ - target: [publishedRecords.uri, publishedRecords.cid], - set: { recordedAt: recordedAt }, // Update recordedAt to avoid empty set - }) - .returning({ id: publishedRecords.id }); - - publishedRecordId = - publishedRecordResult[0]?.id || publishedRecord.id; - } + // First, ensure the collection exists (upsert) + const collectionData = + CollectionMapper.toPersistence(collection).collection; - // Batch insert published records if needed - const publishedRecordsBatch: any[] = []; - if (linkPublishedRecords && linkPublishedRecords.length > 0) { - for (const record of linkPublishedRecords) { - publishedRecordsBatch.push({ - id: record.id, - uri: record.uri, - cid: record.cid, - recordedAt: record.recordedAt || new Date(), - }); - } + // Handle collection published record if it exists + let publishedRecordId: string | undefined; + if (collection.publishedRecordId) { + // Check if record already exists + const existing = await tx + .select({ id: publishedRecords.id }) + .from(publishedRecords) + .where( + and( + eq(publishedRecords.uri, collection.publishedRecordId.uri), + eq(publishedRecords.cid, collection.publishedRecordId.cid), + ), + ) + .limit(1); - // Batch insert all published records at once - if (publishedRecordsBatch.length > 0) { - await tx - .insert(publishedRecords) - .values(publishedRecordsBatch) - .onConflictDoNothing({ - target: [publishedRecords.uri, publishedRecords.cid], - }); + if (existing[0]) { + publishedRecordId = existing[0].id; + } else { + const recordId = new UniqueEntityID().toString(); + await tx.insert(publishedRecords).values({ + id: recordId, + uri: collection.publishedRecordId.uri, + cid: collection.publishedRecordId.cid, + recordedAt: new Date(), + }); + publishedRecordId = recordId; } } @@ -880,29 +675,140 @@ export class DrizzleCollectionRepository implements ICollectionRepository { }, }); - // Only do full resync if this is an initial save or full update - if (collection.getIsFullySynced()) { - // Delete existing collaborators and card links - await tx - .delete(collectionCollaborators) - .where(eq(collectionCollaborators.collectionId, collectionData.id)); + // Process each command + for (const command of pendingCommands) { + switch (command.type) { + case CollectionCommandType.ADD_CARD: { + const link = command.payload as CardLink; + const cardLinkId = new UniqueEntityID().toString(); + + // Handle published record if present + let publishedRecordId: string | undefined; + if (link.publishedRecordId) { + // Check if record already exists + const existing = await tx + .select({ id: publishedRecords.id }) + .from(publishedRecords) + .where( + and( + eq(publishedRecords.uri, link.publishedRecordId.uri), + eq(publishedRecords.cid, link.publishedRecordId.cid), + ), + ) + .limit(1); + + if (existing[0]) { + publishedRecordId = existing[0].id; + } else { + const recordId = new UniqueEntityID().toString(); + await tx.insert(publishedRecords).values({ + id: recordId, + uri: link.publishedRecordId.uri, + cid: link.publishedRecordId.cid, + recordedAt: new Date(), + }); + publishedRecordId = recordId; + } + } - await tx - .delete(collectionCards) - .where(eq(collectionCards.collectionId, collectionData.id)); + // Insert the new card link + await tx + .insert(collectionCards) + .values({ + id: cardLinkId, + collectionId: collectionId, + cardId: link.cardId.getStringValue(), + addedBy: link.addedBy.value, + addedAt: link.addedAt, + viaCardId: link.viaCardId?.getStringValue(), + publishedRecordId: publishedRecordId, + }) + .onConflictDoNothing(); // Idempotent - ignore if already exists + break; + } - // Insert new collaborators - if (collaborators.length > 0) { - await tx.insert(collectionCollaborators).values(collaborators); - } + case CollectionCommandType.UPDATE_CARD_LINK: { + const { cardId, publishedRecordId } = command.payload; + + // Handle published record + let recordId: string | undefined; + if (publishedRecordId) { + // Check if record already exists + const existing = await tx + .select({ id: publishedRecords.id }) + .from(publishedRecords) + .where( + and( + eq(publishedRecords.uri, publishedRecordId.uri), + eq(publishedRecords.cid, publishedRecordId.cid), + ), + ) + .limit(1); + + if (existing[0]) { + recordId = existing[0].id; + } else { + const newRecordId = new UniqueEntityID().toString(); + await tx.insert(publishedRecords).values({ + id: newRecordId, + uri: publishedRecordId.uri, + cid: publishedRecordId.cid, + recordedAt: new Date(), + }); + recordId = newRecordId; + } + } + + // Update the card link + await tx + .update(collectionCards) + .set({ + publishedRecordId: recordId, + }) + .where( + and( + eq(collectionCards.collectionId, collectionId), + eq(collectionCards.cardId, cardId.getStringValue()), + ), + ); + break; + } - // Insert new card links - if (cardLinks.length > 0) { - await tx.insert(collectionCards).values(cardLinks); + case CollectionCommandType.REMOVE_CARD: { + const { cardId } = command.payload; + + // Delete the card link + await tx + .delete(collectionCards) + .where( + and( + eq(collectionCards.collectionId, collectionId), + eq(collectionCards.cardId, cardId.getStringValue()), + ), + ); + break; + } + + case CollectionCommandType.ADD_COLLABORATOR: + case CollectionCommandType.REMOVE_COLLABORATOR: + // Handle collaborator changes if needed + break; } } + + // Update collection metadata only (count and timestamp) + // Other fields were already handled in the upsert above + await tx + .update(collections) + .set({ + cardCount: collectionData.cardCount, + updatedAt: collectionData.updatedAt, + }) + .where(eq(collections.id, collectionId)); }); + // Clear commands after successful save + collection.clearPendingCommands(); return ok(undefined); } catch (error) { return err(error as Error); @@ -923,6 +829,54 @@ export class DrizzleCollectionRepository implements ICollectionRepository { } } + async create(collection: Collection): Promise> { + try { + return await this.db.transaction(async (tx) => { + const collectionData = + CollectionMapper.toPersistence(collection).collection; + + // Handle published record if it exists + let publishedRecordId: string | undefined; + if (collection.publishedRecordId) { + // Check if record already exists + const existing = await tx + .select({ id: publishedRecords.id }) + .from(publishedRecords) + .where( + and( + eq(publishedRecords.uri, collection.publishedRecordId.uri), + eq(publishedRecords.cid, collection.publishedRecordId.cid), + ), + ) + .limit(1); + + if (existing[0]) { + publishedRecordId = existing[0].id; + } else { + const recordId = new UniqueEntityID().toString(); + await tx.insert(publishedRecords).values({ + id: recordId, + uri: collection.publishedRecordId.uri, + cid: collection.publishedRecordId.cid, + recordedAt: new Date(), + }); + publishedRecordId = recordId; + } + } + + // Insert the new collection + await tx.insert(collections).values({ + ...collectionData, + publishedRecordId: publishedRecordId, + }); + + return ok(undefined); + }); + } catch (error) { + return err(error as Error); + } + } + async updateMetadata( collectionId: CollectionId, updates: { diff --git a/src/modules/cards/tests/application/GetCollectionPageUseCase.test.ts b/src/modules/cards/tests/application/GetCollectionPageUseCase.test.ts index c4716176..3d4e7b7a 100644 --- a/src/modules/cards/tests/application/GetCollectionPageUseCase.test.ts +++ b/src/modules/cards/tests/application/GetCollectionPageUseCase.test.ts @@ -709,6 +709,7 @@ describe('GetCollectionPageUseCase', () => { .fn() .mockRejectedValue(new Error('Database connection failed')), findByIds: jest.fn(), + create: jest.fn(), save: jest.fn(), delete: jest.fn(), updateMetadata: jest.fn(), diff --git a/src/modules/cards/tests/utils/InMemoryCollectionRepository.ts b/src/modules/cards/tests/utils/InMemoryCollectionRepository.ts index 7fd3c559..9334243b 100644 --- a/src/modules/cards/tests/utils/InMemoryCollectionRepository.ts +++ b/src/modules/cards/tests/utils/InMemoryCollectionRepository.ts @@ -135,6 +135,19 @@ export class InMemoryCollectionRepository implements ICollectionRepository { } } + async create(collection: Collection): Promise> { + try { + const collectionId = collection.collectionId.getStringValue(); + if (this.collections.has(collectionId)) { + return err(new Error('Collection already exists')); + } + this.collections.set(collectionId, this.clone(collection)); + return ok(undefined); + } catch (error) { + return err(error as Error); + } + } + async save(collection: Collection): Promise> { try { this.collections.set(