fix(rest): correctly handle basePath set via basePath() API

- Fix `server.basePath()` method to update `basePath` in the config object too. - This fixes the server url in openapi.json to include basePath again. - Add more test to verify the changes and prevent future regressions Signed-off-by: Miroslav Bajtoš <[email protected]>
loopbackio · Jun 28, 2019 · 89913aa · 89913aa
1 parent c50bedd
commit 89913aa
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 2 deletions.
diff --git a/packages/rest/src/__tests__/integration/request-context.integration.ts b/packages/rest/src/__tests__/integration/request-context.integration.ts
@@ -88,6 +88,20 @@ describe('RequestContext', () => {
 
       expect(observedCtx.basePath).to.equal('/api/v1');
     });
+
+    it('honors basePath from server config', async () => {
+      await givenRunningAppWithClient({basePath: '/api'});
+      await client.get('/api/products').expect(200);
+      expect(observedCtx.basePath).to.equal('/api');
+    });
+
+    it('honors basePath set via basePath() method', async () => {
+      await givenRunningAppWithClient({}, a => {
+        a.restServer.basePath('/api');
+      });
+      await client.get('/api/products').expect(200);
+      expect(observedCtx.basePath).to.equal('/api');
+    });
   });
 
   describe('requestedBaseUrl', () => {
@@ -142,12 +156,16 @@ async function teardown() {
   if (app) await app.stop();
 }
 
-async function givenRunningAppWithClient(restOptions?: RestServerConfig) {
+async function givenRunningAppWithClient(
+  restOptions?: RestServerConfig,
+  setupFn: (app: RestApplication) => void = () => {},
+) {
   const options: ApplicationConfig = {
     rest: givenHttpServerConfig(restOptions),
   };
   app = new RestApplication(options);
   app.handler(contextObservingHandler);
+  setupFn(app);
   await app.start();
   client = createRestAppClient(app);
 }

diff --git a/packages/rest/src/__tests__/integration/rest.server.integration.ts b/packages/rest/src/__tests__/integration/rest.server.integration.ts
@@ -854,6 +854,14 @@ paths:
       expect(response.body.servers).to.containEql({url: '/api'});
     });
 
+    it('controls server urls even when set via server.basePath() API', async () => {
+      server.basePath('/v2');
+      const response = await createClientForHandler(server.requestHandler).get(
+        '/openapi.json',
+      );
+      expect(response.body.servers).to.containEql({url: '/v2'});
+    });
+
     it('controls redirect locations', async () => {
       server.controller(DummyController);
       server.redirect('/page/html', '/html');

diff --git a/packages/rest/src/request-context.ts b/packages/rest/src/request-context.ts
@@ -38,7 +38,12 @@ export class RequestContext extends Context implements HandlerContext {
     const request = this.request;
     let basePath = this.serverConfig.basePath || '';
     if (request.baseUrl && request.baseUrl !== '/') {
-      basePath = request.baseUrl + basePath;
+      if (!basePath || request.baseUrl.endsWith(basePath)) {
+        // Express has already applied basePath to baseUrl
+        basePath = request.baseUrl;
+      } else {
+        basePath = request.baseUrl + basePath;
+      }
     }
     return basePath;
   }

diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts
@@ -776,6 +776,7 @@ export class RestServer extends Context implements Server, HttpServerLike {
     path = path.replace(/(^\/)|(\/$)/, '');
     if (path) path = '/' + path;
     this._basePath = path;
+    this.config.basePath = path;
   }
 
   /**