Skip to content

Commit

Permalink
[FEATURE] Custom Middleware Extensibility: Allow multiple definitions…
Browse files Browse the repository at this point in the history
… of the same custom middleware (#246)

Similar to the implementation for custom tasks:
https://github.com/SAP/ui5-builder/blob/d60c67dafc6d96a8345da2004972cd4b839eece6/lib/types/AbstractBuilder.js#L81-L89

Resolves #222
  • Loading branch information
RandomByte authored Oct 14, 2019
1 parent 648b278 commit 55a24ef
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 21 deletions.
24 changes: 15 additions & 9 deletions lib/middleware/MiddlewareManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,27 @@ class MiddlewareManager {
});
}

async addMiddleware(middlewareName, {
async addMiddleware(configuredMiddlewareName, {
wrapperCallback, mountPath = "/",
beforeMiddleware, afterMiddleware
} = {}) {
let middlewareCallback = middlewareRepository.getMiddleware(middlewareName);
let middlewareCallback = middlewareRepository.getMiddleware(configuredMiddlewareName);
if (wrapperCallback) {
middlewareCallback = wrapperCallback(middlewareCallback);
}
if (this.middleware[middlewareName] || this.middlewareExecutionOrder.includes(middlewareName)) {
throw new Error(`Failed to add duplicate middleware ${middlewareName}`);

let middlewareName = configuredMiddlewareName;
if (this.middleware[middlewareName]) {
// Middleware is already known
// => add a suffix to allow for multiple configurations of the same middleware
let suffixCounter = 0;
while (this.middleware[middlewareName]) {
suffixCounter++; // Start at 1
middlewareName = `${configuredMiddlewareName}--${suffixCounter}`;
}
}
if (this.middlewareExecutionOrder.includes(middlewareName)) {
throw new Error(`Middleware ${middlewareName} already added to execution order. This should not happen.`);
}

if (beforeMiddleware || afterMiddleware) {
Expand Down Expand Up @@ -154,11 +165,6 @@ class MiddlewareManager {
`defines neither a "beforeMiddleware" nor an "afterMiddleware" parameter. One must be defined.`);
}

if (this.middleware[middlewareDef.name]) {
// Middleware is already known
throw new Error(`Failed to add custom middleware ${middlewareDef.name}. ` +
`A middleware with the same name is already known.`);
}
await this.addMiddleware(middlewareDef.name, {
wrapperCallback: (middleware) => {
return ({resources}) => {
Expand Down
60 changes: 48 additions & 12 deletions test/lib/server/middleware/MiddlewareManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test("applyMiddleware", async (t) => {
t.deepEqual(appUseStub.getCall(0).args[1], "myMiddleware", "app.use got called with correct middleware parameter");
});

test("addMiddleware: Add already added middleware", async (t) => {
test("addMiddleware: Adding already added middleware produces unique middleware name", async (t) => {
const middlewareManager = new MiddlewareManager({
tree: {},
resources: {
Expand All @@ -68,11 +68,50 @@ test("addMiddleware: Add already added middleware", async (t) => {
}
});

await middlewareManager.addMiddleware("serveIndex");
await middlewareManager.addMiddleware("serveIndex", {
mountPath: "/pony"
});
await middlewareManager.addMiddleware("serveIndex", {
mountPath: "/seagull"
});
await middlewareManager.addMiddleware("serveIndex", {
mountPath: "/goose"
});
t.truthy(middlewareManager.middleware["serveIndex"], "Middleware got added to internal map with unique name");
t.deepEqual(middlewareManager.middleware["serveIndex"].mountPath, "/pony",
"Middleware got added correct mount path");
t.truthy(middlewareManager.middleware["serveIndex--1"], "Middleware got added to internal map with unique name");
t.deepEqual(middlewareManager.middleware["serveIndex--1"].mountPath, "/seagull",
"Middleware got added correct mount path");
t.truthy(middlewareManager.middleware["serveIndex--2"], "Middleware got added to internal map with unique name");
t.deepEqual(middlewareManager.middleware["serveIndex--2"].mountPath, "/goose",
"Middleware got added correct mount path");

t.deepEqual(middlewareManager.middlewareExecutionOrder, [
"serveIndex",
"serveIndex--1",
"serveIndex--2"
], "Middlewares got added to middlewareExecutionOrder in correct order and with correct unique names");
});

test("addMiddleware: Adding middleware already added to middlewareExecutionOrder", async (t) => {
const middlewareManager = new MiddlewareManager({
tree: {},
resources: {
all: "I",
rootProject: "like",
dependencies: "ponies"
}
});

middlewareManager.middlewareExecutionOrder.push("serveIndex");

const err = await t.throwsAsync(() => {
return middlewareManager.addMiddleware("serveIndex");
});
t.deepEqual(err.message, "Failed to add duplicate middleware serveIndex", "Rejected with correct error message");
t.deepEqual(err.message,
"Middleware serveIndex already added to execution order. This should not happen.",
"Rejected with correct error message");
});

test("addMiddleware: Add middleware", async (t) => {
Expand Down Expand Up @@ -330,15 +369,15 @@ test("addCustomMiddleware: Custom middleware got added", async (t) => {
"addMiddleware was called with correct afterMiddleware option");
});

test("addCustomMiddleware: Custom middleware with duplicate name", async (t) => {
test("addCustomMiddleware: No special handling for custom middleware with duplicate name", async (t) => {
const project = {
metadata: {
name: "my project"
},
server: {
customMiddleware: [{
name: "my custom middleware A",
afterMiddleware: "my custom middleware A"
afterMiddleware: "serveIndex"
}]
}
};
Expand All @@ -352,14 +391,11 @@ test("addCustomMiddleware: Custom middleware with duplicate name", async (t) =>
});
middlewareManager.middleware["my custom middleware A"] = true;
const addMiddlewareStub = sinon.stub(middlewareManager, "addMiddleware").resolves();
const err = await t.throwsAsync(() => {
return middlewareManager.addCustomMiddleware();
});
await middlewareManager.addCustomMiddleware();

t.deepEqual(err.message, "Failed to add custom middleware my custom middleware A. " +
"A middleware with the same name is already known.",
"Rejected with correct error message");
t.deepEqual(addMiddlewareStub.callCount, 0, "Add middleware did not get called");
t.deepEqual(addMiddlewareStub.callCount, 1, "addMiddleware was called once");
t.deepEqual(addMiddlewareStub.getCall(0).args[0], "my custom middleware A",
"addMiddleware was called with correct middleware name");
});

test("addCustomMiddleware: Missing name configuration", async (t) => {
Expand Down

0 comments on commit 55a24ef

Please sign in to comment.