Skip to content

Commit

Permalink
Ensure page translation CPs are initialized
Browse files Browse the repository at this point in the history
PR gohugoio#9342 introduced a regression in which calling .Translations in a
template and calling RenderString on a translated Page caused a nil
pointer dereference. The issue was that some Pages returned from
.Translations had a nil cp field at the time the calling template was
being executed.

While PR gohugoio#9342 had attempted to ensure that all ContentProviders were
initialized for translations at build time, it only performed the
initialization for receivers of ContentProvider methods such as
.Summary. However, the ContentProvider's *pageState.pageOutput.cp would
remain uninitialized, causing the nil pointer dereference.

This change edits the .Translations and .AllTranslations methods to
ensure that all of a page's translations have an initialized
content provider in time for a template to be executed.

Since LazyContentProvider is no longer needed with this approach, this
change also reverts the following commits:

- cdcd15b
- 25d645f

Fixes gohugoio#9383
  • Loading branch information
ptgott committed Jan 16, 2022
1 parent 348d300 commit ab8ed9f
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 117 deletions.
56 changes: 38 additions & 18 deletions hugolib/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,50 @@ func (p *pageState) TranslationKey() string {
// AllTranslations returns all translations, including the current Page.
func (p *pageState) AllTranslations() page.Pages {
p.s.h.init.translations.Do()

// Hugo attempts to reuse content providers while preparing each page for
// rendering. This approach means that sometimes, content providers for
// translations are not initialized. If needed, initialize them here.
for _, tr := range p.allTranslations {
tp, ok := tr.(*pageState)
if !ok {
continue
}
if tp.pageOutput.cp == nil {
cp, err := newPageContentOutput(tp, tp.pageOutput)
if err != nil {
// This will be caught by texttemplate.safeCall
panic(fmt.Errorf("error rendering page translation: %w", err))
}
tp.initContentProvider(cp)
}
}

return p.allTranslations
}

// Translations returns the translations excluding the current Page.
func (p *pageState) Translations() page.Pages {
p.s.h.init.translations.Do()

// Hugo attempts to reuse content providers while preparing each page for
// rendering. This approach means that sometimes, content providers for
// translations are not initialized. If needed, initialize them here.
for _, tr := range p.translations {
tp, ok := tr.(*pageState)
if !ok {
continue
}
if tp.pageOutput.cp == nil {
cp, err := newPageContentOutput(tp, tp.pageOutput)
if err != nil {
// This will be caught by texttemplate.safeCall
panic(fmt.Errorf("error rendering page translation: %w", err))
}
tp.initContentProvider(cp)
}
}

return p.translations
}

Expand Down Expand Up @@ -971,24 +1009,6 @@ func (p *pageState) shiftToOutputFormat(isRenderingSite bool, idx int) error {
}
}
p.pageOutput.initContentProvider(cp)
} else {
// We attempt to assign pageContentOutputs while preparing each site
// for rendering and before rendering each site. This lets us share
// content between page outputs to conserve resources. But if a template
// unexpectedly calls a method of a ContentProvider that is not yet
// initialized, we assign a LazyContentProvider that performs the
// initialization just in time.
if lcp, ok := (p.pageOutput.ContentProvider.(*page.LazyContentProvider)); ok {
lcp.Reset()
} else {
p.pageOutput.ContentProvider = page.NewLazyContentProvider(func() (page.ContentProvider, error) {
cp, err := newPageContentOutput(p, p.pageOutput)
if err != nil {
return nil, err
}
return cp, nil
})
}
}

return nil
Expand Down
63 changes: 63 additions & 0 deletions hugolib/page_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,69 @@ Here is the last report for commits in the year 2016. It covers hrev50718-hrev50
`)
}

// Issue 9383
func TestRenderStringForRegularPageTranslations(t *testing.T) {
c := qt.New(t)
b := newTestSitesBuilder(t)
b.WithLogger(loggers.NewBasicLoggerForWriter(jwalterweatherman.LevelDebug, os.Stderr))

b.WithConfigFile("toml",
`baseurl = "https://example.org/"
title = "My Site"
defaultContentLanguage = "ru"
defaultContentLanguageInSubdir = true
[languages.ru]
contentDir = 'content/ru'
weight = 1
[languages.en]
weight = 2
contentDir = 'content/en'
[outputs]
home = ["HTML", "JSON"]`)

b.WithTemplates("index.html", `
{{- range .Site.Home.Translations -}}
<p>{{- .RenderString "foo" -}}</p>
{{- end -}}
{{- range .Site.Home.AllTranslations -}}
<p>{{- .RenderString "bar" -}}</p>
{{- end -}}
`, "_default/single.html",
`{{ .Content }}`,
"index.json",
`{"Title": "My Site"}`,
)

b.WithContent(
"ru/a.md",
"",
"en/a.md",
"",
)

err := b.BuildE(BuildCfg{})
c.Assert(err, qt.Equals, nil)

b.AssertFileContent("public/ru/index.html", `
<p>foo</p>
<p>foo</p>
<p>bar</p>
<p>bar</p>
`)

b.AssertFileContent("public/en/index.html", `
<p>foo</p>
<p>foo</p>
<p>bar</p>
<p>bar</p>
`)

}

// Issue 8919
func TestContentProviderWithCustomOutputFormat(t *testing.T) {
b := newTestSitesBuilder(t)
Expand Down
99 changes: 0 additions & 99 deletions resources/page/page_lazy_contentprovider.go

This file was deleted.

0 comments on commit ab8ed9f

Please sign in to comment.