tpl/partials: Allow any key type in partialCached

Fixes #6572

9 files changed, 159 insertions, 54 deletions

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...)
-
-}
-
-// 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
-	}
+	return k.Name + "_" + helpers.HashString(k.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
+	}
+
+	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 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 ns.getOrCreate(key, name, context)
+
+	return partialCacheKey{name: name, variant: variant}, nil
 }
 
-func (ns *Namespace) getOrCreate(key, name string, context interface{}) (interface{}, error) {
+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"})
+	}
+
+}