From 570624cfda27117026e5a3471ebc9cdfc371c546 Mon Sep 17 00:00:00 2001 From: "Babeckis, Arnas" Date: Thu, 18 Jun 2026 12:05:40 -0400 Subject: [PATCH 1/2] Fixed logging overrides breaking the device state logging. --- .../State/Logging/StateLoggerManagerTests.cs | 215 ++++++++++++++++++ .../State/Logging/StateLoggerManager.cs | 83 ++++--- .../Execution/Executors/CampaignExecutor.cs | 8 +- 3 files changed, 274 insertions(+), 32 deletions(-) create mode 100644 Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs diff --git a/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs b/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs new file mode 100644 index 00000000..e4a0d50b --- /dev/null +++ b/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs @@ -0,0 +1,215 @@ +using Ares.Core.Device.Providers; +using Ares.Core.Device.State.Logging; +using Ares.Core.Tests.Data.Device; +using Ares.Datamodel.Device; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; +using Moq; + +namespace Ares.Core.Tests.Device.State.Logging; + +internal class StateLoggerManagerTests +{ + private DbContextOptions _dbOptions; + private Mock> _dbContextFactory; + private DeviceStateLoggerRepository _repository; + private Mock _loggerFactory; + private StateLoggerManager _manager; + + [SetUp] + public void SetUp() + { + _dbOptions = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + _dbContextFactory = new Mock>(); + _dbContextFactory + .Setup(factory => factory.CreateDbContext()) + .Returns(() => new CoreDatabaseContext(_dbOptions)); + _repository = new DeviceStateLoggerRepository(); + _loggerFactory = new Mock(); + _manager = new StateLoggerManager( + _repository, + _loggerFactory.Object, + Mock.Of>(), + _dbContextFactory.Object, + Mock.Of()); + } + + [Test] + public async Task EnableOverrideAsync_WithLoggingEnabled_AppliesEnabledClone() + { + var logger = AddLogger("device-1"); + var settings = CreateSettings(loggingEnabled: false); + DeviceLoggingSettings appliedSettings = null; + logger + .Setup(stateLogger => stateLogger.UpdateSettings(It.IsAny())) + .Callback(value => appliedSettings = value) + .Returns(Task.CompletedTask); + + await _manager.EnableOverrideAsync(settings, true); + + Assert.Multiple(() => + { + Assert.That(appliedSettings, Is.Not.Null); + Assert.That(appliedSettings!.LoggingEnabled, Is.True); + Assert.That(appliedSettings.LoggingType, Is.EqualTo(settings.LoggingType)); + Assert.That(settings.LoggingEnabled, Is.False); + Assert.That(appliedSettings, Is.Not.SameAs(settings)); + }); + } + + [Test] + public async Task EnableOverrideAsync_WithLoggingDisabled_AppliesDisabledSettings() + { + var logger = AddLogger("device-1"); + DeviceLoggingSettings appliedSettings = null; + logger + .Setup(stateLogger => stateLogger.UpdateSettings(It.IsAny())) + .Callback(value => appliedSettings = value) + .Returns(Task.CompletedTask); + + await _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), false); + + Assert.That(appliedSettings, Is.Not.Null); + Assert.That(appliedSettings!.LoggingEnabled, Is.False); + } + + [Test] + public async Task DisableOverrideAsync_RestoresPersistedSettings() + { + var logger = AddLogger("device-1"); + var persistedSettings = CreateSettings("device-1", loggingEnabled: false); + await SaveSettings(persistedSettings); + + await _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), true); + await _manager.DisableOverrideAsync(); + + logger.Verify( + stateLogger => stateLogger.UpdateSettings(It.Is( + settings => settings.DeviceId == "device-1" && !settings.LoggingEnabled)), + Times.Once); + } + + [Test] + public async Task DisableOverrideAsync_RestoresSettingsUpdatedDuringOverride() + { + var logger = AddLogger("device-1"); + await SaveSettings(CreateSettings("device-1", loggingEnabled: false)); + await _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), true); + var updatedSettings = new DeviceLoggingSettings + { + DeviceId = "device-1", + LoggingEnabled = true, + LoggingType = DeviceLoggingSettings.Types.LoggingType.Interval, + IntervalMs = 250 + }; + + await _manager.UpdateLogger("device-1", updatedSettings); + await _manager.DisableOverrideAsync(); + + logger.Verify( + stateLogger => stateLogger.UpdateSettings(It.Is( + settings => settings.DeviceId == "device-1" + && settings.LoggingEnabled + && settings.LoggingType == DeviceLoggingSettings.Types.LoggingType.Interval + && settings.IntervalMs == 250)), + Times.Once); + } + + [Test] + public async Task SetupLogger_DuringOverride_StartsWithOverrideSettings() + { + var device = new TestDevice("Device", "device-1"); + var logger = CreateFactoryLogger(device.UniqueId); + var suppliedSettings = CreateSettings(loggingEnabled: false); + DeviceLoggingSettings startedSettings = null; + logger + .Setup(stateLogger => stateLogger.Start(It.IsAny())) + .Callback(value => startedSettings = value) + .Returns(Task.CompletedTask); + + await _manager.EnableOverrideAsync(suppliedSettings, true); + await _manager.SetupLogger(device); + + Assert.Multiple(() => + { + Assert.That(startedSettings, Is.Not.Null); + Assert.That(startedSettings!.LoggingEnabled, Is.True); + Assert.That(startedSettings, Is.Not.SameAs(suppliedSettings)); + }); + } + + [Test] + public async Task AfterDisable_NewDevicesUseDatabaseSettingsAndUpdatesApplyNormally() + { + var existingLogger = AddLogger("existing-device"); + existingLogger + .Setup(logger => logger.UpdateSettings(It.IsAny())) + .Returns(Task.CompletedTask); + await _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), true); + await _manager.DisableOverrideAsync(); + + var device = new TestDevice("Device", "device-1"); + var persistedSettings = CreateSettings(device.UniqueId, loggingEnabled: false); + await SaveSettings(persistedSettings); + var newLogger = CreateFactoryLogger(device.UniqueId); + DeviceLoggingSettings startedSettings = null; + newLogger + .Setup(logger => logger.Start(It.IsAny())) + .Callback(value => startedSettings = value) + .Returns(Task.CompletedTask); + + await _manager.SetupLogger(device); + var updatedSettings = CreateSettings(device.UniqueId, loggingEnabled: true); + await _manager.UpdateLogger(device.UniqueId, updatedSettings); + + Assert.Multiple(() => + { + Assert.That(startedSettings, Is.Not.Null); + Assert.That(startedSettings.DeviceId, Is.EqualTo(persistedSettings.DeviceId)); + Assert.That(startedSettings.LoggingEnabled, Is.EqualTo(persistedSettings.LoggingEnabled)); + Assert.That(startedSettings.LoggingType, Is.EqualTo(persistedSettings.LoggingType)); + Assert.That(startedSettings.IntervalMs, Is.EqualTo(persistedSettings.IntervalMs)); + }); + newLogger.Verify(logger => logger.UpdateSettings(updatedSettings), Times.Once); + } + + private Mock AddLogger(string deviceId) + { + var logger = new Mock(); + logger.SetupGet(stateLogger => stateLogger.DeviceId).Returns(deviceId); + logger + .Setup(stateLogger => stateLogger.UpdateSettings(It.IsAny())) + .Returns(Task.CompletedTask); + _repository[deviceId] = logger.Object; + return logger; + } + + private Mock CreateFactoryLogger(string deviceId) + { + var logger = new Mock(); + logger.SetupGet(stateLogger => stateLogger.DeviceId).Returns(deviceId); + logger.Setup(stateLogger => stateLogger.Start(It.IsAny())).Returns(Task.CompletedTask); + _loggerFactory.Setup(factory => factory.Create(It.IsAny())).Returns(logger.Object); + return logger; + } + + private async Task SaveSettings(DeviceLoggingSettings settings) + { + await using var context = new CoreDatabaseContext(_dbOptions); + context.DeviceLoggingSettings.Add(settings); + await context.SaveChangesAsync(); + } + + private static DeviceLoggingSettings CreateSettings(string deviceId = "", bool loggingEnabled = true) + { + return new DeviceLoggingSettings + { + DeviceId = deviceId, + LoggingEnabled = loggingEnabled, + LoggingType = DeviceLoggingSettings.Types.LoggingType.OnChange, + IntervalMs = 100 + }; + } +} diff --git a/Ares.Core/Device/State/Logging/StateLoggerManager.cs b/Ares.Core/Device/State/Logging/StateLoggerManager.cs index 38f8aba8..f6afb469 100644 --- a/Ares.Core/Device/State/Logging/StateLoggerManager.cs +++ b/Ares.Core/Device/State/Logging/StateLoggerManager.cs @@ -20,6 +20,7 @@ public class StateLoggerManager private readonly IAresDeviceProvider _deviceProvider; private readonly CompositeDisposable _cleanup = new(); private bool _overrideActive; + private DeviceLoggingSettings? _overrideSettings; private readonly SemaphoreSlim _overrideLock = new(1, 1); public StateLoggerManager(IDeviceStateLoggerRepository stateLoggerRepository, @@ -68,44 +69,65 @@ private async Task HandleChangesAsync(Change change) public async Task SetupLogger(IAresDevice device) { - // Stop and remove any existing logger for the device - if(_stateLoggerRepository.TryGetValue(device.UniqueId, out var existingLogger)) - { - await existingLogger.Stop(); - _stateLoggerRepository.Remove(device.UniqueId); - } + await _overrideLock.WaitAsync(); - if(_deviceLoggerFactory is null) + try { - _logger.LogError("No suitable logger factory found for device type {DeviceType}", device.GetType().Name); - return; - } + // Stop and remove any existing logger for the device + if(_stateLoggerRepository.TryGetValue(device.UniqueId, out var existingLogger)) + { + await existingLogger.Stop(); + _stateLoggerRepository.Remove(device.UniqueId); + } + + if(_deviceLoggerFactory is null) + { + _logger.LogError("No suitable logger factory found for device type {DeviceType}", device.GetType().Name); + return; + } - var logger = _deviceLoggerFactory.Create(device); + var logger = _deviceLoggerFactory.Create(device); - using var ctx = _dbContextFactory.CreateDbContext(); - var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == device.UniqueId); + using var ctx = _dbContextFactory.CreateDbContext(); + var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == device.UniqueId); + var settings = _overrideActive ? _overrideSettings?.Clone() : existingSettings; - await logger.Start(existingSettings); - _stateLoggerRepository.Add(device.UniqueId, logger); + await logger.Start(settings); + _stateLoggerRepository.Add(device.UniqueId, logger); + } + finally + { + _overrideLock.Release(); + } } public async Task RemoveLogger(string deviceId) { - if(_stateLoggerRepository.TryGetValue(deviceId, out var logger)) + await _overrideLock.WaitAsync(); + + try { - await logger.Stop(); - _stateLoggerRepository.Remove(deviceId); - } + if(_stateLoggerRepository.TryGetValue(deviceId, out var logger)) + { + await logger.Stop(); + _stateLoggerRepository.Remove(deviceId); + } - using var ctx = _dbContextFactory.CreateDbContext(); - var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == deviceId); - if(existingSettings is not null) + using var ctx = _dbContextFactory.CreateDbContext(); + var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == deviceId); + if(existingSettings is not null) + { + ctx.DeviceLoggingSettings.Remove(existingSettings); + } + + await ctx.SaveChangesAsync(); + + _logger.LogInformation("Removed logger for device id {DeviceId}", deviceId); + } + finally { - ctx.DeviceLoggingSettings.Remove(existingSettings); + _overrideLock.Release(); } - - _logger.LogInformation("Removed logger for device id {DeviceId}", deviceId); } public async Task UpdateLogger(string deviceId, DeviceLoggingSettings settings) @@ -114,10 +136,11 @@ public async Task UpdateLogger(string deviceId, DeviceLoggingSettings settings) { await _overrideLock.WaitAsync(); - await UpdateDatabase(deviceId, settings); try { + await UpdateDatabase(deviceId, settings); + if(!_overrideActive) { await logger.UpdateSettings(settings); @@ -206,23 +229,27 @@ public async Task DisableOverrideAsync() } finally { + _overrideActive = false; + _overrideSettings = null; _overrideLock.Release(); _logger.LogDebug("Released the device logging override lock. (Disable method)"); } } - public async Task EnableOverrideAsync(DeviceLoggingSettings settings) + public async Task EnableOverrideAsync(DeviceLoggingSettings settings, bool loggingEnabled) { _logger.LogInformation("Attempting to enable device logging override."); await _overrideLock.WaitAsync(); + _overrideSettings = settings.Clone(); + _overrideSettings.LoggingEnabled = loggingEnabled; _overrideActive = true; try { _logger.LogDebug("Preparing the tasks to enable device logging override"); var loggerOverrideTasks = _stateLoggerRepository - .Select(async logger => await logger.Value.UpdateSettings(settings)); + .Select(logger => logger.Value.UpdateSettings(_overrideSettings.Clone())); await Task.WhenAll(loggerOverrideTasks); _logger.LogInformation("Enabled state logging override"); diff --git a/Ares.Core/Execution/Executors/CampaignExecutor.cs b/Ares.Core/Execution/Executors/CampaignExecutor.cs index 1ff90f92..515bea48 100644 --- a/Ares.Core/Execution/Executors/CampaignExecutor.cs +++ b/Ares.Core/Execution/Executors/CampaignExecutor.cs @@ -110,10 +110,10 @@ internal CampaignExecutor(ICommandComposer Execute(ExecutionControlToken token) { // Make sure we log all the device state changes during execution. - await _stateLoggerManager.EnableOverrideAsync(new Datamodel.Device.DeviceLoggingSettings - { - LoggingType = Datamodel.Device.DeviceLoggingSettings.Types.LoggingType.OnChange - }); + await _stateLoggerManager.EnableOverrideAsync(new Datamodel.Device.DeviceLoggingSettings + { + LoggingType = Datamodel.Device.DeviceLoggingSettings.Types.LoggingType.OnChange + }, true); await _planningHelper.ReseedManualPlanner(); From 211288f82c47a867b850a8152d8c739792ac5585 Mon Sep 17 00:00:00 2001 From: "Babeckis, Arnas" Date: Thu, 18 Jun 2026 12:56:46 -0400 Subject: [PATCH 2/2] More logging bugfixes that optimizer thought were pertinent. --- .../State/Logging/StateLoggerManagerTests.cs | 104 +++++++++++++++- .../Device/Remote/RemoteDeviceManager.cs | 2 +- .../State/Logging/AresDeviceStateLogger.cs | 2 + .../State/Logging/StateLoggerManager.cs | 113 +++++++++++++----- 4 files changed, 187 insertions(+), 34 deletions(-) diff --git a/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs b/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs index e4a0d50b..c451e7cd 100644 --- a/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs +++ b/Ares.Core.Tests/Device/State/Logging/StateLoggerManagerTests.cs @@ -54,6 +54,7 @@ public async Task EnableOverrideAsync_WithLoggingEnabled_AppliesEnabledClone() Assert.That(appliedSettings, Is.Not.Null); Assert.That(appliedSettings!.LoggingEnabled, Is.True); Assert.That(appliedSettings.LoggingType, Is.EqualTo(settings.LoggingType)); + Assert.That(appliedSettings.DeviceId, Is.EqualTo("device-1")); Assert.That(settings.LoggingEnabled, Is.False); Assert.That(appliedSettings, Is.Not.SameAs(settings)); }); @@ -91,6 +92,28 @@ public async Task DisableOverrideAsync_RestoresPersistedSettings() Times.Once); } + [Test] + public async Task DisableOverrideAsync_WithoutPersistedSettings_DisablesLogger() + { + var logger = AddLogger("device-1"); + DeviceLoggingSettings restoredSettings = null; + logger + .Setup(stateLogger => stateLogger.UpdateSettings(It.IsAny())) + .Callback(value => restoredSettings = value) + .Returns(Task.CompletedTask); + + await _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), true); + await _manager.DisableOverrideAsync(); + + Assert.Multiple(() => + { + Assert.That(restoredSettings, Is.Not.Null); + Assert.That(restoredSettings.DeviceId, Is.EqualTo("device-1")); + Assert.That(restoredSettings.LoggingEnabled, Is.False); + Assert.That(restoredSettings.LoggingType, Is.EqualTo(DeviceLoggingSettings.Types.LoggingType.None)); + }); + } + [Test] public async Task DisableOverrideAsync_RestoresSettingsUpdatedDuringOverride() { @@ -136,6 +159,7 @@ public async Task SetupLogger_DuringOverride_StartsWithOverrideSettings() { Assert.That(startedSettings, Is.Not.Null); Assert.That(startedSettings!.LoggingEnabled, Is.True); + Assert.That(startedSettings.DeviceId, Is.EqualTo(device.UniqueId)); Assert.That(startedSettings, Is.Not.SameAs(suppliedSettings)); }); } @@ -172,7 +196,85 @@ public async Task AfterDisable_NewDevicesUseDatabaseSettingsAndUpdatesApplyNorma Assert.That(startedSettings.LoggingType, Is.EqualTo(persistedSettings.LoggingType)); Assert.That(startedSettings.IntervalMs, Is.EqualTo(persistedSettings.IntervalMs)); }); - newLogger.Verify(logger => logger.UpdateSettings(updatedSettings), Times.Once); + newLogger.Verify( + logger => logger.UpdateSettings(It.Is( + settings => settings.DeviceId == updatedSettings.DeviceId + && settings.LoggingEnabled == updatedSettings.LoggingEnabled + && settings.LoggingType == updatedSettings.LoggingType + && settings.IntervalMs == updatedSettings.IntervalMs)), + Times.Once); + } + + [Test] + public async Task RemoveLogger_PreservesSettingsByDefault() + { + AddLogger("device-1"); + await SaveSettings(CreateSettings("device-1", loggingEnabled: true)); + + await _manager.RemoveLogger("device-1"); + + Assert.That(await _manager.GetDatabaseLoggerSettings("device-1"), Is.Not.Null); + } + + [Test] + public async Task RemoveLogger_WithRemoveSettings_DeletesSettings() + { + AddLogger("device-1"); + await SaveSettings(CreateSettings("device-1", loggingEnabled: true)); + + await _manager.RemoveLogger("device-1", removeSettings: true); + + Assert.That(await _manager.GetDatabaseLoggerSettings("device-1"), Is.Null); + } + + [Test] + public async Task EnableOverrideAsync_WhenLoggerUpdateFails_PropagatesFailureAndRollsBack() + { + var logger = AddLogger("device-1"); + logger + .SetupSequence(stateLogger => stateLogger.UpdateSettings(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Override failed")) + .Returns(Task.CompletedTask); + + Assert.ThrowsAsync( + () => _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), true)); + + var device = new TestDevice("Device", "device-2"); + var newLogger = CreateFactoryLogger(device.UniqueId); + DeviceLoggingSettings startedSettings = null; + newLogger + .Setup(stateLogger => stateLogger.Start(It.IsAny())) + .Callback(value => startedSettings = value) + .Returns(Task.CompletedTask); + + await _manager.SetupLogger(device); + + Assert.Multiple(() => + { + Assert.That(startedSettings, Is.Not.Null); + Assert.That(startedSettings.DeviceId, Is.EqualTo(device.UniqueId)); + Assert.That(startedSettings.LoggingEnabled, Is.False); + }); + } + + [Test] + public async Task DisableOverrideAsync_WhenRestoreFails_PropagatesAndCanBeRetried() + { + var logger = AddLogger("device-1"); + await SaveSettings(CreateSettings("device-1", loggingEnabled: false)); + await _manager.EnableOverrideAsync(CreateSettings(loggingEnabled: true), true); + logger + .SetupSequence(stateLogger => stateLogger.UpdateSettings(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Restore failed")) + .Returns(Task.CompletedTask); + + Assert.ThrowsAsync(() => _manager.DisableOverrideAsync()); + await _manager.DisableOverrideAsync(); + + logger.Verify( + stateLogger => stateLogger.UpdateSettings(It.Is( + settings => settings.DeviceId == "device-1" && !settings.LoggingEnabled)), + Times.Exactly(2)); } private Mock AddLogger(string deviceId) diff --git a/Ares.Core/Device/Remote/RemoteDeviceManager.cs b/Ares.Core/Device/Remote/RemoteDeviceManager.cs index 961b71d5..e7c0857d 100644 --- a/Ares.Core/Device/Remote/RemoteDeviceManager.cs +++ b/Ares.Core/Device/Remote/RemoteDeviceManager.cs @@ -100,7 +100,7 @@ public async Task RemoveDevice(string deviceId) monitor.Dispose(); _deviceMonitors.Remove(monitor); - await _stateLoggerManager.RemoveLogger(device.UniqueId); + await _stateLoggerManager.RemoveLogger(device.UniqueId, removeSettings: true); return true; } diff --git a/Ares.Core/Device/State/Logging/AresDeviceStateLogger.cs b/Ares.Core/Device/State/Logging/AresDeviceStateLogger.cs index e8055248..6d6b6721 100644 --- a/Ares.Core/Device/State/Logging/AresDeviceStateLogger.cs +++ b/Ares.Core/Device/State/Logging/AresDeviceStateLogger.cs @@ -34,6 +34,8 @@ public AresDeviceStateLogger(IDbContextFactory dbContextFac public Task Start(DeviceLoggingSettings? settings = null) { Settings = settings ?? Settings; + _lastDeltaValues.Clear(); + _eligibleDeltas = []; if(Settings.LoggingEnabled) { diff --git a/Ares.Core/Device/State/Logging/StateLoggerManager.cs b/Ares.Core/Device/State/Logging/StateLoggerManager.cs index f6afb469..d18bccb3 100644 --- a/Ares.Core/Device/State/Logging/StateLoggerManager.cs +++ b/Ares.Core/Device/State/Logging/StateLoggerManager.cs @@ -90,7 +90,9 @@ public async Task SetupLogger(IAresDevice device) using var ctx = _dbContextFactory.CreateDbContext(); var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == device.UniqueId); - var settings = _overrideActive ? _overrideSettings?.Clone() : existingSettings; + var settings = _overrideActive + ? CreateOverrideSettings(device.UniqueId) + : existingSettings ?? CreateDisabledSettings(device.UniqueId); await logger.Start(settings); _stateLoggerRepository.Add(device.UniqueId, logger); @@ -101,7 +103,7 @@ public async Task SetupLogger(IAresDevice device) } } - public async Task RemoveLogger(string deviceId) + public async Task RemoveLogger(string deviceId, bool removeSettings = false) { await _overrideLock.WaitAsync(); @@ -113,14 +115,19 @@ public async Task RemoveLogger(string deviceId) _stateLoggerRepository.Remove(deviceId); } - using var ctx = _dbContextFactory.CreateDbContext(); - var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == deviceId); - if(existingSettings is not null) + if(removeSettings) { - ctx.DeviceLoggingSettings.Remove(existingSettings); - } + using var ctx = _dbContextFactory.CreateDbContext(); + var existingSettings = await ctx.DeviceLoggingSettings.FirstOrDefaultAsync(s => s.DeviceId == deviceId); + if(existingSettings is not null) + { + ctx.DeviceLoggingSettings.Remove(existingSettings); + await ctx.SaveChangesAsync(); + } - await ctx.SaveChangesAsync(); + _logger.LogInformation("Removed logger and settings for device id {DeviceId}", deviceId); + return; + } _logger.LogInformation("Removed logger for device id {DeviceId}", deviceId); } @@ -132,33 +139,33 @@ public async Task RemoveLogger(string deviceId) public async Task UpdateLogger(string deviceId, DeviceLoggingSettings settings) { - if(_stateLoggerRepository.TryGetValue(deviceId, out var logger)) - { - await _overrideLock.WaitAsync(); - - - try - { - await UpdateDatabase(deviceId, settings); + await _overrideLock.WaitAsync(); - if(!_overrideActive) - { - await logger.UpdateSettings(settings); - _logger.LogInformation("Updated logger for device id {DeviceId}", deviceId); - } - } - catch(Exception e) + try + { + if(!_stateLoggerRepository.TryGetValue(deviceId, out var logger)) { - _logger.LogError("Error updating device state logger for device {DeviceId}: {Exception}", deviceId, e); + throw new KeyNotFoundException($"No logger found for device {deviceId}"); } - finally + + var deviceSettings = settings.Clone(); + deviceSettings.DeviceId = deviceId; + await UpdateDatabase(deviceId, deviceSettings); + + if(!_overrideActive) { - _overrideLock.Release(); + await logger.UpdateSettings(deviceSettings); + _logger.LogInformation("Updated logger for device id {DeviceId}", deviceId); } } - else + catch(Exception e) + { + _logger.LogError("Error updating device state logger for device {DeviceId}: {Exception}", deviceId, e); + throw; + } + finally { - throw new KeyNotFoundException($"No logger found for device {deviceId}"); + _overrideLock.Release(); } } @@ -216,21 +223,22 @@ public async Task DisableOverrideAsync() _logger.LogDebug("Preparing the tasks to disable device logging override"); var loggerRestoreTasks = _stateLoggerRepository.Select(async logger => { - var settings = await GetDatabaseLoggerSettings(logger.Key); + var settings = await GetDatabaseLoggerSettings(logger.Key) ?? CreateDisabledSettings(logger.Key); await logger.Value.UpdateSettings(settings); }); await Task.WhenAll(loggerRestoreTasks); + _overrideActive = false; + _overrideSettings = null; _logger.LogInformation("Disabled state logging override"); } catch(Exception e) { _logger.LogError("Error disabling state logging override: {Exception}", e); + throw; } finally { - _overrideActive = false; - _overrideSettings = null; _overrideLock.Release(); _logger.LogDebug("Released the device logging override lock. (Disable method)"); } @@ -241,6 +249,8 @@ public async Task EnableOverrideAsync(DeviceLoggingSettings settings, bool loggi _logger.LogInformation("Attempting to enable device logging override."); await _overrideLock.WaitAsync(); + var previousOverrideActive = _overrideActive; + var previousOverrideSettings = _overrideSettings; _overrideSettings = settings.Clone(); _overrideSettings.LoggingEnabled = loggingEnabled; _overrideActive = true; @@ -249,7 +259,7 @@ public async Task EnableOverrideAsync(DeviceLoggingSettings settings, bool loggi { _logger.LogDebug("Preparing the tasks to enable device logging override"); var loggerOverrideTasks = _stateLoggerRepository - .Select(logger => logger.Value.UpdateSettings(_overrideSettings.Clone())); + .Select(logger => logger.Value.UpdateSettings(CreateOverrideSettings(logger.Key))); await Task.WhenAll(loggerOverrideTasks); _logger.LogInformation("Enabled state logging override"); @@ -257,6 +267,28 @@ public async Task EnableOverrideAsync(DeviceLoggingSettings settings, bool loggi catch(Exception e) { _logger.LogError("Error enabling state logging override: {Exception}", e); + _overrideActive = previousOverrideActive; + _overrideSettings = previousOverrideSettings; + + try + { + var loggerRollbackTasks = _stateLoggerRepository.Select(async logger => + { + var rollbackSettings = previousOverrideActive + ? CreateOverrideSettings(logger.Key) + : await GetDatabaseLoggerSettings(logger.Key) ?? CreateDisabledSettings(logger.Key); + await logger.Value.UpdateSettings(rollbackSettings); + }); + + await Task.WhenAll(loggerRollbackTasks); + } + catch(Exception rollbackException) + { + _logger.LogError("Error rolling back state logging override: {Exception}", rollbackException); + throw new AggregateException(e, rollbackException); + } + + throw; } finally { @@ -264,4 +296,21 @@ public async Task EnableOverrideAsync(DeviceLoggingSettings settings, bool loggi _logger.LogDebug("Released the device logging override lock. (Enable method)"); } } + + private DeviceLoggingSettings CreateOverrideSettings(string deviceId) + { + var settings = _overrideSettings?.Clone() ?? CreateDisabledSettings(deviceId); + settings.DeviceId = deviceId; + return settings; + } + + private static DeviceLoggingSettings CreateDisabledSettings(string deviceId) + { + return new DeviceLoggingSettings + { + DeviceId = deviceId, + LoggingEnabled = false, + LoggingType = DeviceLoggingSettings.Types.LoggingType.None + }; + } }