Refactored all UserManager db access methods

Fixed stale cached entities in UserManager
Fixed wrong state persisting though method calls
This commit is contained in:
JPVenson
2026-04-30 15:42:46 +00:00
parent b37ebec5f6
commit 2ac0edc052
7 changed files with 195 additions and 113 deletions

View File

@@ -1,7 +1,7 @@
#pragma warning disable CA1307
#pragma warning disable RS0030 // Do not use banned APIs
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
@@ -52,7 +52,6 @@ namespace Jellyfin.Server.Implementations.Users
private readonly DefaultPasswordResetProvider _defaultPasswordResetProvider;
private readonly IServerConfigurationManager _serverConfigurationManager;
private readonly IDictionary<Guid, User> _users;
private readonly AsyncKeyedLocker<Guid> _userLock = new();
/// <summary>
@@ -92,30 +91,28 @@ namespace Jellyfin.Server.Implementations.Users
_invalidAuthProvider = _authenticationProviders.OfType<InvalidAuthProvider>().First();
_defaultAuthenticationProvider = _authenticationProviders.OfType<DefaultAuthenticationProvider>().First();
_defaultPasswordResetProvider = _passwordResetProviders.OfType<DefaultPasswordResetProvider>().First();
_users = new ConcurrentDictionary<Guid, User>();
using var dbContext = _dbProvider.CreateDbContext();
foreach (var user in dbContext.Users
.AsSplitQuery()
.Include(user => user.Permissions)
.Include(user => user.Preferences)
.Include(user => user.AccessSchedules)
.Include(user => user.ProfileImage)
.AsNoTracking()
.AsEnumerable())
{
_users.Add(user.Id, user);
}
}
/// <inheritdoc/>
public event EventHandler<GenericEventArgs<User>>? OnUserUpdated;
/// <inheritdoc/>
public IEnumerable<User> Users => _users.Values;
public IEnumerable<User> GetUsers()
{
using var dbContext = _dbProvider.CreateDbContext();
return UserQuery(dbContext)
.ToArray();
}
/// <inheritdoc/>
public IEnumerable<Guid> UsersIds => _users.Keys;
public IEnumerable<Guid> GetUsersIds()
{
using var dbContext = _dbProvider.CreateDbContext();
return dbContext.Users
.AsNoTracking()
.Select(user => user.Id)
.ToArray();
}
// This is some regex that matches only on unicode "word" characters, as well as -, _ and @
// In theory this will cut out most if not all 'control' characters which should help minimize any weirdness
@@ -131,11 +128,28 @@ namespace Jellyfin.Server.Implementations.Users
throw new ArgumentException("Guid can't be empty", nameof(id));
}
using (_userLock.Lock(id))
{
_users.TryGetValue(id, out var user);
return user;
}
using var dbContext = _dbProvider.CreateDbContext();
return UserQuery(dbContext)
.FirstOrDefault(user => user.Id == id);
}
private static IQueryable<User> UserQuery(JellyfinDbContext dbContext)
{
return dbContext.Users
.AsSingleQuery()
.Include(user => user.Permissions)
.Include(user => user.Preferences)
.Include(user => user.AccessSchedules)
.Include(user => user.ProfileImage)
.AsNoTracking();
}
/// <inheritdoc/>
public User? GetFirstUser()
{
using var dbContext = _dbProvider.CreateDbContext();
return UserQuery(dbContext)
.FirstOrDefault();
}
/// <inheritdoc/>
@@ -146,22 +160,29 @@ namespace Jellyfin.Server.Implementations.Users
throw new ArgumentException("Invalid username", nameof(name));
}
return _users.Values.FirstOrDefault(u => string.Equals(u.Username, name, StringComparison.OrdinalIgnoreCase));
using var dbContext = _dbProvider.CreateDbContext();
#pragma warning disable CA1862 // Use the 'StringComparison' method overloads to perform case-insensitive string comparisons
#pragma warning disable CA1311 // Specify a culture or use an invariant version to avoid implicit dependency on current culture
#pragma warning disable CA1304 // The behavior of 'string.ToUpper()' could vary based on the current user's locale settings
return UserQuery(dbContext)
.FirstOrDefault(u => u.Username.ToUpper() == name.ToUpper());
#pragma warning restore CA1304 // The behavior of 'string.ToUpper()' could vary based on the current user's locale settings
#pragma warning restore CA1311 // Specify a culture or use an invariant version to avoid implicit dependency on current culture
#pragma warning restore CA1862 // Use the 'StringComparison' method overloads to perform case-insensitive string comparisons
}
/// <inheritdoc/>
public async Task RenameUser(User user, string newName)
public async Task RenameUser(Guid userId, string oldName, string newName)
{
ArgumentNullException.ThrowIfNull(user);
ThrowIfInvalidUsername(newName);
if (user.Username.Equals(newName, StringComparison.Ordinal))
if (oldName.Equals(newName, StringComparison.Ordinal))
{
throw new ArgumentException("The new and old names must be different.");
}
using (await _userLock.LockAsync(user.Id).ConfigureAwait(false))
User user = null!; // user is never actually null where its used afterwards so we can just ignore.
using (await _userLock.LockAsync(userId).ConfigureAwait(false))
{
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
@@ -170,7 +191,7 @@ namespace Jellyfin.Server.Implementations.Users
#pragma warning disable CA1311 // Specify a culture or use an invariant version to avoid implicit dependency on current culture
#pragma warning disable CA1304 // The behavior of 'string.ToUpper()' could vary based on the current user's locale settings
if (await dbContext.Users
.AnyAsync(u => u.Username.ToUpper() == newName.ToUpper() && !u.Id.Equals(user.Id))
.AnyAsync(u => u.Username.ToUpper() == newName.ToUpper() && u.Id != userId)
.ConfigureAwait(false))
{
throw new ArgumentException(string.Format(
@@ -182,6 +203,16 @@ namespace Jellyfin.Server.Implementations.Users
#pragma warning restore CA1311 // Specify a culture or use an invariant version to avoid implicit dependency on current culture
#pragma warning restore CA1862 // Use the 'StringComparison' method overloads to perform case-insensitive string comparisons
// Load a fresh tracked entity inside the lock so the RowVersion is current.
user = await dbContext.Users
.Include(u => u.Permissions)
.Include(u => u.Preferences)
.Include(u => u.AccessSchedules)
.Include(u => u.ProfileImage)
.FirstOrDefaultAsync(u => u.Id == userId)
.ConfigureAwait(false)
?? throw new ResourceNotFoundException(nameof(userId));
user.Username = newName;
await UpdateUserInternalAsync(dbContext, user).ConfigureAwait(false);
}
@@ -197,11 +228,7 @@ namespace Jellyfin.Server.Implementations.Users
{
using (await _userLock.LockAsync(user.Id).ConfigureAwait(false))
{
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
{
await UpdateUserInternalAsync(dbContext, user).ConfigureAwait(false);
}
await UpdateUserInternalAsync(user).ConfigureAwait(false);
}
}
@@ -231,23 +258,30 @@ namespace Jellyfin.Server.Implementations.Users
{
ThrowIfInvalidUsername(name);
if (Users.Any(u => u.Username.Equals(name, StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture,
"A user with the name '{0}' already exists.",
name));
}
User newUser;
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
{
#pragma warning disable CA1862 // Use the 'StringComparison' method overloads to perform case-insensitive string comparisons
#pragma warning disable CA1311 // Specify a culture or use an invariant version to avoid implicit dependency on current culture
#pragma warning disable CA1304 // The behavior of 'string.ToUpper()' could vary based on the current user's locale settings
if (await dbContext.Users
.AnyAsync(u => u.Username.ToUpper() == name.ToUpper())
.ConfigureAwait(false))
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture,
"A user with the name '{0}' already exists.",
name));
}
#pragma warning restore CA1304 // The behavior of 'string.ToUpper()' could vary based on the current user's locale settings
#pragma warning restore CA1311 // Specify a culture or use an invariant version to avoid implicit dependency on current culture
#pragma warning restore CA1862 // Use the 'StringComparison' method overloads to perform case-insensitive string comparisons
newUser = await CreateUserInternalAsync(name, dbContext).ConfigureAwait(false);
dbContext.Users.Add(newUser);
await dbContext.SaveChangesAsync().ConfigureAwait(false);
_users.Add(newUser.Id, newUser);
}
await _eventManager.PublishAsync(new UserCreatedEventArgs(newUser)).ConfigureAwait(false);
@@ -261,39 +295,44 @@ namespace Jellyfin.Server.Implementations.Users
User? user;
using (await _userLock.LockAsync(userId).ConfigureAwait(false))
{
if (!_users.TryGetValue(userId, out user))
{
throw new ResourceNotFoundException(nameof(userId));
}
if (_users.Count == 1)
{
throw new InvalidOperationException(string.Format(
CultureInfo.InvariantCulture,
"The user '{0}' cannot be deleted because there must be at least one user in the system.",
user.Username));
}
if (user.HasPermission(PermissionKind.IsAdministrator)
&& Users.Count(i => i.HasPermission(PermissionKind.IsAdministrator)) == 1)
{
throw new ArgumentException(
string.Format(
CultureInfo.InvariantCulture,
"The user '{0}' cannot be deleted because there must be at least one admin user in the system.",
user.Username),
nameof(userId));
}
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
{
dbContext.Users.Attach(user);
user = await dbContext.Users
.Include(u => u.Permissions)
.FirstOrDefaultAsync(u => u.Id.Equals(userId))
.ConfigureAwait(false);
if (user is null)
{
throw new ResourceNotFoundException(nameof(userId));
}
var userCount = await dbContext.Users.CountAsync().ConfigureAwait(false);
if (userCount == 1)
{
throw new InvalidOperationException(string.Format(
CultureInfo.InvariantCulture,
"The user '{0}' cannot be deleted because there must be at least one user in the system.",
user.Username));
}
if (user.HasPermission(PermissionKind.IsAdministrator)
&& await dbContext.Users
.CountAsync(i => i.Permissions.Any(p => p.Kind == PermissionKind.IsAdministrator && p.Value))
.ConfigureAwait(false) == 1)
{
throw new ArgumentException(
string.Format(
CultureInfo.InvariantCulture,
"The user '{0}' cannot be deleted because there must be at least one admin user in the system.",
user.Username),
nameof(userId));
}
dbContext.Users.Remove(user);
await dbContext.SaveChangesAsync().ConfigureAwait(false);
}
_users.Remove(userId);
}
await _eventManager.PublishAsync(new UserDeletedEventArgs(user)).ConfigureAwait(false);
@@ -316,8 +355,27 @@ namespace Jellyfin.Server.Implementations.Users
using (await _userLock.LockAsync(user.Id).ConfigureAwait(false))
{
await GetAuthenticationProvider(user).ChangePassword(user, newPassword).ConfigureAwait(false);
await UpdateUserAsync(user).ConfigureAwait(false);
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
{
// Reload the user inside the lock to get the current RowVersion.
// The incoming user may have been loaded with AsNoTracking() before this
// lock was acquired; a concurrent write (e.g. a login updating LastLoginDate)
// could have incremented RowVersion in the database in the meantime.
// Since we now hold the per-user lock, no other write can race with us, so
// the freshly loaded RowVersion is guaranteed to be current.
var dbUser = await dbContext.Users
.Include(u => u.Permissions)
.Include(u => u.Preferences)
.Include(u => u.AccessSchedules)
.Include(u => u.ProfileImage)
.FirstOrDefaultAsync(u => u.Id == user.Id)
.ConfigureAwait(false)
?? throw new ResourceNotFoundException(nameof(user.Id));
await GetAuthenticationProvider(dbUser).ChangePassword(dbUser, newPassword).ConfigureAwait(false);
await dbContext.SaveChangesAsync().ConfigureAwait(false);
}
}
await _eventManager.PublishAsync(new UserPasswordChangedEventArgs(user)).ConfigureAwait(false);
@@ -424,9 +482,17 @@ namespace Jellyfin.Server.Implementations.Users
}
bool success;
var user = Users.FirstOrDefault(i => string.Equals(username, i.Username, StringComparison.OrdinalIgnoreCase));
var user = GetUserByName(username);
using (await _userLock.LockAsync(user?.Id ?? Guid.Empty).ConfigureAwait(false))
{
// Reload the user now that we hold the lock so the RowVersion is current.
// GetUserByName uses AsNoTracking and the snapshot may be stale if another
// write (e.g. a concurrent login) incremented RowVersion after our initial load.
if (user is not null)
{
user = GetUserById(user.Id) ?? user;
}
var authResult = await AuthenticateLocalUser(username, password, user)
.ConfigureAwait(false);
var authenticationProvider = authResult.AuthenticationProvider;
@@ -445,7 +511,7 @@ namespace Jellyfin.Server.Implementations.Users
// Search the database for the user again
// the authentication provider might have created it
user = Users.FirstOrDefault(i => string.Equals(username, i.Username, StringComparison.OrdinalIgnoreCase));
user = GetUserByName(username);
if (authenticationProvider is IHasNewUserPolicy hasNewUserPolicy && user is not null)
{
@@ -461,7 +527,7 @@ namespace Jellyfin.Server.Implementations.Users
if (providerId is not null && !string.Equals(providerId, user.AuthenticationProviderId, StringComparison.OrdinalIgnoreCase))
{
user.AuthenticationProviderId = providerId;
await UpdateUserAsync(user).ConfigureAwait(false);
await UpdateUserInternalAsync(user).ConfigureAwait(false);
}
}
@@ -512,7 +578,7 @@ namespace Jellyfin.Server.Implementations.Users
}
user.InvalidLoginAttemptCount = 0;
await UpdateUserAsync(user).ConfigureAwait(false);
await UpdateUserInternalAsync(user).ConfigureAwait(false);
_logger.LogInformation("Authentication request for {UserName} has succeeded.", user.Username);
}
else
@@ -566,22 +632,22 @@ namespace Jellyfin.Server.Implementations.Users
public async Task InitializeAsync()
{
// TODO: Refactor the startup wizard so that it doesn't require a user to already exist.
if (_users.Any())
{
return;
}
var defaultName = Environment.UserName;
if (string.IsNullOrWhiteSpace(defaultName) || !ValidUsernameRegex().IsMatch(defaultName))
{
defaultName = "MyJellyfinUser";
}
_logger.LogWarning("No users, creating one with username {UserName}", defaultName);
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
{
if (await dbContext.Users.AnyAsync().ConfigureAwait(false))
{
return;
}
var defaultName = Environment.UserName;
if (string.IsNullOrWhiteSpace(defaultName) || !ValidUsernameRegex().IsMatch(defaultName))
{
defaultName = "MyJellyfinUser";
}
_logger.LogWarning("No users, creating one with username {UserName}", defaultName);
var newUser = await CreateUserInternalAsync(defaultName, dbContext).ConfigureAwait(false);
newUser.SetPermission(PermissionKind.IsAdministrator, true);
newUser.SetPermission(PermissionKind.EnableContentDeletion, true);
@@ -589,7 +655,6 @@ namespace Jellyfin.Server.Implementations.Users
dbContext.Users.Add(newUser);
await dbContext.SaveChangesAsync().ConfigureAwait(false);
_users.Add(newUser.Id, newUser);
}
}
@@ -664,7 +729,6 @@ namespace Jellyfin.Server.Implementations.Users
user.SetPreference(PreferenceKind.LatestItemExcludes, config.LatestItemsExcludes);
dbContext.Update(user);
_users[user.Id] = user;
await dbContext.SaveChangesAsync().ConfigureAwait(false);
}
}
@@ -745,7 +809,6 @@ namespace Jellyfin.Server.Implementations.Users
user.SetPreference(PreferenceKind.EnableContentDeletionFromFolders, policy.EnableContentDeletionFromFolders);
dbContext.Update(user);
_users[user.Id] = user;
await dbContext.SaveChangesAsync().ConfigureAwait(false);
}
}
@@ -769,7 +832,6 @@ namespace Jellyfin.Server.Implementations.Users
}
user.ProfileImage = null;
_users[user.Id] = user;
}
}
@@ -916,14 +978,22 @@ namespace Jellyfin.Server.Implementations.Users
user.InvalidLoginAttemptCount);
}
await UpdateUserAsync(user).ConfigureAwait(false);
await UpdateUserInternalAsync(user).ConfigureAwait(false);
}
private async Task UpdateUserInternalAsync(User user)
{
var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false);
await using (dbContext.ConfigureAwait(false))
{
await UpdateUserInternalAsync(dbContext, user).ConfigureAwait(false);
}
}
private async Task UpdateUserInternalAsync(JellyfinDbContext dbContext, User user)
{
dbContext.Users.Attach(user);
dbContext.Entry(user).State = EntityState.Modified;
_users[user.Id] = user;
await dbContext.SaveChangesAsync().ConfigureAwait(false);
}