Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entire NestJS Module Revamp + Handling multiple connections #18

Closed
wants to merge 4 commits into from

Conversation

EnriqCG
Copy link
Owner

@EnriqCG EnriqCG commented Nov 13, 2021

Context of the revamp

Previous partial support for handling multiple AMQP connections involved using the same root AMQPModule and passing an array of connections.

static forRoot(options: AMQPModuleOptions | AMQPModuleOptions[]): DynamicModule {

amqplib connection objects were stored within the same module in a Map. When performing actions on a particular connection or channel, retrieval from the Map had to happen:

getChannel(connectionName?: string): ChannelWrapper {
if (!connectionName) {
connectionName = this.amqpClient.defaultKey
}
if (!this.amqpClient.clients.has(connectionName)) {
throw new Error(`client ${connectionName} does not exist`)
}

This deviates from what NestJS does on modules like @nestjs/mongoose or @nestjs/typeorm where multiple connections are handled by different instances of the module. This is how @nestjs/mongoose handles multiple connections:

@Module({
  imports: [
    MongooseModule.forRoot('mongodb://localhost/test', {
      connectionName: 'cats',
    }),
    MongooseModule.forRoot('mongodb://localhost/users', {
      connectionName: 'users',
    }),
  ],
})
export class AppModule {}

And when wanting to use a resource for a module, an Injector is provided to inject the resource for a particular connection:

export class CatsService {
  constructor(@InjectConnection('cats') private connection: Connection) {}
}

This behavior also deviates from the current implementation where no injection happens, and instead, a shared "AMQPService" is provided to get access to channels. Not doing dependency injection right has implications for testing and goes against core design principles for NestJS.

(Breaking) changes made to fix this behavior

(not a comprehensive list)

  • AMQPModule.forRoot now accepts two parameters. The first is for connection details (hostname, port, etc.), and the second is for connection options (exchanges, asserting queues, serviceName, etc.)
forRoot(connection: string | Options.Connect, options: AMQPModuleOptions)
  • Created two different NestJS providers on the AMQPCoreModule. One for an AMQP Connection and one for an AMQP Channel.
  • New channel and connection token getters for handling multiple connections.
  • Exported injectors so a connection/channel can be injected to any class.
    • Removed AMQPService in favor of injectors.
export class JobsService {
  constructor(
    @InjectAMQPChannel('connectionName!')
    private amqpChannel: Channel,
  ) {}
  • Moved AMQP initializers (queue assertions, consumer handlers, etc.) from the AMQPModule to the AMQPModuleExplorer.

TO-DOs

Closes #3.

@EnriqCG EnriqCG added the enhancement New feature or request label Nov 13, 2021
@EnriqCG
Copy link
Owner Author

EnriqCG commented May 17, 2022

Closed in favor of #24.

@EnriqCG EnriqCG closed this May 17, 2022
@EnriqCG EnriqCG deleted the multiple-connections branch February 28, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need Async initalizing Modules with forRootAsync Multiple AMQP connections
1 participant