Change API surface for using fields with navigation properties

[ajcvickers]

Because navigation properties are configured using the relationship API there is not currently any way to easily address the navigation properties such that backing fields can be easily configured.

[ajcvickers]

This is the current code needed to set property access mode for a navigation property:

modelBuilder.Entity<Blog>()
    .Metadata
    .FindNavigation(nameof(Blog.Posts))
    .SetPropertyAccessMode(PropertyAccessMode.Field);

Proposal 1:

modelBuilder.Entity<Blog>()
    .HasMany(e => e.Posts)
    .WithOne(e => e.Blog)
    .UsePropertyAccessMode(PropertyAccessMode.Field);

This would set the property access mode for all navigation properties of the relationship. This is probably sufficient granularity for almost all applications. Dropping down to core metadata can still be used if each navigation property needs different access.

Proposal 2:
As above, but also sets the property access mode of the FK. This makes the following a little less ambiguous:

modelBuilder.Entity<Blog>()
    .HasMany(e => e.Posts)
    .WithOne(e => e.Blog)
    .HasForeignKey(e => e.BlogId)
    .UsePropertyAccessMode(PropertyAccessMode.Field);

Proposal 3:
As above, but with a modifier flag to choose which of the dependent/principal/FK property to set the access mode for. The default would be to set it for all--i.e. collapsing into proposal 2.

modelBuilder.Entity<Blog>()
    .HasMany(e => e.Posts)
    .WithOne(e => e.Blog)
    .HasForeignKey(e => e.BlogId)
    .UsePropertyAccessMode(
       PropertyAccessMode.Field, 
       Navigation.Dependant | Navigation.Principal);

Proposal 4:
Add an optional second argument to HasMany, WithOne, etc. that allows a nested closure of configuration for the navigation property.

modelBuilder.Entity<Blog>()
    .HasMany(
        e => e.Posts, 
        nb => nb.UsePropertyAccessMode(PropertyAccessMode.Field))
    .WithOne(
        e => e.Blog, 
        nb => nb.UsePropertyAccessMode(PropertyAccessMode.Field))

This has the advantage that it is a more general solution for the addressing of navigation properties in the fluent API, but may be over-kill.

[rowanmiller]

We will do #4 and can look at sugar in the future if folks get confused

[julielerman]

With the current workaround syntax that allows us to use the Metadata to identify a backing field for a navigation property that is a collection, is there a way to do that when the navigation property is an entity type, not iEnumerable? Whether I express the property the way you did above (nameof(type.property)) or create a propertyInfo object and pass that in, FindNavigation is returning null.
I've just used the "old fashioned" way for now, making the get public and having a private setter.
Thanks

[ajcvickers]

@julielerman Can you post a code snippet that is not working? It should not matter if the navigation is private or not or a collection or not, but maybe there is something not working correctly in the metadata lookup?

[julielerman]

Here are a few attempts: ```C# modelBuilder.Entity<Samurai>() .Metadata .FindNavigation (nameof (Samurai.Entrance)) .SetPropertyAccessMode (PropertyAccessMode.Field); var propinfo = typeof (Samurai).GetProperty ("Entrance"); modelBuilder.Entity<Samurai> () .Metadata .FindNavigation (propinfo) .SetPropertyAccessMode (PropertyAccessMode.Field); modelBuilder.Entity<Samurai> () .Metadata .FindNavigation (nameof (Entrance)) .SetPropertyAccessMode (PropertyAccessMode.Field); ``` In each case, FindNavigation returns null. I can recreate a focused sample if this isn't quickly obvious. ("can" = "I'm doing that now cuz I can't help myself")

[julielerman]

mini repro still fails. I'll show it to you if you aren't having the problem (e.g. I'm doing something wrong ) FWIW I'm using rtm-* and

Product Information:
 Version:            2.0.0-preview3-006736
 Commit SHA-1 hash:  49dfa8e96b

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.12
 OS Platform: Darwin
 RID:         osx.10.12-x64
 Base Path:   /usr/local/share/dotnet/sdk/2.0.0-preview3-006736/

Microsoft .NET Core Shared Framework Host

  Version  : 2.0.0-preview3-25511-03
  Build    : 6e7dadfdaed031624319dfd61415b2ce3498aa51

[ajcvickers]

@julielerman I am not able to reproduce this. Can you post the entity types and OnModelCreating code?

[ajcvickers]

Ah, it might be this. If the relationship is configured like this:

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

which looks reasonable, but is wrong, then it will result in no exception, but also will not configure the relationship correctly. This is because in the single string overload the string is the entity type name, not the navigation name. This is pretty confusing. @AndriySvyryd @divega.

The correct way to configure it is something like:

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

[smitpatel]

There isn't overload of HasOne which takes 1 string parameter. How the first one would compile?

[ajcvickers]

@smitpatel It compiled for me...

[ajcvickers]

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

[julielerman]

Oh! I didn’t explicitly configure the relationship. It’s one of those nice FK in the dependent mappings that EF Core can figure out. I’ll try that first! Can’t do it right away though. But good to know there IS a way! Thanks

[divega]

@ajcvickers 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.

[julielerman]

I'm still messing about with this trying to find the pattern that works.
I'll go back to my original setup:

public class Samurai {
  public int Id { get; set; }
  public string Name { get; set; }
  private Entrance Entrance{get; set;} <- totally private like I've done with collection
  public void CreateEntrance(string description){
    Entrance=new Entrance{Description=description};
  }
  public string EntranceDescription=>Entrance.Description;
  public int EntranceId=>Entrance.Id;  //this was just for testing :)
}
public class Entrance {
  public int Id { get; set; }
  public string Description { get; set; }
  public int SamuraiId { get; set; } <- letting convention figure out this is dependent
}

Before trying to encapsulate the Entrance property, this was fine with the EFCore groking the 1:1 relationship and that Entrance was the dependent.

I had no other configs in the context than those listed above.
I had to use the propertyinfo setup since Entrance wasn't visible to the compiler.
This ran but caused FindNavigation to return null.

Focusing on the fact that you seem to have explicit Fluent Api configuration, I first tried adding in Arthurs first suggestion .... even trying with no parameter in withOne and still not adding the samurai navigation property to Entrance:

protected override void OnModelCreating (ModelBuilder modelBuilder) {
   modelBuilder.Entity<Samurai> ()
     .HasOne (typeof (Entrance), "Entrance")
     .WithOne ();
   var propinfo = typeof (Samurai).GetProperty ("Entrance");
   modelBuilder.Entity<Samurai> ()
     .Metadata
     .FindNavigation (propinfo)
     .SetPropertyAccessMode (PropertyAccessMode.Field);
   base.OnModelCreating (modelBuilder);
 }

Again, it threw when it hit FindNavigation.

Next I added in the Samurai navigation property to Entrance and added "Samurai" as the parameter of WithOne(). (Interesting that I couldn't use a lambda. Is that because I didn't use a lambda with the HasOne?)
Here's what it looks like for this iteration

protected override void OnModelCreating (ModelBuilder modelBuilder) {
    modelBuilder.Entity<Samurai> ()
      .HasOne (typeof (Entrance), "Entrance")
      .WithOne ("Samurai");
    var propinfo = typeof (Samurai).GetProperty ("Entrance");
    modelBuilder.Entity<Samurai> ()
      .Metadata
      .FindNavigation (propinfo)
      .SetPropertyAccessMode (PropertyAccessMode.Field);
    }

I got the same result...null error on FindNavigation.

Do you see yet what I'm doing differently than you?

[ajcvickers]

There are quite a few things going on here, so I'll try to explain them one by one. First the relationship discovery. Using this code:

public class Samurai
{
    public int Id { get; set; }

    public string Name { get; set; }

    public Entrance Entrance { get; set; }

    public void CreateEntrance(string description)
        => Entrance = new Entrance { Description = description };

    public string EntranceDescription => Entrance.Description;

    public int EntranceId => Entrance.Id;
}

public class Entrance
{
    public int Id { get; set; }

    public string Description { get; set; }

    public int SamuraiId { get; set; }
}

public class SamuraiContext : DbContext
{
    public DbSet<Samurai> Samurais { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseSqlServer(
            @"Server=(localdb)\mssqllocaldb;Database=Test;Trusted_Connection=True;ConnectRetryCount=0");

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
    }
}

Notice that the navigation property is public. Conventions only use public navigation properties when traversing the graph to discover relationships. (We tried various other options early in EF Core pre-releases, but trying to use non-public members tended to bring in too many things erroneously.) So because there is a public navigation from Samurai to Entrance, Entrance is brought into the model together with the association between the types. At this point the relationship exists, so the FK can be discovered, etc. and the relationship becomes a unidirectional 1:1.

Now if the navigation property is made private it will no longer be found by convention. Therefore, it has to be configured explicitly--this is generally true for navigations and properties; once they are made private, they need to be configured. The way to configure navigation properties in EF is to use the relationship API:

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Samurai>().HasOne(typeof(Entrance), "Entrance").WithOne();
    }
}

Notice that the FK still does not need to be configured because once EF knows about the private navigation property it can then find the public FK by convention.

Also notice that I use a string here for the navigation name. If it were public I could have the compiler help me generate the string and keep in sync with the code using nameof(Samurai.Entrance). But after compilation the result is the same--call to HasOne with a string as the second argument.

Now, moving on to the code to force use of a backing field, for which we need to find the navigation property in the underlying metadata. The easiest way to do this with a private navigation is again to use its name as a string:

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Samurai>().HasOne(typeof(Entrance), "Entrance").WithOne();

        var navigation = modelBuilder.Entity<Samurai>()
            .Metadata
            .FindNavigation("Entrance");

        navigation.SetPropertyAccessMode(PropertyAccessMode.Field);
    }

I'm not using nameof as before because the navigation is private, but the code is again exactly the same after compilation.

Notice that this is only a call to find an existing navigation from metadata--it will never create the navigation in the way that a fluent API might.

It is possible to use the PropertyInfo instead of the string, but this doesn't usually help anything because you need to use the string anyway to find the PropertyInfo. (The PropertyInfo overloads are useful when you already have a PropertyInfo from doing other things and don't want EF to go look it up again.) Now if the navigation property is public, then this will work:

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Samurai>().HasOne(typeof(Entrance), "Entrance").WithOne();

        var propinfo = typeof(Samurai).GetProperty("Entrance");

        var navigation = modelBuilder.Entity<Samurai>()
            .Metadata
            .FindNavigation(propinfo);

        navigation.SetPropertyAccessMode(PropertyAccessMode.Field);
    }

But if the navigation is private then GetProperty returns null because it only finds public properties. Of course, there are ways of getting the PropertyInfo for a private property, and then this would work, but I think it's much easier just to use the string directly and let EF take care of the rest.

Hope this helps!

[julielerman]

Thanks Arthur.

The bottom line was that the only change I needed was to use the string in FindNavigation. That was one thing I had either never tried, or tried with the wrong set up and didn't try again after adjusting things.
My entities:

public class Samurai {
  public int Id { get; set; }
  public string Name { get; set; }
  private Entrance Entrance {get; set;} //note this is private YAY
  public void CreateEntrance(string description){
    Entrance=new Entrance{Description=description};
  }
   public string EntranceDescription=>Entrance?.Description;
}
public class Entrance {
  public int Id { get; set; }
  public string Description { get; set; }
  public int SamuraiId { get; set; }  //note there is no navigation property YAY
 }

My context:

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;


public class MyContext : DbContext {
  public DbSet<Samurai> Samurais { get; set; }
  protected override void OnModelCreating (ModelBuilder modelBuilder) {
    modelBuilder.Entity<Samurai>().HasOne (typeof (Entrance), "Entrance").WithOne ();
    modelBuilder.Entity<Samurai> ()
      .Metadata
      .FindNavigation ("Entrance")
      .SetPropertyAccessMode (PropertyAccessMode.Field);
  }
  protected override void OnConfiguring (DbContextOptionsBuilder optionsBuilder) {
    optionsBuilder.UseInMemoryDatabase ("OwnedOneToOne");
  }
}

I'm able to build and store Samurai/Entrance graphs and even query using Include (with the string).

Thank you.

[ajcvickers]

@julielerman Curious, are you actually going to create a backing field for Entrance? As the code is now the behavior should be the same regardless of whether you set PropertyAccessMode.Field or not, since the property is an auto-property and there is no difference accessing through the property or field.

[julielerman]

This is interesting and a really nice surprise. With the hasone/withone I can make the Entrance private with {get;set;} and EF sees it. Is that because the backing field logic is picking up the auto property? I would never have thought to try that.
Now I will add in a backing field and see the difference (with/without the setpropertyaccessmode config)

[lajones]

Poaching.

[AndriySvyryd]

@lajones Take a look at #11610, especially the tests should be useful

[lajones]

Thanks @AndriySvyryd . I came to the same conclusion. Only I didn't want to use NonRelationshipTestBase as my tests are specifically about navigations - so created a new class in the same directory.

[lajones]

• Whichever approach the team likes remove the other one