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

Many-to-many (skip) navigation properties #19003

Closed
5 tasks done
ajcvickers opened this issue Nov 20, 2019 · 31 comments
Closed
5 tasks done

Many-to-many (skip) navigation properties #19003

ajcvickers opened this issue Nov 20, 2019 · 31 comments
Assignees
Labels
area-change-tracking area-model-building area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@ajcvickers
Copy link
Member

ajcvickers commented Nov 20, 2019

This is one of the building blocks for many-to-many. However, it is also more broadly useful than just many-to-many.

Problem

Consider a model typical for a many-to-many relationship:

public class Post
{
    public int PostId { get; set; }
    public string Title { get; set; }
    public string Content { get; set; }

    public List<PostTag> PostTags { get; set; }
}

public class Tag
{
    public string TagId { get; set; }
    public string Content { get; set; }

    public List<PostTag> PostTags { get; set; }
}

public class PostTag
{
    public int PostId { get; set; }
    public Post Post { get; set; }

    public string TagId { get; set; }
    public Tag Tag { get; set; }

    public DateTime LinkCreated { get; set; }
}

Now consider how to load a Post and include all associated Tags. This requires using two navigation properties--one to go from Post to PostTag, and a second to go from PostTag to Post. For example:

var postsAndTags 
    = context.Posts
        .Include(e => e.PostTags)
        .ThenInclude(e => e.Tag)
        .ToList();

Likewise, querying across Posts and Tags also requires use of PostTags:

var post
    = context.Posts
        .Where(e => e.PostTags.Select(e => e.Tag.Content).Contains("MyTag")).ToList();

Proposal

The goal of forwarded navigation properties is to allow navigation properties to be defined which forward to some of the direct navigation properties in the model.

For example, forwarded navigation properties could be added to Post and Tag in the model above:

public class Post
{
    public int PostId { get; set; }
    public string Title { get; set; }
    public string Content { get; set; }

    public List<Tag> Tags { get; set; } // Skips over PostTag to Tag
    
    public List<PostTag> PostTags { get; set; }
}

public class Tag
{
    public string TagId { get; set; }
    public string Content { get; set; }

    public List<Post> Posts { get; set; } // Skips over PostTag to Post
    
    public List<PostTag> PostTags { get; set; }
}

These forwarded navigation properties can then used to write simpler queries. For example, the queries from above can now be:

var postsAndTags 
    = context.Posts
        .Include(e => e.Tags)
        .ToList();
var post
    = context.Posts
        .Where(e => e.Tags.Select(e => e.Content).Contains("MyTag")).ToList();

Notes

Things to consider (non-exhaustive):

  • Model building API will be needed for the general case
    • Conventions/sugar may be use later for many-to-many pattern
    • Configuration by attribute could be considered, but might be too complex
  • It should be possible to define the skip-level navigation that skip over relationships that do not themselves expose navigation properties
    • This could involve shadow navigation properties if needed
    • However, also note that it is expected that "skipped" entity types (like for the join table in the example) can still be used explicitly as needed. This allows additional properties to be used, such as the LinkCreated property in the example above.

Tasks:

  • Model support
  • Fluent API
  • Change tracking support
  • Query support
  • Tracking query tests
@AndriySvyryd
Copy link
Member

AndriySvyryd commented Nov 21, 2019

Implementation proposal

Fluent API

modelBuilder.Entity<Tag>()
  .HasMany(e => e.Posts)
  .WithMany(e => e.Tags)
  .UsingEntity<PostTag>(
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags),
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags))
  .ToTable("PostTags");

This is the signature:

EntityTypeBuilder<TAssociationEntity> UsingEntity<TAssociationEntity>(
    Func<EntityTypeBuilder<TAssociationEntity>, ReferenceCollectionBuilder<TLeftEntity, TAssociationEntity>> configureLeft,
    Func<EntityTypeBuilder<TAssociationEntity>, ReferenceCollectionBuilder<TRightEntity, TAssociationEntity>> configureRight)

Plus nested closure, non-generic and string-based overloads

This shape has the following advantages:

  • The types have equal standing in the relationship, there's no 'main' or 'principal' end.
  • Navigations on the foreign keys can be omitted.
  • Foreign keys are configured using existing API and can specify FK and PK properties in the same call chain.
  • Allows to omit the foreign key configuration when by-convention configuration is supported.
  • Allows to omit the association entity configuration when property bag entity types are supported.

