ref: bec4bdae992841f011239dac8c685e13470a90f3
parent: bec22f8981c251f88594689c65ad7b8822fe1b09
author: bep <[email protected]>
date: Tue Mar 31 17:33:24 EDT 2015
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
--- a/helpers/general.go
+++ b/helpers/general.go
@@ -153,26 +153,37 @@
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.
--- a/hugolib/pagination.go
+++ b/hugolib/pagination.go
@@ -22,6 +22,7 @@
"html/template"
"math"
"path"
+ "reflect"
)
type pager struct {@@ -37,8 +38,10 @@
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 @@
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 @@
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 @@
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 @@
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) {--- a/hugolib/pagination_test.go
+++ b/hugolib/pagination_test.go
@@ -184,7 +184,7 @@
n1 := s.newHomeNode()
n2 := s.newHomeNode()
- var paginator1 *pager
+ var paginator1, paginator2 *pager
var err error
if useViper {@@ -199,13 +199,14 @@
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)
@@ -238,6 +239,71 @@
_, err := paginatePages(Site{}, 11, "t")assert.NotNil(t, err)
+}
+
+// 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 {--- a/hugolib/site.go
+++ b/hugolib/site.go
@@ -48,6 +48,8 @@
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 @@
}
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 @@
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 @@
}
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 @@
}
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 @@
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)
}