Azure / Azure/azure-sdk-for-cpp

Mock AMQP server cannot serve a link reattach

Open
#7,329 0 comments 0 reactions 0 assignees View on GitHub
needs-triage
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.