Marc/wrapped async reader (#1278)

* fix #1277 - make sure that the command is disposed at the correct time when using ExecuteReaderAsync; this means pushing it into WrappedReader, which was in turn overdue some love; as part of that, add netcoreapp2.1 target so we can expose the best reader possible

* fix tests - removed MSSQLCLIENT for now

Dapper.StrongName/Dapper.StrongName.csproj

@@ -5,7 +5,7 @@
     <Title>Dapper (Strong Named)</Title>
     <Description>A high performance Micro-ORM supporting SQL Server, MySQL, Sqlite, SqlCE, Firebird etc..</Description>
     <Authors>Sam Saffron;Marc Gravell;Nick Craver</Authors>
-    <TargetFrameworks>net451;netstandard1.3;netstandard2.0</TargetFrameworks>
+    <TargetFrameworks>net451;netstandard1.3;netstandard2.0;netcoreapp2.1</TargetFrameworks>
     <SignAssembly>true</SignAssembly>
     <PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
   </PropertyGroup>
@@ -24,6 +24,10 @@
     <PackageReference Include="System.Reflection.TypeExtensions" Version="4.4.0" />
 
     <!-- it would be nice to use System.Data.Common here, but we need SqlClient for SqlDbType in 1.3, and legacy SqlDataRecord API-->
+    <!--<PackageReference Include="System.Data.Common" Version="4.3.0" />-->
+    <PackageReference Include="System.Data.SqlClient" Version="4.6.0" />
+  </ItemGroup>
+  <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.1'">
     <PackageReference Include="System.Data.SqlClient" Version="4.6.0" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' ">

Dapper.Tests/AsyncTests.cs

@@ -11,13 +11,17 @@ namespace Dapper.Tests
 {
     [Collection(NonParallelDefinition.Name)]
     public sealed class SystemSqlClientAsyncTests : AsyncTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     [Collection(NonParallelDefinition.Name)]
     public sealed class MicrosoftSqlClientAsyncTests : AsyncTests<MicrosoftSqlClientProvider> { }
+#endif
 
     [Collection(NonParallelDefinition.Name)]
     public sealed class SystemSqlClientAsyncQueryCacheTests : AsyncQueryCacheTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     [Collection(NonParallelDefinition.Name)]
     public sealed class MicrosoftSqlClientAsyncQueryCacheTests : AsyncQueryCacheTests<MicrosoftSqlClientProvider> { }
+#endif
 
 
     public abstract class AsyncTests<TProvider> : TestBase<TProvider> where TProvider : SqlServerDatabaseProvider

Dapper.Tests/ConstructorTests.cs

@@ -6,7 +6,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientConstructorTests : ConstructorTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientConstructorTests : ConstructorTests<MicrosoftSqlClientProvider> { }
+#endif
 
     public abstract class ConstructorTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {

Dapper.Tests/Dapper.Tests.csproj

@@ -16,6 +16,14 @@
   <ItemGroup>
     <None Remove="Test.DB.sdf" />
   </ItemGroup>
+  <!-- these two go together
+  <PropertyGroup>
+    <DefineConstants>$(DefineConstants);MSSQLCLIENT</DefineConstants>
+  </PropertyGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.Data.SqlClient" Version="1.0.19128.1-Preview" />
+  </ItemGroup>
+  -->
   <ItemGroup>
     <ProjectReference Include="..\Dapper\Dapper.csproj" />
     <ProjectReference Include="..\Dapper.Contrib\Dapper.Contrib.csproj" />
@@ -26,7 +34,6 @@
     <PackageReference Include="System.ValueTuple" Version="4.4.0" />
     <PackageReference Include="xunit" Version="$(xUnitVersion)" />
     <PackageReference Include="xunit.runner.visualstudio" Version="$(xUnitVersion)" />
-    <PackageReference Include="Microsoft.Data.SqlClient" Version="1.0.19128.1-Preview" />
     <PackageReference Include="System.Data.SqlClient" Version="4.6.0" />
   </ItemGroup>
 

Dapper.Tests/DataReaderTests.cs

@@ -5,7 +5,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientDataReaderTests : DataReaderTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientDataReaderTests : DataReaderTests<MicrosoftSqlClientProvider> { }
+#endif
 
     public abstract class DataReaderTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {

Dapper.Tests/DecimalTests.cs

@@ -6,7 +6,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientDecimalTests : DecimalTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientDecimalTests : DecimalTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class DecimalTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/EnumTests.cs

@@ -5,7 +5,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientEnumTests : EnumTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientEnumTests : EnumTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class EnumTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/LiteralTests.cs

@@ -4,7 +4,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientLiteralTests : LiteralTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientLiteralTests : LiteralTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class LiteralTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/MiscTests.cs

@@ -40,7 +40,9 @@ public GenericUriParser(GenericUriParserOptions options)
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientMiscTests : MiscTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientMiscTests : MiscTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class MiscTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/MultiMapTests.cs

@@ -7,7 +7,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientMultiMapTests : MultiMapTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientMultiMapTests : MultiMapTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class MultiMapTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/NullTests.cs

@@ -4,8 +4,10 @@ namespace Dapper.Tests
 {
     [Collection(NonParallelDefinition.Name)]
     public sealed class SystemSqlClientNullTests : NullTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     [Collection(NonParallelDefinition.Name)]
     public sealed class MicrosoftSqlClientNullTests : NullTests<MicrosoftSqlClientProvider> { }
+#endif
 
     public abstract class NullTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {

Dapper.Tests/ParameterTests.cs

@@ -20,8 +20,10 @@ namespace Dapper.Tests
 {
     [Collection(NonParallelDefinition.Name)] // because it creates SQL types that compete between the two providers
     public sealed class SystemSqlClientParameterTests : ParameterTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     [Collection(NonParallelDefinition.Name)] // because it creates SQL types that compete between the two providers
     public sealed class MicrosoftSqlClientParameterTests : ParameterTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class ParameterTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         public class DbParams : SqlMapper.IDynamicParameters, IEnumerable<IDbDataParameter>

Dapper.Tests/ProcedureTests.cs

@@ -7,7 +7,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientProcedureTests : ProcedureTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientProcedureTests : ProcedureTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class ProcedureTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/Providers/EntityFrameworkTests.cs

@@ -7,7 +7,9 @@
 namespace Dapper.Tests.Providers
 {
     public sealed class SystemSqlClientEntityFrameworkTests : EntityFrameworkTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientEntityFrameworkTests : EntityFrameworkTests<MicrosoftSqlClientProvider> { }
+#endif
 
     [Collection("TypeHandlerTests")]
     public abstract class EntityFrameworkTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider

Dapper.Tests/Providers/Linq2SqlTests.cs

@@ -9,7 +9,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientLinq2SqlTests : Linq2SqlTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientLinq2SqlTests : Linq2SqlTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class Linq2SqlTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/Providers/MySQLTests.cs

 using System;
+using System.Data;
 using System.Data.Common;
 using System.Linq;
+using System.Threading.Tasks;
 using Xunit;
 
 namespace Dapper.Tests
@@ -155,6 +157,52 @@ public void SO36303462_Tinyint_Bools()
             }
         }
 
+        [Fact]
+        public void Issue1277_ReaderSync()
+        {
+            using (var conn = Provider.GetMySqlConnection())
+            {
+                try { conn.Execute("drop table Issue1277_Test"); } catch { /* don't care */ }
+                conn.Execute("create table Issue1277_Test (Id int not null, IsBold tinyint not null);");
+                conn.Execute("insert Issue1277_Test (Id, IsBold) values (1,1);");
+                conn.Execute("insert Issue1277_Test (Id, IsBold) values (2,0);");
+                conn.Execute("insert Issue1277_Test (Id, IsBold) values (3,1);");
+
+                using (var reader = conn.ExecuteReader(
+                    "select * from Issue1277_Test where Id < @id",
+                    new { id = 42 }))
+                {
+                    var table = new DataTable();
+                    table.Load(reader);
+                    Assert.Equal(2, table.Columns.Count);
+                    Assert.Equal(3, table.Rows.Count);
+                }
+            }
+        }
+
+        [Fact]
+        public async Task Issue1277_ReaderAsync()
+        {
+            using (var conn = Provider.GetMySqlConnection())
+            {
+                try { await conn.ExecuteAsync("drop table Issue1277_Test"); } catch { /* don't care */ }
+                await conn.ExecuteAsync("create table Issue1277_Test (Id int not null, IsBold tinyint not null);");
+                await conn.ExecuteAsync("insert Issue1277_Test (Id, IsBold) values (1,1);");
+                await conn.ExecuteAsync("insert Issue1277_Test (Id, IsBold) values (2,0);");
+                await conn.ExecuteAsync("insert Issue1277_Test (Id, IsBold) values (3,1);");
+
+                using (var reader = await conn.ExecuteReaderAsync(
+                    "select * from Issue1277_Test where Id < @id",
+                    new { id = 42 }))
+                {
+                    var table = new DataTable();
+                    table.Load(reader);
+                    Assert.Equal(2, table.Columns.Count);
+                    Assert.Equal(3, table.Rows.Count);
+                }
+            }
+        }
+
         private class SO36303462
         {
             public int Id { get; set; }

Dapper.Tests/QueryMultipleTests.cs

@@ -7,7 +7,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientQueryMultipleTests : QueryMultipleTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientQueryMultipleTests : QueryMultipleTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class QueryMultipleTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/TestBase.cs

@@ -72,10 +72,12 @@ public sealed class SystemSqlClientProvider : SqlServerDatabaseProvider
     {
         public override DbProviderFactory Factory => System.Data.SqlClient.SqlClientFactory.Instance;
     }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientProvider : SqlServerDatabaseProvider
     {
         public override DbProviderFactory Factory => Microsoft.Data.SqlClient.SqlClientFactory.Instance;
     }
+#endif
 
     public abstract class TestBase<TProvider> : IDisposable where TProvider : DatabaseProvider
     {

Dapper.Tests/TransactionTests.cs

@@ -8,7 +8,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientTransactionTests : TransactionTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientTransactionTests : TransactionTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class TransactionTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/TupleTests.cs

@@ -4,7 +4,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientTupleTests : TupleTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientTupleTests : TupleTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class TupleTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper.Tests/TypeHandlerTests.cs

@@ -10,8 +10,10 @@ namespace Dapper.Tests
 {
     [Collection(NonParallelDefinition.Name)]
     public sealed class SystemSqlClientTypeHandlerTests : TypeHandlerTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     [Collection(NonParallelDefinition.Name)]
     public sealed class MicrosoftSqlClientTypeHandlerTests : TypeHandlerTests<MicrosoftSqlClientProvider> { }
+#endif
 
     public abstract class TypeHandlerTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {

Dapper.Tests/XmlTests.cs

@@ -5,7 +5,9 @@
 namespace Dapper.Tests
 {
     public sealed class SystemSqlClientXmlTests : XmlTests<SystemSqlClientProvider> { }
+#if MSSQLCLIENT
     public sealed class MicrosoftSqlClientXmlTests : XmlTests<MicrosoftSqlClientProvider> { }
+#endif
     public abstract class XmlTests<TProvider> : TestBase<TProvider> where TProvider : DatabaseProvider
     {
         [Fact]

Dapper/Dapper.csproj

@@ -5,7 +5,7 @@
     <Title>Dapper</Title>
     <Description>A high performance Micro-ORM supporting SQL Server, MySQL, Sqlite, SqlCE, Firebird etc..</Description>
     <Authors>Sam Saffron;Marc Gravell;Nick Craver</Authors>
-    <TargetFrameworks>net451;netstandard1.3;netstandard2.0</TargetFrameworks>
+    <TargetFrameworks>net451;netstandard1.3;netstandard2.0;netcoreapp2.1</TargetFrameworks>
   </PropertyGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'net451'">
     <Reference Include="System" />
@@ -19,6 +19,10 @@
     <PackageReference Include="System.Reflection.TypeExtensions" Version="4.4.0" />
 
     <!-- it would be nice to use System.Data.Common here, but we need SqlClient for SqlDbType in 1.3, and legacy SqlDataRecord API-->
+    <!--<PackageReference Include="System.Data.Common" Version="4.3.0" />-->
+    <PackageReference Include="System.Data.SqlClient" Version="4.6.0" />
+  </ItemGroup>
+  <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.1'">
     <PackageReference Include="System.Data.SqlClient" Version="4.6.0" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' ">

Dapper/SqlDataRecordListTVPParameter.cs

@@ -84,7 +84,11 @@ static class StructuredHelper
             {
                 il.Emit(OpCodes.Ldarg_0);
                 il.Emit(OpCodes.Castclass, type);
+#if NETSTANDARD1_3
+                il.Emit(OpCodes.Ldc_I4, (int)30); // hard-code the known enum value
+#else
                 il.Emit(OpCodes.Ldc_I4, (int)SqlDbType.Structured);
+#endif
                 il.EmitCall(OpCodes.Callvirt, dbType.GetSetMethod(), null);
             }
 

Dapper/SqlMapper.Async.cs

@@ -1101,7 +1101,7 @@ public static async Task<GridReader> QueryMultipleAsync(this IDbConnection cnn,
         /// </code>
         /// </example>
         public static Task<IDataReader> ExecuteReaderAsync(this IDbConnection cnn, string sql, object param = null, IDbTransaction transaction = null, int? commandTimeout = null, CommandType? commandType = null) =>
-            ExecuteReaderImplAsync(cnn, new CommandDefinition(sql, param, transaction, commandTimeout, commandType, CommandFlags.Buffered), CommandBehavior.Default);
+            ExecuteWrappedReaderImplAsync(cnn, new CommandDefinition(sql, param, transaction, commandTimeout, commandType, CommandFlags.Buffered), CommandBehavior.Default);
 
         /// <summary>
         /// Execute parameterized SQL and return an <see cref="IDataReader"/>.
@@ -1114,7 +1114,7 @@ public static async Task<GridReader> QueryMultipleAsync(this IDbConnection cnn,
         /// or <see cref="T:DataSet"/>.
         /// </remarks>
         public static Task<IDataReader> ExecuteReaderAsync(this IDbConnection cnn, CommandDefinition command) =>
-            ExecuteReaderImplAsync(cnn, command, CommandBehavior.Default);
+            ExecuteWrappedReaderImplAsync(cnn, command, CommandBehavior.Default);
 
         /// <summary>
         /// Execute parameterized SQL and return an <see cref="IDataReader"/>.
@@ -1128,26 +1128,27 @@ public static async Task<GridReader> QueryMultipleAsync(this IDbConnection cnn,
         /// or <see cref="T:DataSet"/>.
         /// </remarks>
         public static Task<IDataReader> ExecuteReaderAsync(this IDbConnection cnn, CommandDefinition command, CommandBehavior commandBehavior) =>
-            ExecuteReaderImplAsync(cnn, command, commandBehavior);
+            ExecuteWrappedReaderImplAsync(cnn, command, commandBehavior);
 
-        private static async Task<IDataReader> ExecuteReaderImplAsync(IDbConnection cnn, CommandDefinition command, CommandBehavior commandBehavior)
+        private static async Task<IDataReader> ExecuteWrappedReaderImplAsync(IDbConnection cnn, CommandDefinition command, CommandBehavior commandBehavior)
         {
             Action<IDbCommand, object> paramReader = GetParameterReader(cnn, ref command);
 
             DbCommand cmd = null;
-            bool wasClosed = cnn.State == ConnectionState.Closed;
+            bool wasClosed = cnn.State == ConnectionState.Closed, disposeCommand = true;
             try
             {
                 cmd = command.TrySetupAsyncCommand(cnn, paramReader);
                 if (wasClosed) await cnn.TryOpenAsync(command.CancellationToken).ConfigureAwait(false);
                 var reader = await ExecuteReaderWithFlagsFallbackAsync(cmd, wasClosed, commandBehavior, command.CancellationToken).ConfigureAwait(false);
                 wasClosed = false;
-                return reader;
+                disposeCommand = false;
+                return WrappedReader.Create(cmd, reader);
             }
             finally
             {
                 if (wasClosed) cnn.Close();
-                cmd?.Dispose();
+                if (cmd != null && disposeCommand) cmd.Dispose();
             }
         }
 

Dapper/SqlMapper.cs

@@ -599,7 +599,7 @@ public static IDataReader ExecuteReader(this IDbConnection cnn, string sql, obje
         {
             var command = new CommandDefinition(sql, param, transaction, commandTimeout, commandType, CommandFlags.Buffered);
             var reader = ExecuteReaderImpl(cnn, ref command, CommandBehavior.Default, out IDbCommand dbcmd);
-            return new WrappedReader(dbcmd, reader);
+            return WrappedReader.Create(dbcmd, reader);
         }
 
         /// <summary>
@@ -615,7 +615,7 @@ public static IDataReader ExecuteReader(this IDbConnection cnn, string sql, obje
         public static IDataReader ExecuteReader(this IDbConnection cnn, CommandDefinition command)
         {
             var reader = ExecuteReaderImpl(cnn, ref command, CommandBehavior.Default, out IDbCommand dbcmd);
-            return new WrappedReader(dbcmd, reader);
+            return WrappedReader.Create(dbcmd, reader);
         }
 
         /// <summary>
@@ -632,7 +632,7 @@ public static IDataReader ExecuteReader(this IDbConnection cnn, CommandDefinitio
         public static IDataReader ExecuteReader(this IDbConnection cnn, CommandDefinition command, CommandBehavior commandBehavior)
         {
             var reader = ExecuteReaderImpl(cnn, ref command, commandBehavior, out IDbCommand dbcmd);
-            return new WrappedReader(dbcmd, reader);
+            return WrappedReader.Create(dbcmd, reader);
         }
 
         /// <summary>
@@ -3696,6 +3696,7 @@ public static void SetTypeName(this DataTable table, string typeName)
         public static ICustomQueryParameter AsTableValuedParameter<T>(this IEnumerable<T> list, string typeName = null) where T : IDataRecord =>
             new SqlDataRecordListTVPParameter<T>(list, typeName);
 
+        /* 
         /// <summary>
         /// Used to pass a IEnumerable&lt;SqlDataRecord&gt; as a TableValuedParameter.
         /// </summary>
@@ -3704,6 +3705,7 @@ public static void SetTypeName(this DataTable table, string typeName)
         public static ICustomQueryParameter AsTableValuedParameter(this IEnumerable<Microsoft.SqlServer.Server.SqlDataRecord> list, string typeName = null) =>
             new SqlDataRecordListTVPParameter<Microsoft.SqlServer.Server.SqlDataRecord>(list, typeName);
         // ^^^ retained to avoid missing-method-exception; can presumably drop in a "major"
+        */
 
         // one per thread
         [ThreadStatic]