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

feat: add command interceptors #1621

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

V3RON
Copy link

@V3RON V3RON commented Feb 1, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • 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?

None. It's not possible to intercept commands. Things like logging must be handled by extension of CommandBus class in the application code.

What is the new behavior?

Additional actions can be executed both before and after the command is handled. This feature enhances the flexibility and extensibility of command processing. For instance, one can implement ValidationCommandInterceptor, run validation on commands.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This commit introduces command interceptors, enabling the interception of commands.
Now, actions can be executed both before and after the command is handled.
This feature enhances the flexibility and extensibility of command processing.
@V3RON V3RON force-pushed the feat/command-interception branch from a260775 to 705ec94 Compare February 1, 2024 16:02
@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Dec 3, 2024

Apologies for the delay in reviewing this PR, and thank you for your contribution!

I just wanted to confirm that I haven’t missed anything here—wouldn't it be simpler to create your own abstraction over the CommandBus? For example:

@Injectable()
class MyCommandBus {
  constructor(private readonly commandBus: CommandBus) {}

  async execute<T extends ICommand>(command: T): Promise<T> {
    console.log('Before command execution');
    const result = await this.commandBus.execute(command);
    console.log('After command execution');
    return result;
  }
}

I'm a bit hesitant about introducing another abstraction here. If we merge this PR, we’d likely need to add similar annotations for queries and possibly even events—introducing a total of three new annotations to achieve something that appears manageable with the approach above.

Please let me know if there’s something I might be overlooking—whether it’s something currently too complex, requires too much overhead, or isn’t feasible at the moment but would be resolved by merging this PR.

Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants