needle-innovision / nestjs-tenancy

Multi-tenancy approach for nestjs - currently supported only for mongodb with mongoose
MIT License
186 stars 58 forks source link

Microservices Support missing #37

Open moparlakci opened 1 year ago

moparlakci commented 1 year ago

Hello

Thanks for the great tenancy module!

The only this that is missing is, microservices support.

We are using the Request/Response pattern from ClientProxy to talk to other microservices.

We use it also in our JwtStrategy (Passport), to validate user data from the database, which is stored in another microservice called 'auth'. We do that by sending a messagepattern to auth microservice. But this is a tcp connection and therefore doesnt have Request object, so we get req.get is not a function error.

To give an example

We have an auth microservice with an auth module, and auth controller that looks like the below

`

@Controller('')
export class AuthController {
  constructor(
    private readonly authService: AuthService
    ) {} 

 @Post('login')
  create(@Body() loginDto: LoginDto) {
    return this.authService.login(loginDto);
  }

@MessagePattern({ cmd: 'get_user_with_permissions' })
  async getUser(@Payload() data, @Ctx() context: RmqContext): Promise<any> { 

  const userWithPermissions = await this.authService.getUserWithPermissions(data.id, data.tenant);

  const channel = context.getChannelRef();
  const originalMsg = context.getMessage();

  channel.ack(originalMsg); 

  return userWithPermissions;
  }}

  }

`

in our JwtStrategy our validate function looks like below

`

async validate(payload: any) { 

const { id, tenant } = payload;

const userWithPermissions = await firstValueFrom(this.authClient.send({ cmd: 'get_user_with_permissions' }, {
  id,
  tenant
}).pipe(
  tap((res) => { 
     return res;
  }),
  catchError((err) => {
    throw new UnauthorizedException();
  }))); 

return userWithPermissions;

}

`

in the Auth context we should be able to switch to another tenant connection, somehow. and in the jwtStrategy we should send the Tenant Identifier by some means via a json property or so.

So a little change should be made to getTenantFromRequest or getTenant function, to check if the call is rpc or http..

So in our Auth Controller, the message comes in via TCP, RPC. And the nestjs-tenancy module cannot know which tenant to use. So basically microservices support is missing.

I hope we can add microservices support.

moparlakci commented 1 year ago

Knipsreq_GetErrorel

moparlakci commented 1 year ago

The function where it needs to be checked

sdfdsfdsfsdfds546

The body of Req property when its a RPC/Microservices call looks like below

req

sandeepsuvit commented 1 year ago

Hi @moparlakci thanks for writing in. Currently this library is intended to work under a http REQUEST scoped session. Hence microservice based request isn't something that this library would be a able to process at the moment. This is a possible candidate for a PR. Happy to review it if someone can give it a try.

moparlakci commented 1 year ago

I think I fixed it myself by changing the TenancyCoreModule a little..

Just if you add a check to the following code inside static getTenantFromRequest(isFastifyAdaptor, req, tenantIdentifier) will be sufficient at the moment.

`

 private static getTenantFromRequest(
    isFastifyAdaptor: boolean,
    req: Request,
    tenantIdentifier: string,
  ) {

    let request = req;
    const { context } = req;
    if(context.getType() === 'rpc') {
        request = context.switchToRpc().getData();
        // inside microservice call..
        // just return a property from the sent data object -- could be dynamic using tenantIdentifier
        return request.data[tenantIdentifier];
    } else if(context.getType() === 'http') {
        request = context.switchToHttp().getRequest();
    }  

    let tenantId = '';

    if (isFastifyAdaptor) {
      // For Fastify
      // Get the tenant id from the header
      tenantId =
      request.headers[`${tenantIdentifier || ''}`.toLowerCase()]?.toString() ||
        '';
    } else {
      // For Express - Default
      // Get the tenant id from the request
      tenantId = request.get(`${tenantIdentifier}`) || '';
    }

    // Validate if tenant id is present
    if (this.isEmpty(tenantId)) {
      throw new BadRequestException(`${tenantIdentifier} is not supplied`);
    }

    return tenantId;
  }
`
moparlakci commented 1 year ago

Because otherwise any controller endpoint using a MessagePattern or EventPattern decorator is not useable with this package.

sandeepsuvit commented 1 year ago

@moparlakci true that and glad that you solved it on your own. Can you raise a PR for this with a unit test case as-well for the microservice change bit that you have made. I would review it and release a new change.

moparlakci commented 1 year ago

Hello, changed the functions to use context in stead of req properties. But injection of the ExecutionContext is still a problem at the moment.

the following injects a Request object into context, but its expecting a ExecuitionContext and therefore getType() method rasies an error.

private static createTenantContextProvider(): Provider { return { provide: TENANT_CONTEXT, scope: Scope.REQUEST, useFactory: ( context: ExecutionContext, moduleOptions: TenancyModuleOptions, adapterHost: HttpAdapterHost, ) => this.getTenant(context, moduleOptions, adapterHost), inject: [CONTEXT, TENANT_MODULE_OPTIONS, DEFAULT_HTTP_ADAPTER_HOST], }; }

moparlakci commented 1 year ago

Hi @sandeepsuvit,

Can you check my fork? Currently the Request type check is not pretty coded, but commits are always welcome. Typescript check for the incoming request was a bit difficult because of the class Request for HTTP calls and RequestContext for anyother call.

Couldnt use context functions, something to do with the injection of the CONTEXT token which is an alias for the REQUEST token.

See my fork here https://github.com/moparlakci/nestjs-tenancy/blob/master/lib/tenancy-core.module.ts

Tests are passing.. https://github.com/moparlakci/nestjs-tenancy/blob/master/tests/src/dogs/dogs.service.ts

Thanks in advance

moparlakci commented 1 year ago

@sandeepsuvit any updates on this? I will also debug further..

Robokishan commented 1 year ago

This is exactly what i was trying to do @moparlakci . This entire library is written on Request architecture. anything which touches this flow will become request scopped. meaning nestjs will create new instance on every request for each repository each services. i think this is not efficient. if it would have been interceptor or some other logic then it would be more efficient. i have used this in production it this is causing issues with large traffic. I am re writing entire logic for multi tenancy. i would checked this before using this library. @moparlakci I think I can help you rewrite logic

Robokishan commented 1 year ago

@moparlakci i tried your code. test cases are passing but

app.connectMicroservice({ transport: Transport.TCP, });

await app.startAllMicroservices();

is not working. When i make http request it hangs is there any reason for not working this ? does not throw any error but application just hangs. i am not getting any response.

Robokishan commented 1 year ago

Alright i now found the issue here. app.listen should have different port other than 3000 since tcp microservice is using 3000 port. i changed port for tcp service and everthing is working fine. now i have implemented same thing for rabbitMQ, and google pub sub message. Everything is on multi tenant superb ! thanks @moparlakci . i didn't used your entire pr. i just changed TENANT_CONTEXT. and this.getTenant function. everything else used same as this library since i have done some of my own implementation for mongoose