From afacdc12ad15cb4272c4db5229152a0ed4951858 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 24 Sep 2014 16:59:45 +1000 Subject: [PATCH] More work on parameterized queries, all tests for repos passing now. --- .../Querying/BaseExpressionHelper.cs | 128 ++++++++++-------- .../Querying/ModelToSqlExpressionHelper.cs | 2 +- .../Querying/PocoToSqlExpressionHelper.cs | 2 +- .../SqlSyntax/SqlSyntaxProviderBase.cs | 10 +- .../Persistence/Querying/PetaPocoSqlTests.cs | 47 +++++-- .../DataTypeDefinitionRepositoryTest.cs | 2 +- 6 files changed, 118 insertions(+), 73 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs index 394f6efa81..f75c9c0c42 100644 --- a/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs @@ -187,12 +187,13 @@ namespace Umbraco.Core.Persistence.Querying object o = getter(); SqlParameters.Add(o); - return string.Format("(@{0})", SqlParameters.Count - 1); + return string.Format("@{0}", SqlParameters.Count - 1); //return GetQuotedValue(o, o.GetType()); } catch (InvalidOperationException) - { // FieldName ? + { + // FieldName ? List exprs = VisitExpressionList(nex.Arguments); var r = new StringBuilder(); foreach (Object e in exprs) @@ -217,7 +218,7 @@ namespace Umbraco.Core.Persistence.Querying return "null"; SqlParameters.Add(c.Value); - return string.Format("(@{0})", SqlParameters.Count - 1); + return string.Format("@{0}", SqlParameters.Count - 1); //if (c.Value is bool) //{ @@ -306,27 +307,29 @@ namespace Umbraco.Core.Persistence.Querying protected virtual string VisitMethodCall(MethodCallExpression m) { - List args = this.VisitExpressionList(m.Arguments); - - Object r; - if (m.Object != null) - r = Visit(m.Object); - else - { - r = args[0]; - args.RemoveAt(0); - } - - //TODO: We should probably add the same logic we've done for ModelToSqlExpressionHelper with checking for: - // InvariantStartsWith, InvariantEndsWith, SqlWildcard, etc... - // since we should be able to easily handle that with the Poco objects too. + //List args = this.VisitExpressionList(m.Arguments); + //Object r; + //if (m.Object != null) + //{ + // r = Visit(m.Object); + //} + //else + //{ + // //TODO: I have no idea what this does and if it's ever used. + // var args = VisitExpressionList(m.Arguments); + // r = args[0]; + // args.RemoveAt(0); + //} switch (m.Method.Name) { + case "ToString": + SqlParameters.Add(m.Object.ToString()); + return string.Format("@{0}", SqlParameters.Count - 1); case "ToUpper": - return string.Format("upper({0})", r); + return string.Format("upper({0})", Visit(m.Object)); case "ToLower": - return string.Format("lower({0})", r); + return string.Format("lower({0})", Visit(m.Object)); case "SqlWildcard": case "StartsWith": case "EndsWith": @@ -341,39 +344,44 @@ namespace Umbraco.Core.Persistence.Querying case "InvariantContains": case "InvariantEquals": - //default + var r = Visit(m.Object); + + //special case, if it is 'Contains' and the member that Contains is being called on is not a string, then + // we should be doing an 'In' clause - but we currently do not support this + if (m.Arguments[0].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(m.Arguments[0].Type)) + { + throw new NotSupportedException("An array Contains method is not supported"); + } + + //default column type var colType = TextColumnType.NVarchar; - //then check if this arg has been passed in + + //then check if the col type argument has been passed to the current method (this will be the case for methods like + // SqlContains and other Sql methods) if (m.Arguments.Count > 1) { - //special case, if it is 'Contains' and the member that Contains is being called on is not a string, then - // we should be doing an 'In' clause - but we currently do not support this, nor is it very necessary - if (TypeHelper.IsTypeAssignableFrom(m.Arguments[0].Type)) - { - throw new NotSupportedException("An array Contains method is not supported"); - } - var colTypeArg = m.Arguments.FirstOrDefault(x => x is ConstantExpression && x.Type == typeof(TextColumnType)); if (colTypeArg != null) { colType = (TextColumnType)((ConstantExpression)colTypeArg).Value; } } - return HandleStringComparison(r.ToString(), args[0].ToString(), m.Method.Name, colType); - case "Substring": - var startIndex = Int32.Parse(args[0].ToString()) + 1; - if (args.Count == 2) - { - var length = Int32.Parse(args[1].ToString()); - return string.Format("substring({0} from {1} for {2})", - r, - startIndex, - length); - } - else - return string.Format("substring({0} from {1})", - r, - startIndex); + + return HandleStringComparison(r, m.Arguments[0].ToString(), m.Method.Name, colType); + //case "Substring": + // var startIndex = Int32.Parse(args[0].ToString()) + 1; + // if (args.Count == 2) + // { + // var length = Int32.Parse(args[1].ToString()); + // return string.Format("substring({0} from {1} for {2})", + // r, + // startIndex, + // length); + // } + // else + // return string.Format("substring({0} from {1})", + // r, + // startIndex); //case "Round": //case "Floor": //case "Ceiling": @@ -421,8 +429,7 @@ namespace Umbraco.Core.Persistence.Querying //case "As": // return string.Format("{0} As {1}", r, // GetQuotedColumnName(RemoveQuoteFromAlias(RemoveQuote(args[0].ToString())))); - case "ToString": - return r.ToString(); + default: throw new ArgumentOutOfRangeException("No logic supported for " + m.Method.Name); @@ -494,7 +501,7 @@ namespace Umbraco.Core.Persistence.Querying /// /// Logic that is shared with the expression helpers /// - internal class BaseExpressionHelper + internal class BaseExpressionHelper { protected List SqlParameters = new List(); @@ -612,16 +619,12 @@ namespace Umbraco.Core.Persistence.Querying protected virtual string RemoveQuote(string exp) { - if (exp.StartsWith("'") && exp.EndsWith("'")) - { - exp = exp.Remove(0, 1); - exp = exp.Remove(exp.Length - 1, 1); - } - return exp; - } - - protected virtual string RemoveQuoteFromAlias(string exp) - { + //if (exp.StartsWith("'") && exp.EndsWith("'")) + //{ + // exp = exp.Remove(0, 1); + // exp = exp.Remove(exp.Length - 1, 1); + //} + //return exp; if ((exp.StartsWith("\"") || exp.StartsWith("`") || exp.StartsWith("'")) && @@ -632,5 +635,18 @@ namespace Umbraco.Core.Persistence.Querying } return exp; } + + //protected virtual string RemoveQuoteFromAlias(string exp) + //{ + + // if ((exp.StartsWith("\"") || exp.StartsWith("`") || exp.StartsWith("'")) + // && + // (exp.EndsWith("\"") || exp.EndsWith("`") || exp.EndsWith("'"))) + // { + // exp = exp.Remove(0, 1); + // exp = exp.Remove(exp.Length - 1, 1); + // } + // return exp; + //} } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs index addb13df01..6566d7ed25 100644 --- a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs @@ -42,7 +42,7 @@ namespace Umbraco.Core.Persistence.Querying object o = getter(); SqlParameters.Add(o); - return string.Format("(@{0})", SqlParameters.Count - 1); + return string.Format("@{0}", SqlParameters.Count - 1); //return GetQuotedValue(o, o != null ? o.GetType() : null); diff --git a/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs index b1ab50b7c3..5caa3a89ab 100644 --- a/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs @@ -40,7 +40,7 @@ namespace Umbraco.Core.Persistence.Querying object o = getter(); SqlParameters.Add(o); - return string.Format("(@{0})", SqlParameters.Count - 1); + return string.Format("@{0}", SqlParameters.Count - 1); //return GetQuotedValue(o, o != null ? o.GetType() : null); diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs b/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs index f4935bf675..08d85e0573 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs @@ -122,7 +122,7 @@ namespace Umbraco.Core.Persistence.SqlSyntax public virtual string GetStringColumnWildcardComparison(string column, int paramIndex, TextColumnType columnType) { //use the 'upper' method to always ensure strings are matched without case sensitivity no matter what the db setting. - return string.Format("upper({0}) like upper(@{1})", column, paramIndex); + return string.Format("upper({0}) LIKE upper(@{1})", column, paramIndex); } [Obsolete("Use the overload with the parameter index instead")] @@ -136,28 +136,28 @@ namespace Umbraco.Core.Persistence.SqlSyntax public virtual string GetStringColumnStartsWithComparison(string column, string value, TextColumnType columnType) { //use the 'upper' method to always ensure strings are matched without case sensitivity no matter what the db setting. - return string.Format("upper({0}) like '{1}%'", column, value.ToUpper()); + return string.Format("upper({0}) LIKE '{1}%'", column, value.ToUpper()); } [Obsolete("Use the overload with the parameter index instead")] public virtual string GetStringColumnEndsWithComparison(string column, string value, TextColumnType columnType) { //use the 'upper' method to always ensure strings are matched without case sensitivity no matter what the db setting. - return string.Format("upper({0}) like '%{1}'", column, value.ToUpper()); + return string.Format("upper({0}) LIKE '%{1}'", column, value.ToUpper()); } [Obsolete("Use the overload with the parameter index instead")] public virtual string GetStringColumnContainsComparison(string column, string value, TextColumnType columnType) { //use the 'upper' method to always ensure strings are matched without case sensitivity no matter what the db setting. - return string.Format("upper({0}) like '%{1}%'", column, value.ToUpper()); + return string.Format("upper({0}) LIKE '%{1}%'", column, value.ToUpper()); } [Obsolete("Use the overload with the parameter index instead")] public virtual string GetStringColumnWildcardComparison(string column, string value, TextColumnType columnType) { //use the 'upper' method to always ensure strings are matched without case sensitivity no matter what the db setting. - return string.Format("upper({0}) like '{1}'", column, value.ToUpper()); + return string.Format("upper({0}) LIKE '{1}'", column, value.ToUpper()); } public virtual string GetQuotedTableName(string tableName) diff --git a/src/Umbraco.Tests/Persistence/Querying/PetaPocoSqlTests.cs b/src/Umbraco.Tests/Persistence/Querying/PetaPocoSqlTests.cs index 2b16b9e1d9..92fab2b59b 100644 --- a/src/Umbraco.Tests/Persistence/Querying/PetaPocoSqlTests.cs +++ b/src/Umbraco.Tests/Persistence/Querying/PetaPocoSqlTests.cs @@ -14,34 +14,63 @@ namespace Umbraco.Tests.Persistence.Querying public class PetaPocoSqlTests : BaseUsingSqlCeSyntax { + [Test] + public void Where_Clause_With_ToUpper() + { + var sql = new Sql("SELECT *").From().Where(x => x.Text.ToUpper() == "hello".ToUpper()); + + Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE (upper([umbracoNode].[text]) = upper(@0))", sql.SQL.Replace("\n", " ")); + Assert.AreEqual(1, sql.Arguments.Length); + Assert.AreEqual("hello", sql.Arguments[0]); + } [Test] - public void Generates_Sql_Parameter_Where_Clause_Single_Constant() + public void Where_Clause_With_ToString() + { + var sql = new Sql("SELECT *").From().Where(x => x.Text == 1.ToString()); + + Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[text] = @0)", sql.SQL.Replace("\n", " ")); + Assert.AreEqual(1, sql.Arguments.Length); + Assert.AreEqual("1", sql.Arguments[0]); + } + + [Test] + public void Where_Clause_With_Wildcard() + { + var sql = new Sql("SELECT *").From().Where(x => x.Text.StartsWith("D")); + + Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE (upper([umbracoNode].[text]) LIKE upper(@0))", sql.SQL.Replace("\n", " ")); + Assert.AreEqual(1, sql.Arguments.Length); + Assert.AreEqual("D%", sql.Arguments[0]); + } + + [Test] + public void Where_Clause_Single_Constant() { var sql = new Sql("SELECT *").From().Where(x => x.NodeId == 2); - Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[id] = (@0))", sql.SQL.Replace("\n", " ")); + Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[id] = @0)", sql.SQL.Replace("\n", " ")); Assert.AreEqual(1, sql.Arguments.Length); Assert.AreEqual(2, sql.Arguments[0]); } [Test] - public void Generates_Sql_Parameter_Where_Clause_And_Constant() + public void Where_Clause_And_Constant() { var sql = new Sql("SELECT *").From().Where(x => x.NodeId != 2 && x.NodeId != 3); - Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[id] <> (@0) AND [umbracoNode].[id] <> (@1))", sql.SQL.Replace("\n", " ")); + Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[id] <> @0 AND [umbracoNode].[id] <> @1)", sql.SQL.Replace("\n", " ")); Assert.AreEqual(2, sql.Arguments.Length); Assert.AreEqual(2, sql.Arguments[0]); Assert.AreEqual(3, sql.Arguments[1]); } [Test] - public void Generates_Sql_Parameter_Where_Clause_Or_Constant() + public void Where_Clause_Or_Constant() { var sql = new Sql("SELECT *").From().Where(x => x.Text == "hello" || x.NodeId == 3); - Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[text] = (@0) OR [umbracoNode].[id] = (@1))", sql.SQL.Replace("\n", " ")); + Assert.AreEqual("SELECT * FROM [umbracoNode] WHERE ([umbracoNode].[text] = @0 OR [umbracoNode].[id] = @1)", sql.SQL.Replace("\n", " ")); Assert.AreEqual(2, sql.Arguments.Length); Assert.AreEqual("hello", sql.Arguments[0]); Assert.AreEqual(3, sql.Arguments[1]); @@ -112,7 +141,7 @@ namespace Umbraco.Tests.Persistence.Querying public void Can_Use_Where_Predicate() { var expected = new Sql(); - expected.Select("*").From("[cmsContent]").Where("[cmsContent].[nodeId] = (@0)", 1045); + expected.Select("*").From("[cmsContent]").Where("[cmsContent].[nodeId] = @0", 1045); var sql = new Sql(); sql.Select("*").From().Where(x => x.NodeId == 1045); @@ -128,8 +157,8 @@ namespace Umbraco.Tests.Persistence.Querying var expected = new Sql(); expected.Select("*") .From("[cmsContent]") - .Where("[cmsContent].[nodeId] = (@0)", 1045) - .Where("[cmsContent].[contentType] = (@0)", 1050); + .Where("[cmsContent].[nodeId] = @0", 1045) + .Where("[cmsContent].[contentType] = @0", 1050); var sql = new Sql(); sql.Select("*") diff --git a/src/Umbraco.Tests/Persistence/Repositories/DataTypeDefinitionRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DataTypeDefinitionRepositoryTest.cs index e59945ac91..fbf7f53414 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/DataTypeDefinitionRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DataTypeDefinitionRepositoryTest.cs @@ -135,7 +135,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(dataTypeDefinitions, Is.Not.Null); Assert.That(dataTypeDefinitions.Any(), Is.True); Assert.That(dataTypeDefinitions.Any(x => x == null), Is.False); - Assert.That(dataTypeDefinitions.Count(), Is.EqualTo(21)); + Assert.That(dataTypeDefinitions.Count(), Is.EqualTo(25)); } }