Backport pull request #18001 from jellyfin/release-12.z

Keep an item's user data rows in agreement

Original-merge: fc98019b12e311fea244dccef72d521278326024

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

3 files changed, 337 insertions, 0 deletions

diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceOwnedRowTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceOwnedRowTests.cs
index 82614c3156..fea6ccb7a1 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceOwnedRowTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceOwnedRowTests.cs
@@ -2,12 +2,16 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
+using System.Threading.Tasks;
 using Jellyfin.Database.Implementations;
+using Jellyfin.Database.Implementations.Entities;
 using Jellyfin.Server.Implementations.Item;
 using MediaBrowser.Controller;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Movies;
 using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.LiveTv;
 using MediaBrowser.Model.Configuration;
 using MediaBrowser.Model.Entities;
 using Microsoft.Extensions.Logging.Abstractions;
@@ -21,12 +25,15 @@ public sealed class ItemPersistenceOwnedRowTests : SqliteDbTestFixture
     private readonly ItemPersistenceService _service;
     private readonly ILibraryManager? _previousLibraryManager;
     private readonly IServerConfigurationManager? _previousConfigurationManager;
+    private readonly IRecordingsManager? _previousRecordingsManager;
+    private readonly Guid _userId = Guid.Parse("11111111-1111-1111-1111-111111111111");
 
     public ItemPersistenceOwnedRowTests()
     {
         // BaseItem resolves these through process-wide statics; restored in Dispose.
         _previousLibraryManager = BaseItem.LibraryManager;
         _previousConfigurationManager = BaseItem.ConfigurationManager;
+        _previousRecordingsManager = Video.RecordingsManager;
 
         var libraryManager = new Mock<ILibraryManager>();
         libraryManager.Setup(l => l.GetCollectionFolders(It.IsAny<BaseItem>()))
@@ -37,6 +44,9 @@ public sealed class ItemPersistenceOwnedRowTests : SqliteDbTestFixture
         configurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration());
         BaseItem.ConfigurationManager = configurationManager.Object;
 
+        // Video.SourceType consults this before it can produce user data keys.
+        Video.RecordingsManager = new Mock<IRecordingsManager>().Object;
+
         _service = new ItemPersistenceService(
             CreateDbContextFactory(),
             new Mock<IServerApplicationHost>().Object,
@@ -47,6 +57,7 @@ public sealed class ItemPersistenceOwnedRowTests : SqliteDbTestFixture
     {
         BaseItem.LibraryManager = _previousLibraryManager!;
         BaseItem.ConfigurationManager = _previousConfigurationManager!;
+        Video.RecordingsManager = _previousRecordingsManager!;
         base.Dispose(disposing);
     }
 
@@ -102,6 +113,99 @@ public sealed class ItemPersistenceOwnedRowTests : SqliteDbTestFixture
         Assert.Equal("777", Assert.Single(ctx.BaseItemProviders.Where(e => e.ItemId.Equals(fresh))).ProviderValue);
     }
 
+    [Fact]
+    public async Task ReattachUserData_DetachedRowsFromDifferentEras_CollapsesToMostRecentPlay()
+    {
+        var movie = CreateMovie(Guid.Parse("dddddddd-dddd-dddd-dddd-dddddddddddd"));
+        var keys = movie.GetUserDataKeys();
+        SeedUserDataItem(movie);
+
+        using (var ctx = CreateDbContext())
+        {
+            // The guid-keyed row was detached by an older deletion than the provider-keyed ones.
+            ctx.UserData.AddRange(
+                CreateDetachedRow(keys[^1], new DateTime(2021, 12, 31, 0, 0, 0, DateTimeKind.Utc), playCount: 7, positionTicks: 490),
+                CreateDetachedRow(keys[0], new DateTime(2023, 8, 14, 0, 0, 0, DateTimeKind.Utc), playCount: 9, positionTicks: 0, played: true),
+                CreateDetachedRow(keys[1], new DateTime(2023, 8, 14, 0, 0, 0, DateTimeKind.Utc), playCount: 9, positionTicks: 0, played: true));
+            await ctx.SaveChangesAsync(TestContext.Current.CancellationToken);
+        }
+
+        await _service.ReattachUserDataAsync(movie, TestContext.Current.CancellationToken);
+
+        using (var ctx = CreateDbContext())
+        {
+            var rows = ctx.UserData.Where(e => e.ItemId.Equals(movie.Id)).ToList();
+
+            Assert.Equal(keys.Count, rows.Count);
+            Assert.Equal(keys.OrderBy(e => e, StringComparer.Ordinal), rows.Select(e => e.CustomDataKey).OrderBy(e => e, StringComparer.Ordinal));
+            Assert.All(rows, row =>
+            {
+                Assert.True(row.Played);
+                Assert.Equal(0, row.PlaybackPositionTicks);
+                Assert.Equal(9, row.PlayCount);
+                Assert.Null(row.RetentionDate);
+            });
+
+            Assert.Empty(ctx.UserData.Where(e => e.ItemId.Equals(BaseItemRepository.PlaceholderId)));
+        }
+    }
+
+    [Fact]
+    public async Task ReattachUserData_ExistingRowUnderUnreportedKey_IsKeptInAgreement()
+    {
+        var movie = CreateMovie(Guid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff"));
+        var keys = movie.GetUserDataKeys();
+        const string UnreportedKey = "tvdb-key-missing-mid-refresh";
+        SeedUserDataItem(movie);
+
+        using (var ctx = CreateDbContext())
+        {
+            ctx.UserData.AddRange(
+                CreateRow(movie.Id, UnreportedKey, new DateTime(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc), playCount: 1, positionTicks: 123),
+                CreateDetachedRow(keys[0], new DateTime(2025, 1, 1, 0, 0, 0, DateTimeKind.Utc), playCount: 2, positionTicks: 0, played: true));
+            await ctx.SaveChangesAsync(TestContext.Current.CancellationToken);
+        }
+
+        await _service.ReattachUserDataAsync(movie, TestContext.Current.CancellationToken);
+
+        using (var ctx = CreateDbContext())
+        {
+            var rows = ctx.UserData.Where(e => e.ItemId.Equals(movie.Id)).ToList();
+
+            Assert.Equal(
+                keys.Append(UnreportedKey).OrderBy(e => e, StringComparer.Ordinal),
+                rows.Select(e => e.CustomDataKey).OrderBy(e => e, StringComparer.Ordinal));
+            Assert.All(rows, row =>
+            {
+                Assert.True(row.Played);
+                Assert.Equal(2, row.PlayCount);
+                Assert.Equal(0, row.PlaybackPositionTicks);
+            });
+        }
+    }
+
+    [Fact]
+    public async Task ReattachUserData_NoDetachedRows_LeavesExistingRowsAlone()
+    {
+        var movie = CreateMovie(Guid.Parse("eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee"));
+        var keys = movie.GetUserDataKeys();
+        SeedUserDataItem(movie);
+
+        using (var ctx = CreateDbContext())
+        {
+            ctx.UserData.Add(CreateRow(movie.Id, keys[0], new DateTime(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc), playCount: 1, positionTicks: 123));
+            await ctx.SaveChangesAsync(TestContext.Current.CancellationToken);
+        }
+
+        await _service.ReattachUserDataAsync(movie, TestContext.Current.CancellationToken);
+
+        using (var ctx = CreateDbContext())
+        {
+            var row = Assert.Single(ctx.UserData.Where(e => e.ItemId.Equals(movie.Id)));
+            Assert.Equal(123, row.PlaybackPositionTicks);
+        }
+    }
+
     private static Book CreateBook(Guid id, Dictionary<string, string> providerIds, MetadataField[] lockedFields)
     {
         var book = new Book
@@ -115,4 +219,59 @@ public sealed class ItemPersistenceOwnedRowTests : SqliteDbTestFixture
         book.SetImage(new ItemImageInfo { Path = "/img/primary.jpg", Type = ImageType.Primary }, 0);
         return book;
     }
+
+    private static Movie CreateMovie(Guid id)
+    {
+        return new Movie
+        {
+            Id = id,
+            Name = "Black Widow",
+            ProviderIds = new Dictionary<string, string>
+            {
+                ["Tmdb"] = "497698",
+                ["Imdb"] = "tt3480822"
+            }
+        };
+    }
+
+    private void SeedUserDataItem(BaseItem item)
+    {
+        using var ctx = CreateDbContext();
+        if (!ctx.Users.Any(e => e.Id.Equals(_userId)))
+        {
+            ctx.Users.Add(new User("user", "auth-provider", "reset-provider") { Id = _userId });
+        }
+
+        if (!ctx.BaseItems.Any(e => e.Id.Equals(BaseItemRepository.PlaceholderId)))
+        {
+            ctx.BaseItems.Add(new BaseItemEntity { Id = BaseItemRepository.PlaceholderId, Type = typeof(Folder).FullName! });
+        }
+
+        ctx.BaseItems.Add(new BaseItemEntity { Id = item.Id, Type = item.GetType().FullName! });
+        ctx.SaveChanges();
+    }
+
+    private UserData CreateDetachedRow(string key, DateTime lastPlayed, int playCount, long positionTicks, bool played = false)
+    {
+        var row = CreateRow(BaseItemRepository.PlaceholderId, key, lastPlayed, playCount, positionTicks, played);
+        row.RetentionDate = new DateTime(2025, 6, 22, 0, 0, 0, DateTimeKind.Utc);
+
+        return row;
+    }
+
+    private UserData CreateRow(Guid itemId, string key, DateTime lastPlayed, int playCount, long positionTicks, bool played = false)
+    {
+        return new UserData
+        {
+            ItemId = itemId,
+            Item = null,
+            UserId = _userId,
+            User = null,
+            CustomDataKey = key,
+            LastPlayedDate = lastPlayed,
+            PlayCount = playCount,
+            PlaybackPositionTicks = positionTicks,
+            Played = played
+        };
+    }
 }
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs
index ba3127bc08..2ed16de15b 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs
@@ -1,5 +1,7 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
 using Emby.Server.Implementations.Library;
 using Jellyfin.Database.Implementations;
 using Jellyfin.Database.Implementations.Entities;
