From 626c4563643566aaa96926e9acfa896b5830d266 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 19 Aug 2024 10:00:38 +0800 Subject: [PATCH 1/3] fix: select readable_content from library_item table if content is requested by update since api --- packages/api/src/resolvers/article/index.ts | 133 +++++++++++--------- packages/api/src/utils/gql-utils.ts | 13 ++ 2 files changed, 85 insertions(+), 61 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 808cb367a..287bdf38c 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -99,7 +99,7 @@ import { Merge } from '../../util' import { analytics } from '../../utils/analytics' import { isSiteBlockedForParse } from '../../utils/blocked' import { enqueueBulkAction } from '../../utils/createTask' -import { authorized } from '../../utils/gql-utils' +import { authorized, isFieldInSelectionSet } from '../../utils/gql-utils' import { cleanUrl, errorHandler, @@ -677,69 +677,80 @@ export const updatesSinceResolver = authorized< >, UpdatesSinceError, QueryUpdatesSinceArgs ->(async (_obj, { since, first, after, sort: sortParams, folder }, { uid }) => { - const startCursor = after || '' - const size = Math.min(first || 10, 100) // limit to 100 items - let startDate = new Date(since) - if (isNaN(startDate.getTime())) { - // for android app compatibility - startDate = new Date(0) - } - const sort = sortParamsToSort(sortParams) - - // create a search query - const query = `in:${ - folder || 'all' - } updated:${startDate.toISOString()} sort:${sort.by}-${sort.order}` - - const searchLibraryItemArgs = { - includeDeleted: true, - query, - includeContent: false, - } - - const libraryItems = await searchLibraryItems( - { - ...searchLibraryItemArgs, - from: Number(startCursor), - size: size + 1, // fetch one more item to get next cursor - useFolders: true, - }, - uid - ) - - const start = - startCursor && !isNaN(Number(startCursor)) ? Number(startCursor) : 0 - const hasNextPage = libraryItems.length > size - const endCursor = String(start + libraryItems.length - (hasNextPage ? 1 : 0)) - - //TODO: refactor so that the lastCursor included - if (hasNextPage) { - // remove an extra if exists - libraryItems.pop() - } - - const edges = libraryItems.map((item) => { - const updateReason = getUpdateReason(item, startDate) - return { - node: item, - cursor: endCursor, - itemID: item.id, - updateReason, +>( + async ( + _obj, + { since, first, after, sort: sortParams, folder }, + { uid }, + info + ) => { + const startCursor = after || '' + const size = Math.min(first || 10, 100) // limit to 100 items + let startDate = new Date(since) + if (isNaN(startDate.getTime())) { + // for android app compatibility + startDate = new Date(0) } - }) + const sort = sortParamsToSort(sortParams) - return { - edges, - pageInfo: { - hasPreviousPage: false, - startCursor, - hasNextPage, - endCursor, - searchLibraryItemArgs, - }, + // create a search query + const query = `in:${ + folder || 'all' + } updated:${startDate.toISOString()} sort:${sort.by}-${sort.order}` + + const isContentRequested = isFieldInSelectionSet(info, 'content') + + const searchLibraryItemArgs = { + includeDeleted: true, + query, + includeContent: isContentRequested, + } + + const libraryItems = await searchLibraryItems( + { + ...searchLibraryItemArgs, + from: Number(startCursor), + size: size + 1, // fetch one more item to get next cursor + useFolders: true, + }, + uid + ) + + const start = + startCursor && !isNaN(Number(startCursor)) ? Number(startCursor) : 0 + const hasNextPage = libraryItems.length > size + const endCursor = String( + start + libraryItems.length - (hasNextPage ? 1 : 0) + ) + + //TODO: refactor so that the lastCursor included + if (hasNextPage) { + // remove an extra if exists + libraryItems.pop() + } + + const edges = libraryItems.map((item) => { + const updateReason = getUpdateReason(item, startDate) + return { + node: item, + cursor: endCursor, + itemID: item.id, + updateReason, + } + }) + + return { + edges, + pageInfo: { + hasPreviousPage: false, + startCursor, + hasNextPage, + endCursor, + searchLibraryItemArgs, + }, + } } -}) +) export const bulkActionResolver = authorized< BulkActionSuccess, diff --git a/packages/api/src/utils/gql-utils.ts b/packages/api/src/utils/gql-utils.ts index 62bbf2d09..180fbabcf 100644 --- a/packages/api/src/utils/gql-utils.ts +++ b/packages/api/src/utils/gql-utils.ts @@ -1,3 +1,4 @@ +import { GraphQLResolveInfo } from 'graphql' import { ResolverFn } from '../generated/graphql' import { Claims, ResolverContext } from '../resolvers/types' @@ -24,3 +25,15 @@ export function authorized< return { errorCodes: ['UNAUTHORIZED'] } as TError } } + +export const isFieldInSelectionSet = ( + info: GraphQLResolveInfo, + fieldName: string +): boolean => { + return info.fieldNodes.some((node) => { + return node.selectionSet?.selections.some( + (selection) => + selection.kind === 'Field' && selection.name.value === fieldName + ) + }) +} From 19ed5501f6684d940037de2fdb5c16d1489ded56 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 19 Aug 2024 10:09:42 +0800 Subject: [PATCH 2/3] fix: select readable_content from library_item table if content is requested by search api --- packages/api/src/resolvers/article/index.ts | 10 +++++----- packages/api/src/resolvers/function_resolvers.ts | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 287bdf38c..e4e34e912 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -596,7 +596,7 @@ export const searchResolver = authorized< >, SearchError, QuerySearchArgs ->(async (_obj, params, { uid }) => { +>(async (_obj, params, { uid }, info) => { const startCursor = params.after || '' const first = Math.min(params.first || 10, 100) // limit to 100 items @@ -605,9 +605,11 @@ export const searchResolver = authorized< return { errorCodes: [SearchErrorCode.QueryTooLong] } } + const isContentRequested = isFieldInSelectionSet(info, 'content') + const searchLibraryItemArgs = { includePending: true, - includeContent: params.includeContent ?? true, // by default include content for offline use for now + includeContent: params.includeContent || isContentRequested, includeDeleted: params.query?.includes('in:trash'), query: params.query, useFolders: params.query?.includes('use:folders'), @@ -698,12 +700,10 @@ export const updatesSinceResolver = authorized< folder || 'all' } updated:${startDate.toISOString()} sort:${sort.by}-${sort.order}` - const isContentRequested = isFieldInSelectionSet(info, 'content') - const searchLibraryItemArgs = { includeDeleted: true, query, - includeContent: isContentRequested, + includeContent: false, } const libraryItems = await searchLibraryItems( diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index d3cd06b1a..d042b7811 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -577,6 +577,7 @@ export const functionResolvers = { pageType: (item: LibraryItem) => item.itemType, highlightsCount: (item: LibraryItem) => item.highlightAnnotations?.length, ...readingProgressHandlers, + content: (item: LibraryItem) => item.readableContent, }, PageInfo: { async totalCount( From 160d690589f09bf1bea6da12c15ea1fdc268d1f2 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 19 Aug 2024 10:21:24 +0800 Subject: [PATCH 3/3] fix: do not select content from database for update since api --- packages/api/src/resolvers/article/index.ts | 138 ++++++++++---------- packages/api/src/utils/gql-utils.ts | 29 ++-- 2 files changed, 86 insertions(+), 81 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index e4e34e912..037f3958e 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -605,7 +605,10 @@ export const searchResolver = authorized< return { errorCodes: [SearchErrorCode.QueryTooLong] } } - const isContentRequested = isFieldInSelectionSet(info, 'content') + const selectionSet = info.fieldNodes[0].selectionSet + const isContentRequested = selectionSet + ? isFieldInSelectionSet(selectionSet, 'content') + : false const searchLibraryItemArgs = { includePending: true, @@ -679,78 +682,69 @@ export const updatesSinceResolver = authorized< >, UpdatesSinceError, QueryUpdatesSinceArgs ->( - async ( - _obj, - { since, first, after, sort: sortParams, folder }, - { uid }, - info - ) => { - const startCursor = after || '' - const size = Math.min(first || 10, 100) // limit to 100 items - let startDate = new Date(since) - if (isNaN(startDate.getTime())) { - // for android app compatibility - startDate = new Date(0) - } - const sort = sortParamsToSort(sortParams) - - // create a search query - const query = `in:${ - folder || 'all' - } updated:${startDate.toISOString()} sort:${sort.by}-${sort.order}` - - const searchLibraryItemArgs = { - includeDeleted: true, - query, - includeContent: false, - } - - const libraryItems = await searchLibraryItems( - { - ...searchLibraryItemArgs, - from: Number(startCursor), - size: size + 1, // fetch one more item to get next cursor - useFolders: true, - }, - uid - ) - - const start = - startCursor && !isNaN(Number(startCursor)) ? Number(startCursor) : 0 - const hasNextPage = libraryItems.length > size - const endCursor = String( - start + libraryItems.length - (hasNextPage ? 1 : 0) - ) - - //TODO: refactor so that the lastCursor included - if (hasNextPage) { - // remove an extra if exists - libraryItems.pop() - } - - const edges = libraryItems.map((item) => { - const updateReason = getUpdateReason(item, startDate) - return { - node: item, - cursor: endCursor, - itemID: item.id, - updateReason, - } - }) - - return { - edges, - pageInfo: { - hasPreviousPage: false, - startCursor, - hasNextPage, - endCursor, - searchLibraryItemArgs, - }, - } +>(async (_obj, { since, first, after, sort: sortParams, folder }, { uid }) => { + const startCursor = after || '' + const size = Math.min(first || 10, 100) // limit to 100 items + let startDate = new Date(since) + if (isNaN(startDate.getTime())) { + // for android app compatibility + startDate = new Date(0) } -) + const sort = sortParamsToSort(sortParams) + + // create a search query + const query = `in:${ + folder || 'all' + } updated:${startDate.toISOString()} sort:${sort.by}-${sort.order}` + + const searchLibraryItemArgs = { + includeDeleted: true, + query, + includeContent: false, + } + + const libraryItems = await searchLibraryItems( + { + ...searchLibraryItemArgs, + from: Number(startCursor), + size: size + 1, // fetch one more item to get next cursor + useFolders: true, + }, + uid + ) + + const start = + startCursor && !isNaN(Number(startCursor)) ? Number(startCursor) : 0 + const hasNextPage = libraryItems.length > size + const endCursor = String(start + libraryItems.length - (hasNextPage ? 1 : 0)) + + //TODO: refactor so that the lastCursor included + if (hasNextPage) { + // remove an extra if exists + libraryItems.pop() + } + + const edges = libraryItems.map((item) => { + const updateReason = getUpdateReason(item, startDate) + return { + node: item, + cursor: endCursor, + itemID: item.id, + updateReason, + } + }) + + return { + edges, + pageInfo: { + hasPreviousPage: false, + startCursor, + hasNextPage, + endCursor, + searchLibraryItemArgs, + }, + } +}) export const bulkActionResolver = authorized< BulkActionSuccess, diff --git a/packages/api/src/utils/gql-utils.ts b/packages/api/src/utils/gql-utils.ts index 180fbabcf..3efed41d5 100644 --- a/packages/api/src/utils/gql-utils.ts +++ b/packages/api/src/utils/gql-utils.ts @@ -1,4 +1,4 @@ -import { GraphQLResolveInfo } from 'graphql' +import { SelectionSetNode } from 'graphql' import { ResolverFn } from '../generated/graphql' import { Claims, ResolverContext } from '../resolvers/types' @@ -27,13 +27,24 @@ export function authorized< } export const isFieldInSelectionSet = ( - info: GraphQLResolveInfo, + selectionSet: SelectionSetNode, fieldName: string -): boolean => { - return info.fieldNodes.some((node) => { - return node.selectionSet?.selections.some( - (selection) => - selection.kind === 'Field' && selection.name.value === fieldName - ) - }) +) => { + // recursively check if the field is in the selection set + for (const selection of selectionSet.selections) { + if (selection.kind === 'Field' && selection.name.value === fieldName) { + return true + } + + if ( + (selection.kind === 'InlineFragment' || selection.kind === 'Field') && + selection.selectionSet + ) { + if (isFieldInSelectionSet(selection.selectionSet, fieldName)) { + return true + } + } + } + + return false }