diff --git a/helpers/general.go b/helpers/general.go index 14cce5cf0..cf87c04b1 100644 --- 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. diff --git a/hugolib/pagination.go b/hugolib/pagination.go index 650a355cf..f5380a337 100644 --- 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,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 { diff --git a/hugolib/pagination_test.go b/hugolib/pagination_test.go index fde81dfb2..45655ef94 100644 --- 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) diff --git a/hugolib/site.go b/hugolib/site.go index e05054847..4f3fd1a81 100644 --- 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) }