From 6f315ef4d92e1b9ee3eb7e3c61d032d022ca1689 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 16 Aug 2023 21:19:03 +0800 Subject: [PATCH 1/8] create a transaction for uploading files in the upload_files resolver --- .../api/src/resolvers/upload_files/index.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/api/src/resolvers/upload_files/index.ts b/packages/api/src/resolvers/upload_files/index.ts index 180cc96e3..b529d2412 100644 --- a/packages/api/src/resolvers/upload_files/index.ts +++ b/packages/api/src/resolvers/upload_files/index.ts @@ -40,7 +40,7 @@ export const uploadFileRequestResolver: ResolverFn< WithDataSourcesContext, MutationUploadFileRequestArgs > = async (_obj, { input }, ctx) => { - const { models, kx, claims, log } = ctx + const { models, authTrx, claims, log } = ctx let uploadFileData: { id: string | null } = { id: null, } @@ -98,8 +98,9 @@ export const uploadFileRequestResolver: ResolverFn< }) if (uploadFileData.id) { + const uploadFileId = uploadFileData.id const uploadFilePathName = generateUploadFilePathName( - uploadFileData.id, + uploadFileId, fileName ) const uploadSignedUrl = await generateUploadSignedUrl( @@ -111,9 +112,15 @@ export const uploadFileRequestResolver: ResolverFn< // If this is a file URL, we swap in the GCS public URL if (isFileUrl(input.url)) { - await models.uploadFile.update(uploadFileData.id, { - url: publicUrl, - status: UploadFileStatus.Initialized, + await authTrx(async (tx) => { + await models.uploadFile.update( + uploadFileId, + { + url: publicUrl, + status: UploadFileStatus.Initialized, + }, + tx + ) }) } From 060904395677b1009435bfab95140f9c9af34e41 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 17:59:24 +0800 Subject: [PATCH 2/8] reduce retry --- packages/api/package.json | 2 +- packages/api/src/routers/svc/integrations.ts | 13 +++--- packages/api/src/routers/svc/newsletters.ts | 14 +++---- packages/api/src/routers/svc/rss_feed.ts | 22 +++++----- packages/api/src/routers/svc/upload.ts | 10 ++--- packages/api/src/routers/svc/webhooks.ts | 33 +++++++-------- packages/api/src/utils/logger.ts | 11 +++-- packages/api/src/utils/uploads.ts | 2 +- yarn.lock | 42 ++++++++++++-------- 9 files changed, 83 insertions(+), 66 deletions(-) diff --git a/packages/api/package.json b/packages/api/package.json index 6db8e39cf..f6d274271 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -61,7 +61,7 @@ "express-rate-limit": "^6.3.0", "fast-safe-stringify": "^2.1.1", "firebase-admin": "^11.5.0", - "googleapis": "^105.0.0", + "googleapis": "^125.0.0", "graphql": "^15.3.0", "graphql-fields": "^2.0.3", "graphql-middleware": "^6.0.10", diff --git a/packages/api/src/routers/svc/integrations.ts b/packages/api/src/routers/svc/integrations.ts index bc5468981..bb09b9d8f 100644 --- a/packages/api/src/routers/svc/integrations.ts +++ b/packages/api/src/routers/svc/integrations.ts @@ -45,7 +45,7 @@ export function integrationsServiceRouter() { const { message: msgStr, expired } = readPushSubscription(req) if (!msgStr) { - return res.status(400).send('Bad Request') + return res.status(200).send('Bad Request') } if (expired) { @@ -58,7 +58,7 @@ export function integrationsServiceRouter() { const type = data.type if (!userId) { logger.info('No userId found in message') - res.status(400).send('Bad Request') + res.status(200).send('Bad Request') return } @@ -92,7 +92,7 @@ export function integrationsServiceRouter() { } if (!id) { logger.info('No id found in message') - res.status(400).send('Bad Request') + res.status(200).send('Bad Request') return } const page = await getPageById(id) @@ -159,13 +159,14 @@ export function integrationsServiceRouter() { res.status(200).send('Unknown action') return } - - res.status(200).send('OK') } catch (err) { logger.error('sync with integrations failed', err) - res.status(500).send(err) + return res.status(500).send(err) } + + res.status(200).send('OK') }) + // import pages from integration task handler router.post('/import', async (req, res) => { logger.info('start cloud task to import pages from integration') diff --git a/packages/api/src/routers/svc/newsletters.ts b/packages/api/src/routers/svc/newsletters.ts index 243fb7f6b..d39ea8280 100644 --- a/packages/api/src/routers/svc/newsletters.ts +++ b/packages/api/src/routers/svc/newsletters.ts @@ -93,7 +93,7 @@ export function newsletterServiceRouter() { try { const { message, expired } = readPushSubscription(req) if (!message) { - return res.status(400).send('Bad Request') + return res.status(200).send('Bad Request') } if (expired) { @@ -104,7 +104,7 @@ export function newsletterServiceRouter() { const data = JSON.parse(message) as unknown if (!isNewsletterMessage(data)) { logger.error('invalid newsletter message', { data }) - return res.status(400).send('Bad Request') + return res.status(200).send('Invalid Message') } // get user from newsletter email @@ -150,17 +150,17 @@ export function newsletterServiceRouter() { // update received email type await updateReceivedEmail(data.receivedEmailId, 'article') - - res.status(200).send('newsletter created') } catch (e) { logger.error(e) if (e instanceof SyntaxError) { // when message is not a valid json string - res.status(400).send(e) - } else { - res.status(500).send(e) + return res.status(400).send(e) } + + return res.status(500).send(e) } + + res.status(200).send('newsletter created') }) return router diff --git a/packages/api/src/routers/svc/rss_feed.ts b/packages/api/src/routers/svc/rss_feed.ts index 72834d774..c53ee726e 100644 --- a/packages/api/src/routers/svc/rss_feed.ts +++ b/packages/api/src/routers/svc/rss_feed.ts @@ -13,15 +13,15 @@ export function rssFeedRouter() { router.post('/fetchAll', async (req, res) => { logger.info('fetch all rss feeds') - const { message: msgStr, expired } = readPushSubscription(req) - logger.info('read pubsub message', msgStr, 'has expired', expired) - - if (expired) { - logger.info('discarding expired message') - return res.status(200).send('Expired') - } - try { + const { message: msgStr, expired } = readPushSubscription(req) + logger.info('read pubsub message', msgStr, 'has expired', expired) + + if (expired) { + logger.info('discarding expired message') + return res.status(200).send('Expired') + } + // get all active rss feed subscriptions const subscriptions = await getRepository(Subscription).find({ select: ['id', 'url', 'user', 'lastFetchedAt'], @@ -42,12 +42,12 @@ export function rssFeedRouter() { } }) ) - - res.send('OK') } catch (error) { logger.info('error fetching rss feeds', error) - res.status(500).send('Internal Server Error') + return res.status(500).send('Internal Server Error') } + + res.send('OK') }) return router diff --git a/packages/api/src/routers/svc/upload.ts b/packages/api/src/routers/svc/upload.ts index 1a42fba7d..9d3167832 100644 --- a/packages/api/src/routers/svc/upload.ts +++ b/packages/api/src/routers/svc/upload.ts @@ -17,7 +17,7 @@ export function uploadServiceRouter() { const { message: msgStr, expired } = readPushSubscription(req) if (!msgStr) { - return res.status(400).send('Bad Request') + return res.status(200).send('Bad Request') } if (expired) { @@ -28,7 +28,7 @@ export function uploadServiceRouter() { const data: { userId: string; type: string } = JSON.parse(msgStr) if (!data.userId || !data.type) { logger.info('No userId or type found in message') - return res.status(400).send('Bad Request') + return res.status(200).send('Bad Request') } const filePath = `${req.params.folder}/${data.type}/${ @@ -42,12 +42,12 @@ export function uploadServiceRouter() { { contentType: 'application/json' }, env.fileUpload.gcsUploadPrivateBucket ) - - res.status(200).send('OK') } catch (err) { logger.error('upload page data failed', err) - res.status(500).send(err) + return res.status(500).send(err) } + + res.status(200).send('OK') }) return router diff --git a/packages/api/src/routers/svc/webhooks.ts b/packages/api/src/routers/svc/webhooks.ts index dd596e74e..c2cdd26e0 100644 --- a/packages/api/src/routers/svc/webhooks.ts +++ b/packages/api/src/routers/svc/webhooks.ts @@ -13,25 +13,26 @@ export function webhooksServiceRouter() { router.post('/trigger/:action', async (req, res) => { logger.info('trigger webhook of action', req.params.action) - const { message: msgStr, expired } = readPushSubscription(req) - - if (!msgStr) { - res.status(400).send('Bad Request') - return - } - - if (expired) { - logger.info('discarding expired message') - res.status(200).send('Expired') - return - } try { + const { message: msgStr, expired } = readPushSubscription(req) + + if (!msgStr) { + res.status(200).send('Bad Request') + return + } + + if (expired) { + logger.info('discarding expired message') + res.status(200).send('Expired') + return + } + const data = JSON.parse(msgStr) const { userId, type } = data as { userId: string; type: string } if (!userId || !type) { logger.info('No userId or type found in message') - res.status(400).send('Bad Request') + res.status(200).send('Bad Request') return } @@ -90,12 +91,12 @@ export function webhooksServiceRouter() { }) }) ) - - res.status(200).send('OK') } catch (err) { logger.error('trigger webhook failed', err) - res.status(500).send(err) + return res.status(500).send(err) } + + res.send('OK') }) return router diff --git a/packages/api/src/utils/logger.ts b/packages/api/src/utils/logger.ts index 4e198cb21..e8e592e2a 100644 --- a/packages/api/src/utils/logger.ts +++ b/packages/api/src/utils/logger.ts @@ -61,10 +61,15 @@ const colors = { debug: 'underline gray', } +const MAX_LOG_SIZE = 250000 + const googleConfigs = { level: 'info', logName: 'logger', levels: config.syslog.levels, + maxEntrySize: MAX_LOG_SIZE, + useMessageField: false, + redirectToStdout: true, } function localConfig(id: string): ConsoleTransportOptions { @@ -100,7 +105,7 @@ const truncateObjectDeep = (object: any, length: number): any => { const truncateDeep = (obj: any, level: number): any => { // reach maximum call stack size if (level >= 5) { - return obj + return {} } if (isString(obj) && obj.length > length) { @@ -129,8 +134,8 @@ const truncateObjectDeep = (object: any, length: number): any => { class GcpLoggingTransport extends LoggingWinston { log(info: any, callback: (err: Error | null, apiResponse?: any) => void) { const sizeInfo = jsonStringify(info).length - if (sizeInfo > 250000) { - info = truncateObjectDeep(info, 5000) as never // the max length for string values is 5000 + if (sizeInfo > MAX_LOG_SIZE) { + info = truncateObjectDeep(info, 500) as never // the max length for string values is 500 } super.log(info, callback) } diff --git a/packages/api/src/utils/uploads.ts b/packages/api/src/utils/uploads.ts index f4b709fb0..7ba3f4613 100644 --- a/packages/api/src/utils/uploads.ts +++ b/packages/api/src/utils/uploads.ts @@ -129,7 +129,7 @@ export const uploadToBucket = async ( await storage .bucket(selectedBucket || bucketName) .file(filePath) - .save(data, options) + .save(data, { ...options, timeout: 30000 }) } export const createGCSFile = (filename: string): File => { diff --git a/yarn.lock b/yarn.lock index 36a39832d..312bcd169 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3174,9 +3174,9 @@ integrity sha512-mB9oAsNCm9aM3/SOv4YtBMqZbYj10R7dkq8byBqxGY/ncFwhf2oQzMV+LCRlWoDSEBJ3COiR1yeDvMtsoOsuFQ== "@grpc/grpc-js@^1.1.8", "@grpc/grpc-js@~1.8.0": - version "1.8.11" - resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.8.11.tgz#f113f7bc197e8d6f3d3f0c6b02925c7a5da1aec4" - integrity sha512-f/xC+6Z2QKsRJ+VSSFlt4hA5KSRm+PKvMWV8kMPkMgGlFidR6PeIkXrOasIY2roe+WROM6GFQLlgDKfeEZo2YQ== + version "1.8.21" + resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.8.21.tgz#d282b122c71227859bf6c5866f4c40f4a2696513" + integrity sha512-KeyQeZpxeEBSqFVTi3q2K7PiPXmgBfECc4updA1ejCLjYmoAlvvM3ZMp5ztTDUCUQmoY3CpDxvchjO1+rFkoHg== dependencies: "@grpc/proto-loader" "^0.7.0" "@types/node" ">=12.12.47" @@ -15393,6 +15393,16 @@ gaxios@^6.0.0, gaxios@^6.0.2: is-stream "^2.0.0" node-fetch "^2.6.9" +gaxios@^6.0.3: + version "6.1.0" + resolved "https://registry.yarnpkg.com/gaxios/-/gaxios-6.1.0.tgz#8ab08adbf9cc600368a57545f58e004ccf831ccb" + integrity sha512-EIHuesZxNyIkUGcTQKQPMICyOpDD/bi+LJIJx+NLsSGmnS7N+xCLRX5bi4e9yAu9AlSZdVq+qlyWWVuTh/483w== + dependencies: + extend "^3.0.2" + https-proxy-agent "^7.0.1" + is-stream "^2.0.0" + node-fetch "^2.6.9" + gcp-metadata@^4.2.0: version "4.3.0" resolved "https://registry.yarnpkg.com/gcp-metadata/-/gcp-metadata-4.3.0.tgz#0423d06becdbfb9cbb8762eaacf14d5324997900" @@ -15926,25 +15936,25 @@ google-proto-files@^3.0.0: protobufjs "^7.0.0" walkdir "^0.4.0" -googleapis-common@^6.0.0: - version "6.0.0" - resolved "https://registry.yarnpkg.com/googleapis-common/-/googleapis-common-6.0.0.tgz#cd3f811f18dbc82191530aedaafa6224333a758e" - integrity sha512-ieZouiuoyTHCckOgu7NU+n5UvA8kAzGTRRMQD+3bobCX9npnRvYDmTHZjM5lQLzf0cGz1xQ1ABCxGQ+xSsMkCw== +googleapis-common@^7.0.0: + version "7.0.0" + resolved "https://registry.yarnpkg.com/googleapis-common/-/googleapis-common-7.0.0.tgz#a7b5262e320c922c25b123edea2a3958f15c3edd" + integrity sha512-58iSybJPQZ8XZNMpjrklICefuOuyJ0lMxfKmBqmaC0/xGT4SiOs4BE60LAOOGtBURy1n8fHa2X2YUNFEWWbXyQ== dependencies: extend "^3.0.2" - gaxios "^4.0.0" - google-auth-library "^8.0.2" + gaxios "^6.0.3" + google-auth-library "^9.0.0" qs "^6.7.0" url-template "^2.0.8" - uuid "^8.0.0" + uuid "^9.0.0" -googleapis@^105.0.0: - version "105.0.0" - resolved "https://registry.yarnpkg.com/googleapis/-/googleapis-105.0.0.tgz#65ff8969cf837a08ee127ae56468ac3d96ddc9d5" - integrity sha512-wH/jU/6QpqwsjTKj4vfKZz97ne7xT7BBbKwzQEwnbsG8iH9Seyw19P+AuLJcxNNrmgblwLqfr3LORg4Okat1BQ== +googleapis@^125.0.0: + version "125.0.0" + resolved "https://registry.yarnpkg.com/googleapis/-/googleapis-125.0.0.tgz#d084565e567081dceaf5aade0c4f5122ae53e083" + integrity sha512-KsMe3gdbiI6bj4M+Zuwcl7xL0Koz8m0kaq0XQj99YT/4zHsZdaLJqGmYMDyWI4SAScVqkW7TvQftzL7L74x1uQ== dependencies: - google-auth-library "^8.0.2" - googleapis-common "^6.0.0" + google-auth-library "^9.0.0" + googleapis-common "^7.0.0" got@^9.6.0: version "9.6.0" From ec88a9fe8e9ca744bb25fb54f330f6ebfc73ca11 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 18:05:51 +0800 Subject: [PATCH 3/8] we don't want to create thumbnail for imported pages --- packages/api/src/services/save_page.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index fa01dff9c..2790ae311 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -186,11 +186,14 @@ export const savePage = async ( } // create a task to update thumbnail and pre-cache all images - try { - const taskId = await enqueueThumbnailTask(saver.userId, slug) - logger.info('Created thumbnail task', { taskId }) - } catch (e) { - logger.error('Failed to create thumbnail task', e) + if (input.source !== 'csv-importer') { + // we don't want to create thumbnail for imported pages + try { + const taskId = await enqueueThumbnailTask(saver.userId, slug) + logger.info('Created thumbnail task', { taskId }) + } catch (e) { + logger.error('Failed to create thumbnail task', e) + } } if (parseResult.highlightData) { From dedd76cfb755f7cbbde61b034643179d69c2d801 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 18:24:57 +0800 Subject: [PATCH 4/8] do not publish a pubsub event if the page is imported --- packages/api/src/elastic/highlights.ts | 8 ++++---- packages/api/src/elastic/labels.ts | 12 ++++++------ packages/api/src/elastic/pages.ts | 18 +++++++++++------- packages/api/src/elastic/recommendation.ts | 4 ++-- packages/api/src/elastic/types.ts | 3 ++- packages/api/src/services/labels.ts | 6 +++--- packages/api/src/services/popular_reads.ts | 4 ++-- packages/api/src/services/save_page.ts | 7 ++++++- 8 files changed, 36 insertions(+), 26 deletions(-) diff --git a/packages/api/src/elastic/highlights.ts b/packages/api/src/elastic/highlights.ts index 3d0808c04..c5b516118 100644 --- a/packages/api/src/elastic/highlights.ts +++ b/packages/api/src/elastic/highlights.ts @@ -3,9 +3,9 @@ import { EntityType } from '../datalayer/pubsub' import { SortBy, SortOrder, SortParams } from '../utils/search' import { client, INDEX_ALIAS, logger } from './index' import { + Context, Highlight, Page, - PageContext, PageType, SearchItem, SearchResponse, @@ -14,7 +14,7 @@ import { export const addHighlightToPage = async ( id: string, highlight: Highlight, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.update({ @@ -97,7 +97,7 @@ export const getHighlightById = async ( export const deleteHighlight = async ( highlightId: string, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.updateByQuery({ @@ -256,7 +256,7 @@ export const searchHighlights = async ( export const updateHighlight = async ( highlight: Highlight, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.updateByQuery({ diff --git a/packages/api/src/elastic/labels.ts b/packages/api/src/elastic/labels.ts index 5864a0dff..a6364ce6d 100644 --- a/packages/api/src/elastic/labels.ts +++ b/packages/api/src/elastic/labels.ts @@ -1,12 +1,12 @@ import { errors } from '@elastic/elasticsearch' import { EntityType } from '../datalayer/pubsub' import { client, INDEX_ALIAS, logger } from './index' -import { Label, PageContext } from './types' +import { Context, Label } from './types' export const addLabelInPage = async ( pageId: string, label: Label, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.update({ @@ -57,7 +57,7 @@ export const addLabelInPage = async ( export const updateLabelsInPage = async ( pageId: string, labels: Label[], - ctx: PageContext, + ctx: Context, labelsToAdd?: Label[] ): Promise => { try { @@ -105,7 +105,7 @@ export const updateLabelsInPage = async ( export const deleteLabel = async ( label: string, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.updateByQuery({ @@ -181,7 +181,7 @@ export const deleteLabel = async ( export const updateLabel = async ( label: Label, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.updateByQuery({ @@ -273,7 +273,7 @@ export const updateLabel = async ( export const setLabelsForHighlight = async ( highlightId: string, labels: Label[], - ctx: PageContext, + ctx: Context, labelsToAdd?: Label[] ): Promise => { try { diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index 0545b1093..d1fba284a 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -18,9 +18,9 @@ import { import { client, INDEX_ALIAS, logger } from './index' import { ArticleSavingRequestStatus, + Context, Label, Page, - PageContext, PageSearchArgs, PageType, ParamSet, @@ -404,7 +404,7 @@ const appendSiteNameFilter = ( export const createPage = async ( page: Page, - ctx: PageContext + ctx: Context ): Promise => { try { if (page.content.length > MAX_CONTENT_LENGTH) { @@ -429,7 +429,11 @@ export const createPage = async ( }) page.id = body._id as string - await ctx.pubsub.entityCreated(EntityType.PAGE, page, ctx.uid) + + // only publish a pubsub event if we should + if (ctx.shouldPublish) { + await ctx.pubsub?.entityCreated(EntityType.PAGE, page, ctx.uid) + } return page.id } catch (e) { @@ -441,7 +445,7 @@ export const createPage = async ( export const updatePage = async ( id: string, page: Partial, - ctx: PageContext + ctx: Context ): Promise => { try { if (page.content && page.content.length > MAX_CONTENT_LENGTH) { @@ -492,7 +496,7 @@ export const updatePage = async ( export const deletePage = async ( id: string, - ctx: PageContext + ctx: Context ): Promise => { try { const { body } = await client.delete({ @@ -755,7 +759,7 @@ export const countByCreatedAt = async ( export const deletePagesByParam = async ( param: Record, - ctx: PageContext + ctx: Context ): Promise => { try { const params = { @@ -852,7 +856,7 @@ export const searchAsYouType = async ( } export const updatePages = async ( - ctx: PageContext, + ctx: Context, action: BulkActionType, args: PageSearchArgs, maxDocs: number, diff --git a/packages/api/src/elastic/recommendation.ts b/packages/api/src/elastic/recommendation.ts index 3caa544e6..bb8a6c4c4 100644 --- a/packages/api/src/elastic/recommendation.ts +++ b/packages/api/src/elastic/recommendation.ts @@ -2,13 +2,13 @@ import { logger } from '.' import { createPage, getPageByParam, updatePage } from './pages' import { ArticleSavingRequestStatus, + Context, Page, - PageContext, Recommendation, } from './types' export const addRecommendation = async ( - ctx: PageContext, + ctx: Context, page: Page, recommendation: Recommendation, highlightIds?: string[] diff --git a/packages/api/src/elastic/types.ts b/packages/api/src/elastic/types.ts index b6e5bf41b..f3c32e87e 100644 --- a/packages/api/src/elastic/types.ts +++ b/packages/api/src/elastic/types.ts @@ -204,10 +204,11 @@ const keys = ['_id', 'url', 'slug', 'userId', 'uploadFileId', 'state'] as const export type ParamSet = PickTuple -export interface PageContext { +export interface Context { pubsub: PubsubClient refresh?: boolean uid: string + shouldPublish?: boolean } export interface PageSearchArgs { diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index 5837ddf72..5ed059029 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -1,7 +1,7 @@ import DataLoader from 'dataloader' import { In } from 'typeorm' import { addLabelInPage } from '../elastic/labels' -import { PageContext } from '../elastic/types' +import { Context } from '../elastic/types' import { Label } from '../entity/label' import { Link } from '../entity/link' import { User } from '../entity/user' @@ -37,7 +37,7 @@ const batchGetLabelsFromLinkIds = async ( export const labelsLoader = new DataLoader(batchGetLabelsFromLinkIds) export const addLabelToPage = async ( - ctx: PageContext, + ctx: Context, pageId: string, label: { name: string @@ -114,7 +114,7 @@ export const createLabel = async ( } export const createLabels = async ( - ctx: PageContext, + ctx: Context, labels: CreateLabelInput[] ): Promise => { const user = await getRepository(User).findOneBy({ diff --git a/packages/api/src/services/popular_reads.ts b/packages/api/src/services/popular_reads.ts index 7f79406d5..a536a6848 100644 --- a/packages/api/src/services/popular_reads.ts +++ b/packages/api/src/services/popular_reads.ts @@ -3,7 +3,7 @@ import { readFileSync } from 'fs' import path from 'path' import { createPubSubClient } from '../datalayer/pubsub' import { createPage } from '../elastic/pages' -import { ArticleSavingRequestStatus, Page, PageContext } from '../elastic/types' +import { ArticleSavingRequestStatus, Context, Page } from '../elastic/types' import { PageType } from '../generated/graphql' import { generateSlug, stringToHash } from '../utils/helpers' import { logger } from '../utils/logger' @@ -61,7 +61,7 @@ export const addPopularRead = async ( userId: string, name: string ): Promise => { - const ctx: PageContext = { + const ctx: Context = { pubsub: createPubSubClient(), refresh: true, uid: userId, diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index 2790ae311..fcac553fc 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -174,7 +174,12 @@ export const savePage = async ( } } } else { - const newPageId = await createPage(articleToSave, ctx) + // do not publish a pubsub event if the page is imported + const shouldPublish = input.source !== 'csv-importer' + const newPageId = await createPage(articleToSave, { + ...ctx, + shouldPublish, + }) if (!newPageId) { return { errorCodes: [SaveErrorCode.Unknown], From e54e61f3c031170a56ffe06a2f18ef83ef3ec934 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 18:33:14 +0800 Subject: [PATCH 5/8] reduce log size --- packages/api/src/utils/logger.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/utils/logger.ts b/packages/api/src/utils/logger.ts index e8e592e2a..99daf5a7b 100644 --- a/packages/api/src/utils/logger.ts +++ b/packages/api/src/utils/logger.ts @@ -105,7 +105,7 @@ const truncateObjectDeep = (object: any, length: number): any => { const truncateDeep = (obj: any, level: number): any => { // reach maximum call stack size if (level >= 5) { - return {} + return undefined } if (isString(obj) && obj.length > length) { From c45b760f714d02f41fe2002a63c665205c208bc4 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 18:42:11 +0800 Subject: [PATCH 6/8] fix test --- packages/api/src/elastic/pages.ts | 3 ++- packages/api/src/services/save_page.ts | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index d1fba284a..de10b4432 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -430,8 +430,9 @@ export const createPage = async ( page.id = body._id as string + const shouldPublish = ctx.shouldPublish ?? true // only publish a pubsub event if we should - if (ctx.shouldPublish) { + if (shouldPublish) { await ctx.pubsub?.entityCreated(EntityType.PAGE, page, ctx.uid) } diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index fcac553fc..2acaa6755 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -118,6 +118,8 @@ export const savePage = async ( ? await createLabels(ctx, input.labels) : undefined + const isImported = input.source === 'csv-importer' + // always parse in backend if the url is in the force puppeteer list if (shouldParseInBackend(input)) { try { @@ -175,10 +177,9 @@ export const savePage = async ( } } else { // do not publish a pubsub event if the page is imported - const shouldPublish = input.source !== 'csv-importer' const newPageId = await createPage(articleToSave, { ...ctx, - shouldPublish, + shouldPublish: !isImported, }) if (!newPageId) { return { @@ -190,10 +191,10 @@ export const savePage = async ( } } - // create a task to update thumbnail and pre-cache all images - if (input.source !== 'csv-importer') { - // we don't want to create thumbnail for imported pages + // we don't want to create thumbnail for imported pages + if (!isImported) { try { + // create a task to update thumbnail and pre-cache all images const taskId = await enqueueThumbnailTask(saver.userId, slug) logger.info('Created thumbnail task', { taskId }) } catch (e) { From cd1fc5c4db361985746b5d0fd0c4588ccc13b9f9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 18:46:39 +0800 Subject: [PATCH 7/8] fix typo --- packages/api/src/elastic/highlights.ts | 8 ++++---- packages/api/src/elastic/labels.ts | 12 ++++++------ packages/api/src/elastic/pages.ts | 12 ++++++------ packages/api/src/elastic/recommendation.ts | 4 ++-- packages/api/src/elastic/types.ts | 2 +- packages/api/src/services/labels.ts | 6 +++--- packages/api/src/services/popular_reads.ts | 4 ++-- 7 files changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/api/src/elastic/highlights.ts b/packages/api/src/elastic/highlights.ts index c5b516118..3d0808c04 100644 --- a/packages/api/src/elastic/highlights.ts +++ b/packages/api/src/elastic/highlights.ts @@ -3,9 +3,9 @@ import { EntityType } from '../datalayer/pubsub' import { SortBy, SortOrder, SortParams } from '../utils/search' import { client, INDEX_ALIAS, logger } from './index' import { - Context, Highlight, Page, + PageContext, PageType, SearchItem, SearchResponse, @@ -14,7 +14,7 @@ import { export const addHighlightToPage = async ( id: string, highlight: Highlight, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.update({ @@ -97,7 +97,7 @@ export const getHighlightById = async ( export const deleteHighlight = async ( highlightId: string, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.updateByQuery({ @@ -256,7 +256,7 @@ export const searchHighlights = async ( export const updateHighlight = async ( highlight: Highlight, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.updateByQuery({ diff --git a/packages/api/src/elastic/labels.ts b/packages/api/src/elastic/labels.ts index a6364ce6d..5864a0dff 100644 --- a/packages/api/src/elastic/labels.ts +++ b/packages/api/src/elastic/labels.ts @@ -1,12 +1,12 @@ import { errors } from '@elastic/elasticsearch' import { EntityType } from '../datalayer/pubsub' import { client, INDEX_ALIAS, logger } from './index' -import { Context, Label } from './types' +import { Label, PageContext } from './types' export const addLabelInPage = async ( pageId: string, label: Label, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.update({ @@ -57,7 +57,7 @@ export const addLabelInPage = async ( export const updateLabelsInPage = async ( pageId: string, labels: Label[], - ctx: Context, + ctx: PageContext, labelsToAdd?: Label[] ): Promise => { try { @@ -105,7 +105,7 @@ export const updateLabelsInPage = async ( export const deleteLabel = async ( label: string, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.updateByQuery({ @@ -181,7 +181,7 @@ export const deleteLabel = async ( export const updateLabel = async ( label: Label, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.updateByQuery({ @@ -273,7 +273,7 @@ export const updateLabel = async ( export const setLabelsForHighlight = async ( highlightId: string, labels: Label[], - ctx: Context, + ctx: PageContext, labelsToAdd?: Label[] ): Promise => { try { diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index de10b4432..bd6b59e32 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -18,9 +18,9 @@ import { import { client, INDEX_ALIAS, logger } from './index' import { ArticleSavingRequestStatus, - Context, Label, Page, + PageContext, PageSearchArgs, PageType, ParamSet, @@ -404,7 +404,7 @@ const appendSiteNameFilter = ( export const createPage = async ( page: Page, - ctx: Context + ctx: PageContext ): Promise => { try { if (page.content.length > MAX_CONTENT_LENGTH) { @@ -446,7 +446,7 @@ export const createPage = async ( export const updatePage = async ( id: string, page: Partial, - ctx: Context + ctx: PageContext ): Promise => { try { if (page.content && page.content.length > MAX_CONTENT_LENGTH) { @@ -497,7 +497,7 @@ export const updatePage = async ( export const deletePage = async ( id: string, - ctx: Context + ctx: PageContext ): Promise => { try { const { body } = await client.delete({ @@ -760,7 +760,7 @@ export const countByCreatedAt = async ( export const deletePagesByParam = async ( param: Record, - ctx: Context + ctx: PageContext ): Promise => { try { const params = { @@ -857,7 +857,7 @@ export const searchAsYouType = async ( } export const updatePages = async ( - ctx: Context, + ctx: PageContext, action: BulkActionType, args: PageSearchArgs, maxDocs: number, diff --git a/packages/api/src/elastic/recommendation.ts b/packages/api/src/elastic/recommendation.ts index bb8a6c4c4..3caa544e6 100644 --- a/packages/api/src/elastic/recommendation.ts +++ b/packages/api/src/elastic/recommendation.ts @@ -2,13 +2,13 @@ import { logger } from '.' import { createPage, getPageByParam, updatePage } from './pages' import { ArticleSavingRequestStatus, - Context, Page, + PageContext, Recommendation, } from './types' export const addRecommendation = async ( - ctx: Context, + ctx: PageContext, page: Page, recommendation: Recommendation, highlightIds?: string[] diff --git a/packages/api/src/elastic/types.ts b/packages/api/src/elastic/types.ts index f3c32e87e..1dab0a69f 100644 --- a/packages/api/src/elastic/types.ts +++ b/packages/api/src/elastic/types.ts @@ -204,7 +204,7 @@ const keys = ['_id', 'url', 'slug', 'userId', 'uploadFileId', 'state'] as const export type ParamSet = PickTuple -export interface Context { +export interface PageContext { pubsub: PubsubClient refresh?: boolean uid: string diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index 5ed059029..5837ddf72 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -1,7 +1,7 @@ import DataLoader from 'dataloader' import { In } from 'typeorm' import { addLabelInPage } from '../elastic/labels' -import { Context } from '../elastic/types' +import { PageContext } from '../elastic/types' import { Label } from '../entity/label' import { Link } from '../entity/link' import { User } from '../entity/user' @@ -37,7 +37,7 @@ const batchGetLabelsFromLinkIds = async ( export const labelsLoader = new DataLoader(batchGetLabelsFromLinkIds) export const addLabelToPage = async ( - ctx: Context, + ctx: PageContext, pageId: string, label: { name: string @@ -114,7 +114,7 @@ export const createLabel = async ( } export const createLabels = async ( - ctx: Context, + ctx: PageContext, labels: CreateLabelInput[] ): Promise => { const user = await getRepository(User).findOneBy({ diff --git a/packages/api/src/services/popular_reads.ts b/packages/api/src/services/popular_reads.ts index a536a6848..7f79406d5 100644 --- a/packages/api/src/services/popular_reads.ts +++ b/packages/api/src/services/popular_reads.ts @@ -3,7 +3,7 @@ import { readFileSync } from 'fs' import path from 'path' import { createPubSubClient } from '../datalayer/pubsub' import { createPage } from '../elastic/pages' -import { ArticleSavingRequestStatus, Context, Page } from '../elastic/types' +import { ArticleSavingRequestStatus, Page, PageContext } from '../elastic/types' import { PageType } from '../generated/graphql' import { generateSlug, stringToHash } from '../utils/helpers' import { logger } from '../utils/logger' @@ -61,7 +61,7 @@ export const addPopularRead = async ( userId: string, name: string ): Promise => { - const ctx: Context = { + const ctx: PageContext = { pubsub: createPubSubClient(), refresh: true, uid: userId, From 2a019166aa8f04bb35ac000538c97d042ef8bd05 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 17 Aug 2023 18:55:29 +0800 Subject: [PATCH 8/8] fix test --- packages/api/test/routers/integrations.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/api/test/routers/integrations.test.ts b/packages/api/test/routers/integrations.test.ts index cef2d1116..f05a7e669 100644 --- a/packages/api/test/routers/integrations.test.ts +++ b/packages/api/test/routers/integrations.test.ts @@ -58,8 +58,8 @@ describe('Integrations routers', () => { token = 'invalid-token' }) - it('returns 400', async () => { - return request.post(endpoint(token)).send(data).expect(400) + it('returns 200', async () => { + return request.post(endpoint(token)).send(data).expect(200) }) }) @@ -98,8 +98,8 @@ describe('Integrations routers', () => { } }) - it('returns 400', async () => { - return request.post(endpoint(token)).send(data).expect(400) + it('returns 200', async () => { + return request.post(endpoint(token)).send(data).expect(200) }) })