Implement Direct Reply-To. Rename RpcClient/RpcServer to Requester/Responder #135
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fixes #133 Signed-off-by: Gabriele Santomaggio <[email protected]>
fixes #133 Signed-off-by: Gabriele Santomaggio <[email protected]>
Gsantomaggio
changed the title
fixes #133 Signed-off-by: Gabriele Santomaggio <[email protected]>
fixes #133 Signed-off-by: Gabriele Santomaggio <[email protected]>
implement the direct reply queue address Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR renames RPC-related classes and interfaces to more descriptive names (RpcClient/RpcServer → Requester/Responder) and adds support for RabbitMQ's DirectReply-To feature for improved RPC performance. The DirectReply-To feature allows the server to send replies directly to clients without creating explicit reply queues when supported by RabbitMQ 4.2+.
Key changes:
- Renamed
IRpcClient/IRpcServertoIRequester/IResponderalong with their implementations and builders - Added DirectReply-To support with automatic detection based on RabbitMQ version
- Added
IConsumer.Queueproperty to expose the queue name (including server-generated DirectReply queue names) - Implemented URL encoding/decoding utilities for queue path segments
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
docs/Examples/Rpc/Program.cs |
Updated example code to use new Requester/Responder terminology |
Tests/RequesterResponser/ResponderTests.cs |
Renamed test class and updated all test methods to use new terminology |
Tests/RequesterResponser/RecoveryRPCTests.cs |
Enhanced recovery test to support both DirectReply and custom reply queue scenarios |
Tests/DirectReply/DirectReplyTests.cs |
Added new tests to validate DirectReply-To queue functionality |
Tests/Consumer/BasicConsumerTests.cs |
Updated test assertion to check for AMQP error code instead of queue name |
Tests/AddressBuilderTests.cs |
Added comprehensive tests for URL encoding/decoding of queue names with special characters |
RabbitMQ.AMQP.Client/Utils.cs |
Added CreateDirectReplyToAttach and DecodePathSegment utility methods |
RabbitMQ.AMQP.Client/PublicAPI.Unshipped.txt |
Updated public API surface to reflect renamed types and new methods |
RabbitMQ.AMQP.Client/Impl/AmqpResponder.cs |
Renamed from AmqpRpcServer with updated configuration classes |
RabbitMQ.AMQP.Client/Impl/AmqpRequester.cs |
Renamed from AmqpRpcClient with DirectReply-To support and GetReplyToQueue() method |
RabbitMQ.AMQP.Client/Impl/AmqpPublisher.cs |
Minor code style improvements (removed Object. prefix) |
RabbitMQ.AMQP.Client/Impl/AmqpConsumerBuilder.cs |
Added DirectReply-To configuration option and improved error handling |
RabbitMQ.AMQP.Client/Impl/AmqpConsumer.cs |
Added Queue property, DirectReply-To attach logic, and refactored validation |
RabbitMQ.AMQP.Client/Impl/AmqpConnection.cs |
Updated builder methods and added DirectReply-To feature flag detection |
RabbitMQ.AMQP.Client/Impl/AddressBuilder.cs |
Added DecodeQueuePathSegment method and code simplifications |
RabbitMQ.AMQP.Client/IResponder.cs |
Renamed from IRpcServer interface |
RabbitMQ.AMQP.Client/IRequester.cs |
Renamed from IRpcClient interface with added GetReplyToQueue() method |
RabbitMQ.AMQP.Client/IConsumerBuilder.cs |
Added DirectReplyTo configuration method |
RabbitMQ.AMQP.Client/IConsumer.cs |
Added Queue property to expose consumer's queue name |
RabbitMQ.AMQP.Client/IConnection.cs |
Renamed builder methods to use Requester/Responder terminology |
RabbitMQ.AMQP.Client/FeatureFlags.cs |
Added IsDirectReplyToSupported feature flag |
.ci/ubuntu/one-node/gha-setup.sh |
Updated RabbitMQ Docker image from 4.2-rc to 4.2 release |
.ci/ubuntu/cluster/gha-setup.sh |
Updated RabbitMQ Docker image from 4.2-rc to 4.2 release |
Comments suppressed due to low confidence (13)
Tests/RequesterResponser/RecoveryRPCTests.cs:27
- Comment incorrectly references "client" instead of "requester" in the test name. The test name is
ResponderAndClientShouldRecoverAfterKillConnectionbut should beResponderAndRequesterShouldRecoverAfterKillConnectionto match the naming convention used throughout the PR where "RpcClient" has been renamed to "Requester".
RabbitMQ.AMQP.Client/Impl/AmqpRequester.cs:163 - Inconsistent spacing in comment formatting. Line 162 has a trailing space after "reply-to" and line 163 starts with extra leading spaces that don't match the indentation style of the rest of the comment.
RabbitMQ.AMQP.Client/IResponder.cs:12 - Documentation comment contains outdated terminology. The comment mentions "IRpcServerBuilder" but should reference "IResponderBuilder" to match the interface name. The description should also update "RPC server" to "Responder" to be consistent with the new naming convention.
Tests/RequesterResponser/ResponderTests.cs:159 - Documentation comment is outdated. The comment mentions "client has to create a reply queue" but now with the DirectReply feature, the test behavior is different - when DirectReply is supported, no reply queue is explicitly created. The comment should be updated to reflect that this tests both DirectReply (when supported) and temporary queue creation (when not supported).
Tests/RequesterResponser/RecoveryRPCTests.cs:21 - The documentation comment says "In this test the Requester has to use the ReplyToQueue provided by the user" but this doesn't accurately describe the test behavior anymore. The test now uses both
DirectReplyfeature (when useReplyToQueue is false) and custom reply queue (when useReplyToQueue is true). The comment should be updated to reflect this dual behavior.
RabbitMQ.AMQP.Client/Impl/AmqpRequester.cs:94 - Documentation comment contains outdated and grammatically incorrect information:
- "AmqpRpcClient" should be "AmqpRequester" to match the renamed class
- "even the PublishAsync is async the RPClient blocks" is grammatically incorrect and unclear - should clarify that despite being async, it waits for the response
- "RPClient" should be "Requester" to match the new naming
RabbitMQ.AMQP.Client/Impl/AmqpRequester.cs:24
- The field order in the
RequesterConfigurationclass is inconsistent. TheRequestAddressfield (line 24) should be grouped with the other request-related fields likeReplyToQueueat the top, rather than being placed at the bottom after the function properties. This improves code readability and maintainability.
RabbitMQ.AMQP.Client/IResponder.cs:71 - Grammar issue in documentation: "where wants to receive" should be "where it wants to receive" or "where the requester wants to receive". The current phrasing is missing a subject.
RabbitMQ.AMQP.Client/Impl/AmqpRequester.cs:152 - Grammar issue: "don't use it" should be "not use it". The phrase "can decide to don't use it" is grammatically incorrect and should be "can decide not to use it".
RabbitMQ.AMQP.Client/IRequester.cs:105 - The documentation comment is unclear and has grammatical issues. The third bullet point "The server uses this address to send the reply message. with direct-reply-to" is confusing - it appears to be an incomplete sentence or two sentences merged together. Consider revising to clarify when direct-reply-to is used versus when a temporary queue is created.
RabbitMQ.AMQP.Client/Impl/AmqpResponder.cs:72 - Documentation comment is outdated. The comment still refers to "AmqpRpcServer" and "RpcClient" but these have been renamed to "AmqpResponder" and "Requester" respectively. The comment should be updated to use the new terminology.
RabbitMQ.AMQP.Client/Impl/AmqpResponder.cs:21 - Documentation comment is outdated. The comment still refers to "AmqpRpcServerBuilder" but the class has been renamed to "AmqpResponderBuilder". The comment should be updated to match the new class name.
RabbitMQ.AMQP.Client/IResponder.cs:72 - Documentation comment contains outdated terminology. Line 72 still refers to "RPC client" and "RPC server" but should be updated to "Requester" and "Responder" to match the new naming convention used throughout this PR.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <summary> | ||
| /// Returns queue name the consumer is consuming from. | ||
| /// The Queue name is usually configured by the user via the <see cref="IConsumerBuilder"/>, | ||
| /// but can also be generated by the client the special direct-reply-to queue. |
There was a problem hiding this comment.
Grammar issue: "by the client the special" should be "by the client for the special". The sentence is missing the preposition "for".
Suggested change
| /// but can also be generated by the client the special direct-reply-to queue. | |
| /// but can also be generated by the client for the special direct-reply-to queue. |
| { | ||
| if (_receiverLink is null) | ||
| { | ||
| throw new ConsumerException($"{ToString()} Receiver link creation failed (null was returned)"); |
There was a problem hiding this comment.
Redundant call to 'ToString' on a String object.
| if (_receiverLink.LinkState != LinkState.Attached) | ||
| { | ||
| string errorMessage = _receiverLink.Error?.ToString() ?? "Unknown error"; | ||
| throw new ConsumerException($"{ToString()} Receiver link not attached. Error: {errorMessage}"); |
There was a problem hiding this comment.
Redundant call to 'ToString' on a String object.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR renames RPC-related classes and interfaces to more descriptive names (RpcClient/RpcServer → Requester/Responder) and adds support for RabbitMQ's DirectReply-To feature for improved RPC performance. The DirectReply-To feature allows the server to send replies directly to clients without creating explicit reply queues when supported by RabbitMQ 4.2+.
Fixes Support Support Direct Reply-To #133