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

Replace custom item display preferences in one transaction

Original-merge: f26319968a

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

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

tests/Jellyfin.Server.Implementations.Tests/Users/DisplayPreferencesManagerConcurrentCreateTests.cs

@@ -0,0 +1,77 @@
+using System;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Server.Implementations.Tests.Item;
+using Jellyfin.Server.Implementations.Users;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Users;
+
+/// <summary>
+/// Two first-ever requests for the same preferences can both find nothing and both insert; the unique index lets one
+/// through. The other has to return the stored preferences instead of failing.
+/// </summary>
+public sealed class DisplayPreferencesManagerConcurrentCreateTests : SqliteDbTestFixture
+{
+    private const string Client = "client";
+
+    private static readonly Guid _userId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa");
+    private static readonly Guid _itemId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb");
+
+    private readonly CompetingInsert _competingInsert;
+    private readonly DisplayPreferencesManager _manager;
+
+    public DisplayPreferencesManagerConcurrentCreateTests()
+        : this(new CompetingInsert())
+    {
+    }
+
+    private DisplayPreferencesManagerConcurrentCreateTests(CompetingInsert competingInsert)
+        : base(competingInsert)
+    {
+        _competingInsert = competingInsert;
+        _manager = new DisplayPreferencesManager(CreateDbContextFactory());
+
+        using var context = CreateDbContext();
+        context.Users.Add(new User("user", "auth-provider", "reset-provider") { Id = _userId });
+        context.SaveChanges();
+    }
+
+    [Fact]
+    public void GetDisplayPreferences_AnotherRequestStoresThemFirst_ReturnsTheStoredPreferences()
+    {
+        var storedId = 0;
+        _competingInsert.Before = () =>
+        {
+            using var context = CreateDbContext();
+            var stored = new DisplayPreferences(_userId, _itemId, Client);
+            context.DisplayPreferences.Add(stored);
+            context.SaveChanges();
+            storedId = stored.Id;
+        };
+
+        var preferences = _manager.GetDisplayPreferences(_userId, _itemId, Client);
+
+        Assert.NotEqual(0, storedId);
+        Assert.Equal(storedId, preferences.Id);
+        using var check = CreateDbContext();
+        Assert.Single(check.DisplayPreferences);
+    }
+
+    /// <summary>
+    /// Stores the other request's row just before the next save, after the manager has looked for the preferences.
+    /// </summary>
+    private sealed class CompetingInsert : SaveChangesInterceptor
+    {
+        public Action? Before { get; set; }
+
+        public override InterceptionResult<int> SavingChanges(DbContextEventData eventData, InterceptionResult<int> result)
+        {
+            // Disarmed first, because the competing save goes through this interceptor as well.
+            var before = Before;
+            Before = null;
+            before?.Invoke();
+            return result;
+        }
+    }
+}

tests/Jellyfin.Server.Implementations.Tests/Users/DisplayPreferencesManagerFailedWriteTests.cs

@@ -0,0 +1,99 @@
+using System;
+using System.Collections.Generic;
+using System.Data.Common;
+using Jellyfin.Server.Implementations.Tests.Item;
+using Jellyfin.Server.Implementations.Users;
+using Microsoft.Data.Sqlite;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Users;
+
+public sealed class DisplayPreferencesManagerFailedWriteTests : SqliteDbTestFixture
+{
+    private const string Client = "client";
+
+    private static readonly Guid _userId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa");
+    private static readonly Guid _itemId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb");
+
+    private readonly StatementInterceptor _statements;
+    private readonly DisplayPreferencesManager _manager;
+
+    public DisplayPreferencesManagerFailedWriteTests()
+        : this(new StatementInterceptor())
+    {
+    }
+
+    private DisplayPreferencesManagerFailedWriteTests(StatementInterceptor statements)
+        : base(statements)
+    {
+        _statements = statements;
+        _manager = new DisplayPreferencesManager(CreateDbContextFactory());
+    }
+
+    [Fact]
+    public void SetCustomItemDisplayPreferences_InsertFails_KeepsThePreviousPreferences()
+    {
+        _manager.SetCustomItemDisplayPreferences(_userId, _itemId, Client, new Dictionary<string, string?> { ["first"] = "1" });
+
+        _statements.FailInserts = true;
+        Assert.Throws<DbUpdateException>(() => _manager.SetCustomItemDisplayPreferences(_userId, _itemId, Client, new Dictionary<string, string?> { ["second"] = "2" }));
+
+        Assert.Equal(
+            new Dictionary<string, string?> { ["first"] = "1" },
+            _manager.ListCustomItemDisplayPreferences(_userId, _itemId, Client));
+    }
+
+    [Fact]
+    public void SetCustomItemDisplayPreferences_AnotherRequestStoredAKeyAfterTheDelete_StoresThePreferencesItWasGiven()
+    {
+        _manager.SetCustomItemDisplayPreferences(_userId, _itemId, Client, new Dictionary<string, string?> { ["first"] = "1" });
+
+        // The delete misses the row, as it does when another request stores it after this one's delete, so the unique
+        // index rejects this request's insert.
+        _statements.SkipNextDelete = true;
+        _manager.SetCustomItemDisplayPreferences(_userId, _itemId, Client, new Dictionary<string, string?> { ["first"] = "2" });
+
+        Assert.Equal(
+            new Dictionary<string, string?> { ["first"] = "2" },
+            _manager.ListCustomItemDisplayPreferences(_userId, _itemId, Client));
+    }
+
+    /// <summary>
+    /// Fails inserts into CustomItemDisplayPreferences with the error SQLite reports when the unique index rejects a row, or
+    /// leaves out the next delete from it.
+    /// </summary>
+    private sealed class StatementInterceptor : DbCommandInterceptor
+    {
+        public bool FailInserts { get; set; }
+
+        public bool SkipNextDelete { get; set; }
+
+        public override InterceptionResult<DbDataReader> ReaderExecuting(DbCommand command, CommandEventData eventData, InterceptionResult<DbDataReader> result)
+        {
+            FailInsert(command);
+            return result;
+        }
+
+        public override InterceptionResult<int> NonQueryExecuting(DbCommand command, CommandEventData eventData, InterceptionResult<int> result)
+        {
+            if (SkipNextDelete && command.CommandText.StartsWith("DELETE FROM \"CustomItemDisplayPreferences\"", StringComparison.Ordinal))
+            {
+                SkipNextDelete = false;
+                return InterceptionResult<int>.SuppressWithResult(0);
+            }
+
+            FailInsert(command);
+            return result;
+        }
+
+        private void FailInsert(DbCommand command)
+        {
+            if (FailInserts && command.CommandText.StartsWith("INSERT INTO \"CustomItemDisplayPreferences\"", StringComparison.Ordinal))
+            {
+                throw new SqliteException("SQLite Error 19: 'UNIQUE constraint failed: CustomItemDisplayPreferences.UserId, CustomItemDisplayPreferences.ItemId, CustomItemDisplayPreferences.Client, CustomItemDisplayPreferences.Key'.", 19);
+            }
+        }
+    }
+}