From b3692a84656199f43d47d82accc33bd3cc70d732 Mon Sep 17 00:00:00 2001
From: Richard Petersen <richard.petersen@open-xchange.com>
Date: Mon, 29 Nov 2021 11:29:52 +0100
Subject: [PATCH] Fix: Multiple manifests are fetched at the same time
Root cause: Due to the timeout, every request that was launched while fetching the previous manifests could led to another fetch and another cache.warmup
Solution: Be more synchronous
---
spec/server_test.js | 17 +++++++---
src/createApp.js | 6 ++--
src/manifests.js | 76 +++++++++++++++++++++++++++------------------
3 files changed, 62 insertions(+), 37 deletions(-)
diff --git a/spec/server_test.js b/spec/server_test.js
index 541c136..53d5216 100644
--- a/spec/server_test.js
+++ b/spec/server_test.js
@@ -26,7 +26,8 @@ describe('Manifest service', () => {
beforeEach(async () => {
mockserver = await createMockServer({ port })
mockserver.respondWith({
- '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'test' })
+ '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'test' }),
+ '/example.js': ''
})
})
@@ -54,7 +55,8 @@ describe('Manifest service', () => {
mockserver.close()
mockserver = await createMockServer({ port })
mockserver.respondWith({
- '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' })
+ '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' }),
+ '/example.js': ''
})
await new Promise(resolve => setTimeout(resolve, 150))
@@ -75,7 +77,8 @@ describe('Manifest service', () => {
mockserver.close()
mockserver = await createMockServer({ port })
mockserver.respondWith({
- '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' })
+ '/api/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' }),
+ '/example.js': ''
})
// wait some time
@@ -97,8 +100,12 @@ describe('Manifest service', () => {
mockserver.close()
mockserver = await createMockServer({ port })
- mockserver.respondWith({ '/api/manifest.json': generateSimpleViteManifest({ 'example1.js': 'other' }) })
- mockserver.respondWith({ '/api/no2/manifest.json': generateSimpleViteManifest({ 'example2.js': 'thing' }) })
+ mockserver.respondWith({
+ '/api/manifest.json': generateSimpleViteManifest({ 'example1.js': 'other' }),
+ '/api/no2/manifest.json': generateSimpleViteManifest({ 'example2.js': 'thing' }),
+ '/example1.js': '',
+ '/example2.js': ''
+ })
process.env.CACHE_TTL = 1
const app = createApp()
diff --git a/src/createApp.js b/src/createApp.js
index 0bd24a8..c7aea10 100644
--- a/src/createApp.js
+++ b/src/createApp.js
@@ -18,7 +18,7 @@ import promBundle from 'express-prom-bundle'
import swaggerUi from 'swagger-ui-express'
import yaml from 'js-yaml'
import fs from 'fs'
-import { getCSSDependenciesFor, getDependencies, getOxManifests, getVersion } from './manifests.js'
+import { getCSSDependenciesFor, getDependencies, getOxManifests, getVersion, loadViteManifests, viteManifestToDeps } from './manifests.js'
import { fileCache } from './files.js'
const ignorePaths = ['/ready', '/healthy']
@@ -37,7 +37,9 @@ export function createApp () {
const healthCheck = new health.HealthChecker()
const readinessCheck = new health.ReadinessCheck('getDependencies', async function () {
try {
- await getDependencies()
+ const viteManifests = await loadViteManifests()
+ const deps = viteManifestToDeps(viteManifests)
+ await fileCache.warmUp(viteManifests, deps)
} catch (e) {
logger.error(`Failed to get dependencies: ${e.message}`)
throw e
diff --git a/src/manifests.js b/src/manifests.js
index f1a1e52..47ea557 100644
--- a/src/manifests.js
+++ b/src/manifests.js
@@ -6,43 +6,60 @@ import { config } from './config.js'
import { hash } from './util.js'
export const loadViteManifests = (() => {
- let cache
+ let cachePromise
let lastCacheTime
+ let lastHash
- return async function loadViteManifests ({ useCache = true } = {}) {
+ async function reload () {
+ await config.load()
+ // vite manifests contains a set of objects with the vite-manifests
+ // from the corresponding registered services
+ const viteManifests = await Promise.all(config.urls.map(async url => {
+ const { origin } = new URL(url)
+ // fetch the manifests
+ const result = await fetch(url)
+ if (!result.ok) throw new Error(`Failed to load manifest for url ${result.url} (Status: ${result.status}: ${result.statusText})`)
+ try {
+ const manifest = await result.json()
+ for (const file in manifest) {
+ manifest[file].meta = manifest[file].meta || {}
+ manifest[file].meta.baseUrl = origin
+ }
+ return manifest
+ } catch (err) {
+ throw new Error(`Failed to load manifest for url ${result.url}: ${err}`)
+ }
+ }))
+
+ // combine all manifests by keys. With duplicates, last wins
+ return viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
+ }
+
+ return function loadViteManifests ({ useCache = true } = {}) {
const CACHE_TTL = parseInt(process.env.CACHE_TTL)
- if (!cache || useCache === false || +new Date() > lastCacheTime + CACHE_TTL) {
- await config.load()
- // vite manifests contains a set of objects with the vite-manifests
- // from the corresponding registered services
- const viteManifests = await Promise.all(config.urls.map(async url => {
- const { origin } = new URL(url)
- // fetch the manifests
- const result = await fetch(url)
- if (!result.ok) throw new Error(`Failed to load manifest for url ${result.url} (Status: ${result.status}: ${result.statusText})`)
- try {
- const manifest = await result.json()
- for (const file in manifest) {
- manifest[file].meta = manifest[file].meta || {}
- manifest[file].meta.baseUrl = origin
+ if (!cachePromise || useCache === false || +new Date() > lastCacheTime + CACHE_TTL) {
+ cachePromise = reload()
+ cachePromise.then(manifests => {
+ // update cache promise
+ const newHash = hash(manifests)
+ if (newHash !== lastHash) {
+ if (lastHash) {
+ const deps = viteManifestToDeps(manifests)
+ // asynchronously rewarm the cache
+ fileCache.warmUp(manifests, deps)
}
- return manifest
- } catch (err) {
- throw new Error(`Failed to load manifest for url ${result.url}: ${err}`)
+ lastHash = newHash
+ Object.defineProperty(manifests, '__hash__', {
+ enumerable: false,
+ writable: true
+ })
+ manifests.__hash__ = newHash
}
- }))
-
- // combine all manifests by keys. With duplicates, last wins
- const viteManifest = viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
- // only update that object if it really changed to prevent any further parsing from being triggered
- if (!cache || JSON.stringify(viteManifest) !== JSON.stringify(cache)) {
- cache = viteManifest
- }
-
+ })
lastCacheTime = +new Date()
}
- return cache
+ return cachePromise
}
})()
@@ -99,7 +116,6 @@ export const getDependencies = (() => {
const viteManifest = await loadViteManifests()
if (viteManifest !== prevViteManifest) {
depCache = viteManifestToDeps(viteManifest)
- fileCache.warmUp(viteManifest, depCache)
prevViteManifest = viteManifest
}
return depCache
--
GitLab