From c15a0cb3b79deb4dc94f789ebaea9fc5ba32cee0 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 1 Sep 2026 08:39:53 -0700 Subject: [PATCH 1/2] Apply Both StringHistory Limits in One Re-Partition Answers two findings from the review of the develop to main promotion, #456. Neither could be pushed to that pull request, whose head is develop, so both land here first. SetLimits validates both values, assigns both fields, and re-partitions once. Assigning MaxFirstLines and MaxLastLines one after the other re-partitions twice, so the first assignment measures against the other limit's previous value and discards lines the final pair would have retained. No ordering of the two assignments avoids it: on an unrestricted history of ten lines, MaxFirstLines = 2 then MaxLastLines = 3 retains two lines and the reverse order retains three, where SetLimits(2, 3) retains five. The two-argument constructor was the only way to apply a pair atomically, and it does not reach an instance that already holds lines. Five tests cover the atomic case, the divergence from separate assignment, both negative arguments, and clearing to the unrestricted mode. The property remarks now point at it rather than only warning about the hazard. The 300000-history fuzz now drives SetLimits alongside the two setters and holds the prefix-plus-suffix contract, the count bound, no duplicates, and that an assignment never adds a line, across four and a half million states. "AOT safe" becomes "AOT-safe" where it modifies a noun, which the review raised against HISTORY.md. The same compound sits in HttpClientFactory and AssemblyInfo doc comments, so those are corrected too rather than left disagreeing with the release note. Editing HttpClientFactory brought its one existing comment under the one-sentence- per-line rule, restructured onto the sub-topic form at the same length. --- ARCHITECTURE.md | 2 +- HISTORY.md | 3 +- README.md | 1 + Utilities/AssemblyInfo.cs | 6 +- Utilities/HttpClientFactory.cs | 12 ++-- Utilities/StringHistory.cs | 35 +++++++++-- UtilitiesTests/StringHistoryTests.cs | 90 ++++++++++++++++++++++++++++ 7 files changed, 134 insertions(+), 15 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3268049..0074209 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -21,6 +21,6 @@ These are behavioral contracts rather than formatting rules, which is why they l - **`Download`** reuses a thread-safe `Lazy`. `GetContentInfo()` reads with `HttpCompletionOption.ResponseHeadersRead`, so asking for a size never fetches the body, while `DownloadString()` buffers the whole response and a large body belongs in `DownloadFile()` instead. A download to a file truncates and rewrites the destination in place, so its permissions, ownership, and any links to it survive. The destination is truncated once the response headers are accepted rather than once the body has arrived, so a request that fails before that leaves it untouched, while one that fails partway through the body leaves a short file. - **`FileEx`** wraps its I/O in retry logic configured through the static `FileEx.Options`, a `FileExOptions`, and honors cancellation from both `FileEx.Options.Cancel` and the method's own token parameter. - **`StringCompression`** uses Deflate, takes a configurable compression level, and passes `leaveOpen` so the caller keeps ownership of the stream it supplied. -- **`StringHistory`** retains at most `MaxFirstLines` from the head and `MaxLastLines` from the tail. Zero on a single side retains no lines on that side, and zero on both is the one unrestricted mode. Either limit rejects a negative value, and assigning one re-partitions the lines already stored, so the history never holds more than the limits then in force allow. Re-partitioning only discards: once a line has been dropped the head is closed, so a later, larger `MaxFirstLines` never promotes a retained tail line into it. +- **`StringHistory`** retains at most `MaxFirstLines` from the head and `MaxLastLines` from the tail. Zero on a single side retains no lines on that side, and zero on both is the one unrestricted mode. Either limit rejects a negative value, and assigning one re-partitions the lines already stored, so the history never holds more than the limits then in force allow. Re-partitioning only discards: once a line has been dropped the head is closed, so a later, larger `MaxFirstLines` never promotes a retained tail line into it. `SetLimits()` applies both limits in one re-partition, which assigning the two properties in turn cannot do. - **`CompressExtensions`** uses the C# `extension` block form inside a static class for its string helpers, and the internal `LogExtensions` does the same for the logger helpers. - **Logging is a seam, never a dependency.** The library depends on `Microsoft.Extensions.Logging.Abstractions` and takes an `ILoggerFactory` through `LogOptions`. It references no logging framework or sink, so a consumer chooses its own. `Serilog` appears only in `Sandbox` and the tests, where an application legitimately picks one. diff --git a/HISTORY.md b/HISTORY.md index e3443f3..319bcd5 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -6,10 +6,11 @@ Some useful and not so useful C# .NET utility classes. - v4.1: - Fixed `Download.DownloadFile()` and `DownloadFileAsync()` corrupting the destination file: both opened it with `File.OpenWrite()`, which does not truncate, so a download over a longer existing file left that file's trailing bytes after the downloaded content and reported success. Both now truncate the destination explicitly and rewrite it in place, which keeps its permissions, ownership, and any links to it. The truncation happens once the response headers are accepted rather than once the body has arrived, so a download that fails partway now leaves a short file where it previously left the original bytes behind the new ones. + - Added `StringHistory.SetLimits()`, which applies both limits in one re-partition. Assigning `MaxFirstLines` and `MaxLastLines` one after the other re-partitions twice, so the first assignment measures against the other limit's previous value and can discard lines the final pair would have retained, which no ordering of the two assignments avoids. - Tightened the `StringHistory` limit contract: `MaxFirstLines` and `MaxLastLines` now document zero as retaining no lines on that side rather than as no limit (both at zero remains the unrestricted mode), reject a negative value with `ArgumentOutOfRangeException` at the constructor and at the property rather than at a later `AppendLine()`, and re-partition the lines already stored when assigned, so a limit set after appending is honored instead of ignored. `AppendLine()` changed with them: what is retained is now always a prefix of the appended lines followed by a suffix of them, so once a line has been discarded the head is trimmed but never refilled, and a later, larger `MaxFirstLines` raises the ceiling without adopting retained tail lines as first lines. - v4.0: - Added `HttpClientFactory`, a reusable resilient HTTP client factory built on `Microsoft.Extensions.Http.Resilience` (Polly) with retry, circuit breaker, and connection pooling, tunable through the new `HttpClientOptions`. It exposes a shared singleton client, caller-owned clients, and the resilience handler for callers that build their own client with a custom base address or headers. - - Added `AssemblyInfo`, an AOT safe assembly and application identity helper whose `For()` substitutes for `Assembly.GetExecutingAssembly()` (unreliable under Native AOT), and which supplies the consuming application name, version, and a default User-Agent. + - Added `AssemblyInfo`, an AOT-safe assembly and application identity helper whose `For()` substitutes for `Assembly.GetExecutingAssembly()` (unreliable under Native AOT), and which supplies the consuming application name, version, and a default User-Agent. - Reworked `Download` to build its `HttpClient` through `HttpClientFactory`, so downloads now flow through the shared retry and circuit-breaker pipeline. The `TimeoutSeconds` property and all method signatures are unchanged. - Changed public members that exposed `List` to safe collection types (a breaking API change): `FileEx.EnumerateDirectories()` / `EnumerateDirectory()` now return `Collection` out parameters and accept `IEnumerable`, and `StringHistory.StringList` is now a `ReadOnlyCollection`. - Gated the library's reference AOT verification behind an explicit `PublishAot` opt-in, and turned the `Sandbox` project into a Native AOT smoke test (published and run as AOT) that proves the resilience pipeline and assembly-identity resolution work under Native AOT. diff --git a/README.md b/README.md index 68c77fb..f05a735 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ Some useful and not so useful C# .NET utility classes. **Version: 4.1**: - Fixed `Download.DownloadFile()` and `DownloadFileAsync()` corrupting the destination when downloading over a longer existing file. The destination is now truncated and rewritten in place, keeping its permissions and any links to it. A download that fails partway leaves a short file rather than a mix of the new content and the old. +- Added `StringHistory.SetLimits()` to apply both limits in one re-partition, which assigning the two properties in turn cannot do. - Fixed the `StringHistory` limit properties: `MaxFirstLines` and `MaxLastLines` now honor a limit assigned after lines have been appended, document zero on one side as retaining no lines on that side rather than as no limit, with both at zero remaining the one unrestricted mode, and reject a negative value with `ArgumentOutOfRangeException`. See [Release History](./HISTORY.md) for complete release notes and older versions. diff --git a/Utilities/AssemblyInfo.cs b/Utilities/AssemblyInfo.cs index 2f917cd..4b2b3c3 100644 --- a/Utilities/AssemblyInfo.cs +++ b/Utilities/AssemblyInfo.cs @@ -9,7 +9,7 @@ namespace ptr727.Utilities; /// /// Native AOT does not support reliably /// (see https://github.com/dotnet/runtime/issues/94200). Use as the -/// AOT safe substitute, passing a marker type from the assembly of interest. The +/// AOT-safe substitute, passing a marker type from the assembly of interest. The /// application identity members make no assumptions about the hosting environment and /// fall back gracefully when the managed entry assembly is unavailable. /// @@ -21,7 +21,7 @@ public static class AssemblyInfo /// A marker type from the assembly of interest. /// The assembly that defines . /// - /// This is the AOT safe replacement for : + /// This is the AOT-safe replacement for : /// typeof(T).Assembly resolves at compile time with no stack walk or reflection. /// public static Assembly For() => typeof(T).Assembly; @@ -49,7 +49,7 @@ public static string AppName /// Gets the version of the consuming application (the entry assembly), or a fallback value. /// /// - /// Uses the AOT safe rather than reflecting over + /// Uses the AOT-safe rather than reflecting over /// informational-version attributes, which the trimmer or AOT compiler may strip. /// public static string AppVersion => diff --git a/Utilities/HttpClientFactory.cs b/Utilities/HttpClientFactory.cs index 1b3d88a..175e548 100644 --- a/Utilities/HttpClientFactory.cs +++ b/Utilities/HttpClientFactory.cs @@ -7,7 +7,7 @@ namespace ptr727.Utilities; /// /// Creates resilient instances with retry and circuit-breaker -/// policies, connection pooling, and an AOT safe default User-Agent. +/// policies, connection pooling, and an AOT-safe default User-Agent. /// /// /// Use for a shared singleton, @@ -166,11 +166,11 @@ private static bool IsTransientFailure(Outcome outcome) && IsTransientStatusCode((int)outcome.Result.StatusCode); } - // Retry known-transient failures: a request timeout (a cancellation with an inner - // TimeoutException), a network or IO error (an HttpRequestException with no status, or an - // IOException), or an HttpRequestException whose own status code is transient (408, 429, - // >= 500). Caller cancellation, an open circuit, a 4xx status, and any other exception - // (including programming errors) are not retried. + // Retry only known-transient failures. + // - A request timeout, which is a cancellation with an inner TimeoutException. + // - A network or IO error, an HttpRequestException with no status or an IOException. + // - An HttpRequestException whose own status code is transient, 408, 429 or >= 500. + // Caller cancellation, an open circuit, a 4xx, and anything else are not retried. return outcome.Exception switch { OperationCanceledException canceled => canceled.InnerException is TimeoutException, diff --git a/Utilities/StringHistory.cs b/Utilities/StringHistory.cs index 3ca05c5..93c39ef 100644 --- a/Utilities/StringHistory.cs +++ b/Utilities/StringHistory.cs @@ -107,6 +107,33 @@ public override string ToString() => string.Join(Environment.NewLine, _stringList) + (_stringList.Count > 0 ? Environment.NewLine : string.Empty); + /// + /// Sets both limits together, re-partitioning the stored lines once against the pair. + /// + /// Maximum number of first lines to retain, or 0 to retain none. + /// Maximum number of last lines to retain, or 0 to retain none. + /// + /// Thrown when or is negative. + /// + /// + /// Assigning and one after the other + /// re-partitions twice, so the first assignment measures against the other limit's previous + /// value and can discard lines the final pair would have retained. This applies both before + /// re-partitioning, which is the only way to avoid that on an instance that already holds lines. + /// + public void SetLimits(int maxFirstLines, int maxLastLines) + { + // Validated here so the exception names the caller's own parameter rather than "value". + ArgumentOutOfRangeException.ThrowIfNegative(maxFirstLines); + ArgumentOutOfRangeException.ThrowIfNegative(maxLastLines); + + // Assigned to the fields rather than through the setters. + // Those re-partition once each, which is the order dependence this method removes. + _maxFirstLines = maxFirstLines; + _maxLastLines = maxLastLines; + Repartition(); + } + /// /// Gets or sets the maximum number of first lines to retain. /// Set to 0 to retain no first lines. Every line is retained only when both limits are 0. @@ -115,8 +142,8 @@ public override string ToString() => /// Assigning this re-partitions the lines already stored against the limits then in force, /// which discards whatever the new limits exclude and never recovers a line already dropped. /// Setting both limits therefore applies them one at a time, and the first assignment can - /// discard lines the second would have retained. Prefer the two-argument constructor when both - /// limits are known up front. + /// discard lines the second would have retained. Use to apply both at + /// once, or the two-argument constructor when both limits are known up front. /// /// Thrown when the assigned value is negative. public int MaxFirstLines @@ -138,8 +165,8 @@ public int MaxFirstLines /// Assigning this re-partitions the lines already stored against the limits then in force, /// which discards whatever the new limits exclude and never recovers a line already dropped. /// Setting both limits therefore applies them one at a time, and the first assignment can - /// discard lines the second would have retained. Prefer the two-argument constructor when both - /// limits are known up front. + /// discard lines the second would have retained. Use to apply both at + /// once, or the two-argument constructor when both limits are known up front. /// /// Thrown when the assigned value is negative. public int MaxLastLines diff --git a/UtilitiesTests/StringHistoryTests.cs b/UtilitiesTests/StringHistoryTests.cs index 6893f9f..e2fa496 100644 --- a/UtilitiesTests/StringHistoryTests.cs +++ b/UtilitiesTests/StringHistoryTests.cs @@ -489,4 +489,94 @@ public void ClearingMaxFirstLines_ThenLimitingItAgain_ShouldNotRebuildTheHead() _ = history.StringList.Should().BeEmpty(); } + + [Fact] + public void SetLimits_OnAnUnrestrictedHistory_ShouldApplyBothSidesAtOnce() + { + StringHistory history = new(); + for (int i = 0; i < 10; i++) + { + history.AppendLine($"Line {i}"); + } + + history.SetLimits(maxFirstLines: 2, maxLastLines: 3); + + // One re-partition against the pair keeps a head and a tail. + // Two assignments would measure the first against the other limit's previous value. + _ = history.StringList.Count.Should().Be(5); + _ = history.StringList[0].Should().Be("Line 0"); + _ = history.StringList[1].Should().Be("Line 1"); + _ = history.StringList[2].Should().Be("Line 7"); + _ = history.StringList[3].Should().Be("Line 8"); + _ = history.StringList[4].Should().Be("Line 9"); + } + + [Fact] + public void SetLimits_ShouldRetainWhatSeparateAssignmentDiscards() + { + StringHistory separate = new(); + StringHistory atomic = new(); + for (int i = 0; i < 10; i++) + { + separate.AppendLine($"Line {i}"); + atomic.AppendLine($"Line {i}"); + } + + // Assigning MaxFirstLines while MaxLastLines is still 0 retains no tail. + // The tail is gone by the time the second assignment raises the limit. + separate.MaxFirstLines = 2; + separate.MaxLastLines = 3; + atomic.SetLimits(maxFirstLines: 2, maxLastLines: 3); + + _ = separate.StringList.Count.Should().Be(2); + _ = atomic.StringList.Count.Should().Be(5); + } + + [Fact] + public void SetLimits_WithNegativeFirstLines_ShouldThrowArgumentOutOfRangeException() + { + StringHistory history = new(maxFirstLines: 3, maxLastLines: 3); + + _ = FluentActions + .Invoking(() => history.SetLimits(maxFirstLines: -1, maxLastLines: 2)) + .Should() + .Throw() + .WithParameterName("maxFirstLines"); + + // Neither limit moves when either value is rejected. + _ = history.MaxFirstLines.Should().Be(3); + _ = history.MaxLastLines.Should().Be(3); + } + + [Fact] + public void SetLimits_WithNegativeLastLines_ShouldThrowArgumentOutOfRangeException() + { + StringHistory history = new(maxFirstLines: 3, maxLastLines: 3); + + _ = FluentActions + .Invoking(() => history.SetLimits(maxFirstLines: 2, maxLastLines: -1)) + .Should() + .Throw() + .WithParameterName("maxLastLines"); + + _ = history.MaxFirstLines.Should().Be(3); + _ = history.MaxLastLines.Should().Be(3); + } + + [Fact] + public void SetLimits_ToZeroOnBothSides_ShouldRestoreTheUnrestrictedMode() + { + StringHistory history = new(maxFirstLines: 2, maxLastLines: 2); + for (int i = 0; i < 10; i++) + { + history.AppendLine($"Line {i}"); + } + + history.SetLimits(maxFirstLines: 0, maxLastLines: 0); + history.AppendLine("Line 10"); + + // What survived the earlier limits is kept, and every later line is added. + _ = history.StringList.Count.Should().Be(5); + _ = history.StringList[4].Should().Be("Line 10"); + } } From 2305a7db902746f0b6846b9d0fcff845dfd535c9 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 1 Sep 2026 08:52:58 -0700 Subject: [PATCH 2/2] Correct What the SetLimits Documentation Claims A local pass found the previous commit over-claiming, in the release note and in the method's own remarks. "No ordering of the two assignments avoids it" is false. For a given target pair, one order is often lossless: on a default history of ten lines targeting (10, 3), MaxFirstLines then MaxLastLines keeps all ten, and only the reverse order drops seven. An exhaustive sweep of the reachable states found 264 where exactly one order is lossless. The defensible claim, and now the stated one, is that which order works depends on the values and on what is stored, so no fixed ordering is safe. The method's two param tags said zero retains none, which is true of one side and false of both: SetLimits(0, 0) is the unrestricted mode and retains every stored line. A caller reading only the method doc and calling it to clear a bounded history would have got the opposite of what it says, and the test added with it pins the retaining behavior, so the two disagreed. Both tags now say "on that side" and the remarks state the both-zero mode. A sixth test covers Repartition's discarded branch reached through SetLimits, which the five earlier ones all missed: four ran with nothing discarded and the fifth hit the both-zero early return. That branch caps each side against its own counter rather than against the whole list, which is where SetLimits differs most visibly from the setters. Three corrections in files this change already touched. ARCHITECTURE.md said StringCompression "passes leaveOpen so the caller keeps ownership of the stream it supplied", but every public member takes and returns a string and the only stream is one the method creates and disposes itself. AssemblyInfo's type summary still read "AOT and trim safe", the last unhyphenated instance in the repository and the copy that ships in the XML docs. The restructured HttpClientFactory comment said a 4xx is not retried, three lines under a bullet that retries 408 and 429. --- ARCHITECTURE.md | 2 +- HISTORY.md | 2 +- Utilities/AssemblyInfo.cs | 2 +- Utilities/HttpClientFactory.cs | 2 +- Utilities/StringHistory.cs | 15 +++++++++------ UtilitiesTests/StringHistoryTests.cs | 26 ++++++++++++++++++++++++++ 6 files changed, 39 insertions(+), 10 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 0074209..5657554 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -20,7 +20,7 @@ These are behavioral contracts rather than formatting rules, which is why they l - **Async methods carry the `Async` suffix** and take an optional `CancellationToken cancellationToken = default`, passed through to the underlying call rather than ignored. An async overload returning several values returns a tuple, since `out` parameters are unavailable there. - **`Download`** reuses a thread-safe `Lazy`. `GetContentInfo()` reads with `HttpCompletionOption.ResponseHeadersRead`, so asking for a size never fetches the body, while `DownloadString()` buffers the whole response and a large body belongs in `DownloadFile()` instead. A download to a file truncates and rewrites the destination in place, so its permissions, ownership, and any links to it survive. The destination is truncated once the response headers are accepted rather than once the body has arrived, so a request that fails before that leaves it untouched, while one that fails partway through the body leaves a short file. - **`FileEx`** wraps its I/O in retry logic configured through the static `FileEx.Options`, a `FileExOptions`, and honors cancellation from both `FileEx.Options.Cancel` and the method's own token parameter. -- **`StringCompression`** uses Deflate, takes a configurable compression level, and passes `leaveOpen` so the caller keeps ownership of the stream it supplied. +- **`StringCompression`** uses Deflate and takes a configurable compression level. It works in strings rather than streams, so a caller hands it no stream to own. - **`StringHistory`** retains at most `MaxFirstLines` from the head and `MaxLastLines` from the tail. Zero on a single side retains no lines on that side, and zero on both is the one unrestricted mode. Either limit rejects a negative value, and assigning one re-partitions the lines already stored, so the history never holds more than the limits then in force allow. Re-partitioning only discards: once a line has been dropped the head is closed, so a later, larger `MaxFirstLines` never promotes a retained tail line into it. `SetLimits()` applies both limits in one re-partition, which assigning the two properties in turn cannot do. - **`CompressExtensions`** uses the C# `extension` block form inside a static class for its string helpers, and the internal `LogExtensions` does the same for the logger helpers. - **Logging is a seam, never a dependency.** The library depends on `Microsoft.Extensions.Logging.Abstractions` and takes an `ILoggerFactory` through `LogOptions`. It references no logging framework or sink, so a consumer chooses its own. `Serilog` appears only in `Sandbox` and the tests, where an application legitimately picks one. diff --git a/HISTORY.md b/HISTORY.md index 319bcd5..525c22e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -6,7 +6,7 @@ Some useful and not so useful C# .NET utility classes. - v4.1: - Fixed `Download.DownloadFile()` and `DownloadFileAsync()` corrupting the destination file: both opened it with `File.OpenWrite()`, which does not truncate, so a download over a longer existing file left that file's trailing bytes after the downloaded content and reported success. Both now truncate the destination explicitly and rewrite it in place, which keeps its permissions, ownership, and any links to it. The truncation happens once the response headers are accepted rather than once the body has arrived, so a download that fails partway now leaves a short file where it previously left the original bytes behind the new ones. - - Added `StringHistory.SetLimits()`, which applies both limits in one re-partition. Assigning `MaxFirstLines` and `MaxLastLines` one after the other re-partitions twice, so the first assignment measures against the other limit's previous value and can discard lines the final pair would have retained, which no ordering of the two assignments avoids. + - Added `StringHistory.SetLimits()`, which applies both limits in one re-partition. Assigning `MaxFirstLines` and `MaxLastLines` one after the other re-partitions twice, so the first assignment measures against the other limit's previous value and can discard lines the final pair would have retained. Which order avoids that, where either does, depends on the values and on what is stored, so no fixed ordering is safe. - Tightened the `StringHistory` limit contract: `MaxFirstLines` and `MaxLastLines` now document zero as retaining no lines on that side rather than as no limit (both at zero remains the unrestricted mode), reject a negative value with `ArgumentOutOfRangeException` at the constructor and at the property rather than at a later `AppendLine()`, and re-partition the lines already stored when assigned, so a limit set after appending is honored instead of ignored. `AppendLine()` changed with them: what is retained is now always a prefix of the appended lines followed by a suffix of them, so once a line has been discarded the head is trimmed but never refilled, and a later, larger `MaxFirstLines` raises the ceiling without adopting retained tail lines as first lines. - v4.0: - Added `HttpClientFactory`, a reusable resilient HTTP client factory built on `Microsoft.Extensions.Http.Resilience` (Polly) with retry, circuit breaker, and connection pooling, tunable through the new `HttpClientOptions`. It exposes a shared singleton client, caller-owned clients, and the resilience handler for callers that build their own client with a custom base address or headers. diff --git a/Utilities/AssemblyInfo.cs b/Utilities/AssemblyInfo.cs index 4b2b3c3..13bbae1 100644 --- a/Utilities/AssemblyInfo.cs +++ b/Utilities/AssemblyInfo.cs @@ -4,7 +4,7 @@ namespace ptr727.Utilities; /// -/// Provides AOT and trim safe access to assembly and consuming-application identity. +/// Provides AOT-safe and trim-safe access to assembly and consuming-application identity. /// /// /// Native AOT does not support reliably diff --git a/Utilities/HttpClientFactory.cs b/Utilities/HttpClientFactory.cs index 175e548..5ea2b34 100644 --- a/Utilities/HttpClientFactory.cs +++ b/Utilities/HttpClientFactory.cs @@ -170,7 +170,7 @@ private static bool IsTransientFailure(Outcome outcome) // - A request timeout, which is a cancellation with an inner TimeoutException. // - A network or IO error, an HttpRequestException with no status or an IOException. // - An HttpRequestException whose own status code is transient, 408, 429 or >= 500. - // Caller cancellation, an open circuit, a 4xx, and anything else are not retried. + // Caller cancellation, an open circuit, and anything not listed above are not retried. return outcome.Exception switch { OperationCanceledException canceled => canceled.InnerException is TimeoutException, diff --git a/Utilities/StringHistory.cs b/Utilities/StringHistory.cs index 93c39ef..d654589 100644 --- a/Utilities/StringHistory.cs +++ b/Utilities/StringHistory.cs @@ -22,8 +22,8 @@ public class StringHistory /// /// Initializes a new instance of the class with specified limits. /// - /// Maximum number of first lines to retain, or 0 to retain none. - /// Maximum number of last lines to retain, or 0 to retain none. + /// Maximum number of first lines to retain, or 0 to retain none on that side. + /// Maximum number of last lines to retain, or 0 to retain none on that side. /// /// Thrown when or is negative. /// @@ -110,16 +110,19 @@ public override string ToString() => /// /// Sets both limits together, re-partitioning the stored lines once against the pair. /// - /// Maximum number of first lines to retain, or 0 to retain none. - /// Maximum number of last lines to retain, or 0 to retain none. + /// Maximum number of first lines to retain, or 0 to retain none on that side. + /// Maximum number of last lines to retain, or 0 to retain none on that side. /// /// Thrown when or is negative. /// /// /// Assigning and one after the other /// re-partitions twice, so the first assignment measures against the other limit's previous - /// value and can discard lines the final pair would have retained. This applies both before - /// re-partitioning, which is the only way to avoid that on an instance that already holds lines. + /// value and can discard lines the final pair would have retained. Which order avoids that, + /// where either does, depends on the values and on what is stored, so no fixed ordering is safe + /// and this applies both before re-partitioning at all. + /// Both limits at zero is the one unrestricted mode, so passing zero twice retains every stored + /// line and every later one rather than discarding them. /// public void SetLimits(int maxFirstLines, int maxLastLines) { diff --git a/UtilitiesTests/StringHistoryTests.cs b/UtilitiesTests/StringHistoryTests.cs index e2fa496..11a465e 100644 --- a/UtilitiesTests/StringHistoryTests.cs +++ b/UtilitiesTests/StringHistoryTests.cs @@ -579,4 +579,30 @@ public void SetLimits_ToZeroOnBothSides_ShouldRestoreTheUnrestrictedMode() _ = history.StringList.Count.Should().Be(5); _ = history.StringList[4].Should().Be("Line 10"); } + + [Fact] + public void SetLimits_OnADiscardedHistory_ShouldTrimEachSideAgainstItsOwnCount() + { + StringHistory history = new(maxFirstLines: 2, maxLastLines: 0); + for (int i = 0; i < 5; i++) + { + history.AppendLine($"Line {i}"); + } + + // Lines 2 through 4 are gone, so the head is closed at two. + history.SetLimits(maxFirstLines: 0, maxLastLines: 0); + history.AppendLine("Line 5"); + history.AppendLine("Line 6"); + history.AppendLine("Line 7"); + + history.SetLimits(maxFirstLines: 5, maxLastLines: 2); + + // Each side is capped against what it holds rather than against the whole list. + // The head stays at two and the tail drops its oldest line. + _ = history.StringList.Count.Should().Be(4); + _ = history.StringList[0].Should().Be("Line 0"); + _ = history.StringList[1].Should().Be("Line 1"); + _ = history.StringList[2].Should().Be("Line 6"); + _ = history.StringList[3].Should().Be("Line 7"); + } }