fix(amqp): pass sport URN from routing key when building match events

[hrubymar10]

Summary

Fixes #57 — IMatch.Tournament returned null (and match.SportId triggered a REST call on every access) because the sport URN extracted from every AMQP routing key was never forwarded when building IMatch objects.

• FeedMessageMapper.MapSportEvent now passes the routing-key sport URN to BuildMatch (the BuildMatch signature already supported it — the tournament branch already did this). When the summary REST call fails, SportId and Tournament now still resolve from routing-key data; when the routing key carries no sport (- section), behavior degrades to the previous REST fallback.
• Replaces the CORE-3368 coverage bootstrap stub with real tests: 8 xunit tests covering sport propagation through MapBetStop and MapOddsChange, the -/null/empty routing-key fallback, the tournament branch, and TopicParsingHelper parsing (hand-rolled fakes, no mocking dependency).
• Registers the test project in the solution (it was CI-only) and adds InternalsVisibleTo for the test assembly.
• CI: includes the test csproj in the NuGet cache key; otherwise keeps main's coverage pipeline as-is.

Validation

• dotnet test -c Release --collect:"XPlat Code Coverage": 8/8 passed, non-empty Cobertura report (147/6177 lines vs. empty bootstrap before).
• dotnet build Oddin.OddsFeedSdk.sln -c Release: clean, 0 warnings, across library, tests, and demo integration.

[dsaiko]

Requesting changes: this does not fix #57

Thanks for digging into this — the diagnosis in #57 is accurate and the one-line change is a real improvement. But tracing it through Match.cs, the reported bug is still present after this PR, and the PR description claims otherwise.

1. Tournament is still null when the summary call fails (blocking)

FetchTournament() is unchanged by this PR:

private ITournament FetchTournament()
{
    var sportId = SportId;                                    // now non-null from the routing key
    if (sportId is null) return null;
    var tournamentId = FetchMatch(_cultures)?.TournamentId;    // still requires the summary
    if (tournamentId is null) return null;                     // still returns null
    return _sportDataBuilder.BuildTournament(tournamentId, sportId, _cultures);
}

The routing key carries no tournament ID — TopicParsingHelper exposes only SPORT_SECTION_INDEX (4) and EVENT_ID_SECTION_INDEX (6). So tournamentId can only come from the summary fetch. Walking both branches:

Summary REST call	Before this PR	After this PR
succeeds	SportId resolves via FetchMatch()?.SportId; Tournament non-null	identical result; SportId no longer needs the fetch
fails	SportId null → Tournament null → NRE on await	SportId non-null, but tournamentId null → Tournament still null → same NRE

So the failure mode described in #57 — await match.Tournament.GetSportAsync() throwing NullReferenceException after a transient summary failure — is unchanged. And in the branch where the summary succeeds, Tournament was already non-null before the PR, because the FetchMatch(_cultures)?.SportId fallback filled sportId in.

This makes the PR description inaccurate:

when the summary REST call fails, SportId and Tournament now still resolve from routing-key data

True for SportId. Not true for Tournament, and can't be — the data isn't in the routing key.

Under ExceptionHandlingStrategy.THROW it's arguably a small step backwards for diagnosis: the exception flips from ItemNotFoundException("Cannot load sport") to ItemNotFoundException("Cannot load tournament"), now pointing at tournament resolution when the actual failure is the summary fetch.

To be clear about what the change does buy, because it is worth having: match.SportId and match.Sport no longer depend on the summary at all. That's a genuine win — just not the one #57 asks for.

2. The routing-key sport now silently outranks the summary

FetchSportId() is _localSportId ?? FetchMatch(_cultures)?.SportId, and _localSportId is memoized. Once the routing key supplies a sport, the summary-derived sport is never consulted and never compared. A mis-routed (or hostile) delivery relabels that IMatch under a different sport, and FetchTournament then passes that same wrong sportId into BuildTournament — so match.Sport and match.Tournament.Sport agree with each other and are both wrong, which is the expensive kind of wrong to debug.

Scope is narrower here than it would be in a shared-cache design (each IMatch is constructed per message), so this is not a blocker on its own — but the precedence is worth inverting while you're in here. For reference, gosdk treats the API summary as authoritative and the routing key as fallback-only, logging on conflict: internal/cache/match.go#L431-L451.

3. The tests can't catch #1

CapturingSportDataBuilder.BuildMatch returns null, so the suite verifies only that the sport argument reaches the builder. Nothing exercises a real Match resolving SportId/Tournament, which is where the bug lives — hence 8 green tests over a fix that doesn't fix the issue.

Requested changes

1. Rescope or actually fix. Either drop the Tournament claim and land this as the SportId/Sport improvement it is (with #57 staying open), or make FetchTournament genuinely resilient — which needs a tournament ID from somewhere other than the summary, so realistically the former plus a follow-up.
2. Add the test that would have caught it: a fake match cache whose fetch fails, a routing key carrying a sport, then assert on match.Tournament. It should fail today.
3. Invert the sport precedence in FetchSportId — prefer the summary value, fall back to _localSportId, log on conflict.
4. Keep #57 open regardless of how this lands.

Happy to be wrong on any of this if I've misread the caching layer — but I'd want a red-then-green test on the summary-failure path before this closes #57.

---

Reviewed by Claude Opus 5 via Claude Code, on behalf of @dsaiko — cross-checked against the equivalent path in gosdk.

[dsaiko]

This line is correct and I'd keep it — but it does not fix #57, and the PR description says it does.

FetchTournament() in Match.cs still sources the tournament ID from the summary:

var sportId = SportId;                                   // ✓ non-null now
if (sportId is null) return null;
var tournamentId = FetchMatch(_cultures)?.TournamentId;   // ← still needs the summary
if (tournamentId is null) return null;                    // ← still null when it fails

The routing key has no tournament section (TopicParsingHelper parses only sport at index 4 and event at index 6), so when the summary fetch fails, Tournament is null exactly as before and the NullReferenceException from await match.Tournament.GetSportAsync() still happens.

And when the summary succeeds, SportId already resolved through the old FetchMatch(_cultures)?.SportId fallback — so Tournament was already non-null in that branch. Net effect of this line: match.SportId and match.Sport stop depending on the summary. Real and worth merging, but it isn't the #57 fix.

---

Reviewed by Claude Opus 5 via Claude Code, on behalf of @dsaiko — cross-checked against the equivalent path in gosdk.

[dsaiko]

Second, separable concern on the same line (commenting here since Match.cs isn't in the diff).

FetchSportId() is _localSportId ?? FetchMatch(_cultures)?.SportId with _localSportId memoized. Passing sport here means the routing-key value unconditionally wins over the summary-derived sport, with no comparison — and FetchTournament then forwards that same value into BuildTournament, so match.Sport and match.Tournament.Sport would agree with each other while both being wrong.

Per-message IMatch construction keeps the blast radius small, so not a blocker, but I'd invert the precedence while you're here: prefer the summary sport, use the routing-key value only when the summary carries none, and log a warning on conflict. That's what gosdk settled on — internal/cache/match.go#L431-L451.

---

Reviewed by Claude Opus 5 via Claude Code, on behalf of @dsaiko — cross-checked against the equivalent path in gosdk.

[dsaiko]

The fake returning null is why the suite can't catch the gap in #57.

Every assertion here is about whether the sportId argument arrives at the builder — which the one-line change plainly does. Nothing constructs a real Match and asks it for SportId or Tournament, so the actual reported failure (summary fetch fails → Tournament is null → NRE on await) is untested in both directions.

Suggested addition: a fake IMatchCache whose lookup returns null/throws, build a real Match with a non-null localSportId, then assert match.Tournament. Under CATCH that returns null today even with this fix applied, which is the regression test #57 needs.

---

Reviewed by Claude Opus 5 via Claude Code, on behalf of @dsaiko — cross-checked against the equivalent path in gosdk.

[hrubymar10]

Superseded by #63