Fix the User TimeZoneService (#16942)

src/OrchardCore.Modules/OrchardCore.Users/TimeZone/Drivers/UserTimeZoneDisplayDriver.cs

@@ -3,20 +3,12 @@
 using OrchardCore.DisplayManagement.Views;
 using OrchardCore.Users.Models;
 using OrchardCore.Users.TimeZone.Models;
-using OrchardCore.Users.TimeZone.Services;
 using OrchardCore.Users.TimeZone.ViewModels;
 
 namespace OrchardCore.Users.TimeZone.Drivers;
 
 public sealed class UserTimeZoneDisplayDriver : SectionDisplayDriver<User, UserTimeZone>
 {
-    private readonly IUserTimeZoneService _userTimeZoneService;
-
-    public UserTimeZoneDisplayDriver(IUserTimeZoneService userTimeZoneService)
-    {
-        _userTimeZoneService = userTimeZoneService;
-    }
-
     public override IDisplayResult Edit(User user, UserTimeZone userTimeZone, BuildEditorContext context)
     {
         return Initialize<UserTimeZoneViewModel>("UserTimeZone_Edit", model =>
@@ -30,10 +22,8 @@ public override async Task<IDisplayResult> UpdateAsync(User user, UserTimeZone u
         var model = new UserTimeZoneViewModel();
 
         await context.Updater.TryUpdateModelAsync(model, Prefix);
-        userTimeZone.TimeZoneId = model.TimeZoneId;
 
-        // Remove the cache entry, don't update it, as the form might still fail validation for other reasons.
-        await _userTimeZoneService.UpdateAsync(user);
+        userTimeZone.TimeZoneId = model.TimeZoneId;
 
         return await EditAsync(user, userTimeZone, context);
     }

src/OrchardCore.Modules/OrchardCore.Users/TimeZone/Services/IUserTimeZoneService.cs

@@ -11,11 +11,20 @@ public interface IUserTimeZoneService
     /// Gets the time zone for the given user.
     /// </summary>
     /// <param name="user">The <see cref="IUser"/>.</param>
+    [Obsolete("This method is deprecated and will be removed in a future release. Please use GetAsync(userName) instead..")]
     ValueTask<ITimeZone> GetAsync(IUser user);
 
     /// <summary>
     /// Updates the time zone for the given user.
     /// </summary>
     /// <param name="user">The <see cref="IUser"/>.</param>
+    [Obsolete("This method is obsolete and will be removed in a future release. Cache invalidation is now handled automatically.")]
     ValueTask UpdateAsync(IUser user);
+
+    /// <summary>
+    /// Gets the time zone for the given user.
+    /// </summary>
+    /// <param name="userName">The user name.</param>
+    ValueTask<ITimeZone> GetAsync(string userName)
+        => ValueTask.FromResult<ITimeZone>(null);
 }

src/OrchardCore.Modules/OrchardCore.Users/TimeZone/Services/UserTimeZoneSelector.cs

@@ -1,5 +1,4 @@
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Identity;
 using OrchardCore.Modules;
 
 namespace OrchardCore.Users.TimeZone.Services;
@@ -10,29 +9,31 @@ namespace OrchardCore.Users.TimeZone.Services;
 public class UserTimeZoneSelector : ITimeZoneSelector
 {
     private readonly IUserTimeZoneService _userTimeZoneService;
-    private readonly UserManager<IUser> _userManager;
     private readonly IHttpContextAccessor _httpContextAccessor;
 
     public UserTimeZoneSelector(
         IUserTimeZoneService userTimeZoneService,
-        UserManager<IUser> userManager,
         IHttpContextAccessor httpContextAccessor)
     {
         _userTimeZoneService = userTimeZoneService;
-        _userManager = userManager;
         _httpContextAccessor = httpContextAccessor;
     }
 
-    public async Task<TimeZoneSelectorResult> GetTimeZoneAsync()
+    public Task<TimeZoneSelectorResult> GetTimeZoneAsync()
     {
-        var currentUser = await _userManager.GetUserAsync(_httpContextAccessor.HttpContext.User);
-
-        return currentUser == null
+        var result = !_httpContextAccessor.HttpContext.User.Identity.IsAuthenticated
             ? null
             : new TimeZoneSelectorResult
             {
                 Priority = 100,
-                TimeZoneId = async () => (await _userTimeZoneService.GetAsync(currentUser))?.TimeZoneId
+                TimeZoneId = async () =>
+                {
+                    var timeZone = await _userTimeZoneService.GetAsync(_httpContextAccessor.HttpContext.User.Identity.Name);
+
+                    return timeZone?.TimeZoneId;
+                },
             };
+
+        return Task.FromResult(result);
     }
 }

src/OrchardCore.Modules/OrchardCore.Users/TimeZone/Services/UserTimeZoneService.cs

@@ -1,35 +1,43 @@
+using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.Caching.Distributed;
 using OrchardCore.Entities;
 using OrchardCore.Modules;
+using OrchardCore.Users.Handlers;
 using OrchardCore.Users.Models;
 using OrchardCore.Users.TimeZone.Models;
 
 namespace OrchardCore.Users.TimeZone.Services;
 
-public class UserTimeZoneService : IUserTimeZoneService
+public class UserTimeZoneService : UserEventHandlerBase, IUserTimeZoneService
 {
     private const string CacheKey = "UserTimeZone/";
-    private const string EmptyTimeZone = "empty";
+    private const string EmptyTimeZone = "NoTimeZoneFound";
 
-    private static readonly DistributedCacheEntryOptions _slidingExpiration = new() { SlidingExpiration = TimeSpan.FromHours(1) };
+    private static readonly DistributedCacheEntryOptions _slidingExpiration = new()
+    {
+        SlidingExpiration = TimeSpan.FromHours(1),
+    };
 
     private readonly IClock _clock;
     private readonly IDistributedCache _distributedCache;
+    private readonly UserManager<IUser> _userManager;
 
     public UserTimeZoneService(
         IClock clock,
-        IDistributedCache distributedCache)
+        IDistributedCache distributedCache,
+        UserManager<IUser> userManager)
     {
         _clock = clock;
         _distributedCache = distributedCache;
+        _userManager = userManager;
     }
 
     /// <inheritdoc/>
-    public async ValueTask<ITimeZone> GetAsync(IUser user)
+    public async ValueTask<ITimeZone> GetAsync(string userName)
     {
-        ArgumentNullException.ThrowIfNull(user);
+        ArgumentException.ThrowIfNullOrEmpty(userName);
 
-        var currentTimeZoneId = await GetTimeZoneIdAsync(user);
+        var currentTimeZoneId = await GetTimeZoneIdAsync(userName);
 
         if (string.IsNullOrEmpty(currentTimeZoneId))
         {
@@ -39,41 +47,37 @@ public async ValueTask<ITimeZone> GetAsync(IUser user)
         return _clock.GetTimeZone(currentTimeZoneId);
     }
 
-    /// <inheritdoc/>
-    public async ValueTask UpdateAsync(IUser user)
-    {
-        ArgumentNullException.ThrowIfNull(user);
 
-        if (string.IsNullOrEmpty(user.UserName))
-        {
-            return;
-        }
+    /// <inheritdoc/>
+    public ValueTask<ITimeZone> GetAsync(IUser user)
+        => GetAsync(user?.UserName);
 
-        await _distributedCache.RemoveAsync(GetCacheKey(user.UserName));
-    }
+    /// <inheritdoc/>
+    public async ValueTask UpdateAsync(IUser user)
+        => await ForgetCacheAsync(user?.UserName);
 
     /// <inheritdoc/>
-    private async ValueTask<string> GetTimeZoneIdAsync(IUser user)
+    private async ValueTask<string> GetTimeZoneIdAsync(string userName)
     {
-        if (string.IsNullOrEmpty(user.UserName))
-        {
-            return null;
-        }
-
-        var key = GetCacheKey(user.UserName);
+        var key = GetCacheKey(userName);
 
         var timeZoneId = await _distributedCache.GetStringAsync(key);
 
-        // The timezone is not cached yet, resolve it and store the value
+        // The timeZone is not cached yet, resolve it and store the value.
         if (string.IsNullOrEmpty(timeZoneId))
         {
+            // At this point, we know the timeZoneId is not cached for the given userName.
+            // Retrieve the user and cache the timeZoneId.
+            var user = await _userManager.FindByNameAsync(userName);
+
             if (user is User u)
             {
                 timeZoneId = u.As<UserTimeZone>()?.TimeZoneId;
             }
 
-            // We store a special string to remember there is no specific value for this user.
-            // And actual distributed cache implementation might not be able to store null values.
+            // We store a placeholder string to indicate that there is no specific value for this user.
+            // This approach ensures compatibility with distributed cache implementations that may not support null values.
+            // Caching this placeholder helps avoid redundant queries for this user on each request when no time zone is set.
             if (string.IsNullOrEmpty(timeZoneId))
             {
                 timeZoneId = EmptyTimeZone;
@@ -82,7 +86,7 @@ private async ValueTask<string> GetTimeZoneIdAsync(IUser user)
             await _distributedCache.SetStringAsync(key, timeZoneId, _slidingExpiration);
         }
 
-        // Do we know this user doesn't have a configured value?
+        // If TimeZoneId matches the placeholder value, we return null instead of the placeholder itself.
         if (timeZoneId == EmptyTimeZone)
         {
             return null;
@@ -91,5 +95,22 @@ private async ValueTask<string> GetTimeZoneIdAsync(IUser user)
         return timeZoneId;
     }
 
-    private static string GetCacheKey(string userName) => CacheKey + userName;
+    public override Task DeletedAsync(UserDeleteContext context)
+        => ForgetCacheAsync(context.User.UserName);
+
+    public override Task UpdatedAsync(UserUpdateContext context)
+        => ForgetCacheAsync(context.User.UserName);
+
+    public override Task DisabledAsync(UserContext context)
+        => ForgetCacheAsync(context.User.UserName);
+
+    private Task ForgetCacheAsync(string userName)
+    {
+        var key = GetCacheKey(userName);
+
+        return _distributedCache.RemoveAsync(key);
+    }
+
+    private static string GetCacheKey(string userName)
+        => CacheKey + userName;
 }

src/OrchardCore.Modules/OrchardCore.Users/TimeZone/Startup.cs

@@ -1,6 +1,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using OrchardCore.DisplayManagement.Handlers;
 using OrchardCore.Modules;
+using OrchardCore.Users.Handlers;
 using OrchardCore.Users.Models;
 using OrchardCore.Users.TimeZone.Drivers;
 using OrchardCore.Users.TimeZone.Services;
@@ -13,7 +14,9 @@ public sealed class Startup : StartupBase
     public override void ConfigureServices(IServiceCollection services)
     {
         services.AddScoped<ITimeZoneSelector, UserTimeZoneSelector>();
-        services.AddScoped<IUserTimeZoneService, UserTimeZoneService>();
+        services.AddScoped<UserTimeZoneService>();
+        services.AddScoped<IUserTimeZoneService>(sp => sp.GetRequiredService<UserTimeZoneService>());
+        services.AddScoped<IUserEventHandler>(sp => sp.GetRequiredService<UserTimeZoneService>());
         services.AddScoped<IDisplayDriver<User>, UserTimeZoneDisplayDriver>();
     }
 }

src/OrchardCore/OrchardCore.Users.Abstractions/Handlers/UserEventHandlerBase.cs

@@ -3,29 +3,38 @@ namespace OrchardCore.Users.Handlers;
 public abstract class UserEventHandlerBase : IUserEventHandler
 {
     /// <inheritdocs />
-    public virtual Task CreatingAsync(UserCreateContext context) => Task.CompletedTask;
+    public virtual Task CreatingAsync(UserCreateContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task CreatedAsync(UserCreateContext context) => Task.CompletedTask;
+    public virtual Task CreatedAsync(UserCreateContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task DeletingAsync(UserDeleteContext context) => Task.CompletedTask;
+    public virtual Task DeletingAsync(UserDeleteContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task DeletedAsync(UserDeleteContext context) => Task.CompletedTask;
+    public virtual Task DeletedAsync(UserDeleteContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task UpdatingAsync(UserUpdateContext context) => Task.CompletedTask;
+    public virtual Task UpdatingAsync(UserUpdateContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task UpdatedAsync(UserUpdateContext context) => Task.CompletedTask;
+    public virtual Task UpdatedAsync(UserUpdateContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task DisabledAsync(UserContext context) => Task.CompletedTask;
+    public virtual Task DisabledAsync(UserContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task EnabledAsync(UserContext context) => Task.CompletedTask;
+    public virtual Task EnabledAsync(UserContext context)
+        => Task.CompletedTask;
 
     /// <inheritdocs />
-    public virtual Task ConfirmedAsync(UserConfirmContext context) => Task.CompletedTask;
+    public virtual Task ConfirmedAsync(UserConfirmContext context)
+        => Task.CompletedTask;
 }