Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,57 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Fixed — BREAKING: `ExternalMediaActivity` could not reach an AudioSocket stream by any configuration (#N)

`ExternalMediaActivity` accepts an `AudioSocketServer` in its constructor and then polls
`GetStream(Channel.Id)` for the stream. It could never hit. Measured against a real Asterisk 22.9.0
and 23.4.1, which behave identically:

- With the activity's **defaults**, `POST /channels/externalMedia` returns 200 and a **UnicastRTP**
channel. Nothing connects to the AudioSocket server, and the poll waits out its 30-second timeout.
- Setting `Encapsulation = "audiosocket"` returns `HTTP 400 transport must be 'tcp' for audiosocket
encapsulation`, because the activity sends `transport` only when a caller sets it.
- Supplying the transport too returns `HTTP 400 data can not be empty`.

And underneath those, the identifiers never matched. Asterisk's `data` parameter becomes the UUID in
the identification frame — the value the stream table is keyed by — while `channelId` becomes the ARI
`Channel.Id`. A probe with two distinct, byte-order-asymmetric UUIDs settles which travels:

```text
channelId = 0a1b2c3d-4e5f-6071-8293-a4b5c6d7e8f9
data = f9e8d7c6-b5a4-3928-1706-f5e4d3c2b1a0

channel.id : 0a1b2c3d-4e5f-6071-8293-a4b5c6d7e8f9
HEX : 01 00 10 f9 e8 d7 c6 b5 a4 39 28 17 06 f5 e4 d3 c2 b1 a0
UUID on the wire == channelId ? False == data ? True
```

- **`CreateExternalMediaAsync` gains a `channelId` parameter** on `IAriChannelsResource` and
`AriChannelsResource`. Asterisk documents it as the unique id to assign the channel on creation, and
spells it camelCase among snake_case siblings — a misspelling is ignored silently, so the URL test
asserts the literal.
- **The activity derives its request from the server it was handed.** AudioSocket encapsulation
implies `transport = "tcp"`, and one freshly minted UUID goes to both `channelId` and `data`, so
`Channel.Id` **is** the key. The mint is canonical lowercase, and that is load-bearing: the table is
an ordinal dictionary keyed by `Guid.ToString()`, and an uppercase identifier creates a channel
whose stream cannot be found.
- **An `AudioSocketServer` supplied against a non-AudioSocket encapsulation is now refused** at the
start, instead of creating an RTP channel and reporting a connection failure thirty seconds later.
- **The contract text now says what the key is.** `IAudioServer.GetStream` said "Get an active stream
by channel ID" — on the interface and, verbatim, on both shipped implementations. It now names the
key and its form for each, including that `WebSocketAudioServer` keys on the last segment of the
request URL instead. That second mismatch is real and is tracked separately, with a probe as its
first task rather than a design.

**What a consumer must do.** Recompile, and name the cancellation token if you passed it positionally.
The parameter was **appended** rather than placed first among the optionals, deliberately: every slot
from `encapsulation` to `data` is `string?`, so inserting ahead of them would have rebound each
argument one place over and gone on compiling — wrong values on the wire with no diagnostic anywhere.
Appending produces `CS1503`, which is loud. Anyone implementing `IAriChannelsResource` gains a member.

**Migration guide:** [`docs/guides/externalmedia-channel-id-migration.md`](docs/guides/externalmedia-channel-id-migration.md)
— required by ADR-0028 for a minor that carries a breaking change.

### Changed — BREAKING: AudioSocket spoke a protocol Asterisk does not, so neither server ever completed a handshake (#302)

Both AudioSocket implementations read a **four**-byte frame header. Asterisk sends **three**: one byte
Expand Down
332 changes: 326 additions & 6 deletions Tests/Verbara.Sdk.Activities.Tests/Activities/ActivityTests.cs

Large diffs are not rendered by default.

36 changes: 36 additions & 0 deletions Tests/Verbara.Sdk.Ari.Tests/Audio/AudioSocketServerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,42 @@ public async Task HandleConnection_ShouldRegisterStream_WhenUuidReceived()
registeredStream.Format.Should().Be("slin16");
}

