Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the User TimeZoneService #16942

Merged
merged 3 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
=> ValueTask.FromResult<ITimeZone>(null);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Identity;
using OrchardCore.Modules;

namespace OrchardCore.Users.TimeZone.Services;
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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))
{
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}