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

Add PreInvoke and PostInvoke events #29

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

hurricane-voronin
Copy link

@hurricane-voronin hurricane-voronin commented Jun 13, 2024

Q A
Bugfix?
Breaks BC?
New feature?
Issues Enchant invoker functionality with PreInvoke and PostInvoke events for a general purpose
Docs PR spiral/docs#...

Summary by CodeRabbit

  • New Features

    • Added event dispatching for gRPC service methods, enabling hooks before and after method invocation.
  • Bug Fixes

    • Updated dependencies to replace "spiral/roadrunner-worker" with "psr/event-dispatcher" for improved event handling.
  • Chores

    • Suppressed MissingClassConstType issue in the psalm.xml configuration for cleaner code analysis.

Copy link

coderabbitai bot commented Jun 13, 2024

Walkthrough

In this update, we replaced the "spiral/roadrunner-worker" dependency with "psr/event-dispatcher" and restructured dependency order. New event classes (Event, PreInvokeEvent, PostInvokeEvent) were introduced to handle event propagation in a gRPC service. The Invoker class was enhanced to support dispatching events before and after method invocations. Additionally, the psalm.xml configuration was updated to suppress the MissingClassConstType issue.

Changes

File Summary
composer.json Replaced "spiral/roadrunner-worker" dependency with "psr/event-dispatcher" and adjusted dependency order.
src/Event/Event.php Introduced Event class implementing StoppableEventInterface with methods to stop/check event propagation.
src/Event/PostInvokeEvent.php Added PostInvokeEvent class that extends Event, with a constructor for a Message object and a method to retrieve it.
src/Event/PreInvokeEvent.php Added PreInvokeEvent class representing pre-invocation events, with methods to retrieve context and input message.
src/Invoker.php Added event dispatching before/after method invocation with custom events and updated constructor to include an optional EventDispatcherInterface parameter.
psalm.xml Suppressed MissingClassConstType issue in the configuration.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Invoker
    participant EventDispatcher
  
    Client->>Invoker: invoke(Method, Context, Message)
    Invoker->>EventDispatcher: dispatch(PreInvokeEvent)
    EventDispatcher-->>Invoker: PreInvokeEvent

    Invoker->>Invoker: Execute Method
    
    Invoker->>EventDispatcher: dispatch(PostInvokeEvent)
    EventDispatcher-->>Invoker: PostInvokeEvent

    Invoker-->>Client: Result
Loading

Poem

In a land of code, dependencies shift,
Events unfold, making methods swift.
With Pre and Post, we watch them flow,
A dance of data, the gRPC show.
Suppress we do, the psalm's small fret,
Roadrunner’s race, an easier bet! 🚀

Warning

Review ran into problems

Problems (1)
  • Git: Failed to clone repository. Please contact CodeRabbit support.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9e5e86f and f0e0058.

Files selected for processing (5)
  • composer.json (1 hunks)
  • src/Event/Event.php (1 hunks)
  • src/Event/PostInvokeEvent.php (1 hunks)
  • src/Event/PreInvokeEvent.php (1 hunks)
  • src/Invoker.php (3 hunks)
Files skipped from review due to trivial changes (1)
  • composer.json
Additional comments not posted (9)
src/Event/PostInvokeEvent.php (2)

11-14: The constructor is correctly defined and ensures immutability of the output property.


16-19: The getOutput() method is implemented correctly as a straightforward getter for the output property.

src/Event/PreInvokeEvent.php (3)

12-16: The constructor is correctly defined, ensuring immutability and type safety for ctx and input properties.


18-21: The getContext() method is implemented correctly as a straightforward getter for the ctx property.


23-26: The getInput() method is implemented correctly as a straightforward getter for the input property.

src/Event/Event.php (2)

16-19: The isPropagationStopped() method correctly implements the interface's requirement to check the event's propagation state.


28-31: The stopPropagation() method is correctly implemented and well-documented, ensuring that event propagation can be halted as required.

src/Invoker.php (2)

23-26: The constructor is well-implemented, providing optional dependency injection for EventDispatcherInterface, which allows for flexible integration of event handling.


39-40: The integration of PreInvokeEvent and PostInvokeEvent dispatching in the invoke method is correctly implemented, enhancing the invoker's functionality as intended.

Also applies to: 46-47

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f0e0058 and 435c3b7.

Files selected for processing (1)
  • psalm.xml (1 hunks)
Files skipped from review due to trivial changes (1)
  • psalm.xml

@msmakouz
Copy link
Member

@hurricane-voronin Hi, thank you. Regarding the implementation, everything is fine. However, what was your reason for adding EventDispatcherInterface to the Invoker? For example, you could have created your own implementation of Spiral\RoadRunner\GRPC\InvokerInterface, where you add EventDispatcherInterface and Spiral\RoadRunner\GRPC\Invoker. And trigger events before and after calling the invoke method using the original implementation of Spiral\RoadRunner\GRPC\Invoker.

@hurricane-voronin
Copy link
Author

Hi @msmakouz!
Yeah, the solution with custom Invoker also works but it needs to copy-paste a ton of logic from the original Invoker and keep it up to date. So I just added two events to handle general stuff, so it's more flexible for common purposes like validation, logging, etc. In my case, I'm validating my custom input messages using symfony validator subscribing to PreInvokeEvent in one place.

@roxblnfk
Copy link
Member

the solution with custom Invoker also works but it needs to copy-paste

Hi. Using the decorator pattern you don't need to copy-paste the ton of logic. Just implement the Injector interface and reuse the existing Injector class.

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