http: funnel cache key generation through HttpSM - #13557
Conversation
The four cache key call sites each open-coded the same `Cache::generate_key` call with the same two `txn_conf` arguments, and one of them also open-coded the ATS 9.2 compatibility branch. Collect that into a single `HttpSM::generate_cache_key` so callers stop repeating the configuration plumbing.
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication in the HTTP state machine by centralizing cache key generation logic in HttpSM, including the ATS 9.2 compatibility-key branch, so cache-related call sites don’t repeat the same configuration plumbing.
Changes:
- Added
HttpSM::generate_cache_key()to encapsulateCache::generate_key()vsCache::generate_key92()selection. - Replaced open-coded cache key generation in cache lookup/delete/write paths with calls to the new helper.
- Preserved the existing compatibility-key retry behavior by passing the 9.2 selection as a boolean at the call site.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/proxy/http/HttpSM.cc | Introduces the helper and updates cache lookup/delete/write code paths to call it. |
| include/proxy/http/HttpSM.h | Declares the new generate_cache_key() helper on HttpSM. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| void do_drain_request_body(HTTPHdr &response); | ||
|
|
||
| void wait_for_full_body(); | ||
| void generate_cache_key(HttpCacheKey *key, URL *url, bool compat = false); |
There was a problem hiding this comment.
This is an interesting refactor! The new method is has private access, as it should. What do you think of putting the 9.x compatibility case into a separate method? Note that this is the same approach used for the Cache methods (there are two methods, not one method with a boolean switch).
The four cache key call sites each open-coded the same
Cache::generate_keycall with the same twotxn_confarguments, and one of them also open-coded the ATS 9.2 compatibility branch. Collect that into a singleHttpSM::generate_cache_keyso callers stop repeating the configuration plumbing.