Fix paths for routes within Express routers (#45)

* Fix paths for routes within Express routers

* Fix

Author: itssimon

src/express/listEndpoints.js

@@ -216,14 +216,14 @@ const parseStack = function (stack, basePath, endpoints) {
   return endpoints;
 };
 
-const getEndpoints = function (app) {
+const getEndpoints = function (app, basePath) {
   const endpoints = parseEndpoints(app);
   return endpoints.flatMap((route) =>
     route.methods
       .filter((method) => !["HEAD", "OPTIONS"].includes(method.toUpperCase()))
       .map((method) => ({
         method,
-        path: route.path,
+        path: basePath + route.path,
       })),
   );
 };

src/express/middleware.ts

@@ -1,4 +1,6 @@
-import type { Express, NextFunction, Request, Response, Router } from "express";
+import type { Express, NextFunction, Request, Response } from "express";
+import { Router } from "express";
+import type { ILayer } from "express-serve-static-core";
 import { performance } from "perf_hooks";
 
 import { ApitallyClient } from "../common/client.js";
@@ -19,12 +21,15 @@ declare module "express" {
   }
 }
 
-export const useApitally = (app: Express | Router, config: ApitallyConfig) => {
+export const useApitally = (
+  app: Express | Router,
+  config: ApitallyConfig & { basePath?: string },
+) => {
   const client = new ApitallyClient(config);
   const middleware = getMiddleware(app, client);
   app.use(middleware);
   setTimeout(() => {
-    client.setStartupData(getAppInfo(app, config.appVersion));
+    client.setStartupData(getAppInfo(app, config.basePath, config.appVersion));
   }, 1000);
 };
 
@@ -55,14 +60,15 @@ const getMiddleware = (app: Express | Router, client: ApitallyClient) => {
       };
       res.on("finish", () => {
         try {
-          if (req.route) {
+          const path = getRoutePath(req);
+          if (path) {
             const responseTime = performance.now() - startTime;
             const consumer = getConsumer(req);
             client.consumerRegistry.addOrUpdateConsumer(consumer);
             client.requestCounter.addRequest({
               consumer: consumer?.identifier,
               method: req.method,
-              path: req.route.path,
+              path,
               statusCode: res.statusCode,
               responseTime: responseTime,
               requestSize: req.get("content-length"),
@@ -128,6 +134,25 @@ const getMiddleware = (app: Express | Router, client: ApitallyClient) => {
   };
 };
 
+const getRoutePath = (req: Request) => {
+  if (!req.route) {
+    return;
+  }
+  if (req.baseUrl) {
+    const router = req.app._router.stack.findLast((layer: ILayer) => {
+      return layer.name === "router" && layer.regexp.test(req.baseUrl);
+    });
+    if (router && router.path) {
+      if (Object.keys(router.params).length > 0) {
+        // Routers mounted with path parameters are not supported yet
+        return;
+      }
+      return router.path + req.route.path;
+    }
+  }
+  return req.route.path;
+};
+
 const getConsumer = (req: Request) => {
   if (req.apitallyConsumer) {
     return consumerFromStringOrObject(req.apitallyConsumer);
@@ -208,6 +233,7 @@ const subsetJoiMessage = (message: string, key: string) => {
 
 const getAppInfo = (
   app: Express | Router,
+  basePath?: string,
   appVersion?: string,
 ): StartupData => {
   const versions: Array<[string, string]> = [
@@ -225,7 +251,7 @@ const getAppInfo = (
     versions.push(["app", appVersion]);
   }
   return {
-    paths: listEndpoints(app),
+    paths: listEndpoints(app, basePath || ""),
     versions: Object.fromEntries(versions),
     client: "js:express",
   };

tests/express/app.test.ts

@@ -6,6 +6,7 @@ import { ApitallyClient } from "../../src/common/client.js";
 import { mockApitallyHub } from "../utils.js";
 import {
   getAppWithCelebrate,
+  getAppWithMiddlewareOnRouter,
   getAppWithRouter,
   getAppWithValidator,
 } from "./app.js";
@@ -137,7 +138,7 @@ describe("Middleware for Express router", () => {
 
   beforeEach(async () => {
     mockApitallyHub();
-    app = getAppWithRouter();
+    app = getAppWithMiddlewareOnRouter();
     appTest = request(app);
     client = ApitallyClient.getInstance();
 
@@ -153,7 +154,55 @@ describe("Middleware for Express router", () => {
     expect(
       requests.some(
         (r) =>
-          r.method === "GET" && r.path === "/hello" && r.status_code === 200,
+          r.method === "GET" &&
+          r.path === "/api/hello" &&
+          r.status_code === 200,
+      ),
+    ).toBe(true);
+  });
+
+  it("List endpoints", async () => {
+    expect(client.startupData?.paths).toEqual([
+      {
+        method: "GET",
+        path: "/api/hello",
+      },
+    ]);
+  });
+
+  afterEach(async () => {
+    if (client) {
+      await client.handleShutdown();
+    }
+  });
+});
+
+describe("Middleware for Express with router", () => {
+  let app: Express;
+  let appTest: request.Agent;
+  let client: ApitallyClient;
+
+  beforeEach(async () => {
+    mockApitallyHub();
+    app = getAppWithRouter();
+    appTest = request(app);
+    client = ApitallyClient.getInstance();
+
+    // Wait for 1.2 seconds for startup data to be set
+    await new Promise((resolve) => setTimeout(resolve, 1200));
+  });
+
+  it("Request logger", async () => {
+    await appTest.get("/api/hello/bob").expect(200);
+
+    const requests = client.requestCounter.getAndResetRequests();
+    expect(requests.length).toBe(1);
+    expect(
+      requests.some(
+        (r) =>
+          r.method === "GET" &&
+          r.path === "/api/hello/:name" &&
+          r.status_code === 200,
       ),
     ).toBe(true);
   });
@@ -162,7 +211,7 @@ describe("Middleware for Express router", () => {
     expect(client.startupData?.paths).toEqual([
       {
         method: "GET",
-        path: "/hello",
+        path: "/api/hello/:name",
       },
     ]);
   });

tests/express/app.ts

@@ -108,14 +108,15 @@ export const getAppWithValidator = () => {
   return app;
 };
 
-export const getAppWithRouter = () => {
+export const getAppWithMiddlewareOnRouter = () => {
   const app = express();
   const router = express.Router();
 
   useApitally(router, {
     clientId: CLIENT_ID,
     env: ENV,
     appVersion: "1.2.3",
+    basePath: "/api",
   });
 
   router.get("/hello", (req, res) => {
@@ -126,3 +127,22 @@ export const getAppWithRouter = () => {
   app.use(errors());
   return app;
 };
+
+export const getAppWithRouter = () => {
+  const app = express();
+  const router = express.Router();
+
+  useApitally(app, {
+    clientId: CLIENT_ID,
+    env: ENV,
+    appVersion: "1.2.3",
+  });
+
+  router.get("/hello/:name", (req, res) => {
+    res.send(`Hello ${req.params.name}!`);
+  });
+
+  app.use("/api", router);
+  app.use(errors());
+  return app;
+};