New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type configured as both owned entity and regular entity requires a primary key to be defined after upgrading from 2.1 to 2.2 #13986

Closed
kluhman opened this Issue Nov 20, 2018 · 13 comments

Comments

@kluhman

kluhman commented Nov 20, 2018

In EF Core 2.1, if you configured a type to be treated as an owned entity, for example using OwnedAttribute, and later used fluent configuration APIs to configure it as a regular entity, e.g. by invoking the Entity method or the ApplyConfiguration method on the model builder, we would mix the implicitly configured key of the owned entity and the new configuration as a regular entity.

This was actually a bug in EF Core 2.1. The behavior that we originally intended is that if a type is configured as en entity type, it requires a primary key to be defined. This primary key can be defined by matching a convention (for example, if the name of the property is Id), by decorating it with KeyAttribute, or by invoking the HasKey method explicitly.

EF Core 2.2 implements the intended behavior, and so a model like the one described above that worked accidentally in EF Core 2.1 will stop working.

The recommended solution is to configure the type either as regular entity type or as an owned entity type. If the type is a regular entity type, then the key can be configured explicitly.

Original issue as reported by customer

I tried upgrading to 2.2.0-preview3-35497 today and a model that has been in the project for a long time is now throwing an exception while creating a migration.

Exception message: The entity type 'Portal' requires a primary key to be defined.
Stack trace:
System.InvalidOperationException: The entity type 'Portal' requires a primary key to be defined.
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelValidator.ValidateNonNullPrimaryKeys(IModel model)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelValidator.Validate(IModel model)
   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelValidator.Validate(IModel model)
   at Microsoft.EntityFrameworkCore.Internal.SqlServerModelValidator.Validate(IModel model)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ValidatingConvention.Apply(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnModelBuilt(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.ModelBuilder.FinalizeModel()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Autofac.Core.Activators.Delegate.DelegateActivator.ActivateInstance(IComponentContext context, IEnumerable`1 parameters)
   at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters)
   --- End of inner exception stack trace ---
   at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters)
   at Autofac.Core.Lifetime.LifetimeScope.GetOrCreateAndShare(Guid id, Func`1 creator)
   at Autofac.Core.Resolving.InstanceLookup.Execute()
   at Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(ISharingLifetimeScope currentOperationScope, IComponentRegistration registration, IEnumerable`1 parameters)
   at Autofac.Core.Activators.Reflection.ConstructorParameterBinding.Instantiate()
   at Autofac.Core.Activators.Reflection.ReflectionActivator.ActivateInstance(IComponentContext context, IEnumerable`1 parameters)
   at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters)
   --- End of inner exception stack trace ---
   at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters)
   at Autofac.Core.Lifetime.LifetimeScope.GetOrCreateAndShare(Guid id, Func`1 creator)
   at Autofac.Core.Resolving.InstanceLookup.Execute()
   at Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(ISharingLifetimeScope currentOperationScope, IComponentRegistration registration, IEnumerable`1 parameters)
   at Autofac.Core.Resolving.ResolveOperation.Execute(IComponentRegistration registration, IEnumerable`1 parameters)
   at Autofac.ResolutionExtensions.TryResolveService(IComponentContext context, Service service, IEnumerable`1 parameters, Object& instance)
   at Autofac.ResolutionExtensions.ResolveService(IComponentContext context, Service service, IEnumerable`1 parameters)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
   at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
   at Microsoft.EntityFrameworkCore.Internal.InternalAccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.CreateContext(Func`1 factory)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.CreateContext(String contextType)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)

Steps to reproduce

I have a project that started on EF Core 2.0.0 and has been upgraded to 2.1.6. After upgrading to 2.2.0-preview3-35497 and trying to create an empty migration, I got the above exception. The models that are involved are below.

public class Program
{
    public int Id { get; set; }

    [Required]
    [MaxLength(50)]
    public string Name { get; set; }

    public Portal Portal { get; set; } = new Portal();
    public DateTime CreateDate { get; set; }
    public DateTime UpdateDate { get; set; }
    public DateTime? DeleteDate { get; set; }
    public bool IsDeleted { get; set; }
}

