fix: fix bugs when recursively appending middleware (#476)

Author: AllanZhengYP

packages/middleware-stack/src/MiddlewareStack.spec.ts

@@ -97,59 +97,83 @@ describe("MiddlewareStack", () => {
   it("should allow adding middleware relatively", async () => {
     const stack = new MiddlewareStack<input, output>();
     type MW = InitializeMiddleware<input, output>;
-    stack.addRelativeTo(getConcatMiddleware("G") as MW, {
-      name: "G",
+    stack.addRelativeTo(getConcatMiddleware("H") as MW, {
+      name: "H",
       relation: "after",
-      toMiddleware: "F"
+      toMiddleware: "G"
     });
     stack.addRelativeTo(getConcatMiddleware("A") as MW, {
       name: "A",
       relation: "after",
       toMiddleware: "nonExist"
     });
-    stack.addRelativeTo(getConcatMiddleware("B") as MW, {
-      name: "B",
-      relation: "after",
-      toMiddleware: "A"
-    });
     stack.addRelativeTo(getConcatMiddleware("C") as MW, {
       name: "C",
       relation: "after",
       toMiddleware: "A"
     });
-    stack.add(getConcatMiddleware("F") as MW, {
-      name: "F",
-      priority: "low"
+    stack.addRelativeTo(getConcatMiddleware("B") as MW, {
+      name: "B",
+      relation: "after",
+      toMiddleware: "A"
     });
     stack.addRelativeTo(getConcatMiddleware("D") as MW, {
       name: "D",
-      relation: "before",
-      toMiddleware: "F"
+      relation: "after",
+      toMiddleware: "C"
+    });
+    stack.add(getConcatMiddleware("G") as MW, {
+      name: "G",
+      priority: "low"
     });
     stack.addRelativeTo(getConcatMiddleware("E") as MW, {
       name: "E",
       relation: "before",
       toMiddleware: "F"
     });
-    stack.addRelativeTo(getConcatMiddleware("H") as MW, {
-      name: "H",
+    stack.addRelativeTo(getConcatMiddleware("F") as MW, {
+      name: "F",
       relation: "before",
-      toMiddleware: "I"
+      toMiddleware: "G"
     });
     stack.addRelativeTo(getConcatMiddleware("I") as MW, {
       name: "I",
+      relation: "before",
+      toMiddleware: "J"
+    });
+    stack.addRelativeTo(getConcatMiddleware("J") as MW, {
+      name: "J",
       relation: "after",
-      toMiddleware: "H"
+      toMiddleware: "I"
     });
     const inner = jest.fn();
 
     const composed = stack.resolve(inner, {} as any);
     await composed({ input: [] });
 
     expect(inner.mock.calls.length).toBe(1);
-    expect(inner).toBeCalledWith({
-      input: ["A", "B", "C", "H", "I", "D", "E", "F", "G"]
-    });
+    const concatenated = inner.mock.calls[0][0].input;
+    expect(concatenated.indexOf("H")).toBeGreaterThan(
+      concatenated.indexOf("G")
+    );
+    expect(concatenated.indexOf("C")).toBeGreaterThan(
+      concatenated.indexOf("A")
+    );
+    expect(concatenated.indexOf("B")).toBeGreaterThan(
+      concatenated.indexOf("A")
+    );
+    expect(concatenated.indexOf("D")).toBeGreaterThan(
+      concatenated.indexOf("C")
+    );
+    expect(concatenated.indexOf("E")).toBeLessThan(concatenated.indexOf("F"));
+    expect(concatenated.indexOf("F")).toBeLessThan(concatenated.indexOf("G"));
+    expect(concatenated.indexOf("I")).toBeLessThan(concatenated.indexOf("J"));
+    expect(concatenated.indexOf("J")).toBeGreaterThan(
+      concatenated.indexOf("I")
+    );
+    expect(concatenated.indexOf("H")).toBeGreaterThan(
+      concatenated.indexOf("G")
+    );
   });
 
   it("should allow cloning", async () => {
@@ -187,26 +211,26 @@ describe("MiddlewareStack", () => {
 
   it("should allow combining stacks", async () => {
     const stack = new MiddlewareStack<input, output>();
-    stack.add(getConcatMiddleware("second") as InitializeMiddleware<
+    stack.add(getConcatMiddleware("first") as InitializeMiddleware<
       input,
       output
     >);
     stack.add(
-      getConcatMiddleware("first") as InitializeMiddleware<input, output>,
+      getConcatMiddleware("second") as InitializeMiddleware<input, output>,
       {
-        name: "first",
-        priority: "high"
+        name: "second",
+        priority: "low"
       }
     );
 
     const secondStack = new MiddlewareStack<input, output>();
     secondStack.add(
       getConcatMiddleware("fourth") as FinalizeRequestMiddleware<input, output>,
-      { step: "build" }
+      { step: "build", priority: "low" }
     );
-    secondStack.add(
+    secondStack.addRelativeTo(
       getConcatMiddleware("third") as FinalizeRequestMiddleware<input, output>,
-      { step: "build", priority: "high" }
+      { step: "build", relation: "after", toMiddleware: "second" }
     );
 
     let inner = jest.fn(({ input }: DeserializeHandlerArguments<input>) => {
@@ -218,11 +242,11 @@ describe("MiddlewareStack", () => {
     expect(inner.mock.calls.length).toBe(1);
 
     secondStack.add(
-      getConcatMiddleware("first") as FinalizeRequestMiddleware<input, output>,
-      { step: "build", priority: "high", name: "first" }
+      getConcatMiddleware("second") as FinalizeRequestMiddleware<input, output>,
+      { step: "build", priority: "high", name: "second" }
     );
     expect(() => stack.concat(secondStack)).toThrow(
-      "Duplicated middleware name 'first'"
+      "Duplicated middleware name 'second'"
     );
   });
 

packages/middleware-stack/src/MiddlewareStack.ts

@@ -147,14 +147,12 @@ export class MiddlewareStack<Input extends object, Output extends object> {
   > {
     //reverse before sorting so that middleware of same step will execute in
     //the order of being added
-    return entries
-      .reverse()
-      .sort(
-        (a, b) =>
-          stepWeights[a.step] - stepWeights[b.step] ||
-          priorityWeights[a.priority || "normal"] -
-            priorityWeights[b.priority || "normal"]
-      );
+    return entries.sort(
+      (a, b) =>
+        stepWeights[b.step] - stepWeights[a.step] ||
+        priorityWeights[b.priority || "normal"] -
+          priorityWeights[a.priority || "normal"]
+    );
   }
 
   clone(): IMiddlewareStack<Input, Output> {
@@ -405,24 +403,28 @@ export class MiddlewareStack<Input extends object, Output extends object> {
       const { prev, next } = entry.name
         ? anchors[entry.name] || defaultAnchorValue
         : defaultAnchorValue;
-      let relativeEntry = next;
+      let relativeEntry = prev;
+      //reverse relative entry linked list and add to ordered handler list
+      while (relativeEntry && relativeEntry.prev) {
+        relativeEntry = relativeEntry.prev;
+      }
       while (relativeEntry) {
         middlewareList.push(relativeEntry.middleware);
         relativeEntry = relativeEntry.next;
       }
+      middlewareList.push(entry.middleware);
       let orphanedEntry = entry as any;
       while ((orphanedEntry as any).next) {
         middlewareList.push((orphanedEntry as any).next.middleware);
         orphanedEntry = (orphanedEntry as any).next;
       }
-      middlewareList.push(entry.middleware);
-      relativeEntry = prev;
+      relativeEntry = next;
       while (relativeEntry) {
         middlewareList.push(relativeEntry.middleware);
-        relativeEntry = relativeEntry.prev;
+        relativeEntry = relativeEntry.next;
       }
     }
-    return middlewareList;
+    return middlewareList.reverse();
   }
 
   resolve<InputType extends Input, OutputType extends Output>(