Skip to content

Commit e846dab

Browse files
committed
Fix ALL-security subscriptions returning wrong SecurityId in OrderBookIncrementManager
For ALL-security subscriptions, TryApply() cached _byId[subscriptionId] = first security's BookInfo. Since ALL subscriptions share one subscriptionId across all securities, every subsequent security incorrectly reused the first security's builder, producing QuoteChangeMessages with wrong SecurityId. Remove _byId.Add for ALL-sec subscriptions so each call goes through _online[securityId] lookup, giving each security its own OrderBookIncrementBuilder.
1 parent 7076fc6 commit e846dab

3 files changed

Lines changed: 91 additions & 2 deletions

File tree

Algo/OrderBookIncrementManagerState.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ public QuoteChangeMessage TryApply(long subscriptionId, QuoteChangeMessage quote
9494
info.SubscriptionIds.Add(subscriptionId);
9595
_online.Add(secId, info);
9696
}
97-
98-
_byId.Add(subscriptionId, info);
9997
}
10098
else
10199
return null;

Tests/OrderBookIncrementManagerStateTests.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,45 @@ public void TryApply_AllSecSubscription_OnlyBuildsBookDynamically()
173173
subscriptionIds.Any(id => id == 42L).AssertTrue();
174174
}
175175

176+
[TestMethod]
177+
public void TryApply_AllSecSubscription_MultipleSecurities_EachGetsOwnBuilder()
178+
{
179+
// Reproduces the bug: ALL subscription shares one subscriptionId for all securities.
180+
// The old code cached _byId[subscriptionId] = first security's BookInfo,
181+
// so all subsequent securities incorrectly got the first security's SecurityId.
182+
var state = new OrderBookIncrementManagerState();
183+
184+
state.AddAllSecSubscription(42);
185+
186+
var secId1 = new SecurityId { SecurityCode = "SEC1", BoardCode = "TEST" };
187+
var secId2 = new SecurityId { SecurityCode = "SEC2", BoardCode = "TEST" };
188+
var secId3 = new SecurityId { SecurityCode = "SEC3", BoardCode = "TEST" };
189+
190+
// First security
191+
var snap1 = CreateSnapshot(secId1, [42]);
192+
var result1 = state.TryApply(42, snap1, out _);
193+
result1.AssertNotNull();
194+
result1.SecurityId.AssertEqual(secId1);
195+
196+
// Second security — must get its OWN builder, not SEC1's
197+
var snap2 = CreateSnapshot(secId2, [42]);
198+
var result2 = state.TryApply(42, snap2, out _);
199+
result2.AssertNotNull();
200+
result2.SecurityId.AssertEqual(secId2);
201+
202+
// Third security
203+
var snap3 = CreateSnapshot(secId3, [42]);
204+
var result3 = state.TryApply(42, snap3, out _);
205+
result3.AssertNotNull();
206+
result3.SecurityId.AssertEqual(secId3);
207+
208+
// Re-verify first security still works correctly
209+
var snap1b = CreateSnapshot(secId1, [42]);
210+
var result1b = state.TryApply(42, snap1b, out _);
211+
result1b.AssertNotNull();
212+
result1b.SecurityId.AssertEqual(secId1);
213+
}
214+
176215
[TestMethod]
177216
public void Clear_RemovesAll()
178217
{

Tests/OrderBookIncrementManagerTests.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,58 @@ public void AllSecSubscription_AppendsToBuiltBook()
437437
built.GetSubscriptionIds().OrderBy(i => i).SequenceEqual([1L, 99L]).AssertTrue();
438438
}
439439

440+
[TestMethod]
441+
public void AllSecSubscription_MultipleSecurities_EachGetsCorrectSecurityId()
442+
{
443+
var logReceiver = new TestReceiver();
444+
var manager = new OrderBookIncrementManager(logReceiver, new OrderBookIncrementManagerState());
445+
446+
var secId1 = new SecurityId { SecurityCode = "MULTI1", BoardCode = "TEST" };
447+
var secId2 = new SecurityId { SecurityCode = "MULTI2", BoardCode = "TEST" };
448+
var secId3 = new SecurityId { SecurityCode = "MULTI3", BoardCode = "TEST" };
449+
450+
// Subscribe to ALL securities
451+
manager.ProcessInMessage(new MarketDataMessage
452+
{
453+
IsSubscribe = true,
454+
TransactionId = 100,
455+
SecurityId = default,
456+
DataType2 = DataType.MarketDepth,
457+
});
458+
459+
// Snapshot for MULTI1
460+
var snap1 = CreateIncrement(secId1, DateTime.UtcNow, QuoteChangeStates.SnapshotComplete, [100],
461+
bids: [new QuoteChange(100m, 10m)],
462+
asks: [new QuoteChange(101m, 20m)]);
463+
var (forward1, extra1) = manager.ProcessOutMessage(snap1);
464+
forward1.AssertNull();
465+
extra1.Length.AssertEqual(1);
466+
((QuoteChangeMessage)extra1[0]).SecurityId.AssertEqual(secId1);
467+
468+
// Snapshot for MULTI2 — must NOT get MULTI1's SecurityId
469+
var snap2 = CreateIncrement(secId2, DateTime.UtcNow, QuoteChangeStates.SnapshotComplete, [100],
470+
bids: [new QuoteChange(200m, 30m)],
471+
asks: [new QuoteChange(201m, 40m)]);
472+
var (forward2, extra2) = manager.ProcessOutMessage(snap2);
473+
forward2.AssertNull();
474+
extra2.Length.AssertEqual(1);
475+
((QuoteChangeMessage)extra2[0]).SecurityId.AssertEqual(secId2);
476+
477+
// Snapshot for MULTI3
478+
var snap3 = CreateIncrement(secId3, DateTime.UtcNow, QuoteChangeStates.SnapshotComplete, [100],
479+
bids: [new QuoteChange(300m, 50m)],
480+
asks: [new QuoteChange(301m, 60m)]);
481+
var (forward3, extra3) = manager.ProcessOutMessage(snap3);
482+
forward3.AssertNull();
483+
extra3.Length.AssertEqual(1);
484+
((QuoteChangeMessage)extra3[0]).SecurityId.AssertEqual(secId3);
485+
486+
// Verify bids/asks are correct per security (not mixed)
487+
((QuoteChangeMessage)extra1[0]).Bids[0].Price.AssertEqual(100m);
488+
((QuoteChangeMessage)extra2[0]).Bids[0].Price.AssertEqual(200m);
489+
((QuoteChangeMessage)extra3[0]).Bids[0].Price.AssertEqual(300m);
490+
}
491+
440492
[TestMethod]
441493
public void NonMarketDataMessage_PassesThrough()
442494
{

0 commit comments

Comments
 (0)