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

Allow mapping a key or index over more than one column across owned entity types. #11336

Open
Tracked by #24107 ...
lzocateli opened this issue Mar 20, 2018 · 33 comments
Open
Tracked by #24107 ...

Comments

@lzocateli
Copy link

lzocateli commented Mar 20, 2018

I need to generate migration for my entity "Contact" has the "Complex Type -> Address".

So far so good!

However, in the "Contact" entity I need to create a unique index containing "Complex Type -> Address" properties, which is causing migration error.

How can I create this unique index?

The code with example is in:
https://github.com/lincolnzocateli/EFCoreExample/blob/master/Map/ContactMap.cs

using System.Linq;
using EfCoreExample.Models;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace EfCoreExample.Map
{

    public class ContactMap : IEntityTypeConfiguration<Contact>
    {


        public void Configure(EntityTypeBuilder<Contact> builder)
        {

            builder.ToTable("Contacts");


            builder.HasKey(x => x.ContactId);

            builder.Property(x => x.ContactId)
                .IsRequired();

            
            builder.Property(x => x.Name)
                .IsRequired()
                .HasColumnType("varchar(40)");

            builder.Property(x => x.Observation)
                .HasColumnType("varchar(100)");

            builder.OwnsOne(x => x.Address).Property(x => x.AddressType)
                .IsRequired()
                .HasColumnType($"varchar(20)")
                .HasColumnName("Type");

            builder.OwnsOne(x => x.Address).Property(x => x.Street)
                .IsRequired()
                .HasColumnType($"varchar(60)")
                .HasColumnName("Street");

            builder.OwnsOne(x => x.Address).Property(x => x.Neighborhood)
                .HasColumnType($"varchar(60)")
                .HasColumnName("Complement");

            builder.OwnsOne(x => x.Address).Property(x => x.City)
                .IsRequired()
                .HasColumnType($"varchar(60)")
                .HasColumnName("City");

            builder.OwnsOne(x => x.Address).Property(x => x.State)
                .IsRequired()
                .HasColumnType($"varchar(2)")
                .HasColumnName("State");

            builder.OwnsOne(x => x.Address).Property(x => x.Zip)
                .IsRequired()
                .HasColumnType($"varchar(8)")
                .HasColumnName("ZipCode");

            //(1:N)
            builder.HasOne(x => x.Person)
                .WithMany(c => c.Contacts)
                .HasForeignKey(x => x.ContactId)
                .OnDelete(DeleteBehavior.Restrict);



           builder.HasIndex(e => new
               {
                   e.Name,
                   e.Address.AddressType,
                   e.Address.Zip,
               }).HasName("IX_MyIndex")
               .IsUnique();

        }
    }

}

Exception message:

The properties expression 'e => new <>f__AnonymousType1`3(Name = e.Name, AddressType = e.Address.AddressType, Zip = e.Address.Zip)' is not valid. The expressionshould represent a property access: 't => t.MyProperty'. When specifying multiple properties use an anonymous type: 't => new { t.MyProperty1, t.MyProperty2 }'.
Parameter name: propertyAccessExpression
Stack trace:
System.ArgumentException: The properties expression 'e => new <>f__AnonymousType1`3(Name = e.Name, AddressType = e.Address.AddressType, Zip = e.Address.Zip)' isnot valid. The expression should represent a property access: 't => t.MyProperty'. When specifying multiple properties use an anonymous type: 't => new { t.MyProperty1, t.MyProperty2 }'.
Parameter name: propertyAccessExpression
   at Microsoft.EntityFrameworkCore.Internal.ExpressionExtensions.GetPropertyAccessList(LambdaExpression propertyAccessExpression)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder`1.HasIndex(Expression`1 indexExpression)
   at EfCoreExample.Map.ContactMap.Configure(EntityTypeBuilder`1 builder) in /home/lincoln/Dropbox/EFCoreExample/Map/ContactMap.cs:line 70
   at Microsoft.EntityFrameworkCore.ModelBuilder.ApplyConfiguration[TEntity](IEntityTypeConfiguration`1 configuration)
   at EfCoreExample.Context.EfCoreExampleContext.OnModelCreating(ModelBuilder modelBuilder) in /home/lincoln/Dropbox/EFCoreExample/Context/EfExampleContext.cs:line 37
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelCustomizer.Customize(ModelBuilder modelBuilder, DbContext context)
   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelCustomizer.Customize(ModelBuilder modelBuilder, DbContext context)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.<>c__DisplayClass5_0.<GetModel>b__0(Object k)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_1(IServiceProvider p)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, ServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, ServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.<>c__DisplayClass22_0.<RealizeService>b__0(ServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.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.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_1.<.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)

Steps to reproduce

Generate migration for the first time:
/> dotnet ef migrations add v1.0.0

Further technical details

EF Core version:
Microsoft.EntityFrameworkCore.Design version 2.0.1
Microsoft.EntityFrameworkCore.Tools.DotNet version 2.0.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer version 2.0.1
Operating system: Linux Debian 9
IDE: VSCODE

@ajcvickers
Copy link
Member

Notes from triage: putting this on the backlog to consider supporting indexes and table splitting together. The implementation will likely require an internal store-side model.

@lincolnzocateli The workaround is to create the index in the migration, rather than in the model.

@ajcvickers ajcvickers added this to the Backlog milestone Mar 22, 2018
@lzocateli
Copy link
Author

Ok I'm following the backlog

@AndriySvyryd
Copy link
Member

AndriySvyryd commented May 16, 2018

@lincolnzocateli Another workaround is to create a shadow property on the owned type and map it to the same column:

builder.OwnsOne(x => x.Address, ab =>
{
    ab.Property<string>("Name")
        .HasColumnName("Name")
        .IsRequired()
        .HasColumnType("varchar(40)");

    ab.HasIndex(
        "Name",
        "AddressType",
        "Zip",
    ).HasName("IX_MyIndex")
        .IsUnique();
}

Though since Name is required you will have to populate it manually when adding a new Address:

context.Entry(contact).Reference(e => e.Address).TargetEntry.Property<string>("Name")
    .CurrentValue = contact.Name;

@lzocateli
Copy link
Author

@AndriySvyryd Thanks for the tip, I tried to do it, but I did not find a viable solution for having to populate it manually when adding a new Address.

Many people in the dotnet community in Brazil have asked me about a solution to this, for now we are waiting for the backlog.

@AndriySvyryd
Copy link
Member

@lincolnzocateli Why didn't my workaround for populating the shadow property work for you? Is that you don't have the context available when a new address is added? If so you could go through all entries and find which ones were added:

foreach (var entityEntry in context.ChangeTracker.Entries())
{
    if (entityEntry.Entity.GetType() != typeof(Contact))
    {
        continue;
    }

    var addressEntry = entityEntry.Reference("Address").TargetEntry;
    if (addressEntry.State == EntityState.Added
        || addressEntry.State == EntityState.Detached)
    {
        addressEntry.Property("Name").CurrentValue = entityEntry.Property("Name").CurrentValue;
    }
}

@lzocateli
Copy link
Author

@AndriySvyryd, I'm doing well with your solution,

thanks for the help.

@divega
Copy link
Contributor

divega commented Jun 26, 2018

We wanted to keep this open in the backlog.

@divega divega reopened this Jun 26, 2018
@AndriySvyryd AndriySvyryd changed the title Fluent Api - Mapping an index over more than one column, When a column is complex type. Fluent Api - Mapping an index over more than one column across entity types. Jul 16, 2018
@dylinmaust
Copy link

dylinmaust commented Nov 19, 2018

@AndriySvyryd does this workaround require an additional column in the table? I've adapted your above code to my own solution by adding a shadow property on "Name", but it's creating two columns, one prefixed with "MyTableName_".

@smitpatel
Copy link
Member

@dylinmaust - It does not require additional column. The point is you create a shadow property in your complex type and configure it using HasColumnName to use the same column as the property which is in your entity type. So they both would share the column in database without requiring additional column.

@dylinmaust
Copy link

dylinmaust commented Nov 19, 2018

@smitpatel Thanks for the clarification. Here's my configuration:

	public class PartEntityConfiguration : IEntityTypeConfiguration<Part>
	{
			public void Configure(EntityTypeBuilder<Part> modelBuilder)
			{
					modelBuilder.HasKey(p => p.Id);

					modelBuilder
							.Property(p => p.Name)
							.IsRequired()
							.HasMaxLength(100);

					modelBuilder
							.HasIndex(p => p.Name)
							.IsUnique();

					modelBuilder
							.OwnsOne(p => p.MountConfiguration, p => 
							{
									// Workaround for unique index that crosses owned type boundaries
									// https://github.com/aspnet/EntityFrameworkCore/issues/11336#issuecomment-389670812
									p.Property<string>("Name")
											.HasColumnName("Name")
											.IsRequired()
											.HasMaxLength(100);

									p.HasIndex("Name", "MountingTypeId", "MountingOrientationId")
											.IsUnique();
							});
			}
	}

which is generating something like this:

	 migrationBuilder.CreateTable(
		name: "Part",
		columns: table => new
		{
				Id = table.Column<int>(nullable: false)
						.Annotation("SqlServer:ValueGenerationStrategy", SqlServerValueGenerationStrategy.IdentityColumn),
				Part_Name = table.Column<string>(maxLength: 100, nullable: false),
				Name = table.Column<string>(maxLength: 100, nullable: false)
		})

@smitpatel
Copy link
Member

Can you make following change and see if it works for you?

modelBuilder
    .Property(p => p.Name)
    .HasColumnName("Name")  // <<-- this line is added by me
    .IsRequired()
    .HasMaxLength(100);

@dylinmaust
Copy link

That was an embarrassingly simple solution. Thank you! And thanks for the Markdown help 👍

@RamunasAdamonis
Copy link

Looking forward to see this feature added.

@sandersaares
Copy link

sandersaares commented Feb 19, 2019

Lack of this discourages me from using owned types. Has this been considered and rejected from 3.0 scope? Is "backlog" equivalent to "maybe after 3.0" in this sense?

I am also interested in owned property referencing support in ForSqlServerInclude

@xu-lei-richard
Copy link

+1 need this

@xaviervv
Copy link

Any news?

@AndriySvyryd
Copy link
Member

There are many other improvements that have bigger impact than this, so we won't get to this one any time soon. Use the 👍 reaction on the first post to indicate your support, this is one of the ways we measure impact.

@ivanrodriguezfernandez
Copy link

Any update on when this is going to be resolved?

@xu-lei-richard
Copy link

Any news?

@roji
Copy link
Member

roji commented May 6, 2020

This issue is in the backlog milestone, which means we don't plan to work on it for 5.0; once that's released, we'll reexamine which issues can make it into 6.0.

Note that this issue has an easy workaround - simply create the index in your migration - which means we generally don't consider this to be extremely high priority.

@pantonis
Copy link

@roji Thanks changing column type from nvarchar(max) to nvarchar(450) did the trick. However this is not a proper solution but rather a hack to make it work. I would like to use the Fluent Api to create the index and in more general It would be good if EF Core 6 provide better support to use Domain Driven Design. It is a shame that after 5 releases and basic features of DDD are not fully supported.

@rcollette
Copy link

I wish we could at least create property accessors over the shadow properties of the the owned entity which should then allow the creation of an index using the index attribute.

@pburdynski
Copy link

I have another workaround that may be useful for somebody if OwnedEntity can be treated as a ValueObject and it is acceptable to configure owned entity by using setters and getters instead of using OwnsOne api.

public class Entity{
  public OwnedEntity OwnedEntity 
  {
    get => new OwnedEntity(propA , propB);
    private set => { propA = value.PropA, propB = value.PropB} 
  }
  private string propA {get; set;}
  private string propB {get; set;}
}

then in the Entity configuration:

builder.Ignore(x => x.OwnedEntity);
builder.Property<string>("propA").HasColumnName("OwnedEntity_PropA"); 
builder.Property<string>("propB").HasColumnName("OwnedEntity_PropB");

//and finally the composite key definition with propB:
builder.hasIndex("someId", "someOtherId", "propB");

@dcby
Copy link

dcby commented Sep 15, 2021

Here is my findings.

It is possible to tamper the metadata using internal APIs to force it generate correct migrations code but there are two drawbacks:

  • It generates broken model snapshot so you need to fix it manually after adding the migration. Also this will lock you out of DatabaseFacade.EnsureCreated functionality (only migrations are possible).
  • Internal APIs is a potential subject to change in the next releases.

To do this you need to get the references to corresponding instances of Microsoft.EntityFrameworkCore.Metadata.Internal.Index and Microsoft.EntityFrameworkCore.Metadata.Internal.Property types and adjust the index manually.

The example:

// ...
using Index = Microsoft.EntityFrameworkCore.Metadata.Internal.Index;
using Property = Microsoft.EntityFrameworkCore.Metadata.Internal.Property;

// ...

public class ProdAttr
{
    public Guid Id { get; set; }
    public Guid ProdRefId { get; set; }
    public string Name { get; set; } = null!;
    public bool IsMany { get; set; }
    public Val Value { get; set; } = null!;

    public record Val(Guid? File)
    {
        public FileAsset? FileRef { get; init; }
    }
}

partial class EntityTypeConfiguration : IEntityTypeConfiguration<ProdAttr>
{
    // Suppress warning on usages of Index and Property types.
    [SuppressMessage("Usage", "EF1001:Internal EF Core API usage.")]
    public void Configure(EntityTypeBuilder<ProdAttr> builder)
    {
        builder.ToTable("prod_attr", "catalog");

        builder.Property(t => t.Id).HasColumnName("prod_attr_id");
        // Store the Property references. The Metadata property is of type
        // Microsoft.EntityFrameworkCore.Metadata.Internal.Property which implements IMutableProperty.
        Property prodRefIdProperty = (Property)builder.Property(t => t.ProdRefId).HasColumnName("prod_refid").Metadata;
        Property nameProperty = (Property)builder.Property(t => t.Name).HasColumnName("name").Metadata;
        builder.Property(t => t.IsMany).HasColumnName("is_many").HasDefaultValue(false).ValueGeneratedNever();

        builder.HasKey(t => t.Id);
        builder.HasIndex(t => new { t.ProdRefId, t.Name });
        builder.HasIndex(t => t.Name);

        builder.OwnsOne(t => t.Value, o =>
        {
            // Store the Property reference.
            Property fileProperty = (Property)o.Property(t => t.File).HasColumnName("file_val").Metadata;

            o.HasIndex(t => t.File);

            o.HasOne(t => t.FileRef).WithMany().HasForeignKey(t => t.File);

            // Create the index with bogus property list. Make sure the list is unique or specify the unique name.
            // Otherwise you will alter the existing index definition.
            // Stote the Index reference. The Metadata property is of type
            // Microsoft.EntityFrameworkCore.Metadata.Internal.Index which implements IMutableIndex.
            Index index = (Index)builder.HasIndex(t => t.Id, "1120efc7-3d77-4ee6-be7d-a2955c6640ce").IsUnique().HasFilter("file_val IS NOT NULL").Metadata;
            // Cast list of properties to concrete run-time type.
            List<Property> properties = (List<Property>)index.Properties;
            // Clear the list.
            properties.Clear();
            // Populate the list with desired properties.
            properties.AddRange(new[] { prodRefIdProperty, nameProperty, fileProperty });
        });

        // ...
    }
}

This will add the index migration code:

migrationBuilder.CreateIndex(
    name: "1120efc7-3d77-4ee6-be7d-a2955c6640ce",
    schema: "catalog",
    table: "prod_attr",
    columns: new[] { "prod_refid", "name", "file_val" },
    unique: true,
    filter: "file_val IS NOT NULL");

But also will populate the model snapshot with:

b.HasIndex(new[] { "ProdRefId", "Name", "File" }, "1120efc7-3d77-4ee6-be7d-a2955c6640ce")
    .IsUnique()
    .HasFilter("file_val IS NOT NULL");

making it invalid, because entity ProdAttr doesn't have the property named File. To fix the model and make it usable you need to remove this index definition in both generated ModelSnapshot.BuildModel and Migration.BuildTargetModel methods.

This is not a practical solution but could bring some insights.

@kl1mm
Copy link

kl1mm commented Oct 4, 2021

Any news to this issue? Any considerations to implement this?

@ajcvickers
Copy link
Member

@kl1mm This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 6.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you.

@asinghca
Copy link

In the case where there is a need to create a composite unique (clustered) index, I've decided to simply not use value objects in those persisted entities to keep things simple and avoid all these complex workarounds, at least until it is correctly supported by EF Core. I prefer to use value objects but not at the cost of this added complexity. So, just put all those properties in your entity/aggregate and save yourself from all this trouble.

Had:

class MyEntity {
int Id
}

Wanted:

class MyEntity {
IntId Id
}

class IntId {
int Id

// immutable logic with validations
}

Ran into limitations from EF Core [this issue]

Changed to: still using value object but indirectly

class MyEntity {
int Id

public MyEntity(int incomingId)
{
this.Id = new IntId(incomingId).Value // <--- still using value object concept!
}

}

class IntId {
int Id
public string Value => Id
public MyEntity(int incomingId)
{
if (!IsNaturalNumber)
throw
// immutable logic with validations + other checking logic
}

}

@rofenix2

This comment was marked as off-topic.

@roji

This comment was marked as off-topic.

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

No branches or pull requests