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

[Question] Single property owned entity with conversion not marked as owned entity #20476

Closed
ilmax opened this issue Mar 31, 2020 · 12 comments
Closed
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@ilmax
Copy link
Contributor

ilmax commented Mar 31, 2020

In EF core there are 2 ways of mapping owned entities sharing the same table composed of only one property. on is tu use the OwnsOne and another one is to use HasConversion what I found unexpected was that using the latter doesn't make the entity as owned, even more strange is that explicitly marking the entity as owned via the modelBuilder.Owned<{Value object class}>(); still doesn't mark it as owned. full code is here:

class Program
{
    static void Main(string[] args)
    {
        var c = new Context();
        _ = c.Model;
        _ = Console.ReadLine();
    }
}

public class Order
{
    public int Id { get; set; }
    public List<OrderLine> Lines { get; set; }
}

public class OrderLine
{
    public int Id { get; set; }
    public int Quantity { get; set; }
    public Price Price { get; set; }
}

public class Price
{
    public Price(int amountInCent) => AmountInCent = amountInCent;
    public int AmountInCent { get; private set; }
    [NotMapped]
    public decimal Amount => AmountInCent /100m;
}

public class Context : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseSqlServer("whatever");

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Owned<Price>();
        modelBuilder.Entity<Order>();
        modelBuilder.Entity<OrderLine>(builder =>
        {
            builder.Property(o => o.Price).HasColumnName("PriceInCent").HasConversion(p => p.AmountInCent, i => new Price(i));
        });

        foreach (var entity in modelBuilder.Model.GetEntityTypes())
        {
            Console.WriteLine($"Name: {entity.Name,-30} Owned: {entity.IsOwned(),-5} Keyless: {entity.IsKeyless}");
        }
    }
}

this outputs

Name: EfCoreInferOwnedType.Order     Owned: False Keyless: False
Name: EfCoreInferOwnedType.OrderLine Owned: False Keyless: False
Name: EfCoreInferOwnedType.Price     Owned: False Keyless: False

While I was expecting this to display for the last one:
Name: EfCoreInferOwnedType.Price Owned: **true** Keyless: **true**
Why in this case the Price class is not marked as owned?

full repo here

Further technical details

EF Core version: 3.1.3
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
Operating system: Win 10
e.g. Visual Studio 2019 16.5

@ajcvickers
Copy link
Member

In EF core there are 2 ways of mapping owned entities sharing the same table composed of only one property. on is tu use the OwnsOne and another one is to use HasConversion

HasConversion doesn't create an owned entity. I'm curious as to what let you to believe it does.

@ilmax
Copy link
Contributor Author

ilmax commented Mar 31, 2020

@ajcvickers I do partially agree, that's the reason why I marked this as a question.
Owned entities, as per the doc are defined as:

EF Core allows you to model entity types that can only ever appear on navigation properties of other entity types. These are called owned entity types. The entity containing an owned entity type is its owner.

Owned entities are essentially a part of the owner and cannot exist without it, they are conceptually similar to aggregates. This means that the owned entity is by definition on the dependent side of the relationship with the owner.

I think the Price entity posted above satisfies all the requirements to be called an owned entity.

I'm curious as to what let you to believe it does.

calling modelBuilder.Owned<Price>() explicitly but despite this the entity.IsOwned() still returns false.

@ajcvickers
Copy link
Member

@ilmax

I do partially agree

I don't follow what you are partially agreeing to. Owned types in EF Core are a specific kind of entity type. Using HasConversion does not create an entity type at all. They can be used for some similar things, but they are entirely different concepts and constructs.

calling modelBuilder.Owned() explicitly but despite this the entity.IsOwned() still returns false.

Please post a small, runnable project that reproduces this so we can investigate. It may be related to #20446

@ilmax
Copy link
Contributor Author

ilmax commented Mar 31, 2020

@ajcvickers

I don't follow what you are partially agreeing to

It was a poor choice of words, sorry

Using HasConversion does not create an entity type at all

My wish is either to not get the Price entity when calling modelBuilder.Model.GetEntityTypes() or get it marked as an owned entity.
What I found and you can see just running my repro above is that HasConversion just creates an entity so I tried to explicitly mark it as owned entity via modelBuilder.Owned<Price>(); but this doesn't change anything. Am I missing something?

Please post a small, runnable project that reproduces this so we can investigate

It's there and there in the first post and there's also a link to a github repo, do I need to add something more?

@ajcvickers
Copy link
Member

@ilmax It makes no sense to use a value conversion (means the target is not an entity type) together with Owned (means that the target is an entity type). If you're doing both of these things, then no repro is needed--but don't do that. If Owned doesn't work when not combined with a value conversion, then please post a repro for this.

@ilmax
Copy link
Contributor Author

ilmax commented Mar 31, 2020

@ajcvickers following your reasoning

use a value conversion (means the target is not an entity type)

you do not expect the class configured with HasConversion to be reachable via the modelBuilder.Model.GetEntityTypes() is it correct?

@ajcvickers
Copy link
Member

@ilmax Correct. At least once the model is fully built.

@ilmax
Copy link
Contributor Author

ilmax commented Mar 31, 2020

Cool, me neither but this code still prints the Price entity which I found a bit strange

    class Program
    {
        static void Main(string[] args)
        {
            var c = new Context();
            _ = c.Model;
            _ = Console.ReadLine();
        }
    }

    public class Order
    {
        public int Id { get; set; }
        public List<OrderLine> Lines { get; set; }
    }

    public class OrderLine
    {
        public int Id { get; set; }
        public int Quantity { get; set; }
        public Price Price { get; set; }
    }

    public class Price
    {
        public Price(int amountInCent) => AmountInCent = amountInCent;
        public int AmountInCent { get; private set; }
        [NotMapped]
        public decimal Amount => AmountInCent / 100m;
    }

    public class Context : DbContext
    {
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseSqlServer("whatever");

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Order>();
            modelBuilder.Entity<OrderLine>(builder =>
            {
                builder.Property(o => o.Price).HasColumnName("PriceInCent").HasConversion(p => p.AmountInCent, i => new Price(i));
            });

            foreach (var entity in modelBuilder.Model.GetEntityTypes())
            {
                Console.WriteLine($"Name: {entity.Name,-30} Owned: {entity.IsOwned(),-5} Keyless: {entity.IsKeyless}");
            }
        }
    }

output:

Name: EfCoreInferOwnedType.Order     Owned: False Keyless: False
Name: EfCoreInferOwnedType.OrderLine Owned: False Keyless: False
Name: EfCoreInferOwnedType.Price     Owned: False Keyless: False

@smitpatel
Copy link
Member

Model is not fully built at that point. Price is added as entityType by convention before it was configured as a property. ModelCleanupConvention cleans it up when model is fully built.

@ilmax
Copy link
Contributor Author

ilmax commented Mar 31, 2020

@smitpatel thanks, make sense, so last question: What's the proper hook to add shadow properties to all the model entity? Is there a hook after ModelCleanupConvention has run?

@AndriySvyryd
Copy link
Member

@ilmax For now you'll just have to loop through modelBuilder.Model.GetEntityTypes() and filter out the entity types that will be removed later. This will be fixed by #15898. Also with #214 you'll be able to do this with a convention.

@ilmax
Copy link
Contributor Author

ilmax commented Apr 1, 2020

Thank you @AndriySvyryd, closing

@ilmax ilmax closed this as completed Apr 1, 2020
@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Apr 1, 2020
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

4 participants