From 046095539bc2ffac64273b20b7767ea18a1949b5 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 17 Mar 2020 07:49:03 -0700 Subject: [PATCH] Disallow setting custom translation for DbFunction mapped to TVFs Resolves #20163 --- .../Metadata/Internal/DbFunction.cs | 11 ++++++ .../Properties/RelationalStrings.Designer.cs | 8 ++++ .../Properties/RelationalStrings.resx | 3 ++ .../Query/UdfDbFunctionTestBase.cs | 28 -------------- .../RelationalModelValidatorTest.cs | 38 +++++++++++++++++++ .../Metadata/DbFunctionMetadataTests.cs | 31 +++++++++++++++ .../Query/UdfDbFunctionSqlServerTests.cs | 9 ----- 7 files changed, 91 insertions(+), 37 deletions(-) diff --git a/src/EFCore.Relational/Metadata/Internal/DbFunction.cs b/src/EFCore.Relational/Metadata/Internal/DbFunction.cs index 563b2cd2cd6..56f3dd5edfc 100644 --- a/src/EFCore.Relational/Metadata/Internal/DbFunction.cs +++ b/src/EFCore.Relational/Metadata/Internal/DbFunction.cs @@ -446,6 +446,17 @@ public virtual Func, SqlExpression> SetTransl [CanBeNull] Func, SqlExpression> translation, ConfigurationSource configurationSource) { + if (translation != null + && IsQueryable) + { + if (configurationSource == ConfigurationSource.Explicit) + { + throw new InvalidOperationException(RelationalStrings.DbFunctionQueryableCustomTranslation(MethodInfo.DisplayName())); + } + + return null; + } + _translation = translation; _translationConfigurationSource = translation == null diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 4ad863c254d..759460ef53d 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -586,6 +586,14 @@ public static string UnknownProjectionMappingType([CanBeNull] object type1, [Can public static string NestedAmbientTransactionError => GetString("NestedAmbientTransactionError"); + /// + /// Cannot set custom translation on the DbFunction '{function}' since it returns IQueryable type. + /// + public static string DbFunctionQueryableCustomTranslation([CanBeNull] object function) + => string.Format( + GetString("DbFunctionQueryableCustomTranslation", nameof(function)), + function); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 3a70d1b3001..2c3ef28ddfa 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -500,4 +500,7 @@ Root ambient transaction was completed before the nested transaction. The more nested transactions should be completed first. + + Cannot set custom translation on the DbFunction '{function}' since it returns IQueryable type. + \ No newline at end of file diff --git a/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs index 9055f34646a..5ffc07b40bc 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs @@ -221,11 +221,6 @@ public IQueryable GetTopSellingProductsForCustomer(int custom return CreateQuery(() => GetTopSellingProductsForCustomer(customerId)); } - public IQueryable GetTopTwoSellingProductsCustomTranslation() - { - return CreateQuery(() => GetTopTwoSellingProductsCustomTranslation()); - } - public IQueryable GetOrdersWithMultipleProducts(int customerId) { return CreateQuery(() => GetOrdersWithMultipleProducts(customerId)); @@ -337,11 +332,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(GetPhoneInformation))); modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(GetOrdersWithMultipleProducts))); - - modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(GetTopTwoSellingProductsCustomTranslation))) -#pragma warning disable CS0618 // Type or member is obsolete - .HasTranslation(args => SqlFunctionExpression.Create("dbo", "GetTopTwoSellingProducts", args, typeof(TopSellingProduct), null)); -#pragma warning restore CS0618 // Type or member is obsolete } } @@ -1456,23 +1446,6 @@ orderby t.ProductId } } - [ConditionalFact(Skip = "Issue#20163")] - public virtual void QF_Stand_Alone_With_Translation() - { - using (var context = CreateContext()) - { - var products = (from t in context.GetTopTwoSellingProductsCustomTranslation() - orderby t.ProductId - select t).ToList(); - - Assert.Equal(2, products.Count); - Assert.Equal(3, products[0].ProductId); - Assert.Equal(249, products[0].AmountSold); - Assert.Equal(4, products[1].ProductId); - Assert.Equal(184, products[1].AmountSold); - } - } - [ConditionalFact] public virtual void QF_Stand_Alone_Parameter() { @@ -2106,7 +2079,6 @@ orderby c.Id #endregion - private void AssertTranslationFailed(Action testCode) => Assert.Contains( CoreStrings.TranslationFailed("").Substring(21), diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs index 4e8f9001968..0a58a859ccf 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs @@ -9,8 +9,10 @@ using Microsoft.EntityFrameworkCore.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.EntityFrameworkCore.TestModels.Inheritance; using Microsoft.EntityFrameworkCore.TestUtilities; using Microsoft.Extensions.Logging; using Xunit; @@ -1012,6 +1014,22 @@ public virtual void Detects_ToTable_on_derived_entity_types() modelBuilder.Model); } + [ConditionalFact] + public void Detects_function_with_empty_name() + { + var modelBuilder = CreateConventionalModelBuilder(); + + var methodInfo + = typeof(DbFunctionMetadataTests.TestMethods) + .GetRuntimeMethod(nameof(DbFunctionMetadataTests.TestMethods.MethodD), Array.Empty()); + + ((IConventionDbFunctionBuilder)modelBuilder.HasDbFunction(methodInfo)).HasName(""); + + VerifyError( + RelationalStrings.DbFunctionNameEmpty(methodInfo.DisplayName()), + modelBuilder.Model); + } + [ConditionalFact] public void Detects_function_with_invalid_return_type() { @@ -1030,6 +1048,26 @@ var methodInfo modelBuilder.Model); } + [ConditionalFact] + public void Detects_function_with_invalid_parameter_type() + { + var modelBuilder = CreateConventionalModelBuilder(); + + var methodInfo + = typeof(DbFunctionMetadataTests.TestMethods) + .GetRuntimeMethod(nameof(DbFunctionMetadataTests.TestMethods.MethodF), + new[] { typeof(DbFunctionMetadataTests.MyBaseContext) }); + + modelBuilder.HasDbFunction(methodInfo); + + VerifyError( + RelationalStrings.DbFunctionInvalidParameterType( + "context", + methodInfo.DisplayName(), + typeof(DbFunctionMetadataTests.MyBaseContext).ShortDisplayName()), + modelBuilder.Model); + } + private static void GenerateMapping(IMutableProperty property) => property[CoreAnnotationNames.TypeMapping] = new TestRelationalTypeMappingSource( diff --git a/test/EFCore.Relational.Tests/Metadata/DbFunctionMetadataTests.cs b/test/EFCore.Relational.Tests/Metadata/DbFunctionMetadataTests.cs index a2055d6c7d3..9ea209c507d 100644 --- a/test/EFCore.Relational.Tests/Metadata/DbFunctionMetadataTests.cs +++ b/test/EFCore.Relational.Tests/Metadata/DbFunctionMetadataTests.cs @@ -8,8 +8,10 @@ using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.TestUtilities; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -711,6 +713,35 @@ public void DbFunction_Annotation_FullName() Assert.NotEqual(funcA.Metadata.Name, funcB.Metadata.Name); } + [ConditionalFact] + public void DbFunction_Queryable_custom_translation() + { + var modelBuilder = GetModelBuilder(); + var methodInfo = typeof(TestMethods).GetMethod(nameof(TestMethods.MethodJ)); + var dbFunctionBuilder = modelBuilder.HasDbFunction(methodInfo); + + ((IConventionDbFunctionBuilder)dbFunctionBuilder).HasTranslation(args => new SqlFragmentExpression("Empty")); + Assert.Null(dbFunctionBuilder.Metadata.Translation); + + ((IConventionDbFunctionBuilder)dbFunctionBuilder) + .HasTranslation(args => new SqlFragmentExpression("Empty"), fromDataAnnotation: true); + Assert.Null(dbFunctionBuilder.Metadata.Translation); + + Assert.Equal(RelationalStrings.DbFunctionQueryableCustomTranslation(methodInfo.DisplayName()), + Assert.Throws( + () => dbFunctionBuilder.HasTranslation(args => new SqlFragmentExpression("Empty"))).Message); + + var dbFunction = dbFunctionBuilder.Metadata; + + Assert.Null(((IConventionDbFunction)dbFunction).SetTranslation(args => new SqlFragmentExpression("Empty"))); + Assert.Null(((IConventionDbFunction)dbFunction) + .SetTranslation(args => new SqlFragmentExpression("Empty"), fromDataAnnotation: true)); + + Assert.Equal(RelationalStrings.DbFunctionQueryableCustomTranslation(methodInfo.DisplayName()), + Assert.Throws( + () => dbFunction.Translation = args => new SqlFragmentExpression("Empty")).Message); + } + private ModelBuilder GetModelBuilder(DbContext dbContext = null) { var conventionSet = new ConventionSet(); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs b/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs index 6a26426f696..eebde15230d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs @@ -455,15 +455,6 @@ FROM [dbo].[GetTopTwoSellingProducts]() AS [t] ORDER BY [t].[ProductId]"); } - public override void QF_Stand_Alone_With_Translation() - { - base.QF_Stand_Alone_With_Translation(); - - AssertSql(@"SELECT [t].[AmountSold], [t].[ProductId] -FROM [dbo].[GetTopTwoSellingProducts]() AS [t] -ORDER BY [t].[ProductId]"); - } - public override void QF_Stand_Alone_Parameter() { base.QF_Stand_Alone_Parameter();