diff options
author | Bjørn Erik Pedersen <[email protected]> | 2020-09-07 15:07:10 +0200 |
---|---|---|
committer | Bjørn Erik Pedersen <[email protected]> | 2020-09-07 21:06:44 +0200 |
commit | 4055c121847847d8bd6b95a928185daee065091b (patch) | |
tree | 6620f51e7e89aa7ff0a9a93361d640ee5b297fea | |
parent | 3ba7c92530a80f2f04fe57705ab05c247a6e8437 (diff) | |
download | hugo-4055c121847847d8bd6b95a928185daee065091b.tar.gz hugo-4055c121847847d8bd6b95a928185daee065091b.zip |
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
-rw-r--r-- | deps/deps.go | 4 | ||||
-rw-r--r-- | hugolib/content_map.go | 41 | ||||
-rw-r--r-- | hugolib/hugo_sites.go | 1 | ||||
-rw-r--r-- | hugolib/hugo_sites_rebuild_test.go | 63 | ||||
-rw-r--r-- | hugolib/page.go | 17 | ||||
-rw-r--r-- | hugolib/site.go | 13 | ||||
-rw-r--r-- | hugolib/template_test.go | 2 | ||||
-rw-r--r-- | identity/identity.go | 2 | ||||
-rw-r--r-- | identity/identity_test.go | 51 | ||||
-rw-r--r-- | resources/page/page.go | 9 | ||||
-rw-r--r-- | resources/page/page_nop.go | 11 | ||||
-rw-r--r-- | resources/page/testhelpers_test.go | 10 | ||||
-rw-r--r-- | tpl/fmt/fmt.go | 9 | ||||
-rw-r--r-- | tpl/tplimpl/template_funcs.go | 23 |
14 files changed, 228 insertions, 28 deletions
diff --git a/deps/deps.go b/deps/deps.go index 82a16ba59..07fe2fc7d 100644 --- 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 43ad7745d..33ef4f8dd 100644 --- 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 d1e3a146d..702e51abb 100644 --- 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 f0c9f8f09..4c4741385 100644 --- 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 28ef1e156..ca93bf18b 100644 --- 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 ac65931d0..dad5ab538 100644 --- 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 6f37b4164..39f1b9e2e 100644 --- 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 ac3558d16..8fce16479 100644 --- 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 adebcad91..c315df1e7 100644 --- 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 de417178f..0b402c4e7 100644 --- 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 c24792157..293b399c7 100644 --- 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 dcd37c41e..17a795a20 100644 --- 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 aa6b8c1a6..924d27a1d 100644 --- 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 a688abb77..25ace365d 100644 --- 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( |