From fa145f87f350858911e6e3039e55d89d7ac289f4 Mon Sep 17 00:00:00 2001 From: Qwertyluk Date: Sun, 4 Feb 2024 13:59:03 +0100 Subject: [PATCH 1/3] Add leaveOpen constructor parameter to the CsvDataReader class --- src/CsvHelper/CsvDataReader.cs | 34 ++++++++++++++++--- .../CsvDataReaderDisposalTests.cs | 33 ++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs diff --git a/src/CsvHelper/CsvDataReader.cs b/src/CsvHelper/CsvDataReader.cs index 752cbdbb8..779da7159 100644 --- a/src/CsvHelper/CsvDataReader.cs +++ b/src/CsvHelper/CsvDataReader.cs @@ -14,9 +14,11 @@ namespace CsvHelper /// public class CsvDataReader : IDataReader { + private readonly bool leaveOpen; private readonly CsvReader csv; private readonly DataTable schemaTable; private bool skipNextRead; + private bool disposed; /// public object this[int i] @@ -71,9 +73,11 @@ public int FieldCount /// /// The CSV. /// The DataTable representing the file schema. - public CsvDataReader(CsvReader csv, DataTable schemaTable = null) + /// true to leave the open after the object is disposed, otherwise false. + public CsvDataReader(CsvReader csv, DataTable schemaTable = null, bool leaveOpen = false) { this.csv = csv; + this.leaveOpen = leaveOpen; csv.Read(); @@ -95,11 +99,33 @@ public void Close() Dispose(); } - /// + /// public void Dispose() { - csv.Dispose(); - IsClosed = true; + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + + /// + /// Disposes the object. + /// + /// Indicates if the object is being disposed. + protected virtual void Dispose(bool disposing) + { + if (disposed) + { + return; + } + + if (disposing) + { + if (!leaveOpen) + { + csv?.Dispose(); + } + } + + disposed = true; } /// diff --git a/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs b/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs new file mode 100644 index 000000000..e2cde5023 --- /dev/null +++ b/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs @@ -0,0 +1,33 @@ +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; +using Xunit; + +namespace CsvHelper.Tests +{ + public class CsvDataReaderDisposalTests + { + [Fact] + public void ShouldNotDisposeCsvReaderWhenLeaveOpenParameterIsTrue() + { + var s = new StringBuilder(); + s.AppendLine("StringColumn"); + s.AppendLine("one"); + using (var reader = new StringReader(s.ToString())) + using (var csv = new CsvReader(reader, CultureInfo.InvariantCulture)) + { + var dataReader = new CsvDataReader(csv, leaveOpen: true); + dataReader.Dispose(); + + var record = csv.GetRecord(); + Assert.NotNull(record); + } + } + + private class TestRecord() + { + public string StringColumn { get; set; } + } + } +} From 28cadfaf435e5b0d6aa655da7c6f35b3dcc7bcc9 Mon Sep 17 00:00:00 2001 From: Qwertyluk Date: Sun, 4 Feb 2024 14:26:34 +0100 Subject: [PATCH 2/3] Handle IsClosed property in the DisposePattern --- src/CsvHelper/CsvDataReader.cs | 6 +++--- .../CsvDataReaderDisposalTests.cs | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/CsvHelper/CsvDataReader.cs b/src/CsvHelper/CsvDataReader.cs index 779da7159..d6cb1d175 100644 --- a/src/CsvHelper/CsvDataReader.cs +++ b/src/CsvHelper/CsvDataReader.cs @@ -38,6 +38,9 @@ public object this[string name] } } + /// + public bool IsClosed => disposed; + /// public int Depth { @@ -47,9 +50,6 @@ public int Depth } } - /// - public bool IsClosed { get; private set; } - /// public int RecordsAffected { diff --git a/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs b/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs index e2cde5023..3e8b90227 100644 --- a/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs +++ b/tests/CsvHelper.Tests/CsvDataReaderDisposalTests.cs @@ -1,6 +1,5 @@ using System.Globalization; using System.IO; -using System.Linq; using System.Text; using Xunit; @@ -25,6 +24,22 @@ public void ShouldNotDisposeCsvReaderWhenLeaveOpenParameterIsTrue() } } + [Fact] + public void DisposeShouldSetIsClosed() + { + var s = new StringBuilder(); + s.AppendLine("StringColumn"); + s.AppendLine("one"); + using (var reader = new StringReader(s.ToString())) + using (var csv = new CsvReader(reader, CultureInfo.InvariantCulture)) + { + var dataReader = new CsvDataReader(csv, leaveOpen: true); + dataReader.Dispose(); + + Assert.True(dataReader.IsClosed); + } + } + private class TestRecord() { public string StringColumn { get; set; } From e0a1be88fd3516bbd2fecbcc005078278d389cbf Mon Sep 17 00:00:00 2001 From: Pawel Krzyzak Date: Mon, 27 Jan 2025 12:55:32 +0100 Subject: [PATCH 3/3] Add documentation for CsvDataReader ctor parameter 'leaveOpen' --- src/CsvHelper/CsvDataReader.cs | 55 +++++++++++++++++----------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/src/CsvHelper/CsvDataReader.cs b/src/CsvHelper/CsvDataReader.cs index c89c3bda5..2572099f7 100644 --- a/src/CsvHelper/CsvDataReader.cs +++ b/src/CsvHelper/CsvDataReader.cs @@ -67,12 +67,13 @@ public int FieldCount } } - /// - /// Initializes a new instance of the class. - /// - /// The CSV. - /// The DataTable representing the file schema. - public CsvDataReader(CsvReader csv, DataTable? schemaTable = null, bool leaveOpen = false) + /// + /// Initializes a new instance of the class. + /// + /// The CSV. + /// The DataTable representing the file schema. + /// true to leave the open after the object is disposed, otherwise false. + public CsvDataReader(CsvReader csv, DataTable? schemaTable = null, bool leaveOpen = false) { this.csv = csv; this.leaveOpen = leaveOpen; @@ -97,35 +98,35 @@ public void Close() Dispose(); } - /// - public void Dispose() + /// + public void Dispose() + { + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + + /// + /// Disposes the object. + /// + /// Indicates if the object is being disposed. + protected virtual void Dispose(bool disposing) + { + if (disposed) { - Dispose(disposing: true); - GC.SuppressFinalize(this); + return; } - /// - /// Disposes the object. - /// - /// Indicates if the object is being disposed. - protected virtual void Dispose(bool disposing) + if (disposing) { - if (disposed) - { - return; - } - - if (disposing) + if (!leaveOpen) { - if (!leaveOpen) - { - csv?.Dispose(); - } + csv?.Dispose(); } - - disposed = true; } + disposed = true; + } + /// public bool GetBoolean(int i) {