Disadvantages:

  • Not immediately clear what arguments should be specified, this is especially true for the non-generic version.

Metadata

public interface ISkipNavigation : IPropertyBase
{
    IEntityType DeclaringEntityType { get; }
    IEntityType TargetEntityType { get; }
    IForeignKey ForeignKey { get; }
    IForwardedNavigation Inverse { get; }
    bool IsCollection { get; }
    bool IsOnPrincipal { get; }
   (Other methods with default implementations)
}

Design limitations

  • Can only be configured to skip over one entity type (spanning two foreign keys)
  • Cannot cross aggregate boundries

Scoping limitations

  • Won't be discovered by convention (Always needs UsingEntity)
    • Later we could support a parameterless UsingEntity<>() call that will find the foreign keys by convention
  • Configuration by attribute will not be supported
  • Has to be many-to-many and the involved foreign keys must be non-unique
  • The skipped entity type is on the dependent end of both foreign keys
  • Must have a non-shadow inverse and it can't use the same CLR property

@AndriySvyryd AndriySvyryd changed the title Skip-level navigation properties Forwarded (skip-level) navigation properties Nov 21, 2019
@AndriySvyryd AndriySvyryd changed the title Forwarded (skip-level) navigation properties Many-to-many (forwarded) navigation properties Nov 25, 2019
@roji
Copy link
Member

roji commented Nov 26, 2019

Here are some comments (remember I'm a beginner here so be patient, there may be nonsense ahead :)):

  • TJoinEntity seems like it's a bit relational-specific, are we expecting this API to possibly be usable also in non-relational scenarios? If so a different name may be better (e.g. TIntermediateEntity).
  • In the lambda parameters of Using, is HasOne missing a lambda parameter?
  • It's a little odd for me that Using returns an EntityTypeBuilder over the join table - we start with modelBuilder.Entity() at the top, and find ourselves configuring a completely different table at the bottom (do we already have examples of this?)... I can imagine people thinking the ToTable should configure the Tags table and not PostTags. It may be cleaner/more consistent to require all configuration for the join table to be done by going back to ModelBuilder.

Can only be configured to skip over one entity type (spanning two foreign keys)

Is this only a limitation of the proposed fluent API, i.e. would users be able to drop down to metadata to set up a forwarded navigation spanning more than one entity type?

Here's a different proposal for a fluent API. There are probably several reasons why this makes no sense, am proposing this mainly to learn:

modelBuilder.Entity<Post>()
    .HasMany(p => p.Tags)
        .Through(p => p.PostTags)
        .ThenThrough(pt => pt.Tag)
    .WithMany(t => t.Posts)
        .Through(t => t.PostTags)
        .ThenThrough(pt => pt.Post);

The Through/ThenThrough (can be called Using/Via/Whatever) allows skipping over more than one entity type and is consistent with Include/ThenInclude (and OrderBy). It's also defined based on the paths, from one side all the way to the other, rather than putting the join table in the center. This also may allow for some wackier setups, e.g. where the paths in the two directions don't go through the same entity types (no concrete/compelling scenario yet though :)).

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Nov 26, 2019

TJoinEntity seems like it's a bit relational-specific, are we expecting this API to possibly be usable also in non-relational scenarios? If so a different name may be better (e.g. TIntermediateEntity).

I think an entity that is joining two other entities also makes sense in non-relational context and has the benefit of being familiar in relational context. Intermediate is also ok, but is too long for my liking.

In the lambda parameters of Using, is HasOne missing a lambda parameter?

No, it's optional

It's a little odd for me that Using returns an EntityTypeBuilder over the join table

After you call HasOne/HasMany or OwnsOne/OwnsMany without nested lambda you can no longer get back the original EntityTypeBuilder, so this is an established pattern

It may be cleaner/more consistent to require all configuration for the join table to be done by going back to ModelBuilder.

For shared type entity types it would be more verbose as the Type and entity type name would need to be specified again.

Is this only a limitation of the proposed fluent API, i.e. would users be able to drop down to metadata to set up a forwarded navigation spanning more than one entity type?

No, in metadata IForwardedNavigation just has a foreign key and the inverse IForwardedNavigation, there's no way to specify any intermediate foreign keys. This was done to simplify implementation as I couldn't think of compelling scenarios requiring skipping more than one entity type (this also becomes confusing quickly if more than two many-to-one FKs are involved)

