Fix the StringHistory Limits and Make the Download Tests Offline

ARCHITECTURE.md

@@ -18,8 +18,9 @@ These are behavioral contracts rather than formatting rules, which is why they l
 
 - **I/O methods return `bool`** for success or failure, and hand back any additional result through an `out` parameter. They do not signal ordinary I/O failure by throwing.
 - **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<HttpClient>` and reads with `HttpCompletionOption.ResponseHeadersRead`, so a large response streams rather than buffering whole.
+- **`Download`** reuses a thread-safe `Lazy<HttpClient>`. `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 download, and a request that fails before the response is accepted leaves it untouched. The destination is opened once the response is accepted, so a failure during the body can leave a partial file.
 - **`FileEx`** wraps its I/O in retry logic configured through `Options`, and honors cancellation from both `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.
 - **`Extensions`** uses the C# `extension` block form inside a static class for its logger and string 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.

HISTORY.md

@@ -5,6 +5,8 @@ Some useful and not so useful C# .NET utility classes.
 ## Release History
 
 - v4.0:
+  - 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 write beside the destination and move the result into place, so the destination is replaced whole on success and left untouched when a download fails partway.
+  - 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 append, and re-partition the lines already stored when assigned, so a limit set after appending is honored instead of ignored. Appending 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.
   - 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<T>()` 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.
@@ -24,7 +26,7 @@ Some useful and not so useful C# .NET utility classes.
   - Added `WORKFLOW.md`, the canonical CI/CD specification, and `repo-config/`, the rulesets and repository settings as code.
   - Bundled the build output and NuGet packages into a `Utilities.7z` asset on each GitHub release.
 - v3.5:
-  - Re-synced the repository structure and agent documentation with the upstream ProjectTemplate: added this `HISTORY.md` and a `CODESTYLE.md` .NET style guide, narrowed `.github/copilot-instructions.md` to the Copilot review runbook, and refreshed `AGENTS.md` conventions.
+  - Re-synced the repository structure and agent documentation: added this `HISTORY.md` and a `CODESTYLE.md` .NET style guide, narrowed `.github/copilot-instructions.md` to the Copilot review runbook, and refreshed `AGENTS.md` conventions.
   - Corrected the versioning policy to bump `version.json` only for functional changes.
   - Swapped the recommended Todo VS Code add-on from Todo Tree to Better Todo Tree.
 - v3.4:

README.md

@@ -29,10 +29,12 @@ Some useful and not so useful C# .NET utility classes.
 - Renamed the NuGet package and namespace from `InsaneGenius.Utilities` to `ptr727.Utilities`.
 - Replaced Serilog with an injectable `Microsoft.Extensions.Logging` logging model.
 - Replaced `List<T>` in public methods with `Collection<T>` and `ReadOnlyCollection<T>`.
+- Changed `StringHistory.MaxFirstLines` and `MaxLastLines` to re-partition the lines already stored when assigned, and to reject a negative value.
 
 **Summary**:
 
 - Added `HttpClientFactory`, a reusable resilient HTTP client factory (Polly retry and circuit breaker) with an AOT safe `AssemblyInfo` identity helper and a tunable `HttpClientOptions`.
+- Fixed `Download.DownloadFile` and `DownloadFileAsync` corrupting the destination when downloading over a longer existing file. The destination is now replaced whole on success and left untouched on failure.
 
 See [Release History](./HISTORY.md) for complete release notes and older versions.
 

Utilities/Download.cs

@@ -102,7 +102,15 @@ public static bool DownloadFile(Uri uri, string fileName)
         try
         {
             using Stream httpStream = GetHttpClient().GetStreamAsync(uri).GetAwaiter().GetResult();
-            using FileStream fileStream = File.OpenWrite(fileName);
+
+            // Create truncates where OpenWrite left a longer file's tail behind the body.
+            // Rewriting in place keeps the destination's own permissions and links.
+            using FileStream fileStream = new(
+                fileName,
+                FileMode.Create,
+                FileAccess.Write,
+                FileShare.None
+            );
             httpStream.CopyTo(fileStream);
         }
         catch (Exception e) when (Log.LogAndHandle(e))
