From 06384c4d72869d1038ce92d6acb88af1460e4a2f Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Wed, 4 Nov 2026 13:37:49 -0400 Subject: [PATCH 1/5] Sanitize unsupported ObjectStore keys and throw when the sanitized name holds another object An unsupported key is saved under its sanitized name and found by either name, with a warning the first time per store. A save whose sanitized name already holds an object this store did not write from that key throws, so an object from an upload, the CLI, another algorithm or a previous run is only replaced through its exact key. LocalObjectStore keeps rejecting unsupported keys with the per-issue message as the backstop. --- ...StoreKeySanitizationRegressionAlgorithm.cs | 158 +++++++ ...StoreKeySanitizationRegressionAlgorithm.py | 62 +++ Algorithm/QCAlgorithm.cs | 2 +- .../Python/Wrappers/AlgorithmPythonWrapper.cs | 2 +- Common/Storage/ObjectStore.cs | 266 +++++++++++- Common/Storage/PythonObjectStore.cs | 6 +- Engine/Storage/LocalObjectStore.cs | 12 +- Research/QuantBook.cs | 1 + Tests/Common/Storage/LocalObjectStoreTests.cs | 406 +++++++++++++++++- Tests/Python/AlgorithmPythonWrapperTests.cs | 23 + 10 files changed, 889 insertions(+), 49 deletions(-) create mode 100644 Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs create mode 100644 Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py diff --git a/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs b/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs new file mode 100644 index 000000000000..a2a85ce59621 --- /dev/null +++ b/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs @@ -0,0 +1,158 @@ +/* + * QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. + * Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +using System; +using System.Collections.Generic; +using QuantConnect.Interfaces; + +namespace QuantConnect.Algorithm.CSharp +{ + /// + /// Regression algorithm asserting the ObjectStore key rules: an unsupported key is sanitized and used on every + /// save and read, and once the sanitized name is written directly a save through the unsupported key throws + /// + public class ObjectStoreKeySanitizationRegressionAlgorithm : QCAlgorithm, IRegressionAlgorithmDefinition + { + /// + /// Initialize the data and resolution required, as well as the cash and start-end dates for your algorithm. All algorithms must initialized. + /// + public override void Initialize() + { + SetStartDate(2013, 10, 7); + SetEndDate(2013, 10, 11); + + SetBenchmark(x => 0); + + AddEquity("SPY", Resolution.Daily); + } + + /// + /// End of algorithm run event handler. This method is called at the end of a backtest or live trading operation. + /// + public override void OnEndOfAlgorithm() + { + // an unsupported key, e.g. built from a user-facing name, is sanitized and used on every save and read + const string invalidKey = "trade_log_ai_hardware_&_cloud.csv"; + const string sanitizedKey = "trade_log_ai_hardware___cloud.csv"; + if (!ObjectStore.Save(invalidKey, "a,b\n1,2")) + { + throw new RegressionTestException("Expected the unsupported key to be sanitized and saved"); + } + if (!ObjectStore.Keys.Contains(sanitizedKey) || ObjectStore.Keys.Contains(invalidKey)) + { + throw new RegressionTestException($"Expected the object to be stored under '{sanitizedKey}', keys: {string.Join(", ", ObjectStore.Keys)}"); + } + if (!ObjectStore.ContainsKey(invalidKey) || ObjectStore.Read(invalidKey) != "a,b\n1,2" || ObjectStore.Read(sanitizedKey) != "a,b\n1,2") + { + throw new RegressionTestException("Expected the object to be read through both the original and the sanitized key"); + } + + // a later save through the same key updates the object + if (!ObjectStore.Save(invalidKey, "a,b\n3,4") || ObjectStore.Read(sanitizedKey) != "a,b\n3,4") + { + throw new RegressionTestException("Expected the second save through the unsupported key to update the object"); + } + + // once the sanitized name is written directly, as an upload or another run would, it is never replaced through the unsupported key + if (!ObjectStore.Save(sanitizedKey, "c,d\n5,6")) + { + throw new RegressionTestException("Expected the save through the sanitized key to succeed"); + } + var errorMessage = string.Empty; + try + { + ObjectStore.Save(invalidKey, "e,f\n7,8"); + } + catch (ArgumentException exception) + { + errorMessage = exception.Message; + } + if (!errorMessage.Contains($"its sanitized name '{sanitizedKey}' already holds another object")) + { + throw new RegressionTestException($"Expected the save through the unsupported key to throw, got: '{errorMessage}'"); + } + if (ObjectStore.Read(sanitizedKey) != "c,d\n5,6") + { + throw new RegressionTestException("Expected the object written through the sanitized key to be kept"); + } + + ObjectStore.Delete(sanitizedKey); + if (ObjectStore.ContainsKey(invalidKey)) + { + throw new RegressionTestException("Expected the delete of the sanitized key to remove the object the unsupported key reads"); + } + } + + /// + /// This is used by the regression test system to indicate if the open source Lean repository has the required data to run this algorithm. + /// + public bool CanRunLocally { get; } = true; + + /// + /// This is used by the regression test system to indicate which languages this algorithm is written in. + /// + public List Languages { get; } = new() { Language.CSharp, Language.Python }; + + /// + /// Data Points count of all timeslices of algorithm + /// + public long DataPoints => 6; + + /// + /// Data Points count of the algorithm history + /// + public int AlgorithmHistoryDataPoints => 0; + + /// + /// Final status of the algorithm + /// + public AlgorithmStatus AlgorithmStatus => AlgorithmStatus.Completed; + + /// + /// This is used by the regression test system to indicate what the expected statistics are from running the algorithm + /// + public Dictionary ExpectedStatistics => new Dictionary + { + {"Total Orders", "0"}, + {"Average Win", "0%"}, + {"Average Loss", "0%"}, + {"Compounding Annual Return", "0%"}, + {"Drawdown", "0%"}, + {"Expectancy", "0"}, + {"Start Equity", "100000"}, + {"End Equity", "100000"}, + {"Net Profit", "0%"}, + {"Sharpe Ratio", "0"}, + {"Sortino Ratio", "0"}, + {"Probabilistic Sharpe Ratio", "0%"}, + {"Loss Rate", "0%"}, + {"Win Rate", "0%"}, + {"Profit-Loss Ratio", "0"}, + {"Alpha", "0"}, + {"Beta", "0"}, + {"Annual Standard Deviation", "0"}, + {"Annual Variance", "0"}, + {"Information Ratio", "0"}, + {"Tracking Error", "0"}, + {"Treynor Ratio", "0"}, + {"Total Fees", "$0.00"}, + {"Estimated Strategy Capacity", "$0"}, + {"Lowest Capacity Asset", ""}, + {"Portfolio Turnover", "0%"}, + {"Drawdown Recovery", "0"}, + {"OrderListHash", "d41d8cd98f00b204e9800998ecf8427e"} + }; + } +} diff --git a/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py b/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py new file mode 100644 index 000000000000..a55f38c3c78a --- /dev/null +++ b/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py @@ -0,0 +1,62 @@ +# QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. +# Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from AlgorithmImports import * + +### +### Regression algorithm asserting the ObjectStore key rules: an unsupported key is sanitized and used on every +### save and read, and once the sanitized name is written directly a save through the unsupported key throws +### +class ObjectStoreKeySanitizationRegressionAlgorithm(QCAlgorithm): + + def initialize(self): + self.set_start_date(2013, 10, 7) + self.set_end_date(2013, 10, 11) + + self.set_benchmark(lambda x: 0) + + self.add_equity("SPY", Resolution.DAILY) + + def on_end_of_algorithm(self): + # an unsupported key, e.g. built from a user-facing name, is sanitized and used on every save and read + invalid_key = "trade_log_ai_hardware_&_cloud.csv" + sanitized_key = "trade_log_ai_hardware___cloud.csv" + if not self.object_store.save(invalid_key, "a,b\n1,2"): + raise AssertionError("Expected the unsupported key to be sanitized and saved") + keys = list(self.object_store.keys) + if sanitized_key not in keys or invalid_key in keys: + raise AssertionError(f"Expected the object to be stored under '{sanitized_key}', keys: {keys}") + if not self.object_store.contains_key(invalid_key) or self.object_store.read(invalid_key) != "a,b\n1,2" \ + or self.object_store.read(sanitized_key) != "a,b\n1,2": + raise AssertionError("Expected the object to be read through both the original and the sanitized key") + + # a later save through the same key updates the object + if not self.object_store.save(invalid_key, "a,b\n3,4") or self.object_store.read(sanitized_key) != "a,b\n3,4": + raise AssertionError("Expected the second save through the unsupported key to update the object") + + # once the sanitized name is written directly, as an upload or another run would, it is never replaced through the unsupported key + if not self.object_store.save(sanitized_key, "c,d\n5,6"): + raise AssertionError("Expected the save through the sanitized key to succeed") + error = "" + try: + self.object_store.save(invalid_key, "e,f\n7,8") + except BaseException as e: + error = str(e) + if f"its sanitized name '{sanitized_key}' already holds another object" not in error: + raise AssertionError(f"Expected the save through the unsupported key to throw, got: '{error}'") + if self.object_store.read(sanitized_key) != "c,d\n5,6": + raise AssertionError("Expected the object written through the sanitized key to be kept") + + self.object_store.delete(sanitized_key) + if self.object_store.contains_key(invalid_key): + raise AssertionError("Expected the delete of the sanitized key to remove the object the unsupported key reads") diff --git a/Algorithm/QCAlgorithm.cs b/Algorithm/QCAlgorithm.cs index 280dee74a02d..20240dae96b8 100644 --- a/Algorithm/QCAlgorithm.cs +++ b/Algorithm/QCAlgorithm.cs @@ -3321,7 +3321,7 @@ public void SetApi(IApi api) public void SetObjectStore(IObjectStore objectStore) { // an already wrapped store, like the Python one, is used as is - ObjectStore = objectStore as ObjectStore ?? new ObjectStore(objectStore); + ObjectStore = objectStore as ObjectStore ?? new ObjectStore(objectStore, Debug); } /// diff --git a/AlgorithmFactory/Python/Wrappers/AlgorithmPythonWrapper.cs b/AlgorithmFactory/Python/Wrappers/AlgorithmPythonWrapper.cs index 2fafbc4f675c..526906505687 100644 --- a/AlgorithmFactory/Python/Wrappers/AlgorithmPythonWrapper.cs +++ b/AlgorithmFactory/Python/Wrappers/AlgorithmPythonWrapper.cs @@ -1231,7 +1231,7 @@ public void SetCurrentSlice(Slice slice) /// Sets the object store /// /// The object store - public void SetObjectStore(IObjectStore objectStore) => _baseAlgorithm.SetObjectStore(new PythonObjectStore(objectStore)); + public void SetObjectStore(IObjectStore objectStore) => _baseAlgorithm.SetObjectStore(new PythonObjectStore(objectStore, _baseAlgorithm.Debug)); /// /// Determines if the Symbol is shortable at the brokerage diff --git a/Common/Storage/ObjectStore.cs b/Common/Storage/ObjectStore.cs index a610c1d00752..87a84821f43b 100644 --- a/Common/Storage/ObjectStore.cs +++ b/Common/Storage/ObjectStore.cs @@ -15,21 +15,43 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; +using System.Threading; +using System.Text.RegularExpressions; using System.Xml.Serialization; using Newtonsoft.Json; using QuantConnect.Interfaces; +using QuantConnect.Logging; using QuantConnect.Packets; namespace QuantConnect.Storage { /// - /// Helper class for easier access to methods + /// Helper class for easier access to methods. A key that does not follow the key + /// format (english letters, numbers, spaces and the characters '/', '\', '_', '#', '-', '$' and '=', plus at + /// most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv') is saved + /// under its sanitized version and found by either name, with a warning the first time it happens. A save whose + /// sanitized name already holds an object this store did not write from that key throws, since nothing records who + /// wrote that object. An object already stored under an unsupported name is read as is; a save to that name goes + /// to the sanitized name, warning every time /// public class ObjectStore : IObjectStore { + // The single source of the key charset/extension rules, also enforced by the LocalObjectStore implementation + private static readonly Regex SupportedKeyRegex = new(@"^\.?[a-zA-Z0-9\\/_#\-\$= ]+\.?[a-zA-Z0-9]*$", RegexOptions.Compiled); + private static readonly Regex UnsupportedKeyCharactersRegex = new(@"[^a-zA-Z0-9\\/_#\-\$= .]", RegexOptions.Compiled); + private static readonly Regex KeyExtensionRegex = new(@"^[a-zA-Z0-9]+$", RegexOptions.Compiled); + + private readonly IObjectStore _store; + private readonly Action _warn; + private int _warned; + // the unsupported keys this store last wrote each sanitized name from: their saves are updates, and reads follow them + private readonly ConcurrentDictionary _savedKeys = new(); + /// /// Gets the maximum storage limit in bytes /// @@ -49,15 +71,15 @@ public event EventHandler ErrorRaised remove { _store.ErrorRaised -= value; } } - private readonly IObjectStore _store; - /// /// Initializes a new instance of the class /// /// The instance to wrap - public ObjectStore(IObjectStore store) + /// Receives the warnings about sanitized keys, the engine log by default + public ObjectStore(IObjectStore store, Action warn = null) { _store = store; + _warn = warn ?? (message => Log.Trace(message)); } /// @@ -90,7 +112,7 @@ public void Initialize(int userId, int projectId, string userToken, Controls con /// True if the key was found public bool ContainsKey(string path) { - return _store.ContainsKey(path); + return _store.ContainsKey(KeyToRead(path)); } /// @@ -100,18 +122,28 @@ public bool ContainsKey(string path) /// A byte array containing the data public byte[] ReadBytes(string path) { - return _store.ReadBytes(path); + return _store.ReadBytes(KeyToRead(path)); } /// /// Saves the object data for the specified path /// - /// The object path + /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' /// The object data /// True if the save operation was successful public bool SaveBytes(string path, byte[] contents) { - return _store.SaveBytes(path, contents); + var key = KeyToSave(path); + if (!_store.SaveBytes(key, contents)) + { + return false; + } + ForgetSavesTo(key, exceptFrom: path); + if (key != path) + { + _savedKeys[path] = key; + } + return true; } /// @@ -121,7 +153,13 @@ public bool SaveBytes(string path, byte[] contents) /// True if the delete operation was successful public bool Delete(string path) { - return _store.Delete(path); + var key = KeyToRead(path); + if (!_store.Delete(key)) + { + return false; + } + ForgetSavesTo(key); + return true; } /// @@ -131,7 +169,7 @@ public bool Delete(string path) /// The path for the file public string GetFilePath(string path) { - return _store.GetFilePath(path); + return _store.GetFilePath(KeyToRead(path)); } /// @@ -144,7 +182,7 @@ public string Read(string path, Encoding encoding = null) { encoding = encoding ?? Encoding.UTF8; - var data = _store.ReadBytes(path); + var data = ReadBytes(path); return data != null ? encoding.GetString(data) : null; } @@ -208,7 +246,7 @@ public T ReadXml(string path, Encoding encoding = null) /// Saves the data from a local file path associated with the specified path /// /// If the file does not exist it will throw an exception - /// The object path + /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' /// True if the object was saved successfully public bool Save(string path) { @@ -220,26 +258,26 @@ public bool Save(string path) } var bytes = File.ReadAllBytes(filePath); - return _store.SaveBytes(path, bytes); + return SaveBytes(path, bytes); } /// /// Saves the object data in text format for the specified path /// - /// The object path + /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' /// The string object to be saved /// The string encoding used, by default /// True if the object was saved successfully public bool Save(string path, string text, Encoding encoding = null) { encoding ??= Encoding.UTF8; - return _store.SaveBytes(path, encoding.GetBytes(text)); + return SaveBytes(path, encoding.GetBytes(text)); } /// /// Saves the object data in text format for the specified path /// - /// The object path + /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' /// The string object to be saved /// The string encoding used /// True if the object was saved successfully @@ -247,7 +285,7 @@ public bool SaveString(string path, string text, Encoding encoding = null) { encoding = encoding ?? Encoding.UTF8; - return _store.SaveBytes(path, encoding.GetBytes(text)); + return SaveBytes(path, encoding.GetBytes(text)); } /// @@ -266,7 +304,7 @@ public bool SaveText(string path, string text, Encoding encoding = null) /// /// Saves the object data in JSON format for the specified path /// - /// The object path + /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' /// The object to be saved /// The string encoding used /// The settings used by the JSON serializer @@ -282,7 +320,7 @@ public bool SaveJson(string path, T obj, Encoding encoding = null, JsonSerial /// /// Saves the object data in XML format for the specified path /// - /// The object path + /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' /// The object to be saved /// The string encoding used /// True if the object was saved successfully @@ -322,5 +360,195 @@ public void Dispose() { _store.Dispose(); } + + /// + /// Throws if the given key does not follow the key format, stating every problem found and the rules. + /// The last line of defense of the store implementations, as this class only hands them supported keys + /// + /// The object store key to validate + /// The key is not supported + internal static void ValidateKey(string key) + { + var issues = GetKeyIssues(key); + if (issues != null) + { + throw new ArgumentException($"ObjectStore: path is not supported: '{key}': {issues}"); + } + } + + /// + /// The key a save goes to: the given one when it follows the key format, else its sanitized version. Throws when + /// that already holds an object this store did not last write from the given key. Null and empty keys are left for the store to reject + /// + private string KeyToSave(string path) + { + if (string.IsNullOrEmpty(path) || IsSupportedKey(path)) + { + return path; + } + + var key = SanitizeKey(path); + WarnOnce(path, key); + if (!_savedKeys.ContainsKey(path) && _store.ContainsKey(key)) + { + // nothing records who wrote that object, a previous run, another algorithm or an upload: it is never replaced through another key + throw new ArgumentException($"ObjectStore: key '{path}' is not supported and its sanitized name '{key}' already holds" + + $" another object. Delete '{key}' or save to it directly to replace it"); + } + if (_store.ContainsKey(path)) + { + // the object under the name as given stays: reads of it from another store still return it + _warn($"ObjectStore: key '{path}' is not supported: the object stored under that name is left as is, this save goes to '{key}'"); + } + return key; + } + + /// + /// Forgets the unsupported keys whose object under the given key is gone or replaced from another key + /// + private void ForgetSavesTo(string key, string exceptFrom = null) + { + foreach (var entry in _savedKeys) + { + if (entry.Value == key && entry.Key != exceptFrom) + { + _savedKeys.TryRemove(entry.Key, out _); + } + } + } + + /// + /// The key the other operations use: where this store saved it while that object exists, else the given one when + /// it follows the key format or the store already has it, else its sanitized version, so an object is found by the key it was saved with + /// + private string KeyToRead(string path) + { + if (string.IsNullOrEmpty(path)) + { + return path; + } + if (_savedKeys.TryGetValue(path, out var key) && _store.ContainsKey(key)) + { + return key; + } + if (IsSupportedKey(path) || _store.ContainsKey(path)) + { + return path; + } + + key = SanitizeKey(path); + WarnOnce(path, key); + return key; + } + + private void WarnOnce(string path, string key) + { + if (Interlocked.Exchange(ref _warned, 1) == 0) + { + _warn($"ObjectStore: key '{path}' is not supported: {GetKeyIssues(path)}. Using '{key}' instead. Any other unsupported" + + " key is sanitized the same way; a save throws when its sanitized name already holds an object not written from that key in this run"); + } + } + + private static bool IsSupportedKey(string key) + { + return !string.IsNullOrEmpty(key) && SupportedKeyRegex.IsMatch(key) + // just in case + && key.Count(c => c == '/') <= 100 && key.Count(c => c == '\\') <= 100; + } + + /// + /// Describes why a key does not follow the key format: the problems found and the rules. Null for a supported key + /// + private static string GetKeyIssues(string key) + { + if (IsSupportedKey(key)) + { + return null; + } + return string.Join("; ", FindKeyIssues(key)) + + ". Keys may only contain english letters, numbers, spaces and the characters" + + " '/', '\\', '_', '#', '-', '$' and '=', plus at most one '.' followed by a letters-and-numbers-only extension," + + " e.g. 'folder/trade_log-2024.csv'"; + } + + /// + /// Lists why a key fails , one entry per problem found + /// + private static List FindKeyIssues(string key) + { + var issues = new List(); + if (string.IsNullOrEmpty(key)) + { + issues.Add("the key is empty"); + return issues; + } + + var unsupported = UnsupportedKeyCharactersRegex.Matches(key).Select(match => $"'{match.Value}'").Distinct().ToList(); + if (unsupported.Count > 0) + { + issues.Add($"unsupported character{(unsupported.Count > 1 ? "s" : "")} {string.Join(", ", unsupported)}"); + } + + // a single leading '.' is allowed, every other '.' must introduce the final extension + var name = key.TrimStart('.'); + var dots = name.Count(c => c == '.'); + if (dots > 1) + { + issues.Add($"it has {dots} '.' but at most one '.extension' is allowed"); + } + else if (dots == 1) + { + var extension = name[(name.IndexOf('.', StringComparison.Ordinal) + 1)..]; + if (extension.Length > 0 && !KeyExtensionRegex.IsMatch(extension)) + { + issues.Add($"the extension '{extension}' may only contain letters and numbers"); + } + } + if (name.Length == 0) + { + issues.Add("it has no name before the '.'"); + } + else if (key.StartsWith("..", StringComparison.Ordinal)) + { + issues.Add("it starts with more than one '.'"); + } + + if (key.Count(c => c == '/') > 100 || key.Count(c => c == '\\') > 100) + { + issues.Add("it has more than 100 '/' or '\\' separators"); + } + + if (issues.Count == 0) + { + issues.Add("it does not follow the key format"); + } + return issues; + } + + /// + /// Converts an unsupported key into a supported one by replacing every unsupported character with '_', + /// keeping at most the final '.extension': 'trade log: AI & Cloud.csv' becomes 'trade log_ AI _ Cloud.csv'. + /// The mapping is lossy, distinct keys can sanitize to the same one. Throws when the key cannot be fixed + /// + private static string SanitizeKey(string key) + { + var sanitized = UnsupportedKeyCharactersRegex.Replace(key, "_"); + + // a single trailing '.extension' of letters and numbers is allowed: keep the last dot if it + // introduces a valid extension and replace every other dot + var lastDot = sanitized.LastIndexOf('.'); + if (lastDot > 0 && KeyExtensionRegex.IsMatch(sanitized.AsSpan(lastDot + 1))) + { + sanitized = string.Concat(sanitized[..lastDot].Replace('.', '_'), sanitized.AsSpan(lastDot)); + } + else if (lastDot >= 0) + { + sanitized = sanitized.Replace('.', '_'); + } + + ValidateKey(sanitized); + return sanitized; + } } } diff --git a/Common/Storage/PythonObjectStore.cs b/Common/Storage/PythonObjectStore.cs index a6792db05535..8d6d0179dcbe 100644 --- a/Common/Storage/PythonObjectStore.cs +++ b/Common/Storage/PythonObjectStore.cs @@ -13,6 +13,7 @@ * limitations under the License. */ +using System; using Python.Runtime; using QuantConnect.Interfaces; @@ -34,8 +35,9 @@ public class PythonObjectStore : ObjectStore /// Initializes a new instance of the class /// /// The instance to wrap - public PythonObjectStore(IObjectStore store) - : base(store) + /// Receives the warnings about sanitized keys, the engine log by default + public PythonObjectStore(IObjectStore store, Action warn = null) + : base(store, warn) { } diff --git a/Engine/Storage/LocalObjectStore.cs b/Engine/Storage/LocalObjectStore.cs index bbe6d7ccaf81..679ef52f414c 100644 --- a/Engine/Storage/LocalObjectStore.cs +++ b/Engine/Storage/LocalObjectStore.cs @@ -19,7 +19,6 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using System.Text.RegularExpressions; using System.Threading; using QuantConnect.Configuration; using QuantConnect.Interfaces; @@ -112,7 +111,6 @@ private bool IsDirty } private Timer _persistenceTimer; - private Regex _pathRegex = new(@"^\.?[a-zA-Z0-9\\/_#\-\$= ]+\.?[a-zA-Z0-9]*$", RegexOptions.Compiled); private readonly ConcurrentDictionary _storage = new(); private readonly object _persistLock = new object(); @@ -333,15 +331,7 @@ public bool SaveBytes(string path, byte[] contents) { throw new InvalidOperationException($"LocalObjectStore.SaveBytes(): {NoWritePermissionsError}"); } - else if (!_pathRegex.IsMatch(path)) - { - throw new ArgumentException($"LocalObjectStore: path is not supported: '{path}'"); - } - else if (path.Count(c => c == '/') > 100 || path.Count(c => c == '\\') > 100) - { - // just in case - throw new ArgumentException($"LocalObjectStore: path is not supported: '{path}'"); - } + ObjectStore.ValidateKey(path); // after we check the regex path = NormalizePath(path); diff --git a/Research/QuantBook.cs b/Research/QuantBook.cs index f53cab5681b8..ad9678d43726 100644 --- a/Research/QuantBook.cs +++ b/Research/QuantBook.cs @@ -166,6 +166,7 @@ public QuantBook() : base() StorageAccess = Config.GetValue("storage-permissions", new Packets.StoragePermissions()) }, AlgorithmMode.Research); + // key warnings go to the engine log, like every other QuantBook message: nothing drains the debug messages here SetObjectStore(_isPythonNotebook ? new PythonObjectStore(algorithmHandlers.ObjectStore) : new ObjectStore(algorithmHandlers.ObjectStore)); diff --git a/Tests/Common/Storage/LocalObjectStoreTests.cs b/Tests/Common/Storage/LocalObjectStoreTests.cs index 4f6b6e42892a..f20d3de8e376 100644 --- a/Tests/Common/Storage/LocalObjectStoreTests.cs +++ b/Tests/Common/Storage/LocalObjectStoreTests.cs @@ -27,6 +27,7 @@ using System.Threading.Tasks; using System.Collections.Generic; using QuantConnect.Lean.Engine.Storage; +using QuantConnect.Lean.Engine.DataFeeds.Transport; using System.Reflection; using Python.Runtime; @@ -234,9 +235,9 @@ public void DeletePermissions([Values] bool read, [Values] bool write, [Values] } } - [TestCase("../prefix/")] - [TestCase("..\\prefix/")] - public void InvalidCustomPathsStore(string path) + [TestCase("../prefix/", "__/prefix/")] + [TestCase("..\\prefix/", "__/prefix/")] + public void InvalidCustomPathsStore(string path, string sanitizedPath) { #pragma warning disable CA2000 using (var store = new ObjectStore(new TestLocalObjectStore())) @@ -245,7 +246,12 @@ public void InvalidCustomPathsStore(string path) store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); Assert.AreEqual(0, store.Count()); - Assert.Throws(() => store.SaveString($"{path}ILove", "Pizza")); + // '..' cannot leave the storage root: it is sanitized away + Assert.IsTrue(store.SaveString($"{path}ILove", "Pizza")); + Assert.AreEqual(new[] { $"{sanitizedPath}ILove" }, store.Keys); + Assert.That(Path.GetFullPath(store.GetFilePath($"{path}ILove")), Does.StartWith(Path.GetFullPath(TestStorageRoot))); + + store.Delete($"{path}ILove"); } } @@ -622,22 +628,392 @@ public void SaveTextAndReadTextAreAliasesOfSaveAndRead() _store.Delete("save_text_alias.txt"); } - [TestCase("my_key", "./LocalObjectStoreTests/my_key")] - [TestCase("test/123", "./LocalObjectStoreTests/test/123")] - [TestCase("**abc**", null)] - [TestCase("", null)] - [TestCase("|", null)] - public void GetFilePathReturnsFileName(string key, string expectedRelativePath) + [TestCase("my_key/file.csv", "my_key/file.csv")] + [TestCase("trade_log_ai_hardware_&_cloud.csv", "trade_log_ai_hardware___cloud.csv")] + [TestCase("trade log: AI & Cloud.csv", "trade log_ AI _ Cloud.csv")] + [TestCase("file.tar.gz", "file_tar.gz")] + [TestCase("résumé.pdf", "r_sum_.pdf")] + [TestCase("a..", "a__")] + [TestCase("100%.json", "100_.json")] + [TestCase("..model.pkl", "__model.pkl")] + public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(string key, string sanitizedKey) + { + Assert.IsTrue(_store.Save(key, "abc")); + + Assert.IsTrue(_store.Keys.Contains(sanitizedKey), string.Join(", ", _store.Keys)); + Assert.AreEqual(key == sanitizedKey, _store.Keys.Contains(key)); + Assert.IsTrue(_store.ContainsKey(key)); + Assert.IsTrue(_store.ContainsKey(sanitizedKey)); + Assert.AreEqual("abc", _store.Read(key)); + Assert.AreEqual("abc", _store.Read(sanitizedKey)); + + Assert.IsTrue(_store.Delete(key)); + Assert.IsFalse(_store.ContainsKey(sanitizedKey)); + } + + [Test] + public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() + { + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + Assert.IsTrue(store.Save("trade log: AI & Cloud.csv", "abc")); + Assert.IsTrue(store.ContainsKey("trade log: AI & Cloud.csv")); + Assert.AreEqual("abc", store.Read("trade log: AI & Cloud.csv")); + Assert.IsTrue(store.Save("trade log: AI & Cloud.csv", "abcd")); + Assert.IsTrue(store.Save("supported.csv", "abc")); + + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. " + + "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " + + "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'. " + + "Using 'trade log_ AI _ Cloud.csv' instead. Any other unsupported key is sanitized the same way; " + + "a save throws when its sanitized name already holds an object not written from that key in this run", warnings[0]); + + // the next unsupported key is sanitized silently + Assert.IsFalse(store.ContainsKey("trade log: AI.v2.csv")); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + + store.Delete("trade log: AI & Cloud.csv"); + store.Delete("supported.csv"); + } + + [TestCase("trade log: AI & Cloud.csv", "unsupported characters ':', '&'")] + [TestCase("résumé.pdf", "unsupported character 'é'")] + [TestCase("file.tar.gz", "it has 2 '.' but at most one '.extension' is allowed")] + [TestCase("file.name_v2", "the extension 'name_v2' may only contain letters and numbers")] + [TestCase("folder.v1/file", "the extension 'v1/file' may only contain letters and numbers")] + [TestCase("trade log: AI.v2.csv", "unsupported character ':'; it has 2 '.' but at most one '.extension' is allowed")] + [TestCase(".", "it has no name before the '.'")] + [TestCase("..model.pkl", "it starts with more than one '.'")] + public void UnsupportedKeyWarningStatesTheIssue(string key, string expectedIssue) + { + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + Assert.IsFalse(store.ContainsKey(key)); + + Assert.AreEqual(1, warnings.Count); + Assert.That(warnings[0], Does.StartWith($"ObjectStore: key '{key}' is not supported: {expectedIssue}.")); + } + + [TestCase("")] + [TestCase(null)] + public void EmptyAndNullKeysAreRejected(string key) { - if (expectedRelativePath == null) + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + var error = Assert.Catch(() => store.Save(key, "abc")); + + Assert.IsEmpty(warnings); + if (key != null) { - Assert.Throws(() => _store.GetFilePath(key)); + Assert.That(error.Message, Does.Contain("path is not supported: '': the key is empty.")); } - else + } + + [TestCase("model.tar.gz")] + [TestCase("SPY (1).csv")] + [TestCase("BRK.B.csv")] + public void ExistingUnsupportedKeysAreUsedAsGiven(string key) + { + // an object put in the store by other means (uploaded, copied into the folder) under an unsupported name + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + var filePath = Path.Combine(TestStorageRoot, key); + File.WriteAllText(filePath, "abc"); + + Assert.IsTrue(store.ContainsKey(key)); + Assert.AreEqual("abc", store.Read(key)); + Assert.IsTrue(store.Keys.Contains(key), string.Join(", ", store.Keys)); + Assert.AreEqual(Path.GetFullPath(filePath), Path.GetFullPath(store.GetFilePath(key))); + Assert.IsEmpty(warnings, string.Join("\n", warnings)); + Assert.IsTrue(store.Delete(key)); + Assert.IsFalse(File.Exists(filePath)); + + // gone from the store, the key falls back to its sanitized name like any other unsupported key + Assert.IsFalse(store.ContainsKey(key)); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.That(warnings[0], Does.StartWith($"ObjectStore: key '{key}' is not supported:")); + } + + [Test] + public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWithAWarningEveryTime() + { + // an uploaded object under an unsupported name, which the algorithm loads, retrains and saves back + var filePath = Path.Combine(TestStorageRoot, "model.tar.gz"); + File.WriteAllText(filePath, "old"); + var warnings = new List(); + using (var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add)) + { + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.AreEqual("old", store.Read("model.tar.gz")); + + Assert.IsTrue(store.Save("model.tar.gz", "new")); + + Assert.AreEqual(2, warnings.Count, string.Join("\n", warnings)); + Assert.That(warnings[0], Does.StartWith("ObjectStore: key 'model.tar.gz' is not supported: it has 2 '.' but at most one '.extension' is allowed.")); + Assert.AreEqual("ObjectStore: key 'model.tar.gz' is not supported: the object stored under that name is left as is," + + " this save goes to 'model_tar.gz'", warnings[1]); + // read after write within the run follows the save + Assert.AreEqual("new", store.Read("model.tar.gz")); + Assert.AreEqual("new", store.Read("model_tar.gz")); + Assert.AreEqual("old", File.ReadAllText(filePath)); + Assert.IsTrue(store.Keys.Contains("model.tar.gz") && store.Keys.Contains("model_tar.gz"), string.Join(", ", store.Keys)); + + // every such save warns, not only the first + Assert.IsTrue(store.Save("model.tar.gz", "newer")); + Assert.AreEqual(3, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual(warnings[1], warnings[2]); + Assert.AreEqual("newer", store.Read("model.tar.gz")); + } + + // a fresh store, the next run: the object under the name as given comes first again, and the save finds + // the previous run's object under the sanitized name, which nothing ties to this key any more + warnings.Clear(); + using (var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add)) + { + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.AreEqual("old", store.Read("model.tar.gz")); + Assert.IsEmpty(warnings); + + var exception = Assert.Throws(() => store.Save("model.tar.gz", "third")); + Assert.AreEqual("ObjectStore: key 'model.tar.gz' is not supported and its sanitized name 'model_tar.gz' already holds" + + " another object. Delete 'model_tar.gz' or save to it directly to replace it", exception.Message); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("newer", store.Read("model_tar.gz")); + + // once the sanitized name is free the save goes there, warning about the original every time + Assert.IsTrue(store.Delete("model_tar.gz")); + Assert.IsTrue(store.Save("model.tar.gz", "third")); + Assert.AreEqual(2, warnings.Count, string.Join("\n", warnings)); + Assert.That(warnings[1], Does.Contain("the object stored under that name is left as is, this save goes to 'model_tar.gz'")); + Assert.AreEqual("third", store.Read("model.tar.gz")); + Assert.AreEqual("third", store.Read("model_tar.gz")); + + // within the run the key follows the save: the first delete removes it and the original is back + Assert.IsTrue(store.Delete("model.tar.gz")); + Assert.IsFalse(store.ContainsKey("model_tar.gz")); + Assert.AreEqual("old", File.ReadAllText(filePath)); + Assert.AreEqual("old", store.Read("model.tar.gz")); + Assert.IsTrue(store.Delete("model.tar.gz")); + Assert.IsFalse(store.ContainsKey("model.tar.gz")); + } + Assert.IsFalse(File.Exists(filePath)); + } + + [Test] + public void DeletingASavedUnsupportedKeyRemovesTheSavedObjectThenTheOriginal() + { + var filePath = Path.Combine(TestStorageRoot, "model.tar.gz"); + File.WriteAllText(filePath, "old"); + using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.IsTrue(store.Save("model.tar.gz", "new")); + Assert.AreEqual("new", store.Read("model.tar.gz")); + + // the first delete removes what this store saved, and the key resolves like a fresh store's from then on + Assert.IsTrue(store.Delete("model.tar.gz")); + Assert.IsFalse(File.Exists(Path.Combine(TestStorageRoot, "model_tar.gz"))); + Assert.IsTrue(store.ContainsKey("model.tar.gz")); + Assert.AreEqual("old", store.Read("model.tar.gz")); + + Assert.IsTrue(store.Delete("model.tar.gz")); + Assert.IsFalse(store.ContainsKey("model.tar.gz")); + Assert.IsFalse(File.Exists(filePath)); + } + + [Test] + public void AFailedSaveToAnUnsupportedKeyDoesNotRedirectItsReads() + { + var filePath = Path.Combine(TestStorageRoot, "model.tar.gz"); + File.WriteAllText(filePath, "old"); + using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); + // the 3 bytes stored fill the storage: the save is rejected, not thrown, in backtesting + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1, StorageLimit = 3 }, AlgorithmMode.Backtesting); + + Assert.IsFalse(store.Save("model.tar.gz", "new")); + + Assert.IsTrue(store.ContainsKey("model.tar.gz")); + Assert.AreEqual("old", store.Read("model.tar.gz")); + Assert.IsFalse(store.ContainsKey("model_tar.gz")); + + Assert.IsTrue(store.Delete("model.tar.gz")); + Assert.IsFalse(File.Exists(filePath)); + } + + [Test] + public void CustomDataIsReadFromAnExistingUnsupportedKey() + { + using var store = new ObjectStore(new TestLocalObjectStore()); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + File.WriteAllText(Path.Combine(TestStorageRoot, "BRK.B.csv"), "2013-10-07,167.42\n2013-10-08,165.48\n"); + + // the data feed reads custom data through the algorithm's store + using var reader = new ObjectStoreSubscriptionStreamReader(store, "BRK.B.csv"); + + Assert.IsFalse(reader.EndOfStream); + Assert.AreEqual("2013-10-07,167.42", reader.ReadLine()); + Assert.AreEqual("2013-10-08,165.48", reader.ReadLine()); + store.Delete("BRK.B.csv"); + } + + [Test] + public void SavingAnUnsupportedKeyWhoseSanitizedNameHoldsAnotherObjectThrowsEveryTime() + { + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.IsTrue(store.Save("trade_log_ai_cloud.csv", "existing")); + Assert.IsEmpty(warnings); + + // sanitizes to the existing object's key: the general warning, then the exception + var exception = Assert.Throws(() => store.Save("trade_log_ai&cloud.csv", "first")); + Assert.AreEqual("ObjectStore: key 'trade_log_ai&cloud.csv' is not supported and its sanitized name 'trade_log_ai_cloud.csv' " + + "already holds another object. Delete 'trade_log_ai_cloud.csv' or save to it directly to replace it", exception.Message); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.That(warnings[0], Does.StartWith("ObjectStore: key 'trade_log_ai&cloud.csv' is not supported: unsupported character '&'.")); + Assert.AreEqual("existing", store.Read("trade_log_ai_cloud.csv")); + + // every such save throws, from this key or another one sanitizing to the same name + Assert.Throws(() => store.Save("trade_log_ai&cloud.csv", "first")); + exception = Assert.Throws(() => store.Save("trade_log_ai:cloud.csv", "second")); + Assert.AreEqual("ObjectStore: key 'trade_log_ai:cloud.csv' is not supported and its sanitized name 'trade_log_ai_cloud.csv' " + + "already holds another object. Delete 'trade_log_ai_cloud.csv' or save to it directly to replace it", exception.Message); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("existing", store.Read("trade_log_ai_cloud.csv")); + + // once the name is free the key is saved there and updated silently + Assert.IsTrue(store.Delete("trade_log_ai_cloud.csv")); + Assert.IsTrue(store.Save("trade_log_ai&cloud.csv", "first")); + Assert.IsTrue(store.Save("trade_log_ai&cloud.csv", "second")); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("second", store.Read("trade_log_ai_cloud.csv")); + Assert.AreEqual(1, store.Keys.Count(key => key.StartsWith("trade_log_ai"))); + + Assert.IsTrue(store.Delete("trade_log_ai&cloud.csv")); + Assert.IsFalse(store.ContainsKey("trade_log_ai_cloud.csv")); + } + + [Test] + public void ANewRunThrowsOnSavingAnUnsupportedKeyAPreviousRunSanitized() + { + // nothing records that a previous run wrote the sanitized object, so no later run replaces it through the unsupported key + using (var firstRun = new ObjectStore(new TestLocalObjectStore(), _ => { })) { - var expectedPath = Path.GetFullPath(expectedRelativePath).Replace("\\", "/"); - Assert.AreEqual(expectedPath, _store.GetFilePath(key).Replace("\\", "/")); + firstRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.IsTrue(firstRun.Save("trade log: AI & Cloud.csv", "run1")); } + + using (var secondRun = new ObjectStore(new TestLocalObjectStore(), _ => { })) + { + secondRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + var exception = Assert.Throws(() => secondRun.Save("trade log: AI & Cloud.csv", "run2")); + Assert.AreEqual("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported and its sanitized name 'trade log_ AI _ Cloud.csv'" + + " already holds another object. Delete 'trade log_ AI _ Cloud.csv' or save to it directly to replace it", exception.Message); + Assert.AreEqual("run1", secondRun.Read("trade log: AI & Cloud.csv")); + + // the exact sanitized key is a supported key: the user replaces the object through it + Assert.IsTrue(secondRun.Save("trade log_ AI _ Cloud.csv", "run2")); + Assert.AreEqual("run2", secondRun.Read("trade log: AI & Cloud.csv")); + Assert.Throws(() => secondRun.Save("trade log: AI & Cloud.csv", "run2 again")); + Assert.IsTrue(secondRun.Delete("trade log_ AI _ Cloud.csv")); + } + } + + [Test] + public void ASaveToTheSanitizedNameFromAnotherKeyEndsTheSilentUpdatesOfAnUnsupportedKey() + { + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + Assert.IsTrue(store.Save("p&q.csv", "A1")); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + + // another unsupported key sanitizing to the same name throws, and the first key keeps updating silently + var exception = Assert.Throws(() => store.Save("p:q.csv", "B1")); + Assert.That(exception.Message, Does.StartWith("ObjectStore: key 'p:q.csv' is not supported and its sanitized name 'p_q.csv' already holds another object")); + Assert.AreEqual("A1", store.Read("p_q.csv")); + Assert.IsTrue(store.Save("p&q.csv", "A2")); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("A2", store.Read("p_q.csv")); + + // a save through the valid name takes the object over: the unsupported key now throws + Assert.IsTrue(store.Save("p_q.csv", "C1")); + exception = Assert.Throws(() => store.Save("p&q.csv", "A3")); + Assert.That(exception.Message, Does.StartWith("ObjectStore: key 'p&q.csv' is not supported and its sanitized name 'p_q.csv' already holds another object")); + Assert.AreEqual("C1", store.Read("p_q.csv")); + + Assert.IsTrue(store.Delete("p_q.csv")); + Assert.IsTrue(store.Save("p&q.csv", "A3")); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("A3", store.Read("p&q.csv")); + Assert.IsTrue(store.Delete("p&q.csv")); + Assert.IsFalse(store.ContainsKey("p_q.csv")); + } + + [Test] + public void QuantBookWarnsThroughTheEngineLog() + { + var traces = new List(); + Log.LogHandler = new FunctionalLogHandler(null, traces.Add, null); + var quantBook = new QuantBook(); + Assert.AreEqual(typeof(ObjectStore), quantBook.ObjectStore.GetType()); + + Assert.IsTrue(quantBook.ObjectStore.Save("trade log: AI & Cloud.csv", "abc")); + + Assert.AreEqual(1, traces.Count(trace => trace.Contains("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'.")), + string.Join("\n", traces)); + quantBook.ObjectStore.Delete("trade log: AI & Cloud.csv"); + } + + [Test] + public void LocalObjectStoreRejectsUnsupportedKeysStatingTheIssue() + { + // the implementation is the last line of defense, only the wrapper sanitizes + using var store = new TestLocalObjectStore(); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }); + + var error = Assert.Throws(() => store.SaveBytes("trade_log_ai_hardware_&_cloud.csv", new byte[] { 1 })); + + Assert.AreEqual("ObjectStore: path is not supported: 'trade_log_ai_hardware_&_cloud.csv': unsupported character '&'. " + + "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " + + "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'", error.Message); + } + + [Test] + public void AlgorithmWrapsTheStoreAndWarnsThroughDebug() + { + using var localStore = new TestLocalObjectStore(); + localStore.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }); + var algorithm = new QCAlgorithm(); + + algorithm.SetObjectStore(localStore); + + Assert.AreEqual(typeof(ObjectStore), algorithm.ObjectStore.GetType()); + Assert.IsTrue(algorithm.ObjectStore.Save("trade_log_ai_hardware_&_cloud.csv", "abc")); + Assert.IsTrue(algorithm.DebugMessages.TryDequeue(out var warning)); + Assert.That(warning, Does.Contain("ObjectStore: key 'trade_log_ai_hardware_&_cloud.csv' is not supported: unsupported character '&'.")); + Assert.That(warning, Does.Contain("Using 'trade_log_ai_hardware___cloud.csv' instead")); + + algorithm.ObjectStore.Delete("trade_log_ai_hardware_&_cloud.csv"); + } + + [TestCase("my_key", "./LocalObjectStoreTests/my_key")] + [TestCase("test/123", "./LocalObjectStoreTests/test/123")] + [TestCase("**abc**", "./LocalObjectStoreTests/__abc__")] + [TestCase("", "./LocalObjectStoreTests/_random_")] + [TestCase("|", "./LocalObjectStoreTests/_")] + public void GetFilePathReturnsFileName(string key, string expectedRelativePath) + { + var expectedPath = Path.GetFullPath(expectedRelativePath).Replace("\\", "/"); + Assert.AreEqual(expectedPath, _store.GetFilePath(key).Replace("\\", "/")); } [Test] diff --git a/Tests/Python/AlgorithmPythonWrapperTests.cs b/Tests/Python/AlgorithmPythonWrapperTests.cs index 2dcab23c97a5..703b6206b284 100644 --- a/Tests/Python/AlgorithmPythonWrapperTests.cs +++ b/Tests/Python/AlgorithmPythonWrapperTests.cs @@ -173,6 +173,29 @@ public void PythonAlgorithmGetsAPythonObjectStore() } } + [Test] + public void PythonAlgorithmKeyWarningGoesThroughDebug() + { + using (Py.GIL()) + { + var algorithm = GetAlgorithm("def on_end_of_algorithm(self): self.object_store.save('trade log: AI & Cloud.json', '{}')"); + using var localStore = new LocalObjectStore(); + localStore.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + algorithm.SetObjectStore(localStore); + + // the key warning lands in the algorithm's log + algorithm.OnEndOfAlgorithm(); + Assert.Null(algorithm.RunTimeError); + Assert.IsTrue(localStore.Keys.Contains("trade log_ AI _ Cloud.json"), string.Join(", ", localStore.Keys)); + Assert.IsTrue(algorithm.BaseAlgorithm.DebugMessages.TryDequeue(out var warning)); + Assert.That(warning, Does.Contain("ObjectStore: key 'trade log: AI & Cloud.json' is not supported: unsupported characters ':', '&'.")); + Assert.That(warning, Does.Contain("Using 'trade log_ AI _ Cloud.json' instead")); + + localStore.Delete("trade log_ AI _ Cloud.json"); + } + } + private AlgorithmPythonWrapper GetAlgorithm(string code) { code = $"{_baseCode}{Environment.NewLine} {code}"; From 5c9c02a08cda32f8ff0ab4c0290816ccf7a00adb Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Thu, 5 Nov 2026 09:44:25 -0400 Subject: [PATCH 2/5] Allow dots anywhere in keys, reject only '..' and pair GetFilePath with the save that follows Keys may now carry dots in folder names and multiple extensions: only '..' is unsupported, since it would leave the storage root, along with a key that is nothing but dots and separators. A backslash is no longer a supported character and is sanitized to '/', the separator the local store already read it as. The supported check reuses the unsupported-characters regex, so the character set is written once and a trailing newline is no longer accepted. GetFilePath of an unsupported key registers its sanitized name in the store, so the save that followed threw the collision error. The key is now paired with that name when the call creates the entry. --- Common/Storage/ObjectStore.cs | 100 ++++----- Common/Storage/PythonObjectStore.cs | 4 +- Tests/Common/Storage/LocalObjectStoreTests.cs | 204 ++++++++++++------ 3 files changed, 188 insertions(+), 120 deletions(-) diff --git a/Common/Storage/ObjectStore.cs b/Common/Storage/ObjectStore.cs index 87a84821f43b..4310d06c26f8 100644 --- a/Common/Storage/ObjectStore.cs +++ b/Common/Storage/ObjectStore.cs @@ -32,19 +32,19 @@ namespace QuantConnect.Storage { /// /// Helper class for easier access to methods. A key that does not follow the key - /// format (english letters, numbers, spaces and the characters '/', '\', '_', '#', '-', '$' and '=', plus at - /// most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv') is saved - /// under its sanitized version and found by either name, with a warning the first time it happens. A save whose + /// format (english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never two + /// '.' in a row, e.g. 'folder/trade_log-2024.v2.csv') is saved under its sanitized version and found by + /// either name, with a warning the first time it happens. A save whose /// sanitized name already holds an object this store did not write from that key throws, since nothing records who /// wrote that object. An object already stored under an unsupported name is read as is; a save to that name goes /// to the sanitized name, warning every time /// public class ObjectStore : IObjectStore { - // The single source of the key charset/extension rules, also enforced by the LocalObjectStore implementation - private static readonly Regex SupportedKeyRegex = new(@"^\.?[a-zA-Z0-9\\/_#\-\$= ]+\.?[a-zA-Z0-9]*$", RegexOptions.Compiled); - private static readonly Regex UnsupportedKeyCharactersRegex = new(@"[^a-zA-Z0-9\\/_#\-\$= .]", RegexOptions.Compiled); - private static readonly Regex KeyExtensionRegex = new(@"^[a-zA-Z0-9]+$", RegexOptions.Compiled); + // The single source of the key charset rules, also enforced by the LocalObjectStore implementation + private static readonly Regex UnsupportedKeyCharactersRegex = new(@"[^a-zA-Z0-9/_#\-\$= .]", RegexOptions.Compiled); + // '..' is never allowed: it would leave the storage root + private static readonly Regex ConsecutiveDotsRegex = new(@"\.{2,}", RegexOptions.Compiled); private readonly IObjectStore _store; private readonly Action _warn; @@ -128,7 +128,7 @@ public byte[] ReadBytes(string path) /// /// Saves the object data for the specified path /// - /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The object data /// True if the save operation was successful public bool SaveBytes(string path, byte[] contents) @@ -169,7 +169,15 @@ public bool Delete(string path) /// The path for the file public string GetFilePath(string path) { - return _store.GetFilePath(KeyToRead(path)); + var key = KeyToRead(path); + // the store registers a missing key when handing out its path: that object is then this key's, so the save that follows is an update + var created = key != path && !_store.ContainsKey(key); + var filePath = _store.GetFilePath(key); + if (created) + { + _savedKeys[path] = key; + } + return filePath; } /// @@ -246,7 +254,7 @@ public T ReadXml(string path, Encoding encoding = null) /// Saves the data from a local file path associated with the specified path /// /// If the file does not exist it will throw an exception - /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// True if the object was saved successfully public bool Save(string path) { @@ -264,7 +272,7 @@ public bool Save(string path) /// /// Saves the object data in text format for the specified path /// - /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The string object to be saved /// The string encoding used, by default /// True if the object was saved successfully @@ -277,7 +285,7 @@ public bool Save(string path, string text, Encoding encoding = null) /// /// Saves the object data in text format for the specified path /// - /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The string object to be saved /// The string encoding used /// True if the object was saved successfully @@ -292,7 +300,7 @@ public bool SaveString(string path, string text, Encoding encoding = null) /// Saves the object data in text format for the specified path. /// Alias of /// - /// The object path + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The string object to be saved /// The string encoding used, by default /// True if the object was saved successfully @@ -304,7 +312,7 @@ public bool SaveText(string path, string text, Encoding encoding = null) /// /// Saves the object data in JSON format for the specified path /// - /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The object to be saved /// The string encoding used /// The settings used by the JSON serializer @@ -320,7 +328,7 @@ public bool SaveJson(string path, T obj, Encoding encoding = null, JsonSerial /// /// Saves the object data in XML format for the specified path /// - /// The object key: english letters, numbers, spaces, '/', '\', '_', '#', '-', '$', '=' and at most one '.extension', e.g. 'folder/trade_log-2024.csv'. Other characters are replaced by '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The object to be saved /// The string encoding used /// True if the object was saved successfully @@ -452,9 +460,17 @@ private void WarnOnce(string path, string key) private static bool IsSupportedKey(string key) { - return !string.IsNullOrEmpty(key) && SupportedKeyRegex.IsMatch(key) + return !string.IsNullOrEmpty(key) && !UnsupportedKeyCharactersRegex.IsMatch(key) && !key.Contains("..", StringComparison.Ordinal) && HasName(key) // just in case - && key.Count(c => c == '/') <= 100 && key.Count(c => c == '\\') <= 100; + && key.Count(c => c == '/') <= 100; + } + + /// + /// Whether the key has something besides dots and separators, so it names an object and not a folder + /// + private static bool HasName(string key) + { + return key.Any(c => c != '.' && c != '/'); } /// @@ -468,8 +484,7 @@ private static string GetKeyIssues(string key) } return string.Join("; ", FindKeyIssues(key)) + ". Keys may only contain english letters, numbers, spaces and the characters" + - " '/', '\\', '_', '#', '-', '$' and '=', plus at most one '.' followed by a letters-and-numbers-only extension," + - " e.g. 'folder/trade_log-2024.csv'"; + " '.', '/', '_', '#', '-', '$' and '=', never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'"; } /// @@ -490,33 +505,18 @@ private static List FindKeyIssues(string key) issues.Add($"unsupported character{(unsupported.Count > 1 ? "s" : "")} {string.Join(", ", unsupported)}"); } - // a single leading '.' is allowed, every other '.' must introduce the final extension - var name = key.TrimStart('.'); - var dots = name.Count(c => c == '.'); - if (dots > 1) + if (key.Contains("..", StringComparison.Ordinal)) { - issues.Add($"it has {dots} '.' but at most one '.extension' is allowed"); + issues.Add("it contains '..'"); } - else if (dots == 1) + if (!HasName(key)) { - var extension = name[(name.IndexOf('.', StringComparison.Ordinal) + 1)..]; - if (extension.Length > 0 && !KeyExtensionRegex.IsMatch(extension)) - { - issues.Add($"the extension '{extension}' may only contain letters and numbers"); - } - } - if (name.Length == 0) - { - issues.Add("it has no name before the '.'"); - } - else if (key.StartsWith("..", StringComparison.Ordinal)) - { - issues.Add("it starts with more than one '.'"); + issues.Add("it has no name, only '.' and separators"); } - if (key.Count(c => c == '/') > 100 || key.Count(c => c == '\\') > 100) + if (key.Count(c => c == '/') > 100) { - issues.Add("it has more than 100 '/' or '\\' separators"); + issues.Add("it has more than 100 '/' separators"); } if (issues.Count == 0) @@ -527,25 +527,15 @@ private static List FindKeyIssues(string key) } /// - /// Converts an unsupported key into a supported one by replacing every unsupported character with '_', - /// keeping at most the final '.extension': 'trade log: AI & Cloud.csv' becomes 'trade log_ AI _ Cloud.csv'. + /// Converts an unsupported key into a supported one: '\' becomes '/', the separator it already was for the local + /// store, every other unsupported character and every '.' of a '..' run become '_', so 'trade log: AI & Cloud.csv' + /// becomes 'trade log_ AI _ Cloud.csv'. /// The mapping is lossy, distinct keys can sanitize to the same one. Throws when the key cannot be fixed /// private static string SanitizeKey(string key) { - var sanitized = UnsupportedKeyCharactersRegex.Replace(key, "_"); - - // a single trailing '.extension' of letters and numbers is allowed: keep the last dot if it - // introduces a valid extension and replace every other dot - var lastDot = sanitized.LastIndexOf('.'); - if (lastDot > 0 && KeyExtensionRegex.IsMatch(sanitized.AsSpan(lastDot + 1))) - { - sanitized = string.Concat(sanitized[..lastDot].Replace('.', '_'), sanitized.AsSpan(lastDot)); - } - else if (lastDot >= 0) - { - sanitized = sanitized.Replace('.', '_'); - } + var sanitized = UnsupportedKeyCharactersRegex.Replace(key.Replace('\\', '/'), "_"); + sanitized = ConsecutiveDotsRegex.Replace(sanitized, match => new string('_', match.Length)); ValidateKey(sanitized); return sanitized; diff --git a/Common/Storage/PythonObjectStore.cs b/Common/Storage/PythonObjectStore.cs index 8d6d0179dcbe..017a4f343e69 100644 --- a/Common/Storage/PythonObjectStore.cs +++ b/Common/Storage/PythonObjectStore.cs @@ -87,7 +87,7 @@ public PyObject ReadDataframe(string path) /// , and so is any other type that would not round trip. A top-level Python /// str, int, float or bool is not accepted, use for text /// - /// The object path + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The Python object to be saved /// True if the object was saved successfully /// A TypeError naming the value that cannot be serialized @@ -107,7 +107,7 @@ public bool SaveJson(string path, PyObject obj) /// Saves the given pandas DataFrame (or Series) for the specified path, in JSON format if the /// path has a '.json' extension and as CSV otherwise /// - /// The object path + /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' /// The pandas DataFrame or Series to be saved /// True if the object was saved successfully public bool SaveDataframe(string path, PyObject dataFrame) diff --git a/Tests/Common/Storage/LocalObjectStoreTests.cs b/Tests/Common/Storage/LocalObjectStoreTests.cs index f20d3de8e376..cca0ebd8f384 100644 --- a/Tests/Common/Storage/LocalObjectStoreTests.cs +++ b/Tests/Common/Storage/LocalObjectStoreTests.cs @@ -629,13 +629,18 @@ public void SaveTextAndReadTextAreAliasesOfSaveAndRead() } [TestCase("my_key/file.csv", "my_key/file.csv")] + [TestCase("folder.v1/file.tar.gz", "folder.v1/file.tar.gz")] + [TestCase("my.folder/file.name_v2", "my.folder/file.name_v2")] [TestCase("trade_log_ai_hardware_&_cloud.csv", "trade_log_ai_hardware___cloud.csv")] [TestCase("trade log: AI & Cloud.csv", "trade log_ AI _ Cloud.csv")] - [TestCase("file.tar.gz", "file_tar.gz")] [TestCase("résumé.pdf", "r_sum_.pdf")] [TestCase("a..", "a__")] [TestCase("100%.json", "100_.json")] [TestCase("..model.pkl", "__model.pkl")] + [TestCase("a/../b.csv", "a/__/b.csv")] + [TestCase("a...b.csv", "a___b.csv")] + [TestCase("folder\\file.csv", "folder/file.csv")] + [TestCase("file.csv\n", "file.csv_")] public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(string key, string sanitizedKey) { Assert.IsTrue(_store.Save(key, "abc")); @@ -651,6 +656,64 @@ public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(s Assert.IsFalse(_store.ContainsKey(sanitizedKey)); } + [TestCase("../secrets.json", "__/secrets.json")] + [TestCase("../../secrets.json", "__/__/secrets.json")] + [TestCase("..\\secrets.json", "__/secrets.json")] + [TestCase("x/../secrets.json", "x/__/secrets.json")] + [TestCase("x/../../secrets.json", "x/__/__/secrets.json")] + [TestCase("x\\..\\..\\secrets.json", "x/__/__/secrets.json")] + [TestCase("x/.../secrets.json", "x/___/secrets.json")] + [TestCase("x/..", "x/__")] + [TestCase("x/....", "x/____")] + public void TraversalKeysAreSanitizedAndStayInsideTheStorageRoot(string key, string sanitizedKey) + { + // '..' never reaches the file system, so a key cannot point at another folder's or organization's objects + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + var root = Path.GetFullPath(TestStorageRoot) + Path.DirectorySeparatorChar; + + Assert.That(Path.GetFullPath(store.GetFilePath(key)), Does.StartWith(root)); + Assert.IsTrue(store.Save(key, "abc")); + + Assert.IsTrue(store.Keys.Contains(sanitizedKey), string.Join(", ", store.Keys)); + Assert.IsFalse(store.Keys.Any(k => k.Contains("..")), string.Join(", ", store.Keys)); + Assert.That(Path.GetFullPath(store.GetFilePath(sanitizedKey)), Does.StartWith(root)); + Assert.AreEqual("abc", store.Read(key)); + Assert.That(warnings[0], Does.Contain("it contains '..'")); + + Assert.IsTrue(store.Delete(key)); + Assert.IsFalse(store.ContainsKey(sanitizedKey)); + } + + [Test] + public void GetFilePathOfAnUnsupportedKeyPairsItWithTheSanitizedNameForTheSaveThatFollows() + { + var warnings = new List(); + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + // the store registers the key when handing out the path, so the file written there is saved through the same key + var filePath = store.GetFilePath("model (1).pkl"); + Assert.AreEqual(Path.GetFullPath(Path.Combine(TestStorageRoot, "model _1_.pkl")), Path.GetFullPath(filePath)); + File.WriteAllText(filePath, "abc"); + Assert.IsTrue(store.Save("model (1).pkl")); + Assert.AreEqual("abc", store.Read("model (1).pkl")); + Assert.AreEqual("abc", store.Read("model _1_.pkl")); + Assert.IsTrue(store.Save("model (1).pkl", "abcd")); + Assert.AreEqual("abcd", store.Read("model _1_.pkl")); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.IsTrue(store.Delete("model (1).pkl")); + + // an object another key wrote to the sanitized name is only read through the path: the save still throws + Assert.IsTrue(store.Save("model _1_.pkl", "other")); + Assert.AreEqual(Path.GetFullPath(filePath), Path.GetFullPath(store.GetFilePath("model (1).pkl"))); + var exception = Assert.Throws(() => store.Save("model (1).pkl", "mine")); + Assert.That(exception.Message, Does.StartWith("ObjectStore: key 'model (1).pkl' is not supported and its sanitized name 'model _1_.pkl' already holds another object")); + Assert.AreEqual("other", store.Read("model _1_.pkl")); + Assert.IsTrue(store.Delete("model _1_.pkl")); + } + [Test] public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() { @@ -666,8 +729,8 @@ public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); Assert.AreEqual("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. " + - "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " + - "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'. " + + "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " + + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'. " + "Using 'trade log_ AI _ Cloud.csv' instead. Any other unsupported key is sanitized the same way; " + "a save throws when its sanitized name already holds an object not written from that key in this run", warnings[0]); @@ -681,12 +744,10 @@ public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() [TestCase("trade log: AI & Cloud.csv", "unsupported characters ':', '&'")] [TestCase("résumé.pdf", "unsupported character 'é'")] - [TestCase("file.tar.gz", "it has 2 '.' but at most one '.extension' is allowed")] - [TestCase("file.name_v2", "the extension 'name_v2' may only contain letters and numbers")] - [TestCase("folder.v1/file", "the extension 'v1/file' may only contain letters and numbers")] - [TestCase("trade log: AI.v2.csv", "unsupported character ':'; it has 2 '.' but at most one '.extension' is allowed")] - [TestCase(".", "it has no name before the '.'")] - [TestCase("..model.pkl", "it starts with more than one '.'")] + [TestCase("folder\\file.csv", "unsupported character '\\'")] + [TestCase("..model.pkl", "it contains '..'")] + [TestCase("a/../b.csv", "it contains '..'")] + [TestCase("trade log: AI..v2.csv", "unsupported character ':'; it contains '..'")] public void UnsupportedKeyWarningStatesTheIssue(string key, string expectedIssue) { var warnings = new List(); @@ -699,10 +760,14 @@ public void UnsupportedKeyWarningStatesTheIssue(string key, string expectedIssue Assert.That(warnings[0], Does.StartWith($"ObjectStore: key '{key}' is not supported: {expectedIssue}.")); } - [TestCase("")] - [TestCase(null)] - public void EmptyAndNullKeysAreRejected(string key) + [TestCase("", "the key is empty")] + [TestCase(null, null)] + [TestCase(".", "it has no name, only '.' and separators")] + [TestCase("./", "it has no name, only '.' and separators")] + [TestCase("/", "it has no name, only '.' and separators")] + public void EmptyAndNamelessKeysAreRejected(string key, string expectedIssue) { + // nothing to sanitize these into: they throw like the store itself would, without a warning var warnings = new List(); using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); @@ -710,15 +775,15 @@ public void EmptyAndNullKeysAreRejected(string key) var error = Assert.Catch(() => store.Save(key, "abc")); Assert.IsEmpty(warnings); - if (key != null) + if (expectedIssue != null) { - Assert.That(error.Message, Does.Contain("path is not supported: '': the key is empty.")); + Assert.That(error.Message, Does.Contain($"path is not supported: '{key}': {expectedIssue}.")); } } - [TestCase("model.tar.gz")] - [TestCase("SPY (1).csv")] - [TestCase("BRK.B.csv")] + [TestCase("model (1).tar.gz")] + [TestCase("100%.json")] + [TestCase("BRK&B.csv")] public void ExistingUnsupportedKeysAreUsedAsGiven(string key) { // an object put in the store by other means (uploaded, copied into the folder) under an unsupported name @@ -746,31 +811,31 @@ public void ExistingUnsupportedKeysAreUsedAsGiven(string key) public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWithAWarningEveryTime() { // an uploaded object under an unsupported name, which the algorithm loads, retrains and saves back - var filePath = Path.Combine(TestStorageRoot, "model.tar.gz"); + var filePath = Path.Combine(TestStorageRoot, "model (1).tar.gz"); File.WriteAllText(filePath, "old"); var warnings = new List(); using (var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add)) { store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - Assert.AreEqual("old", store.Read("model.tar.gz")); + Assert.AreEqual("old", store.Read("model (1).tar.gz")); - Assert.IsTrue(store.Save("model.tar.gz", "new")); + Assert.IsTrue(store.Save("model (1).tar.gz", "new")); Assert.AreEqual(2, warnings.Count, string.Join("\n", warnings)); - Assert.That(warnings[0], Does.StartWith("ObjectStore: key 'model.tar.gz' is not supported: it has 2 '.' but at most one '.extension' is allowed.")); - Assert.AreEqual("ObjectStore: key 'model.tar.gz' is not supported: the object stored under that name is left as is," + - " this save goes to 'model_tar.gz'", warnings[1]); + Assert.That(warnings[0], Does.StartWith("ObjectStore: key 'model (1).tar.gz' is not supported: unsupported characters '(', ')'.")); + Assert.AreEqual("ObjectStore: key 'model (1).tar.gz' is not supported: the object stored under that name is left as is," + + " this save goes to 'model _1_.tar.gz'", warnings[1]); // read after write within the run follows the save - Assert.AreEqual("new", store.Read("model.tar.gz")); - Assert.AreEqual("new", store.Read("model_tar.gz")); + Assert.AreEqual("new", store.Read("model (1).tar.gz")); + Assert.AreEqual("new", store.Read("model _1_.tar.gz")); Assert.AreEqual("old", File.ReadAllText(filePath)); - Assert.IsTrue(store.Keys.Contains("model.tar.gz") && store.Keys.Contains("model_tar.gz"), string.Join(", ", store.Keys)); + Assert.IsTrue(store.Keys.Contains("model (1).tar.gz") && store.Keys.Contains("model _1_.tar.gz"), string.Join(", ", store.Keys)); // every such save warns, not only the first - Assert.IsTrue(store.Save("model.tar.gz", "newer")); + Assert.IsTrue(store.Save("model (1).tar.gz", "newer")); Assert.AreEqual(3, warnings.Count, string.Join("\n", warnings)); Assert.AreEqual(warnings[1], warnings[2]); - Assert.AreEqual("newer", store.Read("model.tar.gz")); + Assert.AreEqual("newer", store.Read("model (1).tar.gz")); } // a fresh store, the next run: the object under the name as given comes first again, and the save finds @@ -779,30 +844,30 @@ public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWithAWarningEv using (var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add)) { store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - Assert.AreEqual("old", store.Read("model.tar.gz")); + Assert.AreEqual("old", store.Read("model (1).tar.gz")); Assert.IsEmpty(warnings); - var exception = Assert.Throws(() => store.Save("model.tar.gz", "third")); - Assert.AreEqual("ObjectStore: key 'model.tar.gz' is not supported and its sanitized name 'model_tar.gz' already holds" + - " another object. Delete 'model_tar.gz' or save to it directly to replace it", exception.Message); + var exception = Assert.Throws(() => store.Save("model (1).tar.gz", "third")); + Assert.AreEqual("ObjectStore: key 'model (1).tar.gz' is not supported and its sanitized name 'model _1_.tar.gz' already holds" + + " another object. Delete 'model _1_.tar.gz' or save to it directly to replace it", exception.Message); Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); - Assert.AreEqual("newer", store.Read("model_tar.gz")); + Assert.AreEqual("newer", store.Read("model _1_.tar.gz")); // once the sanitized name is free the save goes there, warning about the original every time - Assert.IsTrue(store.Delete("model_tar.gz")); - Assert.IsTrue(store.Save("model.tar.gz", "third")); + Assert.IsTrue(store.Delete("model _1_.tar.gz")); + Assert.IsTrue(store.Save("model (1).tar.gz", "third")); Assert.AreEqual(2, warnings.Count, string.Join("\n", warnings)); - Assert.That(warnings[1], Does.Contain("the object stored under that name is left as is, this save goes to 'model_tar.gz'")); - Assert.AreEqual("third", store.Read("model.tar.gz")); - Assert.AreEqual("third", store.Read("model_tar.gz")); + Assert.That(warnings[1], Does.Contain("the object stored under that name is left as is, this save goes to 'model _1_.tar.gz'")); + Assert.AreEqual("third", store.Read("model (1).tar.gz")); + Assert.AreEqual("third", store.Read("model _1_.tar.gz")); // within the run the key follows the save: the first delete removes it and the original is back - Assert.IsTrue(store.Delete("model.tar.gz")); - Assert.IsFalse(store.ContainsKey("model_tar.gz")); + Assert.IsTrue(store.Delete("model (1).tar.gz")); + Assert.IsFalse(store.ContainsKey("model _1_.tar.gz")); Assert.AreEqual("old", File.ReadAllText(filePath)); - Assert.AreEqual("old", store.Read("model.tar.gz")); - Assert.IsTrue(store.Delete("model.tar.gz")); - Assert.IsFalse(store.ContainsKey("model.tar.gz")); + Assert.AreEqual("old", store.Read("model (1).tar.gz")); + Assert.IsTrue(store.Delete("model (1).tar.gz")); + Assert.IsFalse(store.ContainsKey("model (1).tar.gz")); } Assert.IsFalse(File.Exists(filePath)); } @@ -810,40 +875,40 @@ public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWithAWarningEv [Test] public void DeletingASavedUnsupportedKeyRemovesTheSavedObjectThenTheOriginal() { - var filePath = Path.Combine(TestStorageRoot, "model.tar.gz"); + var filePath = Path.Combine(TestStorageRoot, "model (1).tar.gz"); File.WriteAllText(filePath, "old"); using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - Assert.IsTrue(store.Save("model.tar.gz", "new")); - Assert.AreEqual("new", store.Read("model.tar.gz")); + Assert.IsTrue(store.Save("model (1).tar.gz", "new")); + Assert.AreEqual("new", store.Read("model (1).tar.gz")); // the first delete removes what this store saved, and the key resolves like a fresh store's from then on - Assert.IsTrue(store.Delete("model.tar.gz")); - Assert.IsFalse(File.Exists(Path.Combine(TestStorageRoot, "model_tar.gz"))); - Assert.IsTrue(store.ContainsKey("model.tar.gz")); - Assert.AreEqual("old", store.Read("model.tar.gz")); + Assert.IsTrue(store.Delete("model (1).tar.gz")); + Assert.IsFalse(File.Exists(Path.Combine(TestStorageRoot, "model _1_.tar.gz"))); + Assert.IsTrue(store.ContainsKey("model (1).tar.gz")); + Assert.AreEqual("old", store.Read("model (1).tar.gz")); - Assert.IsTrue(store.Delete("model.tar.gz")); - Assert.IsFalse(store.ContainsKey("model.tar.gz")); + Assert.IsTrue(store.Delete("model (1).tar.gz")); + Assert.IsFalse(store.ContainsKey("model (1).tar.gz")); Assert.IsFalse(File.Exists(filePath)); } [Test] public void AFailedSaveToAnUnsupportedKeyDoesNotRedirectItsReads() { - var filePath = Path.Combine(TestStorageRoot, "model.tar.gz"); + var filePath = Path.Combine(TestStorageRoot, "model (1).tar.gz"); File.WriteAllText(filePath, "old"); using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); // the 3 bytes stored fill the storage: the save is rejected, not thrown, in backtesting store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1, StorageLimit = 3 }, AlgorithmMode.Backtesting); - Assert.IsFalse(store.Save("model.tar.gz", "new")); + Assert.IsFalse(store.Save("model (1).tar.gz", "new")); - Assert.IsTrue(store.ContainsKey("model.tar.gz")); - Assert.AreEqual("old", store.Read("model.tar.gz")); - Assert.IsFalse(store.ContainsKey("model_tar.gz")); + Assert.IsTrue(store.ContainsKey("model (1).tar.gz")); + Assert.AreEqual("old", store.Read("model (1).tar.gz")); + Assert.IsFalse(store.ContainsKey("model _1_.tar.gz")); - Assert.IsTrue(store.Delete("model.tar.gz")); + Assert.IsTrue(store.Delete("model (1).tar.gz")); Assert.IsFalse(File.Exists(filePath)); } @@ -852,15 +917,15 @@ public void CustomDataIsReadFromAnExistingUnsupportedKey() { using var store = new ObjectStore(new TestLocalObjectStore()); store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - File.WriteAllText(Path.Combine(TestStorageRoot, "BRK.B.csv"), "2013-10-07,167.42\n2013-10-08,165.48\n"); + File.WriteAllText(Path.Combine(TestStorageRoot, "BRK&B.csv"), "2013-10-07,167.42\n2013-10-08,165.48\n"); // the data feed reads custom data through the algorithm's store - using var reader = new ObjectStoreSubscriptionStreamReader(store, "BRK.B.csv"); + using var reader = new ObjectStoreSubscriptionStreamReader(store, "BRK&B.csv"); Assert.IsFalse(reader.EndOfStream); Assert.AreEqual("2013-10-07,167.42", reader.ReadLine()); Assert.AreEqual("2013-10-08,165.48", reader.ReadLine()); - store.Delete("BRK.B.csv"); + store.Delete("BRK&B.csv"); } [Test] @@ -983,8 +1048,8 @@ public void LocalObjectStoreRejectsUnsupportedKeysStatingTheIssue() var error = Assert.Throws(() => store.SaveBytes("trade_log_ai_hardware_&_cloud.csv", new byte[] { 1 })); Assert.AreEqual("ObjectStore: path is not supported: 'trade_log_ai_hardware_&_cloud.csv': unsupported character '&'. " + - "Keys may only contain english letters, numbers, spaces and the characters '/', '\\', '_', '#', '-', '$' and '=', " + - "plus at most one '.' followed by a letters-and-numbers-only extension, e.g. 'folder/trade_log-2024.csv'", error.Message); + "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " + + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'", error.Message); } [Test] @@ -1789,6 +1854,19 @@ def test(object_store): "); } + [Test] + public void PythonHelpersSanitizeUnsupportedKeys() + { + RunPythonTest(@" +def test(object_store): + object_store.save_json('helpers/trade log: AI & Cloud.json', {'a': 1}) + assert 'helpers/trade log_ AI _ Cloud.json' in list(object_store.keys), list(object_store.keys) + assert object_store.read_json('helpers/trade log: AI & Cloud.json') == {'a': 1} + object_store.delete('helpers/trade log: AI & Cloud.json') + assert not object_store.contains_key('helpers/trade log_ AI _ Cloud.json') +"); + } + /// /// Runs the 'test(object_store)' function of the given Python source against the test store /// From e2b689de419da626176aeaca5a622b0afcc06e3a Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Thu, 5 Nov 2026 12:38:14 -0400 Subject: [PATCH 3/5] Keep the backslash as a separator, as the local store always read it A key such as folder\results.csv worked before this branch: the local store reads the backslash as a forward slash. Treating it as unsupported sent it through the sanitized-save path, whose collision rule threw from the second run on. The backslash is an accepted separator again and is passed to the store unchanged, with no warning. Adds a test that saves a backslash key in two runs. --- Common/Storage/ObjectStore.cs | 37 +++++++++---------- Common/Storage/PythonObjectStore.cs | 4 +- Tests/Common/Storage/LocalObjectStoreTests.cs | 36 ++++++++++++++---- 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/Common/Storage/ObjectStore.cs b/Common/Storage/ObjectStore.cs index 4310d06c26f8..3610f239a6a6 100644 --- a/Common/Storage/ObjectStore.cs +++ b/Common/Storage/ObjectStore.cs @@ -32,8 +32,8 @@ namespace QuantConnect.Storage { /// /// Helper class for easier access to methods. A key that does not follow the key - /// format (english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never two - /// '.' in a row, e.g. 'folder/trade_log-2024.v2.csv') is saved under its sanitized version and found by + /// format (english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never + /// two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv') is saved under its sanitized version and found by /// either name, with a warning the first time it happens. A save whose /// sanitized name already holds an object this store did not write from that key throws, since nothing records who /// wrote that object. An object already stored under an unsupported name is read as is; a save to that name goes @@ -42,7 +42,7 @@ namespace QuantConnect.Storage public class ObjectStore : IObjectStore { // The single source of the key charset rules, also enforced by the LocalObjectStore implementation - private static readonly Regex UnsupportedKeyCharactersRegex = new(@"[^a-zA-Z0-9/_#\-\$= .]", RegexOptions.Compiled); + private static readonly Regex UnsupportedKeyCharactersRegex = new(@"[^a-zA-Z0-9\\/_#\-\$= .]", RegexOptions.Compiled); // '..' is never allowed: it would leave the storage root private static readonly Regex ConsecutiveDotsRegex = new(@"\.{2,}", RegexOptions.Compiled); @@ -128,7 +128,7 @@ public byte[] ReadBytes(string path) /// /// Saves the object data for the specified path /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The object data /// True if the save operation was successful public bool SaveBytes(string path, byte[] contents) @@ -254,7 +254,7 @@ public T ReadXml(string path, Encoding encoding = null) /// Saves the data from a local file path associated with the specified path /// /// If the file does not exist it will throw an exception - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// True if the object was saved successfully public bool Save(string path) { @@ -272,7 +272,7 @@ public bool Save(string path) /// /// Saves the object data in text format for the specified path /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The string object to be saved /// The string encoding used, by default /// True if the object was saved successfully @@ -285,7 +285,7 @@ public bool Save(string path, string text, Encoding encoding = null) /// /// Saves the object data in text format for the specified path /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The string object to be saved /// The string encoding used /// True if the object was saved successfully @@ -300,7 +300,7 @@ public bool SaveString(string path, string text, Encoding encoding = null) /// Saves the object data in text format for the specified path. /// Alias of /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The string object to be saved /// The string encoding used, by default /// True if the object was saved successfully @@ -312,7 +312,7 @@ public bool SaveText(string path, string text, Encoding encoding = null) /// /// Saves the object data in JSON format for the specified path /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The object to be saved /// The string encoding used /// The settings used by the JSON serializer @@ -328,7 +328,7 @@ public bool SaveJson(string path, T obj, Encoding encoding = null, JsonSerial /// /// Saves the object data in XML format for the specified path /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The object to be saved /// The string encoding used /// True if the object was saved successfully @@ -462,7 +462,7 @@ private static bool IsSupportedKey(string key) { return !string.IsNullOrEmpty(key) && !UnsupportedKeyCharactersRegex.IsMatch(key) && !key.Contains("..", StringComparison.Ordinal) && HasName(key) // just in case - && key.Count(c => c == '/') <= 100; + && key.Count(c => c == '/') <= 100 && key.Count(c => c == '\\') <= 100; } /// @@ -470,7 +470,7 @@ private static bool IsSupportedKey(string key) /// private static bool HasName(string key) { - return key.Any(c => c != '.' && c != '/'); + return key.Any(c => c != '.' && c != '/' && c != '\\'); } /// @@ -484,7 +484,7 @@ private static string GetKeyIssues(string key) } return string.Join("; ", FindKeyIssues(key)) + ". Keys may only contain english letters, numbers, spaces and the characters" + - " '.', '/', '_', '#', '-', '$' and '=', never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'"; + " '.', '/', '\\', '_', '#', '-', '$' and '=', never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'"; } /// @@ -514,9 +514,9 @@ private static List FindKeyIssues(string key) issues.Add("it has no name, only '.' and separators"); } - if (key.Count(c => c == '/') > 100) + if (key.Count(c => c == '/') > 100 || key.Count(c => c == '\\') > 100) { - issues.Add("it has more than 100 '/' separators"); + issues.Add("it has more than 100 '/' or '\\' separators"); } if (issues.Count == 0) @@ -527,14 +527,13 @@ private static List FindKeyIssues(string key) } /// - /// Converts an unsupported key into a supported one: '\' becomes '/', the separator it already was for the local - /// store, every other unsupported character and every '.' of a '..' run become '_', so 'trade log: AI & Cloud.csv' - /// becomes 'trade log_ AI _ Cloud.csv'. + /// Converts an unsupported key into a supported one by replacing every unsupported character, and every '.' + /// of a '..' run, with '_': 'trade log: AI & Cloud.csv' becomes 'trade log_ AI _ Cloud.csv'. /// The mapping is lossy, distinct keys can sanitize to the same one. Throws when the key cannot be fixed /// private static string SanitizeKey(string key) { - var sanitized = UnsupportedKeyCharactersRegex.Replace(key.Replace('\\', '/'), "_"); + var sanitized = UnsupportedKeyCharactersRegex.Replace(key, "_"); sanitized = ConsecutiveDotsRegex.Replace(sanitized, match => new string('_', match.Length)); ValidateKey(sanitized); diff --git a/Common/Storage/PythonObjectStore.cs b/Common/Storage/PythonObjectStore.cs index 017a4f343e69..a0d64ec199bf 100644 --- a/Common/Storage/PythonObjectStore.cs +++ b/Common/Storage/PythonObjectStore.cs @@ -87,7 +87,7 @@ public PyObject ReadDataframe(string path) /// , and so is any other type that would not round trip. A top-level Python /// str, int, float or bool is not accepted, use for text /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The Python object to be saved /// True if the object was saved successfully /// A TypeError naming the value that cannot be serialized @@ -107,7 +107,7 @@ public bool SaveJson(string path, PyObject obj) /// Saves the given pandas DataFrame (or Series) for the specified path, in JSON format if the /// path has a '.json' extension and as CSV otherwise /// - /// The object key: english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. '\' becomes '/', other characters '_' + /// The object key: english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never '..', e.g. 'folder/trade_log-2024.v2.csv'. Other characters are replaced by '_' /// The pandas DataFrame or Series to be saved /// True if the object was saved successfully public bool SaveDataframe(string path, PyObject dataFrame) diff --git a/Tests/Common/Storage/LocalObjectStoreTests.cs b/Tests/Common/Storage/LocalObjectStoreTests.cs index cca0ebd8f384..f4f73cc0998a 100644 --- a/Tests/Common/Storage/LocalObjectStoreTests.cs +++ b/Tests/Common/Storage/LocalObjectStoreTests.cs @@ -639,7 +639,6 @@ public void SaveTextAndReadTextAreAliasesOfSaveAndRead() [TestCase("..model.pkl", "__model.pkl")] [TestCase("a/../b.csv", "a/__/b.csv")] [TestCase("a...b.csv", "a___b.csv")] - [TestCase("folder\\file.csv", "folder/file.csv")] [TestCase("file.csv\n", "file.csv_")] public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(string key, string sanitizedKey) { @@ -658,10 +657,10 @@ public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(s [TestCase("../secrets.json", "__/secrets.json")] [TestCase("../../secrets.json", "__/__/secrets.json")] - [TestCase("..\\secrets.json", "__/secrets.json")] + [TestCase("..\\secrets.json", "__\\secrets.json")] [TestCase("x/../secrets.json", "x/__/secrets.json")] [TestCase("x/../../secrets.json", "x/__/__/secrets.json")] - [TestCase("x\\..\\..\\secrets.json", "x/__/__/secrets.json")] + [TestCase("x\\..\\..\\secrets.json", "x\\__\\__\\secrets.json")] [TestCase("x/.../secrets.json", "x/___/secrets.json")] [TestCase("x/..", "x/__")] [TestCase("x/....", "x/____")] @@ -676,7 +675,7 @@ public void TraversalKeysAreSanitizedAndStayInsideTheStorageRoot(string key, str Assert.That(Path.GetFullPath(store.GetFilePath(key)), Does.StartWith(root)); Assert.IsTrue(store.Save(key, "abc")); - Assert.IsTrue(store.Keys.Contains(sanitizedKey), string.Join(", ", store.Keys)); + Assert.IsTrue(store.Keys.Contains(sanitizedKey.Replace('\\', '/')), string.Join(", ", store.Keys)); Assert.IsFalse(store.Keys.Any(k => k.Contains("..")), string.Join(", ", store.Keys)); Assert.That(Path.GetFullPath(store.GetFilePath(sanitizedKey)), Does.StartWith(root)); Assert.AreEqual("abc", store.Read(key)); @@ -729,7 +728,7 @@ public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); Assert.AreEqual("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. " + - "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " + + "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '\\', '_', '#', '-', '$' and '=', " + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'. " + "Using 'trade log_ AI _ Cloud.csv' instead. Any other unsupported key is sanitized the same way; " + "a save throws when its sanitized name already holds an object not written from that key in this run", warnings[0]); @@ -744,7 +743,6 @@ public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() [TestCase("trade log: AI & Cloud.csv", "unsupported characters ':', '&'")] [TestCase("résumé.pdf", "unsupported character 'é'")] - [TestCase("folder\\file.csv", "unsupported character '\\'")] [TestCase("..model.pkl", "it contains '..'")] [TestCase("a/../b.csv", "it contains '..'")] [TestCase("trade log: AI..v2.csv", "unsupported character ':'; it contains '..'")] @@ -991,6 +989,30 @@ public void ANewRunThrowsOnSavingAnUnsupportedKeyAPreviousRunSanitized() } } + [Test] + public void ABackslashKeyIsSavedInEveryRunAsOnTheStoreItAlwaysReadAsASeparator() + { + var warnings = new List(); + for (var run = 1; run <= 2; run++) + { + using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + + Assert.IsTrue(store.Save("backslash_folder\\results.csv", $"run{run}")); + + Assert.AreEqual($"run{run}", store.Read("backslash_folder\\results.csv")); + Assert.AreEqual($"run{run}", store.Read("backslash_folder/results.csv")); + Assert.IsTrue(store.Keys.Contains("backslash_folder/results.csv"), string.Join(", ", store.Keys)); + Assert.IsTrue(store.ContainsKey("backslash_folder\\results.csv")); + } + Assert.IsEmpty(warnings, string.Join("\n", warnings)); + + using var lastRun = new ObjectStore(new TestLocalObjectStore()); + lastRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.IsTrue(lastRun.Delete("backslash_folder\\results.csv")); + Assert.IsFalse(lastRun.ContainsKey("backslash_folder/results.csv")); + } + [Test] public void ASaveToTheSanitizedNameFromAnotherKeyEndsTheSilentUpdatesOfAnUnsupportedKey() { @@ -1048,7 +1070,7 @@ public void LocalObjectStoreRejectsUnsupportedKeysStatingTheIssue() var error = Assert.Throws(() => store.SaveBytes("trade_log_ai_hardware_&_cloud.csv", new byte[] { 1 })); Assert.AreEqual("ObjectStore: path is not supported: 'trade_log_ai_hardware_&_cloud.csv': unsupported character '&'. " + - "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '_', '#', '-', '$' and '=', " + + "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '\\', '_', '#', '-', '$' and '=', " + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'", error.Message); } From e09336f76fd0f20388d234d84a611c9b0f08cda3 Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Thu, 5 Nov 2026 17:10:27 -0400 Subject: [PATCH 4/5] Save unsupported keys without tracking who wrote the sanitized name and list both keys A save through an unsupported key always goes to its sanitized name and replaces the object there. Reads use the sanitized name when it exists, else an object stored under the name as given. ObjectStore.Keys lists an unsupported key saved in the run next to its sanitized name, and its Contains finds a key the way ContainsKey does. --- ...StoreKeySanitizationRegressionAlgorithm.cs | 37 +-- ...StoreKeySanitizationRegressionAlgorithm.py | 27 +- Common/Storage/ObjectStore.cs | 169 +++++++------ Tests/Common/Storage/LocalObjectStoreTests.cs | 232 +++++++++--------- 4 files changed, 229 insertions(+), 236 deletions(-) diff --git a/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs b/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs index a2a85ce59621..0557fae700f9 100644 --- a/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs +++ b/Algorithm.CSharp/ObjectStoreKeySanitizationRegressionAlgorithm.cs @@ -13,15 +13,15 @@ * limitations under the License. */ -using System; using System.Collections.Generic; +using System.Linq; using QuantConnect.Interfaces; namespace QuantConnect.Algorithm.CSharp { /// /// Regression algorithm asserting the ObjectStore key rules: an unsupported key is sanitized and used on every - /// save and read, and once the sanitized name is written directly a save through the unsupported key throws + /// save and read, and a save through it replaces the object stored under its sanitized name /// public class ObjectStoreKeySanitizationRegressionAlgorithm : QCAlgorithm, IRegressionAlgorithmDefinition { @@ -50,11 +50,13 @@ public override void OnEndOfAlgorithm() { throw new RegressionTestException("Expected the unsupported key to be sanitized and saved"); } - if (!ObjectStore.Keys.Contains(sanitizedKey) || ObjectStore.Keys.Contains(invalidKey)) + var keys = ObjectStore.Keys.ToList(); + if (!keys.Contains(sanitizedKey) || !keys.Contains(invalidKey)) { - throw new RegressionTestException($"Expected the object to be stored under '{sanitizedKey}', keys: {string.Join(", ", ObjectStore.Keys)}"); + throw new RegressionTestException($"Expected the keys to hold '{sanitizedKey}' and '{invalidKey}', keys: {string.Join(", ", keys)}"); } - if (!ObjectStore.ContainsKey(invalidKey) || ObjectStore.Read(invalidKey) != "a,b\n1,2" || ObjectStore.Read(sanitizedKey) != "a,b\n1,2") + if (!ObjectStore.ContainsKey(invalidKey) || !ObjectStore.Keys.Contains(invalidKey) + || ObjectStore.Read(invalidKey) != "a,b\n1,2" || ObjectStore.Read(sanitizedKey) != "a,b\n1,2") { throw new RegressionTestException("Expected the object to be read through both the original and the sanitized key"); } @@ -65,31 +67,14 @@ public override void OnEndOfAlgorithm() throw new RegressionTestException("Expected the second save through the unsupported key to update the object"); } - // once the sanitized name is written directly, as an upload or another run would, it is never replaced through the unsupported key - if (!ObjectStore.Save(sanitizedKey, "c,d\n5,6")) + // a save through the unsupported key replaces the object under its sanitized name, whoever wrote it + if (!ObjectStore.Save(sanitizedKey, "c,d\n5,6") || !ObjectStore.Save(invalidKey, "e,f\n7,8") || ObjectStore.Read(sanitizedKey) != "e,f\n7,8") { - throw new RegressionTestException("Expected the save through the sanitized key to succeed"); - } - var errorMessage = string.Empty; - try - { - ObjectStore.Save(invalidKey, "e,f\n7,8"); - } - catch (ArgumentException exception) - { - errorMessage = exception.Message; - } - if (!errorMessage.Contains($"its sanitized name '{sanitizedKey}' already holds another object")) - { - throw new RegressionTestException($"Expected the save through the unsupported key to throw, got: '{errorMessage}'"); - } - if (ObjectStore.Read(sanitizedKey) != "c,d\n5,6") - { - throw new RegressionTestException("Expected the object written through the sanitized key to be kept"); + throw new RegressionTestException("Expected the save through the unsupported key to replace the object under its sanitized name"); } ObjectStore.Delete(sanitizedKey); - if (ObjectStore.ContainsKey(invalidKey)) + if (ObjectStore.ContainsKey(invalidKey) || ObjectStore.Keys.ToList().Contains(invalidKey)) { throw new RegressionTestException("Expected the delete of the sanitized key to remove the object the unsupported key reads"); } diff --git a/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py b/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py index a55f38c3c78a..17884f04ef9e 100644 --- a/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py +++ b/Algorithm.Python/ObjectStoreKeySanitizationRegressionAlgorithm.py @@ -15,7 +15,7 @@ ### ### Regression algorithm asserting the ObjectStore key rules: an unsupported key is sanitized and used on every -### save and read, and once the sanitized name is written directly a save through the unsupported key throws +### save and read, and a save through it replaces the object stored under its sanitized name ### class ObjectStoreKeySanitizationRegressionAlgorithm(QCAlgorithm): @@ -34,9 +34,10 @@ def on_end_of_algorithm(self): if not self.object_store.save(invalid_key, "a,b\n1,2"): raise AssertionError("Expected the unsupported key to be sanitized and saved") keys = list(self.object_store.keys) - if sanitized_key not in keys or invalid_key in keys: - raise AssertionError(f"Expected the object to be stored under '{sanitized_key}', keys: {keys}") - if not self.object_store.contains_key(invalid_key) or self.object_store.read(invalid_key) != "a,b\n1,2" \ + if sanitized_key not in keys or invalid_key not in keys: + raise AssertionError(f"Expected the keys to hold '{sanitized_key}' and '{invalid_key}', keys: {keys}") + if not self.object_store.contains_key(invalid_key) or invalid_key not in self.object_store.keys \ + or self.object_store.read(invalid_key) != "a,b\n1,2" \ or self.object_store.read(sanitized_key) != "a,b\n1,2": raise AssertionError("Expected the object to be read through both the original and the sanitized key") @@ -44,19 +45,11 @@ def on_end_of_algorithm(self): if not self.object_store.save(invalid_key, "a,b\n3,4") or self.object_store.read(sanitized_key) != "a,b\n3,4": raise AssertionError("Expected the second save through the unsupported key to update the object") - # once the sanitized name is written directly, as an upload or another run would, it is never replaced through the unsupported key - if not self.object_store.save(sanitized_key, "c,d\n5,6"): - raise AssertionError("Expected the save through the sanitized key to succeed") - error = "" - try: - self.object_store.save(invalid_key, "e,f\n7,8") - except BaseException as e: - error = str(e) - if f"its sanitized name '{sanitized_key}' already holds another object" not in error: - raise AssertionError(f"Expected the save through the unsupported key to throw, got: '{error}'") - if self.object_store.read(sanitized_key) != "c,d\n5,6": - raise AssertionError("Expected the object written through the sanitized key to be kept") + # a save through the unsupported key replaces the object under its sanitized name, whoever wrote it + if not self.object_store.save(sanitized_key, "c,d\n5,6") or not self.object_store.save(invalid_key, "e,f\n7,8") \ + or self.object_store.read(sanitized_key) != "e,f\n7,8": + raise AssertionError("Expected the save through the unsupported key to replace the object under its sanitized name") self.object_store.delete(sanitized_key) - if self.object_store.contains_key(invalid_key): + if self.object_store.contains_key(invalid_key) or invalid_key in list(self.object_store.keys): raise AssertionError("Expected the delete of the sanitized key to remove the object the unsupported key reads") diff --git a/Common/Storage/ObjectStore.cs b/Common/Storage/ObjectStore.cs index 3610f239a6a6..aa8b135345c6 100644 --- a/Common/Storage/ObjectStore.cs +++ b/Common/Storage/ObjectStore.cs @@ -33,11 +33,9 @@ namespace QuantConnect.Storage /// /// Helper class for easier access to methods. A key that does not follow the key /// format (english letters, numbers, spaces and the characters '.', '/', '\', '_', '#', '-', '$' and '=', never - /// two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv') is saved under its sanitized version and found by - /// either name, with a warning the first time it happens. A save whose - /// sanitized name already holds an object this store did not write from that key throws, since nothing records who - /// wrote that object. An object already stored under an unsupported name is read as is; a save to that name goes - /// to the sanitized name, warning every time + /// two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv') is saved under its sanitized version, then found and + /// listed by either name, with a warning the first time it happens. An object already stored under an unsupported name + /// is read as is until a save through that name creates the sanitized one /// public class ObjectStore : IObjectStore { @@ -48,9 +46,9 @@ public class ObjectStore : IObjectStore private readonly IObjectStore _store; private readonly Action _warn; + // The unsupported keys saved in this run and the sanitized name each one is stored under + private readonly ConcurrentDictionary _sanitizedKeys = new(); private int _warned; - // the unsupported keys this store last wrote each sanitized name from: their saves are updates, and reads follow them - private readonly ConcurrentDictionary _savedKeys = new(); /// /// Gets the maximum storage limit in bytes @@ -96,9 +94,19 @@ public void Initialize(int userId, int projectId, string userToken, Controls con } /// - /// Returns the file paths present in the object store. This is specially useful not to load the object store into memory + /// Returns the file paths present in the object store. This is specially useful not to load the object store into memory. + /// An unsupported key saved in this run is listed next to the sanitized name its object is stored under, + /// and Contains finds a key the way does /// - public ICollection Keys => _store.Keys; + public ICollection Keys + { + get + { + var keys = _store.Keys; + var unsupportedKeys = _sanitizedKeys.Where(pair => _store.ContainsKey(pair.Value)).Select(pair => pair.Key).ToList(); + return new KeyCollection(unsupportedKeys.Count == 0 ? keys : keys.Union(unsupportedKeys).ToList(), ContainsKey); + } + } /// /// Will clear the object store state cache. This is useful when the object store is used concurrently by nodes which want to share information @@ -133,17 +141,7 @@ public byte[] ReadBytes(string path) /// True if the save operation was successful public bool SaveBytes(string path, byte[] contents) { - var key = KeyToSave(path); - if (!_store.SaveBytes(key, contents)) - { - return false; - } - ForgetSavesTo(key, exceptFrom: path); - if (key != path) - { - _savedKeys[path] = key; - } - return true; + return _store.SaveBytes(KeyToSave(path), contents); } /// @@ -153,13 +151,7 @@ public bool SaveBytes(string path, byte[] contents) /// True if the delete operation was successful public bool Delete(string path) { - var key = KeyToRead(path); - if (!_store.Delete(key)) - { - return false; - } - ForgetSavesTo(key); - return true; + return _store.Delete(KeyToRead(path)); } /// @@ -169,15 +161,7 @@ public bool Delete(string path) /// The path for the file public string GetFilePath(string path) { - var key = KeyToRead(path); - // the store registers a missing key when handing out its path: that object is then this key's, so the save that follows is an update - var created = key != path && !_store.ContainsKey(key); - var filePath = _store.GetFilePath(key); - if (created) - { - _savedKeys[path] = key; - } - return filePath; + return _store.GetFilePath(KeyToRead(path)); } /// @@ -385,8 +369,8 @@ internal static void ValidateKey(string key) } /// - /// The key a save goes to: the given one when it follows the key format, else its sanitized version. Throws when - /// that already holds an object this store did not last write from the given key. Null and empty keys are left for the store to reject + /// The key a save goes to: the given one when it follows the key format, else its sanitized version, replacing + /// the object stored there. Null and empty keys are left for the store to reject /// private string KeyToSave(string path) { @@ -396,55 +380,28 @@ private string KeyToSave(string path) } var key = SanitizeKey(path); + _sanitizedKeys[path] = key; WarnOnce(path, key); - if (!_savedKeys.ContainsKey(path) && _store.ContainsKey(key)) - { - // nothing records who wrote that object, a previous run, another algorithm or an upload: it is never replaced through another key - throw new ArgumentException($"ObjectStore: key '{path}' is not supported and its sanitized name '{key}' already holds" + - $" another object. Delete '{key}' or save to it directly to replace it"); - } - if (_store.ContainsKey(path)) - { - // the object under the name as given stays: reads of it from another store still return it - _warn($"ObjectStore: key '{path}' is not supported: the object stored under that name is left as is, this save goes to '{key}'"); - } return key; } /// - /// Forgets the unsupported keys whose object under the given key is gone or replaced from another key - /// - private void ForgetSavesTo(string key, string exceptFrom = null) - { - foreach (var entry in _savedKeys) - { - if (entry.Value == key && entry.Key != exceptFrom) - { - _savedKeys.TryRemove(entry.Key, out _); - } - } - } - - /// - /// The key the other operations use: where this store saved it while that object exists, else the given one when - /// it follows the key format or the store already has it, else its sanitized version, so an object is found by the key it was saved with + /// The key the other operations use: the given one when it follows the key format, else its sanitized version, + /// so an object is found by the key it was saved with. An object stored under the name as given, before the + /// key format applied, is used while the sanitized one does not exist /// private string KeyToRead(string path) { - if (string.IsNullOrEmpty(path)) + if (string.IsNullOrEmpty(path) || IsSupportedKey(path)) { return path; } - if (_savedKeys.TryGetValue(path, out var key) && _store.ContainsKey(key)) - { - return key; - } - if (IsSupportedKey(path) || _store.ContainsKey(path)) + + var key = SanitizeKey(path); + if (!_store.ContainsKey(key) && _store.ContainsKey(path)) { return path; } - - key = SanitizeKey(path); WarnOnce(path, key); return key; } @@ -453,8 +410,8 @@ private void WarnOnce(string path, string key) { if (Interlocked.Exchange(ref _warned, 1) == 0) { - _warn($"ObjectStore: key '{path}' is not supported: {GetKeyIssues(path)}. Using '{key}' instead. Any other unsupported" + - " key is sanitized the same way; a save throws when its sanitized name already holds an object not written from that key in this run"); + _warn($"ObjectStore: key '{path}' is not supported: {GetKeyIssues(path)}. Using '{key}' instead." + + " Any other unsupported key is sanitized the same way"); } } @@ -539,5 +496,67 @@ private static string SanitizeKey(string key) ValidateKey(sanitized); return sanitized; } + + /// + /// The keys of the object store, whose Contains finds a key the way does + /// + private sealed class KeyCollection : ICollection + { + private readonly ICollection _keys; + private readonly Func _containsKey; + + public int Count => _keys.Count; + + public bool IsReadOnly => _keys.IsReadOnly; + + public KeyCollection(ICollection keys, Func containsKey) + { + _keys = keys; + _containsKey = containsKey; + } + + public bool Contains(string item) + { + return !string.IsNullOrEmpty(item) && _containsKey(item); + } + + /// + /// Python's 'in' operator calls this on a type that also has a Count + /// + public bool ContainsKey(string key) + { + return Contains(key); + } + + public void Add(string item) + { + _keys.Add(item); + } + + public void Clear() + { + _keys.Clear(); + } + + public bool Remove(string item) + { + return _keys.Remove(item); + } + + public void CopyTo(string[] array, int arrayIndex) + { + _keys.CopyTo(array, arrayIndex); + } + + public IEnumerator GetEnumerator() + { + return _keys.GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + } } } diff --git a/Tests/Common/Storage/LocalObjectStoreTests.cs b/Tests/Common/Storage/LocalObjectStoreTests.cs index f4f73cc0998a..df8b919947b6 100644 --- a/Tests/Common/Storage/LocalObjectStoreTests.cs +++ b/Tests/Common/Storage/LocalObjectStoreTests.cs @@ -248,7 +248,7 @@ public void InvalidCustomPathsStore(string path, string sanitizedPath) // '..' cannot leave the storage root: it is sanitized away Assert.IsTrue(store.SaveString($"{path}ILove", "Pizza")); - Assert.AreEqual(new[] { $"{sanitizedPath}ILove" }, store.Keys); + Assert.AreEqual(new[] { $"{sanitizedPath}ILove", $"{path}ILove" }, store.Keys); Assert.That(Path.GetFullPath(store.GetFilePath($"{path}ILove")), Does.StartWith(Path.GetFullPath(TestStorageRoot))); store.Delete($"{path}ILove"); @@ -645,7 +645,7 @@ public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(s Assert.IsTrue(_store.Save(key, "abc")); Assert.IsTrue(_store.Keys.Contains(sanitizedKey), string.Join(", ", _store.Keys)); - Assert.AreEqual(key == sanitizedKey, _store.Keys.Contains(key)); + Assert.That(_store.Keys.ToList(), Does.Contain(key)); Assert.IsTrue(_store.ContainsKey(key)); Assert.IsTrue(_store.ContainsKey(sanitizedKey)); Assert.AreEqual("abc", _store.Read(key)); @@ -653,6 +653,7 @@ public void UnsupportedKeysAreSanitizedOnSaveAndFoundByTheKeyTheyWereSavedWith(s Assert.IsTrue(_store.Delete(key)); Assert.IsFalse(_store.ContainsKey(sanitizedKey)); + Assert.That(_store.Keys.ToList(), Does.Not.Contain(key)); } [TestCase("../secrets.json", "__/secrets.json")] @@ -676,7 +677,9 @@ public void TraversalKeysAreSanitizedAndStayInsideTheStorageRoot(string key, str Assert.IsTrue(store.Save(key, "abc")); Assert.IsTrue(store.Keys.Contains(sanitizedKey.Replace('\\', '/')), string.Join(", ", store.Keys)); - Assert.IsFalse(store.Keys.Any(k => k.Contains("..")), string.Join(", ", store.Keys)); + // the keys list the key as saved too, the objects are only stored under names without '..' + Assert.That(store.Keys.ToList(), Does.Contain(key)); + Assert.IsFalse(store.Any(entry => entry.Key.Contains("..")), string.Join(", ", store.Select(entry => entry.Key))); Assert.That(Path.GetFullPath(store.GetFilePath(sanitizedKey)), Does.StartWith(root)); Assert.AreEqual("abc", store.Read(key)); Assert.That(warnings[0], Does.Contain("it contains '..'")); @@ -686,31 +689,23 @@ public void TraversalKeysAreSanitizedAndStayInsideTheStorageRoot(string key, str } [Test] - public void GetFilePathOfAnUnsupportedKeyPairsItWithTheSanitizedNameForTheSaveThatFollows() + public void GetFilePathOfAnUnsupportedKeyIsTheFileTheSaveThatFollowsReads() { var warnings = new List(); using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - // the store registers the key when handing out the path, so the file written there is saved through the same key var filePath = store.GetFilePath("model (1).pkl"); Assert.AreEqual(Path.GetFullPath(Path.Combine(TestStorageRoot, "model _1_.pkl")), Path.GetFullPath(filePath)); File.WriteAllText(filePath, "abc"); + Assert.IsTrue(store.Save("model (1).pkl")); + Assert.AreEqual("abc", store.Read("model (1).pkl")); Assert.AreEqual("abc", store.Read("model _1_.pkl")); - Assert.IsTrue(store.Save("model (1).pkl", "abcd")); - Assert.AreEqual("abcd", store.Read("model _1_.pkl")); Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); Assert.IsTrue(store.Delete("model (1).pkl")); - - // an object another key wrote to the sanitized name is only read through the path: the save still throws - Assert.IsTrue(store.Save("model _1_.pkl", "other")); - Assert.AreEqual(Path.GetFullPath(filePath), Path.GetFullPath(store.GetFilePath("model (1).pkl"))); - var exception = Assert.Throws(() => store.Save("model (1).pkl", "mine")); - Assert.That(exception.Message, Does.StartWith("ObjectStore: key 'model (1).pkl' is not supported and its sanitized name 'model _1_.pkl' already holds another object")); - Assert.AreEqual("other", store.Read("model _1_.pkl")); - Assert.IsTrue(store.Delete("model _1_.pkl")); + Assert.IsFalse(store.ContainsKey("model _1_.pkl")); } [Test] @@ -730,8 +725,7 @@ public void UnsupportedKeysAreWarnedAboutOnceStatingTheIssueAndTheKeyUsed() Assert.AreEqual("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported: unsupported characters ':', '&'. " + "Keys may only contain english letters, numbers, spaces and the characters '.', '/', '\\', '_', '#', '-', '$' and '=', " + "never two '.' in a row, e.g. 'folder/trade_log-2024.v2.csv'. " + - "Using 'trade log_ AI _ Cloud.csv' instead. Any other unsupported key is sanitized the same way; " + - "a save throws when its sanitized name already holds an object not written from that key in this run", warnings[0]); + "Using 'trade log_ AI _ Cloud.csv' instead. Any other unsupported key is sanitized the same way", warnings[0]); // the next unsupported key is sanitized silently Assert.IsFalse(store.ContainsKey("trade log: AI.v2.csv")); @@ -806,9 +800,9 @@ public void ExistingUnsupportedKeysAreUsedAsGiven(string key) } [Test] - public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWithAWarningEveryTime() + public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWhichReadsThenReturn() { - // an uploaded object under an unsupported name, which the algorithm loads, retrains and saves back + // an object put in the store by other means under an unsupported name keeps it: saves through that name go to the sanitized one var filePath = Path.Combine(TestStorageRoot, "model (1).tar.gz"); File.WriteAllText(filePath, "old"); var warnings = new List(); @@ -816,58 +810,32 @@ public void SavingToAnExistingUnsupportedKeyGoesToTheSanitizedNameWithAWarningEv { store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); Assert.AreEqual("old", store.Read("model (1).tar.gz")); + Assert.IsEmpty(warnings, string.Join("\n", warnings)); Assert.IsTrue(store.Save("model (1).tar.gz", "new")); - Assert.AreEqual(2, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); Assert.That(warnings[0], Does.StartWith("ObjectStore: key 'model (1).tar.gz' is not supported: unsupported characters '(', ')'.")); - Assert.AreEqual("ObjectStore: key 'model (1).tar.gz' is not supported: the object stored under that name is left as is," + - " this save goes to 'model _1_.tar.gz'", warnings[1]); - // read after write within the run follows the save Assert.AreEqual("new", store.Read("model (1).tar.gz")); Assert.AreEqual("new", store.Read("model _1_.tar.gz")); Assert.AreEqual("old", File.ReadAllText(filePath)); - Assert.IsTrue(store.Keys.Contains("model (1).tar.gz") && store.Keys.Contains("model _1_.tar.gz"), string.Join(", ", store.Keys)); - - // every such save warns, not only the first - Assert.IsTrue(store.Save("model (1).tar.gz", "newer")); - Assert.AreEqual(3, warnings.Count, string.Join("\n", warnings)); - Assert.AreEqual(warnings[1], warnings[2]); - Assert.AreEqual("newer", store.Read("model (1).tar.gz")); } - // a fresh store, the next run: the object under the name as given comes first again, and the save finds - // the previous run's object under the sanitized name, which nothing ties to this key any more - warnings.Clear(); + // a later run reads and replaces the sanitized object through the same name using (var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add)) { store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - Assert.AreEqual("old", store.Read("model (1).tar.gz")); - Assert.IsEmpty(warnings); - - var exception = Assert.Throws(() => store.Save("model (1).tar.gz", "third")); - Assert.AreEqual("ObjectStore: key 'model (1).tar.gz' is not supported and its sanitized name 'model _1_.tar.gz' already holds" + - " another object. Delete 'model _1_.tar.gz' or save to it directly to replace it", exception.Message); - Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + Assert.AreEqual("new", store.Read("model (1).tar.gz")); + Assert.IsTrue(store.Save("model (1).tar.gz", "newer")); Assert.AreEqual("newer", store.Read("model _1_.tar.gz")); - // once the sanitized name is free the save goes there, warning about the original every time - Assert.IsTrue(store.Delete("model _1_.tar.gz")); - Assert.IsTrue(store.Save("model (1).tar.gz", "third")); - Assert.AreEqual(2, warnings.Count, string.Join("\n", warnings)); - Assert.That(warnings[1], Does.Contain("the object stored under that name is left as is, this save goes to 'model _1_.tar.gz'")); - Assert.AreEqual("third", store.Read("model (1).tar.gz")); - Assert.AreEqual("third", store.Read("model _1_.tar.gz")); - - // within the run the key follows the save: the first delete removes it and the original is back + // with the sanitized object gone, the one stored under the name as given is read again Assert.IsTrue(store.Delete("model (1).tar.gz")); Assert.IsFalse(store.ContainsKey("model _1_.tar.gz")); - Assert.AreEqual("old", File.ReadAllText(filePath)); Assert.AreEqual("old", store.Read("model (1).tar.gz")); Assert.IsTrue(store.Delete("model (1).tar.gz")); Assert.IsFalse(store.ContainsKey("model (1).tar.gz")); } - Assert.IsFalse(File.Exists(filePath)); } [Test] @@ -927,66 +895,43 @@ public void CustomDataIsReadFromAnExistingUnsupportedKey() } [Test] - public void SavingAnUnsupportedKeyWhoseSanitizedNameHoldsAnotherObjectThrowsEveryTime() + public void SavingAnUnsupportedKeyReplacesTheObjectUnderItsSanitizedName() { var warnings = new List(); using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - Assert.IsTrue(store.Save("trade_log_ai_cloud.csv", "existing")); - Assert.IsEmpty(warnings); + Assert.IsTrue(store.Save("p_q.csv", "direct")); - // sanitizes to the existing object's key: the general warning, then the exception - var exception = Assert.Throws(() => store.Save("trade_log_ai&cloud.csv", "first")); - Assert.AreEqual("ObjectStore: key 'trade_log_ai&cloud.csv' is not supported and its sanitized name 'trade_log_ai_cloud.csv' " + - "already holds another object. Delete 'trade_log_ai_cloud.csv' or save to it directly to replace it", exception.Message); - Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); - Assert.That(warnings[0], Does.StartWith("ObjectStore: key 'trade_log_ai&cloud.csv' is not supported: unsupported character '&'.")); - Assert.AreEqual("existing", store.Read("trade_log_ai_cloud.csv")); - - // every such save throws, from this key or another one sanitizing to the same name - Assert.Throws(() => store.Save("trade_log_ai&cloud.csv", "first")); - exception = Assert.Throws(() => store.Save("trade_log_ai:cloud.csv", "second")); - Assert.AreEqual("ObjectStore: key 'trade_log_ai:cloud.csv' is not supported and its sanitized name 'trade_log_ai_cloud.csv' " + - "already holds another object. Delete 'trade_log_ai_cloud.csv' or save to it directly to replace it", exception.Message); - Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); - Assert.AreEqual("existing", store.Read("trade_log_ai_cloud.csv")); + Assert.IsTrue(store.Save("p&q.csv", "ampersand")); + Assert.AreEqual("ampersand", store.Read("p_q.csv")); - // once the name is free the key is saved there and updated silently - Assert.IsTrue(store.Delete("trade_log_ai_cloud.csv")); - Assert.IsTrue(store.Save("trade_log_ai&cloud.csv", "first")); - Assert.IsTrue(store.Save("trade_log_ai&cloud.csv", "second")); + // distinct unsupported keys can share a sanitized name: the last save wins + Assert.IsTrue(store.Save("p:q.csv", "colon")); + Assert.AreEqual("colon", store.Read("p&q.csv")); + Assert.AreEqual("colon", store.Read("p_q.csv")); Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); - Assert.AreEqual("second", store.Read("trade_log_ai_cloud.csv")); - Assert.AreEqual(1, store.Keys.Count(key => key.StartsWith("trade_log_ai"))); - Assert.IsTrue(store.Delete("trade_log_ai&cloud.csv")); - Assert.IsFalse(store.ContainsKey("trade_log_ai_cloud.csv")); + Assert.IsTrue(store.Delete("p&q.csv")); + Assert.IsFalse(store.ContainsKey("p_q.csv")); } [Test] - public void ANewRunThrowsOnSavingAnUnsupportedKeyAPreviousRunSanitized() + public void ANewRunSavesThroughTheSameUnsupportedKeyAgain() { - // nothing records that a previous run wrote the sanitized object, so no later run replaces it through the unsupported key - using (var firstRun = new ObjectStore(new TestLocalObjectStore(), _ => { })) + for (var run = 1; run <= 2; run++) { - firstRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - Assert.IsTrue(firstRun.Save("trade log: AI & Cloud.csv", "run1")); - } + using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - using (var secondRun = new ObjectStore(new TestLocalObjectStore(), _ => { })) - { - secondRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); - var exception = Assert.Throws(() => secondRun.Save("trade log: AI & Cloud.csv", "run2")); - Assert.AreEqual("ObjectStore: key 'trade log: AI & Cloud.csv' is not supported and its sanitized name 'trade log_ AI _ Cloud.csv'" + - " already holds another object. Delete 'trade log_ AI _ Cloud.csv' or save to it directly to replace it", exception.Message); - Assert.AreEqual("run1", secondRun.Read("trade log: AI & Cloud.csv")); + Assert.IsTrue(store.Save("trade log: AI & Cloud.csv", $"run{run}")); - // the exact sanitized key is a supported key: the user replaces the object through it - Assert.IsTrue(secondRun.Save("trade log_ AI _ Cloud.csv", "run2")); - Assert.AreEqual("run2", secondRun.Read("trade log: AI & Cloud.csv")); - Assert.Throws(() => secondRun.Save("trade log: AI & Cloud.csv", "run2 again")); - Assert.IsTrue(secondRun.Delete("trade log_ AI _ Cloud.csv")); + Assert.AreEqual($"run{run}", store.Read("trade log: AI & Cloud.csv")); + Assert.AreEqual($"run{run}", store.Read("trade log_ AI _ Cloud.csv")); } + + using var lastRun = new ObjectStore(new TestLocalObjectStore(), _ => { }); + lastRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.IsTrue(lastRun.Delete("trade log: AI & Cloud.csv")); } [Test] @@ -1014,35 +959,64 @@ public void ABackslashKeyIsSavedInEveryRunAsOnTheStoreItAlwaysReadAsASeparator() } [Test] - public void ASaveToTheSanitizedNameFromAnotherKeyEndsTheSilentUpdatesOfAnUnsupportedKey() + public void KeysHoldAnUnsupportedKeySavedInThisRunAndItsSanitizedName() { - var warnings = new List(); - using var store = new ObjectStore(new TestLocalObjectStore(), warnings.Add); + using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.IsTrue(store.Save("keys check: A & B.csv", "abc")); + Assert.IsTrue(store.Save("keys check: A & B.csv", "def")); + Assert.IsTrue(store.Save("keys_folder\\file.csv", "abc")); + + var keys = store.Keys; + + var listed = keys.ToList(); + Assert.AreEqual(1, listed.Count(key => key == "keys check: A & B.csv"), string.Join(", ", listed)); + Assert.AreEqual(1, listed.Count(key => key == "keys check_ A _ B.csv"), string.Join(", ", listed)); + Assert.That(listed, Does.Contain("keys_folder/file.csv")); + Assert.AreEqual(listed.Count, keys.Count); + var copy = new string[keys.Count]; + keys.CopyTo(copy, 0); + Assert.AreEqual(listed, copy); + + Assert.IsTrue(keys.Contains("keys check: A & B.csv")); + Assert.IsTrue(keys.Contains("keys check_ A _ B.csv")); + Assert.IsTrue(keys.Contains("keys_folder\\file.csv")); + Assert.IsTrue(keys.Contains("keys_folder/file.csv")); + Assert.IsFalse(keys.Contains("keys_missing.csv")); + Assert.IsFalse(keys.Contains(null)); + + // a run that did not save through the unsupported key lists the stored name only, and still finds the key + using (var otherRun = new ObjectStore(new TestLocalObjectStore(), _ => { })) + { + otherRun.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + Assert.That(otherRun.Keys.ToList(), Does.Contain("keys check_ A _ B.csv")); + Assert.That(otherRun.Keys.ToList(), Does.Not.Contain("keys check: A & B.csv")); + Assert.IsTrue(otherRun.Keys.Contains("keys check: A & B.csv")); + } - Assert.IsTrue(store.Save("p&q.csv", "A1")); - Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); + // the unsupported key is listed while the object it was saved to exists + Assert.IsTrue(store.Delete("keys check_ A _ B.csv")); + Assert.That(store.Keys.ToList(), Does.Not.Contain("keys check: A & B.csv")); + Assert.IsFalse(store.Keys.Contains("keys check: A & B.csv")); + Assert.IsTrue(store.Delete("keys_folder\\file.csv")); + } - // another unsupported key sanitizing to the same name throws, and the first key keeps updating silently - var exception = Assert.Throws(() => store.Save("p:q.csv", "B1")); - Assert.That(exception.Message, Does.StartWith("ObjectStore: key 'p:q.csv' is not supported and its sanitized name 'p_q.csv' already holds another object")); - Assert.AreEqual("A1", store.Read("p_q.csv")); - Assert.IsTrue(store.Save("p&q.csv", "A2")); - Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); - Assert.AreEqual("A2", store.Read("p_q.csv")); + [Test] + public void KeysListAnExistingUnsupportedNameOnceAfterASaveThroughIt() + { + using var store = new ObjectStore(new TestLocalObjectStore(), _ => { }); + store.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, AlgorithmMode.Backtesting); + File.WriteAllText(Path.Combine(TestStorageRoot, "keys legacy (1).csv"), "old"); - // a save through the valid name takes the object over: the unsupported key now throws - Assert.IsTrue(store.Save("p_q.csv", "C1")); - exception = Assert.Throws(() => store.Save("p&q.csv", "A3")); - Assert.That(exception.Message, Does.StartWith("ObjectStore: key 'p&q.csv' is not supported and its sanitized name 'p_q.csv' already holds another object")); - Assert.AreEqual("C1", store.Read("p_q.csv")); + Assert.IsTrue(store.Save("keys legacy (1).csv", "new")); - Assert.IsTrue(store.Delete("p_q.csv")); - Assert.IsTrue(store.Save("p&q.csv", "A3")); - Assert.AreEqual(1, warnings.Count, string.Join("\n", warnings)); - Assert.AreEqual("A3", store.Read("p&q.csv")); - Assert.IsTrue(store.Delete("p&q.csv")); - Assert.IsFalse(store.ContainsKey("p_q.csv")); + var listed = store.Keys.ToList(); + Assert.AreEqual(1, listed.Count(key => key == "keys legacy (1).csv"), string.Join(", ", listed)); + Assert.AreEqual(1, listed.Count(key => key == "keys legacy _1_.csv"), string.Join(", ", listed)); + + Assert.IsTrue(store.Delete("keys legacy _1_.csv")); + Assert.IsTrue(store.Delete("keys legacy (1).csv")); + Assert.That(store.Keys.ToList(), Does.Not.Contain("keys legacy (1).csv")); } [Test] @@ -1889,6 +1863,28 @@ assert not object_store.contains_key('helpers/trade log_ AI _ Cloud.json') "); } + [Test] + public void PythonKeysHoldAnUnsupportedKeySavedInThisRunAndItsSanitizedName() + { + RunPythonTest(@" +def test(object_store): + key = 'helpers/in check: A & B.csv' + object_store.save(key, 'abc') + + assert key in object_store.keys + assert 'helpers/in check_ A _ B.csv' in object_store.keys + assert 'helpers/missing.csv' not in object_store.keys + # the keys list the key as saved next to the name the object is stored under + assert list(object_store.keys).count('helpers/in check_ A _ B.csv') == 1 + assert list(object_store.keys).count(key) == 1 + assert len(object_store.keys) == len(list(object_store.keys)) + + object_store.delete(key) + assert key not in object_store.keys + assert key not in list(object_store.keys) +"); + } + /// /// Runs the 'test(object_store)' function of the given Python source against the test store /// From e24e6d0048f87834b6392c82342317f542f99d6a Mon Sep 17 00:00:00 2001 From: Jhonathan Abreu Date: Thu, 5 Nov 2026 17:49:50 -0400 Subject: [PATCH 5/5] List the ObjectStore keys on first use only ObjectStore.Keys returns at once and lists the store when it is first enumerated, counted or copied. Contains, and Python's in, look the key up without listing. With no sanitized key in the run the store's own keys are returned as they are. --- Common/Storage/ObjectStore.cs | 51 ++++++++++-------- Tests/Common/Storage/LocalObjectStoreTests.cs | 54 +++++++++++++++++++ 2 files changed, 83 insertions(+), 22 deletions(-) diff --git a/Common/Storage/ObjectStore.cs b/Common/Storage/ObjectStore.cs index aa8b135345c6..74b6af0963fb 100644 --- a/Common/Storage/ObjectStore.cs +++ b/Common/Storage/ObjectStore.cs @@ -95,18 +95,10 @@ public void Initialize(int userId, int projectId, string userToken, Controls con /// /// Returns the file paths present in the object store. This is specially useful not to load the object store into memory. - /// An unsupported key saved in this run is listed next to the sanitized name its object is stored under, - /// and Contains finds a key the way does + /// An unsupported key saved in this run is listed next to the sanitized name its object is stored under. + /// The store is listed on first use, and Contains finds a key the way does, without listing it /// - public ICollection Keys - { - get - { - var keys = _store.Keys; - var unsupportedKeys = _sanitizedKeys.Where(pair => _store.ContainsKey(pair.Value)).Select(pair => pair.Key).ToList(); - return new KeyCollection(unsupportedKeys.Count == 0 ? keys : keys.Union(unsupportedKeys).ToList(), ContainsKey); - } - } + public ICollection Keys => new KeyCollection(ListKeys, ContainsKey); /// /// Will clear the object store state cache. This is useful when the object store is used concurrently by nodes which want to share information @@ -368,6 +360,20 @@ internal static void ValidateKey(string key) } } + /// + /// The names the objects are stored under, then the unsupported keys saved in this run whose object exists + /// + private ICollection ListKeys() + { + var keys = _store.Keys; + if (_sanitizedKeys.IsEmpty) + { + return keys; + } + var unsupportedKeys = _sanitizedKeys.Where(pair => _store.ContainsKey(pair.Value)).Select(pair => pair.Key).ToList(); + return unsupportedKeys.Count == 0 ? keys : keys.Union(unsupportedKeys).ToList(); + } + /// /// The key a save goes to: the given one when it follows the key format, else its sanitized version, replacing /// the object stored there. Null and empty keys are left for the store to reject @@ -498,20 +504,21 @@ private static string SanitizeKey(string key) } /// - /// The keys of the object store, whose Contains finds a key the way does + /// The keys of the object store, listed on first use. Its Contains finds a key the way + /// does, without listing the store /// private sealed class KeyCollection : ICollection { - private readonly ICollection _keys; + private readonly Lazy> _keys; private readonly Func _containsKey; - public int Count => _keys.Count; + public int Count => _keys.Value.Count; - public bool IsReadOnly => _keys.IsReadOnly; + public bool IsReadOnly => _keys.Value.IsReadOnly; - public KeyCollection(ICollection keys, Func containsKey) + public KeyCollection(Func> listKeys, Func containsKey) { - _keys = keys; + _keys = new Lazy>(listKeys); _containsKey = containsKey; } @@ -530,27 +537,27 @@ public bool ContainsKey(string key) public void Add(string item) { - _keys.Add(item); + _keys.Value.Add(item); } public void Clear() { - _keys.Clear(); + _keys.Value.Clear(); } public bool Remove(string item) { - return _keys.Remove(item); + return _keys.Value.Remove(item); } public void CopyTo(string[] array, int arrayIndex) { - _keys.CopyTo(array, arrayIndex); + _keys.Value.CopyTo(array, arrayIndex); } public IEnumerator GetEnumerator() { - return _keys.GetEnumerator(); + return _keys.Value.GetEnumerator(); } IEnumerator IEnumerable.GetEnumerator() diff --git a/Tests/Common/Storage/LocalObjectStoreTests.cs b/Tests/Common/Storage/LocalObjectStoreTests.cs index df8b919947b6..ca883f9e4700 100644 --- a/Tests/Common/Storage/LocalObjectStoreTests.cs +++ b/Tests/Common/Storage/LocalObjectStoreTests.cs @@ -1019,6 +1019,49 @@ public void KeysListAnExistingUnsupportedNameOnceAfterASaveThroughIt() Assert.That(store.Keys.ToList(), Does.Not.Contain("keys legacy (1).csv")); } + [Test] + public void KeysListTheStoreOnFirstUseOnlyAndContainsNeverDoes() + { + var fileHandler = new ListingCountFileHandler(); + var localStore = new TestLocalObjectStore(); + using var store = new ObjectStore(localStore, _ => { }); + localStore.Initialize(0, 0, "", new Controls() { PersistenceIntervalSeconds = -1 }, fileHandler); + Assert.IsTrue(store.Save("lazy keys: A & B.csv", "abc")); + + var listings = fileHandler.Listings; + var keys = store.Keys; + Assert.IsTrue(keys.Contains("lazy keys: A & B.csv")); + Assert.IsTrue(keys.Contains("lazy keys_ A _ B.csv")); + Assert.IsFalse(keys.Contains("lazy keys missing.csv")); + using (Py.GIL()) + { + var module = PyModule.FromString("TestLazyObjectStoreKeys", "def contains(keys, key):\n return key in keys\n"); + dynamic contains = module.GetAttr("contains"); + PyObject found = contains(store.Keys, "lazy keys: A & B.csv"); + PyObject missing = contains(store.Keys, "lazy keys missing.csv"); + Assert.IsTrue(found.As()); + Assert.IsFalse(missing.As()); + } + Assert.AreEqual(listings, fileHandler.Listings); + + // the listing is built on first use, once per collection + var listed = keys.ToList(); + Assert.AreEqual(listed.Count, keys.Count); + Assert.AreEqual(listed, keys.ToList()); + Assert.AreEqual(listings + 1, fileHandler.Listings); + Assert.That(listed, Does.Contain("lazy keys: A & B.csv")); + Assert.That(listed, Does.Contain("lazy keys_ A _ B.csv")); + + // a new access lists again, so it sees what changed since + Assert.IsTrue(store.Save("lazy_keys_later.csv", "abc")); + listings = fileHandler.Listings; + Assert.That(store.Keys.ToList(), Does.Contain("lazy_keys_later.csv")); + Assert.AreEqual(listings + 1, fileHandler.Listings); + + Assert.IsTrue(store.Delete("lazy keys: A & B.csv")); + Assert.IsTrue(store.Delete("lazy_keys_later.csv")); + } + [Test] public void QuantBookWarnsThroughTheEngineLog() { @@ -1955,6 +1998,17 @@ protected override bool PersistData() protected override string StorageRoot() => TestStorageRoot; } + private sealed class ListingCountFileHandler : FileHandler + { + public int Listings { get; private set; } + + public override IEnumerable EnumerateFiles(string path, string pattern, SearchOption searchOption, out string rootfolder) + { + Listings++; + return base.EnumerateFiles(path, pattern, searchOption, out rootfolder); + } + } + public class TestFileHandler : FileHandler { public override void WriteAllBytes(string path, byte[] data)