Azure / Azure/azure-sdk-for-cpp
Mock AMQP server cannot serve a link reattach
- Dominant language
- C++
- Stars
- 205
- Forks
- 172
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 37
Description
## Summary
The mock AMQP server in `sdk/core/azure-core-amqp/test/ut/mock_amqp_server.hpp` refuses a second attach on a link name that it holds. The server also leaves its message loop after the last link detaches, and it does not start the loop again. A test therefore cannot make the server detach a link and then accept a new attach from the client. The Event Hubs link rebuild code needs that sequence, so the rebuild code has no offline test today. The link rebuild work belongs to #7254.
## Motivation
Four properties of the mock server block the sequence.
- `MockServiceEndpoint::OnLinkAttached` refuses a duplicate link name, and the two roles fail in different ways. For an incoming receiver role, the method sends a detach with the condition `EntityAlreadyExists` and returns false (`mock_amqp_server.hpp:99-108`). For an incoming sender role, the method throws a `std::runtime_error` from the callback (`mock_amqp_server.hpp:138-144`).
- `MessageLoop` leaves the loop when both link maps become empty (`mock_amqp_server.hpp:343-347`). `OnLinkAttached` starts the loop thread only when `m_serverThread` is not joinable (`mock_amqp_server.hpp:155-161`). A `std::thread` stays joinable after its function returns, so the loop never starts again. The message dispatch, the link removal, and the call to `EnableLinkPolling` all run in that loop (`mock_amqp_server.hpp:298-351`). A new link therefore gets no service after the loop stops.
- The server offers no method that detaches a link on demand. The one call to `Session::SendDetach` is the duplicate name refusal (`mock_amqp_server.hpp:106`).
- `AmqpServerMock::OnConnectionStateChanged` cancels `m_listenerContext` when a connection reaches `End` or `Error` (`mock_amqp_server.hpp:699-704`). Both constructors copy that context into every service endpoint (`mock_amqp_server.hpp:552` and `mock_amqp_server.hpp:563`). A copy of `Azure::Core::Context` shares the cancellation state (`sdk/core/azure-core/inc/azure/core/context.hpp:225`), so one closed connection stops the whole server.
The Event Hubs client rebuilds a link after the service detaches it, and only a live namespace exercises that code today. `PartitionClient::RebuildReceiver` closes the faulted receiver, computes a new start position, and opens a new receiver with the same receiver name (`sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:230-262`). `PartitionClient::ReceiveEvents` holds a per call rebuild counter, and a received message resets that counter (`partition_client.cpp:298` and `partition_client.cpp:309`). The same method stores a pending error when the rebuild budget ends and the call already holds events (`partition_client.cpp:326-336`). The next call recovers from the pending error before it reads a message (`partition_client.cpp:373-378`). `ProducerClient::EnsureSenderOrInvalidate` discards the sender, the session, and the connection when an attach fails (`sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp:318-352`), and `ProducerClient::InvalidateSender` does that teardown (`producer_client.cpp:362-389`).
The offline tests cover only the decision functions. `sdk/eventhubs/azure-messaging-eventhubs/test/ut/reattach_policy_test.cpp` tests `ShouldRebuildReceiver`, `ShouldInvalidateSender`, and `ResumeStartPosition`. No offline test drives a rebuild sequence, because a rebuild needs a server that detaches a link and then accepts a new attach.
The build configuration makes the gap larger. `sdk/core/azure-core-amqp/CMakeLists.txt:36` forces `USE_RUST_AMQP` on, and only `DISABLE_RUST_IN_BUILD` turns it off (`sdk/core/azure-core-amqp/CMakeLists.txt:38-41`). No file under `eng/` sets `DISABLE_RUST_IN_BUILD`, and no pipeline matrix uses the `no-rust-amqp` presets (`CMakePresets.json:333`). No pipeline therefore builds uAMQP. The mock server compiles only under `ENABLE_UAMQP` (`mock_amqp_server.hpp:25`), so a new mock server test runs in a local uAMQP build until a pipeline builds that transport.
## Proposal
Give the mock server the behaviour that a reattach test needs.
- Add a method that detaches a named link on demand, with an AMQP error condition that the caller supplies. A test then makes the server send `amqp:link:detach-forced` to the client.
- Remove the link from `m_sender` or `m_receiver` when the server detaches it, so a new attach with the same name does not hit the duplicate name refusal in `OnLinkAttached`.
- Keep the message loop alive while the server listens, or start a new loop thread when a link attaches after the loop stopped. The joinable condition at `mock_amqp_server.hpp:155` needs a state flag instead, because a finished `std::thread` stays joinable.
- Report the duplicate name failure the same way for both roles. A throw from the callback thread ends the test process, so the sender role must send a detach with `EntityAlreadyExists` like the receiver role does.
- Add a method that detaches every link and keeps the connection open, so a test can drive the pending error path in `PartitionClient::ReceiveEvents`.
- Give each service endpoint a child context, so a closed connection stops that endpoint and not the whole server.
- Add a pipeline configuration that builds the uAMQP transport, so the new tests run in CI.
## Behaviours that become testable
- `PartitionClient::RebuildReceiver` opens a new receiver with the same receiver name after a forced detach.
- The rebuilt receiver starts after the last offset that the client received, through `_detail::ResumeStartPosition`.
- The rebuild counter in `ReceiveEvents` resets after a received message, so a long lived consumer keeps a full budget.
- `ReceiveEvents` returns the events that it holds, and it stores the pending error when the rebuild budget ends.
- The next call to `ReceiveEvents` recovers from the pending error before it reads a message.
- `ReceiveEvents` throws when the rebuild budget ends and the call holds no event.
- `ProducerClient::Send` rebuilds the sender after a forced detach and sends the batch.
- `EnsureSenderOrInvalidate` discards the session and the connection after a failed attach.
Contributor guide
Research direction
Start with sdk/core/azure-core-amqp/test/ut/mock_amqp_server.hpp, especially OnLinkAttached, MessageLoop, and OnConnectionStateChanged, then review reattach_policy_test.cpp and the Event Hubs entry points named in the issue. Run the existing offline tests in a local ENABLE_UAMQP build. Done means the mock can force-detach links, accept reattachments, keep endpoints isolated, and support the proposed rebuild tests and CI configuration.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, testing
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100