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

feat: ensure Bearer serialization round trip is possible #48

Closed
wants to merge 1 commit into from

Conversation

Lesny
Copy link

@Lesny Lesny commented Sep 28, 2023

Fixes #47

No breaking changes intended.
Tokens serialized by previous versions upon deserialization should be corrupted in exactly same way as before. (didn't test this).

@kyrias
Copy link

kyrias commented Feb 15, 2024

No breaking changes intended.

Adding a new member to a struct where all existing members are pub and the struct isn't marked with non_exhaustive is a breaking change nonetheless. I don't think that's necessarily a problem, but it would require a breaking version bump.

It does seem a bit strange to me though to have have both received_at and expires be DateTimes and then adding or subtracting them to go from one struct to the other. I feel like it would maybe make more sense to only have a single struct with the new received_at, but then replace expires with the raw expires_in and adding a method that returns the actual calculated expiration DateTime.

@@ -109,4 +110,21 @@ mod tests {
assert_eq!(None, bearer.refresh_token);
assert_eq!(None, bearer.expires);
}

#[test]
fn round_trip() {
Copy link
Owner

@kilork kilork Feb 15, 2024

Choose a reason for hiding this comment

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

I do not plan to merge the PR, but to mention: this test would work even if Bearer had no fields.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=973b03e123b710f87e0bc438c7c40775

@kilork kilork closed this Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bearer’s expires field gets scrambled after serialize → deserialize round-trip
3 participants