HasOne/HasMany used with single string can be confusing

[ajcvickers]

See discussion on #6674

If a relationship is configured like this:

modelBuilder.Entity<Samurai>().HasOne("Entrance").WithOne();

it looks like it should be using the navigation property "Entrance", which may be private. But really it is the related entity type name:

 public virtual ReferenceNavigationBuilder HasOne(
            [NotNull] string relatedTypeName,
            [CanBeNull] string navigationName = null)

The correct way to configure it is something like:

modelBuilder.Entity<Samurai>().HasOne(typeof(Entrance), "Entrance").WithOne();

[ajcvickers]

Comment from @divega:

I agree that is confusing 😞 The fact that the type isn't being specified as a generic argument but is still needed is very easy to miss. I wonder if this is the only API in which this happens.

If we believe it is worth addressing it, something like this may help:

1. Obsolete this version of HasOne that doesn't take a generic argument
2. Create a new API called HasOneOfType for this case. It would look like this:

modelBuilder.Entity<Samurai>().HasOneOfType("Entrance", navigationName: "Entrance")
    .WithOne();

I think it helps because it reads like the (first) argument is the type.

This looks complicated though:

modelBuilder.Entity<Samurai>().HasOneOfType(typeof(Entrance), navigationName: "Entrance")
    .WithOne();

Maybe we do it only for the one that works with strings?

Perhaps there is a better solution.

[ajcvickers]

@divega A few of things I have been thinking about:

• If the navigation property exists (even if it is private) that we don't need the type name because it can be obtained from the navigation property. The only reason to pass both as arguments is when configuring a shadow state nav property, currently used only for the model snapshot.
• Having a generic overload for the type would be nice, even if the type is not needed, because it allows the generic to flow through for lambda expressions later later in the chain. I wonder if there is a reason we didn't add it here.
• I wonder if we could just change it (probably in 3.0) to really have an single string overload just for the navigation name. This might be okay, because the times when you really want to only pass the single string type name seem rare to me--relationship with no navigation property where you don't want to use typeof() for some reason. Also, in this case there isn't a navigation property that matches, so after the change if there was code using this it could throw. It's hackish, but it could even warn and then drop back to using the name as a type if the navigation was not found--but not sure we even need to go that far.

[smitpatel]

But we need type always to find out what is related entity type. i.e. where to search that navigation.

[ajcvickers]

@smitpatel In the example, Samurai has a property called Entrance, which is of type Entrance. So the type of the navigation property tells us the type of the related entity.

[ajcvickers]

Filed #9191 for the generic overload. For the rest, putting in 3.0 as a possible breaking change that we could make at that time. Note that the type name has to be a full entity type name, and hence the code above while passing in OnModelCreating will fail at model validation time, which may make it easier to change the behavior without breaking any real code, at least once re-compiled.

[AndriySvyryd]

If the single string overload is changed to mean navigation it needs to retain old behavior when called on a shadow entity type to ensure old snapshots aren't broken.

New snapshots can be generated to always call the two parameter overload.

[ajcvickers]

Maybe we need a way to determine that we are building a snapshot--e.g. by setting some flag. That way we could special case places like this.

[AndriySvyryd]

We can use the ProductVersion annotation to do this only on pre 3.0.0 snapshots

[AndriySvyryd]

Also replace HasForeignKey and HasPrincipalKey overloads on ReferenceReferenceBuilder and ReferenceReferenceBuilder<,> that have the same issue.
When the type is not specified then the call order would determine which entity type is the dependent - the one on which the relationship configuration began.