Skip to content
Closed
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
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ jobs:
with:
dotnet-version: 6.0.x
cache: true
cache-dependency-path: src/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk.csproj
cache-dependency-path: |
src/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk.csproj
src/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk.Tests/Oddin.OddsFeedSdk.Tests.csproj

- name: Restore NuGet packages for Library
run: dotnet restore src/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk.csproj
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Threading.Tasks;
using Oddin.OddsFeedSdk.AMQP.Mapping;
using Oddin.OddsFeedSdk.AMQP.Messages;
using Oddin.OddsFeedSdk.API.Abstractions;
using Oddin.OddsFeedSdk.API.Entities.Abstractions;
using Oddin.OddsFeedSdk.Common;
using Oddin.OddsFeedSdk.Configuration;
using Oddin.OddsFeedSdk.Configuration.Abstractions;
using Oddin.OddsFeedSdk.Managers.Abstractions;
using Xunit;

namespace Oddin.OddsFeedSdk.Tests.AMQP.Mapping;

public class FeedMessageMapperTests
{
private const string MatchId = "od:match:123";
private const string TournamentId = "od:tournament:456";

[Fact]
public void MapBetStopPassesRoutingKeySportToMatchBuilder()
{
var builder = new CapturingSportDataBuilder();
var mapper = CreateMapper(builder);

mapper.MapBetStop<IMatch>(CreateMessage(MatchId, "42"), Array.Empty<CultureInfo>(), Array.Empty<byte>());

Assert.Equal(MatchId, builder.MatchId?.ToString());
Assert.Equal("od:sport:42", builder.MatchSportId?.ToString());
Assert.Equal(1, builder.MatchBuildCount);
}

[Fact]
public void MapOddsChangePassesRoutingKeySportToMatchBuilder()
{
var builder = new CapturingSportDataBuilder();
var mapper = CreateMapper(builder);
var message = new odds_change
{
event_id = MatchId,
product = 1,
timestamp = 1,
RoutingKey = "hi.pre.-.odds_change.42.-.123.-"
};

mapper.MapOddsChange<IMatch>(message, Array.Empty<CultureInfo>(), Array.Empty<byte>());

Assert.Equal("od:sport:42", builder.MatchSportId?.ToString());
Assert.Equal(1, builder.MatchBuildCount);
}

[Fact]
public void MapBetStopPreservesMatchFallbackWhenRoutingKeyHasNoSport()
{
var builder = new CapturingSportDataBuilder();
var mapper = CreateMapper(builder);

mapper.MapBetStop<IMatch>(CreateMessage(MatchId, "-"), Array.Empty<CultureInfo>(), Array.Empty<byte>());

Assert.Null(builder.MatchSportId);
Assert.Equal(1, builder.MatchBuildCount);
}

[Fact]
public void MapBetStopStillPassesRoutingKeySportToTournamentBuilder()
{
var builder = new CapturingSportDataBuilder();
var mapper = CreateMapper(builder);

mapper.MapBetStop<ITournament>(CreateMessage(TournamentId, "42"), Array.Empty<CultureInfo>(), Array.Empty<byte>());

Assert.Equal(TournamentId, builder.TournamentId?.ToString());
Assert.Equal("od:sport:42", builder.TournamentSportId?.ToString());
Assert.Equal(1, builder.TournamentBuildCount);
}

private static FeedMessageMapper CreateMapper(CapturingSportDataBuilder builder) =>
new(
new StubProducerManager(),
new StubFeedConfiguration(),
null,
builder);

private static bet_stop CreateMessage(string eventId, string sportSection) =>
new()
{
event_id = eventId,
product = 1,
timestamp = 1,
RoutingKey = $"hi.pre.-.bet_stop.{sportSection}.-.123.-"
};

private sealed class CapturingSportDataBuilder : ISportDataBuilder
{
public URN MatchId { get; private set; }
public URN MatchSportId { get; private set; }
public int MatchBuildCount { get; private set; }
public URN TournamentId { get; private set; }
public URN TournamentSportId { get; private set; }
public int TournamentBuildCount { get; private set; }

public IMatch BuildMatch(URN id, IEnumerable<CultureInfo> cultures, URN sportId = null)
{
MatchId = id;
MatchSportId = sportId;
MatchBuildCount++;
return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

}

public ITournament BuildTournament(URN id, URN sportId, IEnumerable<CultureInfo> locales)
{
TournamentId = id;
TournamentSportId = sportId;
TournamentBuildCount++;
return null;
}

public Task<IEnumerable<ISport>> BuildSports(IEnumerable<CultureInfo> locales) =>
Task.FromResult<IEnumerable<ISport>>(Array.Empty<ISport>());

public IEnumerable<ITournament> BuildTournaments(IEnumerable<URN> ids, URN sportId,
IEnumerable<CultureInfo> locales) => Array.Empty<ITournament>();

public ISport BuildSport(URN id, IEnumerable<CultureInfo> locales) => null;
public IEnumerable<ICompetitor> BuildCompetitors(IEnumerable<URN> ids,
IEnumerable<CultureInfo> cultures) => Array.Empty<ICompetitor>();
public IEnumerable<IMatch> BuildMatches(IEnumerable<URN> ids, IEnumerable<CultureInfo> cultures) =>
Array.Empty<IMatch>();
public ICompetitor BuildCompetitor(URN id, IEnumerable<CultureInfo> cultures) => null;
public IPlayer BuildPlayer(URN id, IEnumerable<CultureInfo> cultures) => null;
public IFixture BuildFixture(URN id, IEnumerable<CultureInfo> cultures) => null;
public IMatchStatus BuildMatchStatus(URN id, IEnumerable<CultureInfo> cultures) => null;
}

private sealed class StubProducerManager : IProducerManager
{
public IReadOnlyCollection<IProducer> Producers => Array.Empty<IProducer>();
public void DisableProducer(int id) { }
public IProducer Get(int id) => null;
public IProducer Get(string name) => null;
public bool Exists(int id) => false;
public bool Exists(string name) => false;
public void AddTimestampBeforeDisconnect(int id, DateTime timestamp) { }
public void RemoveTimestampBeforeDisconnect(int id) { }
public void Lock() { }
}

private sealed class StubFeedConfiguration : IFeedConfiguration
{
public string AccessToken => string.Empty;
public CultureInfo DefaultLocale => CultureInfo.GetCultureInfo("en-US");
public int MaxInactivitySeconds => 0;
public int MaxRecoveryExecutionMinutes => 0;
public int? NodeId => null;
public ExceptionHandlingStrategy ExceptionHandlingStrategy => ExceptionHandlingStrategy.CATCH;
public string Host => string.Empty;
public int Port => 0;
public bool UseSsl => false;
public string ApiHost => string.Empty;
public bool UseApiSsl => false;
public int HttpClientTimeout => 0;
public int InitialSnapshotTimeInMinutes => 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using Oddin.OddsFeedSdk.AMQP;
using Xunit;

namespace Oddin.OddsFeedSdk.Tests.AMQP;

public class TopicParsingHelperTests
{
[Theory]
[InlineData(null, null)]
[InlineData("", null)]
[InlineData("hi.pre.-.bet_stop.42.-.123.-", "od:sport:42")]
[InlineData("hi.pre.-.bet_stop.-.-.123.-", null)]
public void GetSportUrnParsesSportSection(string routingKey, string expected)
{
Assert.Equal(expected, TopicParsingHelper.GetSportURN(routingKey)?.ToString());
}
}

This file was deleted.

6 changes: 6 additions & 0 deletions src/Oddin.OddsFeedSdk/Oddin.OddsFeedSdk.sln
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Oddin.OddsFeedSdk", "Oddin.
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Oddin.OddsFeedSdkDemoIntegration", "Oddin.OddsFeedSdkDemoIntegration\Oddin.OddsFeedSdkDemoIntegration.csproj", "{D82F279F-1979-4EE8-A47B-7B75739E22E5}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Oddin.OddsFeedSdk.Tests", "Oddin.OddsFeedSdk.Tests\Oddin.OddsFeedSdk.Tests.csproj", "{2B829B71-6FB3-4A91-A17E-1D05BF8B7207}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -21,6 +23,10 @@ Global
{D82F279F-1979-4EE8-A47B-7B75739E22E5}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D82F279F-1979-4EE8-A47B-7B75739E22E5}.Release|Any CPU.ActiveCfg = Release|Any CPU
{D82F279F-1979-4EE8-A47B-7B75739E22E5}.Release|Any CPU.Build.0 = Release|Any CPU
{2B829B71-6FB3-4A91-A17E-1D05BF8B7207}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{2B829B71-6FB3-4A91-A17E-1D05BF8B7207}.Debug|Any CPU.Build.0 = Debug|Any CPU
{2B829B71-6FB3-4A91-A17E-1D05BF8B7207}.Release|Any CPU.ActiveCfg = Release|Any CPU
{2B829B71-6FB3-4A91-A17E-1D05BF8B7207}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ private ISportEvent MapSportEvent(URN id, URN sport, IEnumerable<CultureInfo> cu
if (sport is null) throw new ArgumentNullException(nameof(sport));
return _sportDataBuilder.BuildTournament(id, sport, cultures);
case URN.TypeMatch:
return _sportDataBuilder.BuildMatch(id, cultures);
return _sportDataBuilder.BuildMatch(id, cultures, sport);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

default:
throw new ArgumentException($"Sport event of unknown type '{id?.Type}'");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,10 @@
<PackageReference Include="System.Runtime.Caching" Version="6.0.0"/>
</ItemGroup>

</Project>
<ItemGroup>
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
<_Parameter1>Oddin.OddsFeedSdk.Tests</_Parameter1>
</AssemblyAttribute>
</ItemGroup>

</Project>
Loading