Updates to shared-type entity type handling in proxies #22338

ajcvickers · 2020-08-31T21:42:35Z

Check for virtual indexer properties and throw unless type is Dictionary<string, _> and has only primary keys
Specific exception for the Dictionary case
Verified many-to-many with payload requires this pattern and works

See also #22336

ajcvickers · 2020-08-31T21:42:59Z

AndriySvyryd · 2020-08-31T22:06:05Z

src/EFCore/Metadata/Internal/TypeBase.cs

+
+            var isPropertyBag = type.IsPropertyBagType();
+            IsPropertyBag = isPropertyBag;
+            HasSharedClrType = isPropertyBag;


Cannot be shared, no name was provided

Without this any navigations to the shared type cause it to be discovered as non-shared and hence the many-to-many tests for this will fail. Whatever it is that makes Dictionary work in this case but prevents any other types from working, even if they match the IsPropertyBagType call, needs to be fixed.

Then it's a bug in relationship discovery. Shared types shouldn't be discovered in general.

But there is no way to know it's a shared type in relationship discovery, so currently relationship discovery only ignores Dictionary, and there isn't any way to make it ignore any other type.

IConventionModel.IsShared can be used for this

How? The first line of OnModelCreating maps the type as a shared type. That's too late, since relationship discovery has already marked it as not-shared and so this throws.

Another way of saying this is that this "Add the specific types used in the tests as shared types in the tests." is what I am trying to make work. If this worked, then I wouldn't be making any product changes in this area.

That's too late, since relationship discovery has already marked it as not-shared and so this throws.

That should not happen. Relationship discover discovers by convention configuration source. Fluent API should override that.

Okay, then what can be done to fix this? It doesn't currently seem like there is any way to mark an entity type as shared once it has already been marked as non-shared, regardless of configuration source.

modelBuilder.SharedTypeEntity<MyDict>("SharedDict")

Pilchie · 2020-09-01T14:51:15Z

Approved for RC1 pending code review signoff if merged before 10am Pacific on 2020-09-01.

Fixes #22337 * Check for virtual indexer properties and throw unless type is Dictionary<string, _> and has only primary keys * Specific exception for the Dictionary case * Verified many-to-many with payload requires this pattern and works See also #22336

…bag type

ajcvickers · 2020-09-01T16:27:58Z

test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs

@@ -152,6 +152,8 @@ public ISetSource GetExpectedData()

        protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
        {
+            modelBuilder.SharedTypeEntity<ProxyablePropertyBag>("JoinOneToThreePayloadFullShared");


@AndriySvyryd This throws.

Then that's a bug

Exactly! That's what I'm trying to fix.

Ok, the fix for this should be made in InternalModelBuilder.Entity

Too late for RC1 now. Will get back to it another time.

Note if you mean, in UsingEntity, then doing that without this also throws saying that the type is not configured as a shared-type, which must be explicitly done before calling UsingEntity.

Dismiss my previous comment. What is the exception that you get without the IsShared change? (Including stacktrace)

The shared type entity type 'JoinOneToThreePayloadFullShared' with clr type 'Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel.ProxyablePropertyBag' cannot be added to the model because a non shared entity type with the same clr type already exists.

System.AggregateException : One or more errors occurred. (The shared type entity type 'JoinOneToThreePayloadFullShared' with clr type 'Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel.ProxyablePropertyBag' cannot be added to the model because a non shared entity type with the same clr type already exists.) (The following constructor parameters did not have matching fixture data: ManyToManyTrackingProxySqlServerFixture fixture) ---- System.InvalidOperationException : The shared type entity type 'JoinOneToThreePayloadFullShared' with clr type 'Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel.ProxyablePropertyBag' cannot be added to the model because a non shared entity type with the same clr type already exists. ---- The following constructor parameters did not have matching fixture data: ManyToManyTrackingProxySqlServerFixture fixture ----- Inner Stack Trace #1 (System.InvalidOperationException) ----- at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.AddEntityType(EntityType entityType) in /home/ajcvickers/efcore/src/EFCore/Metadata/Internal/Model.cs:line 247 at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.AddEntityType(String name, Type type, ConfigurationSource configurationSource) in /home/ajcvickers/efcore/src/EFCore/Metadata/Internal/Model.cs:line 193 at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilder.Entity(TypeIdentity& type, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned) in /home/ajcvickers/efcore/src/EFCore/Metadata/Internal/InternalModelBuilder.cs:line 185 at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilder.SharedTypeEntity(String name, Type type, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned) in /home/ajcvickers/efcore/src/EFCore/Metadata/Internal/InternalModelBuilder.cs:line 67 at Microsoft.EntityFrameworkCore.ModelBuilder.SharedTypeEntity[TEntity](String name) in /home/ajcvickers/efcore/src/EFCore/ModelBuilder.cs:line 158 at Microsoft.EntityFrameworkCore.Query.ManyToManyQueryFixtureBase.OnModelCreating(ModelBuilder modelBuilder, DbContext context) in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs:line 155 at Microsoft.EntityFrameworkCore.ManyToManyTrackingSqlServerTestBase`1.ManyToManyTrackingSqlServerFixtureBase.OnModelCreating(ModelBuilder modelBuilder, DbContext context) in /home/ajcvickers/efcore/test/EFCore.SqlServer.FunctionalTests/ManyToManyTrackingSqlServerTestBase.cs:line 30 at Microsoft.EntityFrameworkCore.TestUtilities.TestModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, ModelDependencies modelDependencies) in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/TestUtilities/TestModelSource.cs:line 31 at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder, ModelDependencies modelDependencies) in /home/ajcvickers/efcore/src/EFCore/Infrastructure/ModelSource.cs:line 100 at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel() in /home/ajcvickers/efcore/src/EFCore/Internal/DbContextServices.cs:line 84 at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model() in /home/ajcvickers/efcore/src/EFCore/Internal/DbContextServices.cs:line 111 at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_3(IServiceProvider p) in /home/ajcvickers/efcore/src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs:line 255 at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, RuntimeResolverContext context) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScopeCache(ServiceCallSite singletonCallSite, RuntimeResolverContext context) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, RuntimeResolverContext context) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScopeCache(ServiceCallSite singletonCallSite, RuntimeResolverContext context) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument) at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite callSite, ServiceProviderEngineScope scope) at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0(ServiceProviderEngineScope scope) at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope) at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType) at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType) at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider) at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies() in /home/ajcvickers/efcore/src/EFCore/DbContext.cs:line 394 at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider() in /home/ajcvickers/efcore/src/EFCore/DbContext.cs:line 376 at Microsoft.EntityFrameworkCore.DbContext.get_ChangeTracker() in /home/ajcvickers/efcore/src/EFCore/DbContext.cs:line 134 at Microsoft.EntityFrameworkCore.ManyToManyTrackingTestBase`1.ManyToManyTrackingFixtureBase.CreateContext() in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs:line 3121 at Microsoft.EntityFrameworkCore.TestUtilities.SqlServerTestStore.Initialize(Func`1 createContext, Action`1 seed, Action`1 clean) in /home/ajcvickers/efcore/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerTestStore.cs:line 93 at Microsoft.EntityFrameworkCore.TestUtilities.TestStore.<>c__DisplayClass13_0.<Initialize>b__0() in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/TestUtilities/TestStore.cs:line 41 at Microsoft.EntityFrameworkCore.TestUtilities.TestStoreIndex.CreateShared(String name, Action initializeDatabase) in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/TestUtilities/TestStoreIndex.cs:line 30 at Microsoft.EntityFrameworkCore.TestUtilities.TestStore.Initialize(IServiceProvider serviceProvider, Func`1 createContext, Action`1 seed, Action`1 clean) in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/TestUtilities/TestStore.cs:line 41 at Microsoft.EntityFrameworkCore.TestUtilities.RelationalTestStore.Initialize(IServiceProvider serviceProvider, Func`1 createContext, Action`1 seed, Action`1 clean) in /home/ajcvickers/efcore/test/EFCore.Relational.Specification.Tests/TestUtilities/RelationalTestStore.cs:line 43 at Microsoft.EntityFrameworkCore.SharedStoreFixtureBase`1..ctor() in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/SharedStoreFixtureBase.cs:line 58 at Microsoft.EntityFrameworkCore.Query.ManyToManyQueryFixtureBase..ctor() in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/Query/ManyToManyQueryFixtureBase.cs:line 15 at Microsoft.EntityFrameworkCore.ManyToManyTrackingTestBase`1.ManyToManyTrackingFixtureBase..ctor() in /home/ajcvickers/efcore/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs:line 3126 at Microsoft.EntityFrameworkCore.ManyToManyTrackingSqlServerTestBase`1.ManyToManyTrackingSqlServerFixtureBase..ctor() at Microsoft.EntityFrameworkCore.ManyToManyTrackingProxySqlServerTest.ManyToManyTrackingProxySqlServerFixture..ctor() in /home/ajcvickers/efcore/test/EFCore.SqlServer.FunctionalTests/ManyToManyTrackingProxySqlServerTest.cs:line 21 ----- Inner Stack Trace #2 (Xunit.Sdk.TestClassException) -----

I see. It probably needs a convention scope around most of the InternalModelBuilder.Entity body to avoid the type being readded as non shared before it's added as shared

Pilchie · 2020-09-01T17:11:06Z

Clearing approval since we passed the deadline. Please re-add servicing-consider with the shiproom template when ready.

AndriySvyryd · 2020-09-02T17:34:20Z

src/EFCore/Properties/CoreStrings.resx

@@ -402,6 +402,9 @@
  <data name="EntityTypeNotFound" xml:space="preserve">
    <value>The entity type '{entityType}' was not found. Ensure that the entity type has been added to the model.</value>
  </data>
+  <data name="EntityTypeNotFoundSharedProxy" xml:space="preserve">
+    <value>The type '{clrType}' is configured as a shared-type entity type, but the entity type name is not known. Ensure that CreateProxy is called on a DbSet created specifically for the shared-type entity type through use of a `DbContext.Set` overload that accepts an entity type name.</value>
+  </data>


Move to ProxiesStrings.resx

ajcvickers · 2020-09-04T21:53:54Z

Superseded by PR #22408 and bugs #22407, #22406

ajcvickers requested a review from a team August 31, 2020 21:42

AndriySvyryd reviewed Aug 31, 2020

View reviewed changes

Pilchie added the Servicing-approved label Sep 1, 2020

ajcvickers added 2 commits September 1, 2020 07:54

Handle any property bag type in the same way as the default property …

001ab5d

…bag type

ajcvickers force-pushed the VotingByProxy0830 branch from 29785fc to 001ab5d Compare September 1, 2020 15:44

ajcvickers requested a review from AndriySvyryd September 1, 2020 15:46

ajcvickers commented Sep 1, 2020

View reviewed changes

Pilchie removed the Servicing-approved label Sep 1, 2020

ajcvickers mentioned this pull request Sep 2, 2020

Improve CreateProxy exception message for weak and shared types #22366

Closed

AndriySvyryd reviewed Sep 2, 2020

View reviewed changes

ajcvickers closed this Sep 4, 2020

smitpatel deleted the VotingByProxy0830 branch October 1, 2020 22:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to shared-type entity type handling in proxies #22338

Updates to shared-type entity type handling in proxies #22338

ajcvickers commented Aug 31, 2020

ajcvickers commented Aug 31, 2020

AndriySvyryd Aug 31, 2020

ajcvickers Aug 31, 2020

AndriySvyryd Aug 31, 2020

ajcvickers Aug 31, 2020

AndriySvyryd Aug 31, 2020

ajcvickers Sep 1, 2020

ajcvickers Sep 1, 2020

smitpatel Sep 1, 2020

ajcvickers Sep 1, 2020 •

edited

Loading

AndriySvyryd Sep 1, 2020

Pilchie commented Sep 1, 2020

ajcvickers Sep 1, 2020

AndriySvyryd Sep 1, 2020

ajcvickers Sep 1, 2020

AndriySvyryd Sep 1, 2020 •

edited

Loading

ajcvickers Sep 1, 2020

ajcvickers Sep 1, 2020

ajcvickers Sep 1, 2020

AndriySvyryd Sep 1, 2020 •

edited

Loading

ajcvickers Sep 1, 2020

AndriySvyryd Sep 1, 2020

Pilchie commented Sep 1, 2020

AndriySvyryd Sep 2, 2020

ajcvickers commented Sep 4, 2020

Updates to shared-type entity type handling in proxies #22338

Updates to shared-type entity type handling in proxies #22338

Conversation

ajcvickers commented Aug 31, 2020

ajcvickers commented Aug 31, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajcvickers Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pilchie commented Sep 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndriySvyryd Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndriySvyryd Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pilchie commented Sep 1, 2020

Choose a reason for hiding this comment

ajcvickers commented Sep 4, 2020

ajcvickers Sep 1, 2020 •

edited

Loading

AndriySvyryd Sep 1, 2020 •

edited

Loading

AndriySvyryd Sep 1, 2020 •

edited

Loading