Here's a different proposal for a fluent API.

A big downside is that it doesn't allow to configure FK and principal key properties

@roji
Copy link
Member

roji commented Nov 26, 2019

It's a little odd for me that Using returns an EntityTypeBuilder over the join table

After you call HasOne/HasMany or OwnsOne/OwnsMany without nested lambda you can no longer get back the original EntityTypeBuilder, so this is an established pattern

Right. What I meant to say is that Using returning an EntityTypeBuilder could be problematic, as using may assume the can configure the original entity type instead of the join table's entity type. In your example above, it kinda looks like the ToTable affects the Tag entity rather than the PostTag entity.

A big downside is that it doesn't allow to configure FK and principal key properties

I guess there's one fundamental thing I'm misunderstanding... I was assuming that the basic navigations (Post.PostTags and Tag.PostTags) need to be configured independently and separately, and that the forwarded navigation is only configured on top of other existing (underlying) navigations. If that were the case, it seems like the forwarded navigation wouldn't need to deal with FK/PK properties, just reference the underlying navigations. In metadata, the IForwardedNavigation would then effectively contain a list of underlying navigations (which is where the FK/PK info would be taken from).

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Nov 26, 2019

I was assuming that the basic navigations (Post.PostTags and Tag.PostTags) need to be configured independently and separately

Yes, that's what happens in the metadata, but we can add Fluent API that circumvents that restriction to allow terser configuration. We don't yet have shared-type entity types, but if we did separate configuration would look something like this:

modelBuilder.Entity<Tag>()
  .HasMany(e => e.PostTags, "PostTag")
  .WithOne()
  .HasForeignKey(e => e.TagId);

modelBuilder.Entity<Post>()
  .HasMany(e => e.PostTags, "PostTag")
  .WithOne()
  .HasForeignKey(e => e.PostId);

modelBuilder.Entity<IDictionary<string, object>>("PostTag")
  .HasKey("TagId", "PostId")
  .ToTable("PostTags");

modelBuilder.Entity<Tag>()
  .HasMany(e => e.Posts)
  .WithMany(e => e.Tags)
  .UsingEntity<IDictionary<string, object>>(
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags),
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags),
    "PostTag");

As opposed to

modelBuilder.Entity<Tag>()
  .HasMany(e => e.Posts)
  .WithMany(e => e.Tags)
  .UsingEntity<IDictionary<string, object>>(
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags)
      .HasForeignKey("PostId"),
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags)
      .HasForeignKey("TagId"),
    "PostTag")
  .ToTable("PostTags")
  .HasKey("TagId", "PostId");

@smitpatel
Copy link
Member

It cannot be totally based on navigations since navigations could be missing in CLR type. Especially if we are going to use property bag entity to represent join entity in future so API needs to use FKs.

Questions for @AndriySvyryd

  • In your code snippets above, in snippet 1, HasMany has both lambda and string (new pattern?). How would it refer to PostTag entity backed by dictionary? Further, the navigations to join entity would not be present in general case.
  • In snippet 2, HasKey would return KeyBuilder, you cannot configure ToTable on it.

Still thinking about how to integrate shared entityType configuration if any so that we are future proof.

@AndriySvyryd
Copy link
Member

In your code snippets above, in snippet 1, HasMany has both lambda and string (new pattern?). How would it refer to PostTag entity backed by dictionary?

The string is necessary to name the shared-type entity type.

Further, the navigations to join entity would not be present in general case.

Navigations are optional.

In snippet 2, HasKey would return KeyBuilder, you cannot configure ToTable on it.

Fixed

@smitpatel
Copy link
Member

I will present this in design meeting on Wednesday.

@AndriySvyryd AndriySvyryd changed the title Many-to-many (forwarded) navigation properties Many-to-many (skip) navigation properties Dec 11, 2019
AndriySvyryd added a commit that referenced this issue Dec 19, 2019
AndriySvyryd added a commit that referenced this issue Dec 19, 2019
AndriySvyryd added a commit that referenced this issue Dec 19, 2019
AndriySvyryd added a commit that referenced this issue Dec 20, 2019
AndriySvyryd added a commit that referenced this issue Dec 20, 2019
AndriySvyryd added a commit that referenced this issue Jan 7, 2020
ajcvickers added a commit that referenced this issue Aug 5, 2020
ajcvickers added a commit that referenced this issue Aug 6, 2020
ajcvickers added a commit that referenced this issue Aug 7, 2020
ajcvickers added a commit that referenced this issue Aug 7, 2020
ajcvickers added a commit that referenced this issue Aug 11, 2020
ajcvickers added a commit that referenced this issue Aug 11, 2020
* MemberEntry support for skip navigations

