diff --git a/lib/framework.js b/lib/framework.js index c1c9db8c..96331368 100644 --- a/lib/framework.js +++ b/lib/framework.js @@ -9,17 +9,16 @@ const httpProxy = require("http-proxy"); const fs = require("fs"); const path = require("path"); const yaml = require("js-yaml"); +const {Router} = require("express"); const stat = fs.statSync; const {ErrorMessage} = require("./errors"); class Framework { constructor() { - this.isPaused = true; - this.queue = []; - this._serveResources = null; - this._serveThemes = null; this.config = {}; + this.beforeMiddleware = new Router(); + this.middleware = new Router(); } createPluginFilesPattern(pattern) { @@ -120,7 +119,6 @@ class Framework { this.config.client.ui5 = {}; this.config.client.ui5.useIframe = true; // for now only allow using iframes in HTML mode this.config.ui5 = config.ui5 || {}; - this.config.proxies = config.proxies || {}; this.config.middleware = config.middleware || []; this.config.files = config.files || []; this.config.beforeMiddleware = config.beforeMiddleware || []; @@ -263,7 +261,6 @@ class Framework { this.config.files.push( this.createProjectFilesPattern(config.basePath + `/{${webappFolder}/**,${webappFolder}/**/.*}`) ); - // No proxy required here, local files will be loaded via karma first } else if (config.ui5.type === "library") { const srcFolder = this.config.ui5.paths.src; const testFolder = this.config.ui5.paths.test; @@ -281,12 +278,6 @@ class Framework { this.createProjectFilesPattern(`${config.basePath}/{${srcFolder}/**,${srcFolder}/**/.*}`), this.createProjectFilesPattern(`${config.basePath}/{${testFolder}/**,${testFolder}/**/.*}`), ); - // Configure proxies to first load files from karma server (e.g. library under test) - // Otherwise the coverage reporting won't work - this.config.proxies[`/base/${this.replaceLast(srcFolder, "resources")}/`] = `/base/${srcFolder}/`; - this.config.proxies[`/base/${this.replaceLast(srcFolder, "test-resources")}/`] = `/base/${testFolder}/`; - this.config.proxies[`/base/${this.replaceLast(testFolder, "resources")}/`] = `/base/${srcFolder}/`; - this.config.proxies[`/base/${this.replaceLast(testFolder, "test-resources")}/`] = `/base/${testFolder}/`; } else { this.logger.log("error", ErrorMessage.invalidProjectType(config.ui5.type) ); throw new Error(ErrorMessage.failure()); @@ -370,6 +361,20 @@ class Framework { // } // } + /** + * Rewrites the given url to use a virtual path that can be resolved + * by the UI5 Tooling middleware or to conform with the UI5 CDN. + * + * Example (application): + * /base/webapp/resources/sap-ui-core.js -> /resources/sap-ui-core.js + * + * Example (library): + * /base/src/resources/sap-ui-core.js -> /resources/sap-ui-core.js + * /base/test/test-resources/sap-ui-core.js -> /test-resources/sap-ui-core.js + * + * @param {string} url + * @returns {string} + */ rewriteUrl(url) { const type = this.config.ui5.type; const webappFolder = this.config.ui5.paths.webapp; @@ -402,22 +407,50 @@ class Framework { return url; } - processRequests() { - this.isPaused = false; - this.queue.forEach(function(next) { - next(); - }); - this.queue = []; - } - - pauseRequests() { - return (req, res, next) => { - if (this.isPaused) { - this.queue.push(next); - } else { - next(); - } - }; + /** + * Rewrites the given url from a virtual path (resources / test-resources) + * to a filesystem path so that the request can be handled by the karma + * middleware that serves the project files. + * + * This is only relevant for type "library", as it has two separate folders (src / test). + * + * Example: + * /base/resources/sap-ui-core.js -> /base/src/sap-ui-core.js + * /base/test-resources/sap/ui/test/ -> /base/test/sap/ui/test/ + * + * @param {string} url + * @returns {string} + */ + rewriteUrlBefore(url) { + const type = this.config.ui5.type; + if (type !== "library") { + // Only rewrite "before" for type library + return url; + } + const srcFolder = this.config.ui5.paths.src; + const testFolder = this.config.ui5.paths.test; + const resourcesSrcPattern = new RegExp( + `/base/${this.replaceLast(srcFolder, "resources")}/` + ); + const resourcesTestPattern = new RegExp( + `/base/${this.replaceLast(testFolder, "resources")}/` + ); + const testResourcesSrcPattern = new RegExp( + `/base/${this.replaceLast(srcFolder, "test-resources")}/` + ); + const testResourcesTestPattern = new RegExp( + `/base/${this.replaceLast(testFolder, "test-resources")}/` + ); + if (resourcesSrcPattern.test(url)) { + return url.replace(resourcesSrcPattern, `/base/${srcFolder}/`); + } else if (resourcesTestPattern.test(url)) { + return url.replace(resourcesTestPattern, `/base/${srcFolder}/`); + } else if (testResourcesSrcPattern.test(url)) { + return url.replace(testResourcesSrcPattern, `/base/${testFolder}/`); + } else if (testResourcesTestPattern.test(url)) { + return url.replace(testResourcesTestPattern, `/base/${testFolder}/`); + } + return url; } async setupUI5Server({basePath, configPath}) { @@ -447,8 +480,7 @@ class Framework { all }; - // eslint-disable-next-line new-cap - const router = require("express").Router(); + const router = new Router(); // TODO: rework ui5-server API and make public const MiddlewareManager = require("@ui5/server/lib/middleware/MiddlewareManager"); @@ -459,9 +491,7 @@ class Framework { await middlewareManager.applyMiddleware(router); - return { - serveResources: router - }; + return router; } setupProxy({url}) { @@ -474,50 +504,42 @@ class Framework { agent }); - return { - serveResources: (req, res, next) => proxy.web(req, res, next), - serveThemes: undefined - }; + return (req, res, next) => proxy.web(req, res, next); + } + + beforeMiddlewareRewriteUrl(req, res, next) { + req.url = this.rewriteUrlBefore(req.url); + next(); + } + + middlewareRewriteUrl(req, res, next) { + req.url = this.rewriteUrl(req.url); + next(); } async setupMiddleware() { const config = this.config; - let server = { - serveResources: undefined, - serveThemes: undefined - }; + if (config.ui5.type === "library") { + this.beforeMiddleware.use(this.beforeMiddlewareRewriteUrl.bind(this)); + config.beforeMiddleware.push("ui5--beforeMiddleware"); + } + let middleware; if (config.ui5.url) { - config.middleware.push("ui5--serveResources"); - server = await this.setupProxy(config.ui5); + middleware = this.setupProxy(config.ui5); } else if (config.ui5.useMiddleware !== false) { - config.beforeMiddleware.push("ui5--pauseRequests"); - config.middleware.push("ui5--serveResources"); - // config.middleware.push("ui5--serveThemes"); - server = await this.setupUI5Server({ + middleware = await this.setupUI5Server({ basePath: config.basePath, configPath: config.ui5.configPath }); } - this._serveResources = server.serveResources; - this._serveThemes = server.serveThemes; - this.processRequests(); - return this; - } - - serveResources() { - return (req, res, next) => { - req.url = this.rewriteUrl(req.url); - this._serveResources(req, res, next); - }; - } - - serveThemes() { - return (req, res, next) => { - this._serveThemes(req, res, next); - }; + if (middleware) { + this.middleware.use(this.middlewareRewriteUrl.bind(this)); + this.middleware.use(middleware); + config.middleware.push("ui5--middleware"); + } } } diff --git a/lib/index.js b/lib/index.js index 2d829731..967f2e19 100644 --- a/lib/index.js +++ b/lib/index.js @@ -7,8 +7,7 @@ async function init(config, logger) { await framework.init({config, logger}); } catch (error) { const _logger = logger.create("ui5.framework"); - _logger.log("error", error.message); - _logger.log("debug", error.stack); + _logger.log("error", error.stack); throw new Error(ErrorMessage.failure()); } } @@ -17,13 +16,6 @@ init.$inject = ["config", "logger"]; module.exports = { "framework:ui5": ["factory", init], - "middleware:ui5--pauseRequests": ["factory", function() { - return framework.pauseRequests(); - }], - "middleware:ui5--serveResources": ["factory", function() { - return framework.serveResources(); - }], - "middleware:ui5--serveThemes": ["factory", function() { - return framework.serveThemes(); - }] + "middleware:ui5--beforeMiddleware": ["value", framework.beforeMiddleware], + "middleware:ui5--middleware": ["value", framework.middleware] }; diff --git a/test/unit/framework.test.js b/test/unit/framework.test.js index 11cccc47..62df08e0 100644 --- a/test/unit/framework.test.js +++ b/test/unit/framework.test.js @@ -17,130 +17,78 @@ const logger = { }; describe("Middleware for UI5", () => { - it("Should pause requests during UI5 server setup and resume once ready", async () => { + it("Should rewrite url in beforeMiddleware (library only)", async () => { let resolve; const donePromise = new Promise((_resolve) => { resolve = _resolve; }); - expect.assertions(7); + expect.assertions(4); const config = { ui5: { - useMiddleware: true + type: "library" } }; const framework = new Framework(); framework.exists = () => true; - const initPromise = framework.init({config, logger}); - - expect(config["beforeMiddleware"]).toContain("ui5--pauseRequests"); - expect(framework.isPaused).toBe(true); + await framework.init({config, logger}); + expect(config["middleware"]).toContain("ui5--middleware"); + expect(config["beforeMiddleware"]).toContain("ui5--beforeMiddleware"); - const processRequestsSpy = jest.spyOn(framework, "processRequests"); + const rewriteUrlBeforeSpy = jest.spyOn(framework, "rewriteUrlBefore"); - const pauseRequestsMiddleware = framework.pauseRequests(); - pauseRequestsMiddleware({}, {}, function() { - expect(processRequestsSpy).toBeCalled(); - expect(framework.isPaused).toBe(false); + const beforeMiddleware = framework.beforeMiddleware; - setTimeout(function() { - // Queue should be empty after paused requests have been called - expect(framework.queue).toHaveLength(0); + const req = { + url: "/foo" + }; - // New requests shouldn't be queued anymore - pauseRequestsMiddleware({}, {}, function() { - expect(framework.queue).toHaveLength(0); - resolve(); - }); - }, 0); + beforeMiddleware(req, {}, function() { + expect(rewriteUrlBeforeSpy).toBeCalledWith("/foo"); + expect(req.url).toBe("/foo"); + resolve(); }); - expect(framework.queue).toHaveLength(1); - - await initPromise; return donePromise; }); - it("Should rewrite url in serveResources middleware", async () => { + it("Should rewrite url in middleware", async () => { let resolve; const donePromise = new Promise((_resolve) => { resolve = _resolve; }); - expect.assertions(6); + expect.assertions(3); const config = { ui5: { - type: "application", - useMiddleware: true + type: "application" } }; const framework = new Framework(); framework.exists = () => true; - const initPromise = framework.init({config, logger}); - expect(config["middleware"]).toContain("ui5--serveResources"); - expect(framework.isPaused).toBe(true); + await framework.init({config, logger}); + expect(config["middleware"]).toContain("ui5--middleware"); const rewriteUrlSpy = jest.spyOn(framework, "rewriteUrl"); - const pauseRequestsMiddleware = framework.pauseRequests(); - const serveResourcesMiddleware = framework.serveResources(); - - pauseRequestsMiddleware({}, {}, function() { - expect(framework.isPaused).toBe(false); - const internalServeResourcesSpy = jest.spyOn(framework, "_serveResources"); - - const req = {url: "/foo"}; - const res = {}; - const next = function() { - expect(internalServeResourcesSpy).toBeCalledWith(req, res, next); - expect(rewriteUrlSpy).toBeCalledWith("/foo"); - expect(req.url).toBe("/foo"); - resolve(); - }; - serveResourcesMiddleware(req, res, next); - }); - - await initPromise; + const middleware = framework.middleware; - return donePromise; - }); - - it.skip("Should not rewrite url in serveThemes middleware", async (done) => { - const config = { - ui5: { - useMiddleware: true - } + const req = { + url: "/foo" }; - const framework = new Framework(); - framework.exists = () => true; - await framework.init({config, logger}); - expect(config["middleware"]).toContain("ui5--serveThemes"); - expect(framework.isPaused).toBe(true); - const rewriteUrlSpy = jest.spyOn(framework, "rewriteUrl"); - - const pauseRequestsMiddleware = framework.pauseRequests(); - const serveThemesMiddleware = framework.serveThemes(); - - pauseRequestsMiddleware({}, {}, function() { - expect(framework.isPaused).toBe(false); - const internalServeThemesSpy = jest.spyOn(framework, "_serveThemes"); - - const req = {url: "/foo"}; - const res = {}; - const next = function() { - expect(internalServeThemesSpy).toBeCalledWith(req, res, next); - expect(rewriteUrlSpy).not.toBeCalled(); - expect(req.url).toBe("/foo"); - done(); - }; - serveThemesMiddleware(req, res, next); + middleware(req, {}, function() { + expect(rewriteUrlSpy).toBeCalledWith("/foo"); + expect(req.url).toBe("/foo"); + resolve(); }); + + return donePromise; }); }); describe("Proxy for UI5 ", () => { - it("Should call proxy module from serveResources middleware (http)", (done) => { + it("Should call proxy module from middleware (http)", (done) => { const proxyServer = new Framework().setupProxy({ url: "http://localhost" }); @@ -159,18 +107,16 @@ describe("Proxy for UI5 ", () => { // const proxy = require("http-proxy").createProxyServer.mock.results[0].value; - expect(proxyServer.serveThemes).toBeUndefined(); - const req = {}; const res = {}; const next = function() { // expect(proxy.web).toBeCalledWith(req, res, next); // TODO: check why this fails done(); }; - proxyServer.serveResources(req, res, next); + proxyServer(req, res, next); }); - it("Should call proxy module from serveResources middleware (https)", (done) => { + it("Should call proxy module from middleware (https)", (done) => { const proxyServer = new Framework().setupProxy({ url: "https://localhost" }); @@ -189,15 +135,13 @@ describe("Proxy for UI5 ", () => { // const proxy = require("http-proxy").createProxyServer.mock.results[0].value; - expect(proxyServer.serveThemes).toBeUndefined(); - const req = {}; const res = {}; const next = function() { // expect(proxy.web).toBeCalledWith(req, res, next); // TODO: check why this fails done(); }; - proxyServer.serveResources(req, res, next); + proxyServer(req, res, next); }); }); @@ -387,7 +331,7 @@ describe("ui5.paths handling", () => { }); }); -describe("Utility functions", () => { +describe("rewriteUrl", () => { const framework = new Framework(); framework.exists = () => true; framework.init({config: { }, logger}); @@ -531,6 +475,135 @@ describe("Utility functions", () => { }); }); +describe("rewriteUrlBefore", () => { + const framework = new Framework(); + framework.exists = () => true; + framework.init({config: { }, logger}); + + const assertRewriteUrlBefore = ([input, expected]) => { + expect(framework.rewriteUrlBefore(input)).toEqual(expected); + }; + + it("Should rewrite url for library", async () => { + framework.config.ui5.type = "library"; + + // Good path + assertRewriteUrlBefore([ + "/base/resources/sap-ui-core.js", + "/base/src/sap-ui-core.js", + ]); + assertRewriteUrlBefore([ + "/base/test-resources/sap/ui/test/", + "/base/test/sap/ui/test/" + ]); + + // Sad path (no rewrite) + assertRewriteUrlBefore([ + "/base/src/sap-ui-core.js", + "/base/src/sap-ui-core.js" + ]); + assertRewriteUrlBefore([ + "/base/test/sap/ui/test/", + "/base/test/sap/ui/test/" + ]); + }); + + it("Should rewrite url for library (nested paths)", async () => { + framework.config.ui5.type = "library"; + framework.config.ui5.paths = { + src: "src/main/js", + test: "src/test/js" + }; + + // Good path + assertRewriteUrlBefore([ + "/base/src/main/resources/sap-ui-core.js", + "/base/src/main/js/sap-ui-core.js", + ]); + assertRewriteUrlBefore([ + "/base/src/test/test-resources/sap/ui/test/", + "/base/src/test/js/sap/ui/test/" + ]); + + assertRewriteUrlBefore([ + "/base/src/test/resources/sap-ui-core.js", + "/base/src/main/js/sap-ui-core.js", + ]); + assertRewriteUrlBefore([ + "/base/src/main/test-resources/sap/ui/test/", + "/base/src/test/js/sap/ui/test/" + ]); + + // Sad path (no rewrite) + assertRewriteUrlBefore([ + "/base/src/main/js/sap-ui-core.js", + "/base/src/main/js/sap-ui-core.js" + ]); + assertRewriteUrlBefore([ + "/base/src/test/js/sap/ui/test/", + "/base/src/test/js/sap/ui/test/" + ]); + }); + + it("Should not rewrite url for type application", async () => { + framework.config.ui5.type = "application"; + + assertRewriteUrlBefore([ + "/base/webapp/resources/sap-ui-core.js", + "/base/webapp/resources/sap-ui-core.js" + ]); + assertRewriteUrlBefore([ + "/base/webapp/test-resources/sap/ui/test/", + "/base/webapp/test-resources/sap/ui/test/" + ]); + assertRewriteUrlBefore([ + "/base/webapp/foo.js", + "/base/webapp/foo.js" + ]); + assertRewriteUrlBefore([ + "/base/src/sap-ui-core.js", + "/base/src/sap-ui-core.js" + ]); + assertRewriteUrlBefore([ + "/base/test/sap/ui/test/", + "/base/test/sap/ui/test/" + ]); + assertRewriteUrlBefore([ + "/base/foo.js", + "/base/foo.js" + ]); + }); + + it("Should not rewrite url when no type is given", async () => { + framework.config.ui5.type = undefined; + + assertRewriteUrlBefore([ + "/base/webapp/resources/sap-ui-core.js", + "/base/webapp/resources/sap-ui-core.js" + ]); + assertRewriteUrlBefore([ + "/base/webapp/test-resources/sap/ui/test/", + "/base/webapp/test-resources/sap/ui/test/" + ]); + assertRewriteUrlBefore([ + "/base/webapp/foo.js", + "/base/webapp/foo.js" + ]); + assertRewriteUrlBefore([ + "/base/src/sap-ui-core.js", + "/base/src/sap-ui-core.js" + ]); + assertRewriteUrlBefore([ + "/base/test/sap/ui/test/", + "/base/test/sap/ui/test/" + ]); + assertRewriteUrlBefore([ + "/base/foo.js", + "/base/foo.js" + ]); + }); +}); + describe("Plugin setup", () => { it("Should include browser bundle", async () => { const config = { @@ -619,9 +692,6 @@ describe("Types configuration", () => { await framework.init({config, logger}); expect(config.files.find((file) => file.pattern.endsWith("/{src/**,src/**/.*}"))).toBeDefined(); expect(config.files.find((file) => file.pattern.endsWith("/{test/**,test/**/.*}"))).toBeDefined(); - - expect(config.proxies["/base/resources/"]).toEqual("/base/src/"); - expect(config.proxies["/base/test-resources/"]).toEqual("/base/test/"); }); // TODO: What should happen?