diff --git a/apps/api/src/knowledge/knowledge.service.sync-tags.spec.ts b/apps/api/src/knowledge/knowledge.service.sync-tags.spec.ts new file mode 100644 index 0000000..494942b --- /dev/null +++ b/apps/api/src/knowledge/knowledge.service.sync-tags.spec.ts @@ -0,0 +1,353 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { Test, TestingModule } from "@nestjs/testing"; +import { KnowledgeService } from "./knowledge.service"; +import { PrismaService } from "../prisma/prisma.service"; +import { LinkSyncService } from "./services/link-sync.service"; +import { KnowledgeCacheService } from "./services/cache.service"; +import { EmbeddingService } from "./services/embedding.service"; +import { OllamaEmbeddingService } from "./services/ollama-embedding.service"; +import { EmbeddingQueueService } from "./queues/embedding-queue.service"; + +/** + * Tests for syncTags N+1 query fix (CQ-API-7). + * + * syncTags is a private method invoked via create(). These tests verify + * that the batch findMany pattern is used instead of individual findUnique + * queries per tag, and that missing tags are created correctly. + */ +describe("KnowledgeService - syncTags (N+1 fix)", () => { + let service: KnowledgeService; + + const workspaceId = "workspace-123"; + const userId = "user-456"; + const entryId = "entry-789"; + + // Transaction mock objects - these simulate the Prisma transaction client + const mockTx = { + knowledgeEntry: { + create: vi.fn(), + findUnique: vi.fn(), + }, + knowledgeEntryVersion: { + create: vi.fn(), + }, + knowledgeTag: { + findMany: vi.fn(), + create: vi.fn(), + }, + knowledgeEntryTag: { + deleteMany: vi.fn(), + createMany: vi.fn(), + }, + }; + + const mockPrismaService = { + knowledgeEntry: { + findUnique: vi.fn(), + }, + $transaction: vi.fn(), + }; + + const mockLinkSyncService = { + syncLinks: vi.fn().mockResolvedValue(undefined), + }; + + const mockCacheService = { + getEntry: vi.fn().mockResolvedValue(null), + setEntry: vi.fn().mockResolvedValue(undefined), + invalidateEntry: vi.fn().mockResolvedValue(undefined), + getSearch: vi.fn().mockResolvedValue(null), + setSearch: vi.fn().mockResolvedValue(undefined), + invalidateSearches: vi.fn().mockResolvedValue(undefined), + getGraph: vi.fn().mockResolvedValue(null), + setGraph: vi.fn().mockResolvedValue(undefined), + invalidateGraphs: vi.fn().mockResolvedValue(undefined), + invalidateGraphsForEntry: vi.fn().mockResolvedValue(undefined), + clearWorkspaceCache: vi.fn().mockResolvedValue(undefined), + getStats: vi.fn().mockReturnValue({ hits: 0, misses: 0, sets: 0, deletes: 0, hitRate: 0 }), + resetStats: vi.fn(), + isEnabled: vi.fn().mockReturnValue(false), + }; + + const mockEmbeddingService = { + isConfigured: vi.fn().mockReturnValue(false), + generateEmbedding: vi.fn().mockResolvedValue(null), + batchGenerateEmbeddings: vi.fn().mockResolvedValue([]), + }; + + const mockOllamaEmbeddingService = { + isConfigured: vi.fn().mockResolvedValue(false), + generateEmbedding: vi.fn().mockResolvedValue([]), + generateAndStoreEmbedding: vi.fn().mockResolvedValue(undefined), + batchGenerateEmbeddings: vi.fn().mockResolvedValue(0), + prepareContentForEmbedding: vi.fn().mockReturnValue("combined content"), + }; + + const mockEmbeddingQueueService = { + queueEmbeddingJob: vi.fn().mockResolvedValue("job-123"), + }; + + /** + * Helper to set up the $transaction mock so it executes the callback + * with our mockTx and returns a properly shaped entry result. + */ + function setupTransactionForCreate( + tags: Array<{ id: string; name: string; slug: string; color: string | null }> + ): void { + const createdEntry = { + id: entryId, + workspaceId, + slug: "test-entry", + title: "Test Entry", + content: "# Test", + contentHtml: "

Test

", + summary: null, + status: "DRAFT", + visibility: "PRIVATE", + createdBy: userId, + updatedBy: userId, + createdAt: new Date("2026-01-01"), + updatedAt: new Date("2026-01-01"), + tags: tags.map((t) => ({ + entryId, + tagId: t.id, + tag: t, + })), + }; + + mockTx.knowledgeEntry.create.mockResolvedValue(createdEntry); + mockTx.knowledgeEntryVersion.create.mockResolvedValue({}); + mockTx.knowledgeEntryTag.deleteMany.mockResolvedValue({ count: 0 }); + mockTx.knowledgeEntryTag.createMany.mockResolvedValue({ count: tags.length }); + mockTx.knowledgeEntry.findUnique.mockResolvedValue(createdEntry); + + // ensureUniqueSlug uses prisma (not tx), so mock the outer prisma + mockPrismaService.knowledgeEntry.findUnique.mockResolvedValue(null); + + mockPrismaService.$transaction.mockImplementation( + async (callback: (tx: typeof mockTx) => Promise) => { + return callback(mockTx); + } + ); + } + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + KnowledgeService, + { provide: PrismaService, useValue: mockPrismaService }, + { provide: LinkSyncService, useValue: mockLinkSyncService }, + { provide: KnowledgeCacheService, useValue: mockCacheService }, + { provide: EmbeddingService, useValue: mockEmbeddingService }, + { provide: OllamaEmbeddingService, useValue: mockOllamaEmbeddingService }, + { provide: EmbeddingQueueService, useValue: mockEmbeddingQueueService }, + ], + }).compile(); + + service = module.get(KnowledgeService); + vi.clearAllMocks(); + }); + + it("should use findMany to batch-fetch existing tags instead of individual queries", async () => { + const existingTag = { + id: "tag-1", + workspaceId, + name: "JavaScript", + slug: "javascript", + color: null, + }; + mockTx.knowledgeTag.findMany.mockResolvedValue([existingTag]); + + setupTransactionForCreate([existingTag]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["JavaScript"], + }); + + // Verify findMany was called with slug IN array (batch query) + expect(mockTx.knowledgeTag.findMany).toHaveBeenCalledWith({ + where: { + workspaceId, + slug: { in: ["javascript"] }, + }, + }); + }); + + it("should create only missing tags when some already exist", async () => { + const existingTag = { + id: "tag-1", + workspaceId, + name: "JavaScript", + slug: "javascript", + color: null, + }; + const newTag = { + id: "tag-2", + workspaceId, + name: "TypeScript", + slug: "typescript", + color: null, + }; + + // findMany returns only the existing tag + mockTx.knowledgeTag.findMany.mockResolvedValue([existingTag]); + // create is called only for the missing tag + mockTx.knowledgeTag.create.mockResolvedValue(newTag); + + setupTransactionForCreate([existingTag, newTag]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["JavaScript", "TypeScript"], + }); + + // findMany should be called once with both slugs + expect(mockTx.knowledgeTag.findMany).toHaveBeenCalledTimes(1); + expect(mockTx.knowledgeTag.findMany).toHaveBeenCalledWith({ + where: { + workspaceId, + slug: { in: ["javascript", "typescript"] }, + }, + }); + + // Only the missing tag should be created + expect(mockTx.knowledgeTag.create).toHaveBeenCalledTimes(1); + expect(mockTx.knowledgeTag.create).toHaveBeenCalledWith({ + data: { + workspaceId, + name: "TypeScript", + slug: "typescript", + }, + }); + }); + + it("should create all tags when none exist", async () => { + const tag1 = { id: "tag-1", workspaceId, name: "React", slug: "react", color: null }; + const tag2 = { id: "tag-2", workspaceId, name: "Vue", slug: "vue", color: null }; + + // No existing tags found + mockTx.knowledgeTag.findMany.mockResolvedValue([]); + mockTx.knowledgeTag.create.mockResolvedValueOnce(tag1).mockResolvedValueOnce(tag2); + + setupTransactionForCreate([tag1, tag2]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["React", "Vue"], + }); + + expect(mockTx.knowledgeTag.findMany).toHaveBeenCalledTimes(1); + expect(mockTx.knowledgeTag.create).toHaveBeenCalledTimes(2); + }); + + it("should not create any tags when all already exist", async () => { + const tag1 = { id: "tag-1", workspaceId, name: "Python", slug: "python", color: null }; + const tag2 = { id: "tag-2", workspaceId, name: "Go", slug: "go", color: null }; + + mockTx.knowledgeTag.findMany.mockResolvedValue([tag1, tag2]); + + setupTransactionForCreate([tag1, tag2]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["Python", "Go"], + }); + + expect(mockTx.knowledgeTag.findMany).toHaveBeenCalledTimes(1); + expect(mockTx.knowledgeTag.create).not.toHaveBeenCalled(); + }); + + it("should use createMany for tag associations instead of individual creates", async () => { + const tag1 = { id: "tag-1", workspaceId, name: "Rust", slug: "rust", color: null }; + const tag2 = { id: "tag-2", workspaceId, name: "Zig", slug: "zig", color: null }; + + mockTx.knowledgeTag.findMany.mockResolvedValue([tag1, tag2]); + + setupTransactionForCreate([tag1, tag2]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["Rust", "Zig"], + }); + + // createMany should be called once with all associations + expect(mockTx.knowledgeEntryTag.createMany).toHaveBeenCalledTimes(1); + expect(mockTx.knowledgeEntryTag.createMany).toHaveBeenCalledWith({ + data: [ + { entryId, tagId: "tag-1" }, + { entryId, tagId: "tag-2" }, + ], + }); + }); + + it("should skip tag sync when no tags are provided", async () => { + setupTransactionForCreate([]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: [], + }); + + // No tag queries should be made when tags array is empty + expect(mockTx.knowledgeTag.findMany).not.toHaveBeenCalled(); + expect(mockTx.knowledgeTag.create).not.toHaveBeenCalled(); + }); + + it("should deduplicate tags with the same slug", async () => { + // "JavaScript" and "javascript" produce the same slug + const existingTag = { + id: "tag-1", + workspaceId, + name: "JavaScript", + slug: "javascript", + color: null, + }; + + mockTx.knowledgeTag.findMany.mockResolvedValue([existingTag]); + + setupTransactionForCreate([existingTag]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["JavaScript", "javascript"], + }); + + // findMany should be called with deduplicated slugs + expect(mockTx.knowledgeTag.findMany).toHaveBeenCalledWith({ + where: { + workspaceId, + slug: { in: ["javascript"] }, + }, + }); + + // Only one association created (deduped by slug) + expect(mockTx.knowledgeEntryTag.createMany).toHaveBeenCalledWith({ + data: [{ entryId, tagId: "tag-1" }], + }); + }); + + it("should delete existing tag associations before syncing", async () => { + const tag1 = { id: "tag-1", workspaceId, name: "Node", slug: "node", color: null }; + mockTx.knowledgeTag.findMany.mockResolvedValue([tag1]); + + setupTransactionForCreate([tag1]); + + await service.create(workspaceId, userId, { + title: "Test Entry", + content: "# Test", + tags: ["Node"], + }); + + expect(mockTx.knowledgeEntryTag.deleteMany).toHaveBeenCalledWith({ + where: { entryId }, + }); + }); +}); diff --git a/apps/api/src/knowledge/knowledge.service.ts b/apps/api/src/knowledge/knowledge.service.ts index 0625e34..f004d91 100644 --- a/apps/api/src/knowledge/knowledge.service.ts +++ b/apps/api/src/knowledge/knowledge.service.ts @@ -821,45 +821,48 @@ export class KnowledgeService { return; } - // Get or create tags - const tags = await Promise.all( - tagNames.map(async (name) => { - const tagSlug = this.generateSlug(name); + // Build slug map: slug -> original tag name + const slugToName = new Map(); + for (const name of tagNames) { + slugToName.set(this.generateSlug(name), name); + } + const tagSlugs = [...slugToName.keys()]; - // Try to find existing tag - let tag = await tx.knowledgeTag.findUnique({ - where: { - workspaceId_slug: { - workspaceId, - slug: tagSlug, - }, - }, - }); + // Batch fetch all existing tags in a single query (fixes N+1) + const existingTags = await tx.knowledgeTag.findMany({ + where: { + workspaceId, + slug: { in: tagSlugs }, + }, + }); - // Create if doesn't exist - tag ??= await tx.knowledgeTag.create({ + // Determine which tags need to be created + const existingSlugs = new Set(existingTags.map((t) => t.slug)); + const missingSlugs = tagSlugs.filter((s) => !existingSlugs.has(s)); + + // Create missing tags + const newTags = await Promise.all( + missingSlugs.map((slug) => { + const name = slugToName.get(slug) ?? slug; + return tx.knowledgeTag.create({ data: { workspaceId, name, - slug: tagSlug, + slug, }, }); - - return tag; }) ); - // Create tag associations - await Promise.all( - tags.map((tag) => - tx.knowledgeEntryTag.create({ - data: { - entryId, - tagId: tag.id, - }, - }) - ) - ); + const allTags = [...existingTags, ...newTags]; + + // Create tag associations in a single batch + await tx.knowledgeEntryTag.createMany({ + data: allTags.map((tag) => ({ + entryId, + tagId: tag.id, + })), + }); } /**