Cache

[bbengfort]

Scope of changes

Implements Ristretto and InMemory Caches

Fixes SC-36557

Type of change

• new feature
• bug fix
• documentation
• testing
• technical debt
• other (describe)

Author checklist

• I have manually tested the change and/or added automation in the form of unit tests or integration tests
• I have added new test fixtures as needed to support added tests
• Check this box if a reviewer can merge this pull request after approval (leave it unchecked if you want to do it yourself)
• I have moved the associated Shortcut story to "Ready for Review"

[Copilot]

Pull request overview

This PR implements cache functionality for httpcache by introducing two cache implementations: an in-memory cache and a Ristretto-backed cache. The PR adds the core Cache interface, cache key generation logic with support for Vary headers, and comprehensive testing and benchmarking for both implementations.

Changes:

• Added Cache interface and cache key generation functions supporting standard keys, header-based keys, and Vary header keys
• Implemented InMemoryCache using a mutex-protected map for simple in-memory caching
• Implemented Ristretto cache wrapper with configuration options for high-performance concurrent caching

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
cache.go	Defines Cache interface and implements cache key generation functions with support for Vary headers
inmem.go	Implements InMemoryCache with thread-safe map-based storage
inmem_test.go	Tests for InMemoryCache including race condition testing
inmem_bench_test.go	Benchmarks for InMemoryCache operations
httpcache.go	Adds normalize function for header value normalization
httpcache_test.go	Tests for normalize function and test helpers
cache_test.go	Tests for cache key generation functions
export_test.go	Exports internal functions for testing
logger.go	Adds logger configuration for httpcache
ristretto/ristretto.go	Implements Ristretto cache wrapper with Cache interface
ristretto/config.go	Configuration struct for Ristretto cache
ristretto/ristretto_test.go	Tests for Ristretto cache including race condition testing
ristretto/ristretto_bench_test.go	Benchmarks for Ristretto cache operations
go.mod	Adds dependencies for testing and Ristretto cache
go.sum	Dependency checksums

Comments suppressed due to low confidence (4)

cache.go:86

• The word "seperation" is misspelled and should be "separation".

// the cached response. This implements RFC 9111 vary seperation. Header values are

inmem.go:6

• The comment states "This cache if volatile" but should be "This cache is volatile".

// map. This cache if volatile and will be cleared when the program exits, but is often

go.mod:3

• Go version 1.25.1 does not exist. The latest stable Go version as of January 2025 is 1.23.x. The version should be updated to a valid Go release version such as 1.23.0 or the actual intended version.

go 1.25.1

inmem.go:38

• The InMemoryCache.Del method doesn't check if the store map is nil before attempting to delete. While Go's delete function handles nil maps gracefully without panicking, this creates an inconsistency with the Put method which explicitly checks and initializes the map. Consider either initializing the map in Del as well for consistency, or documenting that Del is safe to call on an uninitialized cache.

// Del removes the cached response associated with the key.
func (c *InMemoryCache) Del(key string) {
	c.Lock()
	delete(c.store, key)
	c.Unlock()
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test calls cache.Put followed immediately by cache.Get without calling cache.Wait() in between. According to the Wait method documentation in ristretto.go (lines 81-82), "This ensures a call to Put() will be visible to future calls to Get()." The ristretto library uses buffered writes, so without Wait(), the Get on line 22 may not see the Put on line 20. This test may pass due to timing but could be flaky. Consider adding cache.Wait() after line 20.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 11 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The key generation pattern string(rune('a' + i%192)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%192) for numeric keys.

[Copilot]

Function documentation should start with the function name. Change "Create a new Ristretto-backed httpcache.Cache" to "New creates a new Ristretto-backed httpcache.Cache" to follow Go documentation conventions.

Suggested change

	// Create a new Ristretto-backed httpcache.Cache with the specified configuration.
	// New creates a new Ristretto-backed httpcache.Cache with the specified configuration.

[Copilot]

The key generation pattern string(rune('a'+i%192)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%192) for numeric keys.

[Copilot]

The key generation pattern string(rune('a'+i%192)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%192) for numeric keys.

[Copilot]

The key generation pattern string(rune('a' + i%128)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%128) for numeric keys.

[Copilot]

The example code in the package documentation doesn't handle the error returned by New. The example should either check the error or use a comment indicating error handling is omitted for brevity.

[Copilot]

The key generation pattern string(rune('a' + i%128)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%128) for numeric keys.

[Copilot]

The key generation pattern string(rune('a' + i%192)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%192) for numeric keys.

[Copilot]

The key generation pattern string(rune('a' + i%128)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%128) for numeric keys.

[Copilot]

The key generation pattern string(rune('a' + i%128)) will produce non-letter characters when the modulo exceeds 25 (e.g., when i=100, this gives rune 197 which is 'Å'). If the intent is to generate letter-based keys, consider using string(rune('a' + i%26)) to cycle through lowercase letters, or use a clearer pattern like fmt.Sprintf("key-%d", i%128) for numeric keys.