diff --git a/Daqifi.Avalonia.Tests/Loggers/SessionRenamePersistenceTests.cs b/Daqifi.Avalonia.Tests/Loggers/SessionRenamePersistenceTests.cs new file mode 100644 index 0000000..103e848 --- /dev/null +++ b/Daqifi.Avalonia.Tests/Loggers/SessionRenamePersistenceTests.cs @@ -0,0 +1,160 @@ +using Daqifi.Desktop.Logger; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Xunit; + +namespace Daqifi.Avalonia.Tests.Loggers; + +/// +/// Renaming a logging session — the write behind the editable name box on the Logged Data pane. +/// +/// Until now this write lived in LoggedDataPanePrototype.axaml.cs: a markup +/// code-behind that resolved IDbContextFactory<LoggingContext> off +/// App.ServiceProvider, opened a DbContext, found the row and saved it. It was the +/// only .axaml.cs in the repo that referenced EF Core or the service locator at all, and +/// nothing in this project can construct a view, so the rename was the one session write with no +/// reachable test. Moving it to — which already owns the sessions this +/// pane lists and the context factory they live in — is what makes these rows possible. +/// +/// These tests are characterization, not regression: the behaviour is unchanged by the move, +/// so none of them can fail against origin/main — they name a member that does not exist +/// there, which is "does not compile", not "0 failed". What shows they bite is the mutation run +/// recorded on the PR, where each row is killed by exactly the mutation its name claims. +/// +public sealed class SessionRenamePersistenceTests : IDisposable +{ + private const int SessionId = 1; + + /// The name the row is seeded with, so every assertion below moves it off a value it + /// already held — an assertion that only ever sees the default proves nothing. + private const string SeededName = "Session"; + + private readonly string _directory = + Path.Combine(Path.GetTempPath(), "daqifi-avalonia-tests", "session-rename-" + Guid.NewGuid().ToString("N")); + + private string DatabasePath => Path.Combine(_directory, "DAQiFiDatabase.db"); + + /// A throwaway profiles file, so nothing here can reach the developer's real one. + private string ProfilePath => Path.Combine(_directory, "DAQifiProfilesConfiguration.xml"); + + public SessionRenamePersistenceTests() => Directory.CreateDirectory(_directory); + + public void Dispose() + { + try { Directory.Delete(_directory, recursive: true); } catch { /* best-effort cleanup */ } + } + + /// The headline: what the user typed is in the database afterwards. + [Fact] + public async Task A_typed_name_reaches_the_sessions_row() + { + SeedSession(); + + await Manager().PersistSessionNameAsync(SessionId, "Bench run 3"); + + Assert.Equal("Bench run 3", StoredName()); + } + + /// + /// The blank case, pinned AS IT BEHAVES TODAY, which is broken — issue #427. + /// + /// The pane sends null when the user empties the box, meaning "go back to the + /// default label". Sessions.Name is nullable: false, and EF writes the backing + /// field rather than LoggingSession.Name's substituting getter, so the save throws + /// NOT NULL constraint failed: Sessions.Name and the row keeps its old name. The + /// in-memory object has already changed, so the pane shows the default until the next launch + /// brings the old name back. The code-behind logged the throw and showed the user nothing, + /// which is why this has gone unnoticed. + /// + /// Not fixed here on purpose: this PR moves the write without changing it, and the fix + /// is a schema or storage decision (nullable column vs storing "") that deserves its own + /// change — #427. When it lands, this row is the one that has to be rewritten. + /// + [Fact] + public async Task Clearing_the_box_fails_to_persist_today_and_leaves_the_old_name_in_place() + { + SeedSession(); + + await Assert.ThrowsAsync( + () => Manager().PersistSessionNameAsync(SessionId, null)); + + Assert.Equal(SeededName, StoredName()); + } + + /// + /// A debounced write can land after its session was deleted — the pane's 250 ms delay is long + /// enough for a Delete to complete in between. That has to be a no-op rather than a throw, or + /// the race surfaces to the user as a failed rename on a session that is already gone. + /// + [Fact] + public async Task Renaming_a_session_that_is_no_longer_there_changes_nothing_and_does_not_throw() + { + SeedSession(); + + await Manager().PersistSessionNameAsync(SessionId + 99, "Ghost"); + + Assert.Equal(SeededName, StoredName()); + Assert.Equal(1L, ScalarLong("SELECT COUNT(*) FROM Sessions")); + } + + /// + /// The debounce contract seen from this side: the caller cancels the previous write on every + /// keystroke, and a cancelled write must not reach the row. Without this, a fast typist's + /// superseded keystrokes could still be racing toward the database behind the final one. + /// + [Fact] + public async Task A_cancelled_write_leaves_the_row_alone() + { + SeedSession(); + using var cancelled = new CancellationTokenSource(); + await cancelled.CancelAsync(); + + await Assert.ThrowsAnyAsync( + () => Manager().PersistSessionNameAsync(SessionId, "Superseded", cancelled.Token)); + + Assert.Equal(SeededName, StoredName()); + } + + #region Helpers + + private LoggingManager Manager() => new(TestDatabase.Contexts(DatabasePath), ProfilePath); + + private void SeedSession() + { + DatabaseMigrator.ApplyMigrations(TestDatabase.Contexts(DatabasePath), DatabasePath); + + using var connection = new SqliteConnection(TestDatabase.ConnectionString(DatabasePath)); + connection.Open(); + using var insert = connection.CreateCommand(); + insert.CommandText = "INSERT INTO Sessions (ID, SessionStart, Name) VALUES ($id, $start, $name)"; + insert.Parameters.AddWithValue("$id", SessionId); + insert.Parameters.AddWithValue("$start", "2026-01-01 00:00:00"); + insert.Parameters.AddWithValue("$name", SeededName); + insert.ExecuteNonQuery(); + } + + /// + /// Read straight out of SQLite rather than back through EF, so a change that only ever existed + /// in a change-tracker cannot pass this. + /// + private string? StoredName() + { + using var connection = new SqliteConnection(TestDatabase.ConnectionString(DatabasePath)); + connection.Open(); + using var query = connection.CreateCommand(); + query.CommandText = "SELECT Name FROM Sessions WHERE ID = $id"; + query.Parameters.AddWithValue("$id", SessionId); + return query.ExecuteScalar() as string; + } + + private long ScalarLong(string sql) + { + using var connection = new SqliteConnection(TestDatabase.ConnectionString(DatabasePath)); + connection.Open(); + using var query = connection.CreateCommand(); + query.CommandText = sql; + return (long)query.ExecuteScalar()!; + } + + #endregion +} diff --git a/Daqifi.Avalonia/Daqifi.Desktop/Loggers/LoggingManager.cs b/Daqifi.Avalonia/Daqifi.Desktop/Loggers/LoggingManager.cs index c8f7e8e..1ed7ddf 100644 --- a/Daqifi.Avalonia/Daqifi.Desktop/Loggers/LoggingManager.cs +++ b/Daqifi.Avalonia/Daqifi.Desktop/Loggers/LoggingManager.cs @@ -889,6 +889,35 @@ public ObservableCollection GetLoggingSessionsSnapshot() .ToList()); } + /// + /// Persists the user-entered for one session. + /// + /// + /// Throws rather than swallowing, unlike : the + /// caller debounces keystrokes, so it is the one that knows whether a failure means "the user is + /// still typing" () or "the rename did not stick". + /// A null name does not currently save. The caller sends null for an + /// emptied box, meaning "go back to the default label", and LoggingSession.Name's getter + /// honours that in memory — but Sessions.Name is nullable: false and EF writes the + /// backing field rather than that getter, so the save throws DbUpdateException and the row + /// keeps its old name. Tracked as issue #427; this method relays the behaviour it inherited from + /// the view code-behind rather than changing it. + /// + /// The session to rename. A row that is no longer there is a no-op. + /// The new name. null throws on save — see the remark. + /// Cancelled by the caller when another keystroke supersedes this write. + public async Task PersistSessionNameAsync( + int sessionId, string? name, CancellationToken cancellationToken = default) + { + await using var context = await _loggingContext.CreateDbContextAsync(cancellationToken); + var tracked = await context.Sessions.FindAsync([sessionId], cancellationToken); + if (tracked != null) + { + tracked.Name = name; + await context.SaveChangesAsync(cancellationToken); + } + } + /// /// Persists for the given session /// by running a single COUNT against the Samples table. Called when a diff --git a/Daqifi.Avalonia/Daqifi.Desktop/View/Prototype/LoggedDataPanePrototype.axaml.cs b/Daqifi.Avalonia/Daqifi.Desktop/View/Prototype/LoggedDataPanePrototype.axaml.cs index cbd32d0..e16666a 100644 --- a/Daqifi.Avalonia/Daqifi.Desktop/View/Prototype/LoggedDataPanePrototype.axaml.cs +++ b/Daqifi.Avalonia/Daqifi.Desktop/View/Prototype/LoggedDataPanePrototype.axaml.cs @@ -9,8 +9,6 @@ using Daqifi.Desktop.Common.Loggers; using Daqifi.Desktop.Logger; using Daqifi.Desktop.ViewModels; -using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.DependencyInjection; namespace Daqifi.Desktop.View.Prototype; @@ -21,7 +19,6 @@ namespace Daqifi.Desktop.View.Prototype; public partial class LoggedDataPanePrototype : UserControl { private readonly AppLogger _logger = AppLogger.Instance; - private readonly IDbContextFactory _loggingContext; private CancellationTokenSource? _renameSessionCts; public LoggedDataPanePrototype() @@ -33,7 +30,6 @@ public LoggedDataPanePrototype() // Avalonia's headless automation drives the ItemsControl directly, so no analog is needed; // production virtualization behavior is unchanged. - _loggingContext = App.ServiceProvider.GetRequiredService>(); Unloaded += (_, _) => { _renameSessionCts?.Cancel(); @@ -86,13 +82,10 @@ private async void OnSessionNameChanged(object? sender, TextChangedEventArgs e) { await Task.Delay(TimeSpan.FromMilliseconds(250), renameCts.Token); - await using var context = await _loggingContext.CreateDbContextAsync(renameCts.Token); - var sessionToUpdate = await context.Sessions.FindAsync([session.ID], renameCts.Token); - if (sessionToUpdate != null) - { - sessionToUpdate.Name = newName; - await context.SaveChangesAsync(renameCts.Token); - } + // The write itself belongs to LoggingManager, which owns the sessions this pane lists + // and the database they live in. A view knowing the Sessions table was the only place + // in the app where markup code-behind opened a DbContext. + await LoggingManager.Instance.PersistSessionNameAsync(session.ID, newName, renameCts.Token); } catch (OperationCanceledException) {