Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .claude/rules/base.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ DO NOT ASSUME I AM RIGHT, VERIFY WHAT I ASSERT
- 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.
- If you are in read-only "Ask" mode, and are asked to modify something, immediately abort saying you can't modify anything.
- Do exactly what I ask, no more. Don't add extra scripts, documentation, etc.
- Always run tests to verify correctness.
- Always write tests for updated/new code.
- Be succinct.
- Don't write comments if the related code itself is simple.
Expand Down
1 change: 0 additions & 1 deletion .claude/rules/go.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
- Where it makes sense, update existing test rather than creating new ones.
- ALWAYS run tests with `-timeout 30s` to ensure that wedged tests don't last forever.
- Don't run tests with `-v` in general, as it produces a large amount of output.
- 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.
- For "unparam" linter warnings about "XXX is unused", remove the parameter unless the type is part of an interface implementation or callback system.
- ALWAYS respect encapsulation of struct fields, even between types in the same package.
- ALWAYS apply the Go proverb "align the happy path to the left", to avoid deep nesting.
Expand Down
8 changes: 2 additions & 6 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ func (c *Client) Open(ctx context.Context, key Key, opts ...RequestOption) (io.R
if err != nil {
return nil, nil, errors.Wrap(err, "failed to create request")
}
for _, opt := range opts {
opt(req)
}
NewRequestOptions(opts...).applyToRequest(req)

resp, err := c.http.Do(req)
if err != nil {
Expand Down Expand Up @@ -204,9 +202,7 @@ func (c *Client) Stat(ctx context.Context, key Key, opts ...RequestOption) (http
if err != nil {
return nil, errors.Wrap(err, "failed to create request")
}
for _, opt := range opts {
opt(req)
}
NewRequestOptions(opts...).applyToRequest(req)

resp, err := c.http.Do(req)
if err != nil {
Expand Down
83 changes: 68 additions & 15 deletions client/preconditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,86 @@ package client

import (
"net/http"
"strings"

"github.com/alecthomas/errors"
)

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

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

// RequestOption configures conditional headers on an outgoing cache request.
type RequestOption func(req *http.Request)
// RequestOptions holds conditional-request parameters. It is the single
// representation shared by the client wire protocol, the cache backends, and
// the server handlers.
type RequestOptions struct {
// IfMatch is the If-Match precondition. Evaluation fails with
// ErrPreconditionFailed if the stored ETag does not match.
IfMatch string
// IfNoneMatch is the If-None-Match precondition. Evaluation reports
// ErrNotModified when the stored ETag matches.
IfNoneMatch string
}

// RequestOption configures conditional request parameters.
type RequestOption func(*RequestOptions)

// IfMatch sets the If-Match header. The server will return 412 Precondition
// Failed if the stored ETag does not match.
// IfMatch sets the If-Match precondition.
func IfMatch(etag string) RequestOption {
return func(req *http.Request) {
req.Header.Set("If-Match", etag)
}
return func(o *RequestOptions) { o.IfMatch = etag }
}

// IfNoneMatch sets the If-None-Match header. For GET/HEAD the server returns
// 304 Not Modified when the ETag matches; for other methods it returns 412.
// IfNoneMatch sets the If-None-Match precondition.
func IfNoneMatch(etag string) RequestOption {
return func(req *http.Request) {
req.Header.Set("If-None-Match", etag)
return func(o *RequestOptions) { o.IfNoneMatch = etag }
}

// NewRequestOptions applies opts and returns the resulting RequestOptions.
func NewRequestOptions(opts ...RequestOption) RequestOptions {
var o RequestOptions
for _, opt := range opts {
opt(&o)
}
return o
}

// Check evaluates the preconditions against the stored ETag. It returns
// ErrNotModified for a satisfied If-None-Match, ErrPreconditionFailed for a
// failed If-Match, or nil when all preconditions pass.
func (o RequestOptions) Check(etag string) error {
if o.IfMatch != "" && (etag == "" || !etagListMatches(o.IfMatch, etag)) {
return ErrPreconditionFailed
}
if o.IfNoneMatch != "" && etag != "" && etagListMatches(o.IfNoneMatch, etag) {
return ErrNotModified
}
return nil
}

// applyToRequest sets the conditional headers on an outgoing request.
func (o RequestOptions) applyToRequest(req *http.Request) {
if o.IfMatch != "" {
req.Header.Set("If-Match", o.IfMatch)
}
if o.IfNoneMatch != "" {
req.Header.Set("If-None-Match", o.IfNoneMatch)
}
}

// etagListMatches reports whether etag matches an If-Match / If-None-Match
// header value, which may be a comma-separated list of ETags or the "*"
// wildcard. Stored ETags are always strong, so weak comparison is not required.
func etagListMatches(headerValue, etag string) bool {
for candidate := range strings.SplitSeq(headerValue, ",") {
candidate = strings.TrimSpace(candidate)
if candidate == "*" || candidate == etag {
return true
}
}
return false
}
31 changes: 29 additions & 2 deletions internal/cache/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,25 @@ type Writer = client.CacheWriter
// ErrNotFound is returned when a cache backend is not found.
var ErrNotFound = errors.New("cache backend not found")

// Option configures conditional parameters on a cache Open or Stat.
type Option = client.RequestOption

// IfMatch sets the If-Match precondition. Open/Stat return ErrPreconditionFailed
// if the stored ETag does not match.
func IfMatch(etag string) Option { return client.IfMatch(etag) }

// IfNoneMatch sets the If-None-Match precondition. Open/Stat return
// ErrNotModified when the stored ETag matches.
func IfNoneMatch(etag string) Option { return client.IfNoneMatch(etag) }

// ErrNotModified is returned by Open/Stat when an If-None-Match precondition is
// satisfied.
var ErrNotModified = client.ErrNotModified

// ErrPreconditionFailed is returned by Open/Stat when an If-Match precondition
// is not met.
var ErrPreconditionFailed = client.ErrPreconditionFailed

// ErrStatsUnavailable is returned when a cache backend cannot provide statistics.
var ErrStatsUnavailable = client.ErrStatsUnavailable

Expand Down Expand Up @@ -133,13 +152,21 @@ type Cache interface {
//
// Expired files MUST not be returned.
// Must return os.ErrNotExist if the file does not exist.
Stat(ctx context.Context, key Key) (http.Header, error)
//
// Conditional opts are evaluated against the stored ETag: a satisfied
// If-None-Match returns ErrNotModified (with headers); a failed If-Match
// returns ErrPreconditionFailed.
Stat(ctx context.Context, key Key, opts ...Option) (http.Header, error)
// Open an existing file in the cache.
//
// Expired files MUST NOT be returned.
// The returned headers MUST include a Last-Modified header.
// Must return os.ErrNotExist if the file does not exist.
Open(ctx context.Context, key Key) (io.ReadCloser, http.Header, error)
//
// Conditional opts are evaluated against the stored ETag: a satisfied
// If-None-Match returns ErrNotModified (with headers, no body); a failed
// If-Match returns ErrPreconditionFailed.
Open(ctx context.Context, key Key, opts ...Option) (io.ReadCloser, http.Header, error)
// Create a new file in the cache.
//
// If "ttl" is zero, a maximum TTL MUST be used by the implementation.
Expand Down
59 changes: 59 additions & 0 deletions internal/cache/cachetest/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ func Suite(t *testing.T, newCache func(t *testing.T) cache.Cache) {
t.Run("ETag", func(t *testing.T) {
testETag(t, newCache(t))
})

t.Run("Conditional", func(t *testing.T) {
testConditional(t, newCache(t))
})
}

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

// testConditional verifies that Open and Stat honour If-Match / If-None-Match
// preconditions against the stored ETag, returning the unified sentinel errors.
func testConditional(t *testing.T, c cache.Cache) {
defer c.Close()
ctx := t.Context()

content := []byte("conditional content")
key := cache.NewKey("test-conditional")

w, err := c.Create(ctx, key, nil, time.Hour)
assert.NoError(t, err)
_, err = w.Write(content)
assert.NoError(t, err)
assert.NoError(t, w.Close())

sum := sha256.Sum256(content)
etag := `"` + hex.EncodeToString(sum[:]) + `"`

t.Run("IfNoneMatchHitReturnsNotModified", func(t *testing.T) {
_, headers, err := c.Open(ctx, key, cache.IfNoneMatch(etag))
assert.IsError(t, err, cache.ErrNotModified)
assert.Equal(t, etag, headers.Get("ETag")) // headers surfaced for the 304

headers, err = c.Stat(ctx, key, cache.IfNoneMatch(etag))
assert.IsError(t, err, cache.ErrNotModified)
assert.Equal(t, etag, headers.Get("ETag"))
})

t.Run("IfNoneMatchMissServesBody", func(t *testing.T) {
reader, _, err := c.Open(ctx, key, cache.IfNoneMatch(`"other"`))
assert.NoError(t, err)
defer reader.Close()
data, err := io.ReadAll(reader)
assert.NoError(t, err)
assert.Equal(t, content, data)
})

t.Run("IfMatchHitServesBody", func(t *testing.T) {
reader, _, err := c.Open(ctx, key, cache.IfMatch(etag))
assert.NoError(t, err)
defer reader.Close()
data, err := io.ReadAll(reader)
assert.NoError(t, err)
assert.Equal(t, content, data)
})

t.Run("IfMatchMissReturnsPreconditionFailed", func(t *testing.T) {
_, _, err := c.Open(ctx, key, cache.IfMatch(`"other"`))
assert.IsError(t, err, cache.ErrPreconditionFailed)

_, err = c.Stat(ctx, key, cache.IfMatch(`"other"`))
assert.IsError(t, err, cache.ErrPreconditionFailed)
})
}

func testNamespaceDelete(t *testing.T, c cache.Cache) {
defer c.Close()
ctx := t.Context()
Expand Down
22 changes: 22 additions & 0 deletions internal/cache/conditional.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package cache

import (
"net/http"

"github.com/alecthomas/errors"

"github.com/block/cachew/client"
)

// conditionalShortCircuit evaluates conditional opts against the stored ETag in
// headers. A nil error means the object should be served normally. A non-nil
// error short-circuits the request: ErrNotModified is returned together with
// headers (so callers can surface a 304 with the stored validators), while
// ErrPreconditionFailed is returned with nil headers.
func conditionalShortCircuit(headers http.Header, opts []Option) (http.Header, error) {
err := errors.WithStack(client.NewRequestOptions(opts...).Check(headers.Get(ETagKey)))
if errors.Is(err, ErrNotModified) {
return headers, err
}
return nil, err
}
11 changes: 9 additions & 2 deletions internal/cache/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (d *Disk) Delete(_ context.Context, key Key) error {
return nil
}

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

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

headers.Set("Content-Length", strconv.FormatInt(info.Size(), 10))
if h, err := conditionalShortCircuit(headers, opts); err != nil {
return h, err
}
return headers, nil
}

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

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

if h, condErr := conditionalShortCircuit(headers, opts); condErr != nil {
return nil, h, errors.Join(condErr, f.Close())
}

return f, headers, nil
}

Expand Down
10 changes: 8 additions & 2 deletions internal/cache/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func NewMemory(ctx context.Context, config MemoryConfig) (*Memory, error) {

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

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

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

headers := maps.Clone(entry.headers)
headers.Set("Content-Length", strconv.Itoa(len(entry.data)))
if h, err := conditionalShortCircuit(headers, opts); err != nil {
return h, err
}
return headers, nil
}

func (m *Memory) Open(_ context.Context, key Key) (io.ReadCloser, http.Header, error) {
func (m *Memory) Open(_ context.Context, key Key, opts ...Option) (io.ReadCloser, http.Header, error) {
m.mu.RLock()
defer m.mu.RUnlock()

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

headers := maps.Clone(entry.headers)
headers.Set("Content-Length", strconv.Itoa(len(entry.data)))
if h, err := conditionalShortCircuit(headers, opts); err != nil {
return nil, h, err
}
return io.NopCloser(bytes.NewReader(entry.data)), headers, nil
}

Expand Down
4 changes: 2 additions & 2 deletions internal/cache/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ func NoOpCache() Cache {

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

func (n *noOpCache) Stat(_ context.Context, _ Key) (http.Header, error) {
func (n *noOpCache) Stat(_ context.Context, _ Key, _ ...Option) (http.Header, error) {
return nil, os.ErrNotExist
}

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

Expand Down
8 changes: 4 additions & 4 deletions internal/cache/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ func (r *Remote) Namespace(namespace Namespace) Cache {
return &Remote{c: r.c.Namespace(namespace)}
}

func (r *Remote) Open(ctx context.Context, key Key) (io.ReadCloser, http.Header, error) {
rc, h, err := r.c.Open(ctx, key)
func (r *Remote) Open(ctx context.Context, key Key, opts ...Option) (io.ReadCloser, http.Header, error) {
rc, h, err := r.c.Open(ctx, key, opts...)
return rc, h, errors.WithStack(err)
}

func (r *Remote) Stat(ctx context.Context, key Key) (http.Header, error) {
return errors.WithStack2(r.c.Stat(ctx, key))
func (r *Remote) Stat(ctx context.Context, key Key, opts ...Option) (http.Header, error) {
return errors.WithStack2(r.c.Stat(ctx, key, opts...))
}

func (r *Remote) Create(ctx context.Context, key Key, headers http.Header, ttl time.Duration) (Writer, error) {
Expand Down
Loading