Merge pull request #60 from abe545/fix-dynamic-async-no-results-shoul…

…d-throw

Bug fixes for release 2.2.5

CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedure.cs

@@ -14,6 +14,7 @@ namespace CodeOnlyStoredProcedure.Dynamic
     internal class DynamicStoredProcedure : DynamicObject
     {
         internal const string asyncParameterDirectionError = "Can not execute a stored procedure asynchronously if called with a ref or out parameter. You can retrieve output or return values from the stored procedure if you pass in an input class, which the library can parse into the correct properties.";
+        internal const string namedParameterException      = "When using the dynamic syntax, parameters must either be passed by name, or as properties of a class (anonymous types work great).";
         private static readonly Func<CSharpArgumentInfo, string>                  getParameterName;
         private static readonly Func<CSharpArgumentInfo, ParameterDirection>      getParameterDirection;
         private static readonly Func<InvokeMemberBinder, int, CSharpArgumentInfo> getArgumentInfo;
@@ -89,6 +90,9 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, o
 
                 if (arg == null || arg == DBNull.Value)
                 {
+                    if (string.IsNullOrWhiteSpace(parmName))
+                        throw new StoredProcedureException(namedParameterException);
+
                     parameters.Add(new InputParameter(parmName, DBNull.Value));
                     continue;
                 }
@@ -105,9 +109,17 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, o
                                            .FirstOrDefault();
 
                     if (attr == null)
+                    {
+                        if (string.IsNullOrWhiteSpace(parmName))
+                            throw new StoredProcedureException(namedParameterException);
+
                         parameters.Add(itemType.CreateTableValuedParameter(parmName, arg));
+                    }
                     else
                     {
+                        if (string.IsNullOrWhiteSpace(attr.Name) && string.IsNullOrWhiteSpace(parmName))
+                            throw new StoredProcedureException("When using the dynamic syntax, parameters must be passed by name.\nBecause you're passing a Table Valued Parameter, if the TableValuedParameterAttribute decorating your class has the Name set, it will be used instead.");
+
                         parameters.Add(
                             new TableValuedParameter(attr.Name ?? parmName,
                                                      (IEnumerable)arg,
@@ -120,6 +132,9 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, o
                     parameters.AddRange(argType.GetParameters(arg));
                 else if (direction == ParameterDirection.Output)
                 {
+                    if (string.IsNullOrWhiteSpace(parmName))
+                        throw new StoredProcedureException(namedParameterException);
+
                     VerifySynchronousExecutionMode(executionMode);
 
                     if ("returnvalue".Equals(parmName, StringComparison.InvariantCultureIgnoreCase))
@@ -129,10 +144,15 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, o
                 }
                 else if (direction == ParameterDirection.InputOutput)
                 {
+                    if (string.IsNullOrWhiteSpace(parmName))
+                        throw new StoredProcedureException(namedParameterException);
+
                     VerifySynchronousExecutionMode(executionMode);
 
                     parameters.Add(new InputOutputParameter(parmName, o => args[idx] = o, arg, argType.InferDbType()));
                 }
+                else if (string.IsNullOrWhiteSpace(parmName))
+                    throw new StoredProcedureException(namedParameterException);
                 else
                     parameters.Add(new InputParameter(parmName, arg, argType.InferDbType()));
             }

CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs

@@ -30,7 +30,7 @@ internal class DynamicStoredProcedureResults : DynamicObject, IDisposable
         private readonly IEnumerable<IDataTransformer> transformers;
         private readonly DynamicExecutionMode          executionMode;
         private readonly CancellationToken             token;
-        private          bool                          continueOnCaller = true;
+        private          bool                          continueOnCaller;
 
         public DynamicStoredProcedureResults(
             IDbConnection                   connection,
@@ -48,10 +48,11 @@ public DynamicStoredProcedureResults(
             Contract.Requires(parameters != null);
             Contract.Requires(transformers != null);
 
-            this.executionMode = executionMode;
-            this.command       = connection.CreateCommand(schema, name, timeout, out this.connection);
-            this.transformers  = transformers;
-            this.token         = token;
+            this.executionMode    = executionMode;
+            this.command          = connection.CreateCommand(schema, name, timeout, out this.connection);
+            this.transformers     = transformers;
+            this.token            = token;
+            this.continueOnCaller = true;
 
             foreach (var p in parameters)
                 command.Parameters.Add(p.CreateDbDataParameter(command));
@@ -128,26 +129,29 @@ private IEnumerable<T> GetResults<T>(bool isSingle)
 
         private Task ContinueNoResults()
         {
-            return resultTask.ContinueWith(_ => Dispose(), token);
+            return resultTask.ContinueWith(r =>
+            {
+                Dispose();
+                if (r.Status == TaskStatus.Faulted)
+                    throw r.Exception;
+            }, token);
         }
 
         private Task<IEnumerable<T>> CreateSingleContinuation<T>()
         {
             return resultTask.ContinueWith(_ =>
             {
-                var res = GetResults<T>(true);
-                Dispose();
-                return res;
+                try { return GetResults<T>(true); }
+                finally { Dispose(); }
             }, token);
         }
 
         private Task<T> CreateSingleRowContinuation<T>()
         {
             return resultTask.ContinueWith(_ =>
             {
-                var res = GetResults<T>(true).SingleOrDefault();
-                Dispose();
-                return res;
+                try { return GetResults<T>(true).SingleOrDefault(); }
+                finally { Dispose(); }
             }, token);
         }
 
@@ -179,9 +183,8 @@ private Task<T> CreateMultipleContinuation<T>()
 
             return resultTask.ContinueWith(_ =>
             {
-                var res = GetMultipleResults<T>();
-                Dispose();
-                return res;
+                try { return GetMultipleResults<T>(); }
+                finally { Dispose(); }
             }, token);
         }
 

CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResultsAwaiter.cs

@@ -157,6 +157,9 @@ public override bool TryGetMember(GetMemberBinder binder, out object result)
         {
             if (binder.Name == "IsCompleted")
             {
+                if (toWait.Status == TaskStatus.Faulted)
+                    throw toWait.Exception;
+
                 result = toWait.IsCompleted;
                 return true;
             }
@@ -168,6 +171,9 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, o
         {
             if (binder.Name == "GetResult")
             {
+                if (toWait.Status == TaskStatus.Faulted)
+                    throw toWait.Exception;
+
                 result = results;
                 return true;
             }

CodeOnlyStoredProcedure/RowFactory/ExpandoObjectRowFactory.cs

@@ -11,6 +11,7 @@ namespace CodeOnlyStoredProcedure.RowFactory
     internal class ExpandoObjectRowFactory<T> : RowFactory<T>
     {
         static MethodInfo          addToDictionaryMethod = typeof(IDictionary<string, object>).GetMethod("Add");
+        static MethodInfo          isDbNull              = typeof(IDataRecord)                .GetMethod("IsDBNull");
         static MethodInfo          getDataValuesMethod   = typeof(IDataRecord)                .GetMethod("GetValues");
         static ParameterExpression readerExpression      = Expression                         .Parameter(typeof(IDataReader));
         static ParameterExpression resultExpression      = Expression                         .Parameter(typeof(ExpandoObject));
@@ -28,7 +29,10 @@ protected override Func<IDataReader, T> CreateRowFactory(IDataReader reader, IEn
 
             for (int i = 0; i < reader.FieldCount; i++)
             {
-                Expression getValue = Expression.ArrayIndex(valuesExpression, Expression.Constant(i));
+                Expression getValue = Expression.Condition( 
+                    Expression.Call(readerExpression, isDbNull, Expression.Constant(i)),
+                    Expression.Constant(null, typeof(object)),
+                    Expression.ArrayIndex(valuesExpression, Expression.Constant(i)));
 
                 if (xFormers.Any())
                 {

CodeOnlyStoredProcedures.nuspec

@@ -9,7 +9,12 @@
         <requireLicenseAcceptance>false</requireLicenseAcceptance>
         <description>A library for easily calling Stored Procedures in .NET. Works great with Entity Framework Code First models. 
 Code Only Stored Procedures will not create any Stored Procedures on your database. Instead, its aim is to make it easy to call your existing stored procedures by writing simple code.</description>
-        <releaseNotes>2.2.4
+        <releaseNotes>2.2.5
+Fixed bug where a dynamic stored procedure wouldn't dispose its database connection if the stored procedure threw an exception.
+Fixed bug in the dynamic syntax where asynchronous execution of a stored procedure that has no results would not throw exceptions from sql server.
+Fixed bug where StoredProcedure&lt;dynamic&gt; (both syntaxes) would return DBNull values instead of null.
+
+2.2.4
 Fixed bug where calling ToString on a stored procedure could print parameters with a double @.
 Fixed bug where the fluent syntax would not infer the type of its parameters from the compile time generic parameter type.
 
@@ -54,10 +59,7 @@ Added StoredProcedure.Execute and StoredProcedure.ExecuteAsync methods to more e
 Added ability to specify an implementation of an interface, so a StoredProcedure can return an IEnumerable&lt;interface&gt;
 
 1.2.1
-Added better exception when a model is missing a public parameterless constructor.
-
-1.2.0
-Added a much cleaner syntax for calling stored procedures, by using dynamic objects.</releaseNotes>
+Added better exception when a model is missing a public parameterless constructor.</releaseNotes>
         <tags>StoredProcedure EntityFramework EF</tags>
         <dependencies>
             <group targetFramework=".NETFramework4.0">

CodeOnlyTests/Dynamic/DynamicStoredProcedureTests.cs

@@ -347,6 +347,21 @@ public void CanPassDBNullParameter()
                 dynamic toTest = new DynamicStoredProcedure(ctx, transformers, CancellationToken.None, TEST_TIMEOUT, DynamicExecutionMode.Synchronous);
                 toTest.usp_GetPeople(id: DBNull.Value);
             }
+
+            [TestMethod]
+            public void UnnamedParameters_Throw_Useful_Exception()
+            {
+                var ctx = CreatePeople("Foo");
+                dynamic toTest = new DynamicStoredProcedure(ctx, transformers, CancellationToken.None, TEST_TIMEOUT, DynamicExecutionMode.Synchronous);
+                try
+                {
+                    toTest.usp_GetPeople("foo");
+                }
+                catch (StoredProcedureException ex)
+                {
+                    ex.Message.Should().Be(DynamicStoredProcedure.namedParameterException);
+                }
+            }
         }
 
         [TestClass]

CodeOnlyTests/RowFactory/ExpandoObjectRowFactoryTests.cs

@@ -208,6 +208,30 @@ public void IDataTransformers_CalledForAppropriateColumns()
                 ((string)item.Name).Should().Be("foo", "because it should have been transformed by the string transformer");
                 ((int)item.Age).Should().Be(55, "because it should have been transformed by the int transformer");
             }
+
+            [TestMethod]
+            public void UsesProper_NullableValues_ForNullObject()
+            {
+                var rdr = new Mock<IDataReader>();
+                rdr.Setup(r => r.FieldCount).Returns(1);
+                rdr.Setup(r => r.GetFieldType(0)).Returns(typeof(int));
+                rdr.Setup(r => r.GetName(0)).Returns("Id");
+                rdr.Setup(r => r.IsDBNull(0)).Returns(true);
+                rdr.Setup(r => r.GetValues(It.IsAny<object[]>()))
+                   .Callback<object[]>(os => os[0] = DBNull.Value);
+                rdr.SetupSequence(r => r.Read())
+                   .Returns(true)
+                   .Returns(false);
+
+                var toTest = new ExpandoObjectRowFactory<dynamic>();
+                var res = toTest.ParseRows(rdr.Object,
+                                              new IDataTransformer[0],
+                                              CancellationToken.None);
+
+                var item = res.Should().ContainSingle("because only 1 row should have been returned").Which;
+                int? asNullableInt = item.Id;
+                asNullableInt.Should().NotHaveValue("because the returned item was null");
+            }
         }
 
         [TestClass]

SmokeTests/DynamicSyntax.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using System.Data;
+using System.Data.SqlClient;
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
@@ -516,5 +517,89 @@ public object Transform(object value, Type targetType, bool isNullable, IEnumera
             }
         }
         #endregion
+
+        #region Non-existant Stored Procedure
+        [SmokeTest("Dynamic Syntax Calling a non-existant stored procedure synchronously, expecitng results")]
+        Tuple<bool, string> NonExistant_WithResults_Synchronously(IDbConnection db)
+        {
+            try
+            {
+                IEnumerable<Item> res = db.Execute(Program.timeout).usp_DoUknownStoredProcedure();
+                return Tuple.Create(false, "Expected exception to be thrown, because the stored procedure doesn't exist, but none was.");
+            }
+            catch (SqlException)
+            {
+                return Tuple.Create(true, "");
+            }
+        }
+
+        [SmokeTest("Dynamic Syntax Calling a non-existant stored procedure synchronously, expecitng no results")]
+        Tuple<bool, string> NonExistant_WithoutResults_Synchronously(IDbConnection db)
+        {
+            try
+            {
+                db.Execute(Program.timeout).usp_DoUknownStoredProcedure();
+                return Tuple.Create(false, "Expected exception to be thrown, because the stored procedure doesn't exist, but none was.");
+            }
+            catch (SqlException)
+            {
+                return Tuple.Create(true, "");
+            }
+        }
+
+        [SmokeTest("Dynamic Syntax Calling a non-existant stored procedure asynchronously, expecitng results (Task)")]
+        Task<Tuple<bool, string>> NonExistant_WithResults_Asynchronously(IDbConnection db)
+        {
+            Task<IEnumerable<Item>> res = db.ExecuteAsync(Program.timeout).usp_DoUknownStoredProcedure();
+            return res.ContinueWith(r =>
+            {
+                if (r.Exception == null)
+                    return Tuple.Create(false, "Expected exception to be thrown, because the stored procedure doesn't exist, but none was.");
+
+                return Tuple.Create(true, "");
+            });
+        }
+
+        [SmokeTest("Dynamic Syntax Calling a non-existant stored procedure asynchronously, expecitng no results (Task)")]
+        Task<Tuple<bool, string>> NonExistant_WithoutResults_Asynchronously(IDbConnection db)
+        {
+            Task res = db.ExecuteAsync(Program.timeout).usp_DoUknownStoredProcedure();
+            return res.ContinueWith(r =>
+            {
+                if (r.Exception == null)
+                    return Tuple.Create(false, "Expected exception to be thrown, because the stored procedure doesn't exist, but none was.");
+
+                return Tuple.Create(true, "");
+            });
+        }
+
+        [SmokeTest("Dynamic Syntax Calling a non-existant stored procedure asynchronously, expecitng results (Await)")]
+        async Task<Tuple<bool, string>> NonExistant_WithResults_Await_Asynchronously(IDbConnection db)
+        {
+            try
+            {
+                IEnumerable<Item> res = await db.ExecuteAsync(Program.timeout).usp_DoUknownStoredProcedure();
+                return Tuple.Create(false, "Expected exception to be thrown, because the stored procedure doesn't exist, but none was.");
+            }
+            catch (AggregateException)
+            {
+                return Tuple.Create(true, "");
+            }
+        }
+
+        [SmokeTest("Dynamic Syntax Calling a non-existant stored procedure asynchronously, expecitng no results (Await)")]
+        async Task<Tuple<bool, string>> NonExistant_WithoutResults_Await_Asynchronously(IDbConnection db)
+        {
+            try
+            {
+                await db.ExecuteAsync(Program.timeout).usp_DoUknownStoredProcedure();
+                return Tuple.Create(false, "Expected exception to be thrown, because the stored procedure doesn't exist, but none was.");
+            }
+            catch (AggregateException)
+            {
+                return Tuple.Create(true, "");
+            }
+        }
+        #endregion
     }
 }

appveyor.yml

@@ -1,4 +1,4 @@
-version: 2.2.4.{build}
+version: 2.2.5.{build}
 skip_tags: false
 
 # Operating system (build VM template)
@@ -10,8 +10,8 @@ branches:
     - gh-pages
 
 environment:
-  releaseVersion: 2.2.4
-  packageVersion: 2.2.4
+  releaseVersion: 2.2.5
+  packageVersion: 2.2.5-pre
 
 assembly_info:
   patch: true