fixed: OXUIB-1631 UI-middleware memory spikes after startup (3)

Cache lookups will immediately register a promise to prevent multiple requests for the same ressource

spec/file_caching_test.js

@@ -312,4 +312,40 @@ describe('File caching service', function () {
     expect(res2.statusCode).to.equal(200)
     expect(spy.callCount).to.equal(1)
   })
+
+  describe('redis request latency', function () {
+    beforeEach(function () {
+      // overwrite redis.get to simulate network latency
+      const get = redis.client.get
+      redis.client.get = function () {
+        return new Promise(resolve => {
+          setTimeout(async () => {
+            resolve(await get.apply(redis.client, arguments))
+          }, 20)
+        })
+      }
+    })
+
+    it('only requests files once, even though the response takes some time', async function () {
+      let spy
+      mockFetch({
+        'http://ui-server': {
+          '/manifest.json': generateSimpleViteManifest({}),
+          '/example.js': spy = sandbox.spy(() => {
+            return new Response('this is example', { headers: { 'content-type': 'application/javascript' } })
+          })
+        }
+      })
+      app = await mockApp()
+
+      expect(spy.callCount).to.equal(0)
+      const [res1, res2] = await Promise.all([
+        request(app).get('/example.js'),
+        request(app).get('/example.js')
+      ])
+      expect(res1.statusCode).to.equal(200)
+      expect(res2.statusCode).to.equal(200)
+      expect(spy.callCount).to.equal(1)
+    })
+  })
 })

src/cache.js

@@ -3,14 +3,6 @@ import * as redis from './redis.js'
 
 const cache = {}
 
-export async function setAsync (key, asyncValue) {
-  logger.trace(`[Cache] Set async ${key}`)
-  cache[key] = asyncValue
-  const value = await asyncValue
-  await set(key, value)
-  return value
-}
-
 export function set (key, value) {
   logger.trace(`[Cache] Set ${key}`)
   if (cache[key] === value) return
@@ -20,28 +12,43 @@ export function set (key, value) {
   }
 }
 
-export async function getBuffer (key) {
-  return get(key, { method: 'getBuffer' })
-}
-
 export async function clear () {
   for (const prop of Object.getOwnPropertyNames(cache)) {
     delete cache[prop]
   }
 }
 
