shithub: hugo

Info  •  Files  •  Log  •  Branches

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

--- a/deps/deps.go

+++ b/deps/deps.go

@@ -97,6 +97,9 @@

 	// This is common/global for all sites.

 	BuildState *BuildState

+	// Whether we are in running (server) mode

+	Running bool

+

 	*globalErrHandler

 }

@@ -279,6 +282,7 @@

 		FileCaches:              fileCaches,

 		BuildStartListeners:     &Listeners{},

 		BuildState:              buildState,

+		Running:                 cfg.Running,

 		Timeout:                 time.Duration(timeoutms) * time.Millisecond,

 		globalErrHandler:        errorHandler,

 	}

--- a/hugolib/content_map.go

+++ b/hugolib/content_map.go

@@ -95,21 +95,23 @@

 	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,10 +1032,19 @@

 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 {

--- a/hugolib/hugo_sites.go

+++ b/hugolib/hugo_sites.go

@@ -997,7 +997,6 @@

 		}

 		return false

 	})

-

 }

 // Used in partial reloading to determine if the change is in a bundle.

--- a/hugolib/hugo_sites_rebuild_test.go

+++ b/hugolib/hugo_sites_rebuild_test.go

@@ -259,3 +259,66 @@

 	})

 }

+

+// 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

+`)

+

+}

--- a/hugolib/page.go

+++ b/hugolib/page.go

@@ -78,6 +78,7 @@

 	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 @@

 	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 {

@@ -125,6 +138,10 @@

 	}

 	return p == pp

+}

+

+func (p *pageState) GetIdentity() identity.Identity {

+	return identity.NewPathIdentity(files.ComponentFolderContent, filepath.FromSlash(p.Path()))

 }

 func (p *pageState) GitInfo() *gitmap.GitInfo {

--- a/hugolib/site.go

+++ b/hugolib/site.go

@@ -1538,6 +1538,7 @@

 	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{}

@@ -1584,6 +1585,18 @@

 		p = page.NilPage

 	}

+	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

 }

--- a/hugolib/template_test.go

+++ b/hugolib/template_test.go

@@ -599,7 +599,7 @@

 	idset := make(map[identity.Identity]bool)

 	collectIdentities(idset, templ.(tpl.Info))

-	b.Assert(idset, qt.HasLen, 10)

+	b.Assert(idset, qt.HasLen, 11)

 }

--- a/identity/identity.go

+++ b/identity/identity.go

@@ -129,6 +129,8 @@

 	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()

--- 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"

@@ -27,6 +30,54 @@

 	c.Assert(im.Search(id1).GetIdentity(), qt.Equals, id1)

 	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 {

--- a/resources/page/page.go

+++ b/resources/page/page.go

@@ -18,8 +18,11 @@

 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 @@

 	// 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.

@@ -259,6 +265,9 @@

 	// GetTerms gets the terms of a given taxonomy,

 	// e.g. GetTerms("categories")

 	GetTerms(taxonomy string) Pages

+

+	// Used in change/dependency tracking.

+	identity.Provider

 	DeprecatedWarningPageMethods

 }

--- a/resources/page/page_nop.go

+++ b/resources/page/page_nop.go

@@ -19,7 +19,10 @@

 	"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 @@

 	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

 }

@@ -483,4 +490,8 @@

 func (p *nopPage) WordCount() int {

 	return 0

+}

+

+func (p *nopPage) GetIdentity() identity.Identity {

+	return identity.NewPathIdentity("content", "foo/bar.md")

 }

--- a/resources/page/testhelpers_test.go

+++ b/resources/page/testhelpers_test.go

@@ -20,6 +20,8 @@

 	"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 @@

 	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")

 }

@@ -562,6 +568,10 @@

 }

 func (p *testPage) WordCount() int {

+	panic("not implemented")

+}

+

+func (p *testPage) GetIdentity() identity.Identity {

 	panic("not implemented")

 }

--- a/tpl/fmt/fmt.go

+++ b/tpl/fmt/fmt.go

@@ -23,10 +23,17 @@

 // 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.

--- a/tpl/tplimpl/template_funcs.go

+++ b/tpl/tplimpl/template_funcs.go

@@ -64,7 +64,8 @@

 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) 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 @@

 	}

 	exeHelper := &templateExecHelper{

-		funcs: funcsv,

+		running: d.Running,

+		funcs:   funcsv,

 	}

 	return texttemplate.NewExecuter(