More changes to clean up and simplify

jhonabreul · jhonabreul · commit 265e9a5fb0ac · 2025-04-04T09:33:21.000-04:00
diff --git a/Algorithm.CSharp/SecurityInitializationOnReAdditionForEquityRegressionAlgorithm.cs b/Algorithm.CSharp/SecurityInitializationOnReAdditionForEquityRegressionAlgorithm.cs
@@ -32,7 +32,7 @@ public class SecurityInitializationOnReAdditionForEquityRegressionAlgorithm : QC
         private Security _equity;
         private Queue<DateTime> _tradableDates;
         private bool _securityWasRemoved;
-        private bool _securityWasInitialized;
+        private int _securityInitializationCount;
 
         public override void Initialize()
         {
@@ -44,7 +44,7 @@ public override void Initialize()
                 if ((_equity != null && ReferenceEquals(security, _equity)) ||
                     (_equity == null && security.Symbol == _symbol))
                 {
-                    _securityWasInitialized = true;
+                    _securityInitializationCount++;
                 }
 
                 Debug($"[{Time}] Seeding {security.Symbol}");
@@ -69,6 +69,13 @@ public override void Initialize()
                     return;
                 }
 
+                // Before we remove the security let's check that it was not initialized again
+                if (_securityInitializationCount != 1)
+                {
+                    throw new RegressionTestException($"Expected the equity to be initialized once and once only, " +
+                        $"but was initialized {_securityInitializationCount} times");
+                }
+
                 // Remove the security every day
                 Debug($"[{Time}] Removing the equity");
                 _securityWasRemoved = RemoveSecurity(_equity.Symbol);
@@ -82,12 +89,13 @@ public override void Initialize()
 
         private Equity AddEquity()
         {
-            _securityWasInitialized = false;
+            _securityInitializationCount = 0;
             var equity = AddEquity("SPY");
 
-            if (!_securityWasInitialized)
+            if (_securityInitializationCount != 1)
             {
-                throw new RegressionTestException("Expected the equity to be initialized");
+                throw new RegressionTestException($"Expected the equity to be initialized once and once only, " +
+                    $"but was initialized {_securityInitializationCount} times");
             }
 
             return equity;
diff --git a/Algorithm.CSharp/SecurityInitializationOnReAdditionForManuallyAddedOptionRegressionAlgorithm.cs b/Algorithm.CSharp/SecurityInitializationOnReAdditionForManuallyAddedOptionRegressionAlgorithm.cs
@@ -39,7 +39,7 @@ public class SecurityInitializationOnReAdditionForManuallyAddedOptionRegressionA
 
         private bool _securityWasRemoved;
 
-        private bool _securityWasInitialized;
+        private int _securityInitializationCount;
 
         private Queue<DateTime> _tradableDates;
 
@@ -54,7 +54,7 @@ public override void Initialize()
                 if ((_manuallyAddedContract != null && ReferenceEquals(security, _manuallyAddedContract)) ||
                     (_manuallyAddedContract == null && security.Symbol == _optionContractSymbol))
                 {
-                    _securityWasInitialized = true;
+                    _securityInitializationCount++;
                 }
 
                 Debug($"[{Time}] Seeding {security.Symbol}");
@@ -80,6 +80,13 @@ public override void Initialize()
                     return;
                 }
 
+                // Before we remove the security let's check that it was not initialized again
+                if (_securityInitializationCount != 1)
+                {
+                    throw new RegressionTestException($"Expected the option to be initialized once and once only, " +
+                        $"but was initialized {_securityInitializationCount} times");
+                }
+
                 // Remove the security every day
                 Debug($"[{Time}] Removing the equity");
                 _securityWasRemoved = RemoveSecurity(_manuallyAddedContract.Symbol);
@@ -93,12 +100,13 @@ public override void Initialize()
 
         public Option AddOptionContract()
         {
-            _securityWasInitialized = false;
+            _securityInitializationCount = 0;
             var option = AddOptionContract(_optionContractSymbol, Resolution.Daily);
 
-            if (!_securityWasInitialized)
+            if (_securityInitializationCount != 1)
             {
-                throw new RegressionTestException($"Expected the option contract to be initialized. Symbol: {option.Symbol}");
+                throw new RegressionTestException($"Expected the option to be initialized once and once only, " +
+                    $"but was initialized {_securityInitializationCount} times");
             }
 
             return option;
diff --git a/Algorithm.CSharp/SecurityInitializationOnReAdditionForSelectedOptionRegressionAlgorithm.cs b/Algorithm.CSharp/SecurityInitializationOnReAdditionForSelectedOptionRegressionAlgorithm.cs
@@ -29,7 +29,7 @@ public class SecurityInitializationOnReAdditionForSelectedOptionRegressionAlgori
     {
         private List<Symbol> _contractsToSelect;
         private HashSet<Option> _selectedContracts = new();
-        private List<Security> _initializedContracts = new();
+        private Dictionary<Security, int> _initializedContracts = new();
 
         private bool _selectSingle;
         private int _selectionsCount;
@@ -44,7 +44,11 @@ public override void Initialize()
             {
                 if (security is Option option)
                 {
-                    _initializedContracts.Add(option);
+                    if (!_initializedContracts.TryGetValue(security, out var count))
+                    {
+                        count = 0;
+                }
+                    _initializedContracts[security] = count + 1;
                 }
 
                 Debug($"[{Time}] Seeding {security.Symbol}");
@@ -104,9 +108,9 @@ public override void OnSecuritiesChanged(SecurityChanges changes)
             }
 
             var addedContracts = changes.AddedSecurities.OfType<Option>().ToList();
-            if (addedContracts.Any(x => !_initializedContracts.Contains(x)))
+            if (addedContracts.Any(x => !_initializedContracts.TryGetValue(x, out var count) || count != 1))
             {
-                throw new RegressionTestException($"Expected all contracts to be initialized. Added: {string.Join(", ", addedContracts.Select(x => x.Symbol.Value))}, Initialized: {string.Join(", ", _initializedContracts.Select(x => x.Symbol.Value))}");
+                throw new RegressionTestException($"Expected all contracts to be initialized. Added: {string.Join(", ", addedContracts.Select(x => x.Symbol.Value))}, Initialized: {string.Join(", ", _initializedContracts.Select(x => $"{x.Key.Symbol.Value} - {x.Value}"))}");
             }
 
             // The first contract will be selected always, so we expect it to be added only once
diff --git a/Algorithm/QCAlgorithm.Universe.cs b/Algorithm/QCAlgorithm.Universe.cs
@@ -589,10 +589,6 @@ private Security AddToUserDefinedUniverse(
                     Securities.Remove(security.Symbol);
                     Securities.Add(security);
                 }
-                else
-                {
-                    security.MakeTradable();
-                }
             }
             else
             {
diff --git a/Algorithm/QCAlgorithm.cs b/Algorithm/QCAlgorithm.cs
@@ -1977,7 +1977,7 @@ public Security AddSecurity(Symbol symbol, Resolution? resolution = null, bool f
 
             if (isCanonical)
             {
-                security.MakeNonTradable();
+                security.IsTradable = false;
                 Securities.Add(security);
 
                 // add this security to the user defined universe
@@ -2503,7 +2503,7 @@ public bool RemoveSecurity(Symbol symbol)
             }
 
             // Mark security as not tradable
-            security.MakeNonTradable();
+            security.Reset();
             if (symbol.IsCanonical())
             {
                 // remove underlying equity data if it's marked as internal
diff --git a/Common/Extensions.cs b/Common/Extensions.cs
@@ -3920,20 +3920,6 @@ public static void MakeTradable(this Security security)
             }
         }
 
-        /// <summary>
-        /// Helper method to set the <see cref="Security.IsTradable"/> property to <code>false</code>
-        /// when a security is being removed from the algorithm or delisted
-        /// </summary>
-        /// <param name="security"></param>
-        public static void MakeNonTradable(this Security security)
-        {
-            if (security is Securities.Index.Index index)
-            {
-                index.ManualSetIsTradable = false;
-            }
-            security.IsTradable = false;
-        }
-
         /// <summary>
         /// Helper method to set an algorithm runtime exception in a normalized fashion
         /// </summary>
diff --git a/Common/Securities/Index/Index.cs b/Common/Securities/Index/Index.cs
@@ -124,5 +124,15 @@ public Index(Symbol symbol,
             IsTradable = false;   //Index are non tradable by default
             Holdings = new IndexHolding(this, currencyConverter);
         }
+
+        /// <summary>
+        /// Resets the security to its initial state by marking it as uninitialized and non-tradable
+        /// and clearing the subscriptions.
+        /// </summary>
+        public override void Reset()
+        {
+            base.Reset();
+            ManualSetIsTradable = false;
+        }
     }
 }
