Use java.time.Instant instead of java.text.SimpleDateFormat

[gferon]

📋 Changes

After spending quite some time trying to understand why our access tokens would not be refreshed, I realized that something was wrong with the expiresAt field. The issue affects (at least) Android because the UTC timezone is added in the format while the formatter itself is timezone-unaware.

With the following added print statements, you can observe that the Credentials.expiresAt is formatted from local time with an appended Z, which means we lose the timezone information.

I/System.out(18918): RenewApiRequestHandler::onSuccess credentials.expiresAt Tue Jul 23 08:49:05 GMT+02:00 2024
I/System.out(18918): RenewApiRequestHandler::onSuccess formattedDate 2024-07-23T08:49:05.555Z

Making sure all SimpleDateFormat are set to the UTC timezone fixes the issue, but since java.util.Date is notably confusing to work with, a better fix is to switch to java.time.Instant and use Instant.toString() as well as Instant.parse() which work with ISO-8601 formatting.

🎯 Testing

Making sure the tests still use SimpleDateFormat is a great way to avoid having the actual implementation have the same bug than the tests. Unit tests should be good enough in that case.

[gferon]

Could anybody in the team review #469 or #468 depending on the preferred change? We're running a patched version of auth0_flutter and I would be happy if this fix could be merged.

[pmathew92]

Please remove all println statements from the codebase

[pmathew92]

@gferon Thank you for raising this PR . the team will have a look into this

[gferon]

Please remove all println statements from the codebase

Sure, I mostly wanted to first know which approach you favor.

EDIT: done.

[desusai7]

Hi @gferon

Thank you for raising the PR, I've reviewed it and it looks good to me, Can you remove the println statements and also update the tests accordingly ?

[desusai7]

@gferon, thanks a lot for addressing the comment, can you please update the unit tests as well to use java.time.Instant ?

[gferon]

@gferon, thanks a lot for addressing the comment, can you please update the unit tests as well to use java.time.Instant ?

I updated the PR description. I think it's actually good to keep the old way of doing it in the tests, to avoid testing a bug itself (which was happening before).

[gferon]

Hi @pmathew92 could you maybe at least approve the workflow runs so I can fix any CI issues before we move forward? The bug is still out there and it's not a nice one.

[pmathew92]

@gferon Apologies . I have approved the workflow runs. Looks like the merge is blocked due to not being signed commits.Could you please sign your commits

[tikal]

Hi, any update on this?

[gferon]

Could you please sign your commits

@pmathew92 I have signed my commits, not sure why it's a requirement in the first place.

[pmathew92]

@gferon We have merged the PR . Thank you for your contributions