Skip to content

Commit

Permalink
Fix some change detection issues on server reloads
Browse files Browse the repository at this point in the history
* Fix change detection when .GetPage/site.GetPage is used from shortcode
* Fix stale content for GetPage results with short name lookups on server reloads

Fixes gohugoio#7623
Fixes gohugoio#7624
  • Loading branch information
bep committed Sep 7, 2020
1 parent 3ba7c92 commit d89b3a0
Show file tree
Hide file tree
Showing 12 changed files with 175 additions and 28 deletions.
4 changes: 4 additions & 0 deletions deps/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ type Deps struct {
// This is common/global for all sites.
BuildState *BuildState

// Whether we are in running (server) mode
Running bool

*globalErrHandler
}

Expand Down Expand Up @@ -279,6 +282,7 @@ func New(cfg DepsCfg) (*Deps, error) {
FileCaches: fileCaches,
BuildStartListeners: &Listeners{},
BuildState: buildState,
Running: cfg.Running,
Timeout: time.Duration(timeoutms) * time.Millisecond,
globalErrHandler: errorHandler,
}
Expand Down
41 changes: 26 additions & 15 deletions hugolib/content_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,23 @@ func newContentMap(cfg contentMapConfig) *contentMap {

m.pageReverseIndex = &contentTreeReverseIndex{
t: []*contentTree{m.pages, m.sections, m.taxonomies},
initFn: func(t *contentTree, m map[interface{}]*contentNode) {
t.Walk(func(s string, v interface{}) bool {
n := v.(*contentNode)
if n.p != nil && !n.p.File().IsZero() {
meta := n.p.File().FileInfo().Meta()
if meta.Path() != meta.PathFile() {
// Keep track of the original mount source.
mountKey := filepath.ToSlash(filepath.Join(meta.Module(), meta.PathFile()))
addToReverseMap(mountKey, n, m)
contentTreeReverseIndexMap: &contentTreeReverseIndexMap{
initFn: func(t *contentTree, m map[interface{}]*contentNode) {
t.Walk(func(s string, v interface{}) bool {
n := v.(*contentNode)
if n.p != nil && !n.p.File().IsZero() {
meta := n.p.File().FileInfo().Meta()
if meta.Path() != meta.PathFile() {
// Keep track of the original mount source.
mountKey := filepath.ToSlash(filepath.Join(meta.Module(), meta.PathFile()))
addToReverseMap(mountKey, n, m)
}
}
}
k := strings.TrimPrefix(strings.TrimSuffix(path.Base(s), cmLeafSeparator), cmBranchSeparator)
addToReverseMap(k, n, m)
return false
})
k := strings.TrimPrefix(strings.TrimSuffix(path.Base(s), cmLeafSeparator), cmBranchSeparator)
addToReverseMap(k, n, m)
return false
})
},
},
}

Expand Down Expand Up @@ -1030,12 +1032,21 @@ func (c *contentTreeRef) getSections() page.Pages {

type contentTreeReverseIndex struct {
t []*contentTree
m map[interface{}]*contentNode
*contentTreeReverseIndexMap
}

type contentTreeReverseIndexMap struct {
m map[interface{}]*contentNode
init sync.Once
initFn func(*contentTree, map[interface{}]*contentNode)
}

func (c *contentTreeReverseIndex) Reset() {
c.contentTreeReverseIndexMap = &contentTreeReverseIndexMap{
initFn: c.initFn,
}
}

func (c *contentTreeReverseIndex) Get(key interface{}) *contentNode {
c.init.Do(func() {
c.m = make(map[interface{}]*contentNode)
Expand Down
1 change: 0 additions & 1 deletion hugolib/hugo_sites.go
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,6 @@ func (h *HugoSites) resetPageStateFromEvents(idset identity.Identities) {
}
return false
})

}

// Used in partial reloading to determine if the change is in a bundle.
Expand Down
63 changes: 63 additions & 0 deletions hugolib/hugo_sites_rebuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,3 +259,66 @@ Output Shortcode AMP Edited
})

}

// Issues #7623 #7625
func TestSitesRebuildOnFilesIncludedWithGetPage(t *testing.T) {
b := newTestSitesBuilder(t).Running()
b.WithContent("pages/p1.md", `---
title: p1
---
P3: {{< GetPage "pages/p3" >}}
`)

b.WithContent("pages/p2.md", `---
title: p2
---
P4: {{< site_GetPage "pages/p4" >}}
P5: {{< site_GetPage "p5" >}}
P6: {{< dot_site_GetPage "p6" >}}
`)

b.WithContent("pages/p3/index.md", "---\ntitle: p3\nheadless: true\n---\nP3 content")
b.WithContent("pages/p4/index.md", "---\ntitle: p4\nheadless: true\n---\nP4 content")
b.WithContent("pages/p5.md", "---\ntitle: p5\n---\nP5 content")
b.WithContent("pages/p6.md", "---\ntitle: p6\n---\nP6 content")

b.WithTemplates(
"_default/single.html", `{{ .Content }}`,
"shortcodes/GetPage.html", `
{{ $arg := .Get 0 }}
{{ $p := .Page.GetPage $arg }}
{{ $p.Content }}
`,
"shortcodes/site_GetPage.html", `
{{ $arg := .Get 0 }}
{{ $p := site.GetPage $arg }}
{{ $p.Content }}
`, "shortcodes/dot_site_GetPage.html", `
{{ $arg := .Get 0 }}
{{ $p := .Site.GetPage $arg }}
{{ $p.Content }}
`,
)

b.Build(BuildCfg{})

b.AssertFileContent("public/pages/p1/index.html", "P3 content")
b.AssertFileContent("public/pages/p2/index.html", `P4 content
P5 content
P6 content
`)

b.EditFiles("content/pages/p3/index.md", "---\ntitle: p3\n---\nP3 changed content")
b.EditFiles("content/pages/p4/index.md", "---\ntitle: p4\n---\nP4 changed content")
b.EditFiles("content/pages/p5.md", "---\ntitle: p5\n---\nP5 changed content")
b.EditFiles("content/pages/p6.md", "---\ntitle: p6\n---\nP6 changed content")

b.Build(BuildCfg{})

b.AssertFileContent("public/pages/p1/index.html", "P3 changed content")
b.AssertFileContent("public/pages/p2/index.html", `P4 changed content
P5 changed content
P6 changed content
`)

}
17 changes: 17 additions & 0 deletions hugolib/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ type pageContext interface {
posOffset(offset int) text.Position
wrapError(err error) error
getContentConverter() converter.Converter
addDependency(dep identity.Provider)
}

// wrapErr adds some context to the given error if possible.
Expand All @@ -93,6 +94,18 @@ type pageSiteAdapter struct {
s *Site
}

func (pa pageSiteAdapter) GetPageWithTemplateInfo(info tpl.Info, ref string) (page.Page, error) {
p, err := pa.GetPage(ref)
if p != nil {
// Track pages referenced by templates/shortcodes
// when in server mode.
if im, ok := info.(identity.Manager); ok {
im.Add(p)
}
}
return p, err
}

func (pa pageSiteAdapter) GetPage(ref string) (page.Page, error) {
p, err := pa.s.getPageNew(pa.p, ref)
if p == nil {
Expand Down Expand Up @@ -127,6 +140,10 @@ func (p *pageState) Eq(other interface{}) bool {
return p == pp
}

func (p *pageState) GetIdentity() identity.Identity {
return identity.NewPathIdentity(files.ComponentFolderContent, filepath.FromSlash(p.Path()))
}

func (p *pageState) GitInfo() *gitmap.GitInfo {
return p.gitInfo
}
Expand Down
13 changes: 13 additions & 0 deletions hugolib/site.go
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,7 @@ func (s *Site) resetBuildState(sourceChanged bool) {
s.init.Reset()

if sourceChanged {
s.pageMap.contentMap.pageReverseIndex.Reset()
s.PageCollections = newPageCollections(s.pageMap)
s.pageMap.withEveryBundlePage(func(p *pageState) bool {
p.pagePages = &pagePages{}
Expand Down Expand Up @@ -1587,6 +1588,18 @@ func (s *SiteInfo) GetPage(ref ...string) (page.Page, error) {
return p, err
}

func (s *SiteInfo) GetPageWithTemplateInfo(info tpl.Info, ref ...string) (page.Page, error) {
p, err := s.GetPage(ref...)
if p != nil {
// Track pages referenced by templates/shortcodes
// when in server mode.
if im, ok := info.(identity.Manager); ok {
im.Add(p)
}
}
return p, err
}

func (s *Site) permalink(link string) string {
return s.PathSpec.PermalinkForBaseURL(link, s.PathSpec.BaseURL.String())
}
Expand Down
2 changes: 1 addition & 1 deletion hugolib/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ title: P1

idset := make(map[identity.Identity]bool)
collectIdentities(idset, templ.(tpl.Info))
b.Assert(idset, qt.HasLen, 10)
b.Assert(idset, qt.HasLen, 11)

}

Expand Down
9 changes: 9 additions & 0 deletions resources/page/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package page
import (
"html/template"

"github.com/gohugoio/hugo/identity"

"github.com/bep/gitmap"
"github.com/gohugoio/hugo/config"
"github.com/gohugoio/hugo/tpl"

"github.com/gohugoio/hugo/common/hugo"
"github.com/gohugoio/hugo/common/maps"
Expand Down Expand Up @@ -97,6 +100,9 @@ type GetPageProvider interface {
// This will return nil when no page could be found, and will return
// an error if the ref is ambiguous.
GetPage(ref string) (Page, error)

// GetPageWithTemplateInfo is for internal use only.
GetPageWithTemplateInfo(info tpl.Info, ref string) (Page, error)
}

// GitInfoProvider provides Git info.
Expand Down Expand Up @@ -260,6 +266,9 @@ type PageWithoutContent interface {
// e.g. GetTerms("categories")
GetTerms(taxonomy string) Pages

// Used in change/dependency tracking.
identity.Provider

DeprecatedWarningPageMethods
}

Expand Down
11 changes: 11 additions & 0 deletions resources/page/page_nop.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import (
"html/template"
"time"

"github.com/gohugoio/hugo/identity"

"github.com/gohugoio/hugo/hugofs/files"
"github.com/gohugoio/hugo/tpl"

"github.com/gohugoio/hugo/hugofs"

Expand Down Expand Up @@ -170,6 +173,10 @@ func (p *nopPage) GetPage(ref string) (Page, error) {
return nil, nil
}

func (p *nopPage) GetPageWithTemplateInfo(info tpl.Info, ref string) (Page, error) {
return nil, nil
}

func (p *nopPage) GetParam(key string) interface{} {
return nil
}
Expand Down Expand Up @@ -484,3 +491,7 @@ func (p *nopPage) Weight() int {
func (p *nopPage) WordCount() int {
return 0
}

func (p *nopPage) GetIdentity() identity.Identity {
return identity.NewPathIdentity("content", "foo/bar.md")
}
10 changes: 10 additions & 0 deletions resources/page/testhelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"time"

"github.com/gohugoio/hugo/hugofs/files"
"github.com/gohugoio/hugo/identity"
"github.com/gohugoio/hugo/tpl"

"github.com/gohugoio/hugo/modules"

Expand Down Expand Up @@ -218,6 +220,10 @@ func (p *testPage) GetPage(ref string) (Page, error) {
panic("not implemented")
}

func (p *testPage) GetPageWithTemplateInfo(info tpl.Info, ref string) (Page, error) {
panic("not implemented")
}

func (p *testPage) GetParam(key string) interface{} {
panic("not implemented")
}
Expand Down Expand Up @@ -565,6 +571,10 @@ func (p *testPage) WordCount() int {
panic("not implemented")
}

func (p *testPage) GetIdentity() identity.Identity {
panic("not implemented")
}

func createTestPages(num int) Pages {
pages := make(Pages, num)

Expand Down
9 changes: 8 additions & 1 deletion tpl/fmt/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,17 @@ import (

// New returns a new instance of the fmt-namespaced template functions.
func New(d *deps.Deps) *Namespace {
return &Namespace{
ns := &Namespace{
errorLogger: helpers.NewDistinctLogger(d.Log.ERROR),
warnLogger: helpers.NewDistinctLogger(d.Log.WARN),
}

d.BuildStartListeners.Add(func() {
ns.errorLogger.Reset()
ns.warnLogger.Reset()
})

return ns
}

// Namespace provides template functions for the "fmt" namespace.
Expand Down
23 changes: 13 additions & 10 deletions tpl/tplimpl/template_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ var _ texttemplate.ExecHelper = (*templateExecHelper)(nil)
var zero reflect.Value

type templateExecHelper struct {
funcs map[string]reflect.Value
running bool // whether we're in server mode.
funcs map[string]reflect.Value
}

func (t *templateExecHelper) GetFunc(tmpl texttemplate.Preparer, name string) (reflect.Value, bool) {
Expand All @@ -91,14 +92,15 @@ func (t *templateExecHelper) GetMapValue(tmpl texttemplate.Preparer, receiver, k
}

func (t *templateExecHelper) GetMethod(tmpl texttemplate.Preparer, receiver reflect.Value, name string) (method reflect.Value, firstArg reflect.Value) {
// This is a hot path and receiver.MethodByName really shows up in the benchmarks.
// Page.Render is the only method with a WithTemplateInfo as of now, so let's just
// check that for now.
// TODO(bep) find a more flexible, but still fast, way.
if name == "Render" {
if info, ok := tmpl.(tpl.Info); ok {
if m := receiver.MethodByName(name + "WithTemplateInfo"); m.IsValid() {
return m, reflect.ValueOf(info)
if t.running {
// This is a hot path and receiver.MethodByName really shows up in the benchmarks,
// so we maintain a list of method names with that signature.
switch name {
case "GetPage", "Render":
if info, ok := tmpl.(tpl.Info); ok {
if m := receiver.MethodByName(name + "WithTemplateInfo"); m.IsValid() {
return m, reflect.ValueOf(info)
}
}
}
}
Expand Down Expand Up @@ -133,7 +135,8 @@ func newTemplateExecuter(d *deps.Deps) (texttemplate.Executer, map[string]reflec
}

exeHelper := &templateExecHelper{
funcs: funcsv,
running: d.Running,
funcs: funcsv,
}

return texttemplate.NewExecuter(
Expand Down

0 comments on commit d89b3a0

Please sign in to comment.