Skip to content

remove cache from other authenticated endpoints#5528

Merged
codeEmpress1 merged 2 commits intocanonical:mainfrom
codeEmpress1:redis-cache-cleanup
Dec 12, 2025
Merged

remove cache from other authenticated endpoints#5528
codeEmpress1 merged 2 commits intocanonical:mainfrom
codeEmpress1:redis-cache-cleanup

Conversation

@codeEmpress1
Copy link
Copy Markdown
Contributor

@codeEmpress1 codeEmpress1 commented Dec 11, 2025

Done

  • Remove cache from other authenticated endpoints

How to QA

Testing

  • This PR has tests
  • No testing required (explain why):

Issue / Card

Fixes #

Screenshots

Copilot AI review requested due to automatic review settings December 11, 2025 12:13
@webteam-app
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes Redis-based server-side caching from authenticated API endpoints to ensure users always receive fresh, user-specific data. The change affects CVE helper methods, validation sets endpoints, and signing keys endpoints, along with removing cache setup from related test files.

Key Changes:

  • Removed Redis cache operations from CVE-related methods (get_revisions_with_cves, get_cve_with_revision)
  • Removed Redis cache from validation sets GET endpoints
  • Removed Redis cache from signing keys GET, POST, and DELETE endpoints
  • Cleaned up unused cache utility functions and test setup code

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
webapp/publisher/cve/cve_helper.py Removed cache get/set operations from CVE revision and file content retrieval methods
webapp/endpoints/validation_sets.py Removed cache layer from validation sets retrieval endpoints; now directly fetches from dashboard API
webapp/endpoints/signing_keys.py Removed cache operations from signing keys GET, CREATE, and DELETE endpoints; removed cache import
webapp/endpoints/models.py Removed unused cache key helper functions (get_models_cache_key, get_policies_cache_key)
tests/publisher/cve/test_has_cve.py Removed cache setup/teardown from test setUp method
tests/endpoints/test_cve_get_by_revision.py Removed cache clearing from test setUp method
tests/endpoints/cve/test_has_cve_api.py Removed cache clearing from test setUp method

Review Notes:

The changes are consistent and thorough in removing Redis caching from authenticated endpoints. All cache imports have been properly removed, cache get/set/delete operations have been eliminated, and test setup code has been cleaned up appropriately.

However, I notice some areas that could be improved:

  1. QA Steps: The PR description's QA steps are minimal and could be more comprehensive. They should include:

    • Steps to test validation sets endpoints (not just signing keys)
    • Steps to test CVE-related functionality
    • Verification that the data is no longer cached (e.g., making changes and immediately seeing them reflected)
    • Testing both success and error scenarios
  2. Unused Code: The helper function get_signing_keys_cache_key in webapp/endpoints/signing_keys.py (line 22-23) is now unused after cache removal but was not removed in this PR. While this is outside the diff regions and thus I cannot comment on it directly, it would be good to remove it in a follow-up.

  3. Testing: The PR is marked as "No testing required" but given that caching behavior is being changed, it would be valuable to verify through manual QA that:

    • All affected endpoints still function correctly
    • Data freshness is maintained (no stale data issues)
    • Performance is acceptable without caching

The implementation itself is clean and correctly removes all cache-related code without introducing bugs. The pattern of removing caching from authenticated endpoints while keeping it for public endpoints makes sense from a security and data freshness perspective.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (b8b6b55) to head (b59a60f).
⚠️ Report is 598 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #5528       +/-   ##
==========================================
- Coverage   66.80%       0   -66.81%     
==========================================
  Files         113       0      -113     
  Lines        3714       0     -3714     
  Branches      965       0      -965     
==========================================
- Hits         2481       0     -2481     
+ Misses       1098       0     -1098     
+ Partials      135       0      -135     

see 62 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@steverydz steverydz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@codeEmpress1 codeEmpress1 merged commit 7d6e99f into canonical:main Dec 12, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants