diff --git a/CHANGELOG.md b/CHANGELOG.md index 785b27904..0db249311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ The following emojis are used to highlight certain changes: ### Added +* `routing/http/server` now adds `Cache-Control` HTTP header to GET requests: 15 seconds for empty responses, or 15 minutes for responses with providers. + ### Changed ### Removed diff --git a/routing/http/server/server.go b/routing/http/server/server.go index d9be47eb2..889d82445 100644 --- a/routing/http/server/server.go +++ b/routing/http/server/server.go @@ -457,9 +457,17 @@ func (s *server) PutIPNS(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) } -func writeJSONResult(w http.ResponseWriter, method string, val any) { +func writeJSONResult(w http.ResponseWriter, method string, val interface{ Length() int }) { w.Header().Add("Content-Type", mediaTypeJSON) + if val.Length() > 0 { + // There's results, cache for 5 minutes + w.Header().Set("Cache-Control", "max-age=900, public") + } else { + // There weren't results, cache for 15 seconds + w.Header().Set("Cache-Control", "max-age=15, public") + } + // keep the marshaling separate from the writing, so we can distinguish bugs (which surface as 500) // from transient network issues (which surface as transport errors) b, err := drjson.MarshalJSONBytes(val) @@ -495,14 +503,15 @@ func writeResultsIterNDJSON[T any](w http.ResponseWriter, resultIter iter.Result defer resultIter.Close() w.Header().Set("Content-Type", mediaTypeNDJSON) - w.WriteHeader(http.StatusOK) + hasResults := false for resultIter.Next() { res := resultIter.Val() if res.Err != nil { logger.Errorw("ndjson iterator error", "Error", res.Err) return } + // don't use an encoder because we can't easily differentiate writer errors from encoding errors b, err := drjson.MarshalJSONBytes(res.Val) if err != nil { @@ -510,6 +519,12 @@ func writeResultsIterNDJSON[T any](w http.ResponseWriter, resultIter iter.Result return } + if !hasResults { + // There's results, cache for 5 minutes + w.Header().Set("Cache-Control", "max-age=900, public") + hasResults = true + } + _, err = w.Write(b) if err != nil { logger.Warn("ndjson write error", "Error", err) @@ -526,4 +541,9 @@ func writeResultsIterNDJSON[T any](w http.ResponseWriter, resultIter iter.Result f.Flush() } } + + if !hasResults { + // There weren't results, cache for 15 seconds + w.Header().Set("Cache-Control", "max-age=15, public") + } } diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go index f823ac25a..ef0642a29 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -79,25 +79,31 @@ func TestProviders(t *testing.T) { cid, err := cid.Decode(cidStr) require.NoError(t, err) - runTest := func(t *testing.T, contentType string, expectedStream bool, expectedBody string) { + runTest := func(t *testing.T, contentType string, empty bool, expectedStream bool, expectedBody string) { t.Parallel() - results := iter.FromSlice([]iter.Result[types.Record]{ - {Val: &types.PeerRecord{ - Schema: types.SchemaPeer, - ID: &pid, - Protocols: []string{"transport-bitswap"}, - Addrs: []types.Multiaddr{}, - }}, - //lint:ignore SA1019 // ignore staticcheck - {Val: &types.BitswapRecord{ + var results *iter.SliceIter[iter.Result[types.Record]] + + if empty { + results = iter.FromSlice([]iter.Result[types.Record]{}) + } else { + results = iter.FromSlice([]iter.Result[types.Record]{ + {Val: &types.PeerRecord{ + Schema: types.SchemaPeer, + ID: &pid, + Protocols: []string{"transport-bitswap"}, + Addrs: []types.Multiaddr{}, + }}, //lint:ignore SA1019 // ignore staticcheck - Schema: types.SchemaBitswap, - ID: &pid2, - Protocol: "transport-bitswap", - Addrs: []types.Multiaddr{}, - }}}, - ) + {Val: &types.BitswapRecord{ + //lint:ignore SA1019 // ignore staticcheck + Schema: types.SchemaBitswap, + ID: &pid2, + Protocol: "transport-bitswap", + Addrs: []types.Multiaddr{}, + }}}, + ) + } router := &mockContentRouter{} server := httptest.NewServer(Handler(router)) @@ -117,8 +123,14 @@ func TestProviders(t *testing.T) { resp, err := http.DefaultClient.Do(req) require.NoError(t, err) require.Equal(t, 200, resp.StatusCode) - header := resp.Header.Get("Content-Type") - require.Equal(t, contentType, header) + + require.Equal(t, contentType, resp.Header.Get("Content-Type")) + + if empty { + require.Equal(t, "max-age=15, public", resp.Header.Get("Cache-Control")) + } else { + require.Equal(t, "max-age=900, public", resp.Header.Get("Cache-Control")) + } body, err := io.ReadAll(resp.Body) require.NoError(t, err) @@ -127,11 +139,19 @@ func TestProviders(t *testing.T) { } t.Run("JSON Response", func(t *testing.T) { - runTest(t, mediaTypeJSON, false, `{"Providers":[{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}]}`) + runTest(t, mediaTypeJSON, false, false, `{"Providers":[{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}]}`) + }) + + t.Run("Empty JSON Response", func(t *testing.T) { + runTest(t, mediaTypeJSON, true, false, `{"Providers":null}`) }) t.Run("NDJSON Response", func(t *testing.T) { - runTest(t, mediaTypeNDJSON, true, `{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}`+"\n") + runTest(t, mediaTypeNDJSON, false, true, `{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}`+"\n") + }) + + t.Run("Empty NDJSON Response", func(t *testing.T) { + runTest(t, mediaTypeNDJSON, true, true, "") }) } @@ -164,7 +184,23 @@ func TestPeers(t *testing.T) { require.Equal(t, 400, resp.StatusCode) }) - t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body (JSON)", func(t *testing.T) { + t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body and headers (No Results, JSON)", func(t *testing.T) { + t.Parallel() + + _, pid := makePeerID(t) + results := iter.FromSlice([]iter.Result[*types.PeerRecord]{}) + + router := &mockContentRouter{} + router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil) + + resp := makeRequest(t, router, mediaTypeJSON, peer.ToCid(pid).String()) + require.Equal(t, 200, resp.StatusCode) + + require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) + require.Equal(t, "max-age=15, public", resp.Header.Get("Cache-Control")) + }) + + t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body and headers (JSON)", func(t *testing.T) { t.Parallel() _, pid := makePeerID(t) @@ -189,8 +225,8 @@ func TestPeers(t *testing.T) { resp := makeRequest(t, router, mediaTypeJSON, peer.ToCid(pid).String()) require.Equal(t, 200, resp.StatusCode) - header := resp.Header.Get("Content-Type") - require.Equal(t, mediaTypeJSON, header) + require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) + require.Equal(t, "max-age=900, public", resp.Header.Get("Cache-Control")) body, err := io.ReadAll(resp.Body) require.NoError(t, err) @@ -199,7 +235,23 @@ func TestPeers(t *testing.T) { require.Equal(t, expectedBody, string(body)) }) - t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body (NDJSON)", func(t *testing.T) { + t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body and headers (No Results, NDJSON)", func(t *testing.T) { + t.Parallel() + + _, pid := makePeerID(t) + results := iter.FromSlice([]iter.Result[*types.PeerRecord]{}) + + router := &mockContentRouter{} + router.On("FindPeers", mock.Anything, pid, 0).Return(results, nil) + + resp := makeRequest(t, router, mediaTypeNDJSON, peer.ToCid(pid).String()) + require.Equal(t, 200, resp.StatusCode) + + require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type")) + require.Equal(t, "max-age=15, public", resp.Header.Get("Cache-Control")) + }) + + t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body and headers (NDJSON)", func(t *testing.T) { t.Parallel() _, pid := makePeerID(t) @@ -224,8 +276,8 @@ func TestPeers(t *testing.T) { resp := makeRequest(t, router, mediaTypeNDJSON, peer.ToCid(pid).String()) require.Equal(t, 200, resp.StatusCode) - header := resp.Header.Get("Content-Type") - require.Equal(t, mediaTypeNDJSON, header) + require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type")) + require.Equal(t, "max-age=900, public", resp.Header.Get("Cache-Control")) body, err := io.ReadAll(resp.Body) require.NoError(t, err) diff --git a/routing/http/types/json/responses.go b/routing/http/types/json/responses.go index cc687df48..d8f659ac5 100644 --- a/routing/http/types/json/responses.go +++ b/routing/http/types/json/responses.go @@ -11,11 +11,19 @@ type ProvidersResponse struct { Providers RecordsArray } +func (r ProvidersResponse) Length() int { + return len(r.Providers) +} + // PeersResponse is the result of a GET Peers request. type PeersResponse struct { Peers []*types.PeerRecord } +func (r PeersResponse) Length() int { + return len(r.Peers) +} + // RecordsArray is an array of [types.Record] type RecordsArray []types.Record @@ -65,6 +73,10 @@ type WriteProvidersResponse struct { ProvideResults []types.Record } +func (r WriteProvidersResponse) Length() int { + return len(r.ProvideResults) +} + func (r *WriteProvidersResponse) UnmarshalJSON(b []byte) error { var tempWPR struct{ ProvideResults []json.RawMessage } err := json.Unmarshal(b, &tempWPR)