Fix some change detection issues on server reloads

* 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 #7623
Fixes #7624
Fixes #7625

diff --git a/deps/deps.go b/deps/deps.go
index 82a16ba5947de33b726197ac682e61ba181715f0..07fe2fc7d4cb69cb2f8b168bf230a28ace3e0662 100644 (file)
--- a/deps/deps.go
+++ b/deps/deps.go
@@ -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
 }
 
@@ -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,
        }

diff --git a/hugolib/content_map.go b/hugolib/content_map.go
index 43ad7745d8b3ae55993ff1f28b9abcf86a3f8f5e..33ef4f8dd99ec07a6c20baa39452bb9043af8bdb 100644 (file)
--- a/hugolib/content_map.go
+++ b/hugolib/content_map.go
@@ -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
+                               })
+                       },
                },
        }
 
@@ -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)

diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go
index d1e3a146d6d1c8ee46eb8c3115e51c209a250f3b..702e51abbe2970e68d75029fb553313f29e04135 100644 (file)
--- a/hugolib/hugo_sites.go
+++ b/hugolib/hugo_sites.go
@@ -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.

diff --git a/hugolib/hugo_sites_rebuild_test.go b/hugolib/hugo_sites_rebuild_test.go
index f0c9f8f09e508bd3f39b0e9af076490e278f1e98..4c47413856626ac47fe27cb57cdc7a9c3488dbfc 100644 (file)
--- a/hugolib/hugo_sites_rebuild_test.go
+++ b/hugolib/hugo_sites_rebuild_test.go
@@ -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
+`)
+
+}

diff --git a/hugolib/page.go b/hugolib/page.go
index 28ef1e156ab9442b49f4743553f13c4b9cf4215d..ca93bf18b9d21134f639b9e594917fa0d8d5ab27 100644 (file)
--- a/hugolib/page.go
+++ b/hugolib/page.go
@@ -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.
@@ -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 {
@@ -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
 }

diff --git a/hugolib/site.go b/hugolib/site.go
index ac65931d0c8dcf9e6d21e9621509142bd944009d..dad5ab538d71a2fffca24a2a69d59c532be75175 100644 (file)
--- a/hugolib/site.go
+++ b/hugolib/site.go
@@ -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{}
@@ -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())
 }

diff --git a/hugolib/template_test.go b/hugolib/template_test.go
index 6f37b416485a29ffc80848b6dbfdb59f7c7dc945..39f1b9e2eef0ab4bbf5009a6d307c9456a95b886 100644 (file)
--- a/hugolib/template_test.go
+++ b/hugolib/template_test.go
@@ -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)
 
 }
 

diff --git a/identity/identity.go b/identity/identity.go
index ac3558d16916acc64b70f6abb0418fb35f221374..8fce16479bbb0ea62f25ebc4e4883f2447609444 100644 (file)
--- a/identity/identity.go
+++ b/identity/identity.go
@@ -129,6 +129,8 @@ func (im *identityManager) Reset() {
        im.Unlock()
 }
 
+// TODO(bep) these identities are currently only read on server reloads
+// so there should be no concurrency issues, but that may change.
 func (im *identityManager) GetIdentities() Identities {
        im.Lock()
        defer im.Unlock()

diff --git a/identity/identity_test.go b/identity/identity_test.go
index adebcad916ff11f2c373ec37aa2d8631a653e847..c315df1e720f131cc5a2d84ff2978da5faf6bff4 100644 (file)
--- a/identity/identity_test.go
+++ b/identity/identity_test.go
@@ -14,6 +14,9 @@
 package identity
 
 import (
+       "fmt"
+       "math/rand"
+       "strconv"
        "testing"
 
        qt "github.com/frankban/quicktest"
@@ -29,6 +32,54 @@ func TestIdentityManager(t *testing.T) {
        c.Assert(im.Search(testIdentity{name: "notfound"}), qt.Equals, nil)
 }
 
+func BenchmarkIdentityManager(b *testing.B) {
+
+       createIds := func(num int) []Identity {
+               ids := make([]Identity, num)
+               for i := 0; i < num; i++ {
+                       ids[i] = testIdentity{name: fmt.Sprintf("id%d", i)}
+               }
+               return ids
+
+       }
+
+       b.Run("Add", func(b *testing.B) {
+               c := qt.New(b)
+               b.StopTimer()
+               ids := createIds(b.N)
+               im := NewManager(testIdentity{"first"})
+               b.StartTimer()
+
+               for i := 0; i < b.N; i++ {
+                       im.Add(ids[i])
+               }
+
+               b.StopTimer()
+               c.Assert(im.GetIdentities(), qt.HasLen, b.N+1)
+       })
+
+       b.Run("Search", func(b *testing.B) {
+               c := qt.New(b)
+               b.StopTimer()
+               ids := createIds(b.N)
+               im := NewManager(testIdentity{"first"})
+
+               for i := 0; i < b.N; i++ {
+                       im.Add(ids[i])
+               }
+
+               b.StartTimer()
+
+               for i := 0; i < b.N; i++ {
+                       name := "id" + strconv.Itoa(rand.Intn(b.N))
+                       id := im.Search(testIdentity{name: name})
+                       c.Assert(id.GetIdentity().Name(), qt.Equals, name)
+               }
+
+       })
+
+}
+
 type testIdentity struct {
        name string
 }

diff --git a/resources/page/page.go b/resources/page/page.go
index de417178f045c51b82fc006cbb351faa7e20bd9c..0b402c4e78fc3a2e24db6049f2107df00db38f49 100644 (file)
--- a/resources/page/page.go
+++ b/resources/page/page.go
@@ -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"
@@ -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.
@@ -260,6 +266,9 @@ type PageWithoutContent interface {
        // e.g. GetTerms("categories")
        GetTerms(taxonomy string) Pages
 
+       // Used in change/dependency tracking.
+       identity.Provider
+
        DeprecatedWarningPageMethods
 }
 

diff --git a/resources/page/page_nop.go b/resources/page/page_nop.go
index c24792157ae443bb841dc06629d8e9b662277d89..293b399c7bbe1ef4bce3cf7bb9dec73bbc9c6491 100644 (file)
--- a/resources/page/page_nop.go
+++ b/resources/page/page_nop.go
@@ -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"
 
@@ -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
 }
@@ -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")
+}

diff --git a/resources/page/testhelpers_test.go b/resources/page/testhelpers_test.go
index dcd37c41e2d9f22515993150742aa415f7befd66..17a795a208cbee53b9d23483772e74603efc0480 100644 (file)
--- a/resources/page/testhelpers_test.go
+++ b/resources/page/testhelpers_test.go
@@ -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"
 
@@ -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")
 }
@@ -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)
 

diff --git a/tpl/fmt/fmt.go b/tpl/fmt/fmt.go
index aa6b8c1a636092ad3eb9903bf31d006bd3ff18e9..924d27a1d5714feec66dd8806797fc6d9d399be9 100644 (file)
--- a/tpl/fmt/fmt.go
+++ b/tpl/fmt/fmt.go
@@ -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.

diff --git a/tpl/tplimpl/template_funcs.go b/tpl/tplimpl/template_funcs.go
index a688abb7709a92b5cc2a92e4cf37c98613a56373..25ace365de280ddbf332875fccb24ce80495c487 100644 (file)
--- a/tpl/tplimpl/template_funcs.go
+++ b/tpl/tplimpl/template_funcs.go
@@ -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) {
@@ -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)
+                               }
                        }
                }
        }
@@ -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(