diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 4e4eeb374..417d83146 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 normalizeUrl from 'normalize-url' import { searchHighlights } from '../../elastic/highlights' import { createPage, @@ -83,7 +82,7 @@ import { createLabels, getLabelsByIds, } from '../../services/labels' -import { parsedContentToPage } from '../../services/save_page' +import { cleanUrl, parsedContentToPage } from '../../services/save_page' import { traceAs } from '../../tracing' import { Merge } from '../../util' import { analytics } from '../../utils/analytics' @@ -228,10 +227,7 @@ export const createArticleResolver = authorized< pageType: PageType.Unknown, contentReader: ContentReader.Web, author: '', - url: normalizeUrl(canonicalUrl || url, { - stripHash: true, - stripWWW: false, - }), + url: cleanUrl(canonicalUrl || url), hash: '', isArchived: false, }, diff --git a/packages/api/src/resolvers/article_saving_request/index.ts b/packages/api/src/resolvers/article_saving_request/index.ts index 9f6e40125..197f970dc 100644 --- a/packages/api/src/resolvers/article_saving_request/index.ts +++ b/packages/api/src/resolvers/article_saving_request/index.ts @@ -1,5 +1,4 @@ /* eslint-disable prefer-const */ -import normalizeUrl from 'normalize-url' import { getPageByParam } from '../../elastic/pages' import { User } from '../../entity/user' import { getRepository } from '../../entity/utils' @@ -16,6 +15,7 @@ import { QueryArticleSavingRequestArgs, } from '../../generated/graphql' import { createPageSaveRequest } from '../../services/create_page_save_request' +import { cleanUrl } from '../../services/save_page' import { analytics } from '../../utils/analytics' import { authorized, @@ -75,12 +75,7 @@ export const articleSavingRequestResolver = authorized< return { errorCodes: [ArticleSavingRequestErrorCode.Unauthorized] } } - const normalizedUrl = url - ? normalizeUrl(url, { - stripHash: true, - stripWWW: false, - }) - : undefined + const normalizedUrl = url ? cleanUrl(url) : undefined const params = { _id: id || undefined, diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index 2734c50e1..20f067f52 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -1,4 +1,3 @@ -import normalizeUrl from 'normalize-url' import * as privateIpLib from 'private-ip' import { v4 as uuidv4 } from 'uuid' import { createPubSubClient, PubsubClient } from '../datalayer/pubsub' @@ -17,6 +16,7 @@ import { } from '../generated/graphql' import { enqueueParseRequest } from '../utils/createTask' import { generateSlug, pageToArticleSavingRequest } from '../utils/helpers' +import { cleanUrl } from './save_page' interface PageSaveRequest { userId: string @@ -104,10 +104,7 @@ export const createPageSaveRequest = async ({ priority = priority || (await getPriorityByRateLimit(userId)) // look for existing page - const normalizedUrl = normalizeUrl(url, { - stripHash: true, - stripWWW: false, - }) + const normalizedUrl = cleanUrl(url) const ctx = { pubsub, @@ -164,7 +161,7 @@ export const createPageSaveRequest = async ({ })) // enqueue task to parse page await enqueueParseRequest({ - url, + url: normalizedUrl, userId, saveRequestId: page.id, priority, diff --git a/packages/api/src/services/save_email.ts b/packages/api/src/services/save_email.ts index 7e49494b0..04999329a 100644 --- a/packages/api/src/services/save_email.ts +++ b/packages/api/src/services/save_email.ts @@ -1,4 +1,3 @@ -import normalizeUrl from 'normalize-url' import { PubsubClient } from '../datalayer/pubsub' import { createPage, getPageByParam, updatePage } from '../elastic/pages' import { ArticleSavingRequestStatus, Page } from '../elastic/types' @@ -14,6 +13,7 @@ import { parsePreparedContent, parseUrlMetadata, } from '../utils/parser' +import { cleanUrl } from './save_page' export type SaveContext = { pubsub: PubsubClient @@ -63,10 +63,7 @@ export const saveEmail = async ( description: metadata?.description || parseResult.parsedContent?.excerpt, title: input.title, author: input.author, - url: normalizeUrl(parseResult.canonicalUrl || url, { - stripHash: true, - stripWWW: false, - }), + url: cleanUrl(parseResult.canonicalUrl || url), pageType: parseResult.pageType, hash: stringToHash(content), image: diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index c58ded776..a65a48d5e 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -36,13 +36,30 @@ type SaverUserData = { username: string } +const TWEET_URL_REGEX = + /twitter\.com\/(?:#!\/)?(\w+)\/status(?:es)?\/(\d+)(?:\/.*)?/ + // where we can use APIs to fetch their underlying content. const FORCE_PUPPETEER_URLS = [ - // twitter status url regex - /twitter\.com\/(?:#!\/)?(\w+)\/status(?:es)?\/(\d+)(?:\/.*)?/, + TWEET_URL_REGEX, /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w-]+\?v=|embed\/|v\/)?)([\w-]+)(\S+)?$/, ] +export const cleanUrl = (url: string) => { + const trackingParams: (RegExp | string)[] = [/^utm_\w+/i] // remove utm tracking parameters + if (TWEET_URL_REGEX.test(url)) { + console.debug('cleaning tweet url', url) + // remove tracking parameters from tweet links: + // https://twitter.com/omnivore/status/1673218959624093698?s=12&t=R91quPajs0E53Yds-fhv2g + trackingParams.push('s', 't') + } + return normalizeUrl(url, { + stripHash: true, + stripWWW: false, + removeQueryParameters: trackingParams, + }) +} + const createSlug = (url: string, title?: Maybe | undefined) => { const { pathname } = new URL(url) const croppedPathname = decodeURIComponent( @@ -104,11 +121,7 @@ export const savePage = async ( originalHtml: parseResult.domContent, canonicalUrl: parseResult.canonicalUrl, }) - // check if the page already exists - const existingPage = await getPageByParam({ - userId: saver.userId, - url: articleToSave.url, - }) + // save state articleToSave.archivedAt = input.state === ArticleSavingRequestStatus.Archived ? new Date() : null @@ -117,28 +130,8 @@ export const savePage = async ( ? await createLabels(ctx, input.labels) : undefined - if (existingPage) { - pageId = existingPage.id - slug = existingPage.slug - if ( - !(await updatePage( - existingPage.id, - { - // update the page with the new content - ...articleToSave, - id: pageId, // we don't want to update the id - slug, // we don't want to update the slug - createdAt: existingPage.createdAt, // we don't want to update the createdAt - }, - ctx - )) - ) { - return { - errorCodes: [SaveErrorCode.Unknown], - message: 'Failed to update existing page', - } - } - } else if (shouldParseInBackend(input)) { + // always parse in backend if the url is in the force puppeteer list + if (shouldParseInBackend(input)) { try { await createPageSaveRequest({ userId: saver.userId, @@ -155,14 +148,42 @@ export const savePage = async ( } } } else { - const newPageId = await createPage(articleToSave, ctx) - if (!newPageId) { - return { - errorCodes: [SaveErrorCode.Unknown], - message: 'Failed to create new page', + // check if the page already exists + const existingPage = await getPageByParam({ + userId: saver.userId, + url: articleToSave.url, + }) + if (existingPage) { + pageId = existingPage.id + slug = existingPage.slug + if ( + !(await updatePage( + existingPage.id, + { + // update the page with the new content + ...articleToSave, + id: pageId, // we don't want to update the id + slug, // we don't want to update the slug + createdAt: existingPage.createdAt, // we don't want to update the createdAt + }, + ctx + )) + ) { + return { + errorCodes: [SaveErrorCode.Unknown], + message: 'Failed to update existing page', + } } + } else { + const newPageId = await createPage(articleToSave, ctx) + if (!newPageId) { + return { + errorCodes: [SaveErrorCode.Unknown], + message: 'Failed to create new page', + } + } + pageId = newPageId } - pageId = newPageId } // create a task to update thumbnail and pre-cache all images @@ -248,10 +269,7 @@ export const parsedContentToPage = ({ parsedContent?.siteName || url, author: parsedContent?.byline ?? undefined, - url: normalizeUrl(canonicalUrl || url, { - stripHash: true, - stripWWW: false, - }), + url: cleanUrl(canonicalUrl || url), pageType, hash: uploadFileHash || stringToHash(parsedContent?.content || url), image: parsedContent?.previewImage ?? undefined,