Skip to content

Commit

Permalink
Don't dispose DbConnections if they were passed in as an actual insta…
Browse files Browse the repository at this point in the history
…nce rather than created with a factory.
  • Loading branch information
MatthewKing committed Jul 5, 2024
1 parent d2ced1e commit 7e85fe2
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 13 deletions.
38 changes: 34 additions & 4 deletions src/DeviceId.SqlServer/SqlServerDeviceIdBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,32 @@ public class SqlServerDeviceIdBuilder
/// </summary>
private readonly Func<DbConnection> _connectionFactory;

/// <summary>
/// A value determining whether the connection should be disposed after use.
/// </summary>
private readonly bool _disposeConnection;

/// <summary>
/// Initializes a new instance of the <see cref="SqlServerDeviceIdBuilder"/> class.
/// </summary>
/// <param name="baseBuilder">The base device identifier builder.</param>
/// <param name="connection">A connection to the SQL Server database.</param>
public SqlServerDeviceIdBuilder(DeviceIdBuilder baseBuilder, DbConnection connection)
: this(baseBuilder, () => connection) { }
{
if (baseBuilder is null)
{
throw new ArgumentNullException(nameof(baseBuilder));
}

if (connection is null)
{
throw new ArgumentNullException(nameof(connection));
}

_baseBuilder = baseBuilder;
_connectionFactory = () => connection;
_disposeConnection = false;
}

/// <summary>
/// Initializes a new instance of the <see cref="SqlServerDeviceIdBuilder"/> class.
Expand All @@ -34,8 +53,19 @@ public SqlServerDeviceIdBuilder(DeviceIdBuilder baseBuilder, DbConnection connec
/// <param name="connectionFactory">A factory used to get a connection to the SQL Server database.</param>
public SqlServerDeviceIdBuilder(DeviceIdBuilder baseBuilder, Func<DbConnection> connectionFactory)
{
_baseBuilder = baseBuilder ?? throw new ArgumentNullException(nameof(baseBuilder));
_connectionFactory = connectionFactory ?? throw new ArgumentNullException(nameof(connectionFactory));
if (baseBuilder is null)
{
throw new ArgumentNullException(nameof(baseBuilder));
}

if (connectionFactory is null)
{
throw new ArgumentNullException(nameof(connectionFactory));
}

_baseBuilder = baseBuilder;
_connectionFactory = connectionFactory;
_disposeConnection = true;
}

/// <summary>
Expand All @@ -58,7 +88,7 @@ public SqlServerDeviceIdBuilder AddQueryResult(string componentName, string sql)
/// <returns>The <see cref="SqlServerDeviceIdBuilder"/> instance.</returns>
public SqlServerDeviceIdBuilder AddQueryResult(string componentName, string sql, Func<object, string> valueTransformer)
{
_baseBuilder.Components.Add(componentName, new DatabaseQueryDeviceIdComponent(_connectionFactory, sql, valueTransformer));
_baseBuilder.Components.Add(componentName, new DatabaseQueryDeviceIdComponent(_connectionFactory, sql, valueTransformer, _disposeConnection));

return this;
}
Expand Down
43 changes: 34 additions & 9 deletions src/DeviceId/Components/DatabaseQueryDeviceIdComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,33 @@ public class DatabaseQueryDeviceIdComponent : IDeviceIdComponent
/// </summary>
private readonly Func<object, string> _valueTransformer;

/// <summary>
/// A value determining whether the connection should be disposed after use or not.
/// </summary>
private readonly bool _disposeConnection;

/// <summary>
/// Initializes a new instance of the <see cref="DatabaseQueryDeviceIdComponent"/> class.
/// </summary>
/// <param name="connectionFactory">A factory used to get a connection to the database.</param>
/// <param name="sql">SQL query that returns a single value to be added to the device identifier.</param>
/// <param name="valueTransformer">A function that transforms the result of the query into a string.</param>
public DatabaseQueryDeviceIdComponent(Func<DbConnection> connectionFactory, string sql, Func<object, string> valueTransformer)
: this(connectionFactory, sql, valueTransformer, true) { }

/// <summary>
/// Initializes a new instance of the <see cref="DatabaseQueryDeviceIdComponent"/> class.
/// </summary>
/// <param name="connectionFactory">A factory used to get a connection to the database.</param>
/// <param name="sql">SQL query that returns a single value to be added to the device identifier.</param>
/// <param name="valueTransformer">A function that transforms the result of the query into a string.</param>
/// <param name="disposeConnection">A value determining whether the connection should be disposed after use or not. Default is true.</param>
public DatabaseQueryDeviceIdComponent(Func<DbConnection> connectionFactory, string sql, Func<object, string> valueTransformer, bool disposeConnection)
{
_connectionFactory = connectionFactory;
_sql = sql;
_valueTransformer = valueTransformer;
_disposeConnection = disposeConnection;
}

/// <summary>
Expand All @@ -44,18 +60,27 @@ public string GetValue()
{
try
{
using var connection = _connectionFactory.Invoke();

connection.Open();
var connection = _connectionFactory.Invoke();
try
{
connection.Open();

using var command = connection.CreateCommand();
command.CommandText = _sql;
using var command = connection.CreateCommand();
command.CommandText = _sql;

var result = command.ExecuteScalar();
if (result != null)
var result = command.ExecuteScalar();
if (result != null)
{
var value = _valueTransformer?.Invoke(result);
return value;
}
}
finally
{
var value = _valueTransformer?.Invoke(result);
return value;
if (_disposeConnection && connection is not null)
{
connection.Dispose();
}
}

return null;
Expand Down

0 comments on commit 7e85fe2

Please sign in to comment.