-
-
Notifications
You must be signed in to change notification settings - Fork 35
Add gRPC server interceptors #157
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughSupport for gRPC server interceptors has been added to the Laravel RoadRunner Bridge. This includes a new interface for interceptors, updates to service registration and invocation logic to apply interceptors globally or per service, and expanded documentation and configuration examples illustrating their usage. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Interceptor1
participant Interceptor2
participant Service
Client->>Server: gRPC request (method, context, body)
Server->>Interceptor1: intercept(method, context, body, next)
Interceptor1->>Interceptor2: intercept(method, context, body, next)
Interceptor2->>Service: invoke(method, context, body)
Service-->>Interceptor2: response
Interceptor2-->>Interceptor1: response
Interceptor1-->>Server: response
Server-->>Client: response
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Note: Using configuration file /phpstan.neon.dist. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md
(1 hunks)config/roadrunner.php
(1 hunks)src/Grpc/GrpcServerInterceptorInterface.php
(1 hunks)src/Grpc/GrpcWorker.php
(1 hunks)src/Grpc/Server.php
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/Grpc/GrpcWorker.php (1)
src/Grpc/Server.php (1)
registerService
(68-74)
src/Grpc/Server.php (1)
src/Grpc/GrpcServerInterceptorInterface.php (1)
intercept
(11-11)
🔇 Additional comments (8)
config/roadrunner.php (2)
21-28
: LGTM! Clear example of service-specific interceptors configuration.The example effectively demonstrates how to configure interceptors for individual services.
29-32
: LGTM! Global interceptors configuration is well-placed.The configuration properly supports global interceptors that apply to all gRPC services.
src/Grpc/GrpcWorker.php (2)
35-36
: LGTM! Global interceptors retrieval is correct.The code properly retrieves global interceptors from configuration with a safe default value.
46-46
: LGTM! Service registration properly includes interceptors.The code correctly passes the merged interceptors to the server's registerService method.
README.md (2)
223-248
: LGTM! Clear interceptor documentation and example.The example effectively demonstrates how to implement a gRPC server interceptor with pre and post-processing logic.
250-278
: LGTM! Comprehensive interceptor configuration examples.The documentation clearly explains both global and service-specific interceptor configuration patterns.
src/Grpc/Server.php (2)
40-42
: LGTM! Interceptors property properly typed and structured.The property follows the existing pattern for storing service-specific data.
145-153
: LGTM! Parameter rename improves clarity.The rename from
$service
to$serviceName
better reflects the parameter's purpose.
|
||
interface GrpcServerInterceptorInterface | ||
{ | ||
public function intercept(string $method, ContextInterface $context, string $body, callable $next); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add return type annotation to the intercept method.
The intercept
method should declare its return type for better type safety and IDE support.
Apply this diff to add the return type:
- public function intercept(string $method, ContextInterface $context, string $body, callable $next);
+ public function intercept(string $method, ContextInterface $context, string $body, callable $next): string;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public function intercept(string $method, ContextInterface $context, string $body, callable $next); | |
public function intercept(string $method, ContextInterface $context, string $body, callable $next): string; |
🤖 Prompt for AI Agents
In src/Grpc/GrpcServerInterceptorInterface.php at line 11, the intercept method
lacks a return type declaration. Add the appropriate return type annotation to
the method signature to improve type safety and IDE support. Determine the
correct return type based on the method's expected output and append it after
the parameter list.
src/Grpc/Server.php
Outdated
$service = $this->services[$serviceName] ?? []; | ||
$interceptors = $this->interceptors[$serviceName] ?? []; | ||
|
||
$handler = function ($method, $context, $body) use ($service) { | ||
return $service->invoke($method, $context, $body); | ||
}; | ||
|
||
$pipeline = array_reduce( | ||
array_reverse($interceptors), | ||
fn($next, $interceptor) => fn($method, $context, $body) => (new $interceptor)->intercept($method, $context, $body, $next), | ||
$handler | ||
); | ||
|
||
return $pipeline($method, $context, $body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove redundant assignment and optimize interceptor instantiation.
The code has redundancy and performance issues:
- Line 155 assigns service with a fallback even though we already verified it exists.
- Interceptors are instantiated on every request, which is inefficient.
Apply this diff to fix the issues:
- $service = $this->services[$serviceName] ?? [];
+ $service = $this->services[$serviceName];
$interceptors = $this->interceptors[$serviceName] ?? [];
$handler = function ($method, $context, $body) use ($service) {
return $service->invoke($method, $context, $body);
};
$pipeline = array_reduce(
array_reverse($interceptors),
- fn($next, $interceptor) => fn($method, $context, $body) => (new $interceptor)->intercept($method, $context, $body, $next),
+ fn($next, $interceptor) => function($method, $context, $body) use ($interceptor, $next) {
+ // Consider caching interceptor instances in registerService method
+ return $this->worker->application()->make($interceptor)->intercept($method, $context, $body, $next);
+ },
$handler
);
This uses the Laravel container to instantiate interceptors, which allows for dependency injection and potential singleton behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
$service = $this->services[$serviceName] ?? []; | |
$interceptors = $this->interceptors[$serviceName] ?? []; | |
$handler = function ($method, $context, $body) use ($service) { | |
return $service->invoke($method, $context, $body); | |
}; | |
$pipeline = array_reduce( | |
array_reverse($interceptors), | |
fn($next, $interceptor) => fn($method, $context, $body) => (new $interceptor)->intercept($method, $context, $body, $next), | |
$handler | |
); | |
return $pipeline($method, $context, $body); | |
$service = $this->services[$serviceName]; | |
$interceptors = $this->interceptors[$serviceName] ?? []; | |
$handler = function ($method, $context, $body) use ($service) { | |
return $service->invoke($method, $context, $body); | |
}; | |
$pipeline = array_reduce( | |
array_reverse($interceptors), | |
fn($next, $interceptor) => function($method, $context, $body) use ($interceptor, $next) { | |
// Consider caching interceptor instances in registerService method | |
return $this->worker->application()->make($interceptor) | |
->intercept($method, $context, $body, $next); | |
}, | |
$handler | |
); | |
return $pipeline($method, $context, $body); |
🤖 Prompt for AI Agents
In src/Grpc/Server.php around lines 155 to 168, remove the redundant fallback
assignment for $service since its existence is already verified. Refactor the
interceptor instantiation inside the array_reduce to instantiate each
interceptor only once before the pipeline creation, using the Laravel container
to resolve them for better performance and dependency injection support. Replace
the inline new interceptor instantiation with container-based resolution outside
the pipeline function to avoid creating interceptors on every request.
What about using the same spiral/interceptors like in gRPC client? We can run it like in the Spiral RoadRunner Bridge: |
I come from Laravel and I am new to Spiral. Right now, I just implement interceptors like Laravel middlewares, but of course I don't want to remake something that Spiral already has. You told me that Spiral offers an Interceptor interface, which sounds great, but I don't really know how to use it. Could you please share a simple example or explain how it works? Also, what benefits does it have compared to my current approach? Thank you! |
A common interface allows to use the same interceptor implementations in many places. Additionally, the code is tested and production ready. Documentation is here: https://spiral.dev/docs/framework-interceptors |
Description
This PR adds support for gRPC Server Interceptors to the Laravel Bridge package, providing powerful middleware-like functionality for gRPC services.
Features Added:
Server Interceptor Interface: Added
GrpcServerInterceptorInterface
for implementing custom interceptorsintercept(string $method, ContextInterface $context, string $body, callable $next)
Global Interceptors Support:
'interceptors'
array in grpc configurationService-Specific Interceptors:
Comprehensive Documentation:
Use Cases:
This enhancement brings Laravel-style middleware concepts to gRPC services, making it easier to implement cross-cutting concerns in a clean and reusable way.
Fixes # (issue)
Checklist
CHANGELOG.md
fileSummary by CodeRabbit
New Features
Documentation