diff --git a/lib/middleware/serveThemes.js b/lib/middleware/serveThemes.js index ab034054..f14e0d5e 100644 --- a/lib/middleware/serveThemes.js +++ b/lib/middleware/serveThemes.js @@ -45,6 +45,54 @@ function createMiddleware({resources, middlewareUtil}) { }); const buildOptions = {}; + const currentRequests = {}; + + async function buildTheme(pathname) { + const filename = basename(pathname); + + if (cssVariablesThemeResources.includes(filename) && !buildOptions.cssVariables) { + // Activate CSS Variables build the first time a relevant resource is requested + buildOptions.cssVariables = true; + // Clear the cache to ensure that the build is executed again with "cssVariables: true" + builder.clearCache(); + } + + const sourceLessPath = dirname(pathname) + "/library.source.less"; + const sourceLessResource = await resources.all.byPath(sourceLessPath); + if (!sourceLessResource) { // Not found + return; + } + + const createdResources = await builder.build([sourceLessResource], buildOptions); + + // Pick requested file resource + const resource = createdResources.find((res) => res.getPath().endsWith(filename)); + if (!resource) { + throw new Error(`Theme Build did not return requested file "${pathname}"`); + } + + return resource; + } + + async function sendResponse(req, res, resource) { + const resourcePath = resource.getPath(); + const {contentType} = middlewareUtil.getMimeInfo(resourcePath); + res.setHeader("Content-Type", contentType); + + const content = await resource.getBuffer(); + + res.setHeader("ETag", etag(content)); + + if (isFresh(req, res)) { + // client has a fresh copy of the resource + res.statusCode = 304; + res.end(); + return; + } + + res.end(content); + } + return async function theme(req, res, next) { try { const pathname = parseurl(req).pathname; @@ -54,43 +102,18 @@ function createMiddleware({resources, middlewareUtil}) { return; } - if (cssVariablesThemeResources.includes(filename) && !buildOptions.cssVariables) { - // Activate CSS Variables build the first time a relevant resource is requested - buildOptions.cssVariables = true; - // Clear the cache to ensure that the build is executed again with "cssVariables: true" - builder.clearCache(); + if (!currentRequests[pathname]) { + currentRequests[pathname] = buildTheme(pathname); } - const sourceLessPath = dirname(pathname) + "/library.source.less"; - const sourceLessResource = await resources.all.byPath(sourceLessPath); - if (!sourceLessResource) { // Not found - next(); - return; - } - - const createdResources = await builder.build([sourceLessResource], buildOptions); - // Pick requested file resource - const resource = createdResources.find((res) => res.getPath().endsWith(filename)); + const resource = await currentRequests[pathname]; if (!resource) { - next(new Error(`Theme Build did not return requested file "${pathname}"`)); - return; - } - - const resourcePath = resource.getPath(); - const {contentType} = middlewareUtil.getMimeInfo(resourcePath); - res.setHeader("Content-Type", contentType); - - const content = await resource.getBuffer(); - res.setHeader("ETag", etag(content)); - - if (isFresh(req, res)) { - // client has a fresh copy of the resource - res.statusCode = 304; - res.end(); - return; + next(); + } else { + await sendResponse(req, res, resource); } - res.end(content.toString()); + delete currentRequests[pathname]; } catch (err) { next(err); } diff --git a/test/lib/server/middleware/serveThemes.js b/test/lib/server/middleware/serveThemes.js index 91c4cbff..22df8197 100644 --- a/test/lib/server/middleware/serveThemes.js +++ b/test/lib/server/middleware/serveThemes.js @@ -401,3 +401,56 @@ test.serial.cb("Error handling: Unexpected exception within middleware should ca t.end(); }); }); + +test.serial("Multiple parallel requests to the same path should only result in one theme build", async (t) => { + const resources = createResources(); + + const build = stubThemeBuild(resources); + + const {middleware, byPath} = createMiddleware(); + byPath.withArgs("/resources/sap/ui/test/themes/base/library.source.less") + .resolves(resources["library.source.less"]); + byPath.withArgs("/resources/sap/ui/test2/themes/base/library.source.less") + .resolves(resources["library.source.less"]); + + function request(url) { + return new Promise((resolve, reject) => { + const req = { + url, + headers: {} + }; + + const res = { + setHeader: sinon.stub(), + getHeader: sinon.stub(), + end: resolve + }; + + middleware(req, res, reject); + }); + } + + await Promise.all([ + request("/resources/sap/ui/test/themes/base/library.css"), + request("/resources/sap/ui/test/themes/base/library.css"), + request("/resources/sap/ui/test/themes/base/library.css"), + + request("/resources/sap/ui/test2/themes/base/library.css"), + request("/resources/sap/ui/test2/themes/base/library.css") + ]); + // Should only build once per url + t.is(build.callCount, 2, "Build should be called 2 times"); + + + // After all requests have finished, the build should be started again when another request comes in + await Promise.all([ + request("/resources/sap/ui/test/themes/base/library.css"), + request("/resources/sap/ui/test/themes/base/library.css"), + request("/resources/sap/ui/test/themes/base/library.css"), + + request("/resources/sap/ui/test2/themes/base/library.css"), + request("/resources/sap/ui/test2/themes/base/library.css") + ]); + // Should only build once per url + t.is(build.callCount, 4, "Build should be called 4 times"); +});