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.

Sign up for GitHub

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

Jump to bottom

Obsolete ILocalClock.LocalNowAsync #16752

Merged
merged 6 commits into from
Sep 18, 2024
Merged

Obsolete ILocalClock.LocalNowAsync #16752

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
df7d62e
Obsolete ILocalClock.LocalNowAsync
hishamco Sep 18, 2024
44cb418
Merge branch 'main' into hishamco/local-clock
hishamco Sep 18, 2024
811ebf6
Update src/OrchardCore/OrchardCore.Abstractions/Modules/Services/ILoc…
hishamco Sep 18, 2024
1dd10ec
Merge branch 'main' into hishamco/local-clock
hishamco Sep 18, 2024
0a1a8da
Address feedback
hishamco Sep 18, 2024
5e8c8f3
Fix build
MikeAlhayek Sep 18, 2024
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
2 changes: 1 addition & 1 deletion ...ardCore.Modules/OrchardCore.AuditTrail/Services/DefaultAuditTrailAdminListQueryService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public async Task<AuditTrailEventQueryResult> QueryAsync(int page, int pageSize,
}
}

var localNow = await _localClock.LocalNowAsync;
var localNow = await _localClock.GetLocalNowAsync();
var startOfWeek = CultureInfo.CurrentUICulture.DateTimeFormat.FirstDayOfWeek;

options.AuditTrailDates =
Expand Down
2 changes: 1 addition & 1 deletion src/OrchardCore.Modules/OrchardCore.Liquid/Filters/LocalTimeZoneFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public async ValueTask<FluidValue> ProcessAsync(FluidValue input, FilterArgument

if (stringValue == "now" || stringValue == "today")
{
value = await _localClock.LocalNowAsync;
value = await _localClock.GetLocalNowAsync();
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/OrchardCore.Modules/OrchardCore.Liquid/Filters/UtcTimeZoneFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public async ValueTask<FluidValue> ProcessAsync(FluidValue input, FilterArgument

if (stringValue == "now" || stringValue == "today")
{
value = await _localClock.LocalNowAsync;
value = await _localClock.GetLocalNowAsync();
}
else
{
Expand Down
6 changes: 5 additions & 1 deletion src/OrchardCore/OrchardCore.Abstractions/Modules/Services/ILocalClock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@ namespace OrchardCore.Modules;
/// </summary>
public interface ILocalClock
{
[Obsolete("This property has been deprecated, please use GetLocalNowAsync() instead.", error: true)]
hishamco marked this conversation as resolved.
Show resolved Hide resolved
hishamco marked this conversation as resolved.
Show resolved Hide resolved
Task<DateTimeOffset> LocalNowAsync => GetLocalNowAsync();

/// <summary>
/// Gets the time for the local time zone.
/// </summary>
Task<DateTimeOffset> LocalNowAsync { get; }

Task<DateTimeOffset> GetLocalNowAsync();
Copy link
Member

@MikeAlhayek MikeAlhayek Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a breaking change. To avoid having a breaking change so this instead

Task<DateTimeOffset> LocalNowAsync { get; }

Task<DateTimeOffset> GetLocalNowAsync()
    => LocalNowAsync ;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it's a breaking change? the value still comes from the new implementation, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for when you have an existing alternative implementation. Should be rare but can happen: Then, with Mike's code, no change is needed, only when we remove LocalNowAsync.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I used error before to let the compiler complains whenever we use it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I used error before to let the compiler complains whenever we use it

this will break people. So we have to avoid it. Doing my suggestion then no breaking change because in the interface we provide the default implementation. In your implementation, you can change the default behavior found in the interface.


/// <summary>
/// Returns the local time zone.
Expand Down
2 changes: 1 addition & 1 deletion src/OrchardCore/OrchardCore.DisplayManagement.Liquid/LiquidViewTemplate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ internal static async Task EnterScopeAsync(this LiquidTemplateContext context, V
}

// Configure Fluid with the local date and time
var now = await localClock.LocalNowAsync;
var now = await localClock.GetLocalNowAsync();

context.Now = () => now;

Expand Down
14 changes: 2 additions & 12 deletions src/OrchardCore/OrchardCore/Modules/Services/LocalClock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,8 @@ public LocalClock(IEnumerable<ITimeZoneSelector> timeZoneSelectors, IClock clock
_calendarManager = calendarManager;
}

public Task<DateTimeOffset> LocalNowAsync
{
get
{
return GetLocalNowAsync();
}
}

private async Task<DateTimeOffset> GetLocalNowAsync()
{
return _clock.ConvertToTimeZone(_clock.UtcNow, await GetLocalTimeZoneAsync());
}
public async Task<DateTimeOffset> GetLocalNowAsync()
=> _clock.ConvertToTimeZone(_clock.UtcNow, await GetLocalTimeZoneAsync());

// Caching the result per request.
public async Task<ITimeZone> GetLocalTimeZoneAsync() => _timeZone ??= await LoadLocalTimeZoneAsync();
Expand Down