Murzaby/jellyfin
1
0
Fork 0
mirror of https://github.com/jellyfin/jellyfin.git synced 2026-09-01 01:12:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

Stop user updates from orphaning permission and preference rows (#17645)

Browse source 
* Stop user updates from orphaning permission and preference rows

* Make UserId non-nullable

* Remove unnecessary ToList

* Update Jellyfin.Server.Implementations/Users/UserManager.cs

Co-authored-by: Claus Vium <cvium@users.noreply.github.com>

---------

Co-authored-by: Claus Vium <cvium@users.noreply.github.com>
This commit is contained in:
Tim Eisele 2026-08-20 18:35:26 +02:00 • committed by GitHub
parent c70933a23f
commit fb50b4df8b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 2173 additions and 59 deletions
Download patch file Download diff file Expand all files Collapse all files

69
Jellyfin.Server.Implementations/Users/UserManager.cs
View file

@ -225,19 +225,8 @@ namespace Jellyfin.Server.Implementations.Users
?? throw new ResourceNotFoundException(nameof(user.Id));
dbContext.Entry(dbUser).CurrentValues.SetValues(user);
dbContext.Permissions.RemoveRange(dbUser.Permissions);
dbUser.Permissions.Clear();
foreach (var permission in user.Permissions)
{
dbUser.Permissions.Add(new Permission(permission.Kind, permission.Value));
}
dbContext.Preferences.RemoveRange(dbUser.Preferences);
dbUser.Preferences.Clear();
foreach (var preference in user.Preferences)
{
dbUser.Preferences.Add(new Preference(preference.Kind, preference.Value));
}
SyncPermissions(dbUser, user.Permissions);
SyncPreferences(dbUser, user.Preferences);
dbUser.AccessSchedules.Clear();
foreach (var accessSchedule in user.AccessSchedules)
@ -271,6 +260,60 @@ namespace Jellyfin.Server.Implementations.Users
}
}
private static void SyncPermissions(User dbUser, ICollection<Permission> source)
{
var incoming = new Dictionary<PermissionKind, bool>();
foreach (var permission in source)
{
incoming[permission.Kind] = permission.Value;
}
foreach (var existing in dbUser.Permissions)
{
if (incoming.Remove(existing.Kind, out var value))
{
// EF only marks the row modified if the value actually differs, so an update that
// touches nothing but the user row - a session activity stamp - writes no children.
existing.Value = value;
}
else
{
dbUser.Permissions.Remove(existing);
}
}
foreach (var (kind, value) in incoming)
{
dbUser.Permissions.Add(new Permission(kind, value));
}
}
private static void SyncPreferences(User dbUser, ICollection<Preference> source)
{
var incoming = new Dictionary<PreferenceKind, string>();
foreach (var preference in source)
{
incoming[preference.Kind] = preference.Value;
}
foreach (var existing in dbUser.Preferences)
{
if (incoming.Remove(existing.Kind, out var value))
{
existing.Value = value;
}
else
{
dbUser.Preferences.Remove(existing);
}
}
foreach (var (kind, value) in incoming)
{
dbUser.Preferences.Add(new Preference(kind, value));
}
}
internal async Task<User> CreateUserInternalAsync(string name, JellyfinDbContext dbContext)
{
// TODO: Remove after user item data is migrated.

2
src/Jellyfin.Database/Jellyfin.Database.Implementations/Entities/Permission.cs
View file

@ -37,7 +37,7 @@ namespace Jellyfin.Database.Implementations.Entities
/// <summary>
/// Gets or sets the id of the associated user.
/// </summary>
public Guid? UserId { get; set; }
public Guid UserId { get; set; }
/// <summary>
/// Gets the type of this permission.

2
src/Jellyfin.Database/Jellyfin.Database.Implementations/Entities/Preference.cs
View file

@ -35,7 +35,7 @@ namespace Jellyfin.Database.Implementations.Entities
/// <summary>
/// Gets or sets the id of the associated user.
/// </summary>
public Guid? UserId { get; set; }
public Guid UserId { get; set; }
/// <summary>
/// Gets the type of this preference.

3
src/Jellyfin.Database/Jellyfin.Database.Implementations/Entities/User.cs
View file

@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using System.Text.Json.Serialization;
using Jellyfin.Database.Implementations.Enums;
using Jellyfin.Database.Implementations.Interfaces;
@ -326,7 +325,6 @@ namespace Jellyfin.Database.Implementations.Entities
/// <summary>
/// Gets the list of permissions this user has.
/// </summary>
[ForeignKey("Permission_Permissions_Guid")]
public virtual ICollection<Permission> Permissions { get; private set; }
/*
@ -339,7 +337,6 @@ namespace Jellyfin.Database.Implementations.Entities
/// <summary>
/// Gets the list of preferences this user has.
/// </summary>
[ForeignKey("Preference_Preferences_Guid")]
public virtual ICollection<Preference> Preferences { get; private set; }
/// <inheritdoc/>

2
src/Jellyfin.Database/Jellyfin.Database.Implementations/ModelConfiguration/PermissionConfiguration.cs
View file

@ -14,10 +14,8 @@ namespace Jellyfin.Database.Implementations.ModelConfiguration
{
// Used to get a user's permissions or a specific permission for a user.
// Also prevents multiple values being created for a user.
// Filtered over non-null user ids for when other entities (groups, API keys) get permissions
builder
.HasIndex(p => new { p.UserId, p.Kind })
.HasFilter("[UserId] IS NOT NULL")
.IsUnique();
}
}

1
src/Jellyfin.Database/Jellyfin.Database.Implementations/ModelConfiguration/PreferenceConfiguration.cs
View file

@ -14,7 +14,6 @@ namespace Jellyfin.Database.Implementations.ModelConfiguration
{
builder
.HasIndex(p => new { p.UserId, p.Kind })
.HasFilter("[UserId] IS NOT NULL")
.IsUnique();
}
}

1805
src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.Designer.cs generated Normal file
View file

File diff suppressed because it is too large Load diff

120
src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.cs Normal file
View file

@ -0,0 +1,120 @@
using System;
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Jellyfin.Database.Providers.Sqlite.Migrations
{
/// <inheritdoc />
public partial class RemoveOrphanedUserPermissionsAndPreferences : Migration
{
/// <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.DropIndex(
name: "IX_Preferences_UserId_Kind",
table: "Preferences");
migrationBuilder.DropIndex(
name: "IX_Permissions_UserId_Kind",
table: "Permissions");
migrationBuilder.DropColumn(
name: "Preference_Preferences_Guid",
table: "Preferences");
migrationBuilder.DropColumn(
name: "Permission_Permissions_Guid",
table: "Permissions");
migrationBuilder.AlterColumn<Guid>(
name: "UserId",
table: "Preferences",
type: "TEXT",
nullable: false,
defaultValue: new Guid("00000000-0000-0000-0000-000000000000"),
oldClrType: typeof(Guid),
oldType: "TEXT",
oldNullable: true);
migrationBuilder.AlterColumn<Guid>(
name: "UserId",
table: "Permissions",
type: "TEXT",
nullable: false,
defaultValue: new Guid("00000000-0000-0000-0000-000000000000"),
oldClrType: typeof(Guid),
oldType: "TEXT",
oldNullable: true);
migrationBuilder.CreateIndex(
name: "IX_Preferences_UserId_Kind",
table: "Preferences",
columns: ["UserId", "Kind"],
unique: true);
migrationBuilder.CreateIndex(
name: "IX_Permissions_UserId_Kind",
table: "Permissions",
columns: ["UserId", "Kind"],
unique: true);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropIndex(
name: "IX_Preferences_UserId_Kind",
table: "Preferences");
migrationBuilder.DropIndex(
name: "IX_Permissions_UserId_Kind",
table: "Permissions");
migrationBuilder.AlterColumn<Guid>(
name: "UserId",
table: "Preferences",
type: "TEXT",
nullable: true,
oldClrType: typeof(Guid),
oldType: "TEXT");
migrationBuilder.AddColumn<Guid>(
name: "Preference_Preferences_Guid",
table: "Preferences",
type: "TEXT",
nullable: true);
migrationBuilder.AlterColumn<Guid>(
name: "UserId",
table: "Permissions",
type: "TEXT",
nullable: true,
oldClrType: typeof(Guid),
oldType: "TEXT");
migrationBuilder.AddColumn<Guid>(
name: "Permission_Permissions_Guid",
table: "Permissions",
type: "TEXT",
nullable: true);
migrationBuilder.CreateIndex(
name: "IX_Preferences_UserId_Kind",
table: "Preferences",
columns: ["UserId", "Kind"],
unique: true,
filter: "[UserId] IS NOT NULL");
migrationBuilder.CreateIndex(
name: "IX_Permissions_UserId_Kind",
table: "Permissions",
columns: ["UserId", "Kind"],
unique: true,
filter: "[UserId] IS NOT NULL");
}
}
}

24
src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/JellyfinDbModelSnapshot.cs
View file

@ -15,7 +15,7 @@ namespace Jellyfin.Server.Implementations.Migrations
protected override void BuildModel(ModelBuilder modelBuilder)
{
#pragma warning disable 612, 618
modelBuilder.HasAnnotation("ProductVersion", "10.0.10");
modelBuilder.HasAnnotation("ProductVersion", "10.0.11");
modelBuilder.Entity("Jellyfin.Database.Implementations.Entities.AccessSchedule", b =>
{
@ -1078,14 +1078,11 @@ namespace Jellyfin.Server.Implementations.Migrations
b.Property<int>("Kind")
.HasColumnType("INTEGER");
b.Property<Guid?>("Permission_Permissions_Guid")
.HasColumnType("TEXT");
b.Property<uint>("RowVersion")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.Property<Guid?>("UserId")
b.Property<Guid>("UserId")
.HasColumnType("TEXT");
b.Property<bool>("Value")
@ -1094,8 +1091,7 @@ namespace Jellyfin.Server.Implementations.Migrations
b.HasKey("Id");
b.HasIndex("UserId", "Kind")
.IsUnique()
.HasFilter("[UserId] IS NOT NULL");
.IsUnique();
b.ToTable("Permissions");
@ -1111,14 +1107,11 @@ namespace Jellyfin.Server.Implementations.Migrations
b.Property<int>("Kind")
.HasColumnType("INTEGER");
b.Property<Guid?>("Preference_Preferences_Guid")
.HasColumnType("TEXT");
b.Property<uint>("RowVersion")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.Property<Guid?>("UserId")
b.Property<Guid>("UserId")
.HasColumnType("TEXT");
b.Property<string>("Value")
@ -1129,8 +1122,7 @@ namespace Jellyfin.Server.Implementations.Migrations
b.HasKey("Id");
b.HasIndex("UserId", "Kind")
.IsUnique()
.HasFilter("[UserId] IS NOT NULL");
.IsUnique();
b.ToTable("Preferences");
@ -1699,7 +1691,8 @@ namespace Jellyfin.Server.Implementations.Migrations
b.HasOne("Jellyfin.Database.Implementations.Entities.User", null)
.WithMany("Permissions")
.HasForeignKey("UserId")
.OnDelete(DeleteBehavior.Cascade);
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
});
modelBuilder.Entity("Jellyfin.Database.Implementations.Entities.Preference", b =>
@ -1707,7 +1700,8 @@ namespace Jellyfin.Server.Implementations.Migrations
b.HasOne("Jellyfin.Database.Implementations.Entities.User", null)
.WithMany("Preferences")
.HasForeignKey("UserId")
.OnDelete(DeleteBehavior.Cascade);
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
});
modelBuilder.Entity("Jellyfin.Database.Implementations.Entities.Security.Device", b =>

23
tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerProfileImageTests.cs
View file

@ -1,6 +1,5 @@
using System;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Jellyfin.Database.Implementations;
@ -92,28 +91,6 @@ namespace Jellyfin.Server.Implementations.Tests.Users
new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
}
[Fact]
public async Task UpdateUserAsync_DoesNotLeaveOrphanedPermissionsOrPreferences()
{
var user = await _userManager.CreateUserAsync("updateduser");
var permissionCount = user.Permissions.Count;
var preferenceCount = user.Preferences.Count;
user.LastActivityDate = DateTime.UtcNow;
await _userManager.UpdateUserAsync(user);
await _userManager.UpdateUserAsync(user);
await using var context = CreateDbContext();
Assert.Empty(await context.Permissions
.Where(permission => !permission.UserId.HasValue)
.ToListAsync(TestContext.Current.CancellationToken));
Assert.Empty(await context.Preferences
.Where(preference => !preference.UserId.HasValue)
.ToListAsync(TestContext.Current.CancellationToken));
Assert.Equal(permissionCount, await context.Permissions.CountAsync(TestContext.Current.CancellationToken));
Assert.Equal(preferenceCount, await context.Preferences.CountAsync(TestContext.Current.CancellationToken));
}
[Fact]
public async Task ClearProfileImageAsync_WhenInMemoryImageHasTemporaryKey_RemovesPersistedImage()
{

181
tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerUpdateUserTests.cs Normal file
View file

@ -0,0 +1,181 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Jellyfin.Data;
using Jellyfin.Database.Implementations;
using Jellyfin.Database.Implementations.Enums;
using Jellyfin.Database.Implementations.Locking;
using Jellyfin.Database.Providers.Sqlite;
using Jellyfin.Server.Implementations.Users;
using MediaBrowser.Common;
using MediaBrowser.Common.Net;
using MediaBrowser.Controller;
using MediaBrowser.Controller.Configuration;
using MediaBrowser.Controller.Drawing;
using MediaBrowser.Controller.Events;
using MediaBrowser.Model.Cryptography;
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging.Abstractions;
using Moq;
using Xunit;
namespace Jellyfin.Server.Implementations.Tests.Users;
public sealed class UserManagerUpdateUserTests : IDisposable
{
private readonly SqliteConnection _connection;
private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
private readonly UserManager _userManager;
public UserManagerUpdateUserTests()
{
_connection = new SqliteConnection("Data Source=:memory:");
_connection.Open();
_dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
.UseSqlite(_connection)
.Options;
// Create the schema
using var ctx = CreateDbContext();
ctx.Database.EnsureCreated();
var factory = new Mock<IDbContextFactory<JellyfinDbContext>>();
factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext);
factory.Setup(f => f.CreateDbContextAsync(It.IsAny<CancellationToken>()))
.ReturnsAsync(CreateDbContext);
var cryptoProvider = new Mock<ICryptoProvider>();
var configManager = new Mock<IServerConfigurationManager>();
var appPaths = new Mock<IServerApplicationPaths>();
appPaths.Setup(x => x.ProgramDataPath).Returns(Path.GetTempPath());
configManager.Setup(x => x.ApplicationPaths).Returns(appPaths.Object);
var appHost = new Mock<IApplicationHost>();
var defaultAuthProvider = new DefaultAuthenticationProvider(
NullLogger<DefaultAuthenticationProvider>.Instance,
cryptoProvider.Object);
var invalidAuthProvider = new InvalidAuthProvider();
var defaultPasswordResetProvider = new DefaultPasswordResetProvider(
configManager.Object,
appHost.Object);
_userManager = new UserManager(
factory.Object,
new NoopEventManager(),
new Mock<INetworkManager>().Object,
appHost.Object,
new Mock<IImageProcessor>().Object,
NullLogger<UserManager>.Instance,
configManager.Object,
[defaultPasswordResetProvider],
[defaultAuthProvider, invalidAuthProvider]);
}
public void Dispose()
{
_userManager.Dispose();
_connection.Dispose();
}
[Fact]
public async Task UpdateUserAsync_DoesNotDetachPermissionsOrPreferences()
{
var user = await _userManager.CreateUserAsync("orphanuser");
var permissionCount = user.Permissions.Count;
var preferenceCount = user.Preferences.Count;
user.LastActivityDate = DateTime.UtcNow;
await _userManager.UpdateUserAsync(user);
await _userManager.UpdateUserAsync(user);
await using var context = CreateDbContext();
Assert.Equal(permissionCount, await context.Permissions.CountAsync(TestContext.Current.CancellationToken));
Assert.Equal(preferenceCount, await context.Preferences.CountAsync(TestContext.Current.CancellationToken));
Assert.All(
await context.Permissions.ToListAsync(TestContext.Current.CancellationToken),
permission => Assert.Equal(user.Id, permission.UserId));
Assert.All(
await context.Preferences.ToListAsync(TestContext.Current.CancellationToken),
preference => Assert.Equal(user.Id, preference.UserId));
}
[Fact]
public async Task UpdateUserAsync_WhenOnlyTheUserRowChanged_LeavesChildRowsUntouched()
{
var user = await _userManager.CreateUserAsync("churnuser");
var before = await ReadChildRowsAsync();
// A session activity stamp goes through the same path. It must not rewrite all 37 child
// rows, which is what tearing the collections down and rebuilding them used to do.
user.LastActivityDate = DateTime.UtcNow;
await _userManager.UpdateUserAsync(user);
Assert.Equal(before, await ReadChildRowsAsync());
}
[Fact]
public async Task UpdateUserAsync_AppliesPermissionAndPreferenceChanges()
{
var user = await _userManager.CreateUserAsync("policyuser");
Assert.False(user.HasPermission(PermissionKind.IsAdministrator));
user.SetPermission(PermissionKind.IsAdministrator, true);
user.SetPreference(PreferenceKind.BlockedTags, ["spoilers"]);
user.Permissions.Remove(user.Permissions.First(permission => permission.Kind == PermissionKind.EnableAllChannels));
await _userManager.UpdateUserAsync(user);
var reloaded = _userManager.GetUserById(user.Id)!;
Assert.True(reloaded.HasPermission(PermissionKind.IsAdministrator));
Assert.Equal(new[] { "spoilers" }, reloaded.GetPreference(PreferenceKind.BlockedTags));
Assert.DoesNotContain(reloaded.Permissions, permission => permission.Kind == PermissionKind.EnableAllChannels);
await using var context = CreateDbContext();
Assert.Equal(reloaded.Permissions.Count, await context.Permissions.CountAsync(TestContext.Current.CancellationToken));
}
private JellyfinDbContext CreateDbContext()
{
return new JellyfinDbContext(
_dbOptions,
NullLogger<JellyfinDbContext>.Instance,
new SqliteDatabaseProvider(null!, NullLogger<SqliteDatabaseProvider>.Instance),
new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
}
/// <summary>
/// Reads the identity and concurrency token of every permission and preference row.
/// </summary>
private async Task<List<(string Table, int Id, int Kind, uint RowVersion)>> ReadChildRowsAsync()
{
await using var context = CreateDbContext();
var permissions = await context.Permissions
.OrderBy(permission => permission.Id)
.Select(permission => new ValueTuple<string, int, int, uint>("Permission", permission.Id, (int)permission.Kind, permission.RowVersion))
.ToListAsync(TestContext.Current.CancellationToken);
var preferences = await context.Preferences
.OrderBy(preference => preference.Id)
.Select(preference => new ValueTuple<string, int, int, uint>("Preference", preference.Id, (int)preference.Kind, preference.RowVersion))
.ToListAsync(TestContext.Current.CancellationToken);
return permissions.Concat(preferences).ToList();
}
private sealed class NoopEventManager : IEventManager
{
public void Publish<T>(T eventArgs)
where T : EventArgs
{
}
public Task PublishAsync<T>(T eventArgs)
where T : EventArgs
=> Task.CompletedTask;
}
}