diff --git a/Directory.Packages.props b/Directory.Packages.props index d2c472d..d463c44 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -3,7 +3,6 @@ true - diff --git a/Frontmatter.Test/CombineFrontmatterTests.cs b/Frontmatter.Test/CombineFrontmatterTests.cs index 8c900c9..3624f5c 100644 --- a/Frontmatter.Test/CombineFrontmatterTests.cs +++ b/Frontmatter.Test/CombineFrontmatterTests.cs @@ -243,4 +243,47 @@ public void CombineFrontmatter_WithComplexPropertyValues_PreservesStructure() Assert.AreEqual("tag1", tags[0]); Assert.AreEqual("tag2", tags[1]); } + + /// + /// The two documents below are distinct but share an FNV-1a-32 hash (0x12657B7E), the function the + /// processed-document cache used to key on. Since both are processed under the same options, the + /// combined key collides too, and the second document is served the first one's output. + /// + [TestMethod] + public void CombineFrontmatter_WithHashCollidingDocuments_ProcessesEachIndependently() + { + // Arrange - "\n" rather than Environment.NewLine so the bytes hashed are the same on every + // platform; DetectNewLine reads the document's own convention, so both still parse. + string first = "---\ntitle: Release Notes 281277\n---\n"; + string second = "---\ntitle: Release Notes 1084130\n---\n"; + Assert.AreNotEqual(first, second, "The two documents must be distinct for this test to mean anything"); + + // Act + string firstResult = Frontmatter.CombineFrontmatter(first); + string secondResult = Frontmatter.CombineFrontmatter(second); + + // Assert + Assert.Contains("Release Notes 281277", firstResult, "The first document should keep its own title"); + Assert.Contains("Release Notes 1084130", secondResult, "The second document should keep its own title"); + Assert.DoesNotContain("Release Notes 281277", secondResult, "The second document must not be served the first document's cached result"); + } + + /// + /// Guards the cache itself: combining the same document twice must still agree. Passes both before + /// and after the collision fix by design, so a later change cannot quietly make the cache incoherent. + /// + [TestMethod] + public void CombineFrontmatter_WithTheSameDocumentTwice_ReturnsEqualResults() + { + // Arrange + string input = "---\ntitle: Cache Coherence Document\n---\nBody text.\n"; + + // Act + string firstResult = Frontmatter.CombineFrontmatter(input); + string secondResult = Frontmatter.CombineFrontmatter(input); + + // Assert + Assert.AreEqual(firstResult, secondResult); + Assert.Contains("Cache Coherence Document", firstResult, "The result should carry the document's own title"); + } } diff --git a/Frontmatter.Test/YamlSerializerTests.cs b/Frontmatter.Test/YamlSerializerTests.cs index 7efa3f2..fd3ef22 100644 --- a/Frontmatter.Test/YamlSerializerTests.cs +++ b/Frontmatter.Test/YamlSerializerTests.cs @@ -435,4 +435,53 @@ public void SerializeYamlObject_WithCachedValues_ReturnsCachedResult() Assert.Contains("Modified Title", secondResult, "Result should contain 'Modified Title'"); Assert.IsFalse(secondResult.Equals(firstResult), "Second result should not equal first result after modification"); } + + /// + /// The two inputs below are distinct but share an FNV-1a-32 hash (0x27196CB9), the function the + /// parse cache used to key on. A cache keyed on that hash alone hands the second input the first + /// input's parse, so the cache must compare the text itself. + /// + [TestMethod] + public void TryParseYamlObject_WithHashCollidingInputs_ParsesEachIndependently() + { + // Arrange - both are single-line documents, so newline conventions do not enter into it + string first = "title: Release Notes 19"; + string second = "title: Release Notes 386342"; + Assert.AreNotEqual(first, second, "The two inputs must be distinct for this test to mean anything"); + + // Act + bool firstParsed = YamlSerializer.TryParseYamlObject(first, out Dictionary? firstResult); + bool secondParsed = YamlSerializer.TryParseYamlObject(second, out Dictionary? secondResult); + + // Assert + Assert.IsTrue(firstParsed, "The first input is valid YAML and should parse"); + Assert.IsTrue(secondParsed, "The second input is valid YAML and should parse"); + Assert.IsNotNull(firstResult); + Assert.IsNotNull(secondResult); + Assert.AreEqual("Release Notes 19", firstResult["title"]); + Assert.AreEqual("Release Notes 386342", secondResult["title"], "The second input must not be served the first input's cached parse"); + } + + /// + /// Guards the cache itself: parsing the same text twice must still agree. Passes both before and + /// after the collision fix by design, so a later change cannot quietly make the cache incoherent. + /// + [TestMethod] + public void TryParseYamlObject_WithTheSameInputTwice_ReturnsEqualResults() + { + // Arrange + string input = "title: Cache Coherence\nauthor: Cache Test Author"; + + // Act + bool firstParsed = YamlSerializer.TryParseYamlObject(input, out Dictionary? firstResult); + bool secondParsed = YamlSerializer.TryParseYamlObject(input, out Dictionary? secondResult); + + // Assert + Assert.IsTrue(firstParsed); + Assert.IsTrue(secondParsed); + Assert.IsNotNull(firstResult); + Assert.IsNotNull(secondResult); + Assert.AreEqual(firstResult["title"], secondResult["title"]); + Assert.AreEqual(firstResult["author"], secondResult["author"]); + } } diff --git a/Frontmatter/Frontmatter.cs b/Frontmatter/Frontmatter.cs index ad38ff8..c147943 100644 --- a/Frontmatter/Frontmatter.cs +++ b/Frontmatter/Frontmatter.cs @@ -19,9 +19,11 @@ public static class Frontmatter private const string FrontmatterDelimiter = "---"; /// - /// Cache for processed frontmatter to avoid repeated processing of identical content + /// Cache for processed frontmatter to avoid repeated processing of identical content. + /// Keyed by the document text together with the option flags it was processed under, so a cache + /// hit means the input really is identical rather than merely hashing alike. /// - private static readonly ConcurrentDictionary ProcessedFrontmatterCache = new(); + private static readonly ConcurrentDictionary<(string Content, uint Options), string> ProcessedFrontmatterCache = new(); /// /// Combines multiple frontmatter sections in a markdown document into a single frontmatter section. @@ -65,7 +67,7 @@ public static string CombineFrontmatter(string input, FrontmatterNaming property // Generate a unique cache key based on the content and options uint optionsHash = (uint)propertyNamingMode | ((uint)orderMode << 8) | ((uint)mergeStrategy << 16); - uint cacheKey = HashUtil.CreateCacheKey(input, optionsHash); + (string Content, uint Options) cacheKey = (input, optionsHash); // Try to get from cache first if (ProcessedFrontmatterCache.TryGetValue(cacheKey, out string? cachedResult)) diff --git a/Frontmatter/Frontmatter.csproj b/Frontmatter/Frontmatter.csproj index 4873ca3..39f46c2 100644 --- a/Frontmatter/Frontmatter.csproj +++ b/Frontmatter/Frontmatter.csproj @@ -4,7 +4,6 @@ - diff --git a/Frontmatter/HashUtil.cs b/Frontmatter/HashUtil.cs deleted file mode 100644 index badbfb5..0000000 --- a/Frontmatter/HashUtil.cs +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) 2023-2026 ktsu-dev contributors - -namespace ktsu.Frontmatter; - -using HashDepot; - -/// -/// Provides utility methods for computing hash values consistently across the application. -/// -internal static class HashUtil -{ - /// - /// The FNV-1a prime value used in the hash algorithm. - /// - private const uint Fnv1aPrime = 16777619; - - /// - /// The FNV-1a offset basis (starting value) used in the hash algorithm. - /// - private const uint Fnv1aOffsetBasis = 2166136261; - - /// - /// Computes an FNV-1a hash of the specified string content. - /// - /// The string content to hash. - /// A 32-bit FNV-1a hash value. - internal static uint ComputeHash(string content) - { - // FNV-1a is chosen for its efficiency, good distribution, and low collision rate for typical content - byte[] contentBytes = System.Text.Encoding.UTF8.GetBytes(content); - return Fnv1a.Hash32(contentBytes); - } - - /// - /// Combines multiple hash values into a single hash value. - /// - /// The hash values to combine. - /// A combined 32-bit hash value. - internal static uint CombineHashes(params uint[] hashes) - { - if (hashes.Length == 0) - { - return 0; - } - - // Start with the basis value for FNV-1a - uint result = Fnv1aOffsetBasis; - - // Combine using FNV-1a algorithm multiplication and XOR - foreach (uint hash in hashes) - { - // Convert hash to bytes - byte[] bytes = BitConverter.GetBytes(hash); - - // Apply FNV-1a to each byte - foreach (byte b in bytes) - { - result = (result * Fnv1aPrime) ^ b; - } - } - - return result; - } - - /// - /// Creates a cache key by combining a base content hash with option flags. - /// - /// The main content to hash. - /// Option values to combine with the content hash. - /// A combined cache key. - internal static uint CreateCacheKey(string content, params uint[] options) - { - uint contentHash = ComputeHash(content); - return CombineHashes([contentHash, .. options]); - } -} diff --git a/Frontmatter/YamlSerializer.cs b/Frontmatter/YamlSerializer.cs index 17132c8..56919e5 100644 --- a/Frontmatter/YamlSerializer.cs +++ b/Frontmatter/YamlSerializer.cs @@ -15,9 +15,11 @@ namespace ktsu.Frontmatter; public static class YamlSerializer { /// - /// Cache for parsed YAML to avoid repeated parsing + /// Cache for parsed YAML to avoid repeated parsing. + /// Keyed by the YAML text itself, compared ordinally, so a cache hit means the input really is + /// identical rather than merely hashing alike. /// - private static readonly ConcurrentDictionary> ParsedYamlCache = new(); + private static readonly ConcurrentDictionary> ParsedYamlCache = new(StringComparer.Ordinal); /// /// Reusable deserializer instance @@ -50,11 +52,8 @@ public static bool TryParseYamlObject(string input, [NotNullWhen(true)] out Dict return false; } - // Compute a hash of the content for the cache key - uint cacheKey = HashUtil.ComputeHash(input); - // Try to get from cache first - if (ParsedYamlCache.TryGetValue(cacheKey, out Dictionary? cachedResult)) + if (ParsedYamlCache.TryGetValue(input, out Dictionary? cachedResult)) { // Create a deep copy of the cached dictionary to prevent mutations from affecting other copies result = []; @@ -97,7 +96,7 @@ public static bool TryParseYamlObject(string input, [NotNullWhen(true)] out Dict cacheResult[pair.Key] = DeepCloneValue(pair.Value); } - ParsedYamlCache.TryAdd(cacheKey, cacheResult); + ParsedYamlCache.TryAdd(input, cacheResult); return true; } }