Skip to content
Permalink
Browse files

Partial fix for #399 - track .Close() where possible

Okay so...we can't fix this completely. It has to be a best effort. The problem here is: not everything is going to call .Close(). The main usage case here is a DbDataReader passed a CommandBehavior.CloseConnection. It will call .Close() *on its expected type*, e.g. a SqlConnection. This will not be triggering the .Close() wrapper introduced here.

Everywhere else we do call .Close() explicitly will work fine, but even things like Dapper's .Query() won't show a .Close() in the profiler customer timings if using a data reader with close-on-finish behavior behind the scenes.
  • Loading branch information...
NickCraver committed Jul 19, 2019
1 parent a71fc6b commit a7322be1d97be0720832ea9667105c0729d9343d
Showing with 41 additions and 7 deletions.
  1. +15 −2 src/MiniProfiler.Shared/Data/ProfiledDbConnection.cs
  2. +26 −5 tests/MiniProfiler.Tests/DbProfilerTests.cs
@@ -89,7 +89,20 @@ public override string ConnectionString
/// Closes the connection to the database.
/// This is the preferred method of closing any open connection.
/// </summary>
public override void Close() => _connection.Close();
public override void Close()
{
var miniProfiler = _profiler as MiniProfiler;
if (miniProfiler == null || !miniProfiler.IsActive || miniProfiler.Options?.TrackConnectionOpenClose == false)
{
_connection.Close();
return;
}

using (miniProfiler.CustomTiming("sql", "Connection Close()", nameof(Close)))
{
_connection.Close();
}
}

/// <summary>
/// Opens a database connection with the settings specified by the <see cref="ConnectionString"/>.
@@ -137,7 +150,7 @@ protected override DbTransaction BeginDbTransaction(IsolationLevel isolationLeve
{
return new ProfiledDbTransaction(_connection.BeginTransaction(isolationLevel), this);
}

/// <summary>
/// Creates and returns a <see cref="DbCommand"/> object associated with the current connection.
/// </summary>
@@ -265,7 +265,24 @@ public void TrackingOptions(bool track)
const string cmdString = "Select 1";
GetUnopenedConnection(profiler).Query(cmdString);

CheckConnectionTracking(track, profiler, cmdString, false);
CheckConnectionTracking(track, profiler, cmdString, false, false);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void TrackingOptionsExplicitClose(bool track)
{
var options = new MiniProfilerTestOptions { TrackConnectionOpenClose = track };
var profiler = options.StartProfiler("Tracking: " + track);

const string cmdString = "Select 1";
var conn = GetUnopenedConnection(profiler);
conn.Open();
conn.Query(cmdString);
conn.Close();

CheckConnectionTracking(track, profiler, cmdString, false, true);
}

[Theory]
@@ -279,7 +296,7 @@ public async Task TrackingOptionsAsync(bool track)
const string cmdString = "Select 1";
await GetUnopenedConnection(profiler).QueryAsync(cmdString).ConfigureAwait(false);

CheckConnectionTracking(track, profiler, cmdString, true);
CheckConnectionTracking(track, profiler, cmdString, true, true);
}

[Fact]
@@ -292,7 +309,7 @@ public void ShimProfiler()
const string cmdString = "Select 1";
GetUnopenedConnection(currentDbProfiler).Query(cmdString);

CheckConnectionTracking(false, profiler, cmdString, false);
CheckConnectionTracking(false, profiler, cmdString, false, false);
}

private class CurrentDbProfiler : IDbProfiler
@@ -314,7 +331,7 @@ private class CurrentDbProfiler : IDbProfiler
public void ReaderFinish(IDataReader reader) => GetProfiler()?.ReaderFinish(reader);
}

private void CheckConnectionTracking(bool track, MiniProfiler profiler, string command, bool async)
private void CheckConnectionTracking(bool track, MiniProfiler profiler, string command, bool async, bool expectClose)
{
Assert.NotNull(profiler.Root.CustomTimings);
Assert.Single(profiler.Root.CustomTimings);
@@ -323,9 +340,13 @@ private void CheckConnectionTracking(bool track, MiniProfiler profiler, string c

if (track)
{
Assert.Equal(2, sqlTimings.Count);
Assert.Equal(expectClose ? 3 : 2, sqlTimings.Count);
Assert.Equal(async ? "Connection OpenAsync()" : "Connection Open()", sqlTimings[0].CommandString);
Assert.Equal(command, sqlTimings[1].CommandString);
if (expectClose)
{
Assert.Equal("Connection Close()", sqlTimings[2].CommandString);
}
}
else
{

0 comments on commit a7322be

Please sign in to comment.
You can’t perform that action at this time.