Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

routing/http/server: add cache control #584

Merged
merged 6 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 5 minutes for responses with providers.

### Changed

### Removed
Expand Down
60 changes: 53 additions & 7 deletions routing/http/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"io"
"mime"
"net/http"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -402,15 +401,26 @@
return
}

var remainingValidity int
// Include 'Expires' header with time when signature expiration happens
if validityType, err := record.ValidityType(); err == nil && validityType == ipns.ValidityEOL {
if validity, err := record.Validity(); err == nil {
w.Header().Set("Expires", validity.UTC().Format(http.TimeFormat))
remainingValidity = int(validity.Sub(time.Now()).Seconds())

Check failure on line 409 in routing/http/server/server.go

View workflow job for this annotation

GitHub Actions / go-check / All

should use time.Until instead of t.Sub(time.Now()) (S1024)
}
} else {
remainingValidity = int(ipns.DefaultRecordLifetime.Seconds())
}

Check warning on line 413 in routing/http/server/server.go

View check run for this annotation

Codecov / codecov/patch

routing/http/server/server.go#L411-L413

Added lines #L411 - L413 were not covered by tests
if ttl, err := record.TTL(); err == nil {
w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%d", int(ttl.Seconds())))
setCacheControl(w, int(ttl.Seconds()), remainingValidity)
hacdias marked this conversation as resolved.
Show resolved Hide resolved
} else {
w.Header().Set("Cache-Control", "max-age=60")
setCacheControl(w, int(ipns.DefaultRecordTTL.Seconds()), remainingValidity)

Check warning on line 417 in routing/http/server/server.go

View check run for this annotation

Codecov / codecov/patch

routing/http/server/server.go#L417

Added line #L417 was not covered by tests
}
w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat))
hacdias marked this conversation as resolved.
Show resolved Hide resolved

recordEtag := strconv.FormatUint(xxhash.Sum64(rawRecord), 32)
w.Header().Set("Etag", recordEtag)
w.Header().Set("Etag", fmt.Sprintf(`"%x"`, xxhash.Sum64(rawRecord)))
hacdias marked this conversation as resolved.
Show resolved Hide resolved
w.Header().Set("Content-Type", mediaTypeIPNSRecord)
w.Header().Add("Vary", "Accept")
hacdias marked this conversation as resolved.
Show resolved Hide resolved
w.Write(rawRecord)
}

Expand Down Expand Up @@ -462,8 +472,30 @@
w.WriteHeader(http.StatusOK)
}

func writeJSONResult(w http.ResponseWriter, method string, val any) {
var (
// Rule-of-thumb Cache-Control policy is to work well with caching proxies and load balancers.
// If there are any results, cache on the client for longer, and hint any in-between caches to
// serve cached result and upddate cache in background as long we have
// result that is within Amino DHT expiration window
maxAgeWithResults = int((5 * time.Minute).Seconds()) // cache >0 results for longer
maxAgeWithoutResults = int((15 * time.Second).Seconds()) // cache no results briefly
maxStale = int((48 * time.Hour).Seconds()) // allow stale results as long within Amino DHT Expiration window
)

func setCacheControl(w http.ResponseWriter, maxAge int, stale int) {
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d, stale-while-revalidate=%d, stale-if-error=%d", maxAge, stale, stale))
}
hacdias marked this conversation as resolved.
Show resolved Hide resolved

func writeJSONResult(w http.ResponseWriter, method string, val interface{ Length() int }) {
w.Header().Add("Content-Type", mediaTypeJSON)
w.Header().Add("Vary", "Accept")

if val.Length() > 0 {
setCacheControl(w, maxAgeWithResults, maxStale)
} else {
setCacheControl(w, maxAgeWithoutResults, maxStale)
}
w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat))

// 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)
Expand Down Expand Up @@ -500,21 +532,30 @@
defer resultIter.Close()

w.Header().Set("Content-Type", mediaTypeNDJSON)
w.WriteHeader(http.StatusOK)
hacdias marked this conversation as resolved.
Show resolved Hide resolved
w.Header().Add("Vary", "Accept")
w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat))

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 {
logger.Errorw("ndjson marshal error", "Error", err)
return
}

if !hasResults {
hasResults = true
// There's results, cache useful result for longer
setCacheControl(w, maxAgeWithResults, maxStale)
}

_, err = w.Write(b)
if err != nil {
logger.Warn("ndjson write error", "Error", err)
Expand All @@ -531,4 +572,9 @@
f.Flush()
}
}

if !hasResults {
// There weren't results, cache for shorter
setCacheControl(w, maxAgeWithoutResults, maxStale)
}
}
135 changes: 104 additions & 31 deletions routing/http/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func TestHeaders(t *testing.T) {
require.Equal(t, 200, resp.StatusCode)
header := resp.Header.Get("Content-Type")
require.Equal(t, mediaTypeJSON, header)
require.Equal(t, "Accept", resp.Header.Get("Vary"))

resp, err = http.Get(serverAddr + "/routing/v1/providers/" + "BAD_CID")
require.NoError(t, err)
Expand Down Expand Up @@ -79,25 +80,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))
Expand All @@ -117,8 +124,17 @@ 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"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))

if empty {
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
} else {
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
}
// 'Last-Modified' is expected to be present and match current time
require.Equal(t, time.Now().UTC().Format(http.TimeFormat), resp.Header.Get("Last-Modified"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand All @@ -127,11 +143,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, "")
})
}

Expand All @@ -155,7 +179,25 @@ func TestPeers(t *testing.T) {
require.Equal(t, 400, resp.StatusCode)
})

t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body (JSON)", func(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-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, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, time.Now().UTC().Format(http.TimeFormat), resp.Header.Get("Last-Modified"))
})

t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (JSON)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
Expand All @@ -181,8 +223,10 @@ func TestPeers(t *testing.T) {
resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID)
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, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, time.Now().UTC().Format(http.TimeFormat), resp.Header.Get("Last-Modified"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand All @@ -191,7 +235,25 @@ func TestPeers(t *testing.T) {
require.Equal(t, expectedBody, string(body))
})

t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body (NDJSON)", func(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-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, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=15, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))
require.Equal(t, time.Now().UTC().Format(http.TimeFormat), resp.Header.Get("Last-Modified"))
})

t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (NDJSON)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
Expand All @@ -217,8 +279,9 @@ func TestPeers(t *testing.T) {
resp := makeRequest(t, router, mediaTypeNDJSON, libp2pKeyCID)
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, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand Down Expand Up @@ -254,6 +317,7 @@ func TestPeers(t *testing.T) {
require.Equal(t, 200, resp.StatusCode)

header := resp.Header.Get("Content-Type")
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, mediaTypeJSON, header)

body, err := io.ReadAll(resp.Body)
Expand Down Expand Up @@ -290,6 +354,7 @@ func TestPeers(t *testing.T) {
require.Equal(t, 200, resp.StatusCode)

header := resp.Header.Get("Content-Type")
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, mediaTypeNDJSON, header)

body, err := io.ReadAll(resp.Body)
Expand All @@ -306,10 +371,8 @@ func makeName(t *testing.T) (crypto.PrivKey, ipns.Name) {
return sk, ipns.NameFromPeer(pid)
}

func makeIPNSRecord(t *testing.T, cid cid.Cid, sk crypto.PrivKey, opts ...ipns.Option) (*ipns.Record, []byte) {
func makeIPNSRecord(t *testing.T, cid cid.Cid, eol time.Time, ttl time.Duration, sk crypto.PrivKey, opts ...ipns.Option) (*ipns.Record, []byte) {
path := path.FromCid(cid)
eol := time.Now().Add(time.Hour * 48)
ttl := time.Second * 20

record, err := ipns.NewRecord(sk, path, 1, eol, ttl, opts...)
require.NoError(t, err)
Expand Down Expand Up @@ -339,7 +402,10 @@ func TestIPNS(t *testing.T) {

runWithRecordOptions := func(t *testing.T, opts ...ipns.Option) {
sk, name1 := makeName(t)
record1, rawRecord1 := makeIPNSRecord(t, cid1, sk)
now := time.Now()
eol := now.Add(24 * time.Hour * 7) // record valid for a week
ttl := 42 * time.Second // distinct TTL
record1, rawRecord1 := makeIPNSRecord(t, cid1, eol, ttl, sk)

_, name2 := makeName(t)

Expand All @@ -355,8 +421,15 @@ func TestIPNS(t *testing.T) {
resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.String())
require.Equal(t, 200, resp.StatusCode)
require.Equal(t, mediaTypeIPNSRecord, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.NotEmpty(t, resp.Header.Get("Etag"))
require.Equal(t, "max-age=20", resp.Header.Get("Cache-Control"))
require.Equal(t, now.UTC().Format(http.TimeFormat), resp.Header.Get("Last-Modified"))

// expected "stale" values are int(eol.Sub(now).Seconds())
require.Equal(t, "public, max-age=42, stale-while-revalidate=604799, stale-if-error=604799", resp.Header.Get("Cache-Control"))

// 'Expires' on IPNS result is expected to match EOL of IPNS Record with ValidityType=0
require.Equal(t, eol.UTC().Format(http.TimeFormat), resp.Header.Get("Expires"))

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
Expand Down
12 changes: 12 additions & 0 deletions routing/http/types/json/responses.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
Loading