From 0cdf59d9297a37b9e511b01c830451f7afc95231 Mon Sep 17 00:00:00 2001 From: Abraham Heidebrecht Date: Wed, 3 Feb 2016 00:32:00 -0500 Subject: [PATCH 1/6] Added code to throw exceptions from dynamic results before the dynamic is cast to a result value. Fixes #59 --- .../Dynamic/DynamicStoredProcedureResults.cs | 18 ++-- .../DynamicStoredProcedureResultsAwaiter.cs | 6 ++ SmokeTests/DynamicSyntax.cs | 85 +++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs b/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs index 2360ad3..d622efa 100644 --- a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs +++ b/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs @@ -30,7 +30,7 @@ internal class DynamicStoredProcedureResults : DynamicObject, IDisposable private readonly IEnumerable 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,7 +129,12 @@ private IEnumerable GetResults(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> CreateSingleContinuation() diff --git a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResultsAwaiter.cs b/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResultsAwaiter.cs index 3374d19..79c8583 100644 --- a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResultsAwaiter.cs +++ b/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; } diff --git a/SmokeTests/DynamicSyntax.cs b/SmokeTests/DynamicSyntax.cs index 1a1faa7..c358f6d 100644 --- a/SmokeTests/DynamicSyntax.cs +++ b/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 NonExistant_WithResults_Synchronously(IDbConnection db) + { + try + { + IEnumerable 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 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> NonExistant_WithResults_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 no results (Task)")] + Task> 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> NonExistant_WithResults_Await_Asynchronously(IDbConnection db) + { + try + { + IEnumerable 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> 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 } } From a8593db0d2e37f2b9e84fb1c1cadd6d0cca83392 Mon Sep 17 00:00:00 2001 From: Abraham Heidebrecht Date: Wed, 3 Feb 2016 00:53:11 -0500 Subject: [PATCH 2/6] Added friendlier exception to the dynamic syntax when the programmer fails to pass the arguments by name. Fixes #56 --- .../Dynamic/DynamicStoredProcedure.cs | 20 +++++++++++++++++++ .../Dynamic/DynamicStoredProcedureTests.cs | 15 ++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedure.cs b/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedure.cs index c493d0e..40ec7d5 100644 --- a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedure.cs +++ b/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 getParameterName; private static readonly Func getParameterDirection; private static readonly Func 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())); } diff --git a/CodeOnlyTests/Dynamic/DynamicStoredProcedureTests.cs b/CodeOnlyTests/Dynamic/DynamicStoredProcedureTests.cs index 43bccd8..12082ae 100644 --- a/CodeOnlyTests/Dynamic/DynamicStoredProcedureTests.cs +++ b/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] From d2da12330498dae1d0fa66236f0e55ef6476fcde Mon Sep 17 00:00:00 2001 From: Abraham Heidebrecht Date: Wed, 3 Feb 2016 01:05:18 -0500 Subject: [PATCH 3/6] Bumped release # --- appveyor.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index b1965cc..b31750a 100644 --- a/appveyor.yml +++ b/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 From 6b0e813b28a144b5028bd14716cd41e525ad32a1 Mon Sep 17 00:00:00 2001 From: Abraham Heidebrecht Date: Sat, 6 Feb 2016 21:26:54 -0500 Subject: [PATCH 4/6] Updated so Dispose always gets called (less chance of leaking database connections) --- .../Dynamic/DynamicStoredProcedureResults.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs b/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs index d622efa..6e9fba9 100644 --- a/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs +++ b/CodeOnlyStoredProcedure/Dynamic/DynamicStoredProcedureResults.cs @@ -141,9 +141,8 @@ private Task> CreateSingleContinuation() { return resultTask.ContinueWith(_ => { - var res = GetResults(true); - Dispose(); - return res; + try { return GetResults(true); } + finally { Dispose(); } }, token); } @@ -151,9 +150,8 @@ private Task CreateSingleRowContinuation() { return resultTask.ContinueWith(_ => { - var res = GetResults(true).SingleOrDefault(); - Dispose(); - return res; + try { return GetResults(true).SingleOrDefault(); } + finally { Dispose(); } }, token); } @@ -185,9 +183,8 @@ private Task CreateMultipleContinuation() return resultTask.ContinueWith(_ => { - var res = GetMultipleResults(); - Dispose(); - return res; + try { return GetMultipleResults(); } + finally { Dispose(); } }, token); } From 2465d4ef72e9c6cd2fff404e11ee7ff4ebfdbb6e Mon Sep 17 00:00:00 2001 From: Abraham Heidebrecht Date: Sat, 6 Feb 2016 21:45:02 -0500 Subject: [PATCH 5/6] Fixed bug where ExpandoObjectRowFactory would return DBNull instead of null. Fixes #57 --- .../RowFactory/ExpandoObjectRowFactory.cs | 6 ++++- .../ExpandoObjectRowFactoryTests.cs | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CodeOnlyStoredProcedure/RowFactory/ExpandoObjectRowFactory.cs b/CodeOnlyStoredProcedure/RowFactory/ExpandoObjectRowFactory.cs index 91fc890..ae0156c 100644 --- a/CodeOnlyStoredProcedure/RowFactory/ExpandoObjectRowFactory.cs +++ b/CodeOnlyStoredProcedure/RowFactory/ExpandoObjectRowFactory.cs @@ -11,6 +11,7 @@ namespace CodeOnlyStoredProcedure.RowFactory internal class ExpandoObjectRowFactory : RowFactory { static MethodInfo addToDictionaryMethod = typeof(IDictionary).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 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()) { diff --git a/CodeOnlyTests/RowFactory/ExpandoObjectRowFactoryTests.cs b/CodeOnlyTests/RowFactory/ExpandoObjectRowFactoryTests.cs index 75288be..85e09da 100644 --- a/CodeOnlyTests/RowFactory/ExpandoObjectRowFactoryTests.cs +++ b/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(); + 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())) + .Callback(os => os[0] = DBNull.Value); + rdr.SetupSequence(r => r.Read()) + .Returns(true) + .Returns(false); + + var toTest = new ExpandoObjectRowFactory(); + 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] From 22df3c81fd593526910125c7b08147509a25c233 Mon Sep 17 00:00:00 2001 From: Abraham Heidebrecht Date: Sat, 6 Feb 2016 22:16:28 -0500 Subject: [PATCH 6/6] Updated release notes for 2.2.5 --- CodeOnlyStoredProcedures.nuspec | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CodeOnlyStoredProcedures.nuspec b/CodeOnlyStoredProcedures.nuspec index 90c585a..2ee8657 100644 --- a/CodeOnlyStoredProcedures.nuspec +++ b/CodeOnlyStoredProcedures.nuspec @@ -9,7 +9,12 @@ false 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. - 2.2.4 + 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<dynamic> (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<interface> 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. +Added better exception when a model is missing a public parameterless constructor. StoredProcedure EntityFramework EF