diff --git a/Common/Securities/Security.cs b/Common/Securities/Security.cs
@@ -52,7 +52,7 @@ public class Security : DynamicObject, ISecurityPrice
         /// Uses concurrent bag to avoid list enumeration threading issues
         /// </summary>
         /// <remarks>Just use a list + lock, not concurrent bag, avoid garbage it creates for features we don't need here. See https://github.com/dotnet/runtime/issues/23103</remarks>
-        private readonly List<SubscriptionDataConfig> _subscriptionsBag;
+        private readonly HashSet<SubscriptionDataConfig> _subscriptionsBag;
 
         /// <summary>
         /// Flag to keep track of initialized securities, to avoid double initialization.
@@ -1073,7 +1073,7 @@ internal void AddData(SubscriptionDataConfig subscription)
                 {
                     throw new ArgumentException(Messages.Security.UnmatchingExchangeTimeZones, $"{nameof(subscription)}.{nameof(subscription.ExchangeTimeZone)}");
                 }
-                AddSubscription(subscription);
+                _subscriptionsBag.Add(subscription);
                 UpdateSubscriptionProperties();
             }
         }
@@ -1096,7 +1096,7 @@ internal void AddData(SubscriptionDataConfigList subscriptions)
                     {
                          throw new ArgumentException(Messages.Security.UnmatchingExchangeTimeZones, $"{nameof(subscription)}.{nameof(subscription.ExchangeTimeZone)}");
                     }
-                    AddSubscription(subscription);
+                    _subscriptionsBag.Add(subscription);
                 }
                 UpdateSubscriptionProperties();
             }
@@ -1179,18 +1179,19 @@ internal virtual void UpdateSymbolProperties(SymbolProperties symbolProperties)
         }
 
         /// <summary>
-        /// Add a new subscription to this security if not already present
+        /// Resets the security to its initial state by marking it as uninitialized and non-tradable
+        /// and clearing the subscriptions.
         /// </summary>
-        /// <remarks>
-        /// The caller must acquire the lock.
-        /// Using this instead of changing <see cref="_subscriptionsBag"/> from a list to a hash set to keep
-        /// the same behavior. Don't want to change the ordering.
-        /// </remarks>
-        private void AddSubscription(SubscriptionDataConfig subscription)
+        public virtual void Reset()
         {
-            if (!_subscriptionsBag.Contains(subscription))
+            IsInitialized = false;
+            IsTradable = false;
+
+            // Reset the subscriptions
+            lock (_subscriptionsBag)
             {
-                _subscriptionsBag.Add(subscription);
+                _subscriptionsBag.Clear();
+                UpdateSubscriptionProperties();
             }
         }
     }
