InMemory: Test scrub

Allow ExpressionTranslator to return null Fix bug in OfType Return null for unmapped property to fail gracefully Part of #16963
dotnet · Sep 3, 2019 · 9b5ab67 · 9b5ab67
1 parent 0b64456
commit 9b5ab67
Show file tree

Hide file tree

Showing 21 changed files with 426 additions and 771 deletions.
diff --git a/src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs b/src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs
@@ -23,6 +23,22 @@ public EntityProjectionExpression(
         public override Type Type => EntityType.ClrType;
         public sealed override ExpressionType NodeType => ExpressionType.Extension;
 
+        public virtual EntityProjectionExpression UpdateEntityType(IEntityType derivedType)
+        {
+            var readExpressionMap = new Dictionary<IProperty, Expression>();
+            foreach (var kvp in _readExpressionMap)
+            {
+                var property = kvp.Key;
+                if (derivedType.IsAssignableFrom(property.DeclaringEntityType)
+                    || property.DeclaringEntityType.IsAssignableFrom(derivedType))
+                {
+                    readExpressionMap[property] = kvp.Value;
+                }
+            }
+
+            return new EntityProjectionExpression(derivedType, readExpressionMap);
+        }
+
         public virtual Expression BindProperty(IProperty property)
         {
             if (!EntityType.IsAssignableFrom(property.DeclaringEntityType)

diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs
@@ -66,23 +66,54 @@ public virtual Expression Translate(Expression expression)
                 : result;
         }
 
+        protected override Expression VisitBinary(BinaryExpression binaryExpression)
+        {
+            var left = Visit(binaryExpression.Left);
+            var right = Visit(binaryExpression.Right);
+            if (left == null || right == null)
+            {
+                return null;
+            }
+
+            return binaryExpression.Update(left, binaryExpression.Conversion, right);
+        }
+
+        protected override Expression VisitConditional(ConditionalExpression conditionalExpression)
+        {
+            var test = Visit(conditionalExpression.Test);
+            var ifTrue = Visit(conditionalExpression.IfTrue);
+            var ifFalse = Visit(conditionalExpression.IfFalse);
+            if (test == null || ifTrue == null || ifFalse == null)
+            {
+                return null;
+            }
+
+            return conditionalExpression.Update(test, ifTrue, ifFalse);
+        }
+
         protected override Expression VisitMember(MemberExpression memberExpression)
         {
             var innerExpression = Visit(memberExpression.Expression);
+            if (memberExpression.Expression != null && innerExpression == null)
+            {
+                return null;
+            }
 
-            if (innerExpression is EntityProjectionExpression
+            if ((innerExpression is EntityProjectionExpression
                 || (innerExpression is UnaryExpression innerUnaryExpression
                     && innerUnaryExpression.NodeType == ExpressionType.Convert
                     && innerUnaryExpression.Operand is EntityProjectionExpression))
+                && TryBindMember(innerExpression, MemberIdentity.Create(memberExpression.Member), memberExpression.Type, out var result))
             {
-                return BindProperty(innerExpression, memberExpression.Member.GetSimpleMemberName(), memberExpression.Type);
+                return result;
             }
 
             return memberExpression.Update(innerExpression);
         }
 
-        private Expression BindProperty(Expression source, string propertyName, Type type)
+        private bool TryBindMember(Expression source, MemberIdentity memberIdentity, Type type, out Expression result)
         {
+            result = null;
             Type convertedType = null;
             if (source is UnaryExpression unaryExpression
                 && unaryExpression.NodeType == ExpressionType.Convert)
@@ -105,21 +136,29 @@ private Expression BindProperty(Expression source, string propertyName, Type typ
                         .FirstOrDefault(et => et.ClrType == convertedType);
                     if (entityType == null)
                     {
-                        return null;
+                        return false;
                     }
                 }
 
-                var property = entityType.GetRootType().GetDerivedTypesInclusive()
-                    .Select(et => et.FindProperty(propertyName))
-                    .FirstOrDefault(p => p != null);
+                var property = memberIdentity.MemberInfo != null
+                    ? entityType.FindProperty(memberIdentity.MemberInfo)
+                    : entityType.FindProperty(memberIdentity.Name);
+                // If unmapped property return null
+                if (property == null)
+                {
+                    return false;
+                }
+
+                result = BindProperty(entityProjection, property);
+                if (result.Type != type)
+                {
+                    result = Expression.Convert(result, type);
+                }
 
-                var result = BindProperty(entityProjection, property);
-                return result.Type == type
-                    ? result
-                    : Expression.Convert(result, type);
+                return true;
             }
 
-            throw new InvalidOperationException(CoreStrings.TranslationFailed(source.Print()));
+            return false;
         }
 
         private Expression BindProperty(EntityProjectionExpression entityProjectionExpression, IProperty property)
@@ -132,7 +171,12 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
             // EF.Property case
             if (methodCallExpression.TryGetEFPropertyArguments(out var source, out var propertyName))
             {
-                return BindProperty(Visit(source), propertyName, methodCallExpression.Type);
+                if (TryBindMember(Visit(source), MemberIdentity.Create(propertyName), methodCallExpression.Type, out var result))
+                {
+                    return result;
+                }
+
+                throw new InvalidOperationException("EF.Property called with wrong property name.");
             }
 
             // Subquery case

diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
@@ -379,6 +380,16 @@ protected override ShapedQueryExpression TranslateOfType(ShapedQueryExpression s
                         inMemoryQueryExpression.ServerQueryExpression,
                         predicate);
 
+                    var projectionBindingExpression = (ProjectionBindingExpression)entityShaperExpression.ValueBufferExpression;
+                    var projectionMember = projectionBindingExpression.ProjectionMember;
+                    var entityProjection = (EntityProjectionExpression)inMemoryQueryExpression.GetMappedProjection(projectionMember);
+
+                    inMemoryQueryExpression.ReplaceProjectionMapping(
+                        new Dictionary<ProjectionMember, Expression>
+                        {
+                            { projectionMember, entityProjection.UpdateEntityType(derivedType)}
+                        });
+
                     source.ShaperExpression = entityShaperExpression.WithEntityType(derivedType);
 
                     return source;

diff --git a/...apedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs b/...apedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs
@@ -3,6 +3,8 @@
 
 using System.Collections.Generic;
 using System.Linq.Expressions;
+using System.Reflection;
+using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Query;
 using Microsoft.EntityFrameworkCore.Storage;
@@ -40,6 +42,14 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
                     return Expression.MakeBinary(ExpressionType.Assign, binaryExpression.Left, updatedExpression);
                 }
 
+                if (binaryExpression.NodeType == ExpressionType.Assign
+                    && binaryExpression.Left is MemberExpression memberExpression
+                    && memberExpression.Member is FieldInfo fieldInfo
+                    && fieldInfo.IsInitOnly)
+                {
+                    return memberExpression.Assign(Visit(binaryExpression.Right));
+                }
+
                 return base.VisitBinary(binaryExpression);
             }
 

