From 546139e82f151b35d6492b7cbf2ac89dbfd5d0b5 Mon Sep 17 00:00:00 2001 From: kamtschatka Date: Sun, 9 Jun 2024 20:11:23 +0200 Subject: [PATCH] fix: Trigger search re-index on bookmark tag manual updates. Fixes #208 (#210) * re-index of database is not scanning all places when bookmark tags are changed. Manual indexing is working as workaround #208 introduced a new function to trigger a reindex to reduce copy/paste added missing reindexes when tags are deleted/bookmarks are updated * give functions a bit more descriptive name --------- Co-authored-by: kamtschatka Co-authored-by: MohamedBassem --- apps/workers/crawlerWorker.ts | 7 ++---- apps/workers/openaiWorker.ts | 7 ++---- packages/shared/queues.ts | 14 ++++++++++++ packages/trpc/routers/admin.ts | 10 ++------- packages/trpc/routers/bookmarks.ts | 24 ++++++--------------- packages/trpc/routers/tags.ts | 34 ++++++++++++------------------ 6 files changed, 41 insertions(+), 55 deletions(-) diff --git a/apps/workers/crawlerWorker.ts b/apps/workers/crawlerWorker.ts index f2e0e4a8..58f1aa85 100644 --- a/apps/workers/crawlerWorker.ts +++ b/apps/workers/crawlerWorker.ts @@ -39,7 +39,7 @@ import { LinkCrawlerQueue, OpenAIQueue, queueConnectionDetails, - SearchIndexingQueue, + triggerSearchReindex, zCrawlLinkRequestSchema, } from "@hoarder/shared/queues"; @@ -490,10 +490,7 @@ async function runCrawler(job: Job) { } // Update the search index - SearchIndexingQueue.add("search_indexing", { - bookmarkId, - type: "index", - }); + triggerSearchReindex(bookmarkId); // Do the archival as a separate last step as it has the potential for failure if (serverConfig.crawler.fullPageArchive) { diff --git a/apps/workers/openaiWorker.ts b/apps/workers/openaiWorker.ts index 7b74e4c3..776d6828 100644 --- a/apps/workers/openaiWorker.ts +++ b/apps/workers/openaiWorker.ts @@ -17,7 +17,7 @@ import logger from "@hoarder/shared/logger"; import { OpenAIQueue, queueConnectionDetails, - SearchIndexingQueue, + triggerSearchReindex, zOpenAIRequestSchema, } from "@hoarder/shared/queues"; @@ -396,8 +396,5 @@ async function runOpenAI(job: Job) { await connectTags(bookmarkId, tags, bookmark.userId); // Update the search index - SearchIndexingQueue.add("search_indexing", { - bookmarkId, - type: "index", - }); + triggerSearchReindex(bookmarkId); } diff --git a/packages/shared/queues.ts b/packages/shared/queues.ts index 86ca32c5..2b890755 100644 --- a/packages/shared/queues.ts +++ b/packages/shared/queues.ts @@ -69,3 +69,17 @@ export const SearchIndexingQueue = new Queue( }, }, ); + +export function triggerSearchReindex(bookmarkId: string) { + SearchIndexingQueue.add("search_indexing", { + bookmarkId, + type: "index", + }); +} + +export function triggerSearchDeletion(bookmarkId: string) { + SearchIndexingQueue.add("search_indexing", { + bookmarkId: bookmarkId, + type: "delete", + }); +} diff --git a/packages/trpc/routers/admin.ts b/packages/trpc/routers/admin.ts index 0a0af173..05831b92 100644 --- a/packages/trpc/routers/admin.ts +++ b/packages/trpc/routers/admin.ts @@ -6,6 +6,7 @@ import { LinkCrawlerQueue, OpenAIQueue, SearchIndexingQueue, + triggerSearchReindex, } from "@hoarder/shared/queues"; import { adminProcedure, router } from "../index"; @@ -129,13 +130,6 @@ export const adminAppRouter = router({ }, }); - await Promise.all( - bookmarkIds.map((b) => - SearchIndexingQueue.add("search_indexing", { - bookmarkId: b.id, - type: "index", - }), - ), - ); + await Promise.all(bookmarkIds.map((b) => triggerSearchReindex(b.id))); }), }); diff --git a/packages/trpc/routers/bookmarks.ts b/packages/trpc/routers/bookmarks.ts index 5f53dd16..15a8c7c0 100644 --- a/packages/trpc/routers/bookmarks.ts +++ b/packages/trpc/routers/bookmarks.ts @@ -22,7 +22,8 @@ import { deleteAsset } from "@hoarder/shared/assetdb"; import { LinkCrawlerQueue, OpenAIQueue, - SearchIndexingQueue, + triggerSearchDeletion, + triggerSearchReindex, } from "@hoarder/shared/queues"; import { getSearchIdxClient } from "@hoarder/shared/search"; import { @@ -295,10 +296,7 @@ export const bookmarksAppRouter = router({ break; } } - SearchIndexingQueue.add("search_indexing", { - bookmarkId: bookmark.id, - type: "index", - }); + triggerSearchReindex(bookmark.id); return bookmark; }), @@ -328,10 +326,7 @@ export const bookmarksAppRouter = router({ message: "Bookmark not found", }); } - SearchIndexingQueue.add("search_indexing", { - bookmarkId: input.bookmarkId, - type: "index", - }); + triggerSearchReindex(input.bookmarkId); return res[0]; }), @@ -357,10 +352,7 @@ export const bookmarksAppRouter = router({ message: "Bookmark not found", }); } - SearchIndexingQueue.add("search_indexing", { - bookmarkId: input.bookmarkId, - type: "index", - }); + triggerSearchReindex(input.bookmarkId); }), deleteBookmark: authedProcedure @@ -385,10 +377,7 @@ export const bookmarksAppRouter = router({ eq(bookmarks.id, input.bookmarkId), ), ); - SearchIndexingQueue.add("search_indexing", { - bookmarkId: input.bookmarkId, - type: "delete", - }); + triggerSearchDeletion(input.bookmarkId); if (deleted.changes > 0 && bookmark) { await cleanupAssetForBookmark({ asset: bookmark.asset, @@ -708,6 +697,7 @@ export const bookmarksAppRouter = router({ })), ) .onConflictDoNothing(); + triggerSearchReindex(input.bookmarkId); return { bookmarkId: input.bookmarkId, attached: allIds, diff --git a/packages/trpc/routers/tags.ts b/packages/trpc/routers/tags.ts index dc70b068..7cb2c971 100644 --- a/packages/trpc/routers/tags.ts +++ b/packages/trpc/routers/tags.ts @@ -5,7 +5,7 @@ import { z } from "zod"; import type { ZAttachedByEnum } from "@hoarder/shared/types/tags"; import { SqliteError } from "@hoarder/db"; import { bookmarkTags, tagsOnBookmarks } from "@hoarder/db/schema"; -import { SearchIndexingQueue } from "@hoarder/shared/queues"; +import { triggerSearchReindex } from "@hoarder/shared/queues"; import { zGetTagResponseSchema } from "@hoarder/shared/types/tags"; import type { Context } from "../index"; @@ -102,18 +102,22 @@ export const tagsAppRouter = router({ ) .use(ensureTagOwnership) .mutation(async ({ input, ctx }) => { + const affectedBookmarks = await ctx.db + .select({ + bookmarkId: tagsOnBookmarks.bookmarkId, + }) + .from(tagsOnBookmarks) + .where(conditionFromInput(input, ctx.user.id)); + const res = await ctx.db .delete(bookmarkTags) - .where( - and( - conditionFromInput(input, ctx.user.id), - eq(bookmarkTags.userId, ctx.user.id), - ), - ); + .where(conditionFromInput(input, ctx.user.id)); if (res.changes == 0) { throw new TRPCError({ code: "NOT_FOUND" }); } - // TODO: Update affected bookmarks in search index + affectedBookmarks.forEach(({ bookmarkId }) => + triggerSearchReindex(bookmarkId), + ); }), deleteUnused: authedProcedure .output( @@ -184,12 +188,7 @@ export const tagsAppRouter = router({ await Promise.all([ affectedBookmarks .map((b) => b.bookmarkId) - .map((id) => - SearchIndexingQueue.add("search_indexing", { - bookmarkId: id, - type: "index", - }), - ), + .map((id) => triggerSearchReindex(id)), ]); } catch (e) { // Best Effort attempt to reindex affected bookmarks @@ -306,12 +305,7 @@ export const tagsAppRouter = router({ try { await Promise.all([ - affectedBookmarks.map((id) => - SearchIndexingQueue.add("search_indexing", { - bookmarkId: id, - type: "index", - }), - ), + affectedBookmarks.map((id) => triggerSearchReindex(id)), ]); } catch (e) { // Best Effort attempt to reindex affected bookmarks