-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Description
Hi and thanks for maintaining this great template, it's very useful to explore clean architecture in .NET.
I’d like to raise awareness of a potential data loss risk in SeedData.cs that could arise in some specific conditions, especially when developers use the sample as a base for real applications.
🔍 Context
In SeedData.cs, the following logic is used:
public static async Task InitializeAsync(AppDbContext dbContext)
{
if (await dbContext.ToDoItems.AnyAsync()) // DANGEROUS CHECK !!!
{
return;
}
// if dbContext.ToDoItems is emptied or removed, potential risk to loose all data !!!
await PopulateTestDataAsync(dbContext);
}
public static async Task PopulateTestDataAsync(AppDbContext dbContext)
{
foreach (var item in dbContext.Projects)
{
dbContext.Remove(item);
}
foreach (var item in dbContext.ToDoItems)
{
dbContext.Remove(item);
}
foreach (var item in dbContext.Contributors)
{
dbContext.Remove(item);
}
// etc etc other production tables could be added here
await dbContext.SaveChangesAsync();
This behavior may cause accidental data loss in production in the following (not unrealistic) situation:
- A production application uses the template but do not remove ToDoList, while not using it, and keeping the seeding mechanism in place (for seeding other data).
- A DBA or ops team removes or cleans the unused ToDoLists table in production to clean up the schema.
- A local/dev version of the application connects to the production DB (by mistake or for diagnostics using prod connection string).
To note that table removal in the DB will not generate exception, as we have in the MiddlewareConfig the EnsureCreatedAsync
so the eventually removed table is recreated.
await context.Database.EnsureCreatedAsync();
await SeedData.InitializeAsync(context);
So, while removing or truncating ToDoItems, the condition !context.ToDoLists.Any()
would be true, and the deletion logic before that would wipe all data from ToDoItems, ToDoLists etc etc, even if other parts of the application used them or the model was extended.
✅ Suggestion
It might be safer to:
- Do not delete data.
- Or provide a warning/comment in the template that this logic is only meant for sample/dev environments and should never be deployed as-is, however, there is always a risk the developers don't real the full code or all comments.
Again, this isn’t a bug per se, and certainly not a mistake from your side, but a small design choice in the sample code that might have big consequences in derived applications if developers aren’t careful.
Thanks again for your work on this project!