Papooch / nestjs-cls

A continuation-local storage (async context) module compatible with NestJS's dependency injection.
https://papooch.github.io/nestjs-cls/
MIT License
434 stars 28 forks source link

Question: How to use Factory Proxy Providers? #89

Closed Duske closed 11 months ago

Duske commented 11 months ago

Thanks for this awesome library! While trying to get rid of request-scope, that we are currently using to have RLS-scoped database connections, those Factory Proxy Providers looked very nice. Unfortunately, I cannot get them to work.

In the docs there is this example, but I just cannot get it to work πŸ˜“


ClsModule.forFeature({
    provide: TENANT_CONNECTION,
    import: [DatabaseConnectionModule],
    inject: [CLS_REQ, DatabaseConnectionService],
    useFactory: async (req: Request, dbService: DatabaseConnectionService) => {
        const tenantId = req.params['tenantId'];
        const connection = await dbService.getTenantConnection(tenantId);
        return connection;
    },
});

The old request scope approach

This works, but we want to get rid of:

@Global()
@Module({})
export class RLSModule {
  static forRoot(
    importModules: (
      | DynamicModule
      | Type<any>
      | Promise<DynamicModule>
      | ForwardReference<any>
    )[],
    // eslint-disable-next-line @typescript-eslint/ban-types
    injectServices: (string | symbol | Function | Type<any> | Abstract<any>)[],
  ): DynamicModule {
    const rlsProvider: Provider = {
      provide: TENANT_CONNECTION,
      inject: [REQUEST, DataSource, ...injectServices],
      scope: Scope.REQUEST,
      durable: true,
      useFactory: async (request: Request, connection: DataSource, ...args) => {
        const authInfo = request.auth.payload as Auth0JWTPayload;
        return createScopedDataSource(
          connection,
          authInfo.sub,
          authInfo.tenantId,
        );
      },
    };

    return {
      module: RLSModule,
      imports: importModules,
      providers: [rlsProvider],
      exports: [TENANT_CONNECTION],
    };
  }
}

Then, this global RLSModule is simply imported in app.module.ts.

embed ClsModule in RlsModule

This didnt work with errors like connection is undefined

@Global()
@Module({})
export class RLSModule {
  static forRoot(
    importModules: (
      | DynamicModule
      | Type<any>
      | Promise<DynamicModule>
      | ForwardReference<any>
    )[],
    // eslint-disable-next-line @typescript-eslint/ban-types
    injectServices: (string | symbol | Function | Type<any> | Abstract<any>)[],
  ) {
// where should we put ClsModule.forFeatureAsync? 
    const dynamicModule = ClsModule.forFeatureAsync({
      provide: TENANT_CONNECTION,
      imports: [TypeOrmModule],
      inject: [CLS_REQ, DataSource],
      useFactory: async (request: Request, connection: DataSource) => {
        const authInfo = request.auth.payload as Auth0JWTPayload;
        return createScopedDataSource(
          connection,
          authInfo.sub,
          authInfo.tenantId,
        );
      },
    });
    return {
      module: RLSModule,
      imports: importModules,
      providers: [...dynamicModule.providers, RLSEnforcerService],
      exports: [TENANT_CONNECTION],
    };
  }
}

standalone ClsModule

here I run into dependency errors, where it cannot resolve the tenant connection:

@Module({
  imports: [
    ClsModule.forRoot({
      global: true,
      middleware: { mount: true },
    }),
    ClsModule.forFeatureAsync({
      provide: TENANT_CONNECTION,
      imports: [TypeOrmModule],
      inject: [CLS_REQ, DataSource],
      useFactory: async (request: Request, connection: DataSource) => {
        const authInfo = request.auth.payload as Auth0JWTPayload;
        return createScopedDataSource(
          connection,
          authInfo.sub,
          authInfo.tenantId,
        );
      },
    }),
    ConfigModule.forRoot({ isGlobal: true }),
//...
});
Nest can't resolve dependencies of the Auth0Guard (ConfigService, ?, ContextService, TenantsService, PinoLogger). Please make sure that the argument TENANT_CONNECTION at index [1] is available in the AuthModule context.
Papooch commented 11 months ago

Hi, thank you for providing such elaborate issue description!

First and foremost - yes, you're correct, there's a mistake in the docs and it should be forFeatureAsync.

Second - the forFeatureAsync registers the provider (in your case TENANT_CONNECTION) only in the module where you define it. You can the use module re-exporting to make the providers also available in the parent module. With that in mind, I think your RLSModule as a wrapper is almost correct.

Without a guarantee whether it would work, I'd suggest this form:

@Global()
@Module({})
export class RLSModule {
  static forRoot(
    importModules: (
      | DynamicModule
      | Type<any>
      | Promise<DynamicModule>
      | ForwardReference<any>
    )[],
    // eslint-disable-next-line @typescript-eslint/ban-types
    injectServices: (string | symbol | Function | Type<any> | Abstract<any>)[],
  ) {
    return {
      module: RLSModule,
      imports: [
        ...importModules,
       // use imports the ClsModule
        ClsModule.forFeatureAsync({
          provide: TENANT_CONNECTION,
          imports: [TypeOrmModule],
          inject: [CLS_REQ, DataSource],
          useFactory: async (request: Request, connection: DataSource) => {
            const authInfo = request.auth.payload as Auth0JWTPayload;
            return createScopedDataSource(
              connection,
              authInfo.sub,
              authInfo.tenantId,
            );
          },
        });
      ]
      // do not put other module's providers to providers (that might be the reason the connection was undefined)
      providers: [RLSEnforcerService],
      // re-export the dynamic ClsModule definition - this makes `TENANT_CONNECTION`
      exports: [ClsModule]
    };
  }
}

The other variant, where you register the ClsModule.forFeatureAsync directly in root doesn't work, because the provider is available only in AppModule (that's why you must use a wrapper module in the current situation).

