Fix missing page data for alternative formats

[ptgott]

When a template calls the .Translations function and a
Hugo environment is using multiple output formats,
a template that calls methods like .Summary and .Len on
each translation will unexpectedly show empty return
values for these methods.

This is because each pageOutput's ContentProvider is
assigned to a page.NopPage in newPageOutput. When
*HugoSites.render assigns pageContentOutputs to
pageOutputs in *pageState.shiftToOutputFormat, it
reuses pageContentOutputs from other pageOutputs,
leaving some pageContentOutputs as NopPages. While this
approach conserves resources, sometimes it means that
a template will unexpectedly call a method on a
pageContentOutput that is actually a NopPage.

In the case of ContentProvider methods called on
translations for alternative output formats, the methods
were called on NopPages.

This change introduces LazyContentProvider, which
performs late initialization when one of its methods is
called. This way, we can reuse content in "normal" cases
but ensure that ContentProvider methods work as expected
when a pageOutput is not assigned a pageContentOutput
during the initial pre-render phase.

Fixes #8919

[bep]

Thanks for this. I think I understand the problem. I think it would be good, however, if we could get one code path doing both of these.

• Either add the "Look for content to reuse" logic to your LazyContentProvider and then do a Reset on every shiftToOutputFormat
• Or somehow include both in shiftToOutputFormat

I would suspect that the first option would be best/simplest/easiest to understand ...

[ptgott]

@bep Thanks for the review. I've implemented the second solution and got the tests to pass.

Here are my notes on the first solution.

I tried making the following changes to my first commit:

diff --git a/hugolib/page.go b/hugolib/page.go
index f35865cf..59031f93 100644
--- a/hugolib/page.go
+++ b/hugolib/page.go
@@ -940,40 +940,16 @@ func (p *pageState) shiftToOutputFormat(isRenderingSite bool, idx int) error {
 		panic(fmt.Sprintf("pageOutput is nil for output idx %d", idx))
 	}
 
+	if z, ok := p.pageOutput.ContentProvider.(*page.LazyContentProvider); ok {
+		z.Reset()
+	}
+
 	// Reset any built paginator. This will trigger when re-rendering pages in
 	// server mode.
 	if isRenderingSite && p.pageOutput.paginator != nil && p.pageOutput.paginator.current != nil {
 		p.pageOutput.paginator.reset()
 	}
 
-	if isRenderingSite {
-		cp := p.pageOutput.cp
-		if cp == nil {
-			// Look for content to reuse.
-			for i := 0; i < len(p.pageOutputs); i++ {
-				if i == idx {
-					continue
-				}
-				po := p.pageOutputs[i]
-
-				if po.cp != nil && po.cp.reuse {
-					cp = po.cp
-					break
-				}
-			}
-		}
-
-		if cp == nil {
-			var err error
-			cp, err = newPageContentOutput(p, p.pageOutput)
-			if err != nil {
-				return err
-			}
-		}
-		p.pageOutput.initContentProvider(cp)
-		p.pageOutput.cp = cp
-	}
-
 	return nil
 }
 
diff --git a/hugolib/page__output.go b/hugolib/page__output.go
index 7f1140e7..7bb3089c 100644
--- a/hugolib/page__output.go
+++ b/hugolib/page__output.go
@@ -64,10 +64,29 @@ func newPageOutput(
 		// will be assigned. In the case of an unanticipated request, we lazily
 		// initialize a pageContentOutput.
 		ContentProvider: page.NewLazyContentProvider(func() (page.ContentProvider, error) {
-			cp, err := newPageContentOutput(ps, ps.pageOutput)
-			if err != nil {
-				return nil, err
+			cp := ps.pageOutput.cp
+			if cp == nil {
+				// Look for content to reuse.
+				for _, po := range ps.pageOutputs {
+					if po == ps.pageOutput {
+						continue
+					}
+
+					if po.cp != nil && po.cp.reuse {
+						cp = po.cp
+						break
+					}
+				}
+			}
+
+			if cp == nil {
+				var err error
+				cp, err = newPageContentOutput(ps, ps.pageOutput)
+				if err != nil {
+					return nil, err
+				}
 			}
+			ps.pageOutput.initContentProvider(cp)
 			return cp, nil
 		}),
 		TableOfContentsProvider: page.NopPage,
diff --git a/resources/page/page_lazy_contentprovider.go b/resources/page/page_lazy_contentprovider.go
index d8d92d7c..c1878a46 100644
--- a/resources/page/page_lazy_contentprovider.go
+++ b/resources/page/page_lazy_contentprovider.go
@@ -49,6 +49,12 @@ func NewLazyContentProvider(f func() (ContentProvider, error)) *LazyContentProvi
 	return &lcp
 }
 
+// Reset returns the LazyContentProvider to its pre-initialization state
+func (lcp *LazyContentProvider) Reset() {
+	lcp.init.Reset()
+	lcp.cp = NopPage
+}
+
 func (lcp *LazyContentProvider) Content() (interface{}, error) {
 	lcp.init.Do()
 	return lcp.cp.Content()

After this, TestRenderStringOnListPage fails with the output:

Running tool: /usr/local/bin/go test -timeout 30s -run ^TestRenderStringOnListPage$ github.com/gohugoio/hugo/hugolib

ERROR 2022/01/07 09:04:22 render of "section" failed: "layouts/_default/list.html:2:3": execute of template failed: template: _default/list.html:2:3: executing "_default/list.html" at <.RenderString>: error calling RenderString: runtime error: invalid memory address or nil pointer dereference
ERROR 2022/01/07 09:04:22 render of "taxonomy" failed: "layouts/_default/list.html:2:3": execute of template failed: template: _default/list.html:2:3: executing "_default/list.html" at <.RenderString>: error calling RenderString: runtime error: invalid memory address or nil pointer dereference
ERROR 2022/01/07 09:04:22 render of "taxonomy" failed: "layouts/_default/list.html:2:3": execute of template failed: template: _default/list.html:2:3: executing "_default/list.html" at <.RenderString>: error calling RenderString: runtime error: invalid memory address or nil pointer dereference
github.com/gohugoio/hugo/hugolib.(*Site).renderPages
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/site_render.go:98
github.com/gohugoio/hugo/hugolib.(*Site).render
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/site.go:1233
github.com/gohugoio/hugo/hugolib.(*HugoSites).render
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/hugo_sites_build.go:318
github.com/gohugoio/hugo/hugolib.(*HugoSites).Build.func4
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/hugo_sites_build.go:147
runtime/trace.WithRegion
	/usr/local/Cellar/go/1.17.2/libexec/src/runtime/trace/annotation.go:141
github.com/gohugoio/hugo/hugolib.(*HugoSites).Build
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/hugo_sites_build.go:149
github.com/gohugoio/hugo/hugolib.(*sitesBuilder).build
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/testhelpers_test.go:602
github.com/gohugoio/hugo/hugolib.(*sitesBuilder).Build
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/testhelpers_test.go:565
github.com/gohugoio/hugo/hugolib.TestRenderStringOnListPage
	/Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/content_render_hooks_test.go:475
testing.tRunner
	/usr/local/Cellar/go/1.17.2/libexec/src/testing/testing.go:1259
runtime.goexit
	/usr/local/Cellar/go/1.17.2/libexec/src/runtime/asm_amd64.s:1581
--- FAIL: TestRenderStringOnListPage (0.01s)
    /Users/paulgottschling/OneDrive/Programming/active-repos/hugo/hugolib/content_render_hooks_test.go:475: Build failed: failed to render pages: render of "home" failed: "layouts/index.html:2:3": execute of template failed: template: index.html:2:3: executing "index.html" at <.RenderString>: error calling RenderString: runtime error: invalid memory address or nil pointer dereference
FAIL
FAIL	github.com/gohugoio/hugo/hugolib	0.412s
FAIL

However, if we make this change to the test, it passes:

diff --git a/hugolib/content_render_hooks_test.go b/hugolib/content_render_hooks_test.go
index f1c27d51..4b0ed3d3 100644
--- a/hugolib/content_render_hooks_test.go
+++ b/hugolib/content_render_hooks_test.go
@@ -462,6 +462,7 @@ RSTART:Hook Heading: 2:REND
 // https://github.com/gohugoio/hugo/issues/6882
 func TestRenderStringOnListPage(t *testing.T) {
 	renderStringTempl := `
+{{ .Summary }}
 {{ .RenderString "**Hello**" }}
 `
 	b := newTestSitesBuilder(t)

It turns out that if we add the content reuse logic to LazyContentProvider, and a template does not call any of LazyContentProvider's methods, cp is nil when we get to this p.pageOutput.cp.renderContentWithConverter(conv, []byte(s), false) call.

[bep]

@ptgott I will pull this branch and see if I understand it.

[bep]

OK; I had a look -- I think we can safely ignore my suggestion earlier. I added a commit to this PR which adjusts the logic slightly to save some mem allocs -- the test still passes and it looks correct to me. Let me know if you agree and I'll merge.

[ptgott]

@bep Looks good to me, thanks!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.