fix(platform): added prefix to adapters

[uc4w6c]

fixes https://github.com/nestjs/nest/issues/4114

PR Checklist

Please check if your PR fulfills the following requirements:

• [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• [x] Tests for the changes have been added (for bug fixes / features)
• [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

When launching multiple NestApplication instances for a single adapter and using setGlobalPrefix for each, the router of the instance that executed the init method later cannot be used.

example

  const server = express();
  const adapter = new ExpressAdapter(server);
  const adminApp = await NestFactory.create(AppModule, adapter);
  const apiApp = await NestFactory.create(AppModule, adapter);

  adminApp.setGlobalPrefix('admin');
  apiApp.setGlobalPrefix('api');

  await adminApp.init();
  await apiApp.init();

  server.listen(3000, () => console.log('Listening at 3000'));

result

$ curl localhost:3000/admin
admin
$ curl localhost:3000/api
{"statusCode":404,"error":"Not Found","message":"Cannot GET /api"}

Issue Number: https://github.com/nestjs/nest/issues/4114

What is the new behavior?

When setting middreware to the adapter, set the prefix registered with setGlobalPrefix.

With the above measures, the middleware is set for each instance, and the router is set appropriately.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

If we do not use setGlobalPrefix, if we set with '/' added, if we set without '/', everything works normally. This has been adapted to maintain compatibility.

Other information

[coveralls]

Pull Request Test Coverage Report for Build 16357

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.06%) to 95.127%

---

Totals
Change from base Build 16231:	0.06%
Covered Lines:	4568
Relevant Lines:	4802

---

💛 - Coveralls

[jmcdo29]

Does this also still add a notFoundHandler and an errorHandler to the '/' route, even when there are multiple applications being created? So in the above example there is the following handlers

Handler	Route
Router	admin/
NotFound	admin/
Exception	admin/
Router	api/
NotFound	api/
Exception	api/
NotFound	/
Exception	/

Would it be possible to keep that, as that is the current functionality? And what happens if no global prefix is set by either application? Does the first application have it's route handler overwritten?

[kamilmysliwiec]

In addition to @jmcdo29 questions, what if I have a single app + I set the global prefix to, let's say, /api and someone tries to hit /users without the /api prefix. What would happen?

[uc4w6c]

@jmcdo29 @kamilmysliwiec Thank you. Certainly the above is not supported. Please wait a moment as there is one idea.

[kamilmysliwiec]

Maybe we should simply add 2 additional properties to the application options interface which will determine whether to mount the "not found" and "exception handler" middleware. Hence, you would be able to compose multiple applications easily.

[uc4w6c]

I changed to keep the globalprefix set by httpadapter. With this fix, NotFoundException is thrown at the right time.

example

  const server = express();
  const adapter = new ExpressAdapter(server);
  const catsApp = await NestFactory.create(AppModule, adapter);
  const dogsApp = await NestFactory.create(AppModule, adapter);

  catsApp.setGlobalPrefix('cats');
  dogsApp.setGlobalPrefix('dogs');

  await catsApp.init();
  await dogsApp.init();

  server.listen(3000, () => console.log('Listening at 3000'));

result

$ curl localhost:3000/cats
Hello!
$ curl localhost:3000/dogs
Hello!
$ curl localhost:3000/cats/dogs
{"statusCode":404,"error":"Not Found","message":"Cannot GET /dogs"}
$ curl localhost:3000/dogs/cats
{"statusCode":404,"error":"Not Found","message":"Cannot GET /cats"}
$ curl localhost:3000/admin
{"statusCode":404,"error":"Not Found","message":"Cannot GET /admin"}

When prefix is ​​not added + prefix is ​​added, it works normally.

But it's a bit complicated

Maybe we should simply add 2 additional properties to the application options interface which will determine whether to mount the "not found" and "exception handler" middleware.

This is fine, but which one do you prefer?

[kamilmysliwiec]

This is fine, but which one do you prefer?

Example:

const server = express();
const adapter = new ExpressAdapter(server);
const catsApp = await NestFactory.create(AppModule, adapter, {
   notFoundHandler: false,
});
const dogsApp = await NestFactory.create(AppModule, adapter);

catsApp.setGlobalPrefix('cats');
dogsApp.setGlobalPrefix('dogs');

await catsApp.init();
await dogsApp.init();

server.listen(3000, () => console.log('Listening at 3000'));

In this case, the dogsApp not found handler would be used.

If one wants to disable it entirely, notFoundHandler can be set to false twice:

const catsApp = await NestFactory.create(AppModule, adapter, {
   notFoundHandler: false,
});
const dogsApp = await NestFactory.create(AppModule, adapter, {
   notFoundHandler: false,
});

[jmcdo29]

What would happen if someone set notFoundHandler: true on both? Would it come around to the same issue? Would that be something that could be warned against from the factory?

[uc4w6c]

What would happen if someone set notFoundHandler: true on both? Would it come around to the same issue?

I also think the same issue occur.

Could you review the commit for b432140: fix(platform): support notfound in root url when added prefix? I'd like to receive your opinion and do better.

[kamilmysliwiec]

Would that be something that could be warned against from the factory?

Probably not. We can't really determine whether it was set on purpose or not. I think setting the notFoundHandler: false twice is very unambiguous though (nobody should be surprised that neither handler was applied[?])

[uc4w6c]

If catsApp is set notFoundHanlder: false and dogsApp is set 'notFoundHanlder: true`, It will be as follows.

Handler	Route
Router	/cats
NotFound	/
Exception	/
Router	/dogs

So We can't get access to dogs router. This is because Express is handled in the order that It is set.

I will make two proposals below.

Plan 1

This add NotFoundHandler to the "/" route and prefix route. This is the commit of b432140: fix(platform): support notfound in root url when added prefix?.

Handler	Route
Router	/cats
NotFound	/cats
Exception	/cats
Router	/dogs
NotFound	/dogs
Exception	/dogs
NotFound	/

• Good Point Be compatible and behave ideally.

• Bad Point Not simple code. (HttpAdapter needs to keep the set prefix.)

Plan 2

This add NotFoundHandler to the prefix route only.

Handler	Route
Router	/cats
NotFound	/cats
Exception	/cats
Router	/dogs
NotFound	/dogs
Exception	/dogs

And user need to add the setNotFoundHandler(callback) to httpadapter by userself.

const server = express();
const adapter = new ExpressAdapter(server);
const catsApp = await NestFactory.create(AppModule, adapter);
const dogsApp = await NestFactory.create(AppModule, adapter);

catsApp.setGlobalPrefix('cats');
dogsApp.setGlobalPrefix('dogs');

await catsApp.init();
await dogsApp.init();

const callback = <TRequest, TResponse>(req: TRequest, res: TResponse) => {
      const method = applicationRef.getRequestMethod(req);
      const url = applicationRef.getRequestUrl(req);
      throw new NotFoundException(`Cannot ${method} ${url}`);
};
adapter.setNotFoundHandler(callback);

server.listen(3000, () => console.log('Listening at 3000'));

• Good Point Simple code.

• Bad Point Incompatible.

[uc4w6c]

I'm sorry. I think my way of communicating was not good.

I want to solve this problem. What should I do?

If you proceed with adding properties, there is a possibility that it can be solved by creating a new method.

Example:

const server = express();
const adapter = new ExpressAdapter(server);
const catsApp = await NestFactory.create(AppModule, adapter, {
   notFoundHandler: false,
});
const dogsApp = await NestFactory.create(AppModule, adapter);

catsApp.setGlobalPrefix('cats');
dogsApp.setGlobalPrefix('dogs');

await adapter.multipleAppsInit();

server.listen(3000, () => console.log('Listening at 3000'));

What do you think?

[sagiBarkol]

any updates?