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

Fix consumer recovery with server-named queues #1324

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

lukebakken
Copy link
Contributor

Fixes #1238

  • Add failing test
  • Fix RecordedConsumer to allow the empty string for a queue name

@lukebakken lukebakken added this to the 6.5.0 milestone Mar 23, 2023
@lukebakken lukebakken self-assigned this Mar 23, 2023
@lukebakken lukebakken marked this pull request as ready for review March 23, 2023 16:29
Copy link
Member

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

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

This looks reasonable but just for the record, in the Java client
we intentionally decided to recover server-named queues in a way
that gives them new server-generated names (which are then propagated to consumers/bindings).

That makes the implementation more complex but can also seem logical
to some users.

Fixes #1238

* Add failing test
* Fix `RecordedConsumer` to allow the empty string for a queue name

* Add `CurrentQueue` to `IModel` to keep track of the last declared queue name as defined in the AMQP 091 spec

* Fix `RecordedConsumer` to use `CurrentQueue` when passed in name is `string.Empty`

Fixup API Approval
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1238 branch from 865dced to 7590feb Compare March 23, 2023 19:55
lukebakken added a commit that referenced this pull request Mar 23, 2023
Port of #1324 to main

Fixes #1238

* Add failing test
* Fix `RecordedConsumer` to allow the empty string for a queue name
* Add `CurrentQueue` to `IChannel` to keep track of the last declared queue name as defined in the AMQP 091 spec
* Fix `RecordedConsumer` to use `CurrentQueue` when passed in name is `string.Empty`
* Fixup API Approval
@lukebakken
Copy link
Contributor Author

recover server-named queues in a way that gives them new server-generated names (which are then propagated to consumers/bindings)

Yep, that's exactly how it works here as well. Recovery creates new server-named queues and the new queue name is propagated to the correct RecordedQueue, RecordedConsumer etc objects.

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

Successfully merging this pull request may close these issues.

2 participants