diff --git a/Brokerages/Brokerage.cs b/Brokerages/Brokerage.cs index 94611485adc0..614f52458a9b 100644 --- a/Brokerages/Brokerage.cs +++ b/Brokerages/Brokerage.cs @@ -372,13 +372,21 @@ protected virtual List GetAccountHoldings(Dictionary br { return new List(); } + + foreach (var holding in result) + { + // the provided ticker might be outdated, the security identifier is the source of truth + holding.Symbol = holding.Symbol.MapToCurrentTicker(); + } + Log.Trace($"Brokerage.GetAccountHoldings(): sourcing holdings from provided brokerage data, found {result.Count} entries"); return result; } return securities?.Where(security => security.Holdings.AbsoluteQuantity > 0) - .OrderBy(security => security.Symbol) - .Select(security => new Holding(security)).ToList() ?? new List(); + // the security ticker might be outdated too, it's set when it's created and does not get updated on renames + .Select(security => new Holding(security) { Symbol = security.Symbol.MapToCurrentTicker() }) + .OrderBy(security => security.Symbol).ToList() ?? []; } /// diff --git a/Common/Extensions.cs b/Common/Extensions.cs index be982b6c8f16..eca1d39b6d9f 100644 --- a/Common/Extensions.cs +++ b/Common/Extensions.cs @@ -4502,6 +4502,48 @@ public static bool RequiresMapping(this Symbol symbol) } } + /// + /// Helper method to get the given symbol using the ticker it's currently mapped to + /// + /// A symbols ticker is set when it's created and does not get updated if the security is renamed, see , + /// this is specially useful for symbols which have been deserialized, since they keep the ticker they were serialized with. + /// The is the source of truth and never changes + /// The symbol to get the current ticker for + /// The given symbol using the ticker it's currently mapped to, the given symbol if it does not require mapping + /// or if the mapping could not be resolved + public static Symbol MapToCurrentTicker(this Symbol symbol) + { + // covers null and empty symbols + if (symbol == null || !symbol.RequiresMapping()) + { + return symbol; + } + + try + { + if (symbol.ID.HasUnderlying && !symbol.HasUnderlying) + { + // the deserialized symbol might be missing its underlying, which is required to resolve the mapping + symbol = new Symbol(symbol.ID, symbol.Value); + } + + var currentTicker = SecurityIdentifier.Ticker(symbol, DateTime.Today); + // for options it's the underlying ticker which gets mapped + if (currentTicker != (symbol.HasUnderlying ? symbol.Underlying.Value : symbol.Value)) + { + Log.Trace($"Extensions.MapToCurrentTicker(): mapping {symbol.Value} to {currentTicker}"); + return symbol.UpdateMappedSymbol(currentTicker); + } + } + catch (Exception exception) + { + // we don't want to fail because of a ticker, the security identifier is what matters + Log.Error(exception, $"Failed to map ticker for {symbol.ID}"); + } + + return symbol; + } + /// /// Checks whether the fill event for closing a trade is a winning trade /// diff --git a/Engine/Results/LiveTradingResultHandler.cs b/Engine/Results/LiveTradingResultHandler.cs index 05d4112f27d6..709bf2b6aa5d 100644 --- a/Engine/Results/LiveTradingResultHandler.cs +++ b/Engine/Results/LiveTradingResultHandler.cs @@ -1317,20 +1317,41 @@ public static Dictionary GetHoldings(IEnumerable secu { var holdings = new Dictionary(); - foreach (var security in securities + foreach (var holding in securities // If we are invested we send it always, if not, we send non internal, non canonical and tradable securities. When securities are removed they are marked as non tradable. .Where(s => s.Invested || !onlyInvested && (!s.IsInternalFeed() && s.IsTradable && !s.Symbol.IsCanonical() // Continuous futures are different because it's mapped securities are internal and the continuous contract is canonical and non tradable but we want to send them anyways // but we don't want to sent non canonical, non tradable futures, these would be the future chain assets, or continuous mapped contracts that have been removed || s.Symbol.SecurityType == QuantConnect.SecurityType.Future && (s.IsTradable || s.Symbol.IsCanonical() && subscriptionDataConfigService.GetSubscriptionDataConfigs(s.Symbol).Any()))) + .Select(s => new Holding(s) { Symbol = GetCurrentSymbol(s, subscriptionDataConfigService) }) + // we order by the ticker we will be sending, which might not be the securities .OrderBy(x => x.Symbol.Value)) { - DictionarySafeAdd(holdings, security.Symbol.ID.ToString(), new Holding(security), "holdings"); + // the mapping does not change the security identifier + DictionarySafeAdd(holdings, holding.Symbol.ID.ToString(), holding, "holdings"); } return holdings; } + /// + /// Helper method to get the security symbol using the ticker it's currently mapped to + /// + /// A securities symbol ticker is set when it's created and does not get updated if the security is renamed, + /// see , but it's subscriptions are, so let's use them as the source of truth. + /// Continuous futures are skipped, their mapping is the contract they are currently mapped to, which depends on + /// each subscriptions + private static Symbol GetCurrentSymbol(Security security, ISubscriptionDataConfigService subscriptionDataConfigService) + { + var symbol = security.Symbol; + if (symbol.SecurityType != QuantConnect.SecurityType.Equity && symbol.SecurityType != QuantConnect.SecurityType.Option) + { + return symbol; + } + + return subscriptionDataConfigService.GetSubscriptionDataConfigs(symbol).FirstOrDefault()?.Symbol ?? symbol; + } + /// /// Calculates and gets the current statistics for the algorithm /// diff --git a/Tests/Brokerages/DefaultBrokerageTests.cs b/Tests/Brokerages/DefaultBrokerageTests.cs index 218611f9ae26..a902104b037a 100644 --- a/Tests/Brokerages/DefaultBrokerageTests.cs +++ b/Tests/Brokerages/DefaultBrokerageTests.cs @@ -19,6 +19,7 @@ using QuantConnect.Securities; using QuantConnect.Brokerages; using System.Collections.Generic; +using QuantConnect.Securities.Equity; namespace QuantConnect.Tests.Brokerages { @@ -38,12 +39,96 @@ public OrderPosition GetsOrderPosition(OrderDirection direction, decimal holding return TestableBrokerage.GetOrderPositionPublic(direction, holdingsQuantity); } + [TestCase("GOOGL")] + [TestCase("GOOG")] + [TestCase("SomeOtherTicker")] + public void UpdatesOutdatedHoldingsTicker(string ticker) + { + // GOOGL first ticker is 'GOOG', so it's security identifier holds the outdated ticker + var expectedSymbol = Symbol.Create("GOOGL", SecurityType.Equity, Market.USA); + var brokerageData = new Dictionary + { + { "live-holdings", $@"[{{""symbol"":{{""id"":""{expectedSymbol.ID}"",""value"":""{ticker}""}},""a"":10,""q"":100}}]" } + }; + + var holdings = new TestableBrokerage("test").GetAccountHoldingsPublic(brokerageData, null); + + Assert.AreEqual(1, holdings.Count); + Assert.AreEqual(expectedSymbol.ID, holdings[0].Symbol.ID); + Assert.AreEqual(expectedSymbol.Value, holdings[0].Symbol.Value); + Assert.AreEqual(100, holdings[0].Quantity); + } + + [Test] + public void UpdatesOutdatedOptionHoldingsUnderlyingTicker() + { + var underlying = Symbol.Create("GOOGL", SecurityType.Equity, Market.USA); + var expectedSymbol = Symbol.CreateOption(underlying, Market.USA, OptionStyle.American, OptionRight.Call, 100, new DateTime(2050, 1, 21)); + // no underlying provided, so it will be created from the security identifier which holds the outdated ticker + var brokerageData = new Dictionary + { + { "live-holdings", $@"[{{""symbol"":{{""id"":""{expectedSymbol.ID}"",""value"":""{expectedSymbol.Value}""}},""q"":1}}]" } + }; + + var holdings = new TestableBrokerage("test").GetAccountHoldingsPublic(brokerageData, null); + + Assert.AreEqual(1, holdings.Count); + Assert.AreEqual(expectedSymbol.ID, holdings[0].Symbol.ID); + Assert.AreEqual(expectedSymbol.Value, holdings[0].Symbol.Value); + Assert.AreEqual(underlying.Value, holdings[0].Symbol.Underlying.Value); + } + + [Test] + public void DoesNotUpdateTickerForSecuritiesWhichDoNotRequireMapping() + { + var expectedSymbol = Symbol.Create("EURUSD", SecurityType.Forex, Market.Oanda); + var brokerageData = new Dictionary + { + { "live-holdings", $@"[{{""symbol"":{{""id"":""{expectedSymbol.ID}"",""value"":""{expectedSymbol.Value}""}},""q"":1000}}]" } + }; + + var holdings = new TestableBrokerage("test").GetAccountHoldingsPublic(brokerageData, null); + + Assert.AreEqual(1, holdings.Count); + Assert.AreEqual(expectedSymbol, holdings[0].Symbol); + Assert.AreEqual(expectedSymbol.Value, holdings[0].Symbol.Value); + } + + [Test] + public void UpdatesOutdatedSecurityHoldingsTicker() + { + var expectedSymbol = Symbol.Create("GOOGL", SecurityType.Equity, Market.USA); + // the security was created before the rename, so it's ticker is outdated + var cashBook = new CashBook(); + var security = new Equity(new Symbol(expectedSymbol.ID, "GOOG"), + SecurityExchangeHours.AlwaysOpen(TimeZones.NewYork), + cashBook.Add(Currencies.USD, 0, 1), + SymbolProperties.GetDefault(Currencies.USD), + cashBook, + RegisteredSecurityDataTypesProvider.Null, + new SecurityCache()); + security.SetLocalTimeKeeper(new TimeKeeper(DateTime.UtcNow, TimeZones.NewYork).GetLocalTimeKeeper(TimeZones.NewYork)); + security.Holdings.SetHoldings(10, 100); + + var holdings = new TestableBrokerage("test").GetAccountHoldingsPublic(null, new[] { security }); + + Assert.AreEqual(1, holdings.Count); + Assert.AreEqual(expectedSymbol.ID, holdings[0].Symbol.ID); + Assert.AreEqual(expectedSymbol.Value, holdings[0].Symbol.Value); + Assert.AreEqual(100, holdings[0].Quantity); + } + private class TestableBrokerage : Brokerage { public TestableBrokerage(string name) : base(name) { } + public List GetAccountHoldingsPublic(Dictionary brokerageData, IEnumerable securities) + { + return GetAccountHoldings(brokerageData, securities); + } + public override bool IsConnected => throw new NotImplementedException(); public override bool CancelOrder(Order order) diff --git a/Tests/Engine/Results/LiveTradingResultHandlerTests.cs b/Tests/Engine/Results/LiveTradingResultHandlerTests.cs index 7da447090874..39c3da24d010 100644 --- a/Tests/Engine/Results/LiveTradingResultHandlerTests.cs +++ b/Tests/Engine/Results/LiveTradingResultHandlerTests.cs @@ -106,6 +106,46 @@ public void GetHoldingsPositions(bool invested) Assert.AreEqual(10, holding2.Quantity); } + [Test] + public void GetHoldingsUsesCurrentTicker() + { + var algorithm = new AlgorithmStub(); + var equity = algorithm.AddEquity("SPY"); + equity.Holdings.SetHoldings(1, 10); + + // the security gets renamed, it's subscriptions get updated but the security symbol does not + foreach (var config in algorithm.SubscriptionManager.SubscriptionDataConfigService.GetSubscriptionDataConfigs(equity.Symbol)) + { + config.MappedSymbol = "NEWSPY"; + } + Assert.AreEqual("SPY", equity.Symbol.Value); + + var result = LiveTradingResultHandler.GetHoldings(algorithm.Securities.Values, algorithm.SubscriptionManager.SubscriptionDataConfigService); + + Assert.IsTrue(result.TryGetValue(equity.Symbol.ID.ToString(), out var holding)); + Assert.AreEqual(equity.Symbol.ID, holding.Symbol.ID); + Assert.AreEqual("NEWSPY", holding.Symbol.Value); + Assert.AreEqual(10, holding.Quantity); + } + + [Test] + public void GetHoldingsAreOrderedByCurrentTicker() + { + var algorithm = new AlgorithmStub(); + var aapl = algorithm.AddEquity("AAPL"); + var spy = algorithm.AddEquity("SPY"); + + foreach (var config in algorithm.SubscriptionManager.SubscriptionDataConfigService.GetSubscriptionDataConfigs(aapl.Symbol)) + { + config.MappedSymbol = "ZZZ"; + } + + var result = LiveTradingResultHandler.GetHoldings(algorithm.Securities.Values, algorithm.SubscriptionManager.SubscriptionDataConfigService); + + // AAPL is now ZZZ so it goes last + CollectionAssert.AreEqual(new[] { spy.Symbol.ID.ToString(), aapl.Symbol.ID.ToString() }, result.Keys); + } + [TestCase(true)] [TestCase(false)] public void GetHoldingsNoPosition(bool invested)