From d213940d2e4c37588fcf810860f5878054be9de2 Mon Sep 17 00:00:00 2001
From: Richard Petersen <richard.petersen@open-xchange.com>
Date: Mon, 28 Feb 2022 14:22:56 +0100
Subject: [PATCH] Fix: OXUIB-1380 - UI forever (enough) broken after update
Root cause: Version has was updated before file cache
Solution: Keep all updates in one place and switch to new version at the same time
---
spec/dependencies_test.js | 2 +-
spec/manifest_parsing_test.js | 2 +-
spec/server_test.js | 46 +++++++++++++++++---
src/createApp.js | 3 +-
src/files.js | 28 +++++++++++-
src/manifests.js | 82 +++++------------------------------
src/util.js | 33 ++++++++++++++
7 files changed, 117 insertions(+), 79 deletions(-)
diff --git a/spec/dependencies_test.js b/spec/dependencies_test.js
index 8eedefa..f7b109a 100644
--- a/spec/dependencies_test.js
+++ b/spec/dependencies_test.js
@@ -1,4 +1,4 @@
-import { viteManifestToDeps } from '../src/manifests.js'
+import { viteManifestToDeps } from '../src/util.js'
import { expect } from 'chai'
describe('Vite manifest parsing', function () {
diff --git a/spec/manifest_parsing_test.js b/spec/manifest_parsing_test.js
index 89f980f..bed5ecd 100644
--- a/spec/manifest_parsing_test.js
+++ b/spec/manifest_parsing_test.js
@@ -1,4 +1,4 @@
-import { viteToOxManifest } from '../src/manifests.js'
+import { viteToOxManifest } from '../src/util.js'
import { expect } from 'chai'
describe('Vite manifest parsing', function () {
diff --git a/spec/server_test.js b/spec/server_test.js
index a29dd3e..08c4afa 100644
--- a/spec/server_test.js
+++ b/spec/server_test.js
@@ -2,12 +2,13 @@ import request from 'supertest'
import { generateSimpleViteManifest, mockApp, mockConfig, mockFetch } from './util.js'
import { expect } from 'chai'
import * as td from 'testdouble'
+import { Response } from 'node-fetch'
describe('UI Middleware', function () {
let app
let fetchConfig
- before(async function () {
+ beforeEach(async function () {
mockConfig({ urls: ['http://ui-server/'] })
mockFetch(fetchConfig = {
'http://ui-server': {
@@ -18,11 +19,8 @@ describe('UI Middleware', function () {
app = await mockApp()
})
- after(function () {
+ afterEach(function () {
td.reset()
- })
-
- afterEach(async function () {
process.env.CACHE_TTL = 30000
})
@@ -80,6 +78,8 @@ describe('UI Middleware', function () {
'/example.js': ''
}
+ // trigger update
+ await request(app).get('/manifests')
// wait some time
await new Promise(resolve => setTimeout(resolve, 10))
@@ -87,6 +87,42 @@ describe('UI Middleware', function () {
expect(response2.statusCode).to.equal(200)
expect(response2.body).to.deep.equal([{ namespace: 'other', path: 'example' }])
})
+
+ it('only updates the version hash when the caches are warm', async function () {
+ process.env.CACHE_TTL = 0
+
+ // fetch the file to get the initial version
+ let response = await request(app).get('/example.js')
+ expect(response.text).to.equal('')
+ const version = response.headers.version
+ await new Promise(resolve => setTimeout(resolve, 1))
+
+ // update resources
+ let resolveExampleJs
+ fetchConfig['http://ui-server'] = {
+ '/manifest.json': generateSimpleViteManifest({ 'example.js': 'other' }),
+ '/example.js': () => new Promise(resolve => (resolveExampleJs = resolve))
+ }
+
+ // fetch file again while the update is still processing
+ // this will also trigger the update
+ response = await request(app).get('/example.js')
+ expect(response.text).to.equal('')
+ expect(response.headers.version).to.equal(version)
+
+ // fetch once again. this will not trigger an update of the file-cache
+ response = await request(app).get('/example.js')
+ expect(response.text).to.equal('')
+ expect(response.headers.version).to.equal(version)
+
+ // resolve the response to the example js file. This will finish the cache warmup
+ resolveExampleJs(new Response('new content'))
+
+ // fetch the file again. Content and version should be updated
+ response = await request(app).get('/example.js')
+ expect(response.text).to.equal('new content')
+ expect(response.headers.version).not.to.equal(version)
+ })
})
describe('multiple configurations', function () {
diff --git a/src/createApp.js b/src/createApp.js
index 4841e0e..9ca9690 100644
--- a/src/createApp.js
+++ b/src/createApp.js
@@ -18,9 +18,10 @@ import promClient from 'prom-client'
import swaggerUi from 'swagger-ui-express'
import yaml from 'js-yaml'
import fs from 'fs'
-import { getCSSDependenciesFor, getDependencies, getOxManifests, getVersion, loadViteManifests, viteManifestToDeps } from './manifests.js'
+import { getCSSDependenciesFor, getDependencies, getOxManifests, getVersion, loadViteManifests } from './manifests.js'
import { fileCache } from './files.js'
import { getMergedMetadata } from './meta.js'
+import { viteManifestToDeps } from './util.js'
const swaggerDocument = yaml.load(fs.readFileSync('./src/swagger.yaml', 'utf8'))
diff --git a/src/files.js b/src/files.js
index c264499..d7b4af4 100644
--- a/src/files.js
+++ b/src/files.js
@@ -3,7 +3,7 @@ import crypto from 'crypto'
import { config } from './config.js'
import promClient from 'prom-client'
import { logger } from './logger.js'
-import { isJSFile } from './util.js'
+import { isJSFile, viteToOxManifest } from './util.js'
async function fetchData (path, baseUrl, appendix) {
const response = await fetch(new URL(path, baseUrl))
@@ -34,6 +34,10 @@ const fileErrorCounter = new promClient.Counter({
class FileCache {
constructor () {
this._cache = {}
+ this._manifests = {}
+ this._hash = ''
+ this._dependencies = {}
+ this._oxManifests = {}
}
async warmUp (manifests, deps) {
@@ -69,7 +73,13 @@ class FileCache {
fileCounter.inc(result.length)
return result
}()))
+
this._cache = cache
+ this._manifests = manifests
+ this._hash = `${+new Date()}.${manifests.__hash__}`
+ this._dependencies = deps
+ this._oxManifests = viteToOxManifest(manifests)
+
logger.debug('cache warmed up')
}
@@ -85,6 +95,22 @@ class FileCache {
get (path) {
return this?._cache[path.slice(1)] || {}
}
+
+ get manifests () {
+ return this?._manifests
+ }
+
+ get hash () {
+ return this?._hash
+ }
+
+ get dependencies () {
+ return this?._dependencies
+ }
+
+ get oxManifests () {
+ return this?._oxManifests
+ }
}
export const fileCache = new FileCache()
diff --git a/src/manifests.js b/src/manifests.js
index 07c6174..6801ea1 100644
--- a/src/manifests.js
+++ b/src/manifests.js
@@ -1,9 +1,8 @@
import fetch from 'node-fetch'
-import path from 'path'
import { URL } from 'url'
import { fileCache } from './files.js'
import { config } from './config.js'
-import { hash } from './util.js'
+import { hash, viteManifestToDeps } from './util.js'
import { logger } from './logger.js'
export const loadViteManifests = (() => {
@@ -74,81 +73,24 @@ export const loadViteManifests = (() => {
}
})()
-export function viteToOxManifest (viteManifests) {
- const deps = viteManifestToDeps(viteManifests)
- return Object.values(viteManifests)
- .filter(manifest => Array.isArray(manifest?.meta?.manifests))
- .map(manifest =>
- manifest.meta.manifests.map(oxManifest => {
- const dependencies = deps[manifest.file]
- const data = {
- ...oxManifest,
- path: manifest.file.slice(0, -path.parse(manifest.file).ext.length)
- }
- if (dependencies?.length > 0) data.dependencies = dependencies
- return data
- })
- )
- .flat()
+export async function getOxManifests () {
+ await loadViteManifests().catch(() => {})
+ return fileCache.oxManifests
}
-export const getOxManifests = (() => {
- let prevHash
- let oxManifestCache
- return async function getOxManifests () {
- const viteManifest = await loadViteManifests().catch(() => {})
- if (viteManifest && viteManifest.__hash__ !== prevHash) {
- oxManifestCache = viteToOxManifest(viteManifest)
- prevHash = viteManifest.__hash__
- }
- return oxManifestCache
- }
-})()
-
-export function viteManifestToDeps (viteManifest) {
- const deps = {}
- for (const [codePoint, { isEntry, file, imports, css, assets }] of Object.entries(viteManifest)) {
- if (isEntry && codePoint.endsWith('.html')) deps[codePoint] = [file]
- if (Array.isArray(assets)) assets.forEach(asset => { deps[asset] = [] })
- if (Array.isArray(css)) css.forEach(css => { deps[css] = [] })
- deps[file] = []
- .concat(imports?.map(path => viteManifest[path].file))
- .concat(css)
- .concat(assets)
- .filter(Boolean)
- }
- return deps
+export async function getDependencies () {
+ // simply catch the error here. This might happen, when one of the UI containers is temporarily not available
+ await loadViteManifests().catch(() => {})
+ return fileCache.dependencies
}
-export const getDependencies = (() => {
- let prevHash
- let depCache
- return async function getDependencies () {
- // simply catch the error here. This might happen, when one of the UI containers is temporarily not available
- const viteManifest = await loadViteManifests().catch(() => {})
- if (viteManifest && viteManifest.__hash__ !== prevHash) {
- depCache = viteManifestToDeps(viteManifest)
- prevHash = viteManifest.__hash__
- }
- return depCache
- }
-})()
-
export async function getCSSDependenciesFor (file) {
const allDependencies = await getDependencies()
const dependencies = allDependencies[file] || []
return dependencies.filter(dep => /\.css/i.test(dep))
}
-export const getVersion = (() => {
- let prevHash
- let versionString
- return async function getVersion () {
- const viteManifest = await loadViteManifests().catch(() => {})
- if (viteManifest && viteManifest.__hash__ !== prevHash) {
- versionString = `${+new Date()}.${viteManifest.__hash__}`
- prevHash = viteManifest.__hash__
- }
- return versionString
- }
-})()
+export async function getVersion () {
+ await loadViteManifests().catch(() => {})
+ return fileCache.hash
+}
diff --git a/src/util.js b/src/util.js
index 55c8043..018a517 100644
--- a/src/util.js
+++ b/src/util.js
@@ -21,3 +21,36 @@ export function isJSFile (name) {
const extname = path.extname(name)
return extname === '.js'
}
+
+export function viteManifestToDeps (viteManifest) {
+ const deps = {}
+ for (const [codePoint, { isEntry, file, imports, css, assets }] of Object.entries(viteManifest)) {
+ if (isEntry && codePoint.endsWith('.html')) deps[codePoint] = [file]
+ if (Array.isArray(assets)) assets.forEach(asset => { deps[asset] = [] })
+ if (Array.isArray(css)) css.forEach(css => { deps[css] = [] })
+ deps[file] = []
+ .concat(imports?.map(path => viteManifest[path].file))
+ .concat(css)
+ .concat(assets)
+ .filter(Boolean)
+ }
+ return deps
+}
+
+export function viteToOxManifest (viteManifests) {
+ const deps = viteManifestToDeps(viteManifests)
+ return Object.values(viteManifests)
+ .filter(manifest => Array.isArray(manifest?.meta?.manifests))
+ .map(manifest =>
+ manifest.meta.manifests.map(oxManifest => {
+ const dependencies = deps[manifest.file]
+ const data = {
+ ...oxManifest,
+ path: manifest.file.slice(0, -path.parse(manifest.file).ext.length)
+ }
+ if (dependencies?.length > 0) data.dependencies = dependencies
+ return data
+ })
+ )
+ .flat()
+}
--
GitLab