From 83cd36e0dea7b827f34b2a1ac5de03c6c37f2eeb Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 17:27:57 +0200 Subject: [PATCH] test(491): run the dedupe fixture against MySql in CI; correct the collation claim The dedupe DML had zero automated coverage on MySql: the migrations job only applies migrations to a fresh EMPTY database, so no dedupe row ever executed there. Two MySql-only collation defects escaped that gate in this session and were caught only by hand-run containers. Parameterize LibraryFolderDedupeMigrationTests over both providers from ONE fixture body - same seeded rows, same expected survivors - rather than adding a MySql-only copy that would drift and recreate the gap. Assertions no longer use WHERE Path = '...', which is itself collation-dependent and would quietly mean something different per provider; rows are read once and compared ordinally in memory. A new step in the existing migrations job runs it against that job's mysql:8.4 service, on a per-test database of its own. Proven red when the collation is wrong: restoring COLLATE utf8mb4_bin fails the MySql half with survivors [1,4,5,6,7,9] - the trailing-space sibling deleted - while SQLite stays green. Proven non-skippable: without ETV_TEST_MYSQL_CONNECTION the fixture ignores visibly, and with ETV_REQUIRE_MYSQL_TESTS=1 (which CI sets) that skip becomes a hard failure, so it cannot pass having connected to nothing. Local runs need no MySql. Also correct an overstated comment. The schema pins only the utf8mb4 charset, never a collation, so the effective comparison is the server default: always case-insensitive, but PAD SPACE only on utf8mb4_general_ci - 8.4's default utf8mb4_0900_ai_ci is NO PAD, verified on the real column. The migration bug was independent of that because the old code applied an EXPLICIT utf8mb4_bin, which is PAD SPACE everywhere; the runtime simply tolerates both. Refs #488 #308 fix #491 --- .gitea/workflows/docker-build.yml | 20 + .../Data/Repositories/LibraryRepository.cs | 33 +- .../LibraryFolderDedupeMigrationTests.cs | 369 +++++++++++++----- .../Integration/LibraryRepositoryTests.cs | 5 +- 4 files changed, 309 insertions(+), 118 deletions(-) diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index 8fa32cda0..b1e3d78e9 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -408,6 +408,26 @@ jobs: done echo "::endgroup::" + # The two checks above only ever apply migrations to a fresh EMPTY database, so they execute no + # rows of any data-migration logic. The #491 LibraryFolder dedupe DELETES rows irreversibly and its + # correctness depends on MySql string-comparison semantics that SQLite does not share — two + # MySql-only collation defects (a case-insensitive grouping, then a PAD SPACE one) escaped exactly + # this gate and were caught only by hand-run servers. LibraryFolderDedupeMigrationTests is + # parameterized over both providers from ONE fixture, so running it here against the live service + # closes that gap and keeps the two providers from silently diverging. + # ETV_REQUIRE_MYSQL_TESTS turns "no MySql reachable" from a skip into a failure, so this can never + # quietly pass having connected to nothing. It reuses the `mysql` service already declared by this + # job, on its own per-test database, so it does not disturb the fresh-DB apply above. + - name: MySql — data-migration fixture (#491 dedupe, same fixture as SQLite) + if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true' + env: + ETV_TEST_MYSQL_CONNECTION: "Server=mysql;Port=3306;Uid=root;Pwd=ersatztv;DefaultCommandTimeout=300;" + ETV_REQUIRE_MYSQL_TESTS: "1" + run: | + set -euo pipefail + dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj --no-build --configuration Release \ + --filter "FullyQualifiedName~LibraryFolderDedupeMigrationTests" + functional-e2e: name: Functional E2E (curl + UI contracts) runs-on: ubuntu-latest diff --git a/ErsatzTV.Infrastructure/Data/Repositories/LibraryRepository.cs b/ErsatzTV.Infrastructure/Data/Repositories/LibraryRepository.cs index b371907ee..59db80a9a 100644 --- a/ErsatzTV.Infrastructure/Data/Repositories/LibraryRepository.cs +++ b/ErsatzTV.Infrastructure/Data/Repositories/LibraryRepository.cs @@ -322,16 +322,29 @@ public class LibraryRepository(IFileSystem fileSystem, IDbContextFactory /// The SQL equality is only a *narrowing* filter, not the identity test. On MySQL, `Path` is a - /// `longtext` compared under the server default, which differs from byte equality on two axes: - /// it is case-INsensitive (the collation the codebase names in - /// ), and it is PAD SPACE, so trailing spaces - /// are insignificant. `Path = @folder` therefore also matches siblings differing only in case - /// or in trailing whitespace — all legal on a case-sensitive filesystem, and all preserved by - /// the #491 migration. Crucially the SQL predicate is a *superset*: both quirks make it more - /// permissive, never less, so it cannot miss a byte-exact match. Identity is then settled in - /// memory by with an ORDINAL comparison, matching - /// PathUtils.GetPathHash, which hashes the exact bytes. Without this the lookup and the - /// hash disagree, and the PathHash heal could stamp one sibling's hash onto the other's row. + /// `longtext` whose collation the schema does not pin — only the `utf8mb4` charset — so the + /// effective comparison is whatever the server defaults to, and it differs from byte equality: + /// + /// + /// always case-INsensitive: both plausible defaults are `_ci` (8.4 verified: + /// `utf8mb4_0900_ai_ci`; older servers `utf8mb4_general_ci`), which is why + /// exists at all; + /// + /// + /// possibly PAD SPACE, making trailing spaces insignificant — true of + /// `utf8mb4_general_ci`, but NOT of `utf8mb4_0900_ai_ci`, which is NO PAD. So this axis + /// is server-dependent rather than guaranteed, and must be tolerated rather than + /// assumed either way. + /// + /// + /// `Path = @folder` can therefore also match siblings differing only in case, or (on a PAD + /// SPACE server) in trailing whitespace — all legal on a case-sensitive filesystem, and all + /// preserved by the #491 migration. Crucially the SQL predicate is a *superset*: every such + /// quirk makes it more permissive, never less, so it cannot miss a byte-exact match. Identity + /// is then settled in memory by with an ORDINAL comparison, + /// matching PathUtils.GetPathHash, which hashes the exact bytes. Without this the lookup + /// and the hash disagree, and the PathHash heal could stamp one sibling's hash onto the other's + /// row. /// /// /// Ordered by Id so the result is deterministic: an unordered FirstOrDefault may return a diff --git a/ErsatzTV.Tests/Integration/LibraryFolderDedupeMigrationTests.cs b/ErsatzTV.Tests/Integration/LibraryFolderDedupeMigrationTests.cs index 8840d163f..0f58a1e9c 100644 --- a/ErsatzTV.Tests/Integration/LibraryFolderDedupeMigrationTests.cs +++ b/ErsatzTV.Tests/Integration/LibraryFolderDedupeMigrationTests.cs @@ -1,69 +1,155 @@ using Dapper; using ErsatzTV.Infrastructure; using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Infrastructure.MySql.Data; using ErsatzTV.Infrastructure.Sqlite.Data; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging.Abstractions; +using MySqlConnector; using NUnit.Framework; using Shouldly; namespace ErsatzTV.Tests.Integration; +public enum TestProvider +{ + Sqlite, + MySql +} + /// /// ersatztv#491: the unique index on LibraryFolder(LibraryPathId, PathHash) ships with an /// audit/cleanup of pre-existing duplicate rows (reachable before #488, when the folder lookup read a /// scan-start in-memory snapshot and could not see a folder created earlier in the same scan). This -/// drives the REAL Sqlite migration against a database seeded at the previous migration, so the -/// cleanup SQL is exercised rather than restated. +/// drives the REAL migration against a database seeded at the previous migration, so the cleanup SQL +/// is exercised rather than restated. +/// +/// Runs against BOTH providers from ONE fixture body. The dedupe deletes rows irreversibly and its +/// correctness turns on string-comparison semantics that differ per provider — two MySQL-only +/// collation defects escaped review in the #491 session (a case-insensitive grouping, then a PAD +/// SPACE one), and neither was reachable from a SQLite-only test, nor from CI's MySQL job, which +/// only applies migrations to a fresh EMPTY database and so executes no dedupe rows at all. +/// Parameterizing one fixture is what makes "the two providers agree" a checked property rather +/// than an assumption; a separate MySQL-only copy would drift and recreate the gap. +/// +/// +/// MySQL needs a live server, supplied via ETV_TEST_MYSQL_CONNECTION. Without it the MySQL +/// fixture ignores — a visible skip, never a silent pass — so local runs need no MySQL. CI +/// sets ETV_REQUIRE_MYSQL_TESTS=1, which turns that skip into a hard failure, so the gate +/// cannot quietly degrade into "connected to nothing and passed". +/// /// -[TestFixture] -public class LibraryFolderDedupeMigrationTests +[TestFixture(TestProvider.Sqlite)] +[TestFixture(TestProvider.MySql)] +[NonParallelizable] +public class LibraryFolderDedupeMigrationTests(TestProvider provider) { // the migration immediately preceding Add_LibraryFolder_PathHash_UniqueIndex private const string PreviousMigration = "Add_Channel_Origin"; + private const string MySqlConnectionVariable = "ETV_TEST_MYSQL_CONNECTION"; + private const string MySqlRequiredVariable = "ETV_REQUIRE_MYSQL_TESTS"; + private string _databasePath = null!; + private string _mySqlConnectionString; // Seeding writes deliberately partial object graphs (a LibraryPath with no Library, a MediaFile with - // no MediaVersion), so it needs foreign keys OFF — as every other harness in this suite does. + // no MediaVersion), so it runs with foreign keys OFF. private DbContextOptions _seedOptions = null!; - // The migration itself runs with foreign keys ON, matching production (`Startup.cs` builds the SQLite - // connection string with `foreign keys=true`). This matters: the single most dangerous statement in - // the #491 migration is `DELETE FROM LibraryFolder` against two Restrict foreign keys - // (MediaFile.LibraryFolderId, LibraryFolder.ParentId). With enforcement off, a wrong repoint order - // would still pass; with it on, the delete fails loudly. + // The migration itself runs with foreign keys ON, matching production. This matters: the single most + // dangerous statement in the #491 migration is DELETE FROM LibraryFolder against two Restrict foreign + // keys (MediaFile.LibraryFolderId, LibraryFolder.ParentId). With enforcement off, a wrong repoint + // order would still pass; with it on, the delete fails loudly. private DbContextOptions _migrateOptions = null!; [SetUp] - public void SetUp() + public async Task SetUp() { - TvContext.IsSqlite = true; - TvContext.IsUniqueConstraintViolation = SqliteErrorClassifier.IsUniqueConstraintViolation; + if (provider is TestProvider.Sqlite) + { + TvContext.IsSqlite = true; + TvContext.LastInsertedRowId = "last_insert_rowid()"; + TvContext.CaseInsensitiveCollation = "NOCASE"; + TvContext.IsUniqueConstraintViolation = SqliteErrorClassifier.IsUniqueConstraintViolation; - _databasePath = Path.Combine(Path.GetTempPath(), $"etv491-{Guid.NewGuid():N}.sqlite3"); - _seedOptions = BuildOptions(foreignKeys: false); - _migrateOptions = BuildOptions(foreignKeys: true); + _databasePath = Path.Combine(Path.GetTempPath(), $"etv491-{Guid.NewGuid():N}.sqlite3"); + _seedOptions = SqliteOptions(foreignKeys: false); + _migrateOptions = SqliteOptions(foreignKeys: true); + return; + } + + string baseConnectionString = Environment.GetEnvironmentVariable(MySqlConnectionVariable); + if (string.IsNullOrWhiteSpace(baseConnectionString)) + { + string message = + $"{MySqlConnectionVariable} is not set, so the MySql half of the #491 dedupe fixture cannot " + + "run. The dedupe deletes rows irreversibly and its correctness is provider-specific, so " + + "this coverage is not optional in CI."; + + // A skip is fine locally; in CI it is the very failure mode this fixture exists to prevent. + if (IsTrue(Environment.GetEnvironmentVariable(MySqlRequiredVariable))) + { + Assert.Fail($"{message} {MySqlRequiredVariable} is set, so this is a failure, not a skip."); + } + + Assert.Ignore($"{message} Set it to run this locally."); + } + + // Use a database of our own rather than the one the surrounding CI step migrates, rebuilt per test + // so each starts from nothing. + var builder = new MySqlConnectionStringBuilder(baseConnectionString); + var database = $"etv491_{Guid.NewGuid():N}"; + builder.Database = string.Empty; + + await using (var connection = new MySqlConnection(builder.ConnectionString)) + { + await connection.OpenAsync(); + await connection.ExecuteAsync($"CREATE DATABASE `{database}`"); + } + + builder.Database = database; + _mySqlConnectionString = builder.ConnectionString; + + TvContext.IsSqlite = false; + TvContext.LastInsertedRowId = "last_insert_id()"; + TvContext.CaseInsensitiveCollation = "utf8mb4_general_ci"; + TvContext.IsUniqueConstraintViolation = MySqlErrorClassifier.IsUniqueConstraintViolation; + + _seedOptions = MySqlOptions(); + _migrateOptions = MySqlOptions(); } - private DbContextOptions BuildOptions(bool foreignKeys) => - new DbContextOptionsBuilder() - .UseSqlite( - $"Data Source={_databasePath};Foreign Keys={foreignKeys}", - o => o.MigrationsAssembly("ErsatzTV.Infrastructure.Sqlite")) - .Options; - [TearDown] - public void TearDown() + public async Task TearDown() { - Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); - foreach (string path in new[] { _databasePath, $"{_databasePath}-wal", $"{_databasePath}-shm" }) + if (provider is TestProvider.Sqlite) { - if (File.Exists(path)) + Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools(); + foreach (string path in new[] { _databasePath, $"{_databasePath}-wal", $"{_databasePath}-shm" }) { - File.Delete(path); + if (File.Exists(path)) + { + File.Delete(path); + } } + + return; } + + if (_mySqlConnectionString is null) + { + return; + } + + var builder = new MySqlConnectionStringBuilder(_mySqlConnectionString); + string database = builder.Database; + builder.Database = string.Empty; + _mySqlConnectionString = null; + + await using var connection = new MySqlConnection(builder.ConnectionString); + await connection.OpenAsync(); + await connection.ExecuteAsync($"DROP DATABASE IF EXISTS `{database}`"); } [Test] @@ -72,12 +158,19 @@ public class LibraryFolderDedupeMigrationTests await using (TvContext context = SeedContext()) { await context.Database.MigrateAsync(PreviousMigration); + await using SeedSession seed = await SeedSession.OpenAsync(context, provider); - // one library path with the SAME folder recorded three times (ids 1, 2, 3) plus an unrelated - // folder (id 4) and a child parented on one of the duplicates (id 5) - await context.Connection.ExecuteAsync( + // one library path with the SAME folder recorded three times (ids 1, 2, 3), an unrelated + // folder (4), a child parented on one of the duplicates (5), a case-differing sibling (6), + // a folder parented on its own duplicate (7/8 — the cycle the ParentId null-out guards), and + // a sibling differing only by a TRAILING SPACE (9/10). The last two pairs are distinct legal + // directories on Linux that hash differently, so the unique index accepts both and the dedupe + // must not collapse them. On MySql a case-insensitive grouping deletes 6, and a PAD SPACE one + // deletes 10 — utf8mb4_bin, the obvious fix for the first, is itself PAD SPACE, which is why + // the migration groups on CONVERT(Path USING binary) instead. + await seed.ExecuteAsync( "INSERT INTO LibraryPath (Id, LibraryId, Path) VALUES (1, 1, '/data/music')"); - await context.Connection.ExecuteAsync( + await seed.ExecuteAsync( """ INSERT INTO LibraryFolder (Id, LibraryPathId, Path, ParentId, Etag) VALUES (1, 1, '/data/music/artist1', NULL, 'etag-keeper'), @@ -85,33 +178,25 @@ public class LibraryFolderDedupeMigrationTests (3, 1, '/data/music/artist1', NULL, 'etag-dupe-b'), (4, 1, '/data/music/artist2', NULL, NULL), (5, 1, '/data/music/artist1/album', 3, NULL), - -- a case-differing sibling, legal on a case-sensitive filesystem: must SURVIVE (6, 1, '/data/music/ARTIST2', NULL, NULL), - -- duplicate 3 is parented on duplicate 2: repointing both would make the survivor - -- its own parent, the cycle the ParentId null-out guards (7, 1, '/data/music/artist3', 8, NULL), (8, 1, '/data/music/artist3', NULL, NULL), - -- a sibling differing only by a TRAILING SPACE. Two distinct legal directories on - -- Linux, and PathUtils.GetPathHash hashes them differently, so the unique index would - -- accept both — the dedupe must not collapse them. MySql's string comparison is PAD - -- SPACE (verified on 8.4, for utf8mb4_bin as well as the ci default), so grouping - -- under any collation rather than binary would delete row 10 irreversibly. (9, 1, '/data/music/pad', NULL, NULL), (10, 1, '/data/music/pad ', NULL, NULL) """); - // a media file on each duplicate, and an image-folder-duration on a duplicate only - await context.Connection.ExecuteAsync( + // a media file on each duplicate and on each half of the trailing-space pair, plus an + // image-folder-duration on duplicates only + await seed.ExecuteAsync( """ INSERT INTO MediaFile (Id, Path, PathHash, MediaVersionId, LibraryFolderId) VALUES (1, '/data/music/artist1/a.mkv', 'hash-a', 1, 1), (2, '/data/music/artist1/b.mkv', 'hash-b', 2, 2), (3, '/data/music/artist1/c.mkv', 'hash-c', 3, 3), - -- dependents of the trailing-space pair: must stay attached to their OWN folder (4, '/data/music/pad/d.mkv', 'hash-d', 4, 9), (5, '/data/music/pad /e.mkv', 'hash-e', 5, 10) """); - await context.Connection.ExecuteAsync( + await seed.ExecuteAsync( """ INSERT INTO ImageFolderDuration (Id, LibraryFolderId, DurationSeconds) VALUES (1, 2, 30.0), @@ -127,70 +212,53 @@ public class LibraryFolderDedupeMigrationTests await using (TvContext context = MigrateContext()) { - // the duplicates are gone; the lowest id survives - List folderIds = - (await context.Connection.QueryAsync( - "SELECT Id FROM LibraryFolder WHERE Path = '/data/music/artist1'")).ToList(); - folderIds.ShouldBe([1]); + // Read the surviving rows once and assert in memory. Deliberately NOT `WHERE Path = '...'`: + // that predicate is itself collation-dependent (on MySQL it would also match the + // case-differing and trailing-space siblings), so an assertion written that way would quietly + // mean something different on each provider — the exact class of bug this fixture guards. + List folders = (await context.Connection.QueryAsync( + "SELECT Id, Path, ParentId, Etag FROM LibraryFolder ORDER BY Id")).ToList(); // survivors: keeper 1, artist2 (4), the child (5), the case-differing sibling (6), artist3's // keeper (7), and BOTH halves of the trailing-space pair (9, 10). Losers 2, 3 and 8 are gone. - List survivors = - (await context.Connection.QueryAsync( - "SELECT Id FROM LibraryFolder ORDER BY Id")).ToList(); - survivors.ShouldBe([1, 4, 5, 6, 7, 9, 10]); + folders.Select(f => f.Id).ToList().ShouldBe([1, 4, 5, 6, 7, 9, 10]); - // a folder differing only in case is NOT a duplicate — it must survive untouched - (await context.Connection.ExecuteScalarAsync( - "SELECT COUNT(*) FROM LibraryFolder WHERE Path = '/data/music/ARTIST2'")).ShouldBe(1); - - // nor is one differing only by a trailing space, and each keeps its OWN dependents - List padded = - (await context.Connection.QueryAsync( - "SELECT Id || '=[' || Path || ']' FROM LibraryFolder WHERE Id IN (9, 10) ORDER BY Id")) - .ToList(); - padded.ShouldBe(["9=[/data/music/pad]", "10=[/data/music/pad ]"]); - - List padMediaFolderIds = - (await context.Connection.QueryAsync( - "SELECT LibraryFolderId FROM MediaFile WHERE Id IN (4, 5) ORDER BY Id")).ToList(); - padMediaFolderIds.ShouldBe([9, 10]); + // and their paths survive BYTE-exactly (ordinal comparison here, matching the hash) + folders.Select(f => f.Path).ToList().ShouldBe( + [ + "/data/music/artist1", + "/data/music/artist2", + "/data/music/artist1/album", + "/data/music/ARTIST2", + "/data/music/artist3", + "/data/music/pad", + "/data/music/pad " + ]); // the keeper's etag is cleared: which duplicate the scanner was writing to was arbitrary, so // MIN(Id)'s etag could suppress the rescan that repairs the collapsed folder - (await context.Connection.ExecuteScalarAsync( - "SELECT Etag FROM LibraryFolder WHERE Id = 1")).ShouldBeNull(); - - // the folder parented on its own duplicate did not become its own parent - (await context.Connection.ExecuteScalarAsync( - "SELECT ParentId FROM LibraryFolder WHERE Id = 7")).ShouldBeNull(); - - // every media file follows the keeper — nothing orphaned, nothing deleted - List mediaFolderIds = - (await context.Connection.QueryAsync( - "SELECT LibraryFolderId FROM MediaFile WHERE Id IN (1, 2, 3) ORDER BY Id")).ToList(); - mediaFolderIds.ShouldBe([1, 1, 1]); + folders.Single(f => f.Id == 1).Etag.ShouldBeNull(); // the child folder is reparented off the deleted duplicate onto the keeper - (await context.Connection.ExecuteScalarAsync( - "SELECT ParentId FROM LibraryFolder WHERE Id = 5")).ShouldBe(1); + folders.Single(f => f.Id == 5).ParentId.ShouldBe(1); + + // the folder parented on its own duplicate did not become its own parent + folders.Single(f => f.Id == 7).ParentId.ShouldBeNull(); + + // every media file follows the keeper — nothing orphaned, nothing deleted — while the + // trailing-space pair's dependents stay attached to their OWN folder + List mediaFolderIds = (await context.Connection.QueryAsync( + "SELECT LibraryFolderId FROM MediaFile ORDER BY Id")).ToList(); + mediaFolderIds.ShouldBe([1, 1, 1, 9, 10]); // the keeper had no ImageFolderDuration, so exactly one duplicate's setting is promoted to it // (the lowest id) and the rest are dropped — the 1:1 unique index cannot hold both - List durations = - (await context.Connection.QueryAsync( - "SELECT Id || ':' || LibraryFolderId FROM ImageFolderDuration ORDER BY Id")).ToList(); - durations.ShouldBe(["1:1"]); + List durationFolderIds = (await context.Connection.QueryAsync( + "SELECT LibraryFolderId FROM ImageFolderDuration ORDER BY Id")).ToList(); + durationFolderIds.ShouldBe([1]); - // the helper tables the cleanup used are not left behind - (await context.Connection.ExecuteScalarAsync( - "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name LIKE '__LibraryFolderDedupe%'")) - .ShouldBe(0); - - // and the unique index is now in place and enforcing - (await context.Connection.ExecuteScalarAsync( - "SELECT COUNT(*) FROM sqlite_master WHERE type = 'index' AND name = 'IX_LibraryFolder_LibraryPathId_PathHash'")) - .ShouldBe(1); + await AssertHelperTablesDropped(context); + await AssertUniqueIndexExists(context); } } @@ -201,15 +269,17 @@ public class LibraryFolderDedupeMigrationTests await using (TvContext context = SeedContext()) { await context.Database.MigrateAsync(PreviousMigration); - await context.Connection.ExecuteAsync( + await using SeedSession seed = await SeedSession.OpenAsync(context, provider); + + await seed.ExecuteAsync( "INSERT INTO LibraryPath (Id, LibraryId, Path) VALUES (1, 1, '/data/music')"); - await context.Connection.ExecuteAsync( + await seed.ExecuteAsync( """ INSERT INTO LibraryFolder (Id, LibraryPathId, Path, ParentId, Etag) VALUES (1, 1, '/data/music/artist1', NULL, 'etag-1'), (2, 1, '/data/music/artist2', 1, 'etag-2') """); - await context.Connection.ExecuteAsync( + await seed.ExecuteAsync( "INSERT INTO ImageFolderDuration (Id, LibraryFolderId, DurationSeconds) VALUES (1, 2, 30.0)"); } @@ -221,9 +291,14 @@ public class LibraryFolderDedupeMigrationTests await using (TvContext context = MigrateContext()) { - (await context.Connection.ExecuteScalarAsync("SELECT COUNT(*) FROM LibraryFolder")).ShouldBe(2); - (await context.Connection.ExecuteScalarAsync("SELECT ParentId FROM LibraryFolder WHERE Id = 2")) - .ShouldBe(1); + List folders = (await context.Connection.QueryAsync( + "SELECT Id, Path, ParentId, Etag FROM LibraryFolder ORDER BY Id")).ToList(); + + folders.Select(f => f.Id).ToList().ShouldBe([1, 2]); + folders.Single(f => f.Id == 1).Etag.ShouldBe("etag-1"); + folders.Single(f => f.Id == 2).Etag.ShouldBe("etag-2"); + folders.Single(f => f.Id == 2).ParentId.ShouldBe(1); + (await context.Connection.ExecuteScalarAsync( "SELECT LibraryFolderId FROM ImageFolderDuration WHERE Id = 1")).ShouldBe(2); @@ -233,17 +308,61 @@ public class LibraryFolderDedupeMigrationTests } } - // The `Foreign Keys=True` keyword above is the whole point of splitting the contexts, so prove it took - // effect rather than trusting the connection string: a typo or a pragma reset would silently revert - // this test to its weaker "enforcement off" form and still pass every assertion. - private static async Task AssertForeignKeysEnforced(TvContext context) + // Foreign-key enforcement is the whole point of splitting the seed and migrate contexts, so prove it + // took effect rather than trusting a connection-string keyword: a typo, a pragma reset or a leaked + // SET FOREIGN_KEY_CHECKS=0 would silently revert this to its weaker form and still pass everything. + private async Task AssertForeignKeysEnforced(TvContext context) { - (await context.Connection.ExecuteScalarAsync("PRAGMA foreign_keys")).ShouldBe( + string sql = provider is TestProvider.Sqlite + ? "PRAGMA foreign_keys" + : "SELECT @@SESSION.foreign_key_checks"; + + (await context.Connection.ExecuteScalarAsync(sql)).ShouldBe( 1, "the migration must run with foreign keys ENFORCED — otherwise DELETE FROM LibraryFolder is " + "not actually tested against the Restrict constraints it must not violate"); } + private async Task AssertHelperTablesDropped(TvContext context) + { + string sql = provider is TestProvider.Sqlite + ? "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name LIKE '__LibraryFolderDedupe%'" + : "SELECT COUNT(*) FROM information_schema.TABLES WHERE TABLE_SCHEMA = DATABASE() " + + "AND TABLE_NAME LIKE '\\_\\_LibraryFolderDedupe%'"; + + (await context.Connection.ExecuteScalarAsync(sql)).ShouldBe(0, "helper tables were left behind"); + } + + private async Task AssertUniqueIndexExists(TvContext context) + { + string sql = provider is TestProvider.Sqlite + ? "SELECT COUNT(*) FROM sqlite_master WHERE type = 'index' " + + "AND name = 'IX_LibraryFolder_LibraryPathId_PathHash'" + : "SELECT COUNT(DISTINCT INDEX_NAME) FROM information_schema.STATISTICS " + + "WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'LibraryFolder' " + + "AND INDEX_NAME = 'IX_LibraryFolder_LibraryPathId_PathHash'"; + + (await context.Connection.ExecuteScalarAsync(sql)).ShouldBe(1, "the unique index was not created"); + } + + private static bool IsTrue(string value) => + value is "1" || string.Equals(value, "true", StringComparison.OrdinalIgnoreCase); + + private DbContextOptions SqliteOptions(bool foreignKeys) => + new DbContextOptionsBuilder() + .UseSqlite( + $"Data Source={_databasePath};Foreign Keys={foreignKeys}", + o => o.MigrationsAssembly("ErsatzTV.Infrastructure.Sqlite")) + .Options; + + private DbContextOptions MySqlOptions() => + new DbContextOptionsBuilder() + .UseMySql( + _mySqlConnectionString, + ServerVersion.AutoDetect(_mySqlConnectionString), + o => o.MigrationsAssembly("ErsatzTV.Infrastructure.MySql")) + .Options; + private TvContext SeedContext() => Create(_seedOptions); private TvContext MigrateContext() => Create(_migrateOptions); @@ -253,4 +372,42 @@ public class LibraryFolderDedupeMigrationTests options, NullLoggerFactory.Instance, new SlowQueryInterceptor(NullLogger.Instance)); + + // Settable properties rather than a positional record: SQLite hands back INTEGER as Int64 while MySQL + // hands back INT as Int32, and Dapper only narrows for property setters, not constructor matching. + private sealed class FolderRow + { + public int Id { get; set; } + public string Path { get; set; } + public int? ParentId { get; set; } + public string Etag { get; set; } + } + + /// + /// Holds one connection open for the whole seeding block. SQLite disables foreign keys through a + /// connection-string keyword, but MySQL's foreign_key_checks is a SESSION variable — and + /// Dapper closes a connection it had to open itself, which would reset it between statements. + /// Opening explicitly keeps the session, and therefore the setting, alive across every insert. + /// + private sealed class SeedSession : IAsyncDisposable + { + private readonly TvContext _context; + + private SeedSession(TvContext context) => _context = context; + + public static async Task OpenAsync(TvContext context, TestProvider provider) + { + await context.Database.OpenConnectionAsync(); + if (provider is TestProvider.MySql) + { + await context.Connection.ExecuteAsync("SET SESSION foreign_key_checks = 0"); + } + + return new SeedSession(context); + } + + public Task ExecuteAsync(string sql) => _context.Connection.ExecuteAsync(sql); + + public async ValueTask DisposeAsync() => await _context.Database.CloseConnectionAsync(); + } } diff --git a/ErsatzTV.Tests/Integration/LibraryRepositoryTests.cs b/ErsatzTV.Tests/Integration/LibraryRepositoryTests.cs index a129bfc64..f464bf3c0 100644 --- a/ErsatzTV.Tests/Integration/LibraryRepositoryTests.cs +++ b/ErsatzTV.Tests/Integration/LibraryRepositoryTests.cs @@ -216,8 +216,9 @@ public class LibraryRepositoryTests LibraryRepository.ResolveExact([], "/x/foo").ShouldBeNull(); } - // MySQL's default collation is not only case-insensitive, it is also PAD SPACE (verified on 8.4: - // '/x/foo' = '/x/foo ' is TRUE under utf8mb4_bin and utf8mb4_general_ci alike), so the SQL narrowing + // MySQL's comparison is always case-insensitive and, on a PAD SPACE collation, also ignores trailing + // spaces ('/x/foo' = '/x/foo ' is TRUE under utf8mb4_general_ci and utf8mb4_bin; 8.4's default + // utf8mb4_0900_ai_ci is NO PAD, so this axis is server-dependent). Where it applies, the SQL narrowing // hands back trailing-space siblings too. Those are distinct directories on Linux and hash // differently, so the ordinal settle has to keep them apart — same defect class as the case variant, // second axis. Ordinal compares length first, so this holds; pin it.