Part of #19003
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 14, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-rc1 Aug 14, 2020
@neoGeneva
Copy link

Hey, is there plans to add scaffolding for many-to-many relationships? It doesn't look like it's included in the tasks for this issue, and I couldn't see any other issues tracking it.

@ajcvickers
Copy link
Member Author

@neoGeneva Detecting existing join tables in reverse engineering is not something we have done--thanks for bringing this up. I have filed #22475 to track this.

@neoGeneva
Copy link

Awesome, thanks @ajcvickers !

@gojanpaolo
Copy link

Hello, is there a way to avoid defining the property for one of the entities?
e.g.

builder.HasMany(p => p.Tags).WithMany(); // notice no parameter in `WithMany`

Thank you.

@AndriySvyryd
Copy link
Member

@gojanpaolo Not yet #3864

@MikeStonoga
Copy link

MikeStonoga commented Nov 8, 2022

Hello,
I had stumbled in a specific case while designing my new system.

Well, I will try to explain this using the concept of responsible:

I can have responsibles for an activity, for a place, for a person, etc...

So, i want to map this relationship on a table called ResponsibleContext which will have three properties: ResponsibleId, ContextId and ContextType.

Then, I had made a mapping to the first case using the suggested syntax.

Implementation proposal

Fluent API

modelBuilder.Entity<Tag>()
  .HasMany(e => e.Posts)
  .WithMany(e => e.Tags)
  .UsingEntity<PostTag>(
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags),
    pt => pt
      .HasOne()
      .WithMany(e => e.PostTags))
  .ToTable("PostTags");

It worked well.
But, when i add another case mapping to the same ResponsibleContext i receive the following error:

The skip navigation 'Person.Responsibles' doesn't have an inverse navigation configured. Every skip navigation should have an inverse skip navigation.

It seems that when i made the first configuration EF has removed the ResponsibleContext table from being able to be mapped for another cases.

My real scenario was with PhoneContext with the following mappings on each IEntityTypeConfiguration

For a Place which have ISet Phone I Used:

builder.HasMany(e => (ISet<Telefone>)e.Telefones) .WithMany(e => (ISet<Local>)e.Contextos) .UsingEntity<TelefoneContexto>(j => j.HasOne<Telefone>() .WithMany() .HasForeignKey("TelefoneId"), j => j.HasOne<Local>() .WithMany() .HasForeignKey("ContextoId") ) .ToTable("TelefoneContexto") .HasNoKey();

For a Person which have ISetPhone I Used:

builder.HasMany(e => (ISet<Telefone>)e.Telefones) .WithMany(e => (ISet<Pessoa>)e.Contextos) .UsingEntity<TelefoneContexto>(j => j.HasOne<Telefone>() .WithMany() .HasForeignKey("TelefoneId"), j => j.HasOne<Pessoa>() .WithMany() .HasForeignKey("ContextoId") ) .ToTable("TelefoneContexto") .HasNoKey();

`public class Telefone : FullAuditedEntity, ITelefone
{
public string Numero { get; private set; }
public string DDD { get; private set; }
public string CodigoPais { get; private set; }

[JsonIgnore]
[IgnoreDataMember]
public virtual ISet<object> Contextos { get; set; } = new HashSet<object>();

protected Telefone() { }
protected Telefone(string numero, string ddd, string codigoPais, Guid creatorId)
    : base(creatorId)
{
    Numero = numero;
    DDD = ddd;
    CodigoPais = codigoPais;
}

public static ITelefone Cadastrar(ICadastrarTelefoneRequisito requisito)
    => new Telefone(requisito.Numero, requisito.DDD, requisito.CodigoPais, requisito.CommanderId);

}`

The JsonIgnore and IgnoreDataMember are there only because swagger falls into self referency loop. and this is the way i found to avoid that. And, all of that works fine until i add a second case.

@StanlyLife
Copy link

@gojanpaolo Not yet #3864

Now?

@AndriySvyryd
Copy link
Member

Now?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking area-model-building area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests