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

Too many ISystemClock definitions #16844

Closed
YohDeadfall opened this issue Nov 5, 2019 · 3 comments
Closed

Too many ISystemClock definitions #16844

YohDeadfall opened this issue Nov 5, 2019 · 3 comments
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform
Milestone

Comments

@YohDeadfall
Copy link

The ISystemClock defines a way to get the current time in UTC timezone, and it has a simple implementation:

public interface ISystemClock
{
    DateTimeOffset UtcNow { get; }
}

public interface SystemClock : ISystemClock
{
    public DateTimeOffset UtcNow => DateTimeOffset.UtcNow;
}

The interface exists in:

  • Microsoft.AspNetCore.Authentication
  • Microsoft.AspNetCore.ResponseCaching
  • Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
  • Microsoft.Extensions.Internal

There is a small difference exists between implementations, but in most cases it can be moved out. In case of Kestrel's clock which has a specific logic inside, there could be made a new interface IScopedSystemClock which will provide the scope start time as UtcNow does now. Therefore, looks like all of them could be merged into a single class/interface and put into Microsoft.Extensions.Primitives.

@mkArtakMSFT mkArtakMSFT added the feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform label Nov 5, 2019
@JunTaoLuo JunTaoLuo added this to the Next sprint planning milestone Sep 29, 2020
@ghost
Copy link

ghost commented Sep 29, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@JunTaoLuo JunTaoLuo added the clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors label Sep 29, 2020
@JunTaoLuo
Copy link
Contributor

ISystemClock seems to be used elsewhere in our codebase as well so the list might be incomplete. Instead of putting this Extensions.Primitives, we should also consider having it as a shared source file.

@JunTaoLuo JunTaoLuo added area-servers area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer labels Oct 1, 2020
@BrennanConroy
Copy link
Member

We aren't planning to do anything here.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 25, 2020
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform
Projects
None yet
Development

No branches or pull requests

5 participants