Skip to content

Comments

Added support for redis expire keys#7

Merged
katorres02 merged 6 commits intomainfrom
add-support-for-redis-expire-time
Aug 6, 2025
Merged

Added support for redis expire keys#7
katorres02 merged 6 commits intomainfrom
add-support-for-redis-expire-time

Conversation

@katorres02
Copy link
Contributor

@katorres02 katorres02 commented Aug 6, 2025

CINT Oauth2 authentication flow does not use a refresh_token logic - instead they expect the clients to request a new authorization token when is expired.

This was validated with the CINT official documentation and the response from the authorization endpoint.

For this particular case we need to add expiration support for tokens stored using this library.
- Added expires_in argument to the constructor - This is an optional value
- Added default value to expires_in - default value nil
- The expires time will be only added to the key if the argument is present - otherwise it will use the default expiration time
- This change does not break previous implementations

⚠️ After some more thoughts we decided to use a different approach for refreshing the token for CINT
What is left in this PR are the gem updates and a couple rubocop fixes

… it does not affect compatibility with other implementations of the client class
@katorres02 katorres02 requested review from a team, VitorGultzgoff, jeromepl, loto, ozhilin, srapilly and ventilooo and removed request for a team August 6, 2025 15:55
Copy link

@loto loto left a comment

Choose a reason for hiding this comment

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

Good job 👍

Comment on lines 32 to 33
value = lock!(&block)
redis.set(key, value)
store_value!(value)
Copy link

Choose a reason for hiding this comment

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

Isn't it weird that the redis.set call is not inside the lock! block? This feels like a bug of the gem 🤔 It looks like it should be

Suggested change
value = lock!(&block)
redis.set(key, value)
store_value!(value)
lock! do
value = block.call
store_value!(value)
end

Otherwise, I think it would be technically possible that a process that started later than another ends up storing its value in Redis first, so it would be overwritten by the first process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, good catch

Copy link

@VitorGultzgoff VitorGultzgoff left a comment

Choose a reason for hiding this comment

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

Nice!

@katorres02 katorres02 merged commit a72b826 into main Aug 6, 2025
4 checks passed
@katorres02 katorres02 deleted the add-support-for-redis-expire-time branch August 6, 2025 18:32
@katorres02 katorres02 changed the title Added support for redis expire keys ~Added support for redis expire keys~ Aug 6, 2025
@katorres02 katorres02 changed the title ~Added support for redis expire keys~ Added support for redis expire keys Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants