From 0efb00c2a86ec3f52000a643f26f54bb2a9dfbd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Mon, 2 Dec 2019 21:10:27 +0100 Subject: [PATCH] tpl/partials: Allow any key type in partialCached Fixes #6572 --- helpers/general.go | 20 +++++++++ helpers/general_test.go | 7 +++ hugolib/template_test.go | 23 ++++++++++ resources/image.go | 6 +-- resources/images/filters_test.go | 10 ++--- resources/internal/key.go | 25 +---------- resources/internal/key_test.go | 7 --- tpl/partials/partials.go | 76 +++++++++++++++++++++++++------- tpl/partials/partials_test.go | 41 +++++++++++++++++ 9 files changed, 160 insertions(+), 55 deletions(-) create mode 100644 tpl/partials/partials_test.go diff --git a/helpers/general.go b/helpers/general.go index 699ddeb53..aa1e00d3a 100644 --- a/helpers/general.go +++ b/helpers/general.go @@ -23,11 +23,14 @@ import ( "os" "path/filepath" "sort" + "strconv" "strings" "sync" "unicode" "unicode/utf8" + "github.com/mitchellh/hashstructure" + "github.com/gohugoio/hugo/hugofs" "github.com/gohugoio/hugo/common/hugo" @@ -482,3 +485,20 @@ func PrintFs(fs afero.Fs, path string, w io.Writer) { return nil }) } + +// HashString returns a hash from the given elements. +// It will panic if the hash cannot be calculated. +func HashString(elements ...interface{}) string { + var o interface{} + if len(elements) == 1 { + o = elements[0] + } else { + o = elements + } + + hash, err := hashstructure.Hash(o, nil) + if err != nil { + panic(err) + } + return strconv.FormatUint(hash, 10) +} diff --git a/helpers/general_test.go b/helpers/general_test.go index b8a98fb69..104a4c35d 100644 --- a/helpers/general_test.go +++ b/helpers/general_test.go @@ -408,3 +408,10 @@ func BenchmarkUniqueStrings(b *testing.B) { }) } + +func TestHashString(t *testing.T) { + c := qt.New(t) + + c.Assert(HashString("a", "b"), qt.Equals, "2712570657419664240") + c.Assert(HashString("ab"), qt.Equals, "590647783936702392") +} diff --git a/hugolib/template_test.go b/hugolib/template_test.go index de93f1c80..71b4b46c0 100644 --- a/hugolib/template_test.go +++ b/hugolib/template_test.go @@ -308,3 +308,26 @@ complex: 80: {{ partial "complex.tpl" 38 }} ) } + +func TestPartialCached(t *testing.T) { + b := newTestSitesBuilder(t) + + b.WithTemplatesAdded( + "index.html", ` +{{ $key1 := (dict "a" "av" ) }} +{{ $key2 := (dict "a" "av2" ) }} +Partial cached1: {{ partialCached "p1" "input1" $key1 }} +Partial cached2: {{ partialCached "p1" "input2" $key1 }} +Partial cached3: {{ partialCached "p1" "input3" $key2 }} +`, + "partials/p1.html", `partial: {{ . }}`, + ) + + b.Build(BuildCfg{}) + + b.AssertFileContent("public/index.html", ` + Partial cached1: partial: input1 + Partial cached2: partial: input1 + Partial cached3: partial: input3 +`) +} diff --git a/resources/image.go b/resources/image.go index cdea8a2a7..076f2ae4d 100644 --- a/resources/image.go +++ b/resources/image.go @@ -34,8 +34,6 @@ import ( "github.com/gohugoio/hugo/cache/filecache" "github.com/gohugoio/hugo/resources/images/exif" - "github.com/gohugoio/hugo/resources/internal" - "github.com/gohugoio/hugo/resources/resource" _errors "github.com/pkg/errors" @@ -218,7 +216,7 @@ func (i *imageResource) Filter(filters ...interface{}) (resource.Image, error) { gfilters = append(gfilters, images.ToFilters(f)...) } - conf.Key = internal.HashString(gfilters) + conf.Key = helpers.HashString(gfilters) conf.TargetFormat = i.Format return i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) { @@ -362,7 +360,7 @@ func (i *imageResource) getImageMetaCacheTargetPath() string { } p1, _ := helpers.FileAndExt(df.file) h, _ := i.hash() - idStr := internal.HashString(h, i.size(), imageMetaVersionNumber, cfg) + idStr := helpers.HashString(h, i.size(), imageMetaVersionNumber, cfg) return path.Join(df.dir, fmt.Sprintf("%s_%s.json", p1, idStr)) } diff --git a/resources/images/filters_test.go b/resources/images/filters_test.go index 658a9a427..1243e483b 100644 --- a/resources/images/filters_test.go +++ b/resources/images/filters_test.go @@ -16,7 +16,7 @@ package images import ( "testing" - "github.com/gohugoio/hugo/resources/internal" + "github.com/gohugoio/hugo/helpers" qt "github.com/frankban/quicktest" ) @@ -26,9 +26,9 @@ func TestFilterHash(t *testing.T) { f := &Filters{} - c.Assert(internal.HashString(f.Grayscale()), qt.Equals, internal.HashString(f.Grayscale())) - c.Assert(internal.HashString(f.Grayscale()), qt.Not(qt.Equals), internal.HashString(f.Invert())) - c.Assert(internal.HashString(f.Gamma(32)), qt.Not(qt.Equals), internal.HashString(f.Gamma(33))) - c.Assert(internal.HashString(f.Gamma(32)), qt.Equals, internal.HashString(f.Gamma(32))) + c.Assert(helpers.HashString(f.Grayscale()), qt.Equals, helpers.HashString(f.Grayscale())) + c.Assert(helpers.HashString(f.Grayscale()), qt.Not(qt.Equals), helpers.HashString(f.Invert())) + c.Assert(helpers.HashString(f.Gamma(32)), qt.Not(qt.Equals), helpers.HashString(f.Gamma(33))) + c.Assert(helpers.HashString(f.Gamma(32)), qt.Equals, helpers.HashString(f.Gamma(32))) } diff --git a/resources/internal/key.go b/resources/internal/key.go index 17543b0d4..d67d4a7e1 100644 --- a/resources/internal/key.go +++ b/resources/internal/key.go @@ -13,11 +13,7 @@ package internal -import ( - "strconv" - - "github.com/mitchellh/hashstructure" -) +import "github.com/gohugoio/hugo/helpers" // ResourceTransformationKey are provided by the different transformation implementations. // It identifies the transformation (name) and its configuration (elements). @@ -42,23 +38,6 @@ func (k ResourceTransformationKey) Value() string { return k.Name } - return k.Name + "_" + HashString(k.elements...) + return k.Name + "_" + helpers.HashString(k.elements...) } - -// HashString returns a hash from the given elements. -// It will panic if the hash cannot be calculated. -func HashString(elements ...interface{}) string { - var o interface{} - if len(elements) == 1 { - o = elements[0] - } else { - o = elements - } - - hash, err := hashstructure.Hash(o, nil) - if err != nil { - panic(err) - } - return strconv.FormatUint(hash, 10) -} diff --git a/resources/internal/key_test.go b/resources/internal/key_test.go index 11a52f2e6..38286333d 100644 --- a/resources/internal/key_test.go +++ b/resources/internal/key_test.go @@ -34,10 +34,3 @@ func TestResourceTransformationKey(t *testing.T) { c := qt.New(t) c.Assert(key.Value(), qt.Equals, "testing_518996646957295636") } - -func TestHashString(t *testing.T) { - c := qt.New(t) - - c.Assert(HashString("a", "b"), qt.Equals, "2712570657419664240") - c.Assert(HashString("ab"), qt.Equals, "590647783936702392") -} diff --git a/tpl/partials/partials.go b/tpl/partials/partials.go index 2599a5d01..da42acb20 100644 --- a/tpl/partials/partials.go +++ b/tpl/partials/partials.go @@ -16,14 +16,18 @@ package partials import ( + "errors" "fmt" "html/template" "io" "io/ioutil" + "reflect" "strings" "sync" texttemplate "text/template" + "github.com/gohugoio/hugo/helpers" + "github.com/gohugoio/hugo/tpl" bp "github.com/gohugoio/hugo/bufferpool" @@ -34,21 +38,26 @@ import ( // NOTE: It's currently unused. var TestTemplateProvider deps.ResourceProvider +type partialCacheKey struct { + name string + variant interface{} +} + // partialCache represents a cache of partials protected by a mutex. type partialCache struct { sync.RWMutex - p map[string]interface{} + p map[partialCacheKey]interface{} } func (p *partialCache) clear() { p.Lock() defer p.Unlock() - p.p = make(map[string]interface{}) + p.p = make(map[partialCacheKey]interface{}) } // New returns a new instance of the templates-namespaced template functions. func New(deps *deps.Deps) *Namespace { - cache := &partialCache{p: make(map[string]interface{})} + cache := &partialCache{p: make(map[partialCacheKey]interface{})} deps.BuildStartListeners.Add( func() { cache.clear() @@ -151,21 +160,56 @@ func (ns *Namespace) Include(name string, contextList ...interface{}) (interface } -// IncludeCached executes and caches partial templates. An optional variant -// string parameter (a string slice actually, but be only use a variadic -// argument to make it optional) can be passed so that a given partial can have -// multiple uses. The cache is created with name+variant as the key. -func (ns *Namespace) IncludeCached(name string, context interface{}, variant ...string) (interface{}, error) { - key := name - if len(variant) > 0 { - for i := 0; i < len(variant); i++ { - key += variant[i] - } +// IncludeCached executes and caches partial templates. The cache is created with name+variants as the key. +func (ns *Namespace) IncludeCached(name string, context interface{}, variants ...interface{}) (interface{}, error) { + key, err := createKey(name, variants...) + if err != nil { + return nil, err } - return ns.getOrCreate(key, name, context) + + result, err := ns.getOrCreate(key, context) + if err == errUnHashable { + // Try one more + key.variant = helpers.HashString(key.variant) + result, err = ns.getOrCreate(key, context) + } + + return result, err } -func (ns *Namespace) getOrCreate(key, name string, context interface{}) (interface{}, error) { +func createKey(name string, variants ...interface{}) (partialCacheKey, error) { + var variant interface{} + + if len(variants) > 1 { + variant = helpers.HashString(variants...) + } else if len(variants) == 1 { + variant = variants[0] + t := reflect.TypeOf(variant) + switch t.Kind() { + // This isn't an exhaustive list of unhashable types. + // There may be structs with slices, + // but that should be very rare. We do recover from that situation + // below. + case reflect.Slice, reflect.Array, reflect.Map: + variant = helpers.HashString(variant) + } + } + + return partialCacheKey{name: name, variant: variant}, nil +} + +var errUnHashable = errors.New("unhashable") + +func (ns *Namespace) getOrCreate(key partialCacheKey, context interface{}) (result interface{}, err error) { + defer func() { + if r := recover(); r != nil { + err = r.(error) + if strings.Contains(err.Error(), "unhashable type") { + ns.cachedPartials.RUnlock() + err = errUnHashable + } + } + }() ns.cachedPartials.RLock() p, ok := ns.cachedPartials.p[key] @@ -175,7 +219,7 @@ func (ns *Namespace) getOrCreate(key, name string, context interface{}) (interfa return p, nil } - p, err := ns.Include(name, context) + p, err = ns.Include(key.name, context) if err != nil { return nil, err } diff --git a/tpl/partials/partials_test.go b/tpl/partials/partials_test.go new file mode 100644 index 000000000..60e9dd721 --- /dev/null +++ b/tpl/partials/partials_test.go @@ -0,0 +1,41 @@ +// 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 partials + +import ( + "testing" + + qt "github.com/frankban/quicktest" +) + +func TestCreateKey(t *testing.T) { + c := qt.New(t) + m := make(map[interface{}]bool) + + create := func(name string, variants ...interface{}) partialCacheKey { + k, err := createKey(name, variants...) + c.Assert(err, qt.IsNil) + m[k] = true + return k + } + + for i := 0; i < 123; i++ { + c.Assert(create("a", "b"), qt.Equals, partialCacheKey{name: "a", variant: "b"}) + c.Assert(create("a", "b", "c"), qt.Equals, partialCacheKey{name: "a", variant: "9629524865311698396"}) + c.Assert(create("a", 1), qt.Equals, partialCacheKey{name: "a", variant: 1}) + c.Assert(create("a", map[string]string{"a": "av"}), qt.Equals, partialCacheKey{name: "a", variant: "4809626101226749924"}) + c.Assert(create("a", []string{"a", "b"}), qt.Equals, partialCacheKey{name: "a", variant: "2712570657419664240"}) + } + +}