@@ -8,6 +10,7 @@ using Jellyfin.Database.Providers.Sqlite;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Model.Configuration;
+using MediaBrowser.Model.Entities;
 using Microsoft.Data.Sqlite;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Logging.Abstractions;
@@ -208,6 +211,43 @@ public sealed class UserDataManagerTests : IDisposable
     }
 
     [Fact]
+    public void SaveUserData_RowUnderRetiredKey_IsKeptInAgreement()
+    {
+        var item = CreateAudioBook();
+
+        using (var ctx = CreateDbContext())
+        {
+            ctx.Users.Add(_user);
+            ctx.BaseItems.Add(new BaseItemEntity { Id = item.Id, Type = typeof(AudioBook).FullName! });
+            ctx.UserData.Add(CreateUserDataRow(item, "Author-Old Album-0001Old File Name", 111));
+            ctx.SaveChanges();
+        }
+
+        _userDataManager.SaveUserData(
+            _user,
+            item,
+            new UserItemData { Key = item.GetUserDataKeys()[0], Played = true },
+            UserDataSaveReason.UpdateUserRating,
+            CancellationToken.None);
+
+        using (var ctx = CreateDbContext())
+        {
+            var rows = ctx.UserData.Where(e => e.ItemId.Equals(item.Id)).ToList();
+
+            // The retired-key row is what a re-added item reattaches by, so it survives, but it must
+            // not keep a playback position that holds the item in Continue Watching.
+            Assert.Equal(
+                item.GetUserDataKeys().Append("Author-Old Album-0001Old File Name").OrderBy(e => e, StringComparer.Ordinal),
+                rows.Select(e => e.CustomDataKey).OrderBy(e => e, StringComparer.Ordinal));
+            Assert.All(rows, row =>
+            {
+                Assert.True(row.Played);
+                Assert.Equal(0, row.PlaybackPositionTicks);
+            });
+        }
+    }
+
+    [Fact]
     public void GetUserData_NullUser_ThrowsArgumentNullException()
     {
         var item = CreateAudioBook();
diff --git a/tests/Jellyfin.Server.Tests/Migrations/HarmonizeConflictingUserDataTests.cs b/tests/Jellyfin.Server.Tests/Migrations/HarmonizeConflictingUserDataTests.cs
new file mode 100644
index 0000000000..ba0829cb56
--- /dev/null
+++ b/tests/Jellyfin.Server.Tests/Migrations/HarmonizeConflictingUserDataTests.cs
@@ -0,0 +1,138 @@
+using System;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Jellyfin.Database.Implementations;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Database.Implementations.Locking;
+using Jellyfin.Database.Providers.Sqlite;
+using Jellyfin.Server.Implementations.Item;
+using Jellyfin.Server.Migrations.Routines;
+using Jellyfin.Server.ServerSetupApp;
+using MediaBrowser.Common.Configuration;
+using MediaBrowser.Controller.Entities;
+using Microsoft.Data.Sqlite;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Tests.Migrations;
+
+public sealed class HarmonizeConflictingUserDataTests : IDisposable
+{
+    private readonly SqliteConnection _connection;
+    private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
+    private readonly Guid _userId = Guid.Parse("11111111-1111-1111-1111-111111111111");
+    private readonly Guid _itemId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa");
+    private readonly Guid _untouchedId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb");
+
+    public HarmonizeConflictingUserDataTests()
+    {
+        _connection = new SqliteConnection("Data Source=:memory:");
+        _connection.Open();
+
+        _dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
+            .UseSqlite(_connection)
+            .Options;
+
+        using var context = CreateDbContext();
+        context.Database.EnsureCreated();
+    }
+
+    public void Dispose() => _connection.Dispose();
+
+    [Fact]
+    public async Task Perform_ConflictingRows_CollapsesToMostRecentPlay()
+    {
+        using (var ctx = CreateDbContext())
+        {
+            ctx.Users.Add(new User("user", "auth-provider", "reset-provider") { Id = _userId });
+            ctx.BaseItems.Add(new BaseItemEntity { Id = _itemId, Type = typeof(Folder).FullName! });
+            ctx.BaseItems.Add(new BaseItemEntity { Id = _untouchedId, Type = typeof(Folder).FullName! });
+
+            // One item holding rows from two different eras of itself...
+            ctx.UserData.Add(Row(_itemId, "497698", new DateTime(2023, 8, 14, 0, 0, 0, DateTimeKind.Utc), playCount: 9, positionTicks: 0, played: true));
+            ctx.UserData.Add(Row(_itemId, "tt3480822", new DateTime(2021, 12, 31, 0, 0, 0, DateTimeKind.Utc), playCount: 7, positionTicks: 490));
+
+            // ...and one whose rows already agree.
+            ctx.UserData.Add(Row(_untouchedId, "121", new DateTime(2022, 1, 1, 0, 0, 0, DateTimeKind.Utc), playCount: 2, positionTicks: 77));
+            ctx.UserData.Add(Row(_untouchedId, "tt0167261", new DateTime(2022, 1, 1, 0, 0, 0, DateTimeKind.Utc), playCount: 2, positionTicks: 77));
+
+            await ctx.SaveChangesAsync(TestContext.Current.CancellationToken);
+        }
+
+        await CreateMigration().PerformAsync(TestContext.Current.CancellationToken);
+
+        using (var ctx = CreateDbContext())
+        {
+            var rows = ctx.UserData.Where(e => e.ItemId.Equals(_itemId)).ToList();
+            Assert.Equal(2, rows.Count);
+            Assert.All(rows, row =>
+            {
+                Assert.True(row.Played);
+                Assert.Equal(0, row.PlaybackPositionTicks);
+                Assert.Equal(9, row.PlayCount);
+            });
+
+            Assert.All(ctx.UserData.Where(e => e.ItemId.Equals(_untouchedId)), row => Assert.Equal(77, row.PlaybackPositionTicks));
+        }
+    }
+
+    [Fact]
+    public async Task Perform_DetachedRows_AreLeftAlone()
+    {
+        using (var ctx = CreateDbContext())
+        {
+            ctx.Users.Add(new User("user", "auth-provider", "reset-provider") { Id = _userId });
+
+            // Detached rows belong to no item, so they are not a conflict and must keep their state
+            // for the next item that claims one of their keys.
+            ctx.UserData.Add(Row(BaseItemRepository.PlaceholderId, "497698", new DateTime(2023, 8, 14, 0, 0, 0, DateTimeKind.Utc), playCount: 9, positionTicks: 0, played: true));
+            ctx.UserData.Add(Row(BaseItemRepository.PlaceholderId, "tt3480822", new DateTime(2021, 12, 31, 0, 0, 0, DateTimeKind.Utc), playCount: 7, positionTicks: 490));
+
+            await ctx.SaveChangesAsync(TestContext.Current.CancellationToken);
+        }
+
+        await CreateMigration().PerformAsync(TestContext.Current.CancellationToken);
+
+        using (var ctx = CreateDbContext())
+        {
+            var rows = ctx.UserData.Where(e => e.ItemId.Equals(BaseItemRepository.PlaceholderId)).OrderBy(e => e.CustomDataKey).ToList();
+            Assert.Equal(0, rows[0].PlaybackPositionTicks);
+            Assert.Equal(490, rows[1].PlaybackPositionTicks);
+        }
+    }
+
+    private UserData Row(Guid itemId, string key, DateTime lastPlayed, int playCount, long positionTicks, bool played = false)
+    {
+        return new UserData
+        {
+            ItemId = itemId,
+            Item = null,
+            UserId = _userId,
+            User = null,
+            CustomDataKey = key,
+            LastPlayedDate = lastPlayed,
+            PlayCount = playCount,
+            PlaybackPositionTicks = positionTicks,
+            Played = played
+        };
+    }
+
+    private JellyfinDbContext CreateDbContext() => new(
+        _dbOptions,
+        NullLogger<JellyfinDbContext>.Instance,
+        new SqliteDatabaseProvider(new Mock<IApplicationPaths>().Object, NullLogger<SqliteDatabaseProvider>.Instance),
+        new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
+
+    private HarmonizeConflictingUserData CreateMigration()
+    {
+        var factory = new Mock<IDbContextFactory<JellyfinDbContext>>();
+        factory.Setup(f => f.CreateDbContextAsync(It.IsAny<CancellationToken>())).ReturnsAsync(CreateDbContext);
+
+        return new HarmonizeConflictingUserData(
+            new StartupLogger<HarmonizeConflictingUserData>(NullLogger<HarmonizeConflictingUserData>.Instance),
+            factory.Object);
+    }
+}