1
0
Fork 0
mirror of https://github.com/gohugoio/hugo.git synced 2025-03-20 20:41:10 +00:00

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 
This commit is contained in:
bep 2015-03-31 21:33:24 +01:00
parent bec22f8981
commit bec4bdae99
4 changed files with 170 additions and 37 deletions

View file

@ -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.

View file

@ -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,18 +263,9 @@ 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
switch seq.(type) {
case Pages:
pages = seq.(Pages)
case *Pages:
pages = *(seq.(*Pages))
case WeightedPages:
pages = (seq.(WeightedPages)).Pages()
case PageGroup:
pages = (seq.(PageGroup)).Pages
default:
return nil, errors.New(fmt.Sprintf("unsupported type in paginate, got %T", seq))
pages, err := toPages(seq)
if err != nil {
return nil, err
}
urlFactory := newPaginationURLFactory(section)
@ -269,6 +275,54 @@ func paginatePages(seq interface{}, pagerSize int, section string) (pagers, erro
return pagers, nil
}
func toPages(seq interface{}) (Pages, error) {
switch seq.(type) {
case Pages:
return seq.(Pages), nil
case *Pages:
return *(seq.(*Pages)), nil
case WeightedPages:
return (seq.(WeightedPages)).Pages(), nil
case PageGroup:
return (seq.(PageGroup)).Pages, nil
default:
return nil, errors.New(fmt.Sprintf("unsupported type in paginate, got %T", seq))
}
}
// 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 {
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) {
if size <= 0 {

View file

@ -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)

View file

@ -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)
}