oracle / ojdbc-extensions

The Oracle JDBC Driver Extensions include providers for centralized configuration or token providers for authentication with the DB.
Universal Permissive License v1.0
13 stars 5 forks source link

Smooth Cache Updates for Access Token Providers #59 issue #87

Closed ayouballali closed 2 months ago

ayouballali commented 4 months ago

Use the Oracle JDBC Driver implementation of caching JWTs, instead of the standard CachedResourceFactory .This cache is designed for "smooth" updates when a token expires. this cache uses a background thread to begin requesting a new token at one minute before the current token expires. If the request completes within 30 seconds (which is almost always the case), then threads will not become blocked. Unlike CachedResourceFactory will block the thread while it requests a new token.

This Pr contain also test to verify if that thread will be blocked or not , and verify how much it would take to request a token after it expires .

issue : #59

note : I aded some new dependencies for testing and jwt generating :Mockito,JJWT

oracle-contributor-agreement[bot] commented 4 months ago

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

ayouballali commented 4 months ago

Now we are relying on CachedResourceFactory<Supplier> to cache token supplier per each parameter set instead of caching the token itself . so the request() method of AccessTokenFactory now returns Supplier , and the driver's cache will handle caching the tokens .

I added tests to make sure that this mechanism is thread safe also to verify the uniqueness of tokens per parameter set .

Michael-A-McMahon commented 4 months ago

Awesome: Looks like the Supplier-as-a-resource design is working out.

mockito and the jwt lib need to go. I'll bet that with just a little bit of code, you can solve the same problems that you're using these libraries to solve. This would be preferable because a few lines of code is much easier to understand than the API and source code of these libraries. I don't have much experience working with these libraries, and I don't know if other programmers on this project do either. When you introduce a new library into a code base, you are forcing everyone else who works on that code to also have learn how that library works. If the only benefit of the library is you not having to write a bit of code, then this doesn't feel like a good trade off :)

In any case, we need to replace mock-ups with integration tests. That is, we need tests that actually request tokens from Azure and OCI. Our test suite is already designed to do this, so you can look to existing tests for examples. We really favor integration testing over mock-ups because at the end of the day, our users need this code to work correctly with cloud services. Mock classes won't provide that level of verification. I think you can hit all the cases you're wanting to test without needing to mock a token request.

jeandelavarene commented 3 months ago

Please add a documentation note in the "Access Token Provider" section of both OCI and Azure and explain how tokens are cached. You can provide a link to the javadoc: https://docs.oracle.com/en//database/oracle/oracle-database/23/jajdb/oracle/jdbc/AccessToken.html#createJsonWebTokenCache(java.util.function.Supplier)

Michael-A-McMahon commented 2 months ago

The latest commit reworks the solution a bit: