Merge pull request #3440 from omnivore-app/fix/cache-ttl-

fix/cache ttl

packages/api/package.json

@@ -71,6 +71,7 @@
     "graphql-shield": "^7.5.0",
     "highlightjs": "^9.16.2",
     "html-entities": "^2.3.2",
+    "image-size": "^1.0.2",
     "intercom-client": "^3.1.4",
     "ioredis": "^5.3.2",
     "jsonwebtoken": "^8.5.1",

packages/api/src/jobs/find_thumbnail.ts

@@ -0,0 +1,176 @@
+import axios, { AxiosResponse } from 'axios'
+import sizeOf from 'image-size'
+import { parseHTML } from 'linkedom'
+import {
+  findLibraryItemById,
+  updateLibraryItem,
+} from '../services/library_item'
+import { createImageProxyUrl, createThumbnailUrl } from '../utils/imageproxy'
+import { logger } from '../utils/logger'
+
+interface Data {
+  libraryItemId: string
+  userId: string
+}
+
+interface ImageSize {
+  src: string
+  width: number
+  height: number
+}
+
+export const THUMBNAIL_JOB = 'find-thumbnail'
+
+const fetchImage = async (url: string): Promise<AxiosResponse | null> => {
+  console.log('fetching image', url)
+  try {
+    // get image file by url
+    return await axios.get(url, {
+      responseType: 'arraybuffer',
+      timeout: 10000, // 10s
+      maxContentLength: 20000000, // 20mb
+    })
+  } catch (e) {
+    logger.error('fetch image error', e)
+    return null
+  }
+}
+
+const getImageSize = async (src: string): Promise<ImageSize | null> => {
+  try {
+    const response = await fetchImage(src)
+    if (!response) {
+      return null
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
+    const buffer = Buffer.from(response.data, 'binary')
+
+    // get image size
+    const { width, height } = sizeOf(buffer)
+
+    if (!width || !height) {
+      return null
+    }
+
+    return {
+      src,
+      width,
+      height,
+    }
+  } catch (e) {
+    logger.error(e)
+    return null
+  }
+}
+
+export const fetchAllImageSizes = async (content: string) => {
+  const dom = parseHTML(content).document
+
+  // fetch all images by src and get their sizes
+  const images = dom.querySelectorAll('img[src]')
+  if (!images || images.length === 0) {
+    console.log('no images')
+    return []
+  }
+
+  return Promise.all(
+    Array.from(images).map((image) => {
+      const src = image.getAttribute('src')
+      if (!src) {
+        return null
+      }
+
+      return getImageSize(src)
+    })
+  )
+}
+
+// credit: https://github.com/reddit-archive/reddit/blob/753b17407e9a9dca09558526805922de24133d53/r2/r2/lib/media.py#L706
+export const _findThumbnail = (imagesSizes: (ImageSize | null)[]) => {
+  // find the largest and squarest image as the thumbnail
+  let thumbnail = ''
+  let largestArea = 0
+  for (const imageSize of Array.from(imagesSizes)) {
+    if (!imageSize) {
+      continue
+    }
+
+    let area = imageSize.width * imageSize.height
+
+    // ignore small images
+    if (area < 5000) {
+      logger.info('ignore small', { src: imageSize.src })
+      continue
+    }
+
+    // penalize excessively long/wide images
+    const ratio =
+      Math.max(imageSize.width, imageSize.height) /
+      Math.min(imageSize.width, imageSize.height)
+    if (ratio > 1.5) {
+      logger.info('penalizing long/wide', { src: imageSize.src })
+      area /= ratio * 2
+    }
+
+    // penalize images with "sprite" in their name
+    if (imageSize.src.toLowerCase().includes('sprite')) {
+      logger.info('penalizing sprite', { src: imageSize.src })
+      area /= 10
+    }
+
+    if (area > largestArea) {
+      largestArea = area
+      thumbnail = imageSize.src
+    }
+  }
+
+  return thumbnail
+}
+
+export const findThumbnail = async (data: Data) => {
+  const { libraryItemId, userId } = data
+
+  const item = await findLibraryItemById(libraryItemId, userId)
+  if (!item) {
+    logger.info('page not found')
+    return false
+  }
+
+  const thumbnail = item.thumbnail
+  if (thumbnail) {
+    logger.info('thumbnail already set')
+    const proxyUrl = createThumbnailUrl(thumbnail)
+    // pre-cache thumbnail first if exists
+    const image = await fetchImage(proxyUrl)
+    if (!image) {
+      logger.info('thumbnail image not found')
+      item.thumbnail = undefined
+    }
+  }
+
+  logger.info('pre-caching all images...')
+  // pre-cache all images in the content and get their sizes
+  const imageSizes = await fetchAllImageSizes(item.readableContent)
+  // find thumbnail from all images if thumbnail not set
+  if (!item.thumbnail && imageSizes.length > 0) {
+    logger.info('finding thumbnail...')
+    const thumbnail = _findThumbnail(imageSizes)
+    if (!thumbnail) {
+      logger.info('no thumbnail found from content')
+      return false
+    }
+
+    // update page with thumbnail
+    await updateLibraryItem(
+      libraryItemId,
+      {
+        thumbnail,
+      },
+      userId
+    )
+    logger.info(`thumbnail updated: ${thumbnail}`)
+  }
+
+  return true
+}

packages/api/src/queue-processor.ts

@@ -2,17 +2,18 @@
 /* eslint-disable @typescript-eslint/restrict-template-expressions */
 /* eslint-disable @typescript-eslint/require-await */
 /* eslint-disable @typescript-eslint/no-misused-promises */
-import { Job, QueueEvents, Worker, Queue, JobType } from 'bullmq'
+import { Job, Queue, QueueEvents, Worker, JobType } from 'bullmq'
 import express, { Express } from 'express'
 import { SnakeNamingStrategy } from 'typeorm-naming-strategies'
 import { appDataSource } from './data_source'
 import { env } from './env'
+import { findThumbnail, THUMBNAIL_JOB } from './jobs/find_thumbnail'
 import { refreshAllFeeds } from './jobs/rss/refreshAllFeeds'
 import { refreshFeed } from './jobs/rss/refreshFeed'
 import { savePageJob } from './jobs/save_page'
+import { updatePDFContentJob } from './jobs/update_pdf_content'
 import { redisDataSource } from './redis_data_source'
 import { CustomTypeOrmLogger } from './utils/logger'
-import { updatePDFContentJob } from './jobs/update_pdf_content'
 
 export const QUEUE_NAME = 'omnivore-backend-queue'
 
@@ -39,8 +40,8 @@ const main = async () => {
   const port = process.env.PORT || 3002
 
   redisDataSource.setOptions({
-    REDIS_URL: env.redis.url,
-    REDIS_CERT: env.redis.cert,
+    cache: env.redis.cache,
+    mq: env.redis.mq,
   })
 
   appDataSource.setOptions({
@@ -119,8 +120,9 @@ const main = async () => {
         case 'update-pdf-content': {
           return updatePDFContentJob(job.data)
         }
+        case THUMBNAIL_JOB:
+          return findThumbnail(job.data)
       }
-      return true
     },
     {
       connection: workerRedisClient,

packages/api/src/redis_data_source.ts

@@ -1,9 +1,14 @@
 import Redis, { RedisOptions } from 'ioredis'
 import { env } from './env'
+import { logger } from './utils/logger'
 
+type RedisClientType = 'cache' | 'mq'
+type RedisDataSourceOption = {
+  url?: string
+  cert?: string
+}
 export type RedisDataSourceOptions = {
-  REDIS_URL?: string
-  REDIS_CERT?: string
+  [key in RedisClientType]: RedisDataSourceOption
 }
 
 export class RedisDataSource {
@@ -22,8 +27,9 @@ export class RedisDataSource {
   async initialize(): Promise<this> {
     if (this.isInitialized) throw 'Error already initialized'
 
-    this.redisClient = createIORedisClient('app', this.options)
-    this.workerRedisClient = createIORedisClient('worker', this.options)
+    this.redisClient = createIORedisClient('cache', this.options)
+    this.workerRedisClient =
+      createIORedisClient('mq', this.options) || this.redisClient // if mq is not defined, use cache
     this.isInitialized = true
 
     return Promise.resolve(this)
@@ -45,17 +51,21 @@ export class RedisDataSource {
 }
 
 const createIORedisClient = (
-  name: string,
+  name: RedisClientType,
   options: RedisDataSourceOptions
 ): Redis | undefined => {
-  const redisURL = options.REDIS_URL
+  const option = options[name]
+  const redisURL = option.url
   if (!redisURL) {
-    throw 'Error: no redisURL supplied'
+    logger.info(`no redisURL supplied: ${name}`)
+    return undefined
   }
+
+  const redisCert = option.cert
   const tls =
-    redisURL.startsWith('rediss://') && options.REDIS_CERT
+    redisURL.startsWith('rediss://') && redisCert
       ? {
-          ca: options.REDIS_CERT,
+          ca: redisCert,
           rejectUnauthorized: false,
         }
       : undefined
@@ -92,7 +102,6 @@ const createIORedisClient = (
   return new Redis(redisURL, redisOptions)
 }
 
-export const redisDataSource = new RedisDataSource({
-  REDIS_URL: env.redis.url,
-  REDIS_CERT: env.redis.cert,
-})
+export const redisDataSource = new RedisDataSource(
+  env.redis as RedisDataSourceOptions
+)

packages/api/src/server.ts

@@ -159,7 +159,7 @@ const main = async (): Promise<void> => {
   await appDataSource.initialize()
 
   // redis is optional for the API server
-  if (env.redis.url) {
+  if (env.redis.cache.url) {
     await redisDataSource.initialize()
   }
 

packages/api/src/services/save_email.ts

@@ -1,5 +1,5 @@
 import { LibraryItem, LibraryItemState } from '../entity/library_item'
-import { enqueueThumbnailTask } from '../utils/createTask'
+import { enqueueThumbnailJob } from '../utils/createTask'
 import {
   cleanUrl,
   generateSlug,
@@ -132,12 +132,14 @@ export const saveEmail = async (
 
   await updateReceivedEmail(input.receivedEmailId, 'article', input.userId)
 
-  // create a task to update thumbnail and pre-cache all images
-  try {
-    const taskId = await enqueueThumbnailTask(input.userId, slug)
-    logger.info('Created thumbnail task', { taskId })
-  } catch (e) {
-    logger.error('Failed to create thumbnail task', e)
+  if (!newLibraryItem.thumbnail) {
+    // create a task to update thumbnail and pre-cache all images
+    try {
+      const job = await enqueueThumbnailJob(input.userId, newLibraryItem.id)
+      logger.info('Created thumbnail job', { taskId: job })
+    } catch (e) {
+      logger.error('Failed to create thumbnail job', e)
+    }
   }
 
   return newLibraryItem

packages/api/src/services/save_page.ts

@@ -13,7 +13,7 @@ import {
   SaveResult,
 } from '../generated/graphql'
 import { authTrx } from '../repository'
-import { enqueueThumbnailTask } from '../utils/createTask'
+import { enqueueThumbnailJob } from '../utils/createTask'
 import {
   cleanUrl,
   generateSlug,
@@ -170,10 +170,10 @@ export const savePage = async (
   if (!isImported && !parseResult.parsedContent?.previewImage) {
     try {
       // create a task to update thumbnail and pre-cache all images
-      const taskId = await enqueueThumbnailTask(user.id, slug)
-      logger.info('Created thumbnail task', { taskId })
+      const job = await enqueueThumbnailJob(user.id, clientRequestId)
+      logger.info('Created thumbnail job', { job })
     } catch (e) {
-      logger.error('Failed to create thumbnail task', e)
+      logger.error('Failed to enqueue thumbnail job', e)
     }
   }
 

packages/api/src/util.ts

@@ -4,6 +4,11 @@
 import * as dotenv from 'dotenv'
 import os from 'os'
 
+interface redisConfig {
+  url?: string
+  cert?: string
+}
+
 export interface BackendEnv {
   pg: {
     host: string
@@ -105,8 +110,8 @@ export interface BackendEnv {
     }
   }
   redis: {
-    url?: string
-    cert?: string
+    mq: redisConfig
+    cache: redisConfig
   }
 }
 
@@ -156,6 +161,8 @@ const nullableEnvVars = [
   'SUBSCRIPTION_FEED_MAX',
   'REDIS_URL',
   'REDIS_CERT',
+  'MQ_REDIS_URL',
+  'MQ_REDIS_CERT',
   'IMPORTER_METRICS_COLLECTOR_URL',
   'INTERNAL_API_URL',
 ] // Allow some vars to be null/empty
@@ -295,8 +302,14 @@ export function getEnv(): BackendEnv {
     },
   }
   const redis = {
-    url: parse('REDIS_URL'),
-    cert: parse('REDIS_CERT')?.replace(/\\n/g, '\n'), // replace \n with new line
+    mq: {
+      url: parse('MQ_REDIS_URL'),
+      cert: parse('MQ_REDIS_CERT')?.replace(/\\n/g, '\n'), // replace \n with new line
+    },
+    cache: {
+      url: parse('REDIS_URL'),
+      cert: parse('REDIS_CERT')?.replace(/\\n/g, '\n'), // replace \n with new line
+    },
   }
 
   return {

packages/api/src/utils/createTask.ts

@@ -14,10 +14,12 @@ import {
   ArticleSavingRequestStatus,
   CreateLabelInput,
 } from '../generated/graphql'
+import { THUMBNAIL_JOB } from '../jobs/find_thumbnail'
 import { queueRSSRefreshFeedJob } from '../jobs/rss/refreshAllFeeds'
+import { getBackendQueue } from '../queue-processor'
 import { redisDataSource } from '../redis_data_source'
 import { signFeatureToken } from '../services/features'
-import { generateVerificationToken, OmnivoreAuthorizationHeader } from './auth'
+import { OmnivoreAuthorizationHeader } from './auth'
 import { CreateTaskError } from './errors'
 import { stringToHash } from './helpers'
 import { logger } from './logger'
@@ -577,52 +579,22 @@ export const enqueueExportToIntegration = async (
   return createdTasks[0].name
 }
 
-export const enqueueThumbnailTask = async (
+export const enqueueThumbnailJob = async (
   userId: string,
-  slug: string
-): Promise<string> => {
-  const { GOOGLE_CLOUD_PROJECT } = process.env
+  libraryItemId: string
+) => {
+  const queue = await getBackendQueue()
+  if (!queue) {
+    return undefined
+  }
   const payload = {
     userId,
-    slug,
+    libraryItemId,
   }
-
-  const headers = {
-    Cookie: `auth=${generateVerificationToken({ id: userId })}`,
-  }
-
-  // If there is no Google Cloud Project Id exposed, it means that we are in local environment
-  if (env.dev.isLocal || !GOOGLE_CLOUD_PROJECT) {
-    if (env.queue.thumbnailTaskHandlerUrl) {
-      // Calling the handler function directly.
-      setTimeout(() => {
-        axios
-          .post(env.queue.thumbnailTaskHandlerUrl, payload, {
-            headers,
-          })
-          .catch((error) => {
-            logError(error)
-          })
-      }, 0)
-    }
-    return ''
-  }
-
-  const createdTasks = await createHttpTaskWithToken({
-    payload,
-    taskHandlerUrl: env.queue.thumbnailTaskHandlerUrl,
-    requestHeaders: headers,
-    queue: 'omnivore-thumbnail-queue',
+  return queue.add(THUMBNAIL_JOB, payload, {
+    priority: 100,
+    attempts: 1,
   })
-
-  if (!createdTasks || !createdTasks[0].name) {
-    logger.error(`Unable to get the name of the task`, {
-      payload,
-      createdTasks,
-    })
-    throw new CreateTaskError(`Unable to get the name of the task`)
-  }
-  return createdTasks[0].name
 }
 
 export interface RssSubscriptionGroup {

packages/api/src/utils/helpers.ts

@@ -22,11 +22,10 @@ import {
   PageType,
   Profile,
   Recommendation,
-  ResolverFn,
   SearchItem,
 } from '../generated/graphql'
 import { createPubSubClient } from '../pubsub'
-import { Claims, WithDataSourcesContext } from '../resolvers/types'
+import { redisDataSource } from '../redis_data_source'
 import { validateUrl } from '../services/create_page_save_request'
 import { updateLibraryItem } from '../services/library_item'
 import { Merge } from '../util'
@@ -391,17 +390,10 @@ export const setRecentlySavedItemInRedis = async (
   url: string
 ) => {
   // save the url in redis for 26 hours so rss-feeder won't try to re-save it
-  if (!redisClient) {
-    console.info(
-      'not setting recently saved item because redis is not configured'
-    )
-    return
-  }
-  // save the url in redis for 8 hours so rss-feeder won't try to re-save it
   const redisKey = `recent-saved-item:${userId}:${url}`
   const ttlInSeconds = 60 * 60 * 26
   try {
-    return redisClient.set(redisKey, 1, 'EX', ttlInSeconds, 'NX')
+    return await redisClient.set(redisKey, 1, 'EX', ttlInSeconds, 'NX')
   } catch (error) {
     logger.error('error setting recently saved item in redis', {
       redisKey,

packages/api/test/global-setup.ts

@@ -7,7 +7,7 @@ export const mochaGlobalSetup = async () => {
   await createTestConnection()
   console.log('db connection created')
 
-  if (env.redis.url) {
+  if (env.redis.cache.url) {
     await redisDataSource.initialize()
     console.log('redis connection created')
   }

packages/api/test/global-teardown.ts

@@ -10,7 +10,7 @@ export const mochaGlobalTeardown = async () => {
   await appDataSource.destroy()
   console.log('db connection closed')
 
-  if (env.redis.url) {
+  if (env.redis.cache.url) {
     await redisDataSource.shutdown()
     console.log('redis connection closed')
   }

packages/content-fetch/src/redis_data_source.ts

@@ -1,8 +1,12 @@
 import Redis, { RedisOptions } from 'ioredis'
 
+type RedisClientType = 'cache' | 'mq'
+type RedisDataSourceOption = {
+  url?: string
+  cert?: string
+}
 export type RedisDataSourceOptions = {
-  REDIS_URL?: string
-  REDIS_CERT?: string
+  [key in RedisClientType]: RedisDataSourceOption
 }
 
 export class RedisDataSource {
@@ -14,12 +18,12 @@ export class RedisDataSource {
   constructor(options: RedisDataSourceOptions) {
     this.options = options
 
-    this.cacheClient = createRedisClient('cache', this.options)
-    this.queueRedisClient = createRedisClient('queue', this.options)
-  }
+    const cacheClient = createIORedisClient('cache', this.options)
+    if (!cacheClient) throw 'Error initializing cache redis client'
 
-  setOptions(options: RedisDataSourceOptions): void {
-    this.options = options
+    this.cacheClient = cacheClient
+    this.queueRedisClient =
+      createIORedisClient('mq', this.options) || this.cacheClient // if mq is not defined, use cache
   }
 
   async shutdown(): Promise<void> {
@@ -32,46 +36,45 @@ export class RedisDataSource {
   }
 }
 
-const createRedisClient = (name: string, options: RedisDataSourceOptions) => {
-  const redisURL = options.REDIS_URL
-  const cert = options.REDIS_CERT?.replace(/\\n/g, '\n') // replace \n with new line
+const createIORedisClient = (
+  name: RedisClientType,
+  options: RedisDataSourceOptions
+): Redis | undefined => {
+  const option = options[name]
+  const redisURL = option.url
   if (!redisURL) {
-    throw 'Error: no redisURL supplied'
+    console.log(`no redisURL supplied: ${name}`)
+    return undefined
   }
 
-  const redisOptions: RedisOptions = {
-    name,
-    connectTimeout: 10000, // 10 seconds
-    tls: cert
+  const redisCert = option.cert
+  const tls =
+    redisURL.startsWith('rediss://') && redisCert
       ? {
-          cert,
-          rejectUnauthorized: false, // for self-signed certs
+          ca: redisCert,
+          rejectUnauthorized: false,
         }
-      : undefined,
+      : undefined
+
+  const redisOptions: RedisOptions = {
+    tls,
+    name,
+    connectTimeout: 10000,
     maxRetriesPerRequest: null,
     offlineQueue: false,
   }
-
-  const redis = new Redis(redisURL, redisOptions)
-
-  redis.on('connect', () => {
-    console.log('Redis connected', name)
-  })
-
-  redis.on('error', (err) => {
-    console.error('Redis error', err, name)
-  })
-
-  redis.on('close', () => {
-    console.log('Redis closed', name)
-  })
-
-  return redis
+  return new Redis(redisURL, redisOptions)
 }
 
 export const redisDataSource = new RedisDataSource({
-  REDIS_URL: process.env.REDIS_URL,
-  REDIS_CERT: process.env.REDIS_CERT,
+  cache: {
+    url: process.env.REDIS_URL,
+    cert: process.env.REDIS_CERT,
+  },
+  mq: {
+    url: process.env.MQ_REDIS_URL,
+    cert: process.env.MQ_REDIS_CERT,
+  },
 })
 
 // eslint-disable-next-line @typescript-eslint/no-misused-promises

packages/content-fetch/src/request_handler.ts

@@ -53,8 +53,8 @@ interface FetchResult {
 }
 
 export const cacheFetchResult = async (fetchResult: FetchResult) => {
-  // cache the fetch result for 4 hours
-  const ttl = 4 * 60 * 60
+  // cache the fetch result for 24 hours
+  const ttl = 24 * 60 * 60
   const key = `fetch-result:${fetchResult.finalUrl}`
   const value = JSON.stringify(fetchResult)
   return redisDataSource.cacheClient.set(key, value, 'EX', ttl, 'NX')