From a6c5f4e8623ff0f5f0a2d287bceb13d1e29db60a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 13 Oct 2023 13:45:09 +0800 Subject: [PATCH] fix bulk action slow query --- packages/api/src/resolvers/article/index.ts | 12 ++-- packages/api/src/services/library_item.ts | 62 +++++++++------------ packages/api/test/resolvers/article.test.ts | 16 ++++++ 3 files changed, 48 insertions(+), 42 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 1c191d0ec..af551b85e 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -5,7 +5,6 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ import { Readability } from '@omnivore/readability' import graphqlFields from 'graphql-fields' -import { Not } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { LibraryItem, LibraryItemState } from '../../entity/library_item' import { env } from '../../env' @@ -792,6 +791,12 @@ export const bulkActionResolver = authorized< }, }) + // parse query + const searchQuery = parseSearchQuery(query) + if (searchQuery.ids.length > 100) { + return { errorCodes: [BulkActionErrorCode.BadRequest] } + } + // get labels if needed let labels = undefined if (action === BulkActionType.AddLabels) { @@ -802,10 +807,7 @@ export const bulkActionResolver = authorized< labels = await findLabelsByIds(labelIds, uid) } - // parse query - const searchQuery = parseSearchQuery(query) - - await updateLibraryItems(action, searchQuery, labels) + await updateLibraryItems(action, searchQuery, uid, labels) return { success: true } } catch (error) { diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 49a363acf..628873cfb 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -1,13 +1,4 @@ -import { - Between, - DeepPartial, - In, - IsNull, - LessThan, - MoreThan, - Not, - SelectQueryBuilder, -} from 'typeorm' +import { DeepPartial, SelectQueryBuilder } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { EntityLabel } from '../entity/entity_label' import { Highlight } from '../entity/highlight' @@ -115,14 +106,10 @@ const buildWhereClause = ( if (args.inFilter !== InFilter.ALL) { switch (args.inFilter) { case InFilter.INBOX: - queryBuilder.andWhere({ - archivedAt: IsNull(), - }) + queryBuilder.andWhere('library_item.archived_at IS NULL') break case InFilter.ARCHIVE: - queryBuilder.andWhere({ - archivedAt: Not(IsNull()), - }) + queryBuilder.andWhere('library_item.archived_at IS NOT NULL') break case InFilter.TRASH: // return only deleted pages within 14 days @@ -133,19 +120,15 @@ const buildWhereClause = ( case InFilter.SUBSCRIPTION: queryBuilder .andWhere("NOT ('library' ILIKE ANY (library_item.label_names))") - .andWhere({ - subscription: Not(IsNull()), - archivedAt: IsNull(), - }) + .andWhere('library_item.archived_at IS NULL') + .andWhere('library_item.subscription IS NOT NULL') break case InFilter.LIBRARY: queryBuilder .andWhere( "(library_item.subscription IS NULL OR 'library' ILIKE ANY (library_item.label_names))" ) - .andWhere({ - archivedAt: IsNull(), - }) + .andWhere('library_item.archived_at IS NULL') break } } @@ -153,17 +136,19 @@ const buildWhereClause = ( if (args.readFilter !== ReadFilter.ALL) { switch (args.readFilter) { case ReadFilter.READ: - queryBuilder.andWhere({ - readingProgressBottomPercent: MoreThan(98), - }) + queryBuilder.andWhere( + 'library_item.reading_progress_bottom_percent > 98' + ) break case ReadFilter.READING: - queryBuilder.andWhere({ readingProgressBottomPercent: Between(2, 98) }) + queryBuilder.andWhere( + 'library_item.reading_progress_bottom_percent BETWEEN 2 AND 98' + ) break case ReadFilter.UNREAD: - queryBuilder.andWhere({ - readingProgressBottomPercent: LessThan(2), - }) + queryBuilder.andWhere( + 'library_item.reading_progress_bottom_percent < 2' + ) break } } @@ -247,20 +232,20 @@ const buildWhereClause = ( } if (args.ids && args.ids.length > 0) { - queryBuilder.andWhere({ - id: In(args.ids), + queryBuilder.andWhere('library_item.id = ANY(:ids)', { + ids: args.ids, }) } if (!args.includePending) { - queryBuilder.andWhere({ - state: Not(LibraryItemState.Processing), + queryBuilder.andWhere('library_item.state <> :state', { + state: LibraryItemState.Processing, }) } if (!args.includeDeleted && args.inFilter !== InFilter.TRASH) { - queryBuilder.andWhere({ - state: Not(LibraryItemState.Deleted), + queryBuilder.andWhere('library_item.state <> :state', { + state: LibraryItemState.Deleted, }) } @@ -528,6 +513,7 @@ export const countByCreatedAt = async ( export const updateLibraryItems = async ( action: BulkActionType, args: SearchArgs, + userId: string, labels?: Label[] ) => { // build the script @@ -561,7 +547,9 @@ export const updateLibraryItems = async ( } await authTrx(async (tx) => { - const queryBuilder = tx.createQueryBuilder(LibraryItem, 'library_item') + const queryBuilder = tx + .createQueryBuilder(LibraryItem, 'library_item') + .where('library_item.user_id = :userId', { userId }) // build the where clause buildWhereClause(queryBuilder, args) diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index d8c2545d9..aeb76604a 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -1545,6 +1545,22 @@ describe('Article API', () => { await deleteLibraryItemsByUserId(user.id) }) + context('when action is MarkAsRead and query is in:unread', () => { + it('marks unread items as read', async () => { + const res = await graphqlRequest( + bulkActionQuery(BulkActionType.MarkAsRead, 'is:unread'), + authToken + ).expect(200) + expect(res.body.data.bulkAction.success).to.be.true + + const items = await graphqlRequest( + searchQuery('is:unread'), + authToken + ).expect(200) + expect(items.body.data.search.pageInfo.totalCount).to.eql(0) + }) + }) + context('when action is Archive', () => { it('archives all items', async () => { const res = await graphqlRequest(