@@ -137,7 +145,14 @@ public static async Task<bool> DownloadFileAsync(
                 .ConfigureAwait(false);
             await using (httpStream.ConfigureAwait(false))
             {
-                FileStream fileStream = File.OpenWrite(fileName);
+                // Create truncates where OpenWrite left a longer file's tail behind the body.
+                // Rewriting in place keeps the destination's own permissions and links.
+                FileStream fileStream = new(
+                    fileName,
+                    FileMode.Create,
+                    FileAccess.Write,
+                    FileShare.None
+                );
                 await using (fileStream.ConfigureAwait(false))
                 {
                     await httpStream

Utilities/StringHistory.cs

@@ -5,7 +5,12 @@ namespace ptr727.Utilities;
 /// </summary>
 /// <remarks>
 /// This class is useful for maintaining a bounded history buffer, keeping the first N and last M lines
-/// while discarding intermediate content when limits are exceeded.
+/// while discarding intermediate content when limits are exceeded. Zero on a single side retains no
+/// lines on that side, and zero on both is the one unrestricted mode, where every appended line is
+/// retained. A limit assigned after lines have been appended re-partitions what is already
+/// stored, so the history never holds more than the limits then in force allow. Re-partitioning
+/// only ever discards: once a line has been dropped the head is closed, and a later, larger
+/// <see cref="MaxFirstLines"/> raises the ceiling without recovering or repopulating it.
 /// </remarks>
 public class StringHistory
 {
@@ -17,13 +22,22 @@ public class StringHistory
     /// <summary>
     /// Initializes a new instance of the <see cref="StringHistory"/> class with specified limits.
     /// </summary>
-    /// <param name="maxFirstLines">Maximum number of first lines to retain.</param>
-    /// <param name="maxLastLines">Maximum number of last lines to retain.</param>
+    /// <param name="maxFirstLines">Maximum number of first lines to retain, or 0 to retain none.</param>
+    /// <param name="maxLastLines">Maximum number of last lines to retain, or 0 to retain none.</param>
+    /// <exception cref="ArgumentOutOfRangeException">
+    /// Thrown when <paramref name="maxFirstLines"/> or <paramref name="maxLastLines"/> is negative.
+    /// </exception>
     public StringHistory(int maxFirstLines, int maxLastLines)
         : this()
     {
-        MaxFirstLines = maxFirstLines;
-        MaxLastLines = 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 because the two limits apply together.
+        // The setters would apply them one at a time and re-partition an empty history twice.
+        _maxFirstLines = maxFirstLines;
+        _maxLastLines = maxLastLines;
     }
 
     /// <summary>
@@ -36,14 +50,27 @@ public void AppendLine(string value)
         ArgumentNullException.ThrowIfNull(value);
 
         // No restrictions
+        // The line extends the tail once anything has been discarded, and the head otherwise.
+        // The counters then keep describing the stored list for a later limit change to read.
         if (MaxFirstLines == 0 && MaxLastLines == 0)
         {
             _stringList.Add(value);
+            if (_discarded)
+            {
+                _lastLines++;
+            }
+            else
+            {
+                _firstLines++;
+            }
+
             return;
         }
 
         // Restrict first lines
-        if (_firstLines < MaxFirstLines)
+        // The head only grows while nothing has been discarded.
+        // A line appended after a discard is not a first line, whatever the limit now allows.
+        if (!_discarded && _lastLines == 0 && _firstLines < MaxFirstLines)
         {
             _stringList.Add(value);
             _firstLines++;
@@ -53,6 +80,7 @@ public void AppendLine(string value)
         // If MaxLastLines is 0, don't add any more lines after MaxFirstLines
         if (MaxLastLines == 0)
         {
+            _discarded = true;
             return;
         }
 
@@ -65,8 +93,10 @@ public void AppendLine(string value)
         }
 
         // Roll the last lines
-        _stringList.RemoveAt(MaxFirstLines);
+        // The head is what is stored rather than the limit, which a widened one would index past.
+        _stringList.RemoveAt(_firstLines);
         _stringList.Add(value);
+        _discarded = true;
     }
 
     /// <summary>
@@ -79,22 +109,100 @@ public override string ToString() =>
 
     /// <summary>
     /// Gets or sets the maximum number of first lines to retain.
-    /// Set to 0 for no limit.
+    /// Set to 0 to retain no first lines. Every line is retained only when both limits are 0.
     /// </summary>
-    public int MaxFirstLines { get; set; }
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    /// <exception cref="ArgumentOutOfRangeException">Thrown when the assigned value is negative.</exception>
+    public int MaxFirstLines
+    {
+        get => _maxFirstLines;
+        set
+        {
+            ArgumentOutOfRangeException.ThrowIfNegative(value);
+            _maxFirstLines = value;
+            Repartition();
+        }
+    }
 
     /// <summary>
     /// Gets or sets the maximum number of last lines to retain.
-    /// Set to 0 for no limit.
+    /// Set to 0 to retain no last lines. Every line is retained only when both limits are 0.
     /// </summary>
-    public int MaxLastLines { get; set; }
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    /// <exception cref="ArgumentOutOfRangeException">Thrown when the assigned value is negative.</exception>
+    public int MaxLastLines
+    {
+        get => _maxLastLines;
+        set
+        {
+            ArgumentOutOfRangeException.ThrowIfNegative(value);
+            _maxLastLines = value;
+            Repartition();
+        }
+    }
 
     /// <summary>
     /// Gets the read-only collection of stored strings.
     /// </summary>
     public ReadOnlyCollection<string> StringList { get; }
 
+    /// <summary>
+    /// Brings the stored lines within the current limits and resets the counters to match, so an
+    /// append after a limit change continues from what is actually stored.
+    /// </summary>
+    private void Repartition()
+    {
+        // Zero on both limits is the unrestricted mode, which retains every line.
+        // Nothing is discarded here, so the counters already describe the stored list.
+        if (MaxFirstLines == 0 && MaxLastLines == 0)
+        {
+            return;
+        }
+
+        int storedCount = _stringList.Count;
+
+        // The head is only ever the stream's own first lines.
+        // Once a line has been discarded the head is trimmed but never refilled.
+        // Before any discard the stored lines are the whole stream, so both sides cut from them.
+        int firstLines = Math.Min(MaxFirstLines, _discarded ? _firstLines : storedCount);
+        int lastLines;
+
+        if (_discarded)
+        {
+            lastLines = Math.Min(MaxLastLines, _lastLines);
+            _stringList.RemoveRange(firstLines, _firstLines - firstLines);
+            _stringList.RemoveRange(firstLines, _lastLines - lastLines);
+        }
+        else
+        {
+            lastLines = Math.Min(MaxLastLines, storedCount - firstLines);
+            _stringList.RemoveRange(firstLines, storedCount - firstLines - lastLines);
+        }
+
+        _firstLines = firstLines;
+        _lastLines = lastLines;
+        if (_stringList.Count < storedCount)
+        {
+            _discarded = true;
+        }
+    }
+
     private readonly List<string> _stringList = [];
+    private int _maxFirstLines;
+    private int _maxLastLines;
     private int _firstLines;
     private int _lastLines;
+    private bool _discarded;
 }