[Owned]
public class Portal
{
    public string Title { get; set; }
}

Further technical details

EF Core version: 2.2.0-preview3-35497
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 Pro
IDE: Visual Studio 2017 15.8.2

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Nov 20, 2018

@kluhman I wasn't able to reproduce this with just the information above. Could you post a runnable project/solution or complete code listing that demonstrates the behavior you are seeing.

@kluhman

This comment has been minimized.

kluhman commented Nov 27, 2018

@ajcvickers I am not able to post the solution I am working on, but I will try to put together a small reproducible solution and post here.

@kluhman

This comment has been minimized.

kluhman commented Nov 28, 2018

@ajcvickers I've attached a minimal solution that is reproducing the behavior. I was actually able to reproduce without doing an upgrade. With the models defined as is, it was able to reproduce on the initial migration using the 2.2.0-preview3-35497 package.

For reference, I ran the following script in the Package Manager Console to attempt to create the migration.

Add-Migration Initial-Migration

You can reproduce the behavior described in the initial post by downgrading the 3 EF packages down to 2.1.4, creating the initial migration. Then upgrade the packages back up to the preview and try to create a new empty migration.

EFCoreMigrationErrorExample.zip

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Nov 28, 2018

Note for triage: this is because the Portal type is not being configured as an owned type due to explicit configuration of it as an entity type:

modelBuilder.ApplyConfiguration(new ProgramEntityConfiguration());
modelBuilder.ApplyConfiguration(new PortalConfiguration());

This behavior appears correct, but in this case is causing a regression due to broken behavior in 2.1.

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Nov 28, 2018

Triage: we will close this as by-design because the behavior for 2.2 is as was always intended. It is very unfortunate that configuring the owned type in this way "worked" in previous versions, but switching back to that behavior would be more problematic.

@AndriySvyryd Do we have an issue tracking configuration of all owned types in a common place?

@kluhman

This comment has been minimized.

kluhman commented Nov 29, 2018

@ajcvickers Do you have guidance or documentation on how to properly configure an owned type? I can achieve the max length and required configurations with attributes, but I don't know of any attribute to remove the default unicode behavior.

@AndriySvyryd

This comment has been minimized.

Member

AndriySvyryd commented Nov 30, 2018

@kluhman Owned type are configured in a similar way to normal entity types:

modelBuilder.Entity<Program>().OwnsOne(p => p.Portal, a =>
{
    a.Property(p => p.Title).HasColumnType("nvarchar(128)");
});

See https://docs.microsoft.com/en-us/ef/core/modeling/owned-entities

@ajcvickers Filed #14047

@divega

This comment has been minimized.

Member

divega commented Nov 30, 2018

@ajcvickers can you please remind me what the action item is for me? 😄

@smitpatel

This comment has been minimized.

Contributor

smitpatel commented Nov 30, 2018

@divega - Release notes since it is breaking functional change.

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Dec 3, 2018

@divega Pinging for blog work.

@divega divega added this to the 2.2.0 milestone Dec 4, 2018

@divega

This comment has been minimized.

Member

divega commented Dec 4, 2018

Adding to milestone so that we can add link to query in announcement blog post. @ajcvickers let me know if you had in mind managing this in any other way.

Also, can this be closed now?

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented Dec 4, 2018

@divega Sounds fine. Yes, it can be closed.

@ajcvickers ajcvickers closed this Dec 4, 2018

@divega divega changed the title from The entity type requires a primary key to be defined after upgrading from 2.1.6 to 2.2.0-preview3 to The entity type requires a primary key to be defined after upgrading from 2.1 to 2.2 Dec 4, 2018

@divega divega changed the title from The entity type requires a primary key to be defined after upgrading from 2.1 to 2.2 to Type configured as both owned entity and regular entity requires a primary key to be defined after upgrading from 2.1 to 2.2 Dec 4, 2018

@divega

This comment has been minimized.

Member

divega commented Dec 4, 2018

@ajcvickers @AndriySvyryd FYI, I decided to add the detailed description of the issue in the initial comment, and I will link from the blog post. Feel free to make edits if I have misunderstood anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment