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

Configuring non-ownership navigation to the owner throws #13912

Closed
ZombieProtectionAgency opened this issue Nov 7, 2018 · 14 comments · Fixed by #18484
Closed

Configuring non-ownership navigation to the owner throws #13912

ZombieProtectionAgency opened this issue Nov 7, 2018 · 14 comments · Fixed by #18484
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Milestone

Comments

@ZombieProtectionAgency
Copy link

ZombieProtectionAgency commented Nov 7, 2018

First off I am terrible at wording and I am not 100% sure how to describe this issue. I searched around a bit but I couldn't find an existing issue that fully captured this so I decided to make a new one.

To summarise: When using an OwnedAttribute property with a one-to-many FK on the type that the FK references the FK and Column are being ignored but it works as expected for every other type and it works as expected if I cut + paste the columns from the OwnedAttribute type directly onto the original type.

I am trying to use OwnedAttribute in a Code First project to add some common columns to many tables.

public class Entity
{
    public Guid Id { get; set; }
}
[Owned]
public class ExtraData
{
    public Guid CreatorId { get; set; }
    public Entity Creator { get; set; }
}

This works as expected when added to a type,

public class SomeType
{
    public Guid Id { get; set; }
    public ExtraData ExtraData { get; set; }
}

which results in a migration of

migrationBuilder.CreateTable(
    name: "SomeTypes",
    columns: table => new
    {
        Id = table.Column<Guid>(nullable: false),
        ExtraData_CreatorId = table.Column<Guid>(nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("PK_SomeTypes", x => x.Id);
        table.ForeignKey(
            name: "FK_SomeTypes_Entities_ExtraData_CreatorId",
            column: x => x.ExtraData_CreatorId,
            principalTable: "Entities",
            principalColumn: "Id",
            onDelete: ReferentialAction.Cascade);
    });

My issue is that when I add this property to the type that it references,

public class Entity
{
    public Guid Id { get; set; }
    public ExtraData ExtraData { get; set; }
}

The FK and its column are completely ignored

migrationBuilder.CreateTable(
    name: "Entities",
    columns: table => new
    {
        Id = table.Column<Guid>(nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("PK_Entities", x => x.Id);
    });

If, instead, I add these columns directly to Entity everything works as expected.

public class Entity
{
    public Guid Id { get; set; }
    public Guid CreatorId { get; set; }
    public Entity Creator { get; set; }
}
migrationBuilder.CreateTable(
    name: "Entities",
    columns: table => new
    {
        Id = table.Column<Guid>(nullable: false),
        CreatorId = table.Column<Guid>(nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("PK_Entities", x => x.Id);
        table.ForeignKey(
            name: "FK_Entities_Entities_CreatorId",
            column: x => x.CreatorId,
            principalTable: "Entities",
            principalColumn: "Id",
            onDelete: ReferentialAction.Cascade);
    });

Also vaguely related, I commented on another issue with this but trying to rename the FK column on the OwnedAttribute type to strip the prefix (ExtraData_) I receive the following error (even if I explicitly set the PKey and FKey.)

The keys {'CreatorId'} on 'ExtraData' and {'Id'} on 'Entity' are both mapped to 'Entities.PK_Entities' but with different columns ({'CreatorId'} and {'Id'}).

Further technical details

EF Core version: 2.1.4.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 Enterprise
IDE: Visual Studio Professional 2017 15.7.6

@ZombieProtectionAgency ZombieProtectionAgency changed the title Self-Referential foreign key/in-row one to many relationship via OwnedAttribute property doesnt work Self-Referential foreign key/in-row one to many relationship via OwnedAttribute property is silently ignored Nov 7, 2018
@ajcvickers
Copy link
Contributor

@smitpatel to respond

@smitpatel
Copy link
Contributor

The FK and its column are completely ignored

They are not ignored. Since ExtraData contains Creator navigation & CreatorId property, by convention CreatorId will be used as FK property. (It is working the same way in your working example too). When you put ExtraData navigation in Entity class, basically you got a pair of reference navigation pointing at each other which constitute single relationship in EF Core. So your relationship would be using Entity.ExtraData & ExtraData.Creator as navigations and ExtraData.CreatorId as FK property. Since ExtraData is a owned type and Entity.ExtraData is navigation pointing to it, the relationship also defines ownership. Hence, the FK of this relationship will be made PK of owned type ExtraData. Due to owned type, Entity and ExtraData will be stored in same table. Since ExtraData.CreatorId takes value of Id through relationship, we don't create 2 different columns and just use single column. After mapping both columns to same database columns, the referential constraint would be defined on same column hence we don't generate Foreign key either.

@smitpatel smitpatel added the closed-no-further-action The issue is closed and no further action is planned. label Nov 7, 2018
@ZombieProtectionAgency
Copy link
Author

ZombieProtectionAgency commented Nov 8, 2018

CreatorId is supposed to be a foreign key to the Id column, not the same data. Any record can reference any other record. Why does it behave exactly as expected when I put CreatorId on Entity but not when I put it on ExtraData and ExtraData on Entity?

@smitpatel
Copy link
Contributor

@ZombieProtectionAgency - When you put CreatorId on Entity directly, you are only configuring one reference navigation. It is one-to-many relationship at that point hence you need separate FK column.
In case of ExtraData.CreatorId you explicitly configured one-to-one relationship. On top of that since its owned, the ExtraData does not need any PK. It is true that any record of ExtraData can reference any Entity but there would be only 1 such entity. Hence, instead of having its own PK, we just CreatorId as PK of owned type too. (That's what very basic concept of ownership, the FK will act as PK so you don't need PK on your owned types).

CreatorId is supposed to be a foreign key to the Id column, not the same data.

Due to foreign key constraint, CreatorId will take exactly same value as Id on referenced data. If you are storing that in single row in same table, it is exactly same data.

If you still have doubts, please describe in detail, what is expected database schema you are looking for and why above configuration is incorrect for that.

@ZombieProtectionAgency
Copy link
Author

ZombieProtectionAgency commented Nov 8, 2018

I was expecting to be able to see something like this in the database

Entities
Id, CreatorId
1, 1
2, 1
3, 1
4, 1

My goal is to end up with the migration builder snippet and its resulting SQL from the original post while using an Owned property. This, specifically

CREATE TABLE [Entities] (
    [Id] uniqueidentifier NOT NULL,
    [CreatorId] uniqueidentifier NOT NULL,
    CONSTRAINT [PK_Entities] PRIMARY KEY ([Id]),
    CONSTRAINT [FK_Entities_Entities_CreatorId] FOREIGN KEY ([CreatorId]) REFERENCES [Entities] ([Id])
);

@smitpatel
Copy link
Contributor

If that is the stored data, then the relationship is one to many & not one-to-one as you are configuring with 2 reference navigations.

@ZombieProtectionAgency
Copy link
Author

So I am misunderstanding OwnedAttribute functionality? Is it possible to do what I want?

@smitpatel
Copy link
Contributor

It is possible with owned collection which is feature in 2.2 release

@ZombieProtectionAgency
Copy link
Author

Could you give me an example of how that solves my issue please? Since each parent "Entity" only references one other entity I didnt think I needed a collection. I dont need to know which records were created by any particular entity.

@smitpatel
Copy link
Contributor

Each parent "Entity" is referencing multiple "ExtraData" entities. Else why 4 different ExtraData.CreatorId which is foreign key has same value pointing to same parent with Entity.Id = 1.
Here is the documentation on owned entities if you want to read further. But it seems like the shape of database schema and C# model both is not properly defined. Stackoverflow may be better place to ask this kind of question.

@ZombieProtectionAgency
Copy link
Author

ZombieProtectionAgency commented Nov 9, 2018

I am still not convinced this is not an issue.

As I said in the original post, the following code works EXACTLY as I expect. It creates a perfect migration, a perfect schema, and all navigations work as expected. I dont see how being self-referential should change that behavior.

public class SomeType
{
    public Guid Id { get; set; }
    public ExtraData ExtraData { get; set; }
}

Which generates this Up migration

migrationBuilder.CreateTable(
    name: "SomeTypes",
    columns: table => new
    {
        Id = table.Column<Guid>(nullable: false),
        ExtraData_CreatorId = table.Column<Guid>(nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("PK_SomeTypes", x => x.Id);
        table.ForeignKey(
            name: "FK_SomeTypes_Entities_ExtraData_CreatorId",
            column: x => x.ExtraData_CreatorId,
            principalTable: "Entities",
            principalColumn: "Id",
            onDelete: ReferentialAction.Cascade);
    });

Which generates this SQL script

CREATE TABLE [SomeTypes] (
    [Id] uniqueidentifier NOT NULL,
    [ExtraData_CreatorId] uniqueidentifier NOT NULL,
    CONSTRAINT [PK_SomeTypes] PRIMARY KEY ([Id]),
    CONSTRAINT [FK_SomeTypes_Entities_ExtraData_CreatorId] FOREIGN KEY ([ExtraData_CreatorId]) REFERENCES [Entities] ([Id]) ON DELETE CASCADE
);

@smitpatel
Copy link
Contributor

Even in this case, Entity still has one-to-many relationship with ExtraData because Entity does not have reference navigation to ExtraData. The moment you add reference navigation in Entity you are making it one to one, so your code model does not match with the data you want to store.

@divega
Copy link
Contributor

divega commented Nov 9, 2018

@ZombieProtectionAgency I believe the key is in this part of Smit's initial explanation:

When you put ExtraData navigation in Entity class, basically you got a pair of reference navigation pointing at each other which constitute single relationship in EF Core.

TL;DR: In this case the prediction of EF Core is incorrect, and you were trying to configure a model with two separate relationships.

Details:

EF Core is trying to infer what your intent is when you have:

public class Entity
{
    public Guid Id { get; set; }
    public ExtraData ExtraData { get; set; }
}
[Owned]
public class ExtraData
{
    public Guid CreatorId { get; set; }
    public Entity Creator { get; set; }
}

Entity.Extradata and ExtraData.Creator in your model matches an EF Core convention that basically says "if there are two objects with matching navigation properties pointing to each other, create a single relationship for them, and then configure the navigation properties to be the reverses of each other".

It just happens that in your particular model this is not the intent, and the navigation properties are supposed to represent 2 separate relationships: You are using one ExtraData to group information about the creator of an entity (so, in terms of the EF Core model, there is a one-to-one relationship between Entity and its ExtraData), but then each entity (with its ExtraData) will have a Creator, that happens to be different Entity, and each of the Entities can be the Creator for multiple other entities (hence there is a separate one-to-many relationship).

EF Core conventions are simple rules that try to guess what your intent is, but are not expected to always get it right. I personally find it a bit puzzling how our guess as humans can be sometimes so much better, but that is because we are usually applying additional domain information to make our decisions. E.g. you know very well what the word "Creator" means, so you understand immediately that if an Entity has a Creator, that Creator is a different Entity. But EF Core is asked to make same guess in a reproducible way, and with zero knowledge of the meaning of the "Creator" string.

Now, the best way I can think of to make this model work the way you want, is to explicitly configure the relationship.

I have attempted to do this in the following snippet:

using System;
using Microsoft.EntityFrameworkCore;

namespace World
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            using (var db = new MyContext())
            {
                db.Database.EnsureDeleted();
                db.Database.EnsureCreated();
            }
        }

    }

    public class Entity
    {
        public Guid Id { get; set; }
        public ExtraData ExtraData { get; set; }
    }

    [Owned]
    public class ExtraData
    {
        public Guid CreatorId { get; set; }
        public Entity Creator { get; set; }
    }

    public class MyContext: DbContext
    {
        public DbSet<Entity> Entities { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(
                @"server=(localdb)\mssqllocaldb;database=hey;integrated security=true;connectretrycount=0");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder
                .Entity<Entity>()
                .OwnsOne(e => e.ExtraData)
                .HasOne(ed => ed.Creator)
                .WithMany()
                .HasForeignKey(ed => ed.CreatorId);
        }
    }
}

Now, unfortunately, it seems that we currently don't support this. I get the exception below from this configuration code. I will let @smitpatel comment if there is a workaround that works.

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=Microsoft.EntityFrameworkCore
  StackTrace:
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.IsWeakTypeDefinition(ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalEntityTypeBuilder.Owns(TypeIdentity& targetEntityType, PropertyIdentity navigation, Nullable`1 inverse, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalEntityTypeBuilder.Owns(Type targetEntityType, PropertyInfo navigationProperty, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.RelationshipDiscoveryConvention.CreateRelationships(IEnumerable`1 relationshipCandidates, InternalEntityTypeBuilder entityTypeBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.RelationshipDiscoveryConvention.DiscoverRelationships(InternalEntityTypeBuilder entityTypeBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.RelationshipDiscoveryConvention.Apply(EntityType entityType, MemberInfo navigationProperty)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnNavigationRemoved(InternalEntityTypeBuilder sourceEntityTypeBuilder, InternalEntityTypeBuilder targetEntityTypeBuilder, String navigationName, MemberInfo memberInfo)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.RunVisitor.VisitOnNavigationRemoved(OnNavigationRemovedNode node)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ConventionVisitor.VisitConventionScope(ConventionScope node)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ConventionVisitor.VisitConventionScope(ConventionScope node)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ConventionBatch.Run()
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ConventionBatch.Run(ForeignKey foreignKey)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.IsUnique(Boolean unique, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.ReferenceNavigationBuilder.WithManyBuilder(PropertyIdentity collection)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.ReferenceNavigationBuilder`2.WithMany(String navigationName)
   at ConsoleApp40.MyContext.OnModelCreating(ModelBuilder modelBuilder) in C:\Users\divega\source\repos\ConsoleApp40\ConsoleApp40\Program.cs:line 46
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
   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.Internal.InternalAccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.get_DatabaseCreator()
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.EnsureDeleted()
   at ConsoleApp40.Program.Main(String[] args) in Program.cs:line 15

@divega divega reopened this Nov 9, 2018
@divega divega removed the closed-no-further-action The issue is closed and no further action is planned. label Nov 9, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Nov 16, 2018
@ZombieProtectionAgency
Copy link
Author

I had actually tried that and ran into the same issue. I am not as familiar with the Fluent API so I assumed I was doing something wrong.

Since this does appear to be an issue, just not the original one I thought it was, I think the title should change? It might be better for somebody more familiar with what is happening to reword it though. I would probably end up making it nonsensical.

@AndriySvyryd AndriySvyryd changed the title Self-Referential foreign key/in-row one to many relationship via OwnedAttribute property is silently ignored Configuring non-ownership navigation to the owner throws Nov 30, 2018
@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Jun 28, 2019
@AndriySvyryd AndriySvyryd modified the milestones: Backlog, 3.1.0 Sep 8, 2019
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 21, 2019
@AndriySvyryd AndriySvyryd removed their assignment Oct 21, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0, 3.1.0-preview2 Oct 24, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0-preview2, 3.1.0 Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants