Disallow setting custom translation for DbFunction mapped to TVFs

Resolves #20163
dotnet · Mar 17, 2020 · 0460955 · 0460955
1 parent 0573af8
commit 0460955
Show file tree

Hide file tree

Showing 7 changed files with 91 additions and 37 deletions.
diff --git a/src/EFCore.Relational/Metadata/Internal/DbFunction.cs b/src/EFCore.Relational/Metadata/Internal/DbFunction.cs
@@ -446,6 +446,17 @@ public virtual Func<IReadOnlyCollection<SqlExpression>, SqlExpression> SetTransl
             [CanBeNull] Func<IReadOnlyCollection<SqlExpression>, 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
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx
@@ -500,4 +500,7 @@
   <data name="NestedAmbientTransactionError" xml:space="preserve">
     <value>Root ambient transaction was completed before the nested transaction. The more nested transactions should be completed first.</value>
   </data>
+  <data name="DbFunctionQueryableCustomTranslation" xml:space="preserve">
+    <value>Cannot set custom translation on the DbFunction '{function}' since it returns IQueryable type.</value>
+  </data>
 </root>
diff --git a/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs
@@ -221,11 +221,6 @@ public IQueryable<TopSellingProduct> GetTopSellingProductsForCustomer(int custom
                 return CreateQuery(() => GetTopSellingProductsForCustomer(customerId));
             }
 
-            public IQueryable<TopSellingProduct> GetTopTwoSellingProductsCustomTranslation()
-            {
-                return CreateQuery(() => GetTopTwoSellingProductsCustomTranslation());
-            }
-
             public IQueryable<MultProductOrders> 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
@@ -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<Type>());
+
+            ((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
@@ -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<InvalidOperationException>(
+                    () => 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<InvalidOperationException>(
+                    () => 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
@@ -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();