The following is from our gemini code review gem

Code Review

This pull request introduces a much-needed caching layer to the UrlJwkProvider, which is a great improvement for performance. However, there are some critical concurrency issues in the implementation that need to be addressed.

Summary

Top three areas for improvement:

1. Fix Concurrency: The primary concern is the incorrect implementation of the double-checked locking pattern for the initial cache load, which can lead to bugs and performance degradation under load.
2. Optimize Cache Refresh: The current refresh-on-miss strategy can cause a "thundering herd" problem. This should be made more robust to prevent redundant network calls.
3. Refactor for Readability: The get method contains duplicated logic for finding a key. Refactoring this into a helper method would significantly improve the code's maintainability and clarity.

Confidence Rating: High

I am highly confident in this assessment. The identified issues, particularly the concurrency problems, are based on well-established Java programming patterns.

@jimmyjames

Thank you for the brief summary, I have addressed the changes.

1. A/C to this, cache-control headers for jwk_url return a default maxAge of 15s. We can take this as the default value of a configurable JWKs_refresh_interval for de-duplicating N/W calls.
2. Why are we coupling local findKey with a network call on the same thread? Write operations are done on shared cache, why not delegate network calls to a dedicated network thread(pool), that uses a single-flight pattern to get data and update cache periodically.

The Single-Flight pattern is a concurrency design pattern that ensures only one execution of a function is in-flight for a given key at any given time

This can prevent deduplication of requests SDK-side and using JWKs_refresh_interval will ensure that rate limits are not hit.

@tusharpandey13

Thank you for adding your inputs !!
The Single-Flight pattern is for preventing the "thundering herd" problem, especially on cache misses.
But the library's design already implements this pattern, and it's handled by a separate, higher-level class.

The UrlJwkProvider class, is a simple low-level provider that performs a synchronous, blocking network call. It's intentionally kept minimal without any caching or concurrency logic. This design allows it to be a flexible building block.

The single-flight pattern and thread management you've described are handled by the GuavaCachedJwkProvider class, which is a decorator that wraps the UrlJwkProvider. This is a common design pattern where a simple provider is decorated with powerful cross-cutting concerns like caching and concurrency.