From 67f2a27a943d89ad9392a7803343513c4dcb0865 Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Tue, 10 Oct 2023 10:45:57 +0200 Subject: [PATCH] fix: link header in keyset pagination (#729) --- pagination/keysetpagination/header.go | 11 ++++----- pagination/keysetpagination/header_test.go | 26 +++++++++++++--------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/pagination/keysetpagination/header.go b/pagination/keysetpagination/header.go index b7474e0c..e523859b 100644 --- a/pagination/keysetpagination/header.go +++ b/pagination/keysetpagination/header.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "github.com/pkg/errors" ) @@ -83,11 +84,11 @@ func header(u *url.URL, rel, token string, size int) string { // It contains links to the first and next page, if one exists. func Header(w http.ResponseWriter, u *url.URL, p *Paginator) { size := p.Size() - w.Header().Set("Link", header(u, "first", p.defaultToken.Encode(), size)) - - if !p.IsLast() { - w.Header().Add("Link", header(u, "next", p.Token().Encode(), size)) + link := []string{header(u, "first", p.defaultToken.Encode(), size)} + if !p.isLast { + link = append(link, header(u, "next", p.Token().Encode(), size)) } + w.Header().Set("Link", strings.Join(link, ",")) } // Parse returns the pagination options from the URL query. @@ -104,7 +105,7 @@ func Parse(q url.Values, p PageTokenConstructor) ([]Option, error) { } opts = append(opts, WithToken(parsed)) } - if q.Has("page_size") { + if q.Get("page_size") != "" { size, err := strconv.Atoi(q.Get("page_size")) if err != nil { return nil, errors.WithStack(err) diff --git a/pagination/keysetpagination/header_test.go b/pagination/keysetpagination/header_test.go index 19186f4e..a6eb20e3 100644 --- a/pagination/keysetpagination/header_test.go +++ b/pagination/keysetpagination/header_test.go @@ -8,6 +8,7 @@ import ( "net/url" "testing" + "github.com/peterhellberg/link" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -26,19 +27,22 @@ func TestHeader(t *testing.T) { Header(r, u, p) - links := r.HeaderMap["Link"] - require.Len(t, links, 2) - assert.Contains(t, links[0], "page_token=default") - assert.Contains(t, links[1], "page_token=next") + assert.Len(t, r.Result().Header.Values("link"), 1, "make sure we send one header with multiple comma-separated values rather than multiple headers") - t.Run("with isLast", func(t *testing.T) { - p.isLast = true + links := link.ParseResponse(r.Result()) + assert.Contains(t, links, "first") + assert.Contains(t, links["first"].URI, "page_token=default") - Header(r, u, p) + assert.Contains(t, links, "next") + assert.Contains(t, links["next"].URI, "page_token=next") - links := r.HeaderMap["Link"] - require.Len(t, links, 1) - assert.Contains(t, links[0], "page_token=default") - }) + p.isLast = true + r = httptest.NewRecorder() + Header(r, u, p) + links = link.ParseResponse(r.Result()) + + assert.Contains(t, links, "first") + assert.Contains(t, links["first"].URI, "page_token=default") + assert.NotContains(t, links, "next") }