From 4eba7df84c86bf3e4f3da7f44a9e2151f9ec4f75 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 6 Feb 2024 13:02:12 +0800 Subject: [PATCH] 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 = (repository: Repository): (keyof T)[] => { ) as (keyof T)[] } +export const getColumnsDbName = (repository: Repository): 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) => { + 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) { + // 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, userId: string, pubsub = createPubSubClient(), skipPubSub = false ): Promise => { - 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>( - 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>( + 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, - 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) }