diff --git a/Common/Securities/SecurityService.cs b/Common/Securities/SecurityService.cs
@@ -80,7 +80,12 @@ private Security CreateSecurity(Symbol symbol,
             if (!reCreateSecurity && _algorithm != null && _algorithm.Securities.TryGetValue(symbol, out var existingSecurity))
             {
                 existingSecurity.AddData(configList);
+
+                // If non-internal, mark as tradable if it was not already since this is an existing security but might include new subscriptions
+                if (!configList.IsInternalFeed)
+                {
                 existingSecurity.MakeTradable();
+                }
 
                 // invoke the security initializer
                 InitializeSecurity(initializeSecurity, existingSecurity);
diff --git a/Engine/DataFeeds/UniverseSelection.cs b/Engine/DataFeeds/UniverseSelection.cs
@@ -430,7 +430,7 @@ public SecurityChanges HandleDelisting(BaseData data, bool isInternalFeed)
             {
                 // don't allow users to open a new position once delisted
                 security.IsDelisted = true;
-                security.MakeNonTradable();
+                security.Reset();
 
                 // Add the security removal to the security changes but only if not pending for removal.
                 // If pending, the removed change event was already emitted for this security

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public class SecurityInitializationOnReAdditionForSelectedOptionRegressionAlgori`
`29`	`29`	`{`
`30`	`30`	`private List<Symbol> _contractsToSelect;`
`31`	`31`	`private HashSet<Option> _selectedContracts = new();`
`32`		`- private List<Security> _initializedContracts = new();`
	`32`	`+ private Dictionary<Security, int> _initializedContracts = new();`
`33`	`33`
`34`	`34`	`private bool _selectSingle;`
`35`	`35`	`private int _selectionsCount;`
`@@ -44,7 +44,11 @@ public override void Initialize()`
`44`	`44`	`{`
`45`	`45`	`if (security is Option option)`
`46`	`46`	`{`
`47`		`- _initializedContracts.Add(option);`
	`47`	`+ if (!_initializedContracts.TryGetValue(security, out var count))`
	`48`	`+ {`
	`49`	`+ count = 0;`
	`50`	`+ }`
	`51`	`+ _initializedContracts[security] = count + 1;`
`48`	`52`	`}`
`49`	`53`
`50`	`54`	`Debug($"[{Time}] Seeding {security.Symbol}");`
`@@ -104,9 +108,9 @@ public override void OnSecuritiesChanged(SecurityChanges changes)`
`104`	`108`	`}`
`105`	`109`
`106`	`110`	`var addedContracts = changes.AddedSecurities.OfType<Option>().ToList();`
`107`		`- if (addedContracts.Any(x => !_initializedContracts.Contains(x)))`
	`111`	`+ if (addedContracts.Any(x => !_initializedContracts.TryGetValue(x, out var count) \|\| count != 1))`
`108`	`112`	`{`
`109`		`- throw new RegressionTestException($"Expected all contracts to be initialized. Added: {string.Join(", ", addedContracts.Select(x => x.Symbol.Value))}, Initialized: {string.Join(", ", _initializedContracts.Select(x => x.Symbol.Value))}");`
	`113`	`+ throw new RegressionTestException($"Expected all contracts to be initialized. Added: {string.Join(", ", addedContracts.Select(x => x.Symbol.Value))}, Initialized: {string.Join(", ", _initializedContracts.Select(x => $"{x.Key.Symbol.Value} - {x.Value}"))}");`
`110`	`114`	`}`
`111`	`115`
`112`	`116`	`// The first contract will be selected always, so we expect it to be added only once`
Original file line number	Diff line number	Diff line change
`@@ -589,10 +589,6 @@ private Security AddToUserDefinedUniverse(`
`589`	`589`	`Securities.Remove(security.Symbol);`
`590`	`590`	`Securities.Add(security);`
`591`	`591`	`}`
`592`		`- else`
`593`		`- {`
`594`		`- security.MakeTradable();`
`595`		`- }`
`596`	`592`	`}`
`597`	`593`	`else`
`598`	`594`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1977,7 +1977,7 @@ public Security AddSecurity(Symbol symbol, Resolution? resolution = null, bool f`
`1977`	`1977`
`1978`	`1978`	`if (isCanonical)`
`1979`	`1979`	`{`
`1980`		`- security.MakeNonTradable();`
	`1980`	`+ security.IsTradable = false;`
`1981`	`1981`	`Securities.Add(security);`
`1982`	`1982`
`1983`	`1983`	`// add this security to the user defined universe`
`@@ -2503,7 +2503,7 @@ public bool RemoveSecurity(Symbol symbol)`
`2503`	`2503`	`}`
`2504`	`2504`
`2505`	`2505`	`// Mark security as not tradable`
`2506`		`- security.MakeNonTradable();`
	`2506`	`+ security.Reset();`
`2507`	`2507`	`if (symbol.IsCanonical())`
`2508`	`2508`	`{`
`2509`	`2509`	`// remove underlying equity data if it's marked as internal`
Original file line number	Diff line number	Diff line change
`@@ -124,5 +124,15 @@ public Index(Symbol symbol,`
`124`	`124`	`IsTradable = false; //Index are non tradable by default`
`125`	`125`	`Holdings = new IndexHolding(this, currencyConverter);`
`126`	`126`	`}`
	`127`	`+`
	`128`	`+ /// <summary>`
	`129`	`+ /// Resets the security to its initial state by marking it as uninitialized and non-tradable`
	`130`	`+ /// and clearing the subscriptions.`
	`131`	`+ /// </summary>`
	`132`	`+ public override void Reset()`
	`133`	`+ {`
	`134`	`+ base.Reset();`
	`135`	`+ ManualSetIsTradable = false;`
	`136`	`+ }`
`127`	`137`	`}`
`128`	`138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ public class Security : DynamicObject, ISecurityPrice`
`52`	`52`	`/// Uses concurrent bag to avoid list enumeration threading issues`
`53`	`53`	`/// </summary>`
`54`	`54`	`/// <remarks>Just use a list + lock, not concurrent bag, avoid garbage it creates for features we don't need here. See https://github.com/dotnet/runtime/issues/23103</remarks>`
`55`		`- private readonly List<SubscriptionDataConfig> _subscriptionsBag;`
	`55`	`+ private readonly HashSet<SubscriptionDataConfig> _subscriptionsBag;`
`56`	`56`
`57`	`57`	`/// <summary>`
`58`	`58`	`/// Flag to keep track of initialized securities, to avoid double initialization.`
`@@ -1073,7 +1073,7 @@ internal void AddData(SubscriptionDataConfig subscription)`
`1073`	`1073`	`{`
`1074`	`1074`	`throw new ArgumentException(Messages.Security.UnmatchingExchangeTimeZones, $"{nameof(subscription)}.{nameof(subscription.ExchangeTimeZone)}");`
`1075`	`1075`	`}`
`1076`		`- AddSubscription(subscription);`
	`1076`	`+ _subscriptionsBag.Add(subscription);`
`1077`	`1077`	`UpdateSubscriptionProperties();`
`1078`	`1078`	`}`
`1079`	`1079`	`}`
`@@ -1096,7 +1096,7 @@ internal void AddData(SubscriptionDataConfigList subscriptions)`
`1096`	`1096`	`{`
`1097`	`1097`	`throw new ArgumentException(Messages.Security.UnmatchingExchangeTimeZones, $"{nameof(subscription)}.{nameof(subscription.ExchangeTimeZone)}");`
`1098`	`1098`	`}`
`1099`		`- AddSubscription(subscription);`
	`1099`	`+ _subscriptionsBag.Add(subscription);`
`1100`	`1100`	`}`
`1101`	`1101`	`UpdateSubscriptionProperties();`
`1102`	`1102`	`}`
`@@ -1179,18 +1179,19 @@ internal virtual void UpdateSymbolProperties(SymbolProperties symbolProperties)`
`1179`	`1179`	`}`
`1180`	`1180`
`1181`	`1181`	`/// <summary>`
`1182`		`- /// Add a new subscription to this security if not already present`
	`1182`	`+ /// Resets the security to its initial state by marking it as uninitialized and non-tradable`
	`1183`	`+ /// and clearing the subscriptions.`
`1183`	`1184`	`/// </summary>`
`1184`		`- /// <remarks>`
`1185`		`- /// The caller must acquire the lock.`
`1186`		`- /// Using this instead of changing <see cref="_subscriptionsBag"/> from a list to a hash set to keep`
`1187`		`- /// the same behavior. Don't want to change the ordering.`
`1188`		`- /// </remarks>`
`1189`		`- private void AddSubscription(SubscriptionDataConfig subscription)`
	`1185`	`+ public virtual void Reset()`
`1190`	`1186`	`{`
`1191`		`- if (!_subscriptionsBag.Contains(subscription))`
	`1187`	`+ IsInitialized = false;`
	`1188`	`+ IsTradable = false;`
	`1189`	`+`
	`1190`	`+ // Reset the subscriptions`
	`1191`	`+ lock (_subscriptionsBag)`
`1192`	`1192`	`{`
`1193`		`- _subscriptionsBag.Add(subscription);`
	`1193`	`+ _subscriptionsBag.Clear();`
	`1194`	`+ UpdateSubscriptionProperties();`
`1194`	`1195`	`}`
`1195`	`1196`	`}`
`1196`	`1197`	`}`
Original file line number	Diff line number	Diff line change
`@@ -430,7 +430,7 @@ public SecurityChanges HandleDelisting(BaseData data, bool isInternalFeed)`
`430`	`430`	`{`
`431`	`431`	`// don't allow users to open a new position once delisted`
`432`	`432`	`security.IsDelisted = true;`
`433`		`- security.MakeNonTradable();`
	`433`	`+ security.Reset();`
`434`	`434`
`435`	`435`	`// Add the security removal to the security changes but only if not pending for removal.`
`436`	`436`	`// If pending, the removed change event was already emitted for this security`