API docs for DbContext.Update etc. have incorrect info about state of related entities

[mikebrind]

Steps to reproduce

var context = new TestContext();
var author = new Author {
    AuthorId = 1,
    FirstName = "William",
    LastName = "Shakespeare"
};
author.Books.Add(new Book { Title = "Othello" });
context.Update(author);
context.SaveChanges();

The issue

I would expect, based on the summary for the Update method that the ChangeTracker should contain two entries as a result of the above code, both in the Modified state. I would also expect a DbUpdateException to be thrown when EF attempts to update a book with no key value provided. However, the Book entry is in the Added state, which, while it makes sense, does not conform to the documentation for the method.

I would also expect the IsKeySet property to be true for the author, but false for the book. It is true for both.

Further technical details

EF Core version: 1.0.1

{
  "version": "1.0.0-*",
  "buildOptions": {
    "emitEntryPoint": true
  },
  "dependencies": {
    "Microsoft.Extensions.Configuration": "1.0.0",
    "Microsoft.Extensions.Configuration.FileExtensions": "1.0.0",
    "Microsoft.Extensions.Configuration.Json": "1.0.0",
    "Microsoft.EntityFrameworkCore.SqlServer": "1.0.1",
    "Microsoft.EntityFrameworkCore.Tools": "1.0.0-preview2-final",
    "Microsoft.NETCore.App": {
      "type": "platform",
      "version": "1.0.0"
    }
  },
  "frameworks": {
    "netcoreapp1.0": {
      "imports": "dnxcore50"
    }
  },
  "tools": {
    "Microsoft.EntityFrameworkCore.Tools": "1.0.0-preview2-final"
  }
}

[mikebrind]

This behaviour also seems to apply to the DbContext.Attach method. If you attach an object graph that has dependent entities with no key value, the root entity state is Unchanged, but the dependent entity's state is Added and IsKeySet is true.

[mikebrind]

OK- I have just found #4424 which suggests that the behaviour I have reported is by design (despite the summary of the methods in source code).

Attach: Attaches every reachable entity, except where a reachable entity has a store generated key and no key value is assigned, these will be marked as added.
Update: Same as Attach, but entities are marked as modified

I guess that leaves one point of confusion in my mind: what exactly does IsKeySet do?

[rowanmiller]

Notes for triage: The API docs are wrong, I will fix this.

IsKeySet is interesting... for the book, it is false until it is added to the context, at which point it gets a temporary key and swaps to true. This aligns with the idea that it "indicates whether the key property is set to the CLR default for the property type", but it is arguably confusing. Our main intention was for IsKeySet to be used while attaching entities with TrackGraph, so I guess we need to decide if it makes sense to use it once an entity is being tracked.

[mikebrind]

I agree that it is confusing. IsKeySet suggests to me that a deliberate action took place at some point to set a valid value. Perhaps the property can be wrapped in a more aptly named method.

[rowanmiller]

Discussed at length and concluded that we want to leave IsKeySet as-is - it is a very literal "is there a value set on the key property". It is designed to be used during attaching of entities, typically via TrackGraph(...), and will always be true once an entity is tracked by the context. We already have an IsTemporary flag to allow checking whether a key value is temporary once the entity is attached.