Merge pull request #14495 from nestjs/fix/execute-global-middleware-f…

…irst fix(core): global module middleware should be executed first
nestjs · Jan 23, 2025 · d9199da · d9199da
2 parents d56553a + 44490dc
commit d9199da
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 16 deletions.
diff --git a/integration/hello-world/e2e/middleware-execute-order.spec.ts b/integration/hello-world/e2e/middleware-execute-order.spec.ts
@@ -1,20 +1,46 @@
-import { INestApplication, MiddlewareConsumer, Module } from '@nestjs/common';
+import {
+  Global,
+  INestApplication,
+  MiddlewareConsumer,
+  Module,
+} from '@nestjs/common';
 import { Test } from '@nestjs/testing';
 import * as request from 'supertest';
 
 const RETURN_VALUE_A = 'test_A';
 const RETURN_VALUE_B = 'test_B';
+const RETURN_VALUE_X = 'test_X';
+const RETURN_VALUE_GLOBAL = 'test_GLOBAL';
 
-@Module({
-  imports: [],
-})
+@Global()
+@Module({})
+class GlobalModule {
+  configure(consumer: MiddlewareConsumer) {
+    consumer
+      .apply((req, res, next) => res.send(RETURN_VALUE_GLOBAL))
+      .forRoutes('ping');
+  }
+}
+
+@Module({ imports: [GlobalModule] })
+class ModuleX {
+  configure(consumer: MiddlewareConsumer) {
+    consumer
+      .apply((req, res, next) => res.send(RETURN_VALUE_X))
+      .forRoutes('hello')
+      .apply((req, res, next) => res.send(RETURN_VALUE_X))
+      .forRoutes('ping');
+  }
+}
+
+@Module({ imports: [ModuleX] })
 class ModuleA {
   configure(consumer: MiddlewareConsumer) {
     consumer
-      .apply((req, res, next) => {
-        res.send(RETURN_VALUE_A);
-      })
-      .forRoutes('hello');
+      .apply((req, res, next) => res.send(RETURN_VALUE_A))
+      .forRoutes('hello')
+      .apply((req, res, next) => res.send(RETURN_VALUE_A))
+      .forRoutes('ping');
   }
 }
 
@@ -24,10 +50,10 @@ class ModuleA {
 class ModuleB {
   configure(consumer: MiddlewareConsumer) {
     consumer
-      .apply((req, res, next) => {
-        res.send(RETURN_VALUE_B);
-      })
-      .forRoutes('hello');
+      .apply((req, res, next) => res.send(RETURN_VALUE_B))
+      .forRoutes('hello')
+      .apply((req, res, next) => res.send(RETURN_VALUE_B))
+      .forRoutes('ping');
   }
 }
 
@@ -55,6 +81,12 @@ describe('Middleware (execution order)', () => {
       .expect(200, RETURN_VALUE_B);
   });
 
+  it('should execute global middleware first', () => {
+    return request(app.getHttpServer())
+      .get('/ping')
+      .expect(200, RETURN_VALUE_GLOBAL);
+  });
+
   afterEach(async () => {
     await app.close();
   });

diff --git a/packages/core/middleware/middleware-module.ts b/packages/core/middleware/middleware-module.ts
@@ -148,10 +148,16 @@ export class MiddlewareModule<
 
     const entriesSortedByDistance = [...configs.entries()].sort(
       ([moduleA], [moduleB]) => {
-        return (
-          this.container.getModuleByKey(moduleA)!.distance -
-          this.container.getModuleByKey(moduleB)!.distance
-        );
+        const moduleARef = this.container.getModuleByKey(moduleA)!;
+        const moduleBRef = this.container.getModuleByKey(moduleB)!;
+        if (moduleARef.distance === Number.MAX_VALUE) {
+          return -1;
+        }
+        if (moduleBRef.distance === Number.MAX_VALUE) {
+          return 1;
+        }
+
+        return moduleARef.distance - moduleBRef.distance;
       },
     );
     for (const [moduleRef, moduleConfigurations] of entriesSortedByDistance) {