From ab8ed9f190a50376dbff4263dda288bbfcf6d6fa Mon Sep 17 00:00:00 2001 From: Paul Gottschling Date: Fri, 14 Jan 2022 16:08:11 -0500 Subject: [PATCH] Ensure page translation CPs are initialized PR #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 #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: - cdcd15b6c2abbb76fd95fbbf90365c56e82f46aa - 25d645f47ae0a32470d303c9478c9e0b2fff0f0e Fixes #9383 --- hugolib/page.go | 56 ++++++++---- hugolib/page_test.go | 63 +++++++++++++ resources/page/page_lazy_contentprovider.go | 99 --------------------- 3 files changed, 101 insertions(+), 117 deletions(-) delete mode 100644 resources/page/page_lazy_contentprovider.go diff --git a/hugolib/page.go b/hugolib/page.go index 286d210752c..dd8d9445594 100644 --- a/hugolib/page.go +++ b/hugolib/page.go @@ -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 } @@ -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 diff --git a/hugolib/page_test.go b/hugolib/page_test.go index fc01bbf25eb..48a81ee4aa0 100644 --- a/hugolib/page_test.go +++ b/hugolib/page_test.go @@ -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 -}} +

{{- .RenderString "foo" -}}

+{{- end -}} +{{- range .Site.Home.AllTranslations -}} +

{{- .RenderString "bar" -}}

+{{- 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", ` +

foo

+

foo

+

bar

+

bar

+`) + + b.AssertFileContent("public/en/index.html", ` +

foo

+

foo

+

bar

+

bar

+`) + +} + // Issue 8919 func TestContentProviderWithCustomOutputFormat(t *testing.T) { b := newTestSitesBuilder(t) diff --git a/resources/page/page_lazy_contentprovider.go b/resources/page/page_lazy_contentprovider.go deleted file mode 100644 index a513a063a16..00000000000 --- a/resources/page/page_lazy_contentprovider.go +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2019 The Hugo Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package page - -import ( - "html/template" - - "github.com/gohugoio/hugo/lazy" -) - -// LazyContentProvider initializes itself when read. Each method of the -// ContentProvider interface initializes a content provider and shares it -// with other methods. -// -// Used in cases where we cannot guarantee whether the content provider -// will be needed. Must create via NewLazyContentProvider. -type LazyContentProvider struct { - init *lazy.Init - cp ContentProvider -} - -// NewLazyContentProvider returns a LazyContentProvider initialized with -// function f. The resulting LazyContentProvider calls f in order to -// retrieve a ContentProvider -func NewLazyContentProvider(f func() (ContentProvider, error)) *LazyContentProvider { - lcp := LazyContentProvider{ - init: lazy.New(), - cp: NopPage, - } - lcp.init.Add(func() (interface{}, error) { - cp, err := f() - if err != nil { - return nil, err - } - lcp.cp = cp - return nil, nil - }) - return &lcp -} - -func (lcp *LazyContentProvider) Reset() { - lcp.init.Reset() -} - -func (lcp *LazyContentProvider) Content() (interface{}, error) { - lcp.init.Do() - return lcp.cp.Content() -} - -func (lcp *LazyContentProvider) Plain() string { - lcp.init.Do() - return lcp.cp.Plain() -} - -func (lcp *LazyContentProvider) PlainWords() []string { - lcp.init.Do() - return lcp.cp.PlainWords() -} - -func (lcp *LazyContentProvider) Summary() template.HTML { - lcp.init.Do() - return lcp.cp.Summary() -} - -func (lcp *LazyContentProvider) Truncated() bool { - lcp.init.Do() - return lcp.cp.Truncated() -} - -func (lcp *LazyContentProvider) FuzzyWordCount() int { - lcp.init.Do() - return lcp.cp.FuzzyWordCount() -} - -func (lcp *LazyContentProvider) WordCount() int { - lcp.init.Do() - return lcp.cp.WordCount() -} - -func (lcp *LazyContentProvider) ReadingTime() int { - lcp.init.Do() - return lcp.cp.ReadingTime() -} - -func (lcp *LazyContentProvider) Len() int { - lcp.init.Do() - return lcp.cp.Len() -}