fix(battle-node): collision-safe battle-id registration + viewer eviction
RegisterPending → TryRegisterPending (TryAdd instead of indexer) so battle-id collisions return false instead of silently evicting a live battle. MatchingBridge retries with fresh IDs on collision (max 5). Before registering, EvictStaleForViewer removes any stale pending battle the viewer left behind, enforcing the one-pending-per-viewer invariant that was previously comment-asserted. Store tests switched to per-test local stores to fix a race under the assembly-wide ParallelScope.All. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -24,20 +24,35 @@ public sealed class MatchingBridge : IMatchingBridge
|
||||
_options = options;
|
||||
}
|
||||
|
||||
private const int MaxIdRetries = 5;
|
||||
|
||||
public PendingMatch RegisterBattle(BattlePlayer p1, BattlePlayer? p2, BattleType type)
|
||||
{
|
||||
ValidateContract(p1, p2, type);
|
||||
EvictStaleForViewer(p1.ViewerId);
|
||||
if (p2 is not null) EvictStaleForViewer(p2.ViewerId);
|
||||
|
||||
// Decimal battle id mirrors the captures (e.g. "975695075012"): two unbiased
|
||||
// BattleIdHalfDigits-wide draws concatenated. RandomNumberGenerator.GetInt32 uses
|
||||
// rejection sampling so each half is uniform on [0, BattleIdHalfExclusiveMax).
|
||||
var hi = RandomNumberGenerator.GetInt32(0, BattleIdHalfExclusiveMax);
|
||||
var lo = RandomNumberGenerator.GetInt32(0, BattleIdHalfExclusiveMax);
|
||||
var halfFormat = "D" + BattleIdHalfDigits;
|
||||
var battleId = hi.ToString(halfFormat) + lo.ToString(halfFormat);
|
||||
|
||||
_store.RegisterPending(new PendingBattle(battleId, type, p1, p2));
|
||||
return new PendingMatch(battleId, _options.NodeServerUrl);
|
||||
for (var attempt = 0; attempt < MaxIdRetries; attempt++)
|
||||
{
|
||||
var hi = RandomNumberGenerator.GetInt32(0, BattleIdHalfExclusiveMax);
|
||||
var lo = RandomNumberGenerator.GetInt32(0, BattleIdHalfExclusiveMax);
|
||||
var battleId = hi.ToString(halfFormat) + lo.ToString(halfFormat);
|
||||
|
||||
if (_store.TryRegisterPending(new PendingBattle(battleId, type, p1, p2)))
|
||||
return new PendingMatch(battleId, _options.NodeServerUrl);
|
||||
}
|
||||
|
||||
throw new InvalidOperationException(
|
||||
$"Failed to mint a unique battle id after {MaxIdRetries} attempts.");
|
||||
}
|
||||
|
||||
private void EvictStaleForViewer(long viewerId)
|
||||
{
|
||||
var stale = _store.TryFindPendingForViewer(viewerId);
|
||||
if (stale is not null)
|
||||
_store.RemovePending(stale.BattleId);
|
||||
}
|
||||
|
||||
private static void ValidateContract(BattlePlayer p1, BattlePlayer? p2, BattleType type)
|
||||
|
||||
@@ -2,8 +2,10 @@ namespace SVSim.BattleNode.Sessions;
|
||||
|
||||
public interface IBattleSessionStore
|
||||
{
|
||||
/// <summary>Register a battle minted by the matching bridge, awaiting a WS connect.</summary>
|
||||
void RegisterPending(PendingBattle battle);
|
||||
/// <summary>Register a battle minted by the matching bridge, awaiting a WS connect.
|
||||
/// Returns false if a battle with the same id is already pending (caller should retry
|
||||
/// with a fresh id).</summary>
|
||||
bool TryRegisterPending(PendingBattle battle);
|
||||
|
||||
/// <summary>Look up the pending battle. Returns null if not present.</summary>
|
||||
PendingBattle? TryGetPending(string battleId);
|
||||
|
||||
@@ -6,8 +6,8 @@ public sealed class InMemoryBattleSessionStore : IBattleSessionStore
|
||||
{
|
||||
private readonly ConcurrentDictionary<string, PendingBattle> _pending = new();
|
||||
|
||||
public void RegisterPending(PendingBattle battle) =>
|
||||
_pending[battle.BattleId] = battle;
|
||||
public bool TryRegisterPending(PendingBattle battle) =>
|
||||
_pending.TryAdd(battle.BattleId, battle);
|
||||
|
||||
public PendingBattle? TryGetPending(string battleId) =>
|
||||
_pending.TryGetValue(battleId, out var b) ? b : null;
|
||||
|
||||
@@ -76,6 +76,21 @@ public class MatchingBridgeTests
|
||||
new BattlePlayer(1, FixtureCtx()), new BattlePlayer(2, FixtureCtx()), BattleType.Bot));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void RegisterBattle_evicts_stale_pending_for_same_viewer()
|
||||
{
|
||||
var store = new InMemoryBattleSessionStore();
|
||||
var bridge = new MatchingBridge(store, new BattleNodeOptions());
|
||||
var p1 = new BattlePlayer(42, FixtureCtx());
|
||||
|
||||
var first = bridge.RegisterBattle(p1, p2: null, BattleType.Bot);
|
||||
Assert.That(store.TryGetPending(first.BattleId), Is.Not.Null);
|
||||
|
||||
var second = bridge.RegisterBattle(p1, p2: null, BattleType.Bot);
|
||||
Assert.That(store.TryGetPending(first.BattleId), Is.Null, "stale entry must be evicted");
|
||||
Assert.That(store.TryGetPending(second.BattleId), Is.Not.Null);
|
||||
}
|
||||
|
||||
private static MatchContext FixtureCtx() => new(
|
||||
SelfDeckCardIds: Enumerable.Range(1, 30).Select(i => 100_011_010L).ToList(),
|
||||
ClassId: "1", CharaId: "1", CardMasterName: "card_master_node_10015",
|
||||
|
||||
@@ -7,39 +7,40 @@ namespace SVSim.UnitTests.BattleNode.Sessions;
|
||||
[TestFixture]
|
||||
public class InMemoryBattleSessionStoreTests
|
||||
{
|
||||
private InMemoryBattleSessionStore _store = null!;
|
||||
|
||||
[SetUp] public void Setup() => _store = new InMemoryBattleSessionStore();
|
||||
|
||||
[Test]
|
||||
public void RegisterThenGet_ReturnsRegisteredBattle()
|
||||
public void TryRegisterThenGet_ReturnsRegisteredBattle()
|
||||
{
|
||||
var store = new InMemoryBattleSessionStore();
|
||||
var battle = new PendingBattle("bid-1", BattleType.Bot, new BattlePlayer(906243102, FixtureCtx()), null);
|
||||
_store.RegisterPending(battle);
|
||||
Assert.That(store.TryRegisterPending(battle), Is.True);
|
||||
|
||||
Assert.That(_store.TryGetPending("bid-1"), Is.EqualTo(battle));
|
||||
Assert.That(store.TryGetPending("bid-1"), Is.EqualTo(battle));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Get_UnknownBattleId_ReturnsNull()
|
||||
{
|
||||
Assert.That(_store.TryGetPending("nope"), Is.Null);
|
||||
var store = new InMemoryBattleSessionStore();
|
||||
Assert.That(store.TryGetPending("nope"), Is.Null);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Remove_ReturnsTrueWhenPresent_FalseWhenAbsent()
|
||||
{
|
||||
_store.RegisterPending(new PendingBattle("bid", BattleType.Bot, new BattlePlayer(1, FixtureCtx()), null));
|
||||
Assert.That(_store.RemovePending("bid"), Is.True);
|
||||
Assert.That(_store.RemovePending("bid"), Is.False);
|
||||
var store = new InMemoryBattleSessionStore();
|
||||
store.TryRegisterPending(new PendingBattle("bid", BattleType.Bot, new BattlePlayer(1, FixtureCtx()), null));
|
||||
Assert.That(store.RemovePending("bid"), Is.True);
|
||||
Assert.That(store.RemovePending("bid"), Is.False);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Register_DuplicateBattleId_OverwritesPrior()
|
||||
public void TryRegister_DuplicateBattleId_ReturnsFalseAndPreservesOriginal()
|
||||
{
|
||||
_store.RegisterPending(new PendingBattle("bid", BattleType.Bot, new BattlePlayer(1, FixtureCtx()), null));
|
||||
_store.RegisterPending(new PendingBattle("bid", BattleType.Bot, new BattlePlayer(2, FixtureCtx()), null));
|
||||
Assert.That(_store.TryGetPending("bid")!.P1.ViewerId, Is.EqualTo(2));
|
||||
var store = new InMemoryBattleSessionStore();
|
||||
store.TryRegisterPending(new PendingBattle("bid", BattleType.Bot, new BattlePlayer(1, FixtureCtx()), null));
|
||||
var second = store.TryRegisterPending(new PendingBattle("bid", BattleType.Bot, new BattlePlayer(2, FixtureCtx()), null));
|
||||
Assert.That(second, Is.False);
|
||||
Assert.That(store.TryGetPending("bid")!.P1.ViewerId, Is.EqualTo(1));
|
||||
}
|
||||
|
||||
private static MatchContext FixtureCtx() => new(
|
||||
|
||||
Reference in New Issue
Block a user