Make Pages.Prev/Next work like the other Prev/Next methods

Fixes #4500

diff --git a/hugolib/pages_test.go b/hugolib/pages_test.go
new file mode 100644 (file)
index 0000000..371ffe8
--- /dev/null
+++ b/hugolib/pages_test.go
@@ -0,0 +1,83 @@
+package hugolib
+
+import (
+       "fmt"
+       "math/rand"
+       "testing"
+
+       "github.com/gohugoio/hugo/resources/page"
+
+       qt "github.com/frankban/quicktest"
+)
+
+func newPagesPrevNextTestSite(t testing.TB, numPages int) *sitesBuilder {
+       pageTemplate := `
+---
+title: "Page %d"
+weight: %d
+---
+
+`
+       b := newTestSitesBuilder(t)
+
+       for i := 1; i <= numPages; i++ {
+               b.WithContent(fmt.Sprintf("page%d.md", i), fmt.Sprintf(pageTemplate, i, rand.Intn(numPages)))
+       }
+
+       return b
+}
+
+func TestPagesPrevNext(t *testing.T) {
+       b := newPagesPrevNextTestSite(t, 100)
+       b.Build(BuildCfg{SkipRender: true})
+
+       pages := b.H.Sites[0].RegularPages()
+
+       b.Assert(pages, qt.HasLen, 100)
+
+       for _, p := range pages {
+               msg := qt.Commentf("w=%d", p.Weight())
+               b.Assert(pages.Next(p), qt.Equals, p.Next(), msg)
+               b.Assert(pages.Prev(p), qt.Equals, p.Prev(), msg)
+       }
+}
+
+func BenchmarkPagesPrevNext(b *testing.B) {
+       type Variant struct {
+               name         string
+               preparePages func(pages page.Pages) page.Pages
+               run          func(p page.Page, pages page.Pages)
+       }
+
+       shufflePages := func(pages page.Pages) page.Pages {
+               rand.Shuffle(len(pages), func(i, j int) { pages[i], pages[j] = pages[j], pages[i] })
+               return pages
+       }
+
+       for _, variant := range []Variant{
+               Variant{".Next", nil, func(p page.Page, pages page.Pages) { p.Next() }},
+               Variant{".Prev", nil, func(p page.Page, pages page.Pages) { p.Prev() }},
+               Variant{"Pages.Next", nil, func(p page.Page, pages page.Pages) { pages.Next(p) }},
+               Variant{"Pages.Prev", nil, func(p page.Page, pages page.Pages) { pages.Prev(p) }},
+               Variant{"Pages.Shuffled.Next", shufflePages, func(p page.Page, pages page.Pages) { pages.Next(p) }},
+               Variant{"Pages.Shuffled.Prev", shufflePages, func(p page.Page, pages page.Pages) { pages.Prev(p) }},
+               Variant{"Pages.ByTitle.Next", func(pages page.Pages) page.Pages { return pages.ByTitle() }, func(p page.Page, pages page.Pages) { pages.Next(p) }},
+       } {
+               for _, numPages := range []int{100, 300, 900, 5000} {
+                       b.Run(fmt.Sprintf("%s-pages-%d", variant.name, numPages), func(b *testing.B) {
+                               b.StopTimer()
+                               builder := newPagesPrevNextTestSite(b, numPages)
+                               builder.Build(BuildCfg{SkipRender: true})
+                               pages := builder.H.Sites[0].RegularPages()
+                               if variant.preparePages != nil {
+                                       pages = variant.preparePages(pages)
+                               }
+                               b.StartTimer()
+                               for i := 0; i < b.N; i++ {
+                                       p := pages[rand.Intn(len(pages))]
+                                       variant.run(p, pages)
+                               }
+                       })
+               }
+       }
+}

diff --git a/resources/page/pages_prev_next.go b/resources/page/pages_prev_next.go
index 9293c98746d3266d89a03f75a4510a2dc8291da5..dd87aa4ce8836d55bf0509e3667337e31ed8bf78 100644 (file)
--- a/resources/page/pages_prev_next.go
+++ b/resources/page/pages_prev_next.go
@@ -13,13 +13,12 @@
 
 package page
 
