Skip to content

Commit e140d32

Browse files
alecthomasclaude
andauthored
feat(cache): support conditional requests on Cache.Open and Stat (#344)
## Summary Unifies conditional-request handling into a single option set shared by the client wire protocol, the cache backends, and the server handlers. Previously `If-Match`/`If-None-Match` were only evaluated at the HTTP layer (`httputil.ServeCacheHit`), and `Cache.Open`/`Stat` had no way to express preconditions — so the `Remote` tier couldn't forward them upstream and backends always read the body before a 304/412 could be decided. This pushes precondition evaluation down into the cache, establishing the option plumbing for forthcoming **Range / If-Range** support. ## Changes - **`client`**: replace the `http.Request`-based `RequestOption` with an inspectable `RequestOptions` struct (`IfMatch`/`IfNoneMatch`) plus a `Check(etag)` evaluator, so non-HTTP backends can evaluate preconditions locally. The matching logic (`etagListMatches`) moves here from `httputil`. - **`internal/cache`**: `Cache.Open`/`Stat` accept variadic `Option` args (an idiomatic alias of `client.RequestOption`). `Disk`/`Memory`/`S3` short-circuit with `ErrNotModified`/`ErrPreconditionFailed` *before* reading the body; `Remote` forwards opts to upstream (zero translation — same type); `Tiered` treats the sentinels as definitive (no backfill, headers preserved for 304). - **`internal/httputil`**: `ServeCacheHit`/`ServeCacheStat` now serve the `Open`/`Stat` outcome, mapping sentinels to 304/412; `CheckConditionals` delegates to the unified `client` logic (still used by the git snapshot path). - **handlers**: `apiv1` and the generic caching handler parse request conditionals and push them into `Open`/`Stat`. ### Note on S3 S3's native conditional options (`SetMatchETag` etc.) match against S3's object ETag, but cachew advertises a SHA256 **content-hash** ETag stored in the companion `.meta` object — a different value. So preconditions are evaluated locally against the stored ETag rather than delegated to S3. ## Tests - Reworked `httputil/conditional_test.go` for the outcome-based serve helpers. - Added a `Conditional` case to the shared cache suite, exercising `If-Match`/`If-None-Match` on `Open` and `Stat` across all backends (including `Remote` end-to-end). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8aa8bf6 commit e140d32

17 files changed

Lines changed: 349 additions & 138 deletions

File tree

.claude/rules/base.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ DO NOT ASSUME I AM RIGHT, VERIFY WHAT I ASSERT
77
- When working on a list of tasks in a README.md bullet list `- [ ] ...`, pick the next incomplete one and implement that, mark it as complete, then stop.
88
- If you are in read-only "Ask" mode, and are asked to modify something, immediately abort saying you can't modify anything.
99
- Do exactly what I ask, no more. Don't add extra scripts, documentation, etc.
10-
- Always run tests to verify correctness.
1110
- Always write tests for updated/new code.
1211
- Be succinct.
1312
- Don't write comments if the related code itself is simple.

.claude/rules/go.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
- Where it makes sense, update existing test rather than creating new ones.
1717
- ALWAYS run tests with `-timeout 30s` to ensure that wedged tests don't last forever.
1818
- Don't run tests with `-v` in general, as it produces a large amount of output.
19-
- Once the change is complete and working, run `golangci-lint run` and fix any linter errors introduced before adding the files to git. Do NOT EVER run `golangci-lint` on individual files.
2019
- For "unparam" linter warnings about "XXX is unused", remove the parameter unless the type is part of an interface implementation or callback system.
2120
- ALWAYS respect encapsulation of struct fields, even between types in the same package.
2221
- ALWAYS apply the Go proverb "align the happy path to the left", to avoid deep nesting.

client/client.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,7 @@ func (c *Client) Open(ctx context.Context, key Key, opts ...RequestOption) (io.R
166166
if err != nil {
167167
return nil, nil, errors.Wrap(err, "failed to create request")
168168
}
169-
for _, opt := range opts {
170-
opt(req)
171-
}
169+
NewRequestOptions(opts...).applyToRequest(req)
172170

173171
resp, err := c.http.Do(req)
174172
if err != nil {
@@ -204,9 +202,7 @@ func (c *Client) Stat(ctx context.Context, key Key, opts ...RequestOption) (http
204202
if err != nil {
205203
return nil, errors.Wrap(err, "failed to create request")
206204
}
207-
for _, opt := range opts {
208-
opt(req)
209-
}
205+
NewRequestOptions(opts...).applyToRequest(req)
210206

211207
resp, err := c.http.Do(req)
212208
if err != nil {

client/preconditions.go

Lines changed: 68 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,86 @@ package client
22

33
import (
44
"net/http"
5+
"strings"
56

67
"github.com/alecthomas/errors"
78
)
89

9-
// ErrNotModified is returned when the server responds with 304 Not Modified,
10-
// indicating the resource has not changed since the ETag in If-None-Match.
10+
// ErrNotModified is returned when an If-None-Match precondition is satisfied,
11+
// indicating the resource has not changed since the supplied ETag. Over HTTP
12+
// this corresponds to 304 Not Modified.
1113
var ErrNotModified = errors.New("not modified")
1214

13-
// ErrPreconditionFailed is returned when the server responds with 412
14-
// Precondition Failed, indicating an If-Match or If-None-Match condition was not met.
15+
// ErrPreconditionFailed is returned when an If-Match precondition is not met.
16+
// Over HTTP this corresponds to 412 Precondition Failed.
1517
var ErrPreconditionFailed = errors.New("precondition failed")
1618

17-
// RequestOption configures conditional headers on an outgoing cache request.
18-
type RequestOption func(req *http.Request)
19+
// RequestOptions holds conditional-request parameters. It is the single
20+
// representation shared by the client wire protocol, the cache backends, and
21+
// the server handlers.
22+
type RequestOptions struct {
23+
// IfMatch is the If-Match precondition. Evaluation fails with
24+
// ErrPreconditionFailed if the stored ETag does not match.
25+
IfMatch string
26+
// IfNoneMatch is the If-None-Match precondition. Evaluation reports
27+
// ErrNotModified when the stored ETag matches.
28+
IfNoneMatch string
29+
}
30+
31+
// RequestOption configures conditional request parameters.
32+
type RequestOption func(*RequestOptions)
1933

20-
// IfMatch sets the If-Match header. The server will return 412 Precondition
21-
// Failed if the stored ETag does not match.
34+
// IfMatch sets the If-Match precondition.
2235
func IfMatch(etag string) RequestOption {
23-
return func(req *http.Request) {
24-
req.Header.Set("If-Match", etag)
25-
}
36+
return func(o *RequestOptions) { o.IfMatch = etag }
2637
}
2738

28-
// IfNoneMatch sets the If-None-Match header. For GET/HEAD the server returns
29-
// 304 Not Modified when the ETag matches; for other methods it returns 412.
39+
// IfNoneMatch sets the If-None-Match precondition.
3040
func IfNoneMatch(etag string) RequestOption {
31-
return func(req *http.Request) {
32-
req.Header.Set("If-None-Match", etag)
41+
return func(o *RequestOptions) { o.IfNoneMatch = etag }
42+
}
43+
44+
// NewRequestOptions applies opts and returns the resulting RequestOptions.
45+
func NewRequestOptions(opts ...RequestOption) RequestOptions {
46+
var o RequestOptions
47+
for _, opt := range opts {
48+
opt(&o)
49+
}
50+
return o
51+
}
52+
53+
// Check evaluates the preconditions against the stored ETag. It returns
54+
// ErrNotModified for a satisfied If-None-Match, ErrPreconditionFailed for a
55+
// failed If-Match, or nil when all preconditions pass.
56+
func (o RequestOptions) Check(etag string) error {
57+
if o.IfMatch != "" && (etag == "" || !etagListMatches(o.IfMatch, etag)) {
58+
return ErrPreconditionFailed
59+
}
60+
if o.IfNoneMatch != "" && etag != "" && etagListMatches(o.IfNoneMatch, etag) {
61+
return ErrNotModified
62+
}
63+
return nil
64+
}
65+
66+
// applyToRequest sets the conditional headers on an outgoing request.
67+
func (o RequestOptions) applyToRequest(req *http.Request) {
68+
if o.IfMatch != "" {
69+
req.Header.Set("If-Match", o.IfMatch)
70+
}
71+
if o.IfNoneMatch != "" {
72+
req.Header.Set("If-None-Match", o.IfNoneMatch)
73+
}
74+
}
75+
76+
// etagListMatches reports whether etag matches an If-Match / If-None-Match
77+
// header value, which may be a comma-separated list of ETags or the "*"
78+
// wildcard. Stored ETags are always strong, so weak comparison is not required.
79+
func etagListMatches(headerValue, etag string) bool {
80+
for candidate := range strings.SplitSeq(headerValue, ",") {
81+
candidate = strings.TrimSpace(candidate)
82+
if candidate == "*" || candidate == etag {
83+
return true
84+
}
3385
}
86+
return false
3487
}

internal/cache/api.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,25 @@ type Writer = client.CacheWriter
3131
// ErrNotFound is returned when a cache backend is not found.
3232
var ErrNotFound = errors.New("cache backend not found")
3333

34+
// Option configures conditional parameters on a cache Open or Stat.
35+
type Option = client.RequestOption
36+
37+
// IfMatch sets the If-Match precondition. Open/Stat return ErrPreconditionFailed
38+
// if the stored ETag does not match.
39+
func IfMatch(etag string) Option { return client.IfMatch(etag) }
40+
41+
// IfNoneMatch sets the If-None-Match precondition. Open/Stat return
42+
// ErrNotModified when the stored ETag matches.
43+
func IfNoneMatch(etag string) Option { return client.IfNoneMatch(etag) }
44+
45+
// ErrNotModified is returned by Open/Stat when an If-None-Match precondition is
46+
// satisfied.
47+
var ErrNotModified = client.ErrNotModified
48+
49+
// ErrPreconditionFailed is returned by Open/Stat when an If-Match precondition
50+
// is not met.
51+
var ErrPreconditionFailed = client.ErrPreconditionFailed
52+
3453
// ErrStatsUnavailable is returned when a cache backend cannot provide statistics.
3554
var ErrStatsUnavailable = client.ErrStatsUnavailable
3655

@@ -133,13 +152,21 @@ type Cache interface {
133152
//
134153
// Expired files MUST not be returned.
135154
// Must return os.ErrNotExist if the file does not exist.
136-
Stat(ctx context.Context, key Key) (http.Header, error)
155+
//
156+
// Conditional opts are evaluated against the stored ETag: a satisfied
157+
// If-None-Match returns ErrNotModified (with headers); a failed If-Match
158+
// returns ErrPreconditionFailed.
159+
Stat(ctx context.Context, key Key, opts ...Option) (http.Header, error)
137160
// Open an existing file in the cache.
138161
//
139162
// Expired files MUST NOT be returned.
140163
// The returned headers MUST include a Last-Modified header.
141164
// Must return os.ErrNotExist if the file does not exist.
142-
Open(ctx context.Context, key Key) (io.ReadCloser, http.Header, error)
165+
//
166+
// Conditional opts are evaluated against the stored ETag: a satisfied
167+
// If-None-Match returns ErrNotModified (with headers, no body); a failed
168+
// If-Match returns ErrPreconditionFailed.
169+
Open(ctx context.Context, key Key, opts ...Option) (io.ReadCloser, http.Header, error)
143170
// Create a new file in the cache.
144171
//
145172
// If "ttl" is zero, a maximum TTL MUST be used by the implementation.

internal/cache/cachetest/suite.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ func Suite(t *testing.T, newCache func(t *testing.T) cache.Cache) {
7979
t.Run("ETag", func(t *testing.T) {
8080
testETag(t, newCache(t))
8181
})
82+
83+
t.Run("Conditional", func(t *testing.T) {
84+
testConditional(t, newCache(t))
85+
})
8286
}
8387

8488
func testCreateAndOpen(t *testing.T, c cache.Cache) {
@@ -485,6 +489,61 @@ func testETag(t *testing.T, c cache.Cache) {
485489
assert.Equal(t, expectedETag, statHeaders.Get("ETag"))
486490
}
487491

492+
// testConditional verifies that Open and Stat honour If-Match / If-None-Match
493+
// preconditions against the stored ETag, returning the unified sentinel errors.
494+
func testConditional(t *testing.T, c cache.Cache) {
495+
defer c.Close()
496+
ctx := t.Context()
497+
498+
content := []byte("conditional content")
499+
key := cache.NewKey("test-conditional")
500+
501+
w, err := c.Create(ctx, key, nil, time.Hour)
502+
assert.NoError(t, err)
503+
_, err = w.Write(content)
504+
assert.NoError(t, err)
505+
assert.NoError(t, w.Close())
506+
507+
sum := sha256.Sum256(content)
508+
etag := `"` + hex.EncodeToString(sum[:]) + `"`
509+
510+
t.Run("IfNoneMatchHitReturnsNotModified", func(t *testing.T) {
511+
_, headers, err := c.Open(ctx, key, cache.IfNoneMatch(etag))
512+
assert.IsError(t, err, cache.ErrNotModified)
513+
assert.Equal(t, etag, headers.Get("ETag")) // headers surfaced for the 304
514+
515+
headers, err = c.Stat(ctx, key, cache.IfNoneMatch(etag))
516+
assert.IsError(t, err, cache.ErrNotModified)
517+
assert.Equal(t, etag, headers.Get("ETag"))
518+
})
519+
520+
t.Run("IfNoneMatchMissServesBody", func(t *testing.T) {
521+
reader, _, err := c.Open(ctx, key, cache.IfNoneMatch(`"other"`))
522+
assert.NoError(t, err)
523+
defer reader.Close()
524+
data, err := io.ReadAll(reader)
525+
assert.NoError(t, err)
526+
assert.Equal(t, content, data)
527+
})
528+
529+
t.Run("IfMatchHitServesBody", func(t *testing.T) {
530+
reader, _, err := c.Open(ctx, key, cache.IfMatch(etag))
531+
assert.NoError(t, err)
532+
defer reader.Close()
533+
data, err := io.ReadAll(reader)
534+
assert.NoError(t, err)
535+
assert.Equal(t, content, data)
536+
})
537+
538+
t.Run("IfMatchMissReturnsPreconditionFailed", func(t *testing.T) {
539+
_, _, err := c.Open(ctx, key, cache.IfMatch(`"other"`))
540+
assert.IsError(t, err, cache.ErrPreconditionFailed)
541+
542+
_, err = c.Stat(ctx, key, cache.IfMatch(`"other"`))
543+
assert.IsError(t, err, cache.ErrPreconditionFailed)
544+
})
545+
}
546+
488547
func testNamespaceDelete(t *testing.T, c cache.Cache) {
489548
defer c.Close()
490549
ctx := t.Context()

internal/cache/conditional.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package cache
2+
3+
import (
4+
"net/http"
5+
6+
"github.com/alecthomas/errors"
7+
8+
"github.com/block/cachew/client"
9+
)
10+
11+
// conditionalShortCircuit evaluates conditional opts against the stored ETag in
12+
// headers. A nil error means the object should be served normally. A non-nil
13+
// error short-circuits the request: ErrNotModified is returned together with
14+
// headers (so callers can surface a 304 with the stored validators), while
15+
// ErrPreconditionFailed is returned with nil headers.
16+
func conditionalShortCircuit(headers http.Header, opts []Option) (http.Header, error) {
17+
err := errors.WithStack(client.NewRequestOptions(opts...).Check(headers.Get(ETagKey)))
18+
if errors.Is(err, ErrNotModified) {
19+
return headers, err
20+
}
21+
return nil, err
22+
}

internal/cache/disk.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func (d *Disk) Delete(_ context.Context, key Key) error {
232232
return nil
233233
}
234234

235-
func (d *Disk) Stat(ctx context.Context, key Key) (http.Header, error) {
235+
func (d *Disk) Stat(ctx context.Context, key Key, opts ...Option) (http.Header, error) {
236236
path := d.keyToPath(d.namespace, key)
237237
fullPath := filepath.Join(d.config.Root, path)
238238

@@ -256,10 +256,13 @@ func (d *Disk) Stat(ctx context.Context, key Key) (http.Header, error) {
256256
}
257257

258258
headers.Set("Content-Length", strconv.FormatInt(info.Size(), 10))
259+
if h, err := conditionalShortCircuit(headers, opts); err != nil {
260+
return h, err
261+
}
259262
return headers, nil
260263
}
261264

262-
func (d *Disk) Open(ctx context.Context, key Key) (io.ReadCloser, http.Header, error) {
265+
func (d *Disk) Open(ctx context.Context, key Key, opts ...Option) (io.ReadCloser, http.Header, error) {
263266
path := d.keyToPath(d.namespace, key)
264267
fullPath := filepath.Join(d.config.Root, path)
265268

@@ -297,6 +300,10 @@ func (d *Disk) Open(ctx context.Context, key Key) (io.ReadCloser, http.Header, e
297300
return nil, nil, errors.Join(errors.Errorf("failed to update expiration time: %w", err), f.Close())
298301
}
299302

303+
if h, condErr := conditionalShortCircuit(headers, opts); condErr != nil {
304+
return nil, h, errors.Join(condErr, f.Close())
305+
}
306+
300307
return f, headers, nil
301308
}
302309

internal/cache/memory.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func NewMemory(ctx context.Context, config MemoryConfig) (*Memory, error) {
5858

5959
func (m *Memory) String() string { return fmt.Sprintf("memory:%dMB", m.config.LimitMB) }
6060

61-
func (m *Memory) Stat(_ context.Context, key Key) (http.Header, error) {
61+
func (m *Memory) Stat(_ context.Context, key Key, opts ...Option) (http.Header, error) {
6262
m.mu.RLock()
6363
defer m.mu.RUnlock()
6464

@@ -78,10 +78,13 @@ func (m *Memory) Stat(_ context.Context, key Key) (http.Header, error) {
7878

7979
headers := maps.Clone(entry.headers)
8080
headers.Set("Content-Length", strconv.Itoa(len(entry.data)))
81+
if h, err := conditionalShortCircuit(headers, opts); err != nil {
82+
return h, err
83+
}
8184
return headers, nil
8285
}
8386

84-
func (m *Memory) Open(_ context.Context, key Key) (io.ReadCloser, http.Header, error) {
87+
func (m *Memory) Open(_ context.Context, key Key, opts ...Option) (io.ReadCloser, http.Header, error) {
8588
m.mu.RLock()
8689
defer m.mu.RUnlock()
8790

@@ -101,6 +104,9 @@ func (m *Memory) Open(_ context.Context, key Key) (io.ReadCloser, http.Header, e
101104

102105
headers := maps.Clone(entry.headers)
103106
headers.Set("Content-Length", strconv.Itoa(len(entry.data)))
107+
if h, err := conditionalShortCircuit(headers, opts); err != nil {
108+
return nil, h, err
109+
}
104110
return io.NopCloser(bytes.NewReader(entry.data)), headers, nil
105111
}
106112

internal/cache/noop.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ func NoOpCache() Cache {
2222

2323
func (n *noOpCache) String() string { return "noop" }
2424

25-
func (n *noOpCache) Stat(_ context.Context, _ Key) (http.Header, error) {
25+
func (n *noOpCache) Stat(_ context.Context, _ Key, _ ...Option) (http.Header, error) {
2626
return nil, os.ErrNotExist
2727
}
2828

29-
func (n *noOpCache) Open(_ context.Context, _ Key) (io.ReadCloser, http.Header, error) {
29+
func (n *noOpCache) Open(_ context.Context, _ Key, _ ...Option) (io.ReadCloser, http.Header, error) {
3030
return nil, nil, os.ErrNotExist
3131
}
3232

0 commit comments

Comments
 (0)