It might be a good idea to add the global flag to the forFeatureAsync option as well to make the provider accessible globally. (EDIT: Hell, it was so simple that I just added it and released with v3.6.0, which means you can also use the second way by adding global: true)

Please let me know if this helped, or create a minimal reproducible example that I can debug on my machine.

Duske commented 11 months ago

Wow, thanks for the quick reply and the new release πŸ™‡πŸΌ

Re-exporting the module did work as you suggested, but now I am running in a more DX problem. Maybe you have a suggestion as well:

When using your approach as a drop-in solution the code will not work due to error at boot time: TypeError: connection.getRepository is not a function


@Injectable()
export class ResourcesService {
  constructor(
    @Inject(TENANT_CONNECTION)
    private connection: RLSConnection,
    private resourceStorageService: ResourceStorageService,
    private eventEmitter: EventEmitter2,
  ) {
    // here we will get 
    this.resourceRepository = connection.getRepository(Resource);
  }
  resourceRepository: Repository<Resource>;

which makes sense, as the connection is only populated during a request right?

We used the connection.getRepository() approach in order to have a repository as member variable for the entire service, so that all other methods can simply run this.resouceRepository.findAll() for example.

Shall we refactor to use it like this? e.g.

  async findAll() {
// Replace
//  const resources = await this.resourceRepository.find({
    const resources = await this.connection.getRepository(Resource).find({
      relations: {
        assets: true,
        requests: true,
        createdBy: true,
        lastEditedBy: true,
      },
    });

or is there a smoother approach to have the repository available in a more nest-idomatic way? Thanks a lot for your support! 🀜🏼 πŸ€›πŸΌ

Papooch commented 11 months ago

Hi, your reasoning is correct, the connection will only be available at request time (or when when the CLS context is initialized by any other means). One of the drawbacks is that the underlying instance is not available in the constructor of singleton scoped providers (the same issue is noticeable with forwardReffed dependencies).

The solution you suggested is definitely a possibility.

Another one would be making the ResourceRepository into an another Proxy provider scoped to the module only.

e.g:

Create a class that would serve both as an interface and an injection token - it can be abstract because we never instantiate it (not sure about extends/implements though):

export abstract class ResourceRepository extends Repository<Resource> {}

(You don't have to do this and just use a string based injection token, but this way greatly simplifies the injection experience)

Then register another factory provider that will return the repository itself:

ClsModule.forFeatureAsync({
  provide: ResourceRepository
  inject: [TENANT_CONNECTION]
  useFactory: (connection: RLSConnection) => {
    return connection.getRepository(Resource)
  }
})

And then you can inject the repository directly into the service without leaking the implementation of TENANT_CONNECTION

@Injectable()
export class ResourcesService {
  constructor(
    private resourceRepository: ResourceRepository,
    private resourceStorageService: ResourceStorageService,
    private eventEmitter: EventEmitter2,
  ) {

You can even go as far as wrapping the repository proxy registration in a higher order function to simplify the registration process:

function registerProxyRepository(repoClass: Type<any>, entityClass: Type<any>) {
  return ClsModule.forFeatureAsync({
    provide: repoClass
    inject: [TENANT_CONNECTION]
    useFactory: (connection: RLSConnection) => {
      return connection.getRepository(entityClass)
    }
  })
}

and then just use it in the consuming module as

imports: [
  registerProxyRepository(ResourceRepository, Resource)
]
Duske commented 11 months ago

Ah nice, this is the DX I was hoping for and it makes this a really viable solution - case closed!πŸ‘ŒπŸΌ Thx for the support and this library πŸ™ŒπŸΌ