-// Prev returns the previous page reletive to the given
-func (p Pages) Prev(cur Page) Page {
+// Next returns the next page reletive to the given
+func (p Pages) Next(cur Page) Page {
        for x, c := range p {
                if c.Eq(cur) {
                        if x == 0 {
-                               // TODO(bep) consider return nil here to get it line with the other Prevs
-                               return p[len(p)-1]
+                               return nil
                        }
                        return p[x-1]
                }
@@ -27,15 +26,14 @@ func (p Pages) Prev(cur Page) Page {
        return nil
 }
 
-// Next returns the next page reletive to the given
-func (p Pages) Next(cur Page) Page {
+// Prev returns the previous page reletive to the given
+func (p Pages) Prev(cur Page) Page {
        for x, c := range p {
                if c.Eq(cur) {
                        if x < len(p)-1 {
                                return p[x+1]
                        }
-                       // TODO(bep) consider return nil here to get it line with the other Nexts
-                       return p[0]
+                       return nil
                }
        }
        return nil

diff --git a/resources/page/pages_prev_next_test.go b/resources/page/pages_prev_next_test.go
index a75335f003b365f8db2a027c5ac8a6d1ca0f22c2..689e102c2ee2091e4a3671e626892e524851700d 100644 (file)
--- a/resources/page/pages_prev_next_test.go
+++ b/resources/page/pages_prev_next_test.go
@@ -38,18 +38,20 @@ func TestPrev(t *testing.T) {
        t.Parallel()
        c := qt.New(t)
        pages := preparePageGroupTestPages(t)
-       c.Assert(pages[4], qt.Equals, pages.Prev(pages[0]))
-       c.Assert(pages[0], qt.Equals, pages.Prev(pages[1]))
-       c.Assert(pages[3], qt.Equals, pages.Prev(pages[4]))
+
+       c.Assert(pages.Prev(pages[3]), qt.Equals, pages[4])
+       c.Assert(pages.Prev(pages[1]), qt.Equals, pages[2])
+       c.Assert(pages.Prev(pages[4]), qt.IsNil)
 }
 
 func TestNext(t *testing.T) {
        t.Parallel()
        c := qt.New(t)
        pages := preparePageGroupTestPages(t)
-       c.Assert(pages[1], qt.Equals, pages.Next(pages[0]))
-       c.Assert(pages[2], qt.Equals, pages.Next(pages[1]))
-       c.Assert(pages[0], qt.Equals, pages.Next(pages[4]))
+
+       c.Assert(pages.Next(pages[0]), qt.IsNil)
+       c.Assert(pages.Next(pages[1]), qt.Equals, pages[0])
+       c.Assert(pages.Next(pages[4]), qt.Equals, pages[3])
 }
 
 func prepareWeightedPagesPrevNext(t *testing.T) WeightedPages {
@@ -72,16 +74,20 @@ func TestWeightedPagesPrev(t *testing.T) {
        t.Parallel()
        c := qt.New(t)
        w := prepareWeightedPagesPrevNext(t)
-       c.Assert(w[4].Page, qt.Equals, w.Prev(w[0].Page))
-       c.Assert(w[0].Page, qt.Equals, w.Prev(w[1].Page))
-       c.Assert(w[3].Page, qt.Equals, w.Prev(w[4].Page))
+
+       c.Assert(w.Prev(w[0].Page), qt.Equals, w[1].Page)
+       c.Assert(w.Prev(w[1].Page), qt.Equals, w[2].Page)
+       c.Assert(w.Prev(w[4].Page), qt.IsNil)
+
 }
 
 func TestWeightedPagesNext(t *testing.T) {
        t.Parallel()
        c := qt.New(t)
        w := prepareWeightedPagesPrevNext(t)
-       c.Assert(w[1].Page, qt.Equals, w.Next(w[0].Page))
-       c.Assert(w[2].Page, qt.Equals, w.Next(w[1].Page))
-       c.Assert(w[0].Page, qt.Equals, w.Next(w[4].Page))
+
+       c.Assert(w.Next(w[0].Page), qt.IsNil)
+       c.Assert(w.Next(w[1].Page), qt.Equals, w[0].Page)
+       c.Assert(w.Next(w[4].Page), qt.Equals, w[3].Page)
+
 }

diff --git a/resources/page/weighted.go b/resources/page/weighted.go
index 48ed736ce0f481f8c753b1fd101cd47c61386447..b2fc8b67b3f58a6e9e7813a68a90374293c340df 100644 (file)
--- a/resources/page/weighted.go
+++ b/resources/page/weighted.go
@@ -95,13 +95,13 @@ func (wp WeightedPages) Pages() Pages {
        return pages
 }
 
-// Prev returns the previous Page relative to the given Page in
+// Next returns the next Page relative to the given Page in
 // this weighted page set.
-func (wp WeightedPages) Prev(cur Page) Page {
+func (wp WeightedPages) Next(cur Page) Page {
        for x, c := range wp {
                if c.Page == cur {
                        if x == 0 {
-                               return wp[len(wp)-1].Page
+                               return nil
                        }
                        return wp[x-1].Page
                }
@@ -109,15 +109,15 @@ func (wp WeightedPages) Prev(cur Page) Page {
        return nil
 }
 
-// Next returns the next Page relative to the given Page in
+// Prev returns the previous Page relative to the given Page in
 // this weighted page set.
-func (wp WeightedPages) Next(cur Page) Page {
+func (wp WeightedPages) Prev(cur Page) Page {
        for x, c := range wp {
                if c.Page == cur {
                        if x < len(wp)-1 {
                                return wp[x+1].Page
                        }
-                       return wp[0].Page
+                       return nil
                }
        }
        return nil