-export async function get (key, { method = 'get' } = {}) {
+export function get (key, fallback) {
   if (cache[key]) {
     logger.trace(`[Cache] Resolve "${key}" from memory`)
     return cache[key]
   }
 
-  if (redis.isEnabled()) {
-    const result = await redis.client[method]?.(key)
-    if (result) {
-      logger.trace(`[Cache] Resolve "${key}" from redis`)
-      cache[key] = result
+  const promise = (async () => {
+    if (redis.isEnabled()) {
+      let result = await redis.client.get(key)
+      if (result) {
+        logger.trace(`[Cache] Resolve "${key}" from redis`)
+        result = JSON.parse(result)
+        cache[key] = result
+        return result
+      }
     }
-    return result
-  }
+
+    if (!fallback) return
+
+    const fallbackResult = await fallback()
+    if (fallbackResult) {
+      logger.trace(`[Cache] Found a fallback for "${key}"`)
+      cache[key] = fallbackResult
+      if (redis.isEnabled()) redis.client.set(key, JSON.stringify(fallbackResult))
+    }
+    return fallbackResult
+  })()
+
+  cache[key] = promise
+
+  return promise
 }
+
+export function getCache () { return cache }

src/files.js

@@ -6,6 +6,12 @@ import { getCSSDependenciesFor, getViteManifests } from './manifests.js'
 import * as cache from './cache.js'
 import { logger } from './logger.js'
 import { NotFoundError } from './errors.js'
+import * as redis from './redis.js'
+
+export function createWritable (body) {
+  if (typeof body !== 'string' && !(body instanceof Buffer)) return JSON.stringify(body)
+  return body
+}
 
 export async function fetchFileWithHeadersFromBaseUrl (path, baseUrl, version) {
   const [response, dependencies] = await Promise.all([
@@ -50,22 +56,41 @@ export async function fetchFileWithHeaders ({ path, version }) {
   return Promise.any(config.urls.map(baseUrl => fetchFileWithHeadersFromBaseUrl(path, baseUrl, version)))
 }
 
-export async function saveAsyncToCache ({ version, path, promise }) {
-  await Promise.all([
-    cache.setAsync(getRedisKey({ version, name: `${path}:body` }), promise.then(({ body }) => {
-      if (typeof body !== 'string' && !(body instanceof Buffer)) return JSON.stringify(body)
-      return body
-    })),
-    cache.setAsync(getRedisKey({ version, name: `${path}:meta` }), promise.then(({ body, ...rest }) => JSON.stringify(rest)))
-  ])
-  return promise
-}
+export function getFile ({ version, path }) {
+  const key = getRedisKey({ version, name: `${path}` })
 
-export async function loadFromCache ({ version, path }) {
-  const [body, meta = '{}'] = await Promise.all([
-    cache.getBuffer(getRedisKey({ version, name: `${path}:body` })),
-    cache.get(getRedisKey({ version, name: `${path}:meta` }))
-  ])
-  if (!body) return
-  return { ...JSON.parse(meta), body }
+  // try to get the file synchronously.
+  const data = cache.getCache()[key]
+  if (data) return data
+
+  // if synchronously does not work, store the async promise for further requests
+  const promise = (async () => {
+    const bodyKey = getRedisKey({ version, name: `${path}:body` })
+    const metaKey = getRedisKey({ version, name: `${path}:meta` })
+
+    if (redis.isEnabled()) {
+      const [body, meta = '{}'] = await Promise.all([
+        redis.client.getBuffer(bodyKey),
+        redis.client.get(metaKey)
+      ])
+
+      if (body) {
+        return (cache.getCache()[key] = { body, ...JSON.parse(meta) })
+      }
+    }
+
+    const dataFromServer = await fetchFileWithHeaders({ version, path })
+    if (redis.isEnabled()) {
+      const { body, ...rest } = dataFromServer
+      redis.client.set(bodyKey, createWritable(body))
+      redis.client.set(metaKey, JSON.stringify(rest))
+    }
+
+    // overwrite cache with synchronous data
+    return (cache.getCache()[key] = dataFromServer)
+  })()
+
+  // temporary set to promise
+  cache.getCache()[key] = promise
+  return promise
 }

src/manifests.js

@@ -28,34 +28,22 @@ export async function fetchViteManifests () {
   return viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
 }
 
-export async function getViteManifests ({ version }) {
-  let manifests = await cache.get(getRedisKey({ version, name: 'viteManifests' }))
-  if (!manifests) {
-    manifests = await cache.setAsync(getRedisKey({ version, name: 'viteManifests' }), fetchViteManifests().then(m => JSON.stringify(m)))
-  }
-  return JSON.parse(manifests)
+export function getViteManifests ({ version }) {
+  return cache.get(getRedisKey({ version, name: 'viteManifests' }), () => fetchViteManifests())
 }
 
-export async function getOxManifests ({ version }) {
-  let manifests = await cache.get(getRedisKey({ version, name: 'oxManifests' }))
-  if (!manifests) {
-    manifests = await cache.setAsync(getRedisKey({ version, name: 'oxManifests' }), (async () => {
-      const viteManifests = await getViteManifests({ version })
-      return JSON.stringify(viteToOxManifest(viteManifests))
-    })())
-  }
-  return JSON.parse(manifests)
+export function getOxManifests ({ version }) {
+  return cache.get(getRedisKey({ version, name: 'oxManifests' }), async () => {
+    const viteManifests = await getViteManifests({ version })
+    return viteToOxManifest(viteManifests)
+  })
 }
 
-export async function getDependencies ({ version }) {
-  let deps = await cache.get(getRedisKey({ version, name: 'dependencies' }))
-  if (!deps) {
-    deps = await await cache.setAsync(getRedisKey({ version, name: 'dependencies' }), (async () => {
-      const viteManifests = await getViteManifests({ version })
-      return JSON.stringify(viteManifestToDeps(viteManifests))
-    })())
-  }
-  return JSON.parse(deps)
+export function getDependencies ({ version }) {
+  return cache.get(getRedisKey({ version, name: 'dependencies' }), async () => {
+    const viteManifests = await getViteManifests({ version })
+    return viteManifestToDeps(viteManifests)
+  })
 }
 
 export async function getCSSDependenciesFor ({ file, version }) {

src/meta.js

@@ -27,10 +27,6 @@ export async function fetchMergedMetadata () {
   return metadata.filter(Boolean)
 }
 
-export async function getMergedMetadata ({ version }) {
-  let metadata = await cache.get(getRedisKey({ version, name: 'mergedMetadata' }))
-  if (!metadata) {
-    metadata = await cache.setAsync(getRedisKey({ version, name: 'mergedMetadata' }), fetchMergedMetadata().then(m => JSON.stringify(m)))
-  }
-  return JSON.parse(metadata)
+export function getMergedMetadata ({ version }) {
+  return cache.get(getRedisKey({ version, name: 'mergedMetadata' }), () => fetchMergedMetadata())
 }

src/middlewares/serve-files.js

-import { fetchFileWithHeaders, loadFromCache, saveAsyncToCache } from '../files.js'
+import { getFile } from '../files.js'
 import { NotFoundError } from '../errors.js'
 import createError from 'http-errors'
 
@@ -7,15 +7,8 @@ export default async function (req, res, next) {
     if (req.method !== 'GET') return next()
     const version = res.version
     const path = req.path === '/' ? '/index.html' : req.path
-    let data = await loadFromCache({
-      path,
-      version
-    })
-    if (!data) {
-      data = await saveAsyncToCache({ version, path, promise: fetchFileWithHeaders({ path, version }) })
-    }
+    const { body, headers, sha256Sum } = await getFile({ version, path })
 
-    const { body, headers, sha256Sum } = data
     res.set(headers)
     res.locals.sha256Sum = sha256Sum
     res.send(body)