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

Add an IEntityTypeConfiguration equivalent for owned types #15681

Open
Tracked by #22952
samueleresca opened this issue May 10, 2019 · 13 comments
Open
Tracked by #22952

Add an IEntityTypeConfiguration equivalent for owned types #15681

samueleresca opened this issue May 10, 2019 · 13 comments

Comments

@samueleresca
Copy link

samueleresca commented May 10, 2019

I implemented the following entities:


namespace VinylStore.Catalog.Domain.Entities
{
    public class Item
    {
        public Guid Id { get; set; }
        public string Name { get; set; }
        public string Description { get; set; }
        public string LabelName { get; set; }
        public Money Price { get; set; }
        public string PictureUri { get; set; }
        public DateTimeOffset ReleaseDate { get; set; }
        public string Format { get; set; }
        public int AvailableStock { get; set; }
        public Guid GenreId { get; set; }
        public Genre Genre { get; set; }
        public Guid ArtistId { get; set; }
        public Artist Artist { get; set; }
        public bool IsInactive { get; set; }
    }

   public class Artist
    {
        public Guid ArtistId { get; set; }
        public string ArtistName { get; set; }
        public ICollection<Item> Items { get; set; }
    }

 public class Genre
    {
        public Guid GenreId { get; set; }
        public string GenreDescription { get; set; }
        public ICollection<Item> Items { get; set; }
    }
}

I've mapped the entities using the following schemas definitions: SchemaDefinitions.cs

When I run some in-memory tests I get the following exception:

Exception message: `System.AggregateException : One or more errors occurred. (The type 'Artist' cannot be marked as owned because a non-owned entity type with the same name already exists.) `
Stack trace: ```
System.AggregateException : One or more errors occurred. (The type 'Artist' cannot be marked as owned because a non-owned entity type with the same name already exists.) (The following constructor parameters did not have matching fixture data: CatalogDataContextFactory catalogDataContextFactory)
---- System.InvalidOperationException : The type 'Artist' cannot be marked as owned because a non-owned entity type with the same name already exists.
---- The following constructor parameters did not have matching fixture data: CatalogDataContextFactory catalogDataContextFactory

----- Inner Stack Trace #1 (System.InvalidOperationException) -----
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilder.Entity(TypeIdentity& type, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalModelBuilder.Entity(Type type, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalEntityTypeBuilder.HasOwnership(TypeIdentity& targetEntityType, PropertyIdentity navigation, Nullable`1 inverse, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalEntityTypeBuilder.HasOwnership(Type targetEntityType, MemberInfo navigationProperty, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder`1.OwnsOneBuilder[TRelatedEntity](PropertyIdentity navigation)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder`1.OwnsOne[TRelatedEntity](Expression`1 navigationExpression, Action`1 buildAction)
   at VinylStore.Catalog.Infrastructure.SchemaDefinitions.ItemEntitySchemaDefinition.Configure(EntityTypeBuilder`1 ```

Further technical details

EF Core version: 3.0.0-preview5.19227.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: MacOS
IDE: Rider

@ajcvickers
Copy link
Member

@samueleresca The configuration for an owned type needs to be done in the OwnsOne/OwnsMany API calls of the entity type that owns it. It is not currently possible to have a separate IEntityTypeConfiguration for an owned type. Doing so causes EF to configure it as a normal, non-owned entity type.

@ajcvickers
Copy link
Member

Putting this on the backlog to consider patterns that allow owned type mappings to be split out like this.

@samueleresca
Copy link
Author

samueleresca commented May 10, 2019

Hi @ajcvickers, thanks for the support. I've tried to convert the code using the OwnsOne API, but I'm still getting the error.
This is the code:

    public class ItemEntitySchemaDefinition : IEntityTypeConfiguration<Item>
    {
        public void Configure(EntityTypeBuilder<Item> builder)
        {
            builder.ToTable("Items", CatalogContext.DEFAULT_SCHEMA);
            
            builder.HasKey(x => x.Id);

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

            builder.Property(x => x.Description)
                .IsRequired()
                .HasMaxLength(1000);


            builder.OwnsOne(x => x.Artist, navigationBuilder =>
            {
                navigationBuilder.WithOwner()
                    .HasForeignKey(e => e.ArtistId)
                    .HasConstraintName("FK_Artists");
            
                navigationBuilder.ToTable("Artists");
                navigationBuilder.HasKey(e => e.ArtistId);

            });
...

This is the exception:

System.InvalidOperationException : The type 'Artist' cannot be marked as owned because a non-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(Type type, ConfigurationSource configurationSource, Nullable`1 shouldBeOwned)

@samueleresca
Copy link
Author

Plus, I'm not sure about the way that OwnsOne/OwnsMany. If my domain model has a chain of three relationships, means that I will have 3 levels of nesting in my code. Something like:

 builder.OwnsOne(x => x.Artist, navigationBuilder =>
            {
               ...
                navigationBuilder.OwnsOne(x => x.Genre, tb => 
                {
                      ...
                      tb.OwnsOne(x => x.Label, lb => {
                           ...
                       });
                });
            });

I think the level of nesting causes a little bit of redundancy? Is there any specific reason why you are keeping this syntax?

@ajcvickers
Copy link
Member

@samueleresca If you are still seeing issues, then please post a small, runnable project/solution or complete code listing that demonstrates the behavior you are seeing.

With regard to the model configuration syntax, the reason for it is that it forms a complete configuration for the aggregate; that is, the 'normal' entity type as the aggregate root, and all its owned types as the aggregate children. Each aggregate can be configured differently even if it's children have the same CLR types as another aggregate's children.

That being said, we recognize that in many cases an owned type is configured the same way for many aggregates, which is why we do plan to support allowing common configuration. It's just not implemented yet.

@Allann
Copy link

Allann commented Jul 15, 2019

I too am getting this after an upgrade to 3.0.0-preview6.19304.10. The configuration was working fine in 2.2 if that counts as I have a few existing migrations that worked fine and produced the structure I was intending.
Interestingly, I do have configuration files for the owned class to define the string fields. How would you do that without them? I can't use attributes at this stage.
This must have been supported in 2.2, is this completely new again?

Happy to supply the code if needed.

@ChristopherHaws
Copy link
Contributor

I am having the same issue as well. I created a sample app to reproduce the issue: https://github.com/ChristopherHaws/EFCore3MigrationBug

If you delete the existing migration, create a new migration with EF Core 3.0, and compare them it looks like it is failing to migrate from:

b1.HasOne("EFCore3MigrationBug.Blog")
     .WithMany("Posts")
     .HasForeignKey("BlogId")
     .OnDelete(DeleteBehavior.Cascade);

to:

b1.WithOwner()
     .HasForeignKey("BlogId");

@AndriySvyryd AndriySvyryd changed the title The type 'Type` cannot be marked as owned because a non-owned entity type with the same name already exists. Add IOwnedTypeConfiguration Aug 22, 2019
@AndriySvyryd AndriySvyryd changed the title Add IOwnedTypeConfiguration Add an IEntityTypeConfiguration equivalent for owned types Aug 22, 2019
@weitzhandler
Copy link
Contributor

weitzhandler commented Sep 26, 2019

Looks like things have changed in EF Core 3.0.
After upgrading, my code doesn't work.

Here's my old code:

protected override void OnModelCreating(ModelBuilder builder)
{
  base.OnModelCreating(builder);

  builder.Entity<Contact>(contact =>
  {
    var key = "Id";
    var fk = $"{nameof(Contact)}{key}";

    void setMany<TRelation>(Expression<Func<Contact,
        IEnumerable<TRelation>>> navigationExpression)
      where TRelation : class =>
      contact.OwnsMany(navigationExpression, collection =>
      {
        collection
          .WithOwner()
          .HasForeignKey(fk);

        collection.Property<int>(key);
        collection.HasKey(key, fk);
      });

    setMany(c => c.Phones);
    setMany(c => c.Emails);
    setMany(c => c.Addresses);        
  });

  /* Some other stuff */

  builder.Entity<Address>()        
    .Property(a => a.Country)
    .HasColumnType(nameof(SqlDbType.Char));

 /* Some more stuff */
}

I've updated my code to:

void setMany<TRelation>(Expression<Func<Contact,
  IEnumerable<TRelation>>> navigationExpression)
where TRelation : class =>
contact.OwnsMany(navigationExpression, collection =>
{
  collection
    .WithOwner()
    .HasForeignKey(fk);

  collection.Property<int>(key);
  collection.HasKey(key, fk);
});

Now I'm getting the following exception on the builder.Entity<Address>() call:

System.InvalidOperationException: 'The type 'Address' cannot be configured as non-owned because an owned entity type with the same name already exists.'

@weitzhandler
Copy link
Contributor

weitzhandler commented Sep 26, 2019

This worked:

builder.Entity<Contact>(contact =>
{
  var key = "Id";
  var fk = $"{nameof(Contact)}{key}";

  void setMany<TRelation>(Expression<Func<Contact,
      IEnumerable<TRelation>>> navigationExpression)
    where TRelation : class =>     
  contact.OwnsMany(navigationExpression, navBuilder =>
  {
    navBuilder
      .WithOwner()
      .HasForeignKey(fk);

    navBuilder.Property<int>(key);
    navBuilder.HasKey(key, fk);

    if (navBuilder is OwnedNavigationBuilder<Contact, Address> addressNavBuilder)
      addressNavBuilder
        .Property(a => a.Country)
        .HasColumnType(nameof(SqlDbType.Char))
        .HasMaxLength(2)
        .IsFixedLength();
  });

  setMany(c => c.Phones);
  setMany(c => c.Emails);
  setMany(c => c.Addresses);
});

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Oct 3, 2019

Here's a proposal:

public interface IOwnedNavigationConfiguration<TEntity, TOwnedEntity>
    where TEntity : class
    where TOwnedEntity : class
{
    void Configure(OwnedNavigationBuilder<TEntity, TOwnedEntity> ownedBuilder);
}

public interface IOwnedNavigationConfiguration
{
    void Configure(OwnedNavigationBuilder ownedBuilder);
}

public static class ModelBuilderExtensions
{
    public static EntityTypeBuilder<TEntity> ApplyOwnsOneConfiguration<TEntity, TOwnedEntity>(
        this EntityTypeBuilder<TEntity> entityTypeBuilder,
        Expression<Func<TEntity, TOwnedEntity>> navigationExpression,
        IOwnedNavigationConfiguration<TEntity, TOwnedEntity> configuration)
        where TEntity : class
        where TOwnedEntity : class
    {
        entityTypeBuilder.OwnsOne(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static EntityTypeBuilder<TEntity> ApplyOwnsManyConfiguration<TEntity, TOwnedEntity>(
        this EntityTypeBuilder<TEntity> entityTypeBuilder,
        Expression<Func<TEntity, IEnumerable<TOwnedEntity>>> navigationExpression,
        IOwnedNavigationConfiguration<TEntity, TOwnedEntity> configuration)
        where TEntity : class
        where TOwnedEntity : class
    {
        entityTypeBuilder.OwnsMany(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static EntityTypeBuilder<TEntity> ApplyOwnsOneConfiguration<TEntity, TOwnedEntity>(
        this EntityTypeBuilder<TEntity> entityTypeBuilder,
        Expression<Func<TEntity, TOwnedEntity>> navigationExpression,
        IOwnedNavigationConfiguration configuration)
        where TEntity : class
        where TOwnedEntity : class
    {
        entityTypeBuilder.OwnsOne(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static EntityTypeBuilder<TEntity> ApplyOwnsManyConfiguration<TEntity, TOwnedEntity>(
        this EntityTypeBuilder<TEntity> entityTypeBuilder,
        Expression<Func<TEntity, IEnumerable<TOwnedEntity>>> navigationExpression,
        IOwnedNavigationConfiguration configuration)
        where TEntity : class
        where TOwnedEntity : class
    {
        entityTypeBuilder.OwnsMany(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static OwnedNavigationBuilder<TEntity, TOwnedEntity> ApplyOwnsOneConfiguration<TEntity, TOwnedEntity, TNewOwnedEntity>(
        this OwnedNavigationBuilder<TEntity, TOwnedEntity> entityTypeBuilder,
        Expression<Func<TOwnedEntity, TNewOwnedEntity>> navigationExpression,
        IOwnedNavigationConfiguration<TOwnedEntity, TNewOwnedEntity> configuration)
        where TEntity : class
        where TOwnedEntity : class
        where TNewOwnedEntity : class
    {
        entityTypeBuilder.OwnsOne(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static OwnedNavigationBuilder<TEntity, TOwnedEntity> ApplyOwnsManyConfiguration<TEntity, TOwnedEntity, TNewOwnedEntity>(
        this OwnedNavigationBuilder<TEntity, TOwnedEntity> entityTypeBuilder,
        Expression<Func<TOwnedEntity, IEnumerable<TNewOwnedEntity>>> navigationExpression,
        IOwnedNavigationConfiguration<TOwnedEntity, TNewOwnedEntity> configuration)
        where TEntity : class
        where TOwnedEntity : class
        where TNewOwnedEntity : class
    {
        entityTypeBuilder.OwnsMany(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static OwnedNavigationBuilder<TEntity, TOwnedEntity> ApplyOwnsOneConfiguration<TEntity, TOwnedEntity, TNewOwnedEntity>(
        this OwnedNavigationBuilder<TEntity, TOwnedEntity> entityTypeBuilder,
        Expression<Func<TOwnedEntity, TNewOwnedEntity>> navigationExpression,
        IOwnedNavigationConfiguration configuration)
        where TEntity : class
        where TOwnedEntity : class
        where TNewOwnedEntity : class
    {
        entityTypeBuilder.OwnsOne(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static OwnedNavigationBuilder<TEntity, TOwnedEntity> ApplyOwnsManyConfiguration<TEntity, TOwnedEntity, TNewOwnedEntity>(
        this OwnedNavigationBuilder<TEntity, TOwnedEntity> entityTypeBuilder,
        Expression<Func<TOwnedEntity, IEnumerable<TNewOwnedEntity>>> navigationExpression,
        IOwnedNavigationConfiguration configuration)
        where TEntity : class
        where TOwnedEntity : class
        where TNewOwnedEntity : class
    {
        entityTypeBuilder.OwnsMany(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }
}

It would be used like this:

public class BuyerConfiguration : IOwnedNavigationConfiguration<Order, Buyer>
{
    public void Configure(OwnedNavigationBuilder<Order, Buyer> buyerConfiguration)
    {
        buyerConfiguration.Property<int>("OtherId");
    }
}

public sealed class OrderConfiguration : IEntityTypeConfiguration<Order>
{
    public void Configure(EntityTypeBuilder<Order> orderConfiguration)
    {
        orderConfiguration.ApplyOwnsOneConfiguration(e => e.Buyer, new BuyerConfiguration());
    }
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.ApplyConfiguration(new OrderConfiguration());
}

vaipatel added a commit to vaipatel/SolarTally that referenced this issue Oct 29, 2019
Can't do it for owned types apparently.
See dotnet/efcore#15681
@GasyTek
Copy link

GasyTek commented Oct 31, 2019

Here's a proposal:

public interface IOwnedNavigationConfiguration<TEntity, TOwnedEntity>
    where TEntity : class
    where TOwnedEntity : class
{
    void Configure(OwnedNavigationBuilder<TEntity, TOwnedEntity> buyerConfiguration);
}

public static class ModelBuilderExtensions
{
    public static EntityTypeBuilder<TEntity> ApplyOwnsOneConfiguration<TEntity, TOwnedEntity>(
        this EntityTypeBuilder<TEntity> entityTypeBuilder,
        Expression<Func<TEntity, TOwnedEntity>> navigationExpression,
        IOwnedNavigationConfiguration<TEntity, TOwnedEntity> configuration)
        where TEntity : class
        where TOwnedEntity : class
    {
        entityTypeBuilder.OwnsOne(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }

    public static EntityTypeBuilder<TEntity> ApplyOwnsManyConfiguration<TEntity, TOwnedEntity>(
        this EntityTypeBuilder<TEntity> entityTypeBuilder,
        Expression<Func<TEntity, IEnumerable<TOwnedEntity>>> navigationExpression,
        IOwnedNavigationConfiguration<TEntity, TOwnedEntity> configuration)
        where TEntity : class
        where TOwnedEntity : class
    {
        entityTypeBuilder.OwnsMany(navigationExpression, configuration.Configure);
        return entityTypeBuilder;
    }
}

It would be used like this:

public class BuyerConfiguration : IOwnedNavigationConfiguration<Order, Buyer>
{
    public void Configure(OwnedNavigationBuilder<Order, Buyer> buyerConfiguration)
    {
        buyerConfiguration.Property<int>("OtherId");
    }
}

public sealed class OrderConfiguration : IEntityTypeConfiguration<Order>
{
    public void Configure(EntityTypeBuilder<Order> orderConfiguration)
    {
        orderConfiguration.ApplyOwnsOneConfiguration(e => e.Buyer, new BuyerConfiguration());
    }
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.ApplyConfiguration(new OrderConfiguration());
}

I think the goalis to be able to configure common properties of an owned type accross all aggregates that reference it, so this proposal miss the point here.

For instance if we have Person, Company and Address classes like this.

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

    public Address Home {get;set;}
}

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

    public Address BillingAddress  {get;set;}
}

public class Address
{
    public string Street {get;set;}
}

The Address class is a owned type of both Person and Company and for both , its Street property should be 10 characters max. Currently, we will have two configurations for both Person and Company and within each one, we must specify that the Street property should be 10 char length max.

It will be greate to be able to put those common configuration within one class file like we do in EF6 with ComplexTypeConfiguration.

@smitpatel
Copy link
Member

@GasyTek - That is being tracked in different issue. See #6787 (comment)

@AndriySvyryd
Copy link
Member

I updated the proposal with a non-generic interface that can be shared between multiple owned types.

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

8 participants