Moved common HTTP polling helpers into feedkit and implemented support for ETag and Last-Modified

2026-03-28 09:59:58 -05:00
parent 3b92c2284d
commit 4910440756
6 changed files with 610 additions and 11 deletions
--- a/transport/http.go
+++ b/transport/http.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"strings"
 	"time"
 )

@@ -28,7 +29,80 @@ func NewHTTPClient(timeout time.Duration) *http.Client {
 }

 func FetchBody(ctx context.Context, client *http.Client, url, userAgent, accept string) ([]byte, error) {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	res, err := doRequest(ctx, client, http.MethodGet, url, userAgent, accept, "", "")
+	if err != nil {
+		return nil, err
+	}
+	defer res.Body.Close()
+
+	if res.StatusCode < 200 || res.StatusCode >= 300 {
+		return nil, fmt.Errorf("HTTP %s", res.Status)
+	}
+
+	return readValidatedBody(res.Body)
+}
+
+// HTTPValidators are cache validators learned from prior successful GET responses.
+//
+// ETag is preferred when present. LastModified is used as a fallback validator
+// when ETag is unavailable.
+type HTTPValidators struct {
+	ETag         string
+	LastModified string
+}
+
+// FetchBodyIfChanged performs an HTTP GET and opportunistically uses conditional
+// request headers based on the provided validators.
+//
+// Behavior:
+//   - if conditional is false, this behaves like a normal GET and leaves validators unchanged
+//   - if validators.ETag is set, sends If-None-Match
+//   - else if validators.LastModified is set, sends If-Modified-Since
+//   - 304 Not Modified is treated as success with changed=false and no body
+//   - 200 responses are treated as changed=true and still enforce the normal body checks
+//
+// Returned validators reflect any updates learned from the response headers.
+func FetchBodyIfChanged(
+	ctx context.Context,
+	client *http.Client,
+	url, userAgent, accept string,
+	conditional bool,
+	validators HTTPValidators,
+) ([]byte, bool, HTTPValidators, error) {
+	headerName, headerValue := conditionalHeader(conditional, validators)
+
+	res, err := doRequest(ctx, client, http.MethodGet, url, userAgent, accept, headerName, headerValue)
+	if err != nil {
+		return nil, false, validators, err
+	}
+	defer res.Body.Close()
+
+	switch res.StatusCode {
+	case http.StatusNotModified:
+		if conditional {
+			validators = refreshValidators(validators, res.Header)
+		}
+		return nil, false, validators, nil
+	default:
+		if res.StatusCode < 200 || res.StatusCode >= 300 {
+			return nil, false, validators, fmt.Errorf("HTTP %s", res.Status)
+		}
+	}
+
+	b, err := readValidatedBody(res.Body)
+	if err != nil {
+		return nil, false, validators, err
+	}
+
+	if conditional {
+		validators = replaceValidators(res.Header)
+	}
+
+	return b, true, validators, nil
+}
+
+func doRequest(ctx context.Context, client *http.Client, method, url, userAgent, accept, headerName, headerValue string) (*http.Response, error) {
+	req, err := http.NewRequestWithContext(ctx, method, url, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -39,19 +113,46 @@ func FetchBody(ctx context.Context, client *http.Client, url, userAgent, accept
 	if accept != "" {
 		req.Header.Set("Accept", accept)
 	}
-
-	res, err := client.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
-
-	if res.StatusCode < 200 || res.StatusCode >= 300 {
-		return nil, fmt.Errorf("HTTP %s", res.Status)
+	if headerName != "" && headerValue != "" {
+		req.Header.Set(headerName, headerValue)
 	}

+	return client.Do(req)
+}
+
+func conditionalHeader(enabled bool, validators HTTPValidators) (string, string) {
+	if !enabled {
+		return "", ""
+	}
+	if etag := strings.TrimSpace(validators.ETag); etag != "" {
+		return "If-None-Match", etag
+	}
+	if lastModified := strings.TrimSpace(validators.LastModified); lastModified != "" {
+		return "If-Modified-Since", lastModified
+	}
+	return "", ""
+}
+
+func replaceValidators(header http.Header) HTTPValidators {
+	return HTTPValidators{
+		ETag:         strings.TrimSpace(header.Get("ETag")),
+		LastModified: strings.TrimSpace(header.Get("Last-Modified")),
+	}
+}
+
+func refreshValidators(current HTTPValidators, header http.Header) HTTPValidators {
+	if etag := strings.TrimSpace(header.Get("ETag")); etag != "" {
+		current.ETag = etag
+	}
+	if lastModified := strings.TrimSpace(header.Get("Last-Modified")); lastModified != "" {
+		current.LastModified = lastModified
+	}
+	return current
+}
+
+func readValidatedBody(r io.Reader) ([]byte, error) {
 	// Read at most maxResponseBodyBytes + 1 so we can detect overflow.
-	limited := io.LimitReader(res.Body, maxResponseBodyBytes+1)
+	limited := io.LimitReader(r, maxResponseBodyBytes+1)

 	b, err := io.ReadAll(limited)
 	if err != nil {
--- a/transport/http_test.go
+++ b/transport/http_test.go
@@ -0,0 +1,232 @@
+package transport
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestFetchBodyIfChangedPrefersETagAndTreats304AsUnchanged(t *testing.T) {
+	t.Helper()
+
+	var call int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		call++
+		switch call {
+		case 1:
+			if got := r.Header.Get("If-None-Match"); got != "" {
+				t.Fatalf("first request If-None-Match = %q, want empty", got)
+			}
+			if got := r.Header.Get("If-Modified-Since"); got != "" {
+				t.Fatalf("first request If-Modified-Since = %q, want empty", got)
+			}
+			w.Header().Set("ETag", `"v1"`)
+			w.Header().Set("Last-Modified", "Mon, 02 Jan 2006 15:04:05 GMT")
+			_, _ = w.Write([]byte(`{"ok":true}`))
+		case 2:
+			if got := r.Header.Get("If-None-Match"); got != `"v1"` {
+				t.Fatalf("second request If-None-Match = %q, want %q", got, `"v1"`)
+			}
+			if got := r.Header.Get("If-Modified-Since"); got != "" {
+				t.Fatalf("second request If-Modified-Since = %q, want empty when ETag is cached", got)
+			}
+			w.WriteHeader(http.StatusNotModified)
+		default:
+			t.Fatalf("unexpected call count %d", call)
+		}
+	}))
+	defer srv.Close()
+
+	validators := HTTPValidators{}
+	body, changed, next, err := FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "application/json", true, validators)
+	if err != nil {
+		t.Fatalf("first FetchBodyIfChanged() error = %v", err)
+	}
+	if !changed {
+		t.Fatalf("first FetchBodyIfChanged() changed = false, want true")
+	}
+	if got := string(body); got != `{"ok":true}` {
+		t.Fatalf("first FetchBodyIfChanged() body = %q", got)
+	}
+	if got := next.ETag; got != `"v1"` {
+		t.Fatalf("cached ETag = %q, want %q", got, `"v1"`)
+	}
+	if got := next.LastModified; got != "Mon, 02 Jan 2006 15:04:05 GMT" {
+		t.Fatalf("cached Last-Modified = %q", got)
+	}
+
+	body, changed, next, err = FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "application/json", true, next)
+	if err != nil {
+		t.Fatalf("second FetchBodyIfChanged() error = %v", err)
+	}
+	if changed {
+		t.Fatalf("second FetchBodyIfChanged() changed = true, want false")
+	}
+	if body != nil {
+		t.Fatalf("second FetchBodyIfChanged() body = %q, want nil", string(body))
+	}
+	if got := next.ETag; got != `"v1"` {
+		t.Fatalf("cached ETag after 304 = %q, want %q", got, `"v1"`)
+	}
+}
+
+func TestFetchBodyIfChangedFallsBackToIfModifiedSince(t *testing.T) {
+	t.Helper()
+
+	var call int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		call++
+		switch call {
+		case 1:
+			w.Header().Set("Last-Modified", "Tue, 03 Jan 2006 15:04:05 GMT")
+			_, _ = w.Write([]byte(`first`))
+		case 2:
+			if got := r.Header.Get("If-None-Match"); got != "" {
+				t.Fatalf("second request If-None-Match = %q, want empty", got)
+			}
+			if got := r.Header.Get("If-Modified-Since"); got != "Tue, 03 Jan 2006 15:04:05 GMT" {
+				t.Fatalf("second request If-Modified-Since = %q", got)
+			}
+			w.WriteHeader(http.StatusNotModified)
+		default:
+			t.Fatalf("unexpected call count %d", call)
+		}
+	}))
+	defer srv.Close()
+
+	_, changed, validators, err := FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "", true, HTTPValidators{})
+	if err != nil {
+		t.Fatalf("first FetchBodyIfChanged() error = %v", err)
+	}
+	if !changed {
+		t.Fatalf("first FetchBodyIfChanged() changed = false, want true")
+	}
+	if got := validators.LastModified; got != "Tue, 03 Jan 2006 15:04:05 GMT" {
+		t.Fatalf("cached Last-Modified = %q", got)
+	}
+
+	_, changed, _, err = FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "", true, validators)
+	if err != nil {
+		t.Fatalf("second FetchBodyIfChanged() error = %v", err)
+	}
+	if changed {
+		t.Fatalf("second FetchBodyIfChanged() changed = true, want false")
+	}
+}
+
+func TestFetchBodyIfChangedClearsValidatorsOn200WithoutValidators(t *testing.T) {
+	t.Helper()
+
+	var call int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		call++
+		switch call {
+		case 1:
+			w.Header().Set("ETag", `"v1"`)
+			_, _ = w.Write([]byte(`first`))
+		case 2:
+			if got := r.Header.Get("If-None-Match"); got != `"v1"` {
+				t.Fatalf("second request If-None-Match = %q", got)
+			}
+			_, _ = w.Write([]byte(`second`))
+		case 3:
+			if got := r.Header.Get("If-None-Match"); got != "" {
+				t.Fatalf("third request If-None-Match = %q, want empty", got)
+			}
+			if got := r.Header.Get("If-Modified-Since"); got != "" {
+				t.Fatalf("third request If-Modified-Since = %q, want empty", got)
+			}
+			_, _ = w.Write([]byte(`third`))
+		default:
+			t.Fatalf("unexpected call count %d", call)
+		}
+	}))
+	defer srv.Close()
+
+	_, _, validators, err := FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "", true, HTTPValidators{})
+	if err != nil {
+		t.Fatalf("first FetchBodyIfChanged() error = %v", err)
+	}
+	_, _, validators, err = FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "", true, validators)
+	if err != nil {
+		t.Fatalf("second FetchBodyIfChanged() error = %v", err)
+	}
+	if validators.ETag != "" || validators.LastModified != "" {
+		t.Fatalf("validators after 200 without validators = %+v, want cleared", validators)
+	}
+	_, _, _, err = FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "", true, validators)
+	if err != nil {
+		t.Fatalf("third FetchBodyIfChanged() error = %v", err)
+	}
+}
+
+func TestFetchBodyIfChangedConditionalDisabledSkipsConditionalHeaders(t *testing.T) {
+	t.Helper()
+
+	var calls int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls++
+		if got := r.Header.Get("If-None-Match"); got != "" {
+			t.Fatalf("request If-None-Match = %q, want empty", got)
+		}
+		if got := r.Header.Get("If-Modified-Since"); got != "" {
+			t.Fatalf("request If-Modified-Since = %q, want empty", got)
+		}
+		_, _ = w.Write([]byte(`body`))
+	}))
+	defer srv.Close()
+
+	validators := HTTPValidators{ETag: `"v1"`, LastModified: "Wed, 04 Jan 2006 15:04:05 GMT"}
+	_, changed, next, err := FetchBodyIfChanged(context.Background(), srv.Client(), srv.URL, "test-agent", "", false, validators)
+	if err != nil {
+		t.Fatalf("FetchBodyIfChanged() error = %v", err)
+	}
+	if !changed {
+		t.Fatalf("FetchBodyIfChanged() changed = false, want true")
+	}
+	if next != validators {
+		t.Fatalf("validators changed when conditional disabled: got %+v want %+v", next, validators)
+	}
+	if calls != 1 {
+		t.Fatalf("calls = %d, want 1", calls)
+	}
+}
+
+func TestFetchBodyIfChangedAllowsEmpty304ButRejectsEmpty200(t *testing.T) {
+	t.Helper()
+
+	notModified := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotModified)
+	}))
+	defer notModified.Close()
+
+	_, changed, _, err := FetchBodyIfChanged(
+		context.Background(),
+		notModified.Client(),
+		notModified.URL,
+		"test-agent",
+		"",
+		true,
+		HTTPValidators{ETag: `"v1"`},
+	)
+	if err != nil {
+		t.Fatalf("304 FetchBodyIfChanged() error = %v", err)
+	}
+	if changed {
+		t.Fatalf("304 FetchBodyIfChanged() changed = true, want false")
+	}
+
+	emptyBody := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer emptyBody.Close()
+
+	_, _, _, err = FetchBodyIfChanged(context.Background(), emptyBody.Client(), emptyBody.URL, "test-agent", "", true, HTTPValidators{})
+	if err == nil {
+		t.Fatalf("empty 200 FetchBodyIfChanged() error = nil, want error")
+	}
+	if err.Error() != "empty response body" {
+		t.Fatalf("empty 200 FetchBodyIfChanged() error = %q", err)
+	}
+}