From 4eba7df84c86bf3e4f3da7f44a9e2151f9ec4f75 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 13:02:12 +0800
Subject: [PATCH 1/7] always upsert library items

---
 packages/api/src/repository/index.ts          |  4 +
 packages/api/src/repository/library_item.ts   | 38 +++++++
 packages/api/src/resolvers/article/index.ts   |  4 +-
 packages/api/src/routers/page_router.ts       |  4 +-
 .../api/src/routers/svc/email_attachment.ts   |  4 +-
 packages/api/src/routers/svc/following.ts     |  4 +-
 .../src/services/create_page_save_request.ts  |  4 +-
 packages/api/src/services/library_item.ts     | 98 ++++++-------------
 packages/api/src/services/recommendation.ts   |  4 +-
 packages/api/src/services/save_email.ts       |  4 +-
 packages/api/src/services/save_page.ts        | 50 ++--------
 packages/api/src/services/upload_file.ts      |  4 +-
 packages/api/test/db.ts                       |  4 +-
 13 files changed, 100 insertions(+), 126 deletions(-)

diff --git a/packages/api/src/repository/index.ts b/packages/api/src/repository/index.ts
index 943dc0dd8..61a204cec 100644
--- a/packages/api/src/repository/index.ts
+++ b/packages/api/src/repository/index.ts
@@ -17,6 +17,10 @@ export const getColumns = <T>(repository: Repository<T>): (keyof T)[] => {
   ) as (keyof T)[]
 }
 
+export const getColumnsDbName = <T>(repository: Repository<T>): string[] => {
+  return repository.metadata.columns.map((col) => col.databaseName)
+}
+
 export const setClaims = async (
   manager: EntityManager,
   uid = '00000000-0000-0000-0000-000000000000',
diff --git a/packages/api/src/repository/library_item.ts b/packages/api/src/repository/library_item.ts
index ebddb4b10..22e1e64f8 100644
--- a/packages/api/src/repository/library_item.ts
+++ b/packages/api/src/repository/library_item.ts
@@ -1,5 +1,16 @@
+import { DeepPartial } from 'typeorm'
+import { getColumnsDbName } from '.'
 import { appDataSource } from '../data_source'
 import { LibraryItem } from '../entity/library_item'
+import { keysToCamelCase, wordsCount } from '../utils/helpers'
+import { logger } from '../utils/logger'
+
+const convertToLibraryItem = (item: DeepPartial<LibraryItem>) => {
+  return {
+    ...item,
+    wordCount: item.wordCount ?? wordsCount(item.readableContent || ''),
+  }
+}
 
 export const libraryItemRepository = appDataSource
   .getRepository(LibraryItem)
@@ -20,6 +31,33 @@ export const libraryItemRepository = appDataSource
       return this.countBy({ createdAt })
     },
 
+    async upsertLibraryItem(item: DeepPartial<LibraryItem>) {
+      // overwrites columns except id and slug
+      const overwrites = getColumnsDbName(this).filter(
+        (column) => !['id', 'slug'].includes(column)
+      )
+      const hashedUrl = 'md5(original_url)'
+
+      const [query, params] = this.createQueryBuilder()
+        .insert()
+        .into(LibraryItem)
+        .values(convertToLibraryItem(item))
+        .orUpdate(overwrites, ['user_id', hashedUrl], {
+          skipUpdateIfNoValuesChanged: true,
+        })
+        .returning('*')
+        .getQueryAndParameters()
+
+      // this is a workaround for the typeorm bug which quotes the md5 function
+      const newQuery = query.replace(`"${hashedUrl}"`, hashedUrl)
+      const results = (await this.query(newQuery, params)) as never[]
+
+      // convert to camel case
+      const newItem = keysToCamelCase(results[0]) as LibraryItem
+
+      return newItem
+    },
+
     createByPopularRead(name: string, userId: string) {
       return this.query(
         `
diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts
index 1d0e95a64..b158c3a9f 100644
--- a/packages/api/src/resolvers/article/index.ts
+++ b/packages/api/src/resolvers/article/index.ts
@@ -74,7 +74,7 @@ import {
 import {
   batchDelete,
   batchUpdateLibraryItems,
-  createLibraryItem,
+  createOrUpdateLibraryItem,
   findLibraryItemById,
   findLibraryItemByUrl,
   findLibraryItemsByPrefix,
@@ -357,7 +357,7 @@ export const createArticleResolver = authorized<
         )
       } else {
         // create new item in database
-        libraryItemToReturn = await createLibraryItem(
+        libraryItemToReturn = await createOrUpdateLibraryItem(
           libraryItemToSave,
           uid,
           pubsub
diff --git a/packages/api/src/routers/page_router.ts b/packages/api/src/routers/page_router.ts
index f3c521301..459456292 100644
--- a/packages/api/src/routers/page_router.ts
+++ b/packages/api/src/routers/page_router.ts
@@ -13,7 +13,7 @@ import { PageType, UploadFileStatus } from '../generated/graphql'
 import { authTrx } from '../repository'
 import { Claims } from '../resolvers/types'
 import {
-  createLibraryItem,
+  createOrUpdateLibraryItem,
   findLibraryItemById,
   findLibraryItemByUrl,
   restoreLibraryItem,
@@ -101,7 +101,7 @@ export function pageRouter() {
     if (item) {
       await restoreLibraryItem(item.id, claims.uid)
     } else {
-      await createLibraryItem(
+      await createOrUpdateLibraryItem(
         {
           originalUrl: signedUrl,
           id: clientRequestId,
diff --git a/packages/api/src/routers/svc/email_attachment.ts b/packages/api/src/routers/svc/email_attachment.ts
index 6c127135b..0de5481a0 100644
--- a/packages/api/src/routers/svc/email_attachment.ts
+++ b/packages/api/src/routers/svc/email_attachment.ts
@@ -9,7 +9,7 @@ import { UploadFile } from '../../entity/upload_file'
 import { env } from '../../env'
 import { PageType, UploadFileStatus } from '../../generated/graphql'
 import { authTrx } from '../../repository'
-import { createLibraryItem } from '../../services/library_item'
+import { createOrUpdateLibraryItem } from '../../services/library_item'
 import { findNewsletterEmailByAddress } from '../../services/newsletters'
 import { updateReceivedEmail } from '../../services/received_emails'
 import {
@@ -170,7 +170,7 @@ export function emailAttachmentRouter() {
             : ContentReaderType.EPUB,
       }
 
-      const item = await createLibraryItem(itemToCreate, user.id)
+      const item = await createOrUpdateLibraryItem(itemToCreate, user.id)
 
       // update received email type
       await updateReceivedEmail(receivedEmailId, 'article', user.id)
diff --git a/packages/api/src/routers/svc/following.ts b/packages/api/src/routers/svc/following.ts
index 834d6c531..18e912fda 100644
--- a/packages/api/src/routers/svc/following.ts
+++ b/packages/api/src/routers/svc/following.ts
@@ -6,7 +6,7 @@ import {
   PreparedDocumentInput,
 } from '../../generated/graphql'
 import { createAndSaveLabelsInLibraryItem } from '../../services/labels'
-import { createLibraryItem } from '../../services/library_item'
+import { createOrUpdateLibraryItem } from '../../services/library_item'
 import { parsedContentToLibraryItem } from '../../services/save_page'
 import { cleanUrl, generateSlug } from '../../utils/helpers'
 import { createThumbnailUrl } from '../../utils/imageproxy'
@@ -123,7 +123,7 @@ export function followingServiceRouter() {
         state: ArticleSavingRequestStatus.ContentNotFetched,
       })
 
-      const newItem = await createLibraryItem(itemToSave, userId)
+      const newItem = await createOrUpdateLibraryItem(itemToSave, userId)
       logger.info('feed item saved in following')
 
       // save RSS label in the item
diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts
index bd336f28c..ea5416d3c 100644
--- a/packages/api/src/services/create_page_save_request.ts
+++ b/packages/api/src/services/create_page_save_request.ts
@@ -18,7 +18,7 @@ import {
 import { logger } from '../utils/logger'
 import {
   countByCreatedAt,
-  createLibraryItem,
+  createOrUpdateLibraryItem,
   findLibraryItemByUrl,
   updateLibraryItem,
 } from './library_item'
@@ -118,7 +118,7 @@ export const createPageSaveRequest = async ({
     logger.info('libraryItem does not exist', { url })
 
     // create processing item
-    libraryItem = await createLibraryItem(
+    libraryItem = await createOrUpdateLibraryItem(
       {
         id: articleSavingRequestId,
         user: { id: userId },
diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts
index a715babef..c567995ba 100644
--- a/packages/api/src/services/library_item.ts
+++ b/packages/api/src/services/library_item.ts
@@ -14,15 +14,9 @@ import { LibraryItem, LibraryItemState } from '../entity/library_item'
 import { BulkActionType, InputMaybe, SortParams } from '../generated/graphql'
 import { createPubSubClient, EntityType } from '../pubsub'
 import { redisDataSource } from '../redis_data_source'
-import {
-  authTrx,
-  getColumns,
-  isUniqueViolation,
-  queryBuilderToRawSql,
-} from '../repository'
+import { authTrx, getColumns, queryBuilderToRawSql } from '../repository'
 import { libraryItemRepository } from '../repository/library_item'
-import { setRecentlySavedItemInRedis, wordsCount } from '../utils/helpers'
-import { logger } from '../utils/logger'
+import { setRecentlySavedItemInRedis } from '../utils/helpers'
 import { parseSearchQuery } from '../utils/search'
 import { addLabelsToLibraryItem } from './labels'
 
@@ -831,74 +825,44 @@ export const createLibraryItems = async (
   )
 }
 
-export const createLibraryItem = async (
+export const createOrUpdateLibraryItem = async (
   libraryItem: DeepPartial<LibraryItem>,
   userId: string,
   pubsub = createPubSubClient(),
   skipPubSub = false
 ): Promise<LibraryItem> => {
-  if (!libraryItem.originalUrl) {
-    throw new Error('Original url is required')
+  const newLibraryItem = await authTrx(
+    async (tx) =>
+      tx.withRepository(libraryItemRepository).upsertLibraryItem(libraryItem),
+    undefined,
+    userId
+  )
+
+  // set recently saved item in redis if redis is enabled
+  if (redisDataSource.redisClient) {
+    await setRecentlySavedItemInRedis(
+      redisDataSource.redisClient,
+      userId,
+      newLibraryItem.originalUrl
+    )
   }
 
-  try {
-    const newLibraryItem = await authTrx(
-      async (tx) =>
-        tx.withRepository(libraryItemRepository).save({
-          ...libraryItem,
-          wordCount:
-            libraryItem.wordCount ??
-            wordsCount(libraryItem.readableContent || ''),
-        }),
-      undefined,
-      userId
-    )
-    logger.info('item created', { url: libraryItem.originalUrl })
-
-    // set recently saved item in redis if redis is enabled
-    if (redisDataSource.redisClient) {
-      await setRecentlySavedItemInRedis(
-        redisDataSource.redisClient,
-        userId,
-        newLibraryItem.originalUrl
-      )
-    }
-
-    if (skipPubSub) {
-      return newLibraryItem
-    }
-
-    await pubsub.entityCreated<DeepPartial<LibraryItem>>(
-      EntityType.PAGE,
-      {
-        ...newLibraryItem,
-        // don't send original content and readable content
-        originalContent: undefined,
-        readableContent: undefined,
-      },
-      userId
-    )
-
+  if (skipPubSub) {
     return newLibraryItem
-  } catch (error) {
-    if (isUniqueViolation(error)) {
-      logger.info('item already created', { url: libraryItem.originalUrl })
-
-      const existingItem = await findLibraryItemByUrl(
-        libraryItem.originalUrl,
-        userId
-      )
-
-      if (!existingItem) {
-        throw new Error(`Item not found for url: ${libraryItem.originalUrl}`)
-      }
-
-      return existingItem
-    }
-
-    logger.error('error creating item', error)
-    throw error
   }
+
+  await pubsub.entityCreated<DeepPartial<LibraryItem>>(
+    EntityType.PAGE,
+    {
+      ...newLibraryItem,
+      // don't send original content and readable content
+      originalContent: undefined,
+      readableContent: undefined,
+    },
+    userId
+  )
+
+  return newLibraryItem
 }
 
 export const findLibraryItemsByPrefix = async (
diff --git a/packages/api/src/services/recommendation.ts b/packages/api/src/services/recommendation.ts
index bf563060a..cd090cdb6 100644
--- a/packages/api/src/services/recommendation.ts
+++ b/packages/api/src/services/recommendation.ts
@@ -5,7 +5,7 @@ import { Recommendation } from '../entity/recommendation'
 import { authTrx } from '../repository'
 import { logger } from '../utils/logger'
 import { createHighlights } from './highlights'
-import { createLibraryItem, findLibraryItemByUrl } from './library_item'
+import { createOrUpdateLibraryItem, findLibraryItemByUrl } from './library_item'
 
 export const addRecommendation = async (
   item: LibraryItem,
@@ -39,7 +39,7 @@ export const addRecommendation = async (
         publishedAt: item.publishedAt,
       }
 
-      recommendedItem = await createLibraryItem(newItem, userId)
+      recommendedItem = await createOrUpdateLibraryItem(newItem, userId)
 
       const highlights = item.highlights
         ?.filter((highlight) => highlightIds?.includes(highlight.id))
diff --git a/packages/api/src/services/save_email.ts b/packages/api/src/services/save_email.ts
index 8ecd25d53..ef8add223 100644
--- a/packages/api/src/services/save_email.ts
+++ b/packages/api/src/services/save_email.ts
@@ -17,7 +17,7 @@ import {
 } from '../utils/parser'
 import { createAndSaveLabelsInLibraryItem } from './labels'
 import {
-  createLibraryItem,
+  createOrUpdateLibraryItem,
   findLibraryItemByUrl,
   restoreLibraryItem,
 } from './library_item'
@@ -80,7 +80,7 @@ export const saveEmail = async (
   }
 
   // start a transaction to create the library item and update the received email
-  const newLibraryItem = await createLibraryItem(
+  const newLibraryItem = await createOrUpdateLibraryItem(
     {
       user: { id: input.userId },
       slug,
diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts
index 793c9d703..a01f20ec3 100644
--- a/packages/api/src/services/save_page.ts
+++ b/packages/api/src/services/save_page.ts
@@ -1,6 +1,5 @@
 import { Readability } from '@omnivore/readability'
 import { DeepPartial } from 'typeorm'
-import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'
 import { Highlight } from '../entity/highlight'
 import { LibraryItem, LibraryItemState } from '../entity/library_item'
 import { User } from '../entity/user'
@@ -12,8 +11,6 @@ import {
   SavePageInput,
   SaveResult,
 } from '../generated/graphql'
-import { authTrx } from '../repository'
-import { libraryItemRepository } from '../repository/library_item'
 import { enqueueThumbnailJob } from '../utils/createTask'
 import {
   cleanUrl,
@@ -28,7 +25,7 @@ import { contentReaderForLibraryItem } from '../utils/uploads'
 import { createPageSaveRequest } from './create_page_save_request'
 import { createHighlight } from './highlights'
 import { createAndSaveLabelsInLibraryItem } from './labels'
-import { createLibraryItem, updateLibraryItem } from './library_item'
+import { createOrUpdateLibraryItem } from './library_item'
 
 // where we can use APIs to fetch their underlying content.
 const FORCE_PUPPETEER_URLS = [
@@ -118,44 +115,15 @@ export const savePage = async (
       }
     }
   } else {
-    // check if the item already exists
-    const existingLibraryItem = await authTrx((t) =>
-      t
-        .withRepository(libraryItemRepository)
-        .findByUserIdAndUrl(user.id, input.url)
+    // do not publish a pubsub event if the item is imported
+    const newItem = await createOrUpdateLibraryItem(
+      itemToSave,
+      user.id,
+      undefined,
+      isImported
     )
-    if (existingLibraryItem) {
-      clientRequestId = existingLibraryItem.id
-      slug = existingLibraryItem.slug
-
-      // we don't want to update an rss feed item if rss-feeder is tring to re-save it
-      if (existingLibraryItem.subscription === input.rssFeedUrl) {
-        return {
-          clientRequestId,
-          url: `${homePageURL()}/${user.profile.username}/${slug}`,
-        }
-      }
-
-      // update the item except for id and slug
-      await updateLibraryItem(
-        clientRequestId,
-        {
-          ...itemToSave,
-          id: undefined,
-          slug: undefined,
-        } as QueryDeepPartialEntity<LibraryItem>,
-        user.id
-      )
-    } else {
-      // do not publish a pubsub event if the item is imported
-      const newItem = await createLibraryItem(
-        itemToSave,
-        user.id,
-        undefined,
-        isImported
-      )
-      clientRequestId = newItem.id
-    }
+    clientRequestId = newItem.id
+    slug = newItem.slug
 
     await createAndSaveLabelsInLibraryItem(
       clientRequestId,
diff --git a/packages/api/src/services/upload_file.ts b/packages/api/src/services/upload_file.ts
index 1df0ab97f..1d284916f 100644
--- a/packages/api/src/services/upload_file.ts
+++ b/packages/api/src/services/upload_file.ts
@@ -17,7 +17,7 @@ import {
   generateUploadSignedUrl,
 } from '../utils/uploads'
 import { validateUrl } from './create_page_save_request'
-import { createLibraryItem } from './library_item'
+import { createOrUpdateLibraryItem } from './library_item'
 
 const isFileUrl = (url: string): boolean => {
   const parsedUrl = new URL(url)
@@ -122,7 +122,7 @@ export const uploadFile = async (
     // If we have a file:// URL, don't try to match it
     // and create a copy of the item, just create a
     // new item.
-    const item = await createLibraryItem(
+    const item = await createOrUpdateLibraryItem(
       {
         id: input.clientRequestId || undefined,
         originalUrl: isFileUrl(input.url) ? attachmentUrl : input.url,
diff --git a/packages/api/test/db.ts b/packages/api/test/db.ts
index 01c9f9b6a..e618b47f3 100644
--- a/packages/api/test/db.ts
+++ b/packages/api/test/db.ts
@@ -12,7 +12,7 @@ import { authTrx, getRepository, setClaims } from '../src/repository'
 import { highlightRepository } from '../src/repository/highlight'
 import { userRepository } from '../src/repository/user'
 import { createUser } from '../src/services/create_user'
-import { createLibraryItem } from '../src/services/library_item'
+import { createOrUpdateLibraryItem } from '../src/services/library_item'
 import { createDeviceToken } from '../src/services/user_device_tokens'
 import {
   bulkEnqueueUpdateLabels,
@@ -120,7 +120,7 @@ export const createTestLibraryItem = async (
     slug: 'test-with-omnivore',
   }
 
-  const createdItem = await createLibraryItem(item, userId)
+  const createdItem = await createOrUpdateLibraryItem(item, userId)
   if (labels) {
     await saveLabelsInLibraryItem(labels, createdItem.id, userId)
   }

From 250d9089b384fba4ccab86fef7f39a93dc1c5405 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 13:10:54 +0800
Subject: [PATCH 2/7] fix tests

---
 packages/api/test/resolvers/article.test.ts | 47 ++++++++++++---------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts
index abf05f437..d35ef6f4f 100644
--- a/packages/api/test/resolvers/article.test.ts
+++ b/packages/api/test/resolvers/article.test.ts
@@ -23,7 +23,7 @@ import { getRepository } from '../../src/repository'
 import { createGroup, deleteGroup } from '../../src/services/groups'
 import { createLabel, deleteLabels } from '../../src/services/labels'
 import {
-  createLibraryItem,
+  createOrUpdateLibraryItem,
   createLibraryItems,
   deleteLibraryItemById,
   deleteLibraryItemByUrl,
@@ -408,7 +408,7 @@ describe('Article API', () => {
         document = '<p>test</p>'
         title = 'new title'
 
-        const item = await createLibraryItem(
+        const item = await createOrUpdateLibraryItem(
           {
             readableContent: document,
             slug: 'test saving an archived article slug',
@@ -450,17 +450,22 @@ describe('Article API', () => {
         readingProgressTopPercent: 100,
         user,
         originalUrl: 'https://blog.omnivore.app/test-with-omnivore',
-        highlights: [
-          {
-            shortId: 'test short id',
-            patch: 'test patch',
-            quote: 'test quote',
-            user,
-          },
-        ],
       }
-      const item = await createLibraryItem(itemToCreate, user.id)
+      const item = await createOrUpdateLibraryItem(itemToCreate, user.id)
       itemId = item.id
+
+      // save highlights
+      await createHighlight(
+        {
+          shortId: 'test short id',
+          patch: 'test patch',
+          quote: 'test quote',
+          user,
+          libraryItem: item,
+        },
+        itemId,
+        user.id
+      )
     })
 
     after(async () => {
@@ -698,7 +703,7 @@ describe('Article API', () => {
         originalUrl: 'https://blog.omnivore.app/setBookmarkArticle',
         slug: 'test-with-omnivore',
       }
-      const item = await createLibraryItem(itemToSave, user.id)
+      const item = await createOrUpdateLibraryItem(itemToSave, user.id)
       itemId = item.id
     })
 
@@ -802,7 +807,7 @@ describe('Article API', () => {
     context('when force is true', () => {
       before(async () => {
         itemId = (
-          await createLibraryItem(
+          await createOrUpdateLibraryItem(
             {
               user: { id: user.id },
               originalUrl: 'https://blog.omnivore.app/setBookmarkArticle',
@@ -843,7 +848,7 @@ describe('Article API', () => {
     let itemId = ''
 
     before(async () => {
-      const item = await createLibraryItem(
+      const item = await createOrUpdateLibraryItem(
         {
           user: { id: user.id },
           originalUrl: 'https://blog.omnivore.app/setBookmarkArticle',
@@ -928,7 +933,7 @@ describe('Article API', () => {
           siteName: 'Example',
           readingProgressBottomPercent: readingProgressArray[i],
         }
-        const item = await createLibraryItem(itemToSave, user.id)
+        const item = await createOrUpdateLibraryItem(itemToSave, user.id)
         items.push(item)
 
         // Create some test highlights
@@ -1973,7 +1978,7 @@ describe('Article API', () => {
           slug: '',
           originalUrl: `https://blog.omnivore.app/p/typeahead-search-${i}`,
         }
-        const item = await createLibraryItem(itemToSave, user.id)
+        const item = await createOrUpdateLibraryItem(itemToSave, user.id)
         items.push(item)
       }
     })
@@ -2047,7 +2052,7 @@ describe('Article API', () => {
           originalUrl: `https://blog.omnivore.app/p/updates-since-${i}`,
           user,
         }
-        const item = await createLibraryItem(itemToSave, user.id)
+        const item = await createOrUpdateLibraryItem(itemToSave, user.id)
         items.push(item)
       }
 
@@ -2163,7 +2168,7 @@ describe('Article API', () => {
       before(async () => {
         // Create some test items
         for (let i = 0; i < 5; i++) {
-          await createLibraryItem(
+          await createOrUpdateLibraryItem(
             {
               user,
               itemType: i == 0 ? PageType.Article : PageType.File,
@@ -2256,7 +2261,7 @@ describe('Article API', () => {
       before(async () => {
         // Create some test items
         for (let i = 0; i < 5; i++) {
-          const item = await createLibraryItem(
+          const item = await createOrUpdateLibraryItem(
             {
               user,
               itemType: i == 0 ? PageType.Article : PageType.File,
@@ -2318,7 +2323,7 @@ describe('Article API', () => {
         readableContent: '<p>test</p>',
         originalUrl: `https://blog.omnivore.app/p/setFavoriteArticle`,
       }
-      const item = await createLibraryItem(itemToSave, user.id)
+      const item = await createOrUpdateLibraryItem(itemToSave, user.id)
       articleId = item.id
     })
 
@@ -2365,7 +2370,7 @@ describe('Article API', () => {
           deletedAt: new Date(),
           state: LibraryItemState.Deleted,
         }
-        const item = await createLibraryItem(itemToSave, user.id)
+        const item = await createOrUpdateLibraryItem(itemToSave, user.id)
         items.push(item)
       }
     })

From a37bfab3480a692fa0ddd1e2ec403d60a4b7b881 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 15:28:50 +0800
Subject: [PATCH 3/7] update create article api

---
 packages/api/src/resolvers/article/index.ts   | 35 ++---------
 .../src/services/create_page_save_request.ts  | 61 ++++++-------------
 2 files changed, 25 insertions(+), 71 deletions(-)

diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts
index b158c3a9f..135e5e845 100644
--- a/packages/api/src/resolvers/article/index.ts
+++ b/packages/api/src/resolvers/article/index.ts
@@ -340,29 +340,12 @@ export const createArticleResolver = authorized<
         }
       }
 
-      let libraryItemToReturn: LibraryItem
-
-      const existingLibraryItem = await findLibraryItemByUrl(
-        libraryItemToSave.originalUrl,
-        uid
+      // create new item in database
+      const libraryItemToReturn = await createOrUpdateLibraryItem(
+        libraryItemToSave,
+        uid,
+        pubsub
       )
-      articleSavingRequestId = existingLibraryItem?.id || articleSavingRequestId
-      if (articleSavingRequestId) {
-        // update existing item's state from processing to succeeded
-        libraryItemToReturn = await updateLibraryItem(
-          articleSavingRequestId,
-          libraryItemToSave as QueryDeepPartialEntity<LibraryItem>,
-          uid,
-          pubsub
-        )
-      } else {
-        // create new item in database
-        libraryItemToReturn = await createOrUpdateLibraryItem(
-          libraryItemToSave,
-          uid,
-          pubsub
-        )
-      }
 
       await createAndSaveLabelsInLibraryItem(
         libraryItemToReturn.id,
@@ -371,14 +354,6 @@ export const createArticleResolver = authorized<
         rssFeedUrl
       )
 
-      log.info(
-        'item created in database',
-        libraryItemToReturn.id,
-        libraryItemToReturn.originalUrl,
-        libraryItemToReturn.slug,
-        libraryItemToReturn.title
-      )
-
       return {
         user,
         created: true,
diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts
index ea5416d3c..733559109 100644
--- a/packages/api/src/services/create_page_save_request.ts
+++ b/packages/api/src/services/create_page_save_request.ts
@@ -16,12 +16,7 @@ import {
   libraryItemToArticleSavingRequest,
 } from '../utils/helpers'
 import { logger } from '../utils/logger'
-import {
-  countByCreatedAt,
-  createOrUpdateLibraryItem,
-  findLibraryItemByUrl,
-  updateLibraryItem,
-} from './library_item'
+import { countByCreatedAt, createOrUpdateLibraryItem } from './library_item'
 
 interface PageSaveRequest {
   userId: string
@@ -112,42 +107,26 @@ export const createPageSaveRequest = async ({
   }
 
   url = cleanUrl(url)
-  // look for existing library item
-  let libraryItem = await findLibraryItemByUrl(url, userId)
-  if (!libraryItem) {
-    logger.info('libraryItem does not exist', { url })
 
-    // create processing item
-    libraryItem = await createOrUpdateLibraryItem(
-      {
-        id: articleSavingRequestId,
-        user: { id: userId },
-        readableContent: SAVING_CONTENT,
-        itemType: PageType.Unknown,
-        slug: generateSlug(url),
-        title: url,
-        originalUrl: url,
-        state: LibraryItemState.Processing,
-        publishedAt,
-        folder,
-        subscription,
-        savedAt,
-      },
-      userId,
-      pubsub
-    )
-  }
-  // reset state to processing
-  if (libraryItem.state !== LibraryItemState.Processing) {
-    libraryItem = await updateLibraryItem(
-      libraryItem.id,
-      {
-        state: LibraryItemState.Processing,
-      },
-      userId,
-      pubsub
-    )
-  }
+  // create processing item
+  const libraryItem = await createOrUpdateLibraryItem(
+    {
+      id: articleSavingRequestId,
+      user: { id: userId },
+      readableContent: SAVING_CONTENT,
+      itemType: PageType.Unknown,
+      slug: generateSlug(url),
+      title: url,
+      originalUrl: url,
+      state: LibraryItemState.Processing,
+      publishedAt,
+      folder,
+      subscription,
+      savedAt,
+    },
+    userId,
+    pubsub
+  )
 
   // get priority by checking rate limit if not specified
   priority = priority || (await getPriorityByRateLimit(userId))

From 7e022e32949c3df9e1776fb1f1c3d686e2790adf Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 16:16:03 +0800
Subject: [PATCH 4/7] reduce find user query

---
 packages/api/src/resolvers/article/index.ts   | 17 +++++++++++-----
 .../resolvers/article_saving_request/index.ts |  7 ++++++-
 packages/api/src/routers/article_router.ts    | 20 ++++++++-----------
 packages/api/src/routers/svc/links.ts         |  8 +++++++-
 .../src/services/create_page_save_request.ts  | 15 ++++----------
 packages/api/src/services/save_page.ts        |  2 +-
 packages/api/src/services/save_url.ts         |  2 +-
 7 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts
index 135e5e845..54ae987c5 100644
--- a/packages/api/src/resolvers/article/index.ts
+++ b/packages/api/src/resolvers/article/index.ts
@@ -261,7 +261,7 @@ export const createArticleResolver = authorized<
         FORCE_PUPPETEER_URLS.some((regex) => regex.test(url))
       ) {
         await createPageSaveRequest({
-          userId: uid,
+          user: userData,
           url,
           state: state || undefined,
           labels: inputLabels || undefined,
@@ -286,7 +286,7 @@ export const createArticleResolver = authorized<
         // We have a URL but no document, so we try to send this to puppeteer
         // and return a dummy response.
         await createPageSaveRequest({
-          userId: uid,
+          user: userData,
           url,
           state: state || undefined,
           labels: inputLabels || undefined,
@@ -977,7 +977,7 @@ export const moveToFolderResolver = authorized<
   if (item.state === LibraryItemState.ContentNotFetched) {
     try {
       await createPageSaveRequest({
-        userId: uid,
+        user: item.user,
         url: item.originalUrl,
         articleSavingRequestId: id,
         priority: 'high',
@@ -1013,7 +1013,14 @@ export const fetchContentResolver = authorized<
     },
   })
 
-  const item = await findLibraryItemById(id, uid)
+  const item = await authTrx((tx) =>
+    tx.getRepository(LibraryItem).findOne({
+      where: {
+        id,
+      },
+      relations: ['user'],
+    })
+  )
   if (!item) {
     return {
       errorCodes: [FetchContentErrorCode.Unauthorized],
@@ -1024,7 +1031,7 @@ export const fetchContentResolver = authorized<
   if (item.state === LibraryItemState.ContentNotFetched) {
     try {
       await createPageSaveRequest({
-        userId: uid,
+        user: item.user,
         url: item.originalUrl,
         articleSavingRequestId: id,
         priority: 'high',
diff --git a/packages/api/src/resolvers/article_saving_request/index.ts b/packages/api/src/resolvers/article_saving_request/index.ts
index 9f4cf7c4a..b42171f6f 100644
--- a/packages/api/src/resolvers/article_saving_request/index.ts
+++ b/packages/api/src/resolvers/article_saving_request/index.ts
@@ -42,9 +42,14 @@ export const createArticleSavingRequestResolver = authorized<
     },
   })
 
+  const user = await userRepository.findById(uid)
+  if (!user) {
+    return { errorCodes: [CreateArticleSavingRequestErrorCode.Unauthorized] }
+  }
+
   try {
     const articleSavingRequest = await createPageSaveRequest({
-      userId: uid,
+      user,
       url,
       pubsub,
     })
diff --git a/packages/api/src/routers/article_router.ts b/packages/api/src/routers/article_router.ts
index efceb02ef..6b3bab37f 100644
--- a/packages/api/src/routers/article_router.ts
+++ b/packages/api/src/routers/article_router.ts
@@ -7,6 +7,7 @@ import * as jwt from 'jsonwebtoken'
 import { Speech } from '../entity/speech'
 import { env } from '../env'
 import { CreateArticleErrorCode } from '../generated/graphql'
+import { userRepository } from '../repository/user'
 import { Claims } from '../resolvers/types'
 import { createPageSaveRequest } from '../services/create_page_save_request'
 import { findLibraryItemById } from '../services/library_item'
@@ -32,6 +33,9 @@ export function articleRouter() {
     const { url } = req.body as {
       url?: string
     }
+    if (!url) {
+      return res.status(400).send({ errorCode: 'BAD_DATA' })
+    }
 
     const token = req?.cookies?.auth || req?.headers?.authorization
     const claims = await getClaimsByToken(token)
@@ -40,20 +44,12 @@ export function articleRouter() {
     }
 
     const { uid } = claims
-
-    logger.info('Article saving request', {
-      body: req.body,
-      labels: {
-        source: 'SaveEndpoint',
-        userId: uid,
-      },
-    })
-
-    if (!url) {
-      return res.status(400).send({ errorCode: 'BAD_DATA' })
+    const user = await userRepository.findById(uid)
+    if (!user) {
+      return res.status(400).send('Bad Request')
     }
 
-    const result = await createPageSaveRequest({ userId: uid, url })
+    const result = await createPageSaveRequest({ user, url })
 
     if (isSiteBlockedForParse(url)) {
       return res
diff --git a/packages/api/src/routers/svc/links.ts b/packages/api/src/routers/svc/links.ts
index 05d69225d..f247b6ffa 100644
--- a/packages/api/src/routers/svc/links.ts
+++ b/packages/api/src/routers/svc/links.ts
@@ -5,6 +5,7 @@ import express from 'express'
 import { LessThan } from 'typeorm'
 import { LibraryItemState } from '../../entity/library_item'
 import { readPushSubscription } from '../../pubsub'
+import { userRepository } from '../../repository/user'
 import { createPageSaveRequest } from '../../services/create_page_save_request'
 import { deleteLibraryItemsByAdmin } from '../../services/library_item'
 import { logger } from '../../utils/logger'
@@ -65,9 +66,14 @@ export function linkServiceRouter() {
     }
     const msg = data as CreateLinkRequestMessage
 
+    const user = await userRepository.findById(msg.userId)
+    if (!user) {
+      return res.status(400).send('Bad Request')
+    }
+
     try {
       const request = await createPageSaveRequest({
-        userId: msg.userId,
+        user,
         url: msg.url,
       })
 
diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts
index 733559109..403ccf3f0 100644
--- a/packages/api/src/services/create_page_save_request.ts
+++ b/packages/api/src/services/create_page_save_request.ts
@@ -1,5 +1,6 @@
 import * as privateIpLib from 'private-ip'
 import { LibraryItemState } from '../entity/library_item'
+import { User } from '../entity/user'
 import {
   ArticleSavingRequest,
   ArticleSavingRequestStatus,
@@ -8,7 +9,6 @@ import {
   PageType,
 } from '../generated/graphql'
 import { createPubSubClient, PubsubClient } from '../pubsub'
-import { userRepository } from '../repository/user'
 import { enqueueParseRequest } from '../utils/createTask'
 import {
   cleanUrl,
@@ -19,7 +19,7 @@ import { logger } from '../utils/logger'
 import { countByCreatedAt, createOrUpdateLibraryItem } from './library_item'
 
 interface PageSaveRequest {
-  userId: string
+  user: User
   url: string
   pubsub?: PubsubClient
   articleSavingRequestId?: string
@@ -75,7 +75,7 @@ export const validateUrl = (url: string): URL => {
 }
 
 export const createPageSaveRequest = async ({
-  userId,
+  user,
   url,
   pubsub = createPubSubClient(),
   articleSavingRequestId,
@@ -97,15 +97,8 @@ export const createPageSaveRequest = async ({
       errorCode: CreateArticleSavingRequestErrorCode.BadData,
     })
   }
-  // if user is not specified, get it from the database
-  const user = await userRepository.findById(userId)
-  if (!user) {
-    logger.info(`User not found: ${userId}`)
-    return Promise.reject({
-      errorCode: CreateArticleSavingRequestErrorCode.BadData,
-    })
-  }
 
+  const userId = user.id
   url = cleanUrl(url)
 
   // create processing item
diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts
index a01f20ec3..d8e28a43c 100644
--- a/packages/api/src/services/save_page.ts
+++ b/packages/api/src/services/save_page.ts
@@ -100,7 +100,7 @@ export const savePage = async (
   if (shouldParseInBackend(input)) {
     try {
       await createPageSaveRequest({
-        userId: user.id,
+        user,
         url: itemToSave.originalUrl,
         articleSavingRequestId: clientRequestId || undefined,
         state: input.state || undefined,
diff --git a/packages/api/src/services/save_url.ts b/packages/api/src/services/save_url.ts
index 806db0e53..04089e0ea 100644
--- a/packages/api/src/services/save_url.ts
+++ b/packages/api/src/services/save_url.ts
@@ -12,7 +12,7 @@ export const saveUrl = async (
   try {
     const pageSaveRequest = await createPageSaveRequest({
       ...input,
-      userId: user.id,
+      user,
       articleSavingRequestId: input.clientRequestId,
       state: input.state || undefined,
       labels: input.labels || undefined,

From efd8119e02e1087c28ce67ffef2c208412866a05 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 16:23:16 +0800
Subject: [PATCH 5/7] reduce sql error

---
 packages/api/src/services/highlights.ts       |  2 +-
 packages/api/src/services/labels.ts           | 10 ++++++----
 packages/api/test/resolvers/highlight.test.ts |  4 ++--
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts
index 436a3dd1a..ac6b08f0c 100644
--- a/packages/api/src/services/highlights.ts
+++ b/packages/api/src/services/highlights.ts
@@ -180,7 +180,7 @@ export const findHighlightById = async (
   return authTrx(
     async (tx) => {
       const highlightRepo = tx.withRepository(highlightRepository)
-      return highlightRepo.findOneByOrFail({
+      return highlightRepo.findOneBy({
         id: highlightId,
       })
     },
diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts
index c4f8d4d13..33206093d 100644
--- a/packages/api/src/services/labels.ts
+++ b/packages/api/src/services/labels.ts
@@ -187,10 +187,12 @@ export const saveLabelsInHighlight = async (
   )
 
   const highlight = await findHighlightById(highlightId, userId)
-  // update labels in library item
-  await bulkEnqueueUpdateLabels([
-    { libraryItemId: highlight.libraryItemId, userId },
-  ])
+  if (highlight) {
+    // update labels in library item
+    await bulkEnqueueUpdateLabels([
+      { libraryItemId: highlight.libraryItemId, userId },
+    ])
+  }
 }
 
 export const findLabelsByIds = async (
diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts
index 1d839d515..a81602cd1 100644
--- a/packages/api/test/resolvers/highlight.test.ts
+++ b/packages/api/test/resolvers/highlight.test.ts
@@ -295,8 +295,8 @@ describe('Highlights API', () => {
       expect(res.body.data.mergeHighlight.highlight.id).to.eq(newHighlightId)
 
       const highlight = await findHighlightById(newHighlightId, user.id)
-      expect(highlight.labels).to.have.lengthOf(1)
-      expect(highlight.labels?.[0]?.name).to.eq(labelName)
+      expect(highlight?.labels).to.have.lengthOf(1)
+      expect(highlight?.labels?.[0]?.name).to.eq(labelName)
 
       highlightId = newHighlightId
     })

From 1b60694579d6b8da5f19426e07a453608709c27c Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 16:40:51 +0800
Subject: [PATCH 6/7] lint

---
 packages/api/src/resolvers/article/index.ts | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts
index 54ae987c5..e3eb2a0c7 100644
--- a/packages/api/src/resolvers/article/index.ts
+++ b/packages/api/src/resolvers/article/index.ts
@@ -6,7 +6,6 @@
 import { Readability } from '@omnivore/readability'
 import graphqlFields from 'graphql-fields'
 import { IsNull } from 'typeorm'
-import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'
 import { LibraryItem, LibraryItemState } from '../../entity/library_item'
 import { env } from '../../env'
 import {
@@ -75,8 +74,6 @@ import {
   batchDelete,
   batchUpdateLibraryItems,
   createOrUpdateLibraryItem,
-  findLibraryItemById,
-  findLibraryItemByUrl,
   findLibraryItemsByPrefix,
   searchLibraryItems,
   sortParamsToSort,
@@ -582,7 +579,7 @@ export const saveArticleReadingProgressResolver = authorized<
         force,
       },
     },
-    { pubsub, uid, dataSources }
+    { authTrx, pubsub, uid, dataSources }
   ) => {
     if (
       readingProgressPercent < 0 ||
@@ -1004,7 +1001,7 @@ export const fetchContentResolver = authorized<
   FetchContentSuccess,
   FetchContentError,
   MutationFetchContentArgs
->(async (_, { id }, { uid, log, pubsub }) => {
+>(async (_, { id }, { authTrx, uid, log, pubsub }) => {
   analytics.track({
     userId: uid,
     event: 'fetch_content',

From d7a81483cfd772bf1ba7b9213b94e94bc271ae2f Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Tue, 6 Feb 2024 17:02:00 +0800
Subject: [PATCH 7/7] resolve conflicts

---
 packages/api/src/resolvers/article/index.ts | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts
index e3eb2a0c7..34c7abdd6 100644
--- a/packages/api/src/resolvers/article/index.ts
+++ b/packages/api/src/resolvers/article/index.ts
@@ -595,7 +595,14 @@ export const saveArticleReadingProgressResolver = authorized<
     // We don't need to update the values of reading progress here
     // because the function resolver will handle that for us when
     // it resolves the properties of the Article object
-    let updatedItem = await findLibraryItemById(id, uid)
+    let updatedItem = await authTrx((tx) =>
+      tx.getRepository(LibraryItem).findOne({
+        where: {
+          id,
+        },
+        relations: ['user'],
+      })
+    )
     if (!updatedItem) {
       return {
         errorCodes: [SaveArticleReadingProgressErrorCode.Unauthorized],