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

[Owned] attribute does not play nicely with lazy loading and nullable references in C# 8.0 #15876

Closed
ibasin opened this issue May 31, 2019 · 6 comments

Comments

@ibasin
Copy link

ibasin commented May 31, 2019

I am using Visual Studio 2019 Preview, .Net Core 3.0 Preview 5 and EF Core Preview 5.

If a class is marked [Owned] and lazy loading is enabled, Lazy loading thinks this is a navigational property. This causes two problems:

  1. It forces you to make the property virtual (not so horrible but seems to be a bug)
  2. If you are using nullable references and instantiate the [Owned] type (obviously not marked as nullable) in place, you cannot seed the data for the entity since system thinks that this is a nav property and you get an exception stating that the seed entity for entity type cannot be added because it has the navigation for the owned type set.

Steps to reproduce

Create a .Net Core Console application. Reference EF Core and Lazy Loading NuGets.

Here is the entity and the owned type

#nullable enable

using System.Collections.Generic;

namespace Tester
{
    [Owned]
    public class USAddress
    {
        public string Street { get; set; } = "";
        public string City { get; set; } = "";
        public string State { get; set; } = "";
        public string Zip { get; set; } = "";
    }    

    public class Author : Entity
    {
        public string Name { get; set; } = "";
        public virtual USAddress Address { get; set; } = new USAddress();
        
        //this must be nullable because it is a nav property and it must be set to null to seed data
        public virtual List<Book>? Books { get; set; } 
    }
}

Attempting to seed the data for this class in OnModelCreating method will always fails because it thinks that a nav property Address is initialized (and it will always be because it is not nullable):

'The seed entity for entity type 'Author' cannot be added because it has the navigation 'Address' set. To seed relationships you need to add the related entity seed to 'USAddress' and specify the foreign key values {'AuthorId'}. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the involved property values.'

Further technical details

EF Core version: 3.0.0-preview5.19227.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2019 Preview

@ibasin ibasin changed the title [Owned] attribute does not play nice with Lazy Loading and nullable references in C# 8.0 [Owned] attribute does not play nicely with Lazy Loading and nullable references in C# 8.0 May 31, 2019
@ibasin ibasin changed the title [Owned] attribute does not play nicely with Lazy Loading and nullable references in C# 8.0 [Owned] attribute does not play nicely with lazy loading and nullable references in C# 8.0 May 31, 2019
@ibasin
Copy link
Author

ibasin commented May 31, 2019

Also, because of the problem I described above, it is impossible to initialize the owned object on an entity - you get the error saying that entity cannot be added because it has the navigation set (where navigation is really an owned property).

@ajcvickers
Copy link
Contributor

@ibasin There's a few things going on here. First, just because something is owned does not mean it is not an entity type. It therefore follows from this that the Author.Address property is a reference from one entity type to another--i.e. it is a navigation property. Also, it means that setting inline is creating a new entity instance that EF should try to insert.

Second, it's involvement in lazy loading is something that is not required: see #12462

Third, the implications of this for non-nullable references is interesting. We will discuss this.

@ibasin
Copy link
Author

ibasin commented Jun 3, 2019

@ajcvickers Thank you for a quick reply.

  1. I read Stop configuring lazy-loading for properties that will always be eager-loaded #12462 and agree that in my sample Author.Address should not be marked as virtual because it should be eager loaded always. However, if my project uses lazy loading elsewhere (references Microsoft.EntityFrameworkCore.Proxies and enables lazy loading in OnConfiguring) and I do not mark it as virtual, I get the following exception (you can easily reproduce yourself):

System.InvalidOperationException: 'Navigation property 'Address' on entity type 'Author' is not virtual. UseLazyLoadingProxies requires all entity types to be public, unsealed, have virtual navigation properties, and have a public or protected constructor.'

  1. Thank you for looking into non-nullable references in the context of data seeding.

I think both of these are real problems with no easy workarounds.

Thank you!

Please let me know if I could be of service.

Ilya

@ajcvickers
Copy link
Contributor

@ibasin The 'workaround' for #12462 is to make the property virtual. Is there a reason you can't do this?

We discussed the non-nullable reference type scenario, but came to the conclusion that it is really no different from any other navigation property, which is discussed on #15520 and #10347. The way to handle this is to use default!. EF will then initialize the value during loading.

There is also a more general issue here, which is that owned entity types are not value objects and don't have the same semantics as value objects. Attempting to use these types as if they are value objects often ends up being difficult and feeling wrong because of the very different semantics.

@ibasin
Copy link
Author

ibasin commented Jun 3, 2019

There is still a problem. Data for an inline owned property cannot be seeded. I see no workaround. Am I missing something?

Thank you.

@ajcvickers
Copy link
Contributor

@ibasin See #12004

@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
Projects
None yet
Development

No branches or pull requests

2 participants