diff --git a/src/NosCore.Core/Persistence/IDaoTransactionScope.cs b/src/NosCore.Core/Persistence/IDaoTransactionScope.cs new file mode 100644 index 000000000..71e8585e5 --- /dev/null +++ b/src/NosCore.Core/Persistence/IDaoTransactionScope.cs @@ -0,0 +1,24 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using System; +using System.Threading.Tasks; + +namespace NosCore.Core.Persistence +{ + // Groups every DAO operation issued on the current async flow into one database + // transaction. Nothing is persisted until CommitAsync; disposing without + // committing rolls everything back. + public interface IDaoTransactionScope + { + IDaoTransaction Begin(); + } + + public interface IDaoTransaction : IAsyncDisposable + { + Task CommitAsync(); + } +} diff --git a/src/NosCore.Database/Hosting/DaoTransactionScope.cs b/src/NosCore.Database/Hosting/DaoTransactionScope.cs new file mode 100644 index 000000000..002fe5d82 --- /dev/null +++ b/src/NosCore.Database/Hosting/DaoTransactionScope.cs @@ -0,0 +1,60 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Storage; +using NosCore.Core.Persistence; +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace NosCore.Database.Hosting +{ + // Carries the transaction's DbContext to every DAO call on the same async flow: + // the DbContext registration consults this slot before building a fresh context. + // AsyncLocal keeps concurrent saves (Task.WhenAll over sessions) isolated from + // each other. + internal static class AmbientDbContext + { + private static readonly AsyncLocal Slot = new(); + + public static DbContext? Current => Slot.Value; + + public static void Set(DbContext? context) + { + Slot.Value = context; + } + } + + public sealed class DaoTransactionScope(Func contextFactory) : IDaoTransactionScope + { + // Synchronous on purpose: an AsyncLocal written inside an awaited method does + // not flow back to the caller, so the ambient slot must be set before the + // first await of the calling flow. + public IDaoTransaction Begin() + { + var context = contextFactory(); + var transaction = context.Database.BeginTransaction(); + AmbientDbContext.Set(context); + return new DaoTransaction(context, transaction); + } + + private sealed class DaoTransaction(NosCoreContext context, IDbContextTransaction transaction) : IDaoTransaction + { + public Task CommitAsync() + { + return transaction.CommitAsync(); + } + + public async ValueTask DisposeAsync() + { + AmbientDbContext.Set(null); + await transaction.DisposeAsync(); + await context.DisposeAsync(); + } + } + } +} diff --git a/src/NosCore.Database/Hosting/PersistenceModule.cs b/src/NosCore.Database/Hosting/PersistenceModule.cs index da94903cf..84e5de342 100644 --- a/src/NosCore.Database/Hosting/PersistenceModule.cs +++ b/src/NosCore.Database/Hosting/PersistenceModule.cs @@ -37,7 +37,18 @@ public PersistenceModule(Action? onDtoTypeRegistered = n protected override void Load(ContainerBuilder builder) { - builder.RegisterType().As(); + // DAOs resolve a fresh DbContext per operation through this registration; when + // a DaoTransactionScope is active on the current async flow they get its + // context instead, so the whole scope commits or rolls back as one. + builder.RegisterType().AsSelf().InstancePerDependency(); + builder.Register(c => AmbientDbContext.Current ?? (DbContext)c.Resolve()) + .As().InstancePerDependency(); + builder.Register(c => + { + var factory = c.Resolve>(); + return new DaoTransactionScope(factory); + }) + .As().SingleInstance(); builder.Register(c => c.Resolve>>().OfType>().ToDictionary( x => x.GetType().GetGenericArguments()[1], y => y.LoadAll().GroupBy(x => x.Key ?? "") @@ -82,6 +93,8 @@ protected override void Load(ContainerBuilder builder) public static void MirrorTo(IServiceCollection services) { services.AddTransient(); + services.AddSingleton(sp => + new DaoTransactionScope(() => sp.GetRequiredService())); foreach (var mapping in DiscoverDaoMappings()) { diff --git a/src/NosCore.GameObject/Services/SaveService/SaveService.cs b/src/NosCore.GameObject/Services/SaveService/SaveService.cs index e7d03bb8b..17df28465 100644 --- a/src/NosCore.GameObject/Services/SaveService/SaveService.cs +++ b/src/NosCore.GameObject/Services/SaveService/SaveService.cs @@ -4,6 +4,7 @@ // |_|\__|\__/ |___/ \__/\__/|_|_\___| // +using NosCore.Core.Persistence; using NosCore.Dao.Interfaces; using NosCore.Data.Dto; using NosCore.Data.Enumerations.I18N; @@ -27,7 +28,8 @@ public class SaveService(IDao characterDao, IDao characterQuestDao, IDao characterQuestObjectiveDao, IDao respawnDao, ILogger logger, - ILogLanguageLocalizer logLanguage) + ILogLanguageLocalizer logLanguage, + IDaoTransactionScope daoTransactionScope) : ISaveService { public async Task SaveAsync(ClientSession session) @@ -67,62 +69,112 @@ public async Task SaveAsync(ClientSession session) characterDto.SpAdditionPoint = character.SpAdditionPoint; characterDto.CurrentScriptId = character.CurrentScriptId; - await accountDao.TryInsertOrUpdateAsync(account); - await characterDao.TryInsertOrUpdateAsync(characterDto); + // Every DAO call below shares this scope's transaction: the DAOs swallow + // their own exceptions and report failure through their return value, so + // each result is checked and the commit only happens when all of them + // succeeded. Returning early rolls the whole save back. + void Fail(string operation) + { + logger.LogError( + new InvalidOperationException($"{operation} failed; character save rolled back."), + logLanguage[LogLanguageKey.SAVE_CHARACTER_FAILED], characterId); + } + + await using var transaction = daoTransactionScope.Begin(); + + if (await accountDao.TryInsertOrUpdateAsync(account) == null) + { + Fail("Account upsert"); + return; + } + + if (await characterDao.TryInsertOrUpdateAsync(characterDto) == null) + { + Fail("Character upsert"); + return; + } var quicklistEntriesToDelete = quicklistEntriesDao .Where(i => i.CharacterId == characterId)!.ToList() .Where(i => quicklistEntries.All(o => o.Id != i.Id)).ToList(); - await quicklistEntriesDao.TryDeleteAsync(quicklistEntriesToDelete.Select(s => s.Id).ToArray()); - await quicklistEntriesDao.TryInsertOrUpdateAsync(quicklistEntries); + if (await quicklistEntriesDao.TryDeleteAsync(quicklistEntriesToDelete.Select(s => s.Id).ToArray()) == null) + { + Fail("QuicklistEntry delete"); + return; + } + if (!await quicklistEntriesDao.TryInsertOrUpdateAsync(quicklistEntries)) + { + Fail("QuicklistEntry upsert"); + return; + } var itemsToDelete = inventoryItemInstanceDao .Where(i => i.CharacterId == characterId)!.ToList() .Where(i => inventoryService.Values.All(o => o.Id != i.Id)).ToList(); // Inventory delete order: child rows first, then parent ItemInstance rows. - await inventoryItemInstanceDao.TryDeleteAsync(itemsToDelete.Select(s => s.Id).ToArray()); - await itemInstanceDao.TryDeleteAsync(itemsToDelete.Select(s => s.ItemInstanceId).ToArray()); + if (await inventoryItemInstanceDao.TryDeleteAsync(itemsToDelete.Select(s => s.Id).ToArray()) == null) + { + Fail("InventoryItemInstance delete"); + return; + } + if (await itemInstanceDao.TryDeleteAsync(itemsToDelete.Select(s => s.ItemInstanceId).ToArray()) == null) + { + Fail("ItemInstance delete"); + return; + } // Inventory insert order: parent ItemInstance rows first so the FK on - // InventoryItemInstance.ItemInstanceId resolves on insert. The DAO swallows - // exceptions and returns false on failure, so we MUST check the result — - // otherwise a silent failure on the ItemInstance insert cascades into a - // confusing FK-violation error on the InventoryItemInstance insert that - // follows. Skipping the child insert keeps the failure mode loud and - // localized to the actual broken layer. - var itemInstancesSaved = await itemInstanceDao - .TryInsertOrUpdateAsync(inventoryService.Values.Select(s => s.ItemInstance).ToArray()); - if (!itemInstancesSaved) + // InventoryItemInstance.ItemInstanceId resolves on insert. + if (!await itemInstanceDao.TryInsertOrUpdateAsync(inventoryService.Values.Select(s => s.ItemInstance).ToArray())) { - logger.LogError( - new InvalidOperationException("ItemInstance batch insert failed; skipping InventoryItemInstance to avoid FK cascade."), - logLanguage[LogLanguageKey.SAVE_CHARACTER_FAILED], session.Character.CharacterId); + Fail("ItemInstance upsert"); + return; + } + if (!await inventoryItemInstanceDao.TryInsertOrUpdateAsync(inventoryService.Values.ToArray())) + { + Fail("InventoryItemInstance upsert"); return; } - await inventoryItemInstanceDao.TryInsertOrUpdateAsync(inventoryService.Values.ToArray()); var staticBonusToDelete = staticBonusDao .Where(i => i.CharacterId == characterId)!.ToList() .Where(i => staticBonusList.All(o => o.StaticBonusId != i.StaticBonusId)).ToList(); - await staticBonusDao.TryDeleteAsync(staticBonusToDelete.Select(s => s.StaticBonusId)); - await staticBonusDao.TryInsertOrUpdateAsync(staticBonusList); + if (await staticBonusDao.TryDeleteAsync(staticBonusToDelete.Select(s => s.StaticBonusId)) == null) + { + Fail("StaticBonus delete"); + return; + } + if (!await staticBonusDao.TryInsertOrUpdateAsync(staticBonusList)) + { + Fail("StaticBonus upsert"); + return; + } - await titleDao.TryInsertOrUpdateAsync(titles); + if (!await titleDao.TryInsertOrUpdateAsync(titles)) + { + Fail("Title upsert"); + return; + } var minilandDto = (MinilandDto)minilandProvider.GetMiniland(characterId); - await minilandDao.TryInsertOrUpdateAsync(minilandDto); + if (await minilandDao.TryInsertOrUpdateAsync(minilandDto) == null) + { + Fail("Miniland upsert"); + return; + } var questsToDelete = characterQuestDao .Where(i => i.CharacterId == characterId)!.ToList() .Where(i => quests.Values.All(o => o.QuestId != i.QuestId)).ToList(); - await characterQuestDao.TryDeleteAsync(questsToDelete.Select(s => s.Id)); - var questsSaved = await characterQuestDao.TryInsertOrUpdateAsync(quests.Values); - if (!questsSaved) + if (await characterQuestDao.TryDeleteAsync(questsToDelete.Select(s => s.Id)) == null) { - logger.LogError( - new InvalidOperationException("CharacterQuest upsert failed; skipping objective upsert to avoid FK cascade."), - logLanguage[LogLanguageKey.SAVE_CHARACTER_FAILED], characterId); + Fail("CharacterQuest delete"); + return; + } + if (!await characterQuestDao.TryInsertOrUpdateAsync(quests.Values)) + { + Fail("CharacterQuest upsert"); return; } @@ -151,24 +203,24 @@ public async Task SaveAsync(ClientSession session) live.Id = match.Id; } } - var objectivesDeleted = await characterQuestObjectiveDao.TryDeleteAsync(objectivesToDelete); - if (objectivesDeleted == null) + if (await characterQuestObjectiveDao.TryDeleteAsync(objectivesToDelete) == null) { - logger.LogError( - new InvalidOperationException("CharacterQuestObjective delete failed; skipping objective upsert to avoid orphaned-row conflicts on next save."), - logLanguage[LogLanguageKey.SAVE_CHARACTER_FAILED], characterId); + Fail("CharacterQuestObjective delete"); return; } - var objectivesSaved = await characterQuestObjectiveDao.TryInsertOrUpdateAsync(liveObjectives); - if (!objectivesSaved) + if (!await characterQuestObjectiveDao.TryInsertOrUpdateAsync(liveObjectives)) { - logger.LogError( - new InvalidOperationException("CharacterQuestObjective upsert failed; quest progress will reset on reconnect."), - logLanguage[LogLanguageKey.SAVE_CHARACTER_FAILED], characterId); + Fail("CharacterQuestObjective upsert"); + return; + } + + if (!await respawnDao.TryInsertOrUpdateAsync(character.Respawns)) + { + Fail("Respawn upsert"); return; } - await respawnDao.TryInsertOrUpdateAsync(character.Respawns); + await transaction.CommitAsync(); } catch (Exception e) { diff --git a/test/NosCore.GameObject.Tests/Services/SaveService/SaveServiceTests.cs b/test/NosCore.GameObject.Tests/Services/SaveService/SaveServiceTests.cs index 199cab18e..6194e7932 100644 --- a/test/NosCore.GameObject.Tests/Services/SaveService/SaveServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/SaveService/SaveServiceTests.cs @@ -5,6 +5,7 @@ // using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using NosCore.Dao; @@ -13,6 +14,7 @@ using NosCore.Data.Enumerations.Map; using NosCore.Data.StaticEntities; using NosCore.Database; +using NosCore.Database.Hosting; using NosCore.Database.Entities; using NosCore.GameObject.Networking.ClientSession; using NosCore.GameObject.Services.ItemGenerationService; @@ -45,7 +47,8 @@ public async Task SetupAsync() ItemProvider = TestHelpers.Instance.GenerateItemProvider(); var optionsBuilder = new DbContextOptionsBuilder().UseInMemoryDatabase( - Guid.NewGuid().ToString()); + Guid.NewGuid().ToString()) + .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning)); NosCoreContext ContextBuilder() => new NosCoreContext(optionsBuilder.Options); var itemInstanceDao = new Dao(NullLogger>.Instance, ContextBuilder); @@ -76,7 +79,8 @@ public async Task SetupAsync() characterQuestObjectiveDao, respawnDao, NullLogger.Instance, - TestHelpers.Instance.LogLanguageLocalizer); + TestHelpers.Instance.LogLanguageLocalizer, + new DaoTransactionScope(ContextBuilder)); } [TestMethod]