From 12792244d4060115ae75c8cbd8e14986411cc7a2 Mon Sep 17 00:00:00 2001
From: Smit Patel <smitpatel@users.noreply.github.com>
Date: Thu, 30 Jul 2020 16:22:35 -0700
Subject: [PATCH] ModelValidator: Throw for unmapped initialized collection
 navigation

We were not checking collection navigations which are initialized inline and without setter.
This worked fine for most part as
- One sided collection were always added by convention
- When there were multiple pairs and convention failed, opposite side had reference navigation which would end up throwing
- Many to Many brings a twist that there is no reference on other side so in case we fail to add many to many skip navigations by convention, we would not throw.
---
 src/EFCore/Infrastructure/ModelValidator.cs      | 16 +++++++++++++++-
 .../ModelBuilding/ManyToManyTestBase.cs          | 13 +++++++++++++
 test/EFCore.Tests/ModelBuilding/TestModel.cs     | 14 ++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs
index c39456d0588..5add1be1b55 100644
--- a/src/EFCore/Infrastructure/ModelValidator.cs
+++ b/src/EFCore/Infrastructure/ModelValidator.cs
@@ -29,6 +29,9 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure
     /// </summary>
     public class ModelValidator : IModelValidator
     {
+        private static readonly IEnumerable<string> _dictionaryProperties =
+            typeof(IDictionary<string, object>).GetRuntimeProperties().Select(e => e.Name);
+
         /// <summary>
         ///     Creates a new instance of <see cref="ModelValidator" />.
         /// </summary>
@@ -169,13 +172,17 @@ protected virtual void ValidatePropertyMapping(
 
                 clrProperties.UnionWith(
                     runtimeProperties.Values
-                        .Where(pi => pi.IsCandidateProperty())
+                        .Where(pi => pi.IsCandidateProperty(needsWrite: false))
                         .Select(pi => pi.GetSimpleMemberName()));
 
                 clrProperties.ExceptWith(entityType.GetProperties().Select(p => p.Name));
                 clrProperties.ExceptWith(entityType.GetNavigations().Select(p => p.Name));
                 clrProperties.ExceptWith(entityType.GetSkipNavigations().Select(p => p.Name));
                 clrProperties.ExceptWith(entityType.GetServiceProperties().Select(p => p.Name));
+                if (entityType.IsPropertyBag)
+                {
+                    clrProperties.ExceptWith(_dictionaryProperties);
+                }
                 clrProperties.RemoveWhere(p => entityType.FindIgnoredConfigurationSource(p) != null);
 
                 if (clrProperties.Count <= 0)
@@ -205,6 +212,13 @@ protected virtual void ValidatePropertyMapping(
 
                     var targetType = FindCandidateNavigationPropertyType(actualProperty);
 
+                    if ((targetType == null // Not a navigation
+                        || targetSequenceType == null) // Not a collection navigation
+                        && actualProperty.FindSetterProperty() == null) // No setter property
+                    {
+                        continue;
+                    }
+
                     var isTargetWeakOrOwned
                         = targetType != null
                         && (conventionModel.HasEntityTypeWithDefiningNavigation(targetType)
diff --git a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs
index 1ea140efe36..1147dc424e2 100644
--- a/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs
+++ b/test/EFCore.Tests/ModelBuilding/ManyToManyTestBase.cs
@@ -480,6 +480,19 @@ public virtual void UsingEntity_with_shared_type_passes_when_configured_as_share
                 Assert.Equal("Shared", joinEntityType.Name);
                 Assert.Equal(typeof(ManyToManyJoinWithFields), joinEntityType.ClrType);
             }
+
+            [ConditionalFact]
+            public virtual void Unconfigured_many_to_many_navigations_throw()
+            {
+                var modelBuilder = CreateModelBuilder();
+
+                modelBuilder.Entity<AmbigiousManyToManyImplicitLeft>();
+
+                Assert.Equal(
+                    CoreStrings.NavigationNotAdded(typeof(AmbigiousManyToManyImplicitLeft).DisplayName(fullName: false), "Navigation1",
+                    typeof(List<AmbigiousManyToManyImplicitRight>).DisplayName(fullName: false)),
+                    Assert.Throws<InvalidOperationException>(() => modelBuilder.FinalizeModel()).Message);
+            }
         }
     }
 }
diff --git a/test/EFCore.Tests/ModelBuilding/TestModel.cs b/test/EFCore.Tests/ModelBuilding/TestModel.cs
index a1d5108a91c..c775958543c 100644
--- a/test/EFCore.Tests/ModelBuilding/TestModel.cs
+++ b/test/EFCore.Tests/ModelBuilding/TestModel.cs
@@ -1106,5 +1106,19 @@ protected class CollectionNavigationToSharedType
             public int Id { get; set; }
             public List<Dictionary<string, object>> Navigation { get; set; }
         }
+
+        protected class AmbigiousManyToManyImplicitLeft
+        {
+            public int Id { get; set; }
+            public List<AmbigiousManyToManyImplicitRight> Navigation1 { get; } = new List<AmbigiousManyToManyImplicitRight>();
+            public List<AmbigiousManyToManyImplicitRight> Navigation2 { get; } = new List<AmbigiousManyToManyImplicitRight>();
+        }
+
+        protected class AmbigiousManyToManyImplicitRight
+        {
+            public int Id { get; set; }
+            public List<AmbigiousManyToManyImplicitLeft> Navigation1 { get; } = new List<AmbigiousManyToManyImplicitLeft>();
+            public List<AmbigiousManyToManyImplicitLeft> Navigation2 { get; } = new List<AmbigiousManyToManyImplicitLeft>();
+        }
     }
 }