Return error on wrong use of the Paginator - hugo - [fork] hugo port for 9front
HTML git clone git@git.drkhsh.at/hugo.git
DIR Log
DIR Files
DIR Refs
DIR Submodules
DIR README
DIR LICENSE
---
DIR commit bec4bdae992841f011239dac8c685e13470a90f3
DIR parent bec22f8981c251f88594689c65ad7b8822fe1b09
HTML Author: bep <bjorn.erik.pedersen@gmail.com>
Date: Tue, 31 Mar 2015 21:33:24 +0100
Return error on wrong use of the Paginator
`Paginate`now returns error when
1) `.Paginate` is called after `.Paginator`
2) `.Paginate` is repeatedly called with different arguments
This should help remove some confusion.
This commit also introduces DistinctErrorLogger, to prevent spamming the log for duplicate rendering errors from the pagers.
Fixes #993
Diffstat:
M helpers/general.go | 37 ++++++++++++++++++++-----------
M hugolib/pagination.go | 76 ++++++++++++++++++++++++++-----
M hugolib/pagination_test.go | 76 +++++++++++++++++++++++++++++--
M hugolib/site.go | 12 +++++++-----
4 files changed, 167 insertions(+), 34 deletions(-)
---
DIR diff --git a/helpers/general.go b/helpers/general.go
@@ -153,26 +153,37 @@ func ThemeSet() bool {
return viper.GetString("theme") != ""
}
-// Avoid spamming the logs with errors
-var deprecatedLogs = struct {
+// DistinctErrorLogger ignores duplicate log statements.
+type DistinctErrorLogger struct {
sync.RWMutex
m map[string]bool
-}{m: make(map[string]bool)}
+}
-func Deprecated(object, item, alternative string) {
- key := object + item + alternative
- deprecatedLogs.RLock()
- logged := deprecatedLogs.m[key]
- deprecatedLogs.RUnlock()
+func (l *DistinctErrorLogger) Printf(format string, v ...interface{}) {
+ logStatement := fmt.Sprintf(format, v...)
+ l.RLock()
+ logged := l.m[logStatement]
+ l.RUnlock()
if logged {
return
}
- deprecatedLogs.Lock()
- if !deprecatedLogs.m[key] {
- jww.ERROR.Printf("%s's %s is deprecated and will be removed in Hugo %s. Use %s instead.", object, item, NextHugoReleaseVersion(), alternative)
- deprecatedLogs.m[key] = true
+ l.Lock()
+ if !l.m[logStatement] {
+ jww.ERROR.Print(logStatement)
+ l.m[logStatement] = true
}
- deprecatedLogs.Unlock()
+ l.Unlock()
+}
+
+func NewDistinctErrorLogger() *DistinctErrorLogger {
+ return &DistinctErrorLogger{m: make(map[string]bool)}
+}
+
+// Avoid spamming the logs with errors
+var deprecatedLogger = NewDistinctErrorLogger()
+
+func Deprecated(object, item, alternative string) {
+ deprecatedLogger.Printf("%s's %s is deprecated and will be removed in Hugo %s. Use %s instead.", object, item, NextHugoReleaseVersion(), alternative)
}
// SliceToLower goes through the source slice and lowers all values.
DIR diff --git a/hugolib/pagination.go b/hugolib/pagination.go
@@ -22,6 +22,7 @@ import (
"html/template"
"math"
"path"
+ "reflect"
)
type pager struct {
@@ -37,8 +38,10 @@ type paginator struct {
paginatedPages []Pages
pagers
paginationURLFactory
- total int
- size int
+ total int
+ size int
+ source interface{}
+ options []interface{}
}
type paginationURLFactory func(int) string
@@ -164,6 +167,8 @@ func (n *Node) Paginator(options ...interface{}) (*pager, error) {
if len(pagers) > 0 {
// the rest of the nodes will be created later
n.paginator = pagers[0]
+ n.paginator.source = "paginator"
+ n.paginator.options = options
n.Site.addToPaginationPageCount(uint64(n.paginator.TotalPages()))
}
@@ -212,6 +217,8 @@ func (n *Node) Paginate(seq interface{}, options ...interface{}) (*pager, error)
if len(pagers) > 0 {
// the rest of the nodes will be created later
n.paginator = pagers[0]
+ n.paginator.source = seq
+ n.paginator.options = options
n.Site.addToPaginationPageCount(uint64(n.paginator.TotalPages()))
}
@@ -221,6 +228,14 @@ func (n *Node) Paginate(seq interface{}, options ...interface{}) (*pager, error)
return nil, initError
}
+ if n.paginator.source == "paginator" {
+ return nil, errors.New("a Paginator was previously built for this Node without filters; look for earlier .Paginator usage")
+ } else {
+ if !reflect.DeepEqual(options, n.paginator.options) || !probablyEqualPageLists(n.paginator.source, seq) {
+ return nil, errors.New("invoked multiple times with different arguments")
+ }
+ }
+
return n.paginator, nil
}
@@ -248,25 +263,64 @@ func paginatePages(seq interface{}, pagerSize int, section string) (pagers, erro
return nil, errors.New("'paginate' configuration setting must be positive to paginate")
}
- var pages Pages
+ pages, err := toPages(seq)
+ if err != nil {
+ return nil, err
+ }
+
+ urlFactory := newPaginationURLFactory(section)
+ paginator, _ := newPaginator(pages, pagerSize, urlFactory)
+ pagers := paginator.Pagers()
+
+ return pagers, nil
+}
+
+func toPages(seq interface{}) (Pages, error) {
switch seq.(type) {
case Pages:
- pages = seq.(Pages)
+ return seq.(Pages), nil
case *Pages:
- pages = *(seq.(*Pages))
+ return *(seq.(*Pages)), nil
case WeightedPages:
- pages = (seq.(WeightedPages)).Pages()
+ return (seq.(WeightedPages)).Pages(), nil
case PageGroup:
- pages = (seq.(PageGroup)).Pages
+ return (seq.(PageGroup)).Pages, nil
default:
return nil, errors.New(fmt.Sprintf("unsupported type in paginate, got %T", seq))
}
+}
- urlFactory := newPaginationURLFactory(section)
- paginator, _ := newPaginator(pages, pagerSize, urlFactory)
- pagers := paginator.Pagers()
+// probablyEqual checks page lists for probable equality.
+// It may return false positives.
+// The motivation behind this is to avoid potential costly reflect.DeepEqual
+// when "probably" is good enough.
+func probablyEqualPageLists(a1 interface{}, a2 interface{}) bool {
- return pagers, nil
+ if a1 == nil || a2 == nil {
+ return a1 == a2
+ }
+
+ p1, err1 := toPages(a1)
+ p2, err2 := toPages(a2)
+
+ // probably the same wrong type
+ if err1 != nil && err2 != nil {
+ return true
+ }
+
+ if err1 != nil || err2 != nil {
+ return false
+ }
+
+ if len(p1) != len(p2) {
+ return false
+ }
+
+ if len(p1) == 0 {
+ return true
+ }
+
+ return p1[0] == p2[0]
}
func newPaginator(pages Pages, size int, urlFactory paginationURLFactory) (*paginator, error) {
DIR diff --git a/hugolib/pagination_test.go b/hugolib/pagination_test.go
@@ -184,7 +184,7 @@ func doTestPaginate(t *testing.T, useViper bool) {
n1 := s.newHomeNode()
n2 := s.newHomeNode()
- var paginator1 *pager
+ var paginator1, paginator2 *pager
var err error
if useViper {
@@ -199,13 +199,14 @@ func doTestPaginate(t *testing.T, useViper bool) {
assert.Equal(t, 6, paginator1.TotalNumberOfElements())
n2.paginator = paginator1.Next()
- paginator2, err := n2.Paginate(pages)
+ if useViper {
+ paginator2, err = n2.Paginate(pages)
+ } else {
+ paginator2, err = n2.Paginate(pages, pagerSize)
+ }
assert.Nil(t, err)
assert.Equal(t, paginator2, paginator1.Next())
- samePaginator, err := n1.Paginate(createTestPages(2))
- assert.Equal(t, paginator1, samePaginator)
-
p, _ := NewPage("test")
_, err = p.Paginate(pages)
assert.NotNil(t, err)
@@ -240,6 +241,71 @@ func TestPaginatePages(t *testing.T) {
}
+// Issue #993
+func TestPaginatorFollowedByPaginateShouldFail(t *testing.T) {
+ viper.Set("paginate", 10)
+ s := &Site{}
+ n1 := s.newHomeNode()
+ n2 := s.newHomeNode()
+
+ _, err := n1.Paginator()
+ assert.Nil(t, err)
+ _, err = n1.Paginate(createTestPages(2))
+ assert.NotNil(t, err)
+
+ _, err = n2.Paginate(createTestPages(2))
+ assert.Nil(t, err)
+
+}
+
+func TestPaginateFollowedByDifferentPaginateShouldFail(t *testing.T) {
+
+ viper.Set("paginate", 10)
+ s := &Site{}
+ n1 := s.newHomeNode()
+ n2 := s.newHomeNode()
+
+ p1 := createTestPages(2)
+ p2 := createTestPages(10)
+
+ _, err := n1.Paginate(p1)
+ assert.Nil(t, err)
+
+ _, err = n1.Paginate(p1)
+ assert.Nil(t, err)
+
+ _, err = n1.Paginate(p2)
+ assert.NotNil(t, err)
+
+ _, err = n2.Paginate(p2)
+ assert.Nil(t, err)
+}
+
+func TestProbablyEqualPageLists(t *testing.T) {
+ fivePages := createTestPages(5)
+ zeroPages := createTestPages(0)
+ for i, this := range []struct {
+ v1 interface{}
+ v2 interface{}
+ expect bool
+ }{
+ {nil, nil, true},
+ {"a", "b", true},
+ {"a", fivePages, false},
+ {fivePages, "a", false},
+ {fivePages, createTestPages(2), false},
+ {fivePages, fivePages, true},
+ {zeroPages, zeroPages, true},
+ } {
+ result := probablyEqualPageLists(this.v1, this.v2)
+
+ if result != this.expect {
+ t.Errorf("[%d] Got %t but expected %t", i, result, this.expect)
+
+ }
+ }
+}
+
func createTestPages(num int) Pages {
pages := make(Pages, num)
DIR diff --git a/hugolib/site.go b/hugolib/site.go
@@ -48,6 +48,8 @@ var _ = transform.AbsURL
var DefaultTimer *nitro.B
+var distinctErrorLogger = helpers.NewDistinctErrorLogger()
+
// Site contains all the information relevant for constructing a static
// site. The basic flow of information is as follows:
//
@@ -1086,7 +1088,7 @@ func taxonomyRenderer(s *Site, taxes <-chan taxRenderInfo, results chan<- error,
}
pageNumber := i + 1
htmlBase := fmt.Sprintf("/%s/%s/%d", base, paginatePath, pageNumber)
- if err := s.renderAndWritePage(fmt.Sprintf("taxononomy_%s_%d", t.singular, pageNumber), htmlBase, taxonomyPagerNode, layouts...); err != nil {
+ if err := s.renderAndWritePage(fmt.Sprintf("taxononomy %s", t.singular), htmlBase, taxonomyPagerNode, layouts...); err != nil {
results <- err
continue
}
@@ -1155,7 +1157,7 @@ func (s *Site) RenderSectionLists() error {
n := s.newSectionListNode(section, data)
- if err := s.renderAndWritePage(fmt.Sprintf("section%s_%d", section, 1), fmt.Sprintf("/%s", section), n, s.appendThemeTemplates(layouts)...); err != nil {
+ if err := s.renderAndWritePage(fmt.Sprintf("section %s", section), fmt.Sprintf("/%s", section), n, s.appendThemeTemplates(layouts)...); err != nil {
return err
}
@@ -1181,7 +1183,7 @@ func (s *Site) RenderSectionLists() error {
}
pageNumber := i + 1
htmlBase := fmt.Sprintf("/%s/%s/%d", section, paginatePath, pageNumber)
- if err := s.renderAndWritePage(fmt.Sprintf("section_%s_%d", section, pageNumber), filepath.FromSlash(htmlBase), sectionPagerNode, layouts...); err != nil {
+ if err := s.renderAndWritePage(fmt.Sprintf("section %s", section), filepath.FromSlash(htmlBase), sectionPagerNode, layouts...); err != nil {
return err
}
}
@@ -1238,7 +1240,7 @@ func (s *Site) RenderHomePage() error {
}
pageNumber := i + 1
htmlBase := fmt.Sprintf("/%s/%d", paginatePath, pageNumber)
- if err := s.renderAndWritePage(fmt.Sprintf("homepage_%d", pageNumber), filepath.FromSlash(htmlBase), homePagerNode, layouts...); err != nil {
+ if err := s.renderAndWritePage(fmt.Sprintf("homepage"), filepath.FromSlash(htmlBase), homePagerNode, layouts...); err != nil {
return err
}
}
@@ -1424,7 +1426,7 @@ func (s *Site) render(name string, d interface{}, renderBuffer *bytes.Buffer, la
if err := s.renderThing(d, layout, renderBuffer); err != nil {
// Behavior here should be dependent on if running in server or watch mode.
- jww.ERROR.Println(fmt.Errorf("Error while rendering %s: %v", name, err))
+ distinctErrorLogger.Printf("Error while rendering %s: %v", name, err)
if !s.Running() {
os.Exit(-1)
}