dbContext.Update()'s current design is a great security threat.

[TanvirArjel]

Although this issue has been filed in https://github.com/dotnet/efcore/issues/10194 and has been closed by saying it's by design. So I am talking about this design which has a serious loophole and is offering a great threat to flood databases with unwanted data.

Consider the following update method:

 public async Task<IActionResult> Update()
 {
     Post postsToBeUpdated = await _dbContext.Posts.AsNoTracking().FirsOrDefaultAsync();
     postsToBeUpdated.Id = 0; // Let say somehow Id value has been set to 0 during mapping.

     _dbContext.Posts.Update(postsToBeUpdated); // This line is generating unwanted Insert command instead of update command with Id = 0 which will throw an exception as expected.
     await _dbContext.SaveChangesAsync();
     return View();
 }

First of all, your design is completely insane. You can not insert unwanted data for an invalid update command which was supposed to throw an exception.

Secondly, Your method name is Update but you are doing Insert for an invalid scenario which is quite ridiculous. If you want to give Insert and Update with the same method then add a new method called AddOrUpdate() so that the user knows what he is doing. More details are Here

Finally, your current misdesign flooded my production database with unwanted records instead of throwing an exception.

I always believe EF Core has a great team, so they should not do such a silly design mistake. Please seriously take care of this.

EF Core version: 5.0.1 Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer) Target framework: (e.g. .NET 5.0) Operating system: IDE: (e.g. Visual Studio 2019 16.9)

[anranruye]

I agree with you. Default value of not nullable CLR type may also be valid key value in the database.

The EF team should consider allow nullable type for key property if the user need to store default value primary key. Then ef core can consider entities with null pk as new entities and "0" as existed entity.

Update: In fact, ef core already allow you to use nullable type primary key property. Then ef core will not consider entities which have '0' key value as new entities. This can be a workaround.

The Update method can do both insert and update. I think this behavior is mainly designed to track related entities of a disconnected entity. These related entities may include both existed and new entities. But this behavior should not apply to the disconnected entity itself, the entity itself should always be tracked as Modified state even if it has a default value key. However, if the entities which have '0' key value are act as related entities, there will still be an Ambiguity unless you use nullable CLR type for the pk property. I think this ambiguity is acceptable.

[TanvirArjel]

@anranruye, Update method should always generate SQL for the update command with the data provided. It will throw an exception at the database level if the provided values are invalid. This is straight and simple.

[anranruye]

@TanvirArjel I don't think so.

It is true that orm tools are designed to communicate with database. But not only relational database, also other data providers. In fact, ef core try to hide underlying data provider from the user. When you configure the DbContext to map to database, you should consider all the knowledge about the data provider you are using. However, when you use the DbContext to do query and change, you should forget all these about tables, relationships, constraints and other things in relational databases. Just consider same as you are working with objects which store in memory.

If you think in this way, then you can understand that functions of ef core and relational database are not all the same. For example, relational database cannot store collection of liter by its limit, and only positive primary key values make sense in ef core by its defualt design(if you use code-first and migration to create database, you will notice that the database generated keys are start from "1")(this can also explain why ef core considers entities which have "0" key value as new entities, "0" is reserved and has special meaning, but "0" may be not special in database). In other words, a function which is not supported by a specified db can be supported by ef(work with another data provider). In reverse, a valid value in database may not be valid in ef.

Also, the term Update has different meanings in ef layer and db layer. In ef core, Update means update an entity object in which its related entities are considered a part of it. While in db, Update may only means Update Sql statement.

[ajcvickers]

Using the default value to indicate a key value has not been set is very useful when using generated keys. This is a key part of the design for Update/Attach. If you don't want this behavior, then either don't use generated keys, or use a nullable primary key property such that null indicates a default value.