diff --git a/test/EFCore.InMemory.FunctionalTests/BuiltInDataTypesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/BuiltInDataTypesInMemoryTest.cs
@@ -16,12 +16,6 @@ public BuiltInDataTypesInMemoryTest(BuiltInDataTypesInMemoryFixture fixture)
         {
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
-        public override void Can_insert_and_read_back_with_string_key()
-        {
-            base.Can_insert_and_read_back_with_string_key();
-        }
-
         public class BuiltInDataTypesInMemoryFixture : BuiltInDataTypesFixtureBase
         {
             protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance;

diff --git a/test/EFCore.InMemory.FunctionalTests/ConvertToProviderTypesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/ConvertToProviderTypesInMemoryTest.cs
@@ -15,12 +15,6 @@ public ConvertToProviderTypesInMemoryTest(ConvertToProviderTypesInMemoryFixture
         {
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
-        public override void Can_insert_and_read_back_with_string_key()
-        {
-            base.Can_insert_and_read_back_with_string_key();
-        }
-
         public class ConvertToProviderTypesInMemoryFixture : ConvertToProviderTypesFixtureBase
         {
             protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance;

diff --git a/test/EFCore.InMemory.FunctionalTests/CustomConvertersInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/CustomConvertersInMemoryTest.cs
@@ -19,12 +19,6 @@ public override void Can_insert_and_read_back_with_case_insensitive_string_key()
         {
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
-        public override void Can_insert_and_read_back_with_string_key()
-        {
-            base.Can_insert_and_read_back_with_string_key();
-        }
-
         public class CustomConvertersInMemoryFixture : CustomConvertersFixtureBase
         {
             public override bool StrictEquality => true;

diff --git a/test/EFCore.InMemory.FunctionalTests/FieldMappingInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/FieldMappingInMemoryTest.cs
@@ -14,72 +14,6 @@ public FieldMappingInMemoryTest(FieldMappingInMemoryFixture fixture)
         {
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
-        public override void Field_mapping_with_conversion_does_not_throw()
-        {
-            base.Field_mapping_with_conversion_does_not_throw();
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_auto_props(bool tracking)
-        {
-            base.Include_collection_auto_props(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_fields_only(bool tracking)
-        {
-            base.Include_collection_fields_only(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_fields_only_for_navs_too(bool tracking)
-        {
-            base.Include_collection_fields_only_for_navs_too(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_full_props(bool tracking)
-        {
-            base.Include_collection_full_props(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_full_props_with_named_fields(bool tracking)
-        {
-            base.Include_collection_full_props_with_named_fields(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_hiding_props(bool tracking)
-        {
-            base.Include_collection_hiding_props(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_read_only_props(bool tracking)
-        {
-            base.Include_collection_read_only_props(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_read_only_props_with_named_fields(bool tracking)
-        {
-            base.Include_collection_read_only_props_with_named_fields(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_write_only_props(bool tracking)
-        {
-            base.Include_collection_write_only_props(tracking);
-        }
-
-        [ConditionalTheory(Skip = "Issue#16963")]
-        public override void Include_collection_write_only_props_with_named_fields(bool tracking)
-        {
-            base.Include_collection_write_only_props_with_named_fields(tracking);
-        }
-
         protected override void Update<TBlog>(string navigation)
         {
             base.Update<TBlog>(navigation);

diff --git a/test/EFCore.InMemory.FunctionalTests/InMemoryComplianceTest.cs b/test/EFCore.InMemory.FunctionalTests/InMemoryComplianceTest.cs
@@ -19,7 +19,6 @@ public class InMemoryComplianceTest : ComplianceTestBase
             typeof(ConferencePlannerTestBase<>),
             // Remaining Issue #16963 3.0 query tests:
             typeof(ComplexNavigationsWeakQueryTestBase<>),
-            typeof(FiltersInheritanceTestBase<>),
             typeof(OwnedQueryTestBase<>),
             typeof(GroupByQueryTestBase<>),
             typeof(ComplexNavigationsQueryTestBase<>),
@@ -30,3 +29,4 @@ public class InMemoryComplianceTest : ComplianceTestBase
         protected override Assembly TargetAssembly { get; } = typeof(InMemoryComplianceTest).Assembly;
     }
 }
+
diff --git a/test/EFCore.InMemory.FunctionalTests/LazyLoadProxyInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/LazyLoadProxyInMemoryTest.cs
@@ -14,25 +14,25 @@ public LazyLoadProxyInMemoryTest(LoadInMemoryFixture fixture)
         {
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Lazy_loading_finds_correct_entity_type_with_already_loaded_owned_types()
         {
             base.Lazy_loading_finds_correct_entity_type_with_already_loaded_owned_types();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Lazy_loading_finds_correct_entity_type_with_alternate_model()
         {
             base.Lazy_loading_finds_correct_entity_type_with_alternate_model();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Lazy_loading_finds_correct_entity_type_with_multiple_queries()
         {
             base.Lazy_loading_finds_correct_entity_type_with_multiple_queries();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Lazy_loading_finds_correct_entity_type_with_opaque_predicate_and_multiple_queries()
         {
             base.Lazy_loading_finds_correct_entity_type_with_opaque_predicate_and_multiple_queries();

diff --git a/test/EFCore.InMemory.FunctionalTests/MonsterFixupSnapshotInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/MonsterFixupSnapshotInMemoryTest.cs
@@ -14,49 +14,49 @@ public MonsterFixupSnapshotInMemoryTest(MonsterFixupSnapshotInMemoryFixture fixt
         {
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Can_build_monster_model_and_seed_data_using_all_navigations()
         {
             base.Can_build_monster_model_and_seed_data_using_all_navigations();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Can_build_monster_model_and_seed_data_using_dependent_navigations()
         {
             base.Can_build_monster_model_and_seed_data_using_dependent_navigations();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Can_build_monster_model_and_seed_data_using_FKs()
         {
             base.Can_build_monster_model_and_seed_data_using_FKs();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Can_build_monster_model_and_seed_data_using_navigations_with_deferred_add()
         {
             base.Can_build_monster_model_and_seed_data_using_navigations_with_deferred_add();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Can_build_monster_model_and_seed_data_using_principal_navigations()
         {
             base.Can_build_monster_model_and_seed_data_using_principal_navigations();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void One_to_one_fixup_happens_when_FKs_change_test()
         {
             base.One_to_one_fixup_happens_when_FKs_change_test();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void One_to_one_fixup_happens_when_reference_change_test()
         {
             base.One_to_one_fixup_happens_when_reference_change_test();
         }
 
-        [ConditionalFact(Skip = "Issue#16963")]
+        [ConditionalFact(Skip = "Issue#16963 Owned")]
         public override void Composite_fixup_happens_when_FKs_change_test()
         {
             base.Composite_fixup_happens_when_FKs_change_test();