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

Use fixed GMT time-zone for WebSessionManager Clock [SPR-15675] #20234

Closed
spring-issuemaster opened this issue Jun 16, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jun 16, 2017

Rob Winch opened SPR-15675 and commented

Currently WebSession API uses Instant for many methods. It might be good to use a type that contains timezone information it rather than the implied timezone of GMT


Affects: 5.0 RC2

Issue Links:

  • #20220 Consider using ZonedDateTime in HttpHeaders

Referenced from: commits ba3a12e

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 22, 2017

Rossen Stoyanchev commented

In the very least we need to fix the following line which uses Clock.systemDefaultZone but should use a fixed timezone (GMT) so that session times are always relative to the same time zone. Beyond that I am not convinced that we need to expose anything more than Instant. It is perfectly comparable to any other "externally created moment". Why does it need to be ZonedDateTime rather than Instant? All that matters is the time relative to session creation.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 22, 2017

Sébastien Deleuze commented

I tend to agree since the usage of ZonedDateTime in HttpHeaders is mainly driven by the fact that the time zone information is directly available in the header value we parse. Not sure that applies here, so I would be for just setting GMT fixed time zone as proposed by Rossen. Would that be ok from your point of view Rob Winch?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 26, 2017

Sébastien Deleuze commented

As discussed with Rob Winch after today's meeting, I turn this issue into using a fixed GMT time-zone with existing Instant type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.