From a807a4a18e42d5681911f0ee643feb42b2bb1060 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 22 Jan 2024 10:45:47 +0800 Subject: [PATCH 1/3] fix: do not throw error when creating duplicate item in db --- packages/api/src/routers/svc/following.ts | 35 +++------ packages/api/src/services/library_item.ts | 93 ++++++++++++++--------- 2 files changed, 68 insertions(+), 60 deletions(-) diff --git a/packages/api/src/routers/svc/following.ts b/packages/api/src/routers/svc/following.ts index 3e0b0eedb..b6e0a595f 100644 --- a/packages/api/src/routers/svc/following.ts +++ b/packages/api/src/routers/svc/following.ts @@ -5,7 +5,6 @@ import { PageType, PreparedDocumentInput, } from '../../generated/graphql' -import { isUniqueViolation } from '../../repository' import { createAndSaveLabelsInLibraryItem } from '../../services/labels' import { createLibraryItem } from '../../services/library_item' import { parsedContentToLibraryItem } from '../../services/save_page' @@ -126,35 +125,21 @@ export function followingServiceRouter() { state: ArticleSavingRequestStatus.ContentNotFetched, }) - try { - const newItem = await createLibraryItem(itemToSave, userId) - logger.info('feed item saved in following') + const newItem = await createLibraryItem(itemToSave, userId) + logger.info('feed item saved in following') - // save RSS label in the item - await createAndSaveLabelsInLibraryItem( - newItem.id, - userId, - [{ name: 'RSS' }], - feedUrl - ) - - logger.info('RSS label added to the item') - } catch (error) { - if (isUniqueViolation(error)) { - logger.info('feed item already saved') - return res.sendStatus(200) - } - - logger.error('error saving feed item', error) - - return res.sendStatus(500) - } + // save RSS label in the item + await createAndSaveLabelsInLibraryItem( + newItem.id, + userId, + [{ name: 'RSS' }], + feedUrl + ) + logger.info('RSS label added to the item') return res.sendStatus(200) } res.sendStatus(200) }) - - return router } diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index be5a9d4e4..6084e50ac 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -8,16 +8,18 @@ import { Label } from '../entity/label' 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, valuesToRawSql, } from '../repository' import { libraryItemRepository } from '../repository/library_item' import { setRecentlySavedItemInRedis, wordsCount } from '../utils/helpers' +import { logger } from '../utils/logger' import { parseSearchQuery } from '../utils/search' -import { redisDataSource } from '../redis_data_source' enum ReadFilter { ALL = 'all', @@ -806,48 +808,69 @@ export const createLibraryItems = async ( } export const createLibraryItem = async ( - libraryItem: DeepPartial, + libraryItem: DeepPartial & { originalUrl: string }, userId: string, pubsub = createPubSubClient(), skipPubSub = false ): Promise => { - const newLibraryItem = await authTrx( - async (tx) => - tx.withRepository(libraryItemRepository).save({ - ...libraryItem, - wordCount: - libraryItem.wordCount ?? - wordsCount(libraryItem.readableContent || ''), - }), - 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 ( From 5964d827c812a8b0c363ad2a574b9ffd95ad9f38 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 22 Jan 2024 11:04:11 +0800 Subject: [PATCH 2/3] fix tests --- packages/api/src/repository/library_item.ts | 17 ++++ packages/api/src/routers/svc/following.ts | 3 + packages/api/src/services/library_item.ts | 90 +++++++-------------- packages/api/test/db.ts | 2 +- 4 files changed, 52 insertions(+), 60 deletions(-) diff --git a/packages/api/src/repository/library_item.ts b/packages/api/src/repository/library_item.ts index 1e7e3d708..2b8b1ee97 100644 --- a/packages/api/src/repository/library_item.ts +++ b/packages/api/src/repository/library_item.ts @@ -1,9 +1,26 @@ +import { DeepPartial } from 'typeorm' import { appDataSource } from '../data_source' import { LibraryItem } from '../entity/library_item' +import { wordsCount } from '../utils/helpers' export const libraryItemRepository = appDataSource .getRepository(LibraryItem) .extend({ + save(libraryItem: DeepPartial) { + return this.createQueryBuilder() + .insert() + .values({ + ...libraryItem, + wordCount: + libraryItem.wordCount ?? + wordsCount(libraryItem.readableContent || ''), + }) + .orIgnore() // ignore if the item already exists + .returning('*') + .execute() + .then((result) => result.generatedMaps[0] as LibraryItem) + }, + findById(id: string) { return this.findOneBy({ id }) }, diff --git a/packages/api/src/routers/svc/following.ts b/packages/api/src/routers/svc/following.ts index b6e0a595f..261dbf4ee 100644 --- a/packages/api/src/routers/svc/following.ts +++ b/packages/api/src/routers/svc/following.ts @@ -137,9 +137,12 @@ export function followingServiceRouter() { ) logger.info('RSS label added to the item') + return res.sendStatus(200) } res.sendStatus(200) }) + + return router } diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 6084e50ac..a3b87140b 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -12,12 +12,11 @@ import { redisDataSource } from '../redis_data_source' import { authTrx, getColumns, - isUniqueViolation, queryBuilderToRawSql, valuesToRawSql, } from '../repository' import { libraryItemRepository } from '../repository/library_item' -import { setRecentlySavedItemInRedis, wordsCount } from '../utils/helpers' +import { setRecentlySavedItemInRedis } from '../utils/helpers' import { logger } from '../utils/logger' import { parseSearchQuery } from '../utils/search' @@ -808,69 +807,42 @@ export const createLibraryItems = async ( } export const createLibraryItem = async ( - libraryItem: DeepPartial & { originalUrl: string }, + libraryItem: DeepPartial, userId: string, pubsub = createPubSubClient(), skipPubSub = false ): Promise => { - try { - const newLibraryItem = await authTrx( - async (tx) => - tx.withRepository(libraryItemRepository).save({ - ...libraryItem, - wordCount: - libraryItem.wordCount ?? - wordsCount(libraryItem.readableContent || ''), - }), - undefined, - userId + const newLibraryItem = await authTrx( + async (tx) => tx.withRepository(libraryItemRepository).save(libraryItem), + undefined, + userId + ) + + // set recently saved item in redis if redis is enabled + if (redisDataSource.redisClient) { + await setRecentlySavedItemInRedis( + redisDataSource.redisClient, + userId, + newLibraryItem.originalUrl ) - 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 - ) - - 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 } + + if (skipPubSub) { + return newLibraryItem + } + + 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/test/db.ts b/packages/api/test/db.ts index d7ed692d9..26a02e965 100644 --- a/packages/api/test/db.ts +++ b/packages/api/test/db.ts @@ -106,7 +106,7 @@ export const createTestLibraryItem = async ( userId: string, labels?: Label[] ): Promise => { - const item: DeepPartial = { + const item = { user: { id: userId }, title: 'test title', originalContent: '

test content

', From 141036dc6c2900f5293604d967473d8ae8d1e771 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 22 Jan 2024 12:18:08 +0800 Subject: [PATCH 3/3] fix tests --- packages/api/src/repository/library_item.ts | 17 ---- packages/api/src/services/library_item.ts | 86 ++++++++++++++------- 2 files changed, 59 insertions(+), 44 deletions(-) diff --git a/packages/api/src/repository/library_item.ts b/packages/api/src/repository/library_item.ts index 2b8b1ee97..1e7e3d708 100644 --- a/packages/api/src/repository/library_item.ts +++ b/packages/api/src/repository/library_item.ts @@ -1,26 +1,9 @@ -import { DeepPartial } from 'typeorm' import { appDataSource } from '../data_source' import { LibraryItem } from '../entity/library_item' -import { wordsCount } from '../utils/helpers' export const libraryItemRepository = appDataSource .getRepository(LibraryItem) .extend({ - save(libraryItem: DeepPartial) { - return this.createQueryBuilder() - .insert() - .values({ - ...libraryItem, - wordCount: - libraryItem.wordCount ?? - wordsCount(libraryItem.readableContent || ''), - }) - .orIgnore() // ignore if the item already exists - .returning('*') - .execute() - .then((result) => result.generatedMaps[0] as LibraryItem) - }, - findById(id: string) { return this.findOneBy({ id }) }, diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index a3b87140b..11a7ee92a 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -12,11 +12,12 @@ import { redisDataSource } from '../redis_data_source' import { authTrx, getColumns, + isUniqueViolation, queryBuilderToRawSql, valuesToRawSql, } from '../repository' import { libraryItemRepository } from '../repository/library_item' -import { setRecentlySavedItemInRedis } from '../utils/helpers' +import { setRecentlySavedItemInRedis, wordsCount } from '../utils/helpers' import { logger } from '../utils/logger' import { parseSearchQuery } from '../utils/search' @@ -812,37 +813,68 @@ export const createLibraryItem = async ( pubsub = createPubSubClient(), skipPubSub = false ): Promise => { - const newLibraryItem = await authTrx( - async (tx) => tx.withRepository(libraryItemRepository).save(libraryItem), - undefined, - userId - ) + if (!libraryItem.originalUrl) { + throw new Error('Original url is required') + } - // 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 (