Skip to content

Conversation

jakedoublev
Copy link
Contributor

Proposed Changes

  • Current implementation allows one type per cache client
  • Cannot genericize cache.Set easily as Go does not support method generics
  • Allows consumers to get/set any type and do their own assertion on retrieved cache hits

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

@github-actions github-actions bot added external-contributor External Org Member size/s labels Jun 17, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @jakedoublev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request modifies the cache implementation to remove generics, enabling the storage of any type in the cache. This change simplifies the cache's internal logic and shifts the burden of type assertion to the consumer, providing greater flexibility in how the cache is used.

Highlights

  • Generics Removal: Removed generics from cache.Manager and cache.Cache to allow storing any type in the cache.
  • Type Assertion Responsibility: Shifted the responsibility of type assertion to the consumer when retrieving values from the cache.
  • Updated Cache Usage: Updated all instances where the cache manager and client are instantiated to remove the generic type.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


The cache, once strict, Now holds all, no type conflict. Assertion's the key, For what it will be, Flexibility's architect.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the cache manager and client to remove generics, opting to use any for stored values. This change addresses limitations with Go's support for generic methods and shifts type assertion responsibility to the consumer. The code modifications are consistent across the affected files. The primary recommendations are to update code comments to accurately reflect the new non-generic nature and behavior of the cache components, particularly how values are retrieved and type-asserted by the caller.

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 505.830586ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 325.691889ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 343.874573ms
Throughput 290.80 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.401831327s
Average Latency 363.103051ms
Throughput 137.36 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.830796443s
Average Latency 257.286644ms
Throughput 193.57 requests/second

@jakedoublev jakedoublev marked this pull request as ready for review June 17, 2025 21:34
@jakedoublev jakedoublev requested a review from a team as a code owner June 17, 2025 21:34
Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 532.170533ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 328.622971ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 336.904826ms
Throughput 296.82 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 35.89629303s
Average Latency 357.086466ms
Throughput 139.29 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.406689276s
Average Latency 252.762087ms
Throughput 196.80 requests/second

@jakedoublev jakedoublev added this pull request to the merge queue Jun 17, 2025
Merged via the queue into main with commit 98c3c16 Jun 17, 2025
29 checks passed
@jakedoublev jakedoublev deleted the fix/caching-interface branch June 17, 2025 22:08
github-merge-queue bot pushed a commit that referenced this pull request Jun 24, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.7.0](service/v0.6.0...service/v0.7.0)
(2025-06-24)


### ⚠ BREAKING CHANGES

* **policy:** disable kas grants in favor of key mappings
([#2220](#2220))

### Features

* **authz:** Add caching to keycloak ERS
([#2466](#2466))
([f5b0a06](f5b0a06))
* **authz:** auth svc registered resource GetDecision support
([#2392](#2392))
([5405674](5405674))
* **authz:** authz v2 GetBulkDecision
([#2448](#2448))
([0da3363](0da3363))
* **authz:** cache entitlement policy within authorization service
([#2457](#2457))
([c16361c](c16361c))
* **authz:** ensure logging parity between authz v2 and v1
([#2443](#2443))
([ef68586](ef68586))
* **core:** add cache manager
([#2449](#2449))
([2b062c5](2b062c5))
* **core:** consume RPC interceptor request context metadata in logging
([#2442](#2442))
([2769c48](2769c48))
* **core:** DSPX-609 - add cli-client to keycloak provisioning
([#2396](#2396))
([48e7489](48e7489))
* **core:** ERS cache setup, fix cache initialization
([#2458](#2458))
([d0c6938](d0c6938))
* inject logger and cache manager to key managers
([#2461](#2461))
([9292162](9292162))
* **kas:** expose provider config from key details.
([#2459](#2459))
([0e7d39a](0e7d39a))
* **main:** Add Close() method to cache manager
([#2465](#2465))
([32630d6](32630d6))
* **policy:** disable kas grants in favor of key mappings
([#2220](#2220))
([30f8cf5](30f8cf5))
* **policy:** Restrict deletion of pc with used key.
([#2414](#2414))
([3b40a46](3b40a46))
* **sdk:** allow Connect-Protocol-Version RPC header for cors
([#2437](#2437))
([4bf241e](4bf241e))


### Bug Fixes

* **core:** remove generics on new platform cache manager and client
([#2456](#2456))
([98c3c16](98c3c16))
* **core:** replace opentdf-public client with cli-client
([#2422](#2422))
([fb18525](fb18525))
* **deps:** bump github.com/casbin/casbin/v2 from 2.106.0 to 2.107.0 in
/service in the external group
([#2416](#2416))
([43afd48](43afd48))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.4.0 to
0.5.0 in /service
([#2470](#2470))
([3a73fc9](3a73fc9))
* **deps:** bump github.com/opentdf/platform/sdk from 0.4.7 to 0.5.0 in
/service ([#2473](#2473))
([ad37476](ad37476))
* **deps:** bump the external group across 1 directory with 2 updates
([#2450](#2450))
([9d8d1f1](9d8d1f1))
* **deps:** bump the external group across 1 directory with 2 updates
([#2472](#2472))
([d45b3c8](d45b3c8))
* only request a token when near expiration
([#2370](#2370))
([556d95e](556d95e))
* **policy:** fix casing bug and get provider config on update.
([#2403](#2403))
([a52b8f9](a52b8f9))
* **policy:** properly formatted pem in test fixtures
([#2409](#2409))
([54ffd23](54ffd23))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants