Skip to content
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

Support enum navigations on owned types #16

Closed
TAGC opened this issue Sep 12, 2020 · 2 comments
Closed

Support enum navigations on owned types #16

TAGC opened this issue Sep 12, 2020 · 2 comments

Comments

@TAGC
Copy link
Contributor

TAGC commented Sep 12, 2020

Consider we define a domain like this:

public class Customer
{
    public int Id { get; set; }
    public Address Address { get; set; }
}

public class Address
{
    public CountryCode CountryCode { get; set; }
}

public enum CountryCode
{
    AF = 4,
    AL = 8
    // ...
}

We configure Customer to "own" an instance of Address, and ideally want Address to have a foreign key constraint on a CountryCode enum lookup table that gets generated by this library:

 protected override void OnModelCreating(ModelBuilder modelBuilder)
 {
    modelBuilder.Entity<Customer>().OwnsOne(x => x.Address).WithOwner();
    modelBuilder.ConfigureEnumLookup(EnumLookupOptions.Default.SetNamingScheme(x => x.Pascalize()));
}

However, trying to create a migration for this causes the following exception to be thrown:

PM> Add-Migration InitialCreate
Build started...
Build succeeded.
System.InvalidOperationException: The type 'experiment.Data.CommissionsContext+Address' cannot be configured as non-owned because an owned entity type with the same name already exists.
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilder.Entity(TypeIdentity& type, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilder.Entity(String name, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned)
   at Microsoft.EntityFrameworkCore.ModelBuilder.Entity(String name)
   at SpatialFocus.EntityFrameworkCore.Extensions.EnumLookupExtension.ConfigureEnumLookup(ModelBuilder modelBuilder, EnumLookupOptions enumOptions)
   at experiment.Data.CommissionsContext.OnModelCreating(ModelBuilder modelBuilder) in C:\Projects\experiment\Data\CommissionsContext.cs:line 46
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelCustomizer.Customize(ModelBuilder modelBuilder, DbContext context)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder)
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_3(IServiceProvider p)
   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()
   at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
   at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
   at Microsoft.EntityFrameworkCore.Infrastructure.Internal.InfrastructureExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.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.AddMigration.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
The type 'experiment.Data.CommissionsContext+Address' cannot be configured as non-owned because an owned entity type with the same name already exists.

The reason seems to be because of this code in the library, which would try to configure Address even though it is owned by Company and can only be configured through an OwnedNavigationBuilder:

modelBuilder.Entity(entityType.Name)
.HasOne(concreteType)
.WithMany()
.HasPrincipalKey(keyName)
.HasForeignKey(property.Name)
.OnDelete(enumOptions.DeleteBehavior);

To support enum properties on owned types, one fix might be to check if the type is owned and skipping configuration on it if that is the case. One way I've found to do this after some quick digging was the following:

bool IsOwned(IEntityType entityType)
{
    return entityType.FindOwnership() != null;
}

If the library bypasses configuration of owned types, it might allow the relationship to be manually configured. After playing around for some time, I found this sort of worked:

modelBuilder.Entity<Customer>().OwnsOne(x => x.Address, addressBuilder =>
{
    var enumLookupType = typeof(EnumWithNumberLookup<CountryCode>);
    modelBuilder.Entity(enumLookupType).ToTable(nameof(CountryCode));

    addressBuilder.WithOwner();
    addressBuilder.HasOne(enumLookupType)
        .WithMany()
        .HasPrincipalKey("Id")
        .HasForeignKey("CountryCode")
        .OnDelete(DeleteBehavior.Cascade);
});

This produces the following migration:

public partial class InitialCreate : Migration
{
    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.CreateTable(
            name: "CountryCode",
            columns: table => new
            {
                Id = table.Column<int>(nullable: false),
                Name = table.Column<string>(nullable: true)
            },
            constraints: table =>
            {
                table.PrimaryKey("PK_CountryCode", x => x.Id);
            });

        migrationBuilder.CreateTable(
            name: "Customers",
            columns: table => new
            {
                Id = table.Column<int>(nullable: false)
                    .Annotation("SqlServer:Identity", "1, 1"),
                Address_CountryCode = table.Column<int>(nullable: true)
            },
            constraints: table =>
            {
                table.PrimaryKey("PK_Customers", x => x.Id);
                table.ForeignKey(
                    name: "FK_Customers_CountryCode_Address_CountryCode",
                    column: x => x.Address_CountryCode,
                    principalTable: "CountryCode",
                    principalColumn: "Id",
                    onDelete: ReferentialAction.Cascade);
            });

        migrationBuilder.CreateIndex(
            name: "IX_Customers_Address_CountryCode",
            table: "Customers",
            column: "Address_CountryCode");
    }

    protected override void Down(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.DropTable(
            name: "Customers");

        migrationBuilder.DropTable(
            name: "CountryCode");
    }
}

Represented diagrammatically:
image

Do you think it would be feasible to improve support for owned types? I guess this would involve three parts:

  1. Bypassing navigation configuration for owned types (as mentioned above). You'd still do everything else though (i.e. setting up the schema for the enum lookup table and populating it).

  2. Adding some sort of utility method that simplifies manual configuration of foreign key constraints from owned types onto enum lookup types.

  3. Documenting this in the README/samples/etc.

pergerch added a commit that referenced this issue Sep 16, 2020
Co-Authored-By: Christopher Dresel <1769090+Dresel@users.noreply.github.com>
@pergerch
Copy link
Member

Thank you for the time spent on investigating the issue. We have implemented your proposed changes.

Owned entities will be skipped in the initial run of ConfigureEnumLookup and will need to be configured seperately on the OwnedNavigationBuilder.

modelBuilder.Entity<Product>()
.OwnsMany<Review>(nameof(Product.Reviews),
builder =>
{
builder.ConfigureOwnedEnumLookup(EnumLookupOptions.Default.Pluralize().UseStringAsIdentifier(), modelBuilder);
});

Please test the changes with your project either using the latest source or the new NuGet package 2.1.0-beta1

@TAGC
Copy link
Contributor Author

TAGC commented Sep 16, 2020

I tried this out, it seems to work really well - thanks for implementing this, I appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants