From be71db97ad56aa9ec2d2b4c668e480815a6cc217 Mon Sep 17 00:00:00 2001
From: rust <david.maltby@hotmail.com>
Date: Sun, 22 Mar 2026 07:48:06 -0500
Subject: [PATCH] fix: override Dispose(bool) instead of hiding base Dispose,
 add Open() state check

Fixes #157: FileConnection, FileTransaction, and FileDataReader used
`protected new void Dispose()` which hid the base class Dispose pattern,
preventing polymorphic disposal via base type references (e.g. using
statements with DbConnection type).

Fixes #153: FileConnection.Open() now throws InvalidOperationException
if the connection is already open, per the ADO.NET contract.

Changes:
- FileConnection: override Dispose(bool disposing), add Open() state check,
  simplify Close() to only transition state (Dispose handles resource cleanup)
- FileTransaction: override Dispose(bool disposing) instead of new Dispose()
- FileDataReader: override Dispose(bool disposing), move cleanup to Close()
  override to avoid recursion with base DbDataReader.Dispose(bool)
- Update tests to use Close()+Open() pattern instead of bare re-Open()

Closes #157, Closes #153

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 src/Data.Common/ADO.NET/FileConnection.cs  | 16 +++++++++++-----
 src/Data.Common/ADO.NET/FileDataReader.cs  | 11 +++++++----
 src/Data.Common/ADO.NET/FileTransaction.cs | 15 +++++++++------
 tests/Data.Tests.Common/InsertTests.cs     |  2 ++
 4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/Data.Common/ADO.NET/FileConnection.cs b/src/Data.Common/ADO.NET/FileConnection.cs
index 5fd7221..07c4be1 100644
--- a/src/Data.Common/ADO.NET/FileConnection.cs
+++ b/src/Data.Common/ADO.NET/FileConnection.cs
@@ -201,7 +201,6 @@ public override void ChangeDatabase(string databaseName)
     /// </summary>
     public override void Close()
     {
-        (DataSourceProvider as IDisposable)?.Dispose();
         state = ConnectionState.Closed;
     }
 
@@ -233,6 +232,9 @@ public override void Close()
     /// </summary>
     public override void Open()
     {
+        if (State == ConnectionState.Open)
+            throw new InvalidOperationException("Connection is already open.");
+
         if (DataSourceProvider is null)
         {
             if (!CreateIfNotExist)
@@ -322,11 +324,15 @@ public override ValueTask DisposeAsync()
     /// <summary>
     /// Disposes the connection.
     /// </summary>
-    protected new void Dispose()
+    protected override void Dispose(bool disposing)
     {
-        (DataSourceProvider as IDisposable)?.Dispose();
-        base.Dispose();
-        state = ConnectionState.Closed;
+        if (disposing)
+        {
+            (DataSourceProvider as IDisposable)?.Dispose();
+            DataSourceProvider = null;
+            state = ConnectionState.Closed;
+        }
+        base.Dispose(disposing);
     }
 
     private DataTable GetTablesSchema()
diff --git a/src/Data.Common/ADO.NET/FileDataReader.cs b/src/Data.Common/ADO.NET/FileDataReader.cs
index 235d5e2..64492d5 100644
--- a/src/Data.Common/ADO.NET/FileDataReader.cs
+++ b/src/Data.Common/ADO.NET/FileDataReader.cs
@@ -85,7 +85,11 @@ public override int RecordsAffected
     /// </summary>
     public override bool HasRows => result.HasRows;
 
-    public override void Close() => Dispose();
+    public override void Close()
+    {
+        log.LogDebug($"{GetType()}.{nameof(Close)}() called.");
+        fileReader.Dispose();
+    }
 
     /// <summary>
     /// Advances to the next result, when reading the results of batch SQL statements.
@@ -714,10 +718,9 @@ public override ValueTask DisposeAsync()
     /// <summary>
     /// Disposes of the resources used by this instance.
     /// </summary>
-    protected new void Dispose()
+    protected override void Dispose(bool disposing)
     {
-        log.LogDebug($"{GetType()}.{nameof(Dispose)}() called.");
-        fileReader.Dispose();
+        base.Dispose(disposing);
     }
 
     /// <summary>
diff --git a/src/Data.Common/ADO.NET/FileTransaction.cs b/src/Data.Common/ADO.NET/FileTransaction.cs
index 068424d..a3f4532 100644
--- a/src/Data.Common/ADO.NET/FileTransaction.cs
+++ b/src/Data.Common/ADO.NET/FileTransaction.cs
@@ -115,15 +115,18 @@ public override ValueTask DisposeAsync()
 #endif
 
     /// <inheritdoc/>
-    protected new void Dispose()
+    protected override void Dispose(bool disposing)
     {
-        log.LogDebug($"{GetType()}.{nameof(Dispose)}() called.");
+        if (disposing)
+        {
+            log.LogDebug($"{GetType()}.{nameof(Dispose)}() called.");
 
-        if (!TransactionDone)
-            Rollback();
+            if (!TransactionDone)
+                Rollback();
 
-        base.Dispose();
-        Writers.Clear();
+            Writers.Clear();
+        }
+        base.Dispose(disposing);
     }
 
     /// <inheritdoc/>
diff --git a/tests/Data.Tests.Common/InsertTests.cs b/tests/Data.Tests.Common/InsertTests.cs
index 37534c2..2393b92 100644
--- a/tests/Data.Tests.Common/InsertTests.cs
+++ b/tests/Data.Tests.Common/InsertTests.cs
@@ -24,6 +24,7 @@ public static void Insert_ShouldInsertData<TFileParameter>(Func<FileConnection<T
 
             // Assert
             Assert.Equal(1, rowsAffected);
+            connection.Close();
             connection.Open();
 
             // Act - Verify the inserted record exists in the locations table
@@ -61,6 +62,7 @@ public static void Insert_ShouldInsertNullData<TFileParameter>(Func<FileConnecti
 
             // Assert
             Assert.Equal(1, rowsAffected);
+            connection.Close();
             connection.Open();
 
             // Act - Verify the inserted record exists in the locations table