[Fact]
public async Task GetStream_ShouldMiss_WhenIdentifierIsNotCanonicalLowercase()
{
// The negative control behind ExternalMediaActivity minting its identifier with
// Guid.ToString(). The fixture is a measurement, not an argument: probe-capture.txt RUN U
// sent this exact UUID to a real Asterisk — 22.9.0 and 23.4.1, identically — as both
// channelId and data:
// -> HTTP 200 id=EC30994A-B0E1-4EBE-99BD-19CA35669F7A
// HEX: 01 00 10 ec 30 99 4a b0 e1 4e be 99 bd 19 ca 35 66 9f 7a
// Asterisk accepted the uppercase spelling and echoed Channel.Id back in it, while the
// identification frame carried sixteen raw bytes. This test puts those sixteen bytes into
// the real server and then asks it for the stream BOTH ways — the way a consumer holding
// Channel.Id would, and the way the table is actually keyed.
const string asSentToAsterisk = "EC30994A-B0E1-4EBE-99BD-19CA35669F7A";
var wireUuid = Guid.Parse(asSentToAsterisk);
var canonical = wireUuid.ToString();
canonical.Should().NotBe(asSentToAsterisk,
"the fixture has to differ in spelling from its canonical form, or this test controls nothing");

var port = GetFreePort();
var server = CreateServer(port);
await server.StartAsync();

using var client = await ConnectAndSendUuidAsync(port, wireUuid, server);

server.ActiveStreamCount.Should().Be(1, "the identification frame was accepted");
server.GetStream(canonical).Should().NotBeNull(
"ParseUuid renders the sixteen wire bytes with new Guid(bytes, bigEndian: true).ToString(), "
+ "so the table is keyed by the canonical lowercase hyphenated form");
server.GetStream(asSentToAsterisk).Should().BeNull(
"the table is a ConcurrentDictionary<string, ...> with the default ORDINAL comparer. A "
+ "channel created with a non-canonical channelId comes back with Channel.Id in that "
+ "spelling, so GetStream(Channel.Id) misses — HTTP 200, a live stream, and no error on "
+ "any hop. That silence is why the identifier is minted rather than accepted");
}

[Fact]
public async Task HandleConnection_ShouldEmitOnStreamConnected_WhenUuidReceived()
{
Expand Down
20 changes: 19 additions & 1 deletion Tests/Verbara.Sdk.Ari.Tests/Resources/AriResourceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,14 @@ public async Task Channels_CreateExternalMediaAsync_ShouldPostWithParams()
using var http = CreateHttpClient(handler);
var sut = new AriChannelsResource(http, DefaultOptions);

// Every optional is SUPPLIED, not just asserted on. Until this change the test passed
// encapsulation and transport only, and the connection_type, direction and data appenders had
// never executed across the whole Ari suite — an assertion about a parameter the test does not
// send holds whatever the appender does, including nothing.
var result = await sut.CreateExternalMediaAsync("myapp", "127.0.0.1:8000", "slin16",
encapsulation: "rtp", transport: "udp");
encapsulation: "rtp", transport: "udp", connectionType: "client", direction: "both",
data: "f9e8d7c6-b5a4-3928-1706-f5e4d3c2b1a0",
channelId: "0a1b2c3d-4e5f-6071-8293-a4b5c6d7e8f9");

result.Id.Should().Be("ch-ext");
handler.LastMethod.Should().Be(HttpMethod.Post);
Expand All @@ -168,6 +174,18 @@ public async Task Channels_CreateExternalMediaAsync_ShouldPostWithParams()
handler.LastRequestUri.Should().Contain("format=slin16");
handler.LastRequestUri.Should().Contain("encapsulation=rtp");
handler.LastRequestUri.Should().Contain("transport=udp");
handler.LastRequestUri.Should().Contain("connection_type=client");
handler.LastRequestUri.Should().Contain("direction=both");
handler.LastRequestUri.Should().Contain("data=f9e8d7c6-b5a4-3928-1706-f5e4d3c2b1a0");

// The literal spelling, deliberately. Asterisk spells this one parameter in camelCase among
// snake_case siblings and ignores an unrecognised query parameter silently — "channel_id="
// would leave the create returning HTTP 200 with an Asterisk-minted id and no error anywhere,
// so only a test on the exact bytes catches the misspelling.
handler.LastRequestUri.Should().Contain("channelId=0a1b2c3d-4e5f-6071-8293-a4b5c6d7e8f9",
"Asterisk spells this parameter channelId, and silently ignores any other spelling");
handler.LastRequestUri.Should().NotContain("channel_id=",
"the snake_case spelling the siblings use is the misspelling this assertion exists to catch");
}

[Fact]
Expand Down
Loading
Loading