zerica
/
hugo
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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
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
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
helpers/general.go
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.

82
hugolib/pagination.go
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 {

76
hugolib/pagination_test.go
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)

12
hugolib/site.go
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)
}