Merge pull request #1686 from haacked/comment-256acf3b

Comment by Ken McCormack on recover-from-dbupdate-exception

_data/comments/recover-from-dbupdate-exception/256acf3b.yml

@@ -0,0 +1,5 @@
+id: 256acf3b
+date: 2023-11-30T13:22:02.6132921Z
+name: Ken McCormack
+avatar: https://secure.gravatar.com/avatar/e8d3a7d475e4749bc0fbe4c6a3f1bb77?s=80&d=identicon&r=pg
+message: "Thanks for the post Phil!  I held back a PR on DbContext pooling - because of this concern.  I couldn't find any doc on whether EF Core auto-cleans.  The symptom first revealed itself in a set of integration tests, which used the same DbContext for a series of fixture creates... (a null string in a non-nullable property will do it), so yes, if the DbContext gets an \"unsavable\" entity added to its state, all the tests in the suite go red. \r\n\r\n(So, would this leak away valid DbContexts in the pool, and cause random failures, slowly bringing the system down??)\r\n\r\nSo, another callout is that every consumer should not need to handle this... it's an orthogonal concern.  Analogous to this are services setting common fields like \"CreatedBy\" and \"CreatedDate\" (DRY fail code smell).   To get around this, we override SaveChangesAsync in our DbContext, and pass in a 'caller' object, so fields like CreatedBy / UpdatedBy, CreatedDate, UpdatedDate are set centrally (if the type implements a custom interface \"IAuditedEntity\") so, every repository was doing this, it's now one piece of code - see below - \r\n\r\npublic virtual async Task<int> SaveChangesAsync(UserId callerId)\r\n{\r\n    var now = _dateTimeService.UtcNow; // so that tests are deterministic\r\n\r\n    // ChangeTracker is null in a unit test - has to be an integ test\r\n    foreach (var changedEntity in ChangeTracker.Entries())\r\n    {\r\n        if (changedEntity.Entity is IAuditedEntity entity)\r\n        {\r\n            switch (changedEntity.State)\r\n            {\r\n                case EntityState.Added:\r\n                    entity.CreatedDate = now;\r\n                    entity.UpdatedDate = null;\r\n                    entity.CreatedBy = callerId;\r\n                    entity.UpdatedBy = null;\r\n                    break;\r\n                case EntityState.Modified:\r\n                    Entry(entity).Property(x => x.CreatedBy).IsModified = false;\r\n                    Entry(entity).Property(x => x.CreatedDate).IsModified = false;\r\n                    entity.UpdatedDate = now;\r\n                    entity.UpdatedBy = callerId;\r\n                    break; \r\n            }\r\n        }\r\n    }\r\n\r\n   // todo - add Detach in a try-catch here, to clean the pooled context\r\n    return await base.SaveChangesAsync();\r\n}\r\n\r\nSo, puzzle... how to write an integration test to replicate a parallel unit of work performing a breaking insert - it needs to interleave with the main flow's read and insert... \r\n\r\nI will try inheriting the DbContext (\"TestDbContext\"), and adding that into the container, casting it first to the base type.  The service should resolve the test context.  Its overload for SaveChangesAsync would create a new DbContext (if some test condition is met), and perform an insert, then continue with the service's unit of work... calling base.SaveChangesAsync().  This will throw a DbUpdateException, the DbContext should catch, detach the failing entity, and throw.  In the test, calling SaveChangesAsync a second time on the same DbContext instance (adding a new, valid, entity), should work.\r\n\r\n(That's a theory!) \r\n\r\n"