Merge pull request #17835 from Shadowghost/fix-migrations

Clean up invalid data before running migrations

Jellyfin.Server/Migrations/Routines/20260825200000_ConsolidateLocalizedUserViews.cs

@@ -156,6 +156,7 @@ internal class ConsolidateLocalizedUserViews : IAsyncMigrationRoutine
 
         await MoveAncestorsAsync(dbContext, canonicalId, staleIds, cancellationToken).ConfigureAwait(false);
         await MoveUserSettingsAsync(dbContext, canonicalId, sourceId, staleIds, cancellationToken).ConfigureAwait(false);
+        await MoveRemainingReferencesAsync(dbContext, newParentId, staleIds, cancellationToken).ConfigureAwait(false);
 
         // Nothing points at them any more, and BaseItems cascades on ParentId, so this has to come last.
         await dbContext.BaseItems
@@ -171,6 +172,31 @@ internal class ConsolidateLocalizedUserViews : IAsyncMigrationRoutine
             canonicalId);
     }
 
+    private static async Task MoveRemainingReferencesAsync(
+        JellyfinDbContext dbContext,
+        Guid? canonicalId,
+        IReadOnlyList<Guid> staleIds,
+        CancellationToken cancellationToken)
+    {
+        await dbContext.BaseItems
+            .Where(e => e.OwnerId.HasValue)
+            .WhereOneOrMany(staleIds, e => e.OwnerId!.Value)
+            .ExecuteUpdateAsync(e => e.SetProperty(f => f.OwnerId, canonicalId), cancellationToken)
+            .ConfigureAwait(false);
+
+        // Keyed by (ParentId, SortOrder), so these cannot be repointed onto the canonical view
+        // without risking a collision, and a view listing linked children is meaningless anyway.
+        await dbContext.LinkedChildren
+            .WhereOneOrMany(staleIds, e => e.ParentId)
+            .ExecuteDeleteAsync(cancellationToken)
+            .ConfigureAwait(false);
+
+        await dbContext.LinkedChildren
+            .WhereOneOrMany(staleIds, e => e.ChildId)
+            .ExecuteDeleteAsync(cancellationToken)
+            .ConfigureAwait(false);
+    }
+
     private async Task<UserView> PickSourceAsync(
         JellyfinDbContext dbContext,
         IReadOnlyList<UserView> stale,
@@ -294,8 +320,10 @@ internal class ConsolidateLocalizedUserViews : IAsyncMigrationRoutine
         IReadOnlyList<Guid> staleIds,
         CancellationToken cancellationToken)
     {
+        // Ancestry recorded against items that no longer exist is dead weight.
         var items = await dbContext.AncestorIds
             .WhereOneOrMany(staleIds, e => e.ParentItemId)
+            .Where(e => dbContext.BaseItems.Any(item => item.Id.Equals(e.ItemId)))
             .Select(e => e.ItemId)
             .Distinct()
             .ToListAsync(cancellationToken)

src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20250913211637_AddProperParentChildRelationBaseItemWithCascade.cs

@@ -11,27 +11,19 @@ namespace Jellyfin.Server.Implementations.Migrations
         protected override void Up(MigrationBuilder migrationBuilder)
         {
             migrationBuilder.Sql("""
-DELETE FROM BaseItems
-        WHERE
-        ParentId IS NOT NULL
-        AND
-        NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId);
-DELETE FROM BaseItems
-        WHERE
-        ParentId IS NOT NULL
-        AND
-        NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId);
-DELETE FROM BaseItems
-        WHERE
-        ParentId IS NOT NULL
-        AND
-        NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId);
-DELETE FROM BaseItems
-        WHERE
-        ParentId IS NOT NULL
-        AND
-        NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId);
-""");
+                WITH RECURSIVE Orphan ("Id") AS (
+                    SELECT Child."Id"
+                    FROM "BaseItems" AS Child
+                    WHERE Child."ParentId" IS NOT NULL
+                      AND NOT EXISTS (SELECT 1 FROM "BaseItems" AS Parent WHERE Parent."Id" = Child."ParentId")
+                    UNION
+                    SELECT Descendant."Id"
+                    FROM "BaseItems" AS Descendant
+                    INNER JOIN Orphan ON Descendant."ParentId" = Orphan."Id"
+                )
+                DELETE FROM "BaseItems" WHERE "Id" IN (SELECT "Id" FROM Orphan);
+                """);
+
             migrationBuilder.AddForeignKey(
                 name: "FK_BaseItems_BaseItems_ParentId",
                 table: "BaseItems",

src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260113203012_ChangeOwnerIdToGuid.cs

@@ -11,6 +11,61 @@ namespace Jellyfin.Database.Providers.Sqlite.Migrations
         /// <inheritdoc />
         protected override void Up(MigrationBuilder migrationBuilder)
         {
+            migrationBuilder.Sql(
+                """
+                DROP TABLE IF EXISTS "OrphanedBaseItemIds";
+                CREATE TEMPORARY TABLE "OrphanedBaseItemIds" ("Id" TEXT NOT NULL PRIMARY KEY);
+
+                INSERT INTO "OrphanedBaseItemIds" ("Id")
+                WITH RECURSIVE Orphan ("Id") AS (
+                    SELECT Child."Id"
+                    FROM "BaseItems" AS Child
+                    WHERE Child."ParentId" IS NOT NULL
+                      AND NOT EXISTS (SELECT 1 FROM "BaseItems" AS Parent WHERE Parent."Id" = Child."ParentId")
+                    UNION
+                    SELECT Descendant."Id"
+                    FROM "BaseItems" AS Descendant
+                    INNER JOIN Orphan ON Descendant."ParentId" = Orphan."Id"
+                )
+                SELECT "Id" FROM Orphan;
+
+                -- Keep the play state of the doomed items the way ItemPersistenceService does when it
+                -- deletes an item: reattach it to the placeholder item instead of letting the
+                -- FK_UserData_BaseItems_ItemId cascade wipe it. The placeholder can only hold one row
+                -- per (UserId, CustomDataKey), so resolve collisions before repointing anything.
+                DELETE FROM "UserData"
+                WHERE "ItemId" = '00000000-0000-0000-0000-000000000001'
+                  AND EXISTS (
+                      SELECT 1
+                      FROM "UserData" AS Doomed
+                      INNER JOIN "OrphanedBaseItemIds" AS Orphan ON Orphan."Id" = Doomed."ItemId"
+                      WHERE Doomed."UserId" = "UserData"."UserId"
+                        AND Doomed."CustomDataKey" = "UserData"."CustomDataKey");
+
+                DELETE FROM "UserData"
+                WHERE "ItemId" IN (SELECT "Id" FROM "OrphanedBaseItemIds")
+                  AND "rowid" NOT IN (
+                      SELECT MIN("rowid")
+                      FROM "UserData"
+                      WHERE "ItemId" IN (SELECT "Id" FROM "OrphanedBaseItemIds")
+                      GROUP BY "UserId", "CustomDataKey");
+
+                UPDATE "UserData"
+                SET "ItemId" = '00000000-0000-0000-0000-000000000001',
+                    "RetentionDate" = datetime('now')
+                WHERE "ItemId" IN (SELECT "Id" FROM "OrphanedBaseItemIds");
+
+                -- FK_LinkedChildren_BaseItems_{ParentId,ChildId} are NO ACTION, so these rows have to
+                -- go by hand or the delete below fails on them.
+                DELETE FROM "LinkedChildren"
+                WHERE "ParentId" IN (SELECT "Id" FROM "OrphanedBaseItemIds")
+                   OR "ChildId" IN (SELECT "Id" FROM "OrphanedBaseItemIds");
+
+                DELETE FROM "BaseItems" WHERE "Id" IN (SELECT "Id" FROM "OrphanedBaseItemIds");
+
+                DROP TABLE "OrphanedBaseItemIds";
+                """);
+
             // Normalize OwnerId to uppercase GUID format
             migrationBuilder.Sql(
                 @"UPDATE BaseItems

src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.cs

@@ -11,8 +11,8 @@ namespace Jellyfin.Database.Providers.Sqlite.Migrations
         /// <inheritdoc />
         protected override void Up(MigrationBuilder migrationBuilder)
         {
-            migrationBuilder.Sql("DELETE FROM Permissions WHERE UserId IS NULL;");
-            migrationBuilder.Sql("DELETE FROM Preferences WHERE UserId IS NULL;");
+            migrationBuilder.Sql("DELETE FROM Permissions WHERE UserId IS NULL OR UserId NOT IN (SELECT Id FROM Users);");
+            migrationBuilder.Sql("DELETE FROM Preferences WHERE UserId IS NULL OR UserId NOT IN (SELECT Id FROM Users);");
 
             migrationBuilder.DropIndex(
                 name: "IX_Preferences_UserId_Kind",

tests/Jellyfin.Server.Tests/Migrations/ConsolidateLocalizedUserViewsTests.cs

@@ -0,0 +1,202 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Jellyfin.Data.Enums;
+using Jellyfin.Database.Implementations;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Database.Implementations.Locking;
+using Jellyfin.Database.Providers.Sqlite;
+using Jellyfin.Server.Migrations.Routines;
+using Jellyfin.Server.ServerSetupApp;
+using MediaBrowser.Common.Configuration;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Model.IO;
+using Microsoft.Data.Sqlite;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Tests.Migrations;
+
+/// <summary>
+/// Covers the references a database carried up from 10.x can still hold against a view the migration
+/// is about to drop. Only ParentId cascades; everything else here is a NO ACTION foreign key that
+/// used to abort the migration, and with it the whole startup.
+/// </summary>
+public sealed class ConsolidateLocalizedUserViewsTests : IDisposable
+{
+    private const string MetadataPath = "/metadata";
+
+    private static readonly Guid _staleId = new("11111111-1111-1111-1111-111111111111");
+    private static readonly Guid _canonicalId = new("22222222-2222-2222-2222-222222222222");
+
+    private readonly SqliteConnection _connection;
+    private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
+
+    public ConsolidateLocalizedUserViewsTests()
+    {
+        _connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True");
+        _connection.Open();
+
+        _dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
+            .UseSqlite(_connection)
+            .Options;
+
+        using var context = CreateDbContext();
+        context.Database.EnsureCreated();
+    }
+
+    private static CancellationToken Ct => TestContext.Current.CancellationToken;
+
+    [Fact]
+    public async Task PerformAsync_ItemOwnedByStaleView_MovesItAndDropsTheView()
+    {
+        using (var context = CreateDbContext())
+        {
+            context.BaseItems.Add(StaleView());
+            context.BaseItems.Add(new BaseItemEntity { Id = Guid.NewGuid(), Type = "Trailer", OwnerId = _staleId });
+            await context.SaveChangesAsync(Ct);
+        }
+
+        await CreateMigration().PerformAsync(Ct);
+
+        using (var context = CreateDbContext())
+        {
+            Assert.Null(await context.BaseItems.FindAsync([_staleId], Ct));
+            Assert.Equal(_canonicalId, (await context.BaseItems.SingleAsync(e => e.Type == "Trailer", Ct)).OwnerId);
+        }
+    }
+
+    [Fact]
+    public async Task PerformAsync_StaleViewInLinkedChildren_DropsTheLinksAndTheView()
+    {
+        var movieId = Guid.NewGuid();
+
+        using (var context = CreateDbContext())
+        {
+            context.BaseItems.Add(StaleView());
+            context.BaseItems.Add(new BaseItemEntity { Id = movieId, Type = "Movie" });
+            context.LinkedChildren.Add(new LinkedChildEntity { ParentId = _staleId, SortOrder = 0, ChildId = movieId, ChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType.Manual });
+            context.LinkedChildren.Add(new LinkedChildEntity { ParentId = movieId, SortOrder = 0, ChildId = _staleId, ChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType.Manual });
+            await context.SaveChangesAsync(Ct);
+        }
+
+        await CreateMigration().PerformAsync(Ct);
+
+        using (var context = CreateDbContext())
+        {
+            Assert.Null(await context.BaseItems.FindAsync([_staleId], Ct));
+            Assert.Empty(context.LinkedChildren);
+            Assert.NotNull(await context.BaseItems.FindAsync([movieId], Ct));
+        }
+    }
+
+    [Fact]
+    public async Task PerformAsync_OrphanedAncestry_IsNotResurrectedUnderTheCanonicalView()
+    {
+        var childId = Guid.NewGuid();
+        var orphanId = Guid.NewGuid();
+
+        using (var context = CreateDbContext())
+        {
+            context.BaseItems.Add(StaleView());
+            context.BaseItems.Add(new BaseItemEntity { Id = childId, Type = "Movie", ParentId = _staleId });
+            await context.SaveChangesAsync(Ct);
+
+            context.AncestorIds.Add(new AncestorId { ItemId = childId, ParentItemId = _staleId, Item = null!, ParentItem = null! });
+            await context.SaveChangesAsync(Ct);
+
+            // Written while foreign keys went unenforced: the item behind it is long gone.
+            await context.Database.ExecuteSqlRawAsync("PRAGMA foreign_keys = OFF", Ct);
+            context.AncestorIds.Add(new AncestorId { ItemId = orphanId, ParentItemId = _staleId, Item = null!, ParentItem = null! });
+            await context.SaveChangesAsync(Ct);
+            await context.Database.ExecuteSqlRawAsync("PRAGMA foreign_keys = ON", Ct);
+        }
+
+        await CreateMigration().PerformAsync(Ct);
+
+        using (var context = CreateDbContext())
+        {
+            Assert.Null(await context.BaseItems.FindAsync([_staleId], Ct));
+            Assert.Equal(_canonicalId, (await context.BaseItems.SingleAsync(e => e.Id.Equals(childId), Ct)).ParentId);
+            var ancestors = await context.AncestorIds.ToListAsync(Ct);
+            Assert.Equal(new[] { childId }, ancestors.Select(e => e.ItemId));
+            Assert.Equal(_canonicalId, ancestors[0].ParentItemId);
+        }
+    }
+
+    public void Dispose()
+    {
+        _connection.Dispose();
+        GC.SuppressFinalize(this);
+    }
+
+    private static BaseItemEntity StaleView() => new()
+    {
+        Id = _staleId,
+        Type = "MediaBrowser.Controller.Entities.UserView",
+        Path = Path.Combine(MetadataPath, "views", "livetv")
+    };
+
+    private JellyfinDbContext CreateDbContext() => new(
+        _dbOptions,
+        NullLogger<JellyfinDbContext>.Instance,
+        new SqliteDatabaseProvider(new Mock<IApplicationPaths>().Object, NullLogger<SqliteDatabaseProvider>.Instance),
+        new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
+
+    private ConsolidateLocalizedUserViews CreateMigration()
+    {
+        var view = new UserView
+        {
+            Id = _staleId,
+            Path = Path.Combine(MetadataPath, "views", "livetv"),
+            Name = "Live TV",
+            ViewType = CollectionType.livetv
+        };
+
+        var applicationPaths = new Mock<IServerApplicationPaths>();
+        applicationPaths.Setup(e => e.InternalMetadataPath).Returns(MetadataPath);
+
+        var configurationManager = new Mock<IServerConfigurationManager>();
+        configurationManager.Setup(e => e.ApplicationPaths).Returns(applicationPaths.Object);
+
+        var fileSystem = new Mock<IFileSystem>();
+        fileSystem.Setup(e => e.GetValidFilename(It.IsAny<string>())).Returns((string name) => name);
+
+        var libraryManager = new Mock<ILibraryManager>();
+        libraryManager.Setup(e => e.GetItemList(It.IsAny<InternalItemsQuery>()))
+            .Returns(new List<BaseItem> { view });
+        libraryManager.Setup(e => e.GetNewItemId(It.IsAny<string>(), It.IsAny<Type>()))
+            .Returns(_canonicalId);
+        libraryManager.Setup(e => e.CreateItem(It.IsAny<BaseItem>(), It.IsAny<BaseItem?>()))
+            .Callback((BaseItem item, BaseItem? parent) =>
+            {
+                using var context = CreateDbContext();
+                context.BaseItems.Add(new BaseItemEntity
+                {
+                    Id = item.Id,
+                    Type = item.GetType().FullName!,
+                    Path = item.Path
+                });
+                context.SaveChanges();
+            });
+
+        var factory = new Mock<IDbContextFactory<JellyfinDbContext>>();
+        factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext);
+        factory.Setup(f => f.CreateDbContextAsync(It.IsAny<CancellationToken>())).ReturnsAsync(CreateDbContext);
+
+        return new ConsolidateLocalizedUserViews(
+            new StartupLogger<ConsolidateLocalizedUserViews>(NullLogger<ConsolidateLocalizedUserViews>.Instance),
+            libraryManager.Object,
+            configurationManager.Object,
+            fileSystem.Object,
+            